From bc95434472951a2ecb62f09ee6fd64d4e1253c55 Mon Sep 17 00:00:00 2001 From: Werner Mendizabal Date: Wed, 10 Aug 2016 21:13:19 +0000 Subject: [PATCH] Make a FernetUtils class By converting our module-level fernet utilities to a class, we can extend it or modify it in ways to make the utilities work for outside the fernet token provider. This change is in preparation to use fernet to encrypt credentials at rest. bp credential-encryption Change-Id: Ia4e9fd2b8597993f006d9fea82b782085f2cdbc2 --- keystone/cmd/cli.py | 16 +- keystone/cmd/doctor/tokens_fernet.py | 4 +- keystone/common/fernet_utils.py | 422 +++++++++--------- .../tests/unit/ksfixtures/key_repository.py | 5 +- .../tests/unit/token/test_fernet_provider.py | 14 +- .../providers/fernet/token_formatters.py | 3 +- 6 files changed, 240 insertions(+), 224 deletions(-) diff --git a/keystone/cmd/cli.py b/keystone/cmd/cli.py index 2f9998368c..ad397ddb69 100644 --- a/keystone/cmd/cli.py +++ b/keystone/cmd/cli.py @@ -524,12 +524,13 @@ class FernetSetup(BasePermissionsSetup): @classmethod def main(cls): - from keystone.common import fernet_utils as fernet + from keystone.common import fernet_utils as utils + fernet_utils = utils.FernetUtils() keystone_user_id, keystone_group_id = cls.get_user_group() - fernet.create_key_directory(keystone_user_id, keystone_group_id) - if fernet.validate_key_repository(requires_write=True): - fernet.initialize_key_repository( + fernet_utils.create_key_directory(keystone_user_id, keystone_group_id) + if fernet_utils.validate_key_repository(requires_write=True): + fernet_utils.initialize_key_repository( keystone_user_id, keystone_group_id) @@ -555,11 +556,12 @@ class FernetRotate(BasePermissionsSetup): @classmethod def main(cls): - from keystone.common import fernet_utils as fernet + from keystone.common import fernet_utils as utils + fernet_utils = utils.FernetUtils() keystone_user_id, keystone_group_id = cls.get_user_group() - if fernet.validate_key_repository(requires_write=True): - fernet.rotate_keys(keystone_user_id, keystone_group_id) + if fernet_utils.validate_key_repository(requires_write=True): + fernet_utils.rotate_keys(keystone_user_id, keystone_group_id) class TokenFlush(BaseApp): diff --git a/keystone/cmd/doctor/tokens_fernet.py b/keystone/cmd/doctor/tokens_fernet.py index d80ca5ed54..e3561567d1 100644 --- a/keystone/cmd/doctor/tokens_fernet.py +++ b/keystone/cmd/doctor/tokens_fernet.py @@ -12,7 +12,7 @@ import keystone.conf -from keystone.common import fernet_utils +from keystone.common import fernet_utils as utils CONF = keystone.conf.CONF @@ -25,6 +25,7 @@ def symptom_usability_of_Fernet_key_repository(): keystone, but not world-readable, because it contains security-sensitive secrets. """ + fernet_utils = utils.FernetUtils() return ( 'fernet' in CONF.token.provider and not fernet_utils.validate_key_repository()) @@ -38,6 +39,7 @@ def symptom_keys_in_Fernet_key_repository(): with keys, and periodically rotate your keys with `keystone-manage fernet_rotate`. """ + fernet_utils = utils.FernetUtils() return ( 'fernet' in CONF.token.provider and not fernet_utils.load_keys()) diff --git a/keystone/common/fernet_utils.py b/keystone/common/fernet_utils.py index eb22c10443..f8b926263f 100644 --- a/keystone/common/fernet_utils.py +++ b/keystone/common/fernet_utils.py @@ -25,240 +25,246 @@ LOG = log.getLogger(__name__) CONF = keystone.conf.CONF -def validate_key_repository(requires_write=False): - """Validate permissions on the key repository directory.""" - # NOTE(lbragstad): We shouldn't need to check if the directory was passed - # in as None because we don't set allow_no_values to True. +class FernetUtils(object): - # ensure current user has sufficient access to the key repository - is_valid = (os.access(CONF.fernet_tokens.key_repository, os.R_OK) and - os.access(CONF.fernet_tokens.key_repository, os.X_OK)) - if requires_write: - is_valid = (is_valid and - os.access(CONF.fernet_tokens.key_repository, os.W_OK)) + def validate_key_repository(self, requires_write=False): + """Validate permissions on the key repository directory.""" + # NOTE(lbragstad): We shouldn't need to check if the directory was + # passed in as None because we don't set allow_no_values to True. - if not is_valid: - LOG.error( - _LE('Either [fernet_tokens] key_repository does not exist or ' - 'Keystone does not have sufficient permission to access it: ' - '%s'), CONF.fernet_tokens.key_repository) - else: - # ensure the key repository isn't world-readable - stat_info = os.stat(CONF.fernet_tokens.key_repository) - if(stat_info.st_mode & stat.S_IROTH or - stat_info.st_mode & stat.S_IXOTH): - LOG.warning(_LW( - '[fernet_tokens] key_repository is world readable: %s'), - CONF.fernet_tokens.key_repository) + # ensure current user has sufficient access to the key repository + is_valid = (os.access(CONF.fernet_tokens.key_repository, os.R_OK) and + os.access(CONF.fernet_tokens.key_repository, os.X_OK)) + if requires_write: + is_valid = (is_valid and + os.access(CONF.fernet_tokens.key_repository, os.W_OK)) - return is_valid + if not is_valid: + LOG.error( + _LE('Either [fernet_tokens] key_repository does not exist or ' + 'Keystone does not have sufficient permission to access ' + 'it: %s'), CONF.fernet_tokens.key_repository) + else: + # ensure the key repository isn't world-readable + stat_info = os.stat(CONF.fernet_tokens.key_repository) + if(stat_info.st_mode & stat.S_IROTH or + stat_info.st_mode & stat.S_IXOTH): + LOG.warning(_LW( + '[fernet_tokens] key_repository is world readable: %s'), + CONF.fernet_tokens.key_repository) + return is_valid -def _convert_to_integers(id_value): - """Cast user and group system identifiers to integers.""" - # NOTE(lbragstad) os.chown() will raise a TypeError here if - # keystone_user_id and keystone_group_id are not integers. Let's - # cast them to integers if we can because it's possible to pass non-integer - # values into the fernet_setup utility. - try: - id_int = int(id_value) - except ValueError as e: - msg = _LE('Unable to convert Keystone user or group ID. Error: %s') - LOG.error(msg, e) - raise - - return id_int - - -def create_key_directory(keystone_user_id=None, keystone_group_id=None): - """If the configured key directory does not exist, attempt to create it.""" - if not os.access(CONF.fernet_tokens.key_repository, os.F_OK): - LOG.info(_LI( - '[fernet_tokens] key_repository does not appear to exist; ' - 'attempting to create it')) - + def _convert_to_integers(self, id_value): + """Cast user and group system identifiers to integers.""" + # NOTE(lbragstad) os.chown() will raise a TypeError here if + # keystone_user_id and keystone_group_id are not integers. Let's cast + # them to integers if we can because it's possible to pass non-integer + # values into the fernet_setup utility. try: - os.makedirs(CONF.fernet_tokens.key_repository, 0o700) - except OSError: - LOG.error(_LE( - 'Failed to create [fernet_tokens] key_repository: either it ' - 'already exists or you don\'t have sufficient permissions to ' - 'create it')) + id_int = int(id_value) + except ValueError as e: + msg = _LE('Unable to convert Keystone user or group ID. Error: %s') + LOG.error(msg, e) + raise + return id_int + + def create_key_directory(self, keystone_user_id=None, + keystone_group_id=None): + """Attempt to create the key directory if it doesn't exist.""" + if not os.access(CONF.fernet_tokens.key_repository, os.F_OK): + LOG.info(_LI( + '[fernet_tokens] key_repository does not appear to exist; ' + 'attempting to create it')) + + try: + os.makedirs(CONF.fernet_tokens.key_repository, 0o700) + except OSError: + LOG.error(_LE( + 'Failed to create [fernet_tokens] key_repository: either' + 'it already exists or you don\'t have sufficient ' + 'permissions to create it')) + + if keystone_user_id and keystone_group_id: + os.chown( + CONF.fernet_tokens.key_repository, + keystone_user_id, + keystone_group_id) + elif keystone_user_id or keystone_group_id: + LOG.warning(_LW( + 'Unable to change the ownership of [fernet_tokens] ' + 'key_repository without a keystone user ID and keystone ' + 'group ID both being provided: %s') % + CONF.fernet_tokens.key_repository) + + def _create_new_key(self, keystone_user_id, keystone_group_id): + """Securely create a new encryption key. + + Create a new key that is readable by the Keystone group and Keystone + user. + """ + key = fernet.Fernet.generate_key() # key is bytes + + # This ensures the key created is not world-readable + old_umask = os.umask(0o177) if keystone_user_id and keystone_group_id: - os.chown( - CONF.fernet_tokens.key_repository, - keystone_user_id, - keystone_group_id) + old_egid = os.getegid() + old_euid = os.geteuid() + os.setegid(keystone_group_id) + os.seteuid(keystone_user_id) elif keystone_user_id or keystone_group_id: LOG.warning(_LW( - 'Unable to change the ownership of [fernet_tokens] ' - 'key_repository without a keystone user ID and keystone group ' - 'ID both being provided: %s') % + 'Unable to change the ownership of the new key without a ' + 'keystone user ID and keystone group ID both being provided: ' + '%s') % CONF.fernet_tokens.key_repository) + # Determine the file name of the new key + key_file = os.path.join(CONF.fernet_tokens.key_repository, '0') + try: + with open(key_file, 'w') as f: + # convert key to str for the file. + f.write(key.decode('utf-8')) + 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 + # or user was supplied. + os.umask(old_umask) + if keystone_user_id and keystone_group_id: + os.seteuid(old_euid) + os.setegid(old_egid) + LOG.info(_LI('Created a new key: %s'), key_file) -def _create_new_key(keystone_user_id, keystone_group_id): - """Securely create a new encryption key. + def initialize_key_repository(self, keystone_user_id=None, + keystone_group_id=None): + """Create a key repository and bootstrap it with a key. - Create a new key that is readable by the Keystone group and Keystone user. - """ - key = fernet.Fernet.generate_key() # key is bytes + :param keystone_user_id: User ID of the Keystone user. + :param keystone_group_id: Group ID of the Keystone user. - # This ensures the key created is not world-readable - old_umask = os.umask(0o177) - if keystone_user_id and keystone_group_id: - old_egid = os.getegid() - old_euid = os.geteuid() - os.setegid(keystone_group_id) - os.seteuid(keystone_user_id) - elif keystone_user_id or keystone_group_id: - LOG.warning(_LW( - 'Unable to change the ownership of the new key without a keystone ' - 'user ID and keystone group ID both being provided: %s') % - CONF.fernet_tokens.key_repository) - # Determine the file name of the new key - key_file = os.path.join(CONF.fernet_tokens.key_repository, '0') - try: - with open(key_file, 'w') as f: - f.write(key.decode('utf-8')) # convert key to str for the file. - 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 or user - # was supplied. - os.umask(old_umask) - if keystone_user_id and keystone_group_id: - os.seteuid(old_euid) - os.setegid(old_egid) + """ + # make sure we have work to do before proceeding + if os.access(os.path.join(CONF.fernet_tokens.key_repository, '0'), + os.F_OK): + LOG.info(_LI('Key repository is already initialized; aborting.')) + return - LOG.info(_LI('Created a new key: %s'), key_file) + # bootstrap an existing key + self._create_new_key(keystone_user_id, keystone_group_id) + # ensure that we end up with a primary and secondary key + self.rotate_keys(keystone_user_id, keystone_group_id) -def initialize_key_repository(keystone_user_id=None, keystone_group_id=None): - """Create a key repository and bootstrap it with a key. + def rotate_keys(self, keystone_user_id=None, keystone_group_id=None): + """Create a new primary key and revoke excess active keys. - :param keystone_user_id: User ID of the Keystone user. - :param keystone_group_id: Group ID of the Keystone user. + :param keystone_user_id: User ID of the Keystone user. + :param keystone_group_id: Group ID of the Keystone user. - """ - # make sure we have work to do before proceeding - if os.access(os.path.join(CONF.fernet_tokens.key_repository, '0'), - os.F_OK): - LOG.info(_LI('Key repository is already initialized; aborting.')) - return + Key rotation utilizes the following behaviors: - # bootstrap an existing key - _create_new_key(keystone_user_id, keystone_group_id) + - The highest key number is used as the primary key (used for + encryption). + - All keys can be used for decryption. + - New keys are always created as key "0," which serves as a placeholder + before promoting it to be the primary key. - # ensure that we end up with a primary and secondary key - rotate_keys(keystone_user_id, keystone_group_id) + This strategy allows you to safely perform rotation on one node in a + cluster, before syncing the results of the rotation to all other nodes + (during both key rotation and synchronization, all nodes must recognize + all primary keys). - -def rotate_keys(keystone_user_id=None, keystone_group_id=None): - """Create a new primary key and revoke excess active keys. - - :param keystone_user_id: User ID of the Keystone user. - :param keystone_group_id: Group ID of the Keystone user. - - Key rotation utilizes the following behaviors: - - - The highest key number is used as the primary key (used for encryption). - - All keys can be used for decryption. - - New keys are always created as key "0," which serves as a placeholder - before promoting it to be the primary key. - - This strategy allows you to safely perform rotation on one node in a - cluster, before syncing the results of the rotation to all other nodes - (during both key rotation and synchronization, all nodes must recognize all - primary keys). - - """ - # read the list of key files - key_files = dict() - for filename in os.listdir(CONF.fernet_tokens.key_repository): - path = os.path.join(CONF.fernet_tokens.key_repository, str(filename)) - if os.path.isfile(path): - try: - key_id = int(filename) - except ValueError: # nosec : name isn't a number, ignore the file. - pass - else: - key_files[key_id] = path - - LOG.info(_LI('Starting key rotation with %(count)s key files: %(list)s'), { - 'count': len(key_files), - 'list': list(key_files.values())}) - - # 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) - new_primary_key = current_primary_key + 1 - LOG.info(_LI('Next primary key will be: %s'), new_primary_key) - - # promote the next primary key to be the primary - os.rename( - os.path.join(CONF.fernet_tokens.key_repository, '0'), - os.path.join(CONF.fernet_tokens.key_repository, str(new_primary_key))) - key_files.pop(0) - key_files[new_primary_key] = os.path.join( - CONF.fernet_tokens.key_repository, - 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 - _create_new_key(keystone_user_id, keystone_group_id) - - max_active_keys = CONF.fernet_tokens.max_active_keys - - # purge excess keys - - # Note that key_files doesn't contain the new active key that was created, - # only the old active keys. - keys = sorted(key_files.keys(), reverse=True) - while len(keys) > (max_active_keys - 1): - index_to_purge = keys.pop() - key_to_purge = key_files[index_to_purge] - LOG.info(_LI('Excess key to purge: %s'), key_to_purge) - os.remove(key_to_purge) - - -def load_keys(): - """Load keys from disk into a list. - - The first key in the list is the primary key used for encryption. All - other keys are active secondary keys that can be used for decrypting - tokens. - - """ - if not validate_key_repository(): - return [] - - # build a dictionary of key_number:encryption_key pairs - keys = dict() - for filename in os.listdir(CONF.fernet_tokens.key_repository): - path = os.path.join(CONF.fernet_tokens.key_repository, str(filename)) - if os.path.isfile(path): - with open(path, 'r') as key_file: + """ + # read the list of key files + key_files = dict() + for filename in os.listdir(CONF.fernet_tokens.key_repository): + path = os.path.join(CONF.fernet_tokens.key_repository, + str(filename)) + if os.path.isfile(path): try: key_id = int(filename) - except ValueError: # nosec : filename isn't a number, ignore - # this file since it's not a key. + except ValueError: # nosec : name isn't a number pass else: - keys[key_id] = key_file.read() + key_files[key_id] = path - if len(keys) != CONF.fernet_tokens.max_active_keys: - # If there haven't been enough key rotations to reach max_active_keys, - # or if the configured value of max_active_keys has changed since the - # last rotation, then reporting the discrepancy might be useful. Once - # the number of keys matches max_active_keys, this log entry is too - # repetitive to be useful. - LOG.info(_LI( - 'Loaded %(count)d encryption keys (max_active_keys=%(max)d) from: ' - '%(dir)s'), { - 'count': len(keys), - 'max': CONF.fernet_tokens.max_active_keys, - 'dir': CONF.fernet_tokens.key_repository}) + LOG.info(_LI('Starting key rotation with %(count)s key files: ' + '%(list)s'), { + 'count': len(key_files), + 'list': list(key_files.values())}) - # return the encryption_keys, sorted by key number, descending - return [keys[x] for x in sorted(keys.keys(), reverse=True)] + # 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) + new_primary_key = current_primary_key + 1 + LOG.info(_LI('Next primary key will be: %s'), new_primary_key) + + # promote the next primary key to be the primary + os.rename( + os.path.join(CONF.fernet_tokens.key_repository, '0'), + os.path.join(CONF.fernet_tokens.key_repository, + str(new_primary_key))) + key_files.pop(0) + key_files[new_primary_key] = os.path.join( + CONF.fernet_tokens.key_repository, + 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) + + max_active_keys = CONF.fernet_tokens.max_active_keys + + # purge excess keys + + # Note that key_files doesn't contain the new active key that was + # created, only the old active keys. + keys = sorted(key_files.keys(), reverse=True) + while len(keys) > (max_active_keys - 1): + index_to_purge = keys.pop() + key_to_purge = key_files[index_to_purge] + LOG.info(_LI('Excess key to purge: %s'), key_to_purge) + os.remove(key_to_purge) + + def load_keys(self): + """Load keys from disk into a list. + + The first key in the list is the primary key used for encryption. All + other keys are active secondary keys that can be used for decrypting + tokens. + + """ + if not self.validate_key_repository(): + return [] + + # build a dictionary of key_number:encryption_key pairs + keys = dict() + for filename in os.listdir(CONF.fernet_tokens.key_repository): + path = os.path.join(CONF.fernet_tokens.key_repository, + str(filename)) + if os.path.isfile(path): + with open(path, 'r') as key_file: + try: + key_id = int(filename) + except ValueError: # nosec : filename isn't a number, + # ignore this file since it's not a key. + pass + else: + keys[key_id] = key_file.read() + + if len(keys) != CONF.fernet_tokens.max_active_keys: + # If there haven't been enough key rotations to reach + # max_active_keys, or if the configured value of max_active_keys + # has changed since the last rotation, then reporting the + # discrepancy might be useful. Once the number of keys matches + # max_active_keys, this log entry is too repetitive to be useful. + LOG.info(_LI( + 'Loaded %(count)d encryption keys (max_active_keys=%(max)d) ' + 'from: %(dir)s'), { + 'count': len(keys), + 'max': CONF.fernet_tokens.max_active_keys, + 'dir': CONF.fernet_tokens.key_repository}) + + # return the encryption_keys, sorted by key number, descending + return [keys[x] for x in sorted(keys.keys(), reverse=True)] diff --git a/keystone/tests/unit/ksfixtures/key_repository.py b/keystone/tests/unit/ksfixtures/key_repository.py index 842cf396d4..2726d0daf8 100644 --- a/keystone/tests/unit/ksfixtures/key_repository.py +++ b/keystone/tests/unit/ksfixtures/key_repository.py @@ -26,5 +26,6 @@ class KeyRepository(fixtures.Fixture): self.config_fixture.config(group='fernet_tokens', key_repository=directory) - utils.create_key_directory() - utils.initialize_key_repository() + fernet_utils = utils.FernetUtils() + fernet_utils.create_key_directory() + fernet_utils.initialize_key_repository() diff --git a/keystone/tests/unit/token/test_fernet_provider.py b/keystone/tests/unit/token/test_fernet_provider.py index 94dbe9a5e2..a7089f5722 100644 --- a/keystone/tests/unit/token/test_fernet_provider.py +++ b/keystone/tests/unit/token/test_fernet_provider.py @@ -507,7 +507,8 @@ class TestFernetKeyRotation(unit.TestCase): """ # Load the keys into a list, keys is list of six.text_type. - keys = fernet_utils.load_keys() + utils = fernet_utils.FernetUtils() + keys = utils.load_keys() # Sort the list of keys by the keys themselves (they were previously # sorted by filename). @@ -543,6 +544,7 @@ class TestFernetKeyRotation(unit.TestCase): # support max_active_keys being set any lower. min_active_keys = 2 + utils = fernet_utils.FernetUtils() # Simulate every rotation strategy up to "rotating once a week while # maintaining a year's worth of keys." for max_active_keys in range(min_active_keys, 52 + 1): @@ -565,7 +567,7 @@ class TestFernetKeyRotation(unit.TestCase): # Rotate the keys just enough times to fully populate the key # repository. for rotation in range(max_active_keys - min_active_keys): - fernet_utils.rotate_keys() + utils.rotate_keys() self.assertRepositoryState(expected_size=rotation + 3) exp_keys.append(next_key_number) @@ -578,7 +580,7 @@ class TestFernetKeyRotation(unit.TestCase): # Rotate an additional number of times to ensure that we maintain # the desired number of active keys. for rotation in range(10): - fernet_utils.rotate_keys() + utils.rotate_keys() self.assertRepositoryState(expected_size=max_active_keys) exp_keys.pop(1) @@ -591,7 +593,8 @@ class TestFernetKeyRotation(unit.TestCase): evil_file = os.path.join(CONF.fernet_tokens.key_repository, '99.bak') with open(evil_file, 'w'): pass - fernet_utils.rotate_keys() + utils = fernet_utils.FernetUtils() + utils.rotate_keys() self.assertTrue(os.path.isfile(evil_file)) keys = 0 for x in os.listdir(CONF.fernet_tokens.key_repository): @@ -607,6 +610,7 @@ class TestLoadKeys(unit.TestCase): evil_file = os.path.join(CONF.fernet_tokens.key_repository, '~1') with open(evil_file, 'w'): pass - keys = fernet_utils.load_keys() + utils = fernet_utils.FernetUtils() + keys = utils.load_keys() self.assertEqual(2, len(keys)) self.assertTrue(len(keys[0])) diff --git a/keystone/token/providers/fernet/token_formatters.py b/keystone/token/providers/fernet/token_formatters.py index ad2613bb90..7353c42127 100644 --- a/keystone/token/providers/fernet/token_formatters.py +++ b/keystone/token/providers/fernet/token_formatters.py @@ -57,7 +57,8 @@ class TokenFormatter(object): ``encrypt(plaintext)`` and ``decrypt(ciphertext)``. """ - keys = utils.load_keys() + fernet_utils = utils.FernetUtils() + keys = fernet_utils.load_keys() if not keys: raise exception.KeysNotFound()