Merge ~dbungert/curtin:cryptoswap into curtin:master

Proposed by Dan Bungert
Status: Merged
Merged at revision: 1997a1614e774cbd152fa33059d53417a641dbd6
Proposed branch: ~dbungert/curtin:cryptoswap
Merge into: curtin:master
Diff against target: 348 lines (+196/-17)
5 files modified
curtin/commands/block_meta.py (+11/-3)
doc/topics/storage.rst (+13/-1)
tests/integration/test_block_meta.py (+45/-1)
tests/unittests/test_commands_block_meta.py (+126/-12)
tools/vmtest-system-setup (+1/-0)
Reviewer Review Type Date Requested Status
Olivier Gayot Approve
Server Team CI bot continuous-integration Approve
Chris Peterson Approve
Review via email: mp+458460@code.launchpad.net

Commit message

Add features to the dm_crypt action to support cryptoswap

To post a comment you must log in.
Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Approve (continuous-integration)
Revision history for this message
Chris Peterson (cpete) :
review: Approve
Revision history for this message
Olivier Gayot (ogayot) wrote :

Thanks! I only have minor suggestions. Nothing blocking really.

Revision history for this message
Dan Bungert (dbungert) :
Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Approve (continuous-integration)
Revision history for this message
Olivier Gayot (ogayot) wrote :

