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
1diff --git a/curtin/commands/block_meta.py b/curtin/commands/block_meta.py
2index ebae27c..8ba7a55 100644
3--- a/curtin/commands/block_meta.py
4+++ b/curtin/commands/block_meta.py
5@@ -1618,6 +1618,8 @@ def dm_crypt_handler(info, storage_config, context):
6 keysize = info.get('keysize')
7 cipher = info.get('cipher')
8 dm_name = info.get('dm_name')
9+ options = ','.join(info.get('options', ['luks']))
10+ crypttab_keyfile = 'none'
11 if not dm_name:
12 dm_name = info.get('id')
13 dmcrypt_dev = os.path.join("/dev", "mapper", dm_name)
14@@ -1634,8 +1636,13 @@ def dm_crypt_handler(info, storage_config, context):
15 if 'keyfile' in info:
16 if 'key' in info:
17 raise ValueError("cannot specify both key and keyfile")
18- keyfile_is_tmp = False
19 keyfile = info['keyfile']
20+ if keyfile in ("/dev/random", "/dev/urandom"):
21+ crypttab_keyfile = keyfile
22+ keyfile = tempfile.mkstemp()[1]
23+ keyfile_is_tmp = True
24+ else:
25+ keyfile_is_tmp = False
26 elif 'key' in info:
27 # TODO: this is insecure, find better way to do this
28 key = info.get('key')
29@@ -1727,8 +1734,9 @@ def dm_crypt_handler(info, storage_config, context):
30 state_dir = os.path.dirname(state['fstab'])
31 crypt_tab_location = os.path.join(state_dir, "crypttab")
32 uuid = block.get_volume_uuid(volume_path)
33- util.write_file(crypt_tab_location,
34- "%s UUID=%s none luks\n" % (dm_name, uuid), omode="a")
35+ util.write_file(
36+ crypt_tab_location,
37+ f"{dm_name} UUID={uuid} {crypttab_keyfile} {options}\n", omode="a")
38 else:
39 LOG.info("fstab configuration is not present in environment, so \
40 cannot locate an appropriate directory to write crypttab in \
41diff --git a/doc/topics/storage.rst b/doc/topics/storage.rst
42index 45b9bbe..97e900d 100644
43--- a/doc/topics/storage.rst
44+++ b/doc/topics/storage.rst
45@@ -886,6 +886,15 @@ system will prompt for this password in order to mount the disk.
46 The ``keyfile`` contains the password of the encryption key. The target
47 system will prompt for this password in order to mount the disk.
48
49+A special case of ``keyfile`` are the values ``/dev/urandom`` and
50+``/dev/random``, which indicate that this ``keyfile`` value will be used in
51+entirety to decrypt the device. In this case, the keyfile is passed along to
52+the crypttab, as the third field.
53+
54+.. note::
55+ ``/dev/urandom`` and ``/dev/random`` are functionally equivalent on modern
56+ systems.
57+
58 Exactly one of **key** and **keyfile** must be supplied.
59
60 **preserve**: *true, false*
61@@ -893,13 +902,16 @@ Exactly one of **key** and **keyfile** must be supplied.
62 If the ``preserve`` option is True, curtin will verify the dm-crypt device
63 specified is composed of the device specified in ``volume``.
64
65-
66 **wipe**: *superblock, superblock-recursive, pvremove, zero, random*
67
68 If ``wipe`` option is set, and ``preserve`` is False, curtin will wipe the
69 contents of the dm-crypt device. Curtin skips wipe settings if it creates
70 the dm-crypt volume.
71
72+**options**: *<list of man 5 crypttab options strings>*
73+
74+Options to pass to crypttab, as the fourth field. See man 5 crypttab for
75+details. The default is ``[luks]``.
76
77 .. note::
78
79diff --git a/tests/integration/test_block_meta.py b/tests/integration/test_block_meta.py
80index acd12e1..ec9b33c 100644
81--- a/tests/integration/test_block_meta.py
82+++ b/tests/integration/test_block_meta.py
83@@ -6,6 +6,7 @@ import contextlib
84 import json
85 import os
86 from parameterized import parameterized
87+from pathlib import Path
88 import re
89 import sys
90 from typing import Optional
91@@ -192,6 +193,15 @@ class StorageConfigBuilder:
92 for action in self.config:
93 action['preserve'] = True
94
95+ def add_dmcrypt(self, *, volume, dm_name=None, **kw):
96+ if dm_name is None:
97+ dm_name = CiTestCase.random_string()
98+ return self._add(
99+ type='dm_crypt',
100+ volume=volume['id'],
101+ dm_name=dm_name,
102+ **kw)
103+
104
105 class TestBlockMeta(IntegrationTestCase):
106 def setUp(self):
107@@ -239,15 +249,17 @@ class TestBlockMeta(IntegrationTestCase):
108 with open(config_path, 'w') as fp:
109 yaml.dump(config, fp)
110
111+ self.fstab_dir = self.tmp_dir()
112 cmd_env = kwargs.pop('env', {})
113 cmd_env.update({
114 'PATH': os.environ['PATH'],
115 'CONFIG': config_path,
116 'WORKING_DIR': '/tmp',
117- 'OUTPUT_FSTAB': self.tmp_path('fstab'),
118+ 'OUTPUT_FSTAB': self.tmp_path('fstab', _dir=self.fstab_dir),
119 'OUTPUT_INTERFACES': '',
120 'OUTPUT_NETWORK_STATE': '',
121 'OUTPUT_NETWORK_CONFIG': '',
122+ 'TARGET_MOUNT_POINT': self.tmp_dir(),
123 })
124
125 cmd = [
126@@ -1285,6 +1297,38 @@ table-length: 256'''.encode()
127 partition_type='82'))
128
129 @parameterized.expand(((1,), (2,)))
130+ def test_cryptoswap(self, sv=2):
131+ self.img = self.tmp_path('image.img')
132+ config = StorageConfigBuilder(version=sv)
133+ config.add_image(path=self.img, create=True, size='200M',
134+ ptable='msdos')
135+ p1 = config.add_part(
136+ number=1, offset=1 << 20, size=19 << 20, flag='swap'
137+ )
138+ cryptoswap = f"cryptoswap-{self.random_string(length=6)}"
139+ dmc1 = config.add_dmcrypt(
140+ volume=p1,
141+ dm_name=cryptoswap,
142+ keyfile="/dev/urandom",
143+ options=["swap", "initramfs"],
144+ )
145+ config.add_format(part=dmc1, fstype="swap")
146+ self.run_bm(config.render())
147+
148+ self.assertPartitions(
149+ PartData(number=1, offset=1 << 20, size=19 << 20, boot=False,
150+ partition_type='82'))
151+
152+ crypttab_path = Path(self.fstab_dir) / "crypttab"
153+ with open(crypttab_path) as fp:
154+ crypttab = fp.read()
155+ tokens = re.split(r'\s+', crypttab)
156+ self.assertEqual(cryptoswap, tokens[0])
157+ self.assertTrue(tokens[1].startswith("UUID="))
158+ self.assertEqual("/dev/urandom", tokens[2])
159+ self.assertEqual("swap,initramfs", tokens[3])
160+
161+ @parameterized.expand(((1,), (2,)))
162 def test_msftres(self, sv):
163 self.img = self.tmp_path('image.img')
164 config = StorageConfigBuilder(version=sv)
165diff --git a/tests/unittests/test_commands_block_meta.py b/tests/unittests/test_commands_block_meta.py
166index 9d7d0f3..5adb46c 100644
167--- a/tests/unittests/test_commands_block_meta.py
168+++ b/tests/unittests/test_commands_block_meta.py
169@@ -2030,10 +2030,10 @@ class TestLvmPartitionHandler(CiTestCase):
170 self.assertEqual(0, self.m_subp.call_count)
171
172
173-class TestDmCryptHandler(CiTestCase):
174+class DmCryptCommon(CiTestCase):
175
176 def setUp(self):
177- super(TestDmCryptHandler, self).setUp()
178+ super().setUp()
179
180 basepath = 'curtin.commands.block_meta.'
181 self.add_patch(basepath + 'get_path_to_storage_volume', 'm_getpath')
182@@ -2047,7 +2047,26 @@ class TestDmCryptHandler(CiTestCase):
183 self.keyfile = self.random_string()
184 self.cipher = self.random_string()
185 self.keysize = self.random_string()
186- self.config = {
187+ self.m_block.zkey_supported.return_value = False
188+ self.block_uuids = [random_uuid() for unused in range(2)]
189+ self.m_block.get_volume_uuid.side_effect = self.block_uuids
190+
191+ self.m_which.return_value = False
192+ self.fstab = self.tmp_path('fstab')
193+ self.crypttab = os.path.join(os.path.dirname(self.fstab), 'crypttab')
194+ self.m_load_env.return_value = {'fstab': self.fstab,
195+ 'target': self.target}
196+
197+ def setUpStorageConfig(self, config):
198+ self.config = config
199+ self.storage_config = block_meta.extract_storage_ordered_dict(config)
200+
201+
202+class TestDmCryptHandler(DmCryptCommon):
203+
204+ def setUp(self):
205+ super().setUp()
206+ self.setUpStorageConfig({
207 'storage': {
208 'version': 1,
209 'config': [
210@@ -2073,15 +2092,7 @@ class TestDmCryptHandler(CiTestCase):
211 'keyfile': self.keyfile},
212 ],
213 }
214- }
215- self.storage_config = (
216- block_meta.extract_storage_ordered_dict(self.config))
217- self.m_block.zkey_supported.return_value = False
218- self.m_which.return_value = False
219- self.fstab = self.tmp_path('fstab')
220- self.crypttab = os.path.join(os.path.dirname(self.fstab), 'crypttab')
221- self.m_load_env.return_value = {'fstab': self.fstab,
222- 'target': self.target}
223+ })
224
225 def test_dm_crypt_calls_cryptsetup(self):
226 """ verify dm_crypt calls (format, open) w/ correct params"""
227@@ -2402,6 +2413,109 @@ class TestDmCryptHandler(CiTestCase):
228 info, self.storage_config, empty_context)
229
230
231+class TestCrypttab(DmCryptCommon):
232+
233+ def test_multi_dm_crypt(self):
234+ """ verify that multiple dm_crypt calls result in the data for both
235+ being present in crypttab"""
236+
237+ self.setUpStorageConfig({
238+ 'storage': {
239+ 'version': 1,
240+ 'config': [
241+ {'grub_device': True,
242+ 'id': 'sda',
243+ 'name': 'sda',
244+ 'path': '/wark/xxx',
245+ 'ptable': 'msdos',
246+ 'type': 'disk',
247+ 'wipe': 'superblock'},
248+ {'device': 'sda',
249+ 'id': 'sda-part1',
250+ 'name': 'sda-part1',
251+ 'number': 1,
252+ 'size': '511705088B',
253+ 'type': 'partition'},
254+ {'id': 'dmcrypt0',
255+ 'type': 'dm_crypt',
256+ 'dm_name': 'cryptroot',
257+ 'volume': 'sda-part1',
258+ 'cipher': self.cipher,
259+ 'keysize': self.keysize,
260+ 'keyfile': self.keyfile},
261+ {'device': 'sda',
262+ 'id': 'sda-part2',
263+ 'name': 'sda-part2',
264+ 'number': 2,
265+ 'size': '511705088B',
266+ 'type': 'partition'},
267+ {'id': 'dmcrypt1',
268+ 'type': 'dm_crypt',
269+ 'dm_name': 'cryptfoo',
270+ 'volume': 'sda-part2',
271+ 'cipher': self.cipher,
272+ 'keysize': self.keysize,
273+ 'keyfile': self.keyfile},
274+ ],
275+ }
276+ })
277+
278+ for i in range(2):
279+ self.m_getpath.return_value = self.random_string()
280+ info = self.storage_config['dmcrypt' + str(i)]
281+ block_meta.dm_crypt_handler(
282+ info, self.storage_config, empty_context
283+ )
284+
285+ data = util.load_file(self.crypttab)
286+ self.assertEqual(
287+ f"cryptroot UUID={self.block_uuids[0]} none luks\n"
288+ f"cryptfoo UUID={self.block_uuids[1]} none luks\n",
289+ data
290+ )
291+
292+ def test_cryptoswap(self):
293+ """ Check the keyfile and options features needed for cryptoswap """
294+
295+ self.setUpStorageConfig({
296+ 'storage': {
297+ 'version': 1,
298+ 'config': [
299+ {'grub_device': True,
300+ 'id': 'sda',
301+ 'name': 'sda',
302+ 'path': '/wark/xxx',
303+ 'ptable': 'msdos',
304+ 'type': 'disk',
305+ 'wipe': 'superblock'},
306+ {'device': 'sda',
307+ 'id': 'sda-part1',
308+ 'name': 'sda-part1',
309+ 'number': 1,
310+ 'size': '511705088B',
311+ 'type': 'partition'},
312+ {'id': 'dmcrypt0',
313+ 'type': 'dm_crypt',
314+ 'dm_name': 'cryptswap',
315+ 'volume': 'sda-part1',
316+ 'options': ["swap", "initramfs"],
317+ 'keyfile': "/dev/urandom"},
318+ ],
319+ }
320+ })
321+
322+ self.m_getpath.return_value = self.random_string()
323+ block_meta.dm_crypt_handler(
324+ self.storage_config['dmcrypt0'], self.storage_config, empty_context
325+ )
326+
327+ uuid = self.block_uuids[0]
328+ self.assertEqual(
329+ f"cryptswap UUID={uuid} /dev/urandom swap,initramfs\n",
330+ util.load_file(self.crypttab),
331+ )
332+
333+
334 class TestRaidHandler(CiTestCase):
335
336 def setUp(self):
337diff --git a/tools/vmtest-system-setup b/tools/vmtest-system-setup
338index 5172741..2f82464 100755
339--- a/tools/vmtest-system-setup
340+++ b/tools/vmtest-system-setup
341@@ -14,6 +14,7 @@ esac
342 DEPS=(
343 build-essential
344 cloud-image-utils
345+ cryptsetup
346 git
347 make
348 net-tools

Subscribers

People subscribed via source and target branches