From e86c21ad64065fd660515b78cade2fadeaf7b1f4 Mon Sep 17 00:00:00 2001 From: Guillaume Abrioux Date: Tue, 25 Jan 2022 10:25:53 +0100 Subject: [PATCH 1/3] ceph-volume: honour osd_dmcrypt_key_size option ceph-volume doesn't honour osd_dmcrypt_key_size. It means the default size is always applied. It also changes the default value in `get_key_size_from_conf()` From cryptsetup manpage: > For XTS mode you can optionally set a key size of 512 bits with the -s option. Using more than 512bits will end up with the following error message: ``` Key size in XTS mode must be 256 or 512 bits. ``` Fixes: https://tracker.ceph.com/issues/54006 Signed-off-by: Guillaume Abrioux --- .../ceph_volume/tests/util/test_encryption.py | 45 +++++++++++++++++++ .../ceph_volume/util/encryption.py | 35 ++++++++++++--- 2 files changed, 73 insertions(+), 7 deletions(-) diff --git a/src/ceph-volume/ceph_volume/tests/util/test_encryption.py b/src/ceph-volume/ceph_volume/tests/util/test_encryption.py index 8cca42689b4..c02646f564b 100644 --- a/src/ceph-volume/ceph_volume/tests/util/test_encryption.py +++ b/src/ceph-volume/ceph_volume/tests/util/test_encryption.py @@ -1,6 +1,33 @@ +import os +import base64 from ceph_volume.util import encryption +class TestGetKeySize(object): + def test_get_size_from_b64key_less_than_512bits(self): + assert encryption.get_key_size_from_b64_key(base64.b64encode(os.urandom(int(128 / 8)))) == 128 + + def test_get_size_from_b64key_greater_than_512bits(self): + assert encryption.get_key_size_from_b64_key(base64.b64encode(os.urandom(int(1024 / 8)))) == 512 + + def test_get_size_from_conf_default(self, conf_ceph_stub): + conf_ceph_stub(''' + [global] + fsid=asdf + ''') + assert encryption.get_key_size_from_conf() == '512' + + def test_get_size_from_conf_custom(self, conf_ceph_stub): + conf_ceph_stub(''' + [global] + fsid=asdf + [osd] + osd_dmcrypt_key_size=256 + ''') + assert encryption.get_key_size_from_conf() == '256' + + + class TestStatus(object): def test_skips_unuseful_lines(self, stub_call): @@ -33,3 +60,21 @@ class TestDmcryptClose(object): file_name = '/path/does/not/exist' encryption.dmcrypt_close(file_name) assert fake_run.calls == [] + + +class TestDmcryptKey(object): + + def test_dmcrypt_with_default_size(self, conf_ceph_stub): + conf_ceph_stub('[global]\nfsid=asdf-lkjh') + result = encryption.create_dmcrypt_key() + assert len(result) == 88 + + def test_dmcrypt_with_custom_size(self, conf_ceph_stub): + conf_ceph_stub(''' + [global] + fsid=asdf + [osd] + osd_dmcrypt_size=8 + ''') + result = encryption.create_dmcrypt_key() + assert len(result) == 172 diff --git a/src/ceph-volume/ceph_volume/util/encryption.py b/src/ceph-volume/ceph_volume/util/encryption.py index cc594a07e83..640074245dd 100644 --- a/src/ceph-volume/ceph_volume/util/encryption.py +++ b/src/ceph-volume/ceph_volume/util/encryption.py @@ -8,21 +8,38 @@ from .disk import lsblk, device_family, get_part_entry_type logger = logging.getLogger(__name__) +def get_key_size_from_conf(): + """ + Return the osd dmcrypt key size from config file. + Default is 512. + """ + key_size = conf.ceph.get_safe( + 'osd', + 'osd_dmcrypt_key_size', + default='512') + + return key_size + +def get_key_size_from_b64_key(key): + """ + Return the size in bits of a given base64 encoded key. + """ + key_size = len(base64.b64decode(key)) * 8 + if key_size > 512: + key_size = 512 + return key_size def create_dmcrypt_key(): """ Create the secret dm-crypt key used to decrypt a device. """ # get the customizable dmcrypt key size (in bits) from ceph.conf fallback - # to the default of 1024 - dmcrypt_key_size = conf.ceph.get_safe( - 'osd', - 'osd_dmcrypt_key_size', - default=1024, - ) + # to the default of 512 + dmcrypt_key_size = int(get_key_size_from_conf()) + # The size of the key is defined in bits, so we must transform that # value to bytes (dividing by 8) because we read in bytes, not bits - random_string = os.urandom(dmcrypt_key_size / 8) + random_string = os.urandom(int(dmcrypt_key_size / 8)) key = base64.b64encode(random_string).decode('utf-8') return key @@ -37,6 +54,8 @@ def luks_format(key, device): command = [ 'cryptsetup', '--batch-mode', # do not prompt + '--key-size', + str(get_key_size_from_b64_key(key)), '--key-file', # misnomer, should be key '-', # because we indicate stdin for the key here 'luksFormat', @@ -81,6 +100,8 @@ def luks_open(key, device, mapping): """ command = [ 'cryptsetup', + '--key-size', + str(get_key_size_from_b64_key(key)), '--key-file', '-', 'luksOpen', -- 2.20.1.windows.1 From 7e02488e2694f83e97fbd5046ae92c29610570a6 Mon Sep 17 00:00:00 2001 From: Guillaume Abrioux Date: Tue, 25 Jan 2022 15:22:28 +0100 Subject: [PATCH 2/3] ceph-volume: fix `TestDmcryptKey()` tests This commits fixes two things: - the override passed in `conf_ceph_stub()` which is incorrect. - the assert itself. Given it was still using the default value (1024) in the second test `test_dmcrypt_with_custom_size()`, the assert was expecting the length to be 172 too. Signed-off-by: Guillaume Abrioux --- src/ceph-volume/ceph_volume/tests/util/test_encryption.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/ceph-volume/ceph_volume/tests/util/test_encryption.py b/src/ceph-volume/ceph_volume/tests/util/test_encryption.py index c02646f564b..0eb95187539 100644 --- a/src/ceph-volume/ceph_volume/tests/util/test_encryption.py +++ b/src/ceph-volume/ceph_volume/tests/util/test_encryption.py @@ -74,7 +74,7 @@ class TestDmcryptKey(object): [global] fsid=asdf [osd] - osd_dmcrypt_size=8 + osd_dmcrypt_key_size=8 ''') result = encryption.create_dmcrypt_key() - assert len(result) == 172 + assert len(result) == 4 -- 2.20.1.windows.1 From 0765b615ba226cb46ce3b09bc4bf80dab8b1d09b Mon Sep 17 00:00:00 2001 From: Guillaume Abrioux Date: Tue, 25 Jan 2022 15:52:07 +0100 Subject: [PATCH 3/3] ceph-volume: add unit tests in util/encryption.py this adds some unit tests in order to cover `luks_format()` and `luks_open()` in `util/encryption.py`. Signed-off-by: Guillaume Abrioux --- .../ceph_volume/tests/util/test_encryption.py | 70 +++++++++++++++++++ 1 file changed, 70 insertions(+) diff --git a/src/ceph-volume/ceph_volume/tests/util/test_encryption.py b/src/ceph-volume/ceph_volume/tests/util/test_encryption.py index 0eb95187539..e7926ffe96d 100644 --- a/src/ceph-volume/ceph_volume/tests/util/test_encryption.py +++ b/src/ceph-volume/ceph_volume/tests/util/test_encryption.py @@ -1,5 +1,6 @@ import os import base64 +from mock.mock import patch from ceph_volume.util import encryption @@ -78,3 +79,72 @@ class TestDmcryptKey(object): ''') result = encryption.create_dmcrypt_key() assert len(result) == 4 + +class TestLuksFormat(object): + @patch('ceph_volume.util.encryption.process.call') + def test_luks_format_command_with_default_size(self, m_call, conf_ceph_stub): + conf_ceph_stub('[global]\nfsid=abcd') + expected = [ + 'cryptsetup', + '--batch-mode', + '--key-size', + '512', + '--key-file', + '-', + 'luksFormat', + '/dev/foo' + ] + encryption.luks_format('4YJycMQmkaXi+nRN07HsTrk8LR7kzKDXe745t7atdAuoJ2OiwuZ06aD+7Z8MifKGoBulTNlu9GsYER0dFJWCSw==', '/dev/foo') + assert m_call.call_args[0][0] == expected + + @patch('ceph_volume.util.encryption.process.call') + def test_luks_format_command_with_custom_size(self, m_call, conf_ceph_stub): + conf_ceph_stub('[global]\nfsid=abcd\n[osd]\nosd_dmcrypt_key_size=256') + expected = [ + 'cryptsetup', + '--batch-mode', + '--key-size', + '256', + '--key-file', + '-', + 'luksFormat', + '/dev/foo' + ] + encryption.luks_format('F2bk3pbcTX0zzuEdwIMW9lNwcRFg0L48G2JM81yICUI=', '/dev/foo') + assert m_call.call_args[0][0] == expected + + +class TestLuksOpen(object): + @patch('ceph_volume.util.encryption.process.call') + def test_luks_open_command_with_default_size(self, m_call, conf_ceph_stub): + conf_ceph_stub('[global]\nfsid=abcd') + expected = [ + 'cryptsetup', + '--key-size', + '256', + '--key-file', + '-', + '--allow-discards', + 'luksOpen', + '/dev/foo', + '/dev/bar' + ] + encryption.luks_open('F2bk3pbcTX0zzuEdwIMW9lNwcRFg0L48G2JM81yICUI=', '/dev/foo', '/dev/bar') + assert m_call.call_args[0][0] == expected + + @patch('ceph_volume.util.encryption.process.call') + def test_luks_open_command_with_custom_size(self, m_call, conf_ceph_stub): + conf_ceph_stub('[global]\nfsid=abcd\n[osd]\nosd_dmcrypt_key_size=128') + expected = [ + 'cryptsetup', + '--key-size', + '256', + '--key-file', + '-', + '--allow-discards', + 'luksOpen', + '/dev/foo', + '/dev/bar' + ] + encryption.luks_open('F2bk3pbcTX0zzuEdwIMW9lNwcRFg0L48G2JM81yICUI=', '/dev/foo', '/dev/bar') + assert m_call.call_args[0][0] == expected \ No newline at end of file -- 2.20.1.windows.1