From 4573f349ea70cbc6b3514ba6c7f5c572d815faca Mon Sep 17 00:00:00 2001 From: Felix Fontein Date: Thu, 14 Mar 2019 10:55:35 +0100 Subject: [PATCH] luks_device: add allow_to_remove_last_key option (#52371) * Add allow_to_remove_last_key option. * Dump headers. * Add support for old versions of cryptsetup luksDump. * Update lib/ansible/modules/crypto/luks_device.py Co-Authored-By: felixfontein * Rename allow_to_remove_last_key -> force_remove_last_key. --- lib/ansible/modules/crypto/luks_device.py | 55 +++++++++++++++++-- .../tasks/tests/key-management.yml | 39 +++++++++++++ 2 files changed, 88 insertions(+), 6 deletions(-) diff --git a/lib/ansible/modules/crypto/luks_device.py b/lib/ansible/modules/crypto/luks_device.py index 128e0876146..4efb3eeded6 100644 --- a/lib/ansible/modules/crypto/luks_device.py +++ b/lib/ansible/modules/crypto/luks_device.py @@ -70,6 +70,9 @@ options: Needs I(keyfile) option for authorization. LUKS container supports up to 8 keys. Parameter value is the path to the keyfile with the passphrase." + - "NOTE that adding additional keys is I(not idempotent). + A new keyslot will be used even if another keyslot already + exists for this keyfile." - "BEWARE that working with keyfiles in plaintext is dangerous. Make sure that they are protected." type: path @@ -78,12 +81,20 @@ options: - "Removes given key from the container on I(device). Does not remove the keyfile from filesystem. Parameter value is the path to the keyfile with the passphrase." - - "BEWARE that it is possible to remove even the last key from the - container. Data in there will be irreversibly lost - without a warning." + - "NOTE that removing keys is I(not idempotent). Trying to remove + a key which no longer exists results in an error." + - "NOTE that to remove the last key from a LUKS container, the + I(force_remove_last_key) option must be set to C(yes)." - "BEWARE that working with keyfiles in plaintext is dangerous. Make sure that they are protected." type: path + force_remove_last_key: + description: + - "If set to C(yes), allows removing the last key from a container." + - "BEWARE that when the last key has been removed from a container, + the container can no longer be opened!" + type: bool + default: no requirements: - "cryptsetup" @@ -284,10 +295,40 @@ class CryptHandler(Handler): raise ValueError('Error while adding new LUKS key to %s: %s' % (device, result[STDERR])) - def run_luks_remove_key(self, device, keyfile): + def run_luks_remove_key(self, device, keyfile, force_remove_last_key=False): ''' Remove key from given device Raises ValueError when command fails ''' + if not force_remove_last_key: + result = self._run_command([self._cryptsetup_bin, 'luksDump', device]) + if result[RETURN_CODE] != 0: + raise ValueError('Error while dumping LUKS header from %s' + % (device, )) + keyslot_count = 0 + keyslot_area = False + keyslot_re = re.compile(r'^Key Slot [0-9]+: ENABLED') + for line in result[STDOUT].splitlines(): + if line.startswith('Keyslots:'): + keyslot_area = True + elif line.startswith(' '): + # LUKS2 header dumps use human-readable indented output. + # Thus we have to look out for 'Keyslots:' and count the + # number of indented keyslot numbers. + if keyslot_area and line[2] in '0123456789': + keyslot_count += 1 + elif line.startswith('\t'): + pass + elif keyslot_re.match(line): + # LUKS1 header dumps have one line per keyslot with ENABLED + # or DISABLED in them. We count such lines with ENABLED. + keyslot_count += 1 + else: + keyslot_area = False + if keyslot_count < 2: + self._module.fail_json(msg="LUKS device %s has less than two active keyslots. " + "To be able to remove a key, please set " + "`force_remove_last_key` to `yes`." % device) + result = self._run_command([self._cryptsetup_bin, 'luksRemoveKey', device, '-q', '--key-file', keyfile]) if result[RETURN_CODE] != 0: @@ -412,7 +453,8 @@ def run_module(): name=dict(type='str'), keyfile=dict(type='path'), new_keyfile=dict(type='path'), - remove_keyfile=dict(type='path') + remove_keyfile=dict(type='path'), + force_remove_last_key=dict(type='bool', default=False), ) # seed the result dict in the object @@ -491,7 +533,8 @@ def run_module(): if conditions.luks_remove_key(): try: crypt.run_luks_remove_key(module.params['device'], - module.params['remove_keyfile']) + module.params['remove_keyfile'], + force_remove_last_key=module.params['force_remove_last_key']) except ValueError as e: module.fail_json(msg="luks_device error: %s" % e) result['changed'] = True diff --git a/test/integration/targets/luks_device/tasks/tests/key-management.yml b/test/integration/targets/luks_device/tasks/tests/key-management.yml index 963d05a7a0a..1866a46dbc9 100644 --- a/test/integration/targets/luks_device/tasks/tests/key-management.yml +++ b/test/integration/targets/luks_device/tasks/tests/key-management.yml @@ -62,6 +62,9 @@ device: "{{ cryptfile_device }}" state: closed +- name: Dump LUKS header + command: "cryptsetup luksDump {{ cryptfile_device }}" + - name: Remove access from keyfile1 luks_device: device: "{{ cryptfile_device }}" @@ -100,6 +103,9 @@ device: "{{ cryptfile_device }}" state: closed +- name: Dump LUKS header + command: "cryptsetup luksDump {{ cryptfile_device }}" + - name: Remove access from keyfile2 luks_device: device: "{{ cryptfile_device }}" @@ -107,6 +113,39 @@ keyfile: "{{ role_path }}/files/keyfile2" remove_keyfile: "{{ role_path }}/files/keyfile2" become: yes + ignore_errors: yes + register: remove_last_key +- assert: + that: + - remove_last_key is failed + - "'force_remove_last_key' in remove_last_key.msg" + +# Access: keyfile2 + +- name: Try to open with keyfile2 + luks_device: + device: "{{ cryptfile_device }}" + state: opened + keyfile: "{{ role_path }}/files/keyfile2" + become: yes + ignore_errors: yes + register: open_try +- assert: + that: + - open_try is not failed +- name: Close + luks_device: + device: "{{ cryptfile_device }}" + state: closed + +- name: Remove access from keyfile2 + luks_device: + device: "{{ cryptfile_device }}" + state: closed + keyfile: "{{ role_path }}/files/keyfile2" + remove_keyfile: "{{ role_path }}/files/keyfile2" + force_remove_last_key: yes + become: yes # Access: none