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
