File 0001-Handle-disk-write-failure-when-doing-Ferne... of Package openstack-keystone

Overview Repositories Revisions Requests Users Attributes Meta

File 0001-Handle-disk-write-failure-when-doing-Fernet-key-rota.patch of Package openstack-keystone

From 5b9d754d50b2b723884005b7111ad51d9d8755b7 Mon Sep 17 00:00:00 2001
From: johnlinp <johnlinp@gmail.com>
Date: Wed, 21 Dec 2016 15:17:01 +0800
Subject: [PATCH] Handle disk write failure when doing Fernet key rotation

_create_new_key() is broke down into 2 parts:

1. _create_tmp_new_key()
2. _become_valid_new_key()

This can avoid empty Fernet keys when the write to the
staged key fails. The _become_valid_new_key() is called
only after a successful call to _create_tmp_new_key().

Change-Id: Iaf33e2b291f13b9eb9464ef345a8664a634121ff
Closes-Bug: #1642457
Signed-off-by: John Lin <johnlinp@gmail.com>
(cherry picked from commit 5b7c9a66f0aed860ea0776d4c5b42710d88fcb5f)
---
 keystone/common/fernet_utils.py               | 43 +++++++++++++++++--
 .../tests/unit/token/test_fernet_provider.py  | 30 +++++++++++++
 .../notes/bug-1642457-4533f9810a8cd927.yaml   |  7 +++
 3 files changed, 76 insertions(+), 4 deletions(-)
 create mode 100644 releasenotes/notes/bug-1642457-4533f9810a8cd927.yaml

diff --git a/keystone/common/fernet_utils.py b/keystone/common/fernet_utils.py
index cb7a69863..d567a85e6 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 temporary key: %s'), key_file)
 
-        LOG.info(_LI('Created a new 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 d99330e45..117a372ca 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
 
@@ -622,6 +623,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 000000000..4d1d5ff95
--- /dev/null
+++ b/releasenotes/notes/bug-1642457-4533f9810a8cd927.yaml
@@ -0,0 +1,7 @@
+---
+fixes:
+  - |
+    [`bug 1642457 <https://bugs.launchpad.net/keystone/+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.
-- 
2.18.0

Places

File 0001-Handle-disk-write-failure-when-doing-Fernet-key-rota.patch of Package openstack-keystone

Places