Handle empty token key files

In some rare cases, an empty key file can get created within the fernet
key repository. When keystone tries to load the keys from disk, it will
fail with an invalid fernet key ValueError.

This change adds a check for empty files with a valid numerical name
within the key repository when rotating keys and loading keys. If an
empty file exists, it will be ignored when loading keys, reported in the
logs, and overwritten with a valid key upon rotation.

Change-Id: Ic19dd02d38e8f6a05c8951ec3dd13659aab98259
Closes-Bug: 1728907

keystone/common/token_utils.py

@@ -167,6 +167,27 @@ class TokenUtils(object):
 
         LOG.info('Become a valid new key: %s', valid_key_file)
 
+    def _get_key_files(self, key_repo):
+        key_files = dict()
+        keys = dict()
+        for filename in os.listdir(key_repo):
+            path = os.path.join(key_repo, str(filename))
+            if os.path.isfile(path):
+                with open(path, 'r') as key_file:
+                    try:
+                        key_id = int(filename)
+                    except ValueError:  # nosec : name is not a number
+                        pass
+                    else:
+                        key = key_file.read()
+                        if len(key) == 0:
+                            LOG.warning('Ignoring empty key found in key '
+                                        'repository: %s', path)
+                            continue
+                        key_files[key_id] = path
+                        keys[key_id] = key
+        return key_files, keys
+
     def initialize_key_repository(self, keystone_user_id=None,
                                   keystone_group_id=None):
         """Create a key repository and bootstrap it with a key.
@@ -208,16 +229,7 @@ class TokenUtils(object):
 
         """
         # read the list of key files
-        key_files = dict()
-        for filename in os.listdir(self.key_repository):
-            path = os.path.join(self.key_repository, str(filename))
-            if os.path.isfile(path):
-                try:
-                    key_id = int(filename)
-                except ValueError:  # nosec : name isn't a number
-                    pass
-                else:
-                    key_files[key_id] = path
+        key_files, _ = self._get_key_files(self.key_repository)
 
         LOG.info('Starting key rotation with %(count)s key files: '
                  '%(list)s', {
@@ -277,18 +289,7 @@ class TokenUtils(object):
             return []
 
         # build a dictionary of key_number:encryption_key pairs
-        keys = dict()
-        for filename in os.listdir(self.key_repository):
-            path = os.path.join(self.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()
+        _, keys = self._get_key_files(self.key_repository)
 
         if len(keys) != self.max_active_keys:
             # Once the number of keys matches max_active_keys, this log entry

keystone/tests/unit/token/test_fernet_provider.py

@@ -634,6 +634,24 @@ class TestFernetKeyRotation(unit.TestCase):
         # Assert that the key repository is now expanded.
         self.assertEqual(self.key_repository_size, 3)
 
+    def test_rotation_empty_file(self):
+        active_keys = 2
+        self.assertRepositoryState(expected_size=active_keys)
+        empty_file = os.path.join(CONF.fernet_tokens.key_repository, '2')
+        with open(empty_file, 'w'):
+            pass
+        key_utils = token_utils.TokenUtils(
+            CONF.fernet_tokens.key_repository,
+            CONF.fernet_tokens.max_active_keys,
+            'fernet_tokens'
+        )
+        # Rotate the keys to overwrite the empty file
+        key_utils.rotate_keys()
+        self.assertTrue(os.path.isfile(empty_file))
+        keys = key_utils.load_keys()
+        self.assertEqual(3, len(keys))
+        self.assertTrue(os.path.getsize(empty_file) > 0)
+
     def test_non_numeric_files(self):
         evil_file = os.path.join(CONF.fernet_tokens.key_repository, '99.bak')
         with open(evil_file, 'w'):
@@ -673,3 +691,16 @@ class TestLoadKeys(unit.TestCase):
         keys = key_utils.load_keys()
         self.assertEqual(2, len(keys))
         self.assertValidFernetKeys(keys)
+
+    def test_empty_files(self):
+        empty_file = os.path.join(CONF.fernet_tokens.key_repository, '2')
+        with open(empty_file, 'w'):
+            pass
+        key_utils = token_utils.TokenUtils(
+            CONF.fernet_tokens.key_repository,
+            CONF.fernet_tokens.max_active_keys,
+            'fernet_tokens'
+        )
+        keys = key_utils.load_keys()
+        self.assertEqual(2, len(keys))
+        self.assertValidFernetKeys(keys)

releasenotes/notes/bug-1728907-bab6769ab46bd8aa.yaml

@@ -0,0 +1,10 @@
+---
+fixes:
+  - |
+    [`bug 1728907 <https://bugs.launchpad.net/keystone/+bug/1728907>`_]
+    In some rare cases, an empty key file can get created within the fernet
+    key repository. When keystone tries to load the keys from disk, it will
+    fail with an invalid fernet key ValueError. Keystone now handles empty
+    key files when loading and rotating keys. If an empty file exists, it
+    will be ignored when loaded, reported as a warning in the log, and
+    overwritten with a valid key upon rotation.