Thanks!

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
diff --git a/curtin/commands/block_meta.py b/curtin/commands/block_meta.py
index ebae27c..8ba7a55 100644
--- a/curtin/commands/block_meta.py
+++ b/curtin/commands/block_meta.py
@@ -1618,6 +1618,8 @@ def dm_crypt_handler(info, storage_config, context):
1618 keysize = info.get('keysize')1618 keysize = info.get('keysize')
1619 cipher = info.get('cipher')1619 cipher = info.get('cipher')
1620 dm_name = info.get('dm_name')1620 dm_name = info.get('dm_name')
1621 options = ','.join(info.get('options', ['luks']))
1622 crypttab_keyfile = 'none'
1621 if not dm_name:1623 if not dm_name:
1622 dm_name = info.get('id')1624 dm_name = info.get('id')
1623 dmcrypt_dev = os.path.join("/dev", "mapper", dm_name)1625 dmcrypt_dev = os.path.join("/dev", "mapper", dm_name)
@@ -1634,8 +1636,13 @@ def dm_crypt_handler(info, storage_config, context):
1634 if 'keyfile' in info:1636 if 'keyfile' in info:
1635 if 'key' in info:1637 if 'key' in info:
1636 raise ValueError("cannot specify both key and keyfile")1638 raise ValueError("cannot specify both key and keyfile")
1637 keyfile_is_tmp = False
1638 keyfile = info['keyfile']1639 keyfile = info['keyfile']
1640 if keyfile in ("/dev/random", "/dev/urandom"):
1641 crypttab_keyfile = keyfile
1642 keyfile = tempfile.mkstemp()[1]
1643 keyfile_is_tmp = True
1644 else:
1645 keyfile_is_tmp = False
1639 elif 'key' in info:1646 elif 'key' in info:
1640 # TODO: this is insecure, find better way to do this1647 # TODO: this is insecure, find better way to do this
1641 key = info.get('key')1648 key = info.get('key')
@@ -1727,8 +1734,9 @@ def dm_crypt_handler(info, storage_config, context):
1727 state_dir = os.path.dirname(state['fstab'])1734 state_dir = os.path.dirname(state['fstab'])
1728 crypt_tab_location = os.path.join(state_dir, "crypttab")1735 crypt_tab_location = os.path.join(state_dir, "crypttab")
1729 uuid = block.get_volume_uuid(volume_path)1736 uuid = block.get_volume_uuid(volume_path)
1730 util.write_file(crypt_tab_location,1737 util.write_file(
1731 "%s UUID=%s none luks\n" % (dm_name, uuid), omode="a")1738 crypt_tab_location,
1739 f"{dm_name} UUID={uuid} {crypttab_keyfile} {options}\n", omode="a")
1732 else:1740 else:
1733 LOG.info("fstab configuration is not present in environment, so \1741 LOG.info("fstab configuration is not present in environment, so \
1734 cannot locate an appropriate directory to write crypttab in \1742 cannot locate an appropriate directory to write crypttab in \
diff --git a/doc/topics/storage.rst b/doc/topics/storage.rst
index 45b9bbe..97e900d 100644
--- a/doc/topics/storage.rst
+++ b/doc/topics/storage.rst
@@ -886,6 +886,15 @@ system will prompt for this password in order to mount the disk.
886The ``keyfile`` contains the password of the encryption key. The target886The ``keyfile`` contains the password of the encryption key. The target
887system will prompt for this password in order to mount the disk.887system will prompt for this password in order to mount the disk.
888888
889A special case of ``keyfile`` are the values ``/dev/urandom`` and
890``/dev/random``, which indicate that this ``keyfile`` value will be used in
891entirety to decrypt the device. In this case, the keyfile is passed along to
892the crypttab, as the third field.
893
894.. note::
895 ``/dev/urandom`` and ``/dev/random`` are functionally equivalent on modern
896 systems.
897
889Exactly one of **key** and **keyfile** must be supplied.898Exactly one of **key** and **keyfile** must be supplied.
890899
891**preserve**: *true, false*900**preserve**: *true, false*
@@ -893,13 +902,16 @@ Exactly one of **key** and **keyfile** must be supplied.
893If the ``preserve`` option is True, curtin will verify the dm-crypt device902If the ``preserve`` option is True, curtin will verify the dm-crypt device
894specified is composed of the device specified in ``volume``.903specified is composed of the device specified in ``volume``.
895904
896
897**wipe**: *superblock, superblock-recursive, pvremove, zero, random*905**wipe**: *superblock, superblock-recursive, pvremove, zero, random*
898906
899If ``wipe`` option is set, and ``preserve`` is False, curtin will wipe the907If ``wipe`` option is set, and ``preserve`` is False, curtin will wipe the
900contents of the dm-crypt device. Curtin skips wipe settings if it creates908contents of the dm-crypt device. Curtin skips wipe settings if it creates
901the dm-crypt volume.909the dm-crypt volume.
902910
911**options**: *<list of man 5 crypttab options strings>*
912
913Options to pass to crypttab, as the fourth field. See man 5 crypttab for
914details. The default is ``[luks]``.
903915
904.. note::916.. note::
905917
diff --git a/tests/integration/test_block_meta.py b/tests/integration/test_block_meta.py
index acd12e1..ec9b33c 100644
--- a/tests/integration/test_block_meta.py
+++ b/tests/integration/test_block_meta.py
@@ -6,6 +6,7 @@ import contextlib
6import json6import json
7import os7import os
8from parameterized import parameterized8from parameterized import parameterized
9from pathlib import Path
9import re10import re
10import sys11import sys
11from typing import Optional12from typing import Optional
@@ -192,6 +193,15 @@ class StorageConfigBuilder:
192 for action in self.config:193 for action in self.config:
193 action['preserve'] = True194 action['preserve'] = True
194195
196 def add_dmcrypt(self, *, volume, dm_name=None, **kw):
197 if dm_name is None:
198 dm_name = CiTestCase.random_string()
199 return self._add(
200 type='dm_crypt',
201 volume=volume['id'],
202 dm_name=dm_name,
203 **kw)
204
195205
196class TestBlockMeta(IntegrationTestCase):206class TestBlockMeta(IntegrationTestCase):
197 def setUp(self):207 def setUp(self):
@@ -239,15 +249,17 @@ class TestBlockMeta(IntegrationTestCase):
239 with open(config_path, 'w') as fp:249 with open(config_path, 'w') as fp:
240 yaml.dump(config, fp)250 yaml.dump(config, fp)
241251
252 self.fstab_dir = self.tmp_dir()
242 cmd_env = kwargs.pop('env', {})253 cmd_env = kwargs.pop('env', {})
243 cmd_env.update({254 cmd_env.update({
244 'PATH': os.environ['PATH'],255 'PATH': os.environ['PATH'],
245 'CONFIG': config_path,256 'CONFIG': config_path,
246 'WORKING_DIR': '/tmp',257 'WORKING_DIR': '/tmp',
247 'OUTPUT_FSTAB': self.tmp_path('fstab'),258 'OUTPUT_FSTAB': self.tmp_path('fstab', _dir=self.fstab_dir),
248 'OUTPUT_INTERFACES': '',259 'OUTPUT_INTERFACES': '',
249 'OUTPUT_NETWORK_STATE': '',260 'OUTPUT_NETWORK_STATE': '',
250 'OUTPUT_NETWORK_CONFIG': '',261 'OUTPUT_NETWORK_CONFIG': '',
262 'TARGET_MOUNT_POINT': self.tmp_dir(),
251 })263 })
252264
253 cmd = [265 cmd = [
@@ -1285,6 +1297,38 @@ table-length: 256'''.encode()
1285 partition_type='82'))1297 partition_type='82'))
12861298
1287 @parameterized.expand(((1,), (2,)))1299 @parameterized.expand(((1,), (2,)))
1300 def test_cryptoswap(self, sv=2):
1301 self.img = self.tmp_path('image.img')
1302 config = StorageConfigBuilder(version=sv)
1303 config.add_image(path=self.img, create=True, size='200M',
1304 ptable='msdos')
1305 p1 = config.add_part(
1306 number=1, offset=1 << 20, size=19 << 20, flag='swap'
1307 )
1308 cryptoswap = f"cryptoswap-{self.random_string(length=6)}"
1309 dmc1 = config.add_dmcrypt(
1310 volume=p1,
1311 dm_name=cryptoswap,
1312 keyfile="/dev/urandom",
1313 options=["swap", "initramfs"],
1314 )
1315 config.add_format(part=dmc1, fstype="swap")
1316 self.run_bm(config.render())
1317
1318 self.assertPartitions(
1319 PartData(number=1, offset=1 << 20, size=19 << 20, boot=False,
1320 partition_type='82'))
1321
1322 crypttab_path = Path(self.fstab_dir) / "crypttab"
1323 with open(crypttab_path) as fp:
1324 crypttab = fp.read()
1325 tokens = re.split(r'\s+', crypttab)
1326 self.assertEqual(cryptoswap, tokens[0])
1327 self.assertTrue(tokens[1].startswith("UUID="))
1328 self.assertEqual("/dev/urandom", tokens[2])
1329 self.assertEqual("swap,initramfs", tokens[3])
1330
1331 @parameterized.expand(((1,), (2,)))
1288 def test_msftres(self, sv):1332 def test_msftres(self, sv):
1289 self.img = self.tmp_path('image.img')1333 self.img = self.tmp_path('image.img')
1290 config = StorageConfigBuilder(version=sv)1334 config = StorageConfigBuilder(version=sv)
diff --git a/tests/unittests/test_commands_block_meta.py b/tests/unittests/test_commands_block_meta.py
index 9d7d0f3..5adb46c 100644
--- a/tests/unittests/test_commands_block_meta.py
+++ b/tests/unittests/test_commands_block_meta.py
@@ -2030,10 +2030,10 @@ class TestLvmPartitionHandler(CiTestCase):
2030 self.assertEqual(0, self.m_subp.call_count)2030 self.assertEqual(0, self.m_subp.call_count)
20312031
20322032
2033class TestDmCryptHandler(CiTestCase):2033class DmCryptCommon(CiTestCase):
20342034
2035 def setUp(self):2035 def setUp(self):
2036 super(TestDmCryptHandler, self).setUp()2036 super().setUp()
20372037
2038 basepath = 'curtin.commands.block_meta.'2038 basepath = 'curtin.commands.block_meta.'
2039 self.add_patch(basepath + 'get_path_to_storage_volume', 'm_getpath')2039 self.add_patch(basepath + 'get_path_to_storage_volume', 'm_getpath')
@@ -2047,7 +2047,26 @@ class TestDmCryptHandler(CiTestCase):
2047 self.keyfile = self.random_string()2047 self.keyfile = self.random_string()
2048 self.cipher = self.random_string()2048 self.cipher = self.random_string()
2049 self.keysize = self.random_string()2049 self.keysize = self.random_string()
2050 self.config = {2050 self.m_block.zkey_supported.return_value = False
2051 self.block_uuids = [random_uuid() for unused in range(2)]
2052 self.m_block.get_volume_uuid.side_effect = self.block_uuids
2053
2054 self.m_which.return_value = False
2055 self.fstab = self.tmp_path('fstab')
2056 self.crypttab = os.path.join(os.path.dirname(self.fstab), 'crypttab')
2057 self.m_load_env.return_value = {'fstab': self.fstab,
2058 'target': self.target}
2059
2060 def setUpStorageConfig(self, config):
2061 self.config = config
2062 self.storage_config = block_meta.extract_storage_ordered_dict(config)
2063
2064
2065class TestDmCryptHandler(DmCryptCommon):
2066
2067 def setUp(self):
2068 super().setUp()
2069 self.setUpStorageConfig({
2051 'storage': {2070 'storage': {
2052 'version': 1,2071 'version': 1,
2053 'config': [2072 'config': [
@@ -2073,15 +2092,7 @@ class TestDmCryptHandler(CiTestCase):
2073 'keyfile': self.keyfile},2092 'keyfile': self.keyfile},
2074 ],2093 ],
2075 }2094 }
2076 }2095 })
2077 self.storage_config = (
2078 block_meta.extract_storage_ordered_dict(self.config))
2079 self.m_block.zkey_supported.return_value = False
2080 self.m_which.return_value = False
2081 self.fstab = self.tmp_path('fstab')
2082 self.crypttab = os.path.join(os.path.dirname(self.fstab), 'crypttab')
2083 self.m_load_env.return_value = {'fstab': self.fstab,
2084 'target': self.target}
20852096
2086 def test_dm_crypt_calls_cryptsetup(self):2097 def test_dm_crypt_calls_cryptsetup(self):
2087 """ verify dm_crypt calls (format, open) w/ correct params"""2098 """ verify dm_crypt calls (format, open) w/ correct params"""
@@ -2402,6 +2413,109 @@ class TestDmCryptHandler(CiTestCase):
2402 info, self.storage_config, empty_context)2413 info, self.storage_config, empty_context)
24032414
24042415
2416class TestCrypttab(DmCryptCommon):
2417
2418 def test_multi_dm_crypt(self):
2419 """ verify that multiple dm_crypt calls result in the data for both
2420 being present in crypttab"""
2421
2422 self.setUpStorageConfig({
2423 'storage': {
2424 'version': 1,
2425 'config': [
2426 {'grub_device': True,
2427 'id': 'sda',
2428 'name': 'sda',
2429 'path': '/wark/xxx',
2430 'ptable': 'msdos',
2431 'type': 'disk',
2432 'wipe': 'superblock'},
2433 {'device': 'sda',
2434 'id': 'sda-part1',
2435 'name': 'sda-part1',
2436 'number': 1,
2437 'size': '511705088B',
2438 'type': 'partition'},
2439 {'id': 'dmcrypt0',
2440 'type': 'dm_crypt',
2441 'dm_name': 'cryptroot',
2442 'volume': 'sda-part1',
2443 'cipher': self.cipher,
2444 'keysize': self.keysize,
2445 'keyfile': self.keyfile},
2446 {'device': 'sda',
2447 'id': 'sda-part2',
2448 'name': 'sda-part2',
2449 'number': 2,
2450 'size': '511705088B',
2451 'type': 'partition'},
2452 {'id': 'dmcrypt1',
2453 'type': 'dm_crypt',
2454 'dm_name': 'cryptfoo',
2455 'volume': 'sda-part2',
2456 'cipher': self.cipher,
2457 'keysize': self.keysize,
2458 'keyfile': self.keyfile},
2459 ],
2460 }
2461 })
2462
2463 for i in range(2):
2464 self.m_getpath.return_value = self.random_string()
2465 info = self.storage_config['dmcrypt' + str(i)]
2466 block_meta.dm_crypt_handler(
2467 info, self.storage_config, empty_context
2468 )
2469
2470 data = util.load_file(self.crypttab)
2471 self.assertEqual(
2472 f"cryptroot UUID={self.block_uuids[0]} none luks\n"
2473 f"cryptfoo UUID={self.block_uuids[1]} none luks\n",
2474 data
2475 )
2476
2477 def test_cryptoswap(self):
2478 """ Check the keyfile and options features needed for cryptoswap """
2479
2480 self.setUpStorageConfig({
2481 'storage': {
2482 'version': 1,
2483 'config': [
2484 {'grub_device': True,
2485 'id': 'sda',
2486 'name': 'sda',
2487 'path': '/wark/xxx',
2488 'ptable': 'msdos',
2489 'type': 'disk',
2490 'wipe': 'superblock'},
2491 {'device': 'sda',
2492 'id': 'sda-part1',
2493 'name': 'sda-part1',
2494 'number': 1,
2495 'size': '511705088B',
2496 'type': 'partition'},
2497 {'id': 'dmcrypt0',
2498 'type': 'dm_crypt',
2499 'dm_name': 'cryptswap',
2500 'volume': 'sda-part1',
2501 'options': ["swap", "initramfs"],
2502 'keyfile': "/dev/urandom"},
2503 ],
2504 }
2505 })
2506
2507 self.m_getpath.return_value = self.random_string()
2508 block_meta.dm_crypt_handler(
2509 self.storage_config['dmcrypt0'], self.storage_config, empty_context
2510 )
2511
2512 uuid = self.block_uuids[0]
2513 self.assertEqual(
2514 f"cryptswap UUID={uuid} /dev/urandom swap,initramfs\n",
2515 util.load_file(self.crypttab),
2516 )
2517
2518
2405class TestRaidHandler(CiTestCase):2519class TestRaidHandler(CiTestCase):
24062520
2407 def setUp(self):2521 def setUp(self):
diff --git a/tools/vmtest-system-setup b/tools/vmtest-system-setup
index 5172741..2f82464 100755
--- a/tools/vmtest-system-setup
+++ b/tools/vmtest-system-setup
@@ -14,6 +14,7 @@ esac
14DEPS=(14DEPS=(
15 build-essential15 build-essential
16 cloud-image-utils16 cloud-image-utils
17 cryptsetup
17 git18 git
18 make19 make
19 net-tools20 net-tools

Subscribers

People subscribed via source and target branches