diff --git a/keystone/common/fernet_utils.py b/keystone/common/fernet_utils.py index 63c82840c8..4eddff52fd 100644 --- a/keystone/common/fernet_utils.py +++ b/keystone/common/fernet_utils.py @@ -100,6 +100,17 @@ class FernetUtils(object): Create a new key that is readable by the Keystone group and Keystone user. + + To avoid disk write failure, this function will create a tmp key file + first, and then rename it as the valid new key. + """ + self._create_tmp_new_key(keystone_user_id, keystone_group_id) + self._become_valid_new_key() + + def _create_tmp_new_key(self, keystone_user_id, keystone_group_id): + """Securely create a new tmp encryption key. + + This created key is not effective until _become_valid_new_key(). """ key = fernet.Fernet.generate_key() # key is bytes @@ -117,11 +128,17 @@ class FernetUtils(object): '%s') % self.key_repository) # Determine the file name of the new key - key_file = os.path.join(self.key_repository, '0') + key_file = os.path.join(self.key_repository, '0.tmp') + create_success = False try: with open(key_file, 'w') as f: # convert key to str for the file. f.write(key.decode('utf-8')) + f.flush() + create_success = True + except IOError: + LOG.error(_LE('Failed to create new temporary key: %s'), key_file) + raise finally: # After writing the key, set the umask back to it's original value. # Do the same with group and user identifiers if a Keystone group @@ -130,8 +147,23 @@ class FernetUtils(object): if keystone_user_id and keystone_group_id: os.seteuid(old_euid) os.setegid(old_egid) + # Deal with the tmp key file + if not create_success and os.access(key_file, os.F_OK): + os.remove(key_file) - LOG.info(_LI('Created a new key: %s'), key_file) + LOG.info(_LI('Created a new temporary key: %s'), key_file) + + def _become_valid_new_key(self): + """Make the tmp new key a valid new key. + + The tmp new key must be created by _create_tmp_new_key(). + """ + tmp_key_file = os.path.join(self.key_repository, '0.tmp') + valid_key_file = os.path.join(self.key_repository, '0') + + os.rename(tmp_key_file, valid_key_file) + + LOG.info(_LI('Become a valid new key: %s'), valid_key_file) def initialize_key_repository(self, keystone_user_id=None, keystone_group_id=None): @@ -190,6 +222,9 @@ class FernetUtils(object): 'count': len(key_files), 'list': list(key_files.values())}) + # add a tmp new key to the rotation, which will be the *next* primary + self._create_tmp_new_key(keystone_user_id, keystone_group_id) + # determine the number of the new primary key current_primary_key = max(key_files.keys()) LOG.info(_LI('Current primary key is: %s'), current_primary_key) @@ -207,8 +242,8 @@ class FernetUtils(object): str(new_primary_key)) LOG.info(_LI('Promoted key 0 to be the primary: %s'), new_primary_key) - # add a new key to the rotation, which will be the *next* primary - self._create_new_key(keystone_user_id, keystone_group_id) + # rename the tmp key to the real staged key + self._become_valid_new_key() max_active_keys = self.max_active_keys diff --git a/keystone/tests/unit/token/test_fernet_provider.py b/keystone/tests/unit/token/test_fernet_provider.py index a9db6d3089..3aa9671ea6 100644 --- a/keystone/tests/unit/token/test_fernet_provider.py +++ b/keystone/tests/unit/token/test_fernet_provider.py @@ -13,6 +13,7 @@ import base64 import datetime import hashlib +import mock import os import uuid @@ -627,6 +628,35 @@ class TestFernetKeyRotation(unit.TestCase): next_key_number += 1 self.assertEqual(exp_keys, self.keys) + def test_rotation_disk_write_fail(self): + # Init the key repository + self.useFixture( + ksfixtures.KeyRepository( + self.config_fixture, + 'fernet_tokens', + CONF.fernet_tokens.max_active_keys + ) + ) + + # Make sure that the init key repository contains 2 keys + self.assertRepositoryState(expected_size=2) + + key_utils = fernet_utils.FernetUtils( + CONF.fernet_tokens.key_repository, + CONF.fernet_tokens.max_active_keys + ) + + # Simulate the disk full situation + mock_open = mock.mock_open() + file_handle = mock_open() + file_handle.flush.side_effect = IOError('disk full') + + with mock.patch('keystone.common.fernet_utils.open', mock_open): + self.assertRaises(IOError, key_utils.rotate_keys) + + # Assert that the key repository is unchanged + self.assertEqual(self.key_repository_size, 2) + def test_non_numeric_files(self): self.useFixture( ksfixtures.KeyRepository( diff --git a/releasenotes/notes/bug-1642457-4533f9810a8cd927.yaml b/releasenotes/notes/bug-1642457-4533f9810a8cd927.yaml new file mode 100644 index 0000000000..4d1d5ff954 --- /dev/null +++ b/releasenotes/notes/bug-1642457-4533f9810a8cd927.yaml @@ -0,0 +1,7 @@ +--- +fixes: + - | + [`bug 1642457 `_] + Handle disk write and IO failures when rotating keys for Fernet tokens. + Rather than creating empty keys, properly catch and log errors when + unable to write to disk.