Merge ~ogayot/curtin:recovery-key into curtin:master

[Olivier Gayot]

block_meta: add luks recovery key if requested

If the storage configuration contains the key "recovery_keyfile", call
cryptsetup luksAddKey after cryptsetup luksFormat so that the key
specified is added as a recovery key (i.e., a normal key in the second
key slot).

Signed-off-by: Olivier Gayot <email address hidden>

This adds support for adding a LUKS recovery key. Something special probably needs to be done for s390x but that's all I have at the moment.

[Server Team CI bot (server-team-bot)]

PASSED: Continuous integration, rev:5d6aed93658f5fb85b4277bdf21c9f10c80d8ebb
https://jenkins.canonical.com/server-team/job/curtin-ci/171/
Executed test runs:
    SUCCESS: https://jenkins.canonical.com/server-team/job/curtin-ci/nodes=metal-amd64/171/
    SUCCESS: https://jenkins.canonical.com/server-team/job/curtin-ci/nodes=metal-arm64/171/
    SUCCESS: https://jenkins.canonical.com/server-team/job/curtin-ci/nodes=metal-ppc64el/171/
    SUCCESS: https://jenkins.canonical.com/server-team/job/curtin-ci/nodes=metal-s390x/171/

Click here to trigger a rebuild:
https://jenkins.canonical.com/server-team/job/curtin-ci/171//rebuild

review: Approve (continuous-integration)

[Michael Hudson-Doyle (mwhudson)]

lgtm

review: Approve

[Dan Bungert (dbungert)]

I think it's fine to not have the zkey part sorted out yet, so long as the Subiquity side is programmed in a way that is consistent.

I suppose in this case it means we shouldn't be asking for a recovery key on s390x if we can't act on it.

review: Approve

[Server Team CI bot (server-team-bot)]

PASSED: Continuous integration, rev:2bfebabc9aa8be019b05d9c2eb0077f83f1a96de
https://jenkins.canonical.com/server-team/job/curtin-ci/187/
Executed test runs:
    SUCCESS: https://jenkins.canonical.com/server-team/job/curtin-ci/nodes=metal-amd64/187/
    SUCCESS: https://jenkins.canonical.com/server-team/job/curtin-ci/nodes=metal-arm64/187/
    SUCCESS: https://jenkins.canonical.com/server-team/job/curtin-ci/nodes=metal-ppc64el/187/
    SUCCESS: https://jenkins.canonical.com/server-team/job/curtin-ci/nodes=metal-s390x/187/

Click here to trigger a rebuild:
https://jenkins.canonical.com/server-team/job/curtin-ci/187//rebuild

review: Approve (continuous-integration)

[Server Team CI bot (server-team-bot)]

Autolanding: FAILED
More details in the following jenkins job:
https://jenkins.canonical.com/server-team/job/curtin-autoland-test/101/
Executed test runs:
    FAILURE: https://jenkins.canonical.com/server-team/job/curtin-autoland-test/nodes=metal-amd64/101/
    FAILURE: https://jenkins.canonical.com/server-team/job/curtin-autoland-test/nodes=metal-arm64/101/
    FAILURE: https://jenkins.canonical.com/server-team/job/curtin-autoland-test/nodes=metal-ppc64el/101/
    FAILURE: https://jenkins.canonical.com/server-team/job/curtin-autoland-test/nodes=metal-s390x/101/

review: Needs Fixing (continuous-integration)

diff --git a/curtin/commands/block_meta.py b/curtin/commands/block_meta.py
index dd6992d..64bf393 100644
--- a/curtin/commands/block_meta.py
+++ b/curtin/commands/block_meta.py
@@ -1645,6 +1645,8 @@ def dm_crypt_handler(info, storage_config, context):
     else:
         raise ValueError("encryption key or keyfile must be specified")
 
+    recovery_keyfile = info.get('recovery_keyfile')
+
     create_dmcrypt = True
     if preserve:
         dm_crypt_verify(dmcrypt_dev, volume_path)
@@ -1688,8 +1690,20 @@ def dm_crypt_handler(info, storage_config, context):
             if keysize:
                 cmd.extend(["--key-size", keysize])
             cmd.extend(["luksFormat", volume_path, keyfile])
+
             util.subp(cmd)
 
+            # Should this be done as well if we are using zkey?
+            if recovery_keyfile is not None:
+                LOG.debug("Adding recovery key to %s", volume_path)
+
+                cmd = [
+                    "cryptsetup", "luksAddKey",
+                    "--key-file", keyfile,
+                    volume_path, recovery_keyfile]
+
+                util.subp(cmd)
+
         cmd = ["cryptsetup", "open", "--type", luks_type, volume_path, dm_name,
                "--key-file", keyfile]
 
diff --git a/tests/unittests/test_commands_block_meta.py b/tests/unittests/test_commands_block_meta.py
index 87e46eb..1a1f65d 100644
--- a/tests/unittests/test_commands_block_meta.py
+++ b/tests/unittests/test_commands_block_meta.py
@@ -2100,6 +2100,58 @@ class TestDmCryptHandler(CiTestCase):
         self.m_subp.assert_has_calls(expected_calls)
         self.assertEqual(len(util.load_file(self.crypttab).splitlines()), 1)
 
+    def test_dm_crypt_calls_cryptsetup_with_recovery_key(self):
+        """ verify dm_crypt calls (format, addKey, open) w/ correct params"""
+        volume_path = self.random_string()
+        self.m_getpath.return_value = volume_path
+
+        recovery_keyfile = self.random_string()
+        config = {
+            'storage': {
+                'version': 1,
+                'config': [
+                    {'grub_device': True,
+                     'id': 'sda',
+                     'name': 'sda',
+                     'path': '/wark/xxx',
+                     'ptable': 'msdos',
+                     'type': 'disk',
+                     'wipe': 'superblock'},
+                    {'device': 'sda',
+                     'id': 'sda-part1',
+                     'name': 'sda-part1',
+                     'number': 1,
+                     'size': '511705088B',
+                     'type': 'partition'},
+                    {'id': 'dmcrypt0',
+                     'type': 'dm_crypt',
+                     'dm_name': 'cryptroot',
+                     'volume': 'sda-part1',
+                     'cipher': self.cipher,
+                     'keysize': self.keysize,
+                     'keyfile': self.keyfile,
+                     'recovery_keyfile': recovery_keyfile},
+                ],
+            }
+        }
+        storage_config = block_meta.extract_storage_ordered_dict(config)
+
+        info = storage_config['dmcrypt0']
+
+        block_meta.dm_crypt_handler(info, storage_config, empty_context)
+        expected_calls = [
+            call(['cryptsetup', '--cipher', self.cipher,
+                  '--key-size', self.keysize,
+                  'luksFormat', volume_path, self.keyfile]),
+            call(['cryptsetup', 'luksAddKey',
+                  '--key-file', self.keyfile,
+                  volume_path, recovery_keyfile]),
+            call(['cryptsetup', 'open', '--type', 'luks', volume_path,
+                  info['dm_name'], '--key-file', self.keyfile])
+        ]
+        self.m_subp.assert_has_calls(expected_calls)
+        self.assertEqual(len(util.load_file(self.crypttab).splitlines()), 1)
+
     def test_dm_crypt_defaults_dm_name_to_id(self):
         """ verify dm_crypt_handler falls back to id with no dm_name. """
         volume_path = self.random_string()