From 1ec0cd8af8202a60306ed0ca996dbd37229022b2 Mon Sep 17 00:00:00 2001 From: Steffen Gufler Date: Sun, 15 Oct 2023 16:19:02 +0200 Subject: [PATCH 1/8] luks_device: add support for keyslots --- plugins/modules/luks_device.py | 140 +++++++++++++++- .../tasks/tests/keyslot-create-destroy.yml | 153 ++++++++++++++++++ .../tasks/tests/keyslot-options.yml | 86 ++++++++++ 3 files changed, 372 insertions(+), 7 deletions(-) create mode 100644 tests/integration/targets/luks_device/tasks/tests/keyslot-create-destroy.yml create mode 100644 tests/integration/targets/luks_device/tasks/tests/keyslot-options.yml diff --git a/plugins/modules/luks_device.py b/plugins/modules/luks_device.py index c2ca47f0b..2a113d121 100644 --- a/plugins/modules/luks_device.py +++ b/plugins/modules/luks_device.py @@ -79,6 +79,16 @@ value is a string with the passphrase." type: str version_added: '1.0.0' + keyslot: + description: + - "Adds the O(keyfile) or O(passphrase) to a specific keyslot when + creating a new container on O(device). Parameter value is the + number of the keyslot." + - "NOTE that a device of O(type) luks1 supports the keyslot numbers + 0-7 and a device of O(type) luks2 supports the keyslot numbers + 0-32. In order to use the keyslots 8-31 when creating a new + container, setting O(type) to luks2 is required." + type: int keysize: description: - "Sets the key size only if LUKS container does not exist." @@ -108,6 +118,15 @@ be used even if another keyslot already exists for this passphrase." type: str version_added: '1.0.0' + new_keyslot: + description: + - "Adds the additional O(new_keyfile) or O(new_passphrase) to a + specific keyslot on the given O(device). Parameter value is the number + of the keyslot." + - "NOTE that a device of O(type) luks1 supports the keyslot numbers + 0-7 and a device of O(type) luks2 supports the keyslot numbers + 0-32." + type: int remove_keyfile: description: - "Removes given key from the container on O(device). Does not @@ -133,6 +152,16 @@ to V(true)." type: str version_added: '1.0.0' + remove_keyslot: + description: + - "Removes the key in the given slot on O(device). Needs + O(keyfile) or O(passphrase) for authorization." + - "NOTE that a device of O(type) luks1 supports the keyslot numbers + 0-7 and a device of O(type) luks2 supports the keyslot numbers + 0-32." + - "NOTE that the given O(keyfile) or O(passphrase) must not be + in the slot to be removed" + type: int force_remove_last_key: description: - "If set to V(true), allows removing the last key from a container." @@ -377,6 +406,26 @@ state: "present" keyfile: "/vault/keyfile" type: luks2 + +- name: Create a container with key in slot 4 + community.crypto.luks_device: + device: "/dev/loop0" + state: "present" + keyfile: "/vault/keyfile" + keyslot: 4 + +- name: Add a new key in slot 5 + community.crypto.luks_device: + device: "/dev/loop0" + keyfile: "/vault/keyfile" + new_keyfile: "/vault/keyfile" + new_keyslot: 5 + +- name: Remove the key from slot 4 (given keyfile must not be slot 4) + community.crypto.luks_device: + device: "/dev/loop0" + keyfile: "/vault/keyfile" + remove_keyslot: 4 ''' RETURN = ''' @@ -523,6 +572,27 @@ def is_luks(self, device): result = self._run_command([self._cryptsetup_bin, 'isLuks', device]) return result[RETURN_CODE] == 0 + def get_luks_type(self, device): + ''' get the luks type of a device + ''' + if self.is_luks(device): + result = self._run_command([self._cryptsetup_bin, 'luksDump', device]) + for line in result[STDOUT].splitlines(): + if 'Version' in line: + version = line.split()[1] + return 'luks2' if version == '2' else 'luks1' + return None + + def is_luks_slot_set(self, device, keyslot): + ''' check if a keyslot is set + ''' + luks_header = result = self._run_command([self._cryptsetup_bin, 'luksDump', device]) + if result[RETURN_CODE] != 0: + raise ValueError('Error while dumping LUKS header from %s' % (device, )) + result_luks1 = f'Key Slot {keyslot}: ENABLED' in luks_header[STDOUT] + result_luks2 = f'{keyslot}: luks2' in luks_header[STDOUT] + return result_luks1 or result_luks2 + def _add_pbkdf_options(self, options, pbkdf): if pbkdf['iteration_time'] is not None: options.extend(['--iter-time', str(int(pbkdf['iteration_time'] * 1000))]) @@ -535,7 +605,7 @@ def _add_pbkdf_options(self, options, pbkdf): if pbkdf['parallel'] is not None: options.extend(['--pbkdf-parallel', str(pbkdf['parallel'])]) - def run_luks_create(self, device, keyfile, passphrase, keysize, cipher, hash_, sector_size, pbkdf): + def run_luks_create(self, device, keyfile, passphrase, keyslot, keysize, cipher, hash_, sector_size, pbkdf): # create a new luks container; use batch mode to auto confirm luks_type = self._module.params['type'] label = self._module.params['label'] @@ -556,6 +626,8 @@ def run_luks_create(self, device, keyfile, passphrase, keysize, cipher, hash_, s self._add_pbkdf_options(options, pbkdf) if sector_size is not None: options.extend(['--sector-size', str(sector_size)]) + if keyslot is not None: + options.extend(['--key-slot', str(keyslot)]) args = [self._cryptsetup_bin, 'luksFormat'] args.extend(options) @@ -615,7 +687,7 @@ def run_luks_remove(self, device): raise ValueError('Error while wiping LUKS container signatures for %s: %s' % (device, exc)) def run_luks_add_key(self, device, keyfile, passphrase, new_keyfile, - new_passphrase, pbkdf): + new_passphrase, new_keyslot, pbkdf): ''' Add new key from a keyfile or passphrase to given 'device'; authentication done using 'keyfile' or 'passphrase'. Raises ValueError when command fails. @@ -625,6 +697,9 @@ def run_luks_add_key(self, device, keyfile, passphrase, new_keyfile, if pbkdf is not None: self._add_pbkdf_options(args, pbkdf) + if new_keyslot is not None: + args.extend(['--key-slot', str(new_keyslot)]) + if keyfile: args.extend(['--key-file', keyfile]) else: @@ -640,7 +715,7 @@ def run_luks_add_key(self, device, keyfile, passphrase, new_keyfile, raise ValueError('Error while adding new LUKS keyslot to %s: %s' % (device, result[STDERR])) - def run_luks_remove_key(self, device, keyfile, passphrase, + def run_luks_remove_key(self, device, keyfile, passphrase, keyslot, force_remove_last_key=False): ''' Remove key from given device Raises ValueError when command fails @@ -675,7 +750,10 @@ def run_luks_remove_key(self, device, keyfile, passphrase, "To be able to remove a key, please set " "`force_remove_last_key` to `true`." % device) - args = [self._cryptsetup_bin, 'luksRemoveKey', device, '-q'] + if keyslot is None: + args = [self._cryptsetup_bin, 'luksRemoveKey', device, '-q'] + else: + args = [self._cryptsetup_bin, 'luksKillSlot', device, '-q', str(keyslot)] if keyfile: args.extend(['--key-file', keyfile]) result = self._run_command(args, data=passphrase) @@ -683,7 +761,7 @@ def run_luks_remove_key(self, device, keyfile, passphrase, raise ValueError('Error while removing LUKS key from %s: %s' % (device, result[STDERR])) - def luks_test_key(self, device, keyfile, passphrase): + def luks_test_key(self, device, keyfile, passphrase, keyslot=None): ''' Check whether the keyfile or passphrase works. Raises ValueError when command fails. ''' @@ -695,12 +773,18 @@ def luks_test_key(self, device, keyfile, passphrase): else: data = passphrase + if keyslot is not None: + args.extend(['--key-slot', str(keyslot)]) + result = self._run_command(args, data=data) + print(f'test_keyslot_result: {result}') if result[RETURN_CODE] == 0: return True for output in (STDOUT, STDERR): if 'No key available with this passphrase' in result[output]: return False + if 'No usable keyslot is available.' in result[output]: + return False raise ValueError('Error while testing whether keyslot exists on %s: %s' % (device, result[STDERR])) @@ -817,7 +901,8 @@ def luks_add_key(self): def luks_remove_key(self): if (self.device is None or (self._module.params['remove_keyfile'] is None and - self._module.params['remove_passphrase'] is None)): + self._module.params['remove_passphrase'] is None and + self._module.params['remove_keyslot'] is None)): # conditions for removing a key not fulfilled return False @@ -825,6 +910,15 @@ def luks_remove_key(self): self._module.fail_json(msg="Contradiction in setup: Asking to " "remove a key from absent LUKS.") + if self._module.params['remove_keyslot']: + if not self._crypthandler.is_luks_slot_set(self.device, self._module.params['remove_keyslot']): + return False + result = self._crypthandler.luks_test_key(self.device, self._module.params['keyfile'], self._module.params['passphrase']) + if self._crypthandler.luks_test_key(self.device, self._module.params['keyfile'], self._module.params['passphrase'], + self._module.params['remove_keyslot']): + self._module.fail_json(msg='Cannot remove keyslot with keyfile or passphrase in same slot') + return result + return self._crypthandler.luks_test_key(self.device, self._module.params['remove_keyfile'], self._module.params['remove_passphrase']) def luks_remove(self): @@ -833,6 +927,15 @@ def luks_remove(self): self._crypthandler.is_luks(self.device)) +def validate_keyslot(keyslot, luks_type): + if luks_type == 'luks1' and not 0 <= keyslot <= 7: + return False + elif luks_type == 'luks2' and not 0 <= keyslot <= 31: + return False + else: + return True + + def run_module(): # available arguments/parameters that a user can pass module_args = dict( @@ -845,6 +948,9 @@ def run_module(): passphrase=dict(type='str', no_log=True), new_passphrase=dict(type='str', no_log=True), remove_passphrase=dict(type='str', no_log=True), + keyslot=dict(type='int'), + new_keyslot=dict(type='int'), + remove_keyslot=dict(type='int'), force_remove_last_key=dict(type='bool', default=False), keysize=dict(type='int'), label=dict(type='str'), @@ -874,7 +980,7 @@ def run_module(): mutually_exclusive = [ ('keyfile', 'passphrase'), ('new_keyfile', 'new_passphrase'), - ('remove_keyfile', 'remove_passphrase') + ('remove_keyfile', 'remove_passphrase', 'remove_keyslot') ] # seed the result dict in the object @@ -904,6 +1010,22 @@ def run_module(): if module.params['label'] is not None and module.params['type'] == 'luks1': module.fail_json(msg='You cannot combine type luks1 with the label option.') + if module.params['remove_keyslot'] is not None: + if module.params['keyfile'] is None and module.params['passphrase'] is None: + module.fail_json(msg='Removing a keyslot requires the passphrase or keyfile of another slot') + + luks_type = crypt.get_luks_type(conditions.get_device_name()) + for parameter in ['keyslot', 'new_keyslot', 'remove_keyslot']: + if module.params[parameter] is not None and not validate_keyslot(module.params[parameter], luks_type): + if luks_type == 'luks1': + module.fail_json(msg=f'{parameter} must be between 0 and 7 when using luks1') + else: + module.fail_json(msg=f'{parameter} must be between 0 and 31 when using luks2') + if conditions.luks_create() and module.params['keyslot'] is not None and module.params['type'] is None: + keyslot = module.params['keyslot'] + if validate_keyslot(keyslot, 'luks2') and not validate_keyslot(keyslot, 'luks1'): + module.fail_json(msg='You must specify type=luks2 when creating a new luks device to use keyslots 8-31') + # The conditions are in order to allow more operations in one run. # (e.g. create luks and add a key to it) @@ -914,6 +1036,7 @@ def run_module(): crypt.run_luks_create(conditions.device, module.params['keyfile'], module.params['passphrase'], + module.params['keyslot'], module.params['keysize'], module.params['cipher'], module.params['hash'], @@ -986,6 +1109,7 @@ def run_module(): module.params['passphrase'], module.params['new_keyfile'], module.params['new_passphrase'], + module.params['new_keyslot'], module.params['pbkdf']) except ValueError as e: module.fail_json(msg="luks_device error: %s" % e) @@ -994,6 +1118,7 @@ def run_module(): module.exit_json(**result) # luks remove key + print(f'cond_luks_remove_key: {conditions.luks_remove_key()}') if conditions.luks_remove_key(): if not module.check_mode: try: @@ -1001,6 +1126,7 @@ def run_module(): crypt.run_luks_remove_key(conditions.device, module.params['remove_keyfile'], module.params['remove_passphrase'], + module.params['remove_keyslot'], force_remove_last_key=last_key) except ValueError as e: module.fail_json(msg="luks_device error: %s" % e) diff --git a/tests/integration/targets/luks_device/tasks/tests/keyslot-create-destroy.yml b/tests/integration/targets/luks_device/tasks/tests/keyslot-create-destroy.yml new file mode 100644 index 000000000..9eed59e28 --- /dev/null +++ b/tests/integration/targets/luks_device/tasks/tests/keyslot-create-destroy.yml @@ -0,0 +1,153 @@ +- name: Create luks with keyslot 4 (check) + luks_device: + device: "{{ cryptfile_device }}" + state: present + keyfile: "{{ remote_tmp_dir }}/keyfile1" + keyslot: 4 + pbkdf: + iteration_time: 0.1 + check_mode: true + become: true + register: create_luks_slot4_check +- name: Create luks with keyslot 4 + luks_device: + device: "{{ cryptfile_device }}" + state: present + keyfile: "{{ remote_tmp_dir }}/keyfile1" + keyslot: 4 + pbkdf: + iteration_time: 0.1 + become: true + register: create_luks_slot4 +- name: Create luks with keyslot 4 (idempotent) + luks_device: + device: "{{ cryptfile_device }}" + state: present + keyfile: "{{ remote_tmp_dir }}/keyfile1" + keyslot: 4 + pbkdf: + iteration_time: 0.1 + become: true + register: create_luks_slot4_idem +- name: Create luks with keyslot 4 (idempotent, check) + luks_device: + device: "{{ cryptfile_device }}" + state: present + keyfile: "{{ remote_tmp_dir }}/keyfile1" + keyslot: 4 + pbkdf: + iteration_time: 0.1 + check_mode: true + become: true + register: create_luks_slot4_idem_check +- name: Dump luks header + command: "cryptsetup luksDump {{ cryptfile_device }}" + become: true + register: luks_header_slot4 +- assert: + that: + - create_luks_slot4_check is changed + - create_luks_slot4 is changed + - create_luks_slot4_idem is not changed + - create_luks_slot4_idem_check is not changed + - "'Key Slot 4: ENABLED' in luks_header_slot4.stdout or '4: luks2' in luks_header_slot4.stdout" + +- name: Add key in slot 2 (check) + luks_device: + device: "{{ cryptfile_device }}" + state: present + keyfile: "{{ remote_tmp_dir }}/keyfile1" + new_keyfile: "{{ remote_tmp_dir }}/keyfile2" + new_keyslot: 2 + pbkdf: + iteration_time: 0.1 + check_mode: true + become: true + register: add_luks_slot2_check +- name: Add key in slot 2 + luks_device: + device: "{{ cryptfile_device }}" + state: present + keyfile: "{{ remote_tmp_dir }}/keyfile1" + new_keyfile: "{{ remote_tmp_dir }}/keyfile2" + new_keyslot: 2 + pbkdf: + iteration_time: 0.1 + become: true + register: add_luks_slot2 +- name: Add key in slot 2 (idempotent) + luks_device: + device: "{{ cryptfile_device }}" + state: present + keyfile: "{{ remote_tmp_dir }}/keyfile1" + new_keyfile: "{{ remote_tmp_dir }}/keyfile2" + new_keyslot: 2 + pbkdf: + iteration_time: 0.1 + become: true + register: add_luks_slot2_idem +- name: Add key in slot 2 (idempotent, check) + luks_device: + device: "{{ cryptfile_device }}" + state: present + keyfile: "{{ remote_tmp_dir }}/keyfile1" + new_keyfile: "{{ remote_tmp_dir }}/keyfile2" + new_keyslot: 2 + pbkdf: + iteration_time: 0.1 + check_mode: true + become: true + register: add_luks_slot2_idem_check +- name: Dump luks header + command: "cryptsetup luksDump {{ cryptfile_device }}" + become: true + register: luks_header_slot2 +- assert: + that: + - add_luks_slot2_check is changed + - add_luks_slot2 is changed + - add_luks_slot2_idem is not changed + - add_luks_slot2_idem_check is not changed + - "'Key Slot 2: ENABLED' in luks_header_slot2.stdout or '2: luks2' in luks_header_slot2.stdout" + +- name: Remove key in slot 4 (check) + luks_device: + device: "{{ cryptfile_device }}" + keyfile: "{{ remote_tmp_dir }}/keyfile2" + remove_keyslot: 4 + check_mode: true + become: true + register: kill_luks_slot4_check +- name: Remove key in slot 4 + luks_device: + device: "{{ cryptfile_device }}" + keyfile: "{{ remote_tmp_dir }}/keyfile2" + remove_keyslot: 4 + become: true + register: kill_luks_slot4 +- name: Remove key in slot 4 (idempotent) + luks_device: + device: "{{ cryptfile_device }}" + keyfile: "{{ remote_tmp_dir }}/keyfile2" + remove_keyslot: 4 + become: true + register: kill_luks_slot4_idem +- name: Remove key in slot 4 (idempotent) + luks_device: + device: "{{ cryptfile_device }}" + keyfile: "{{ remote_tmp_dir }}/keyfile2" + remove_keyslot: 4 + check_mode: true + become: true + register: kill_luks_slot4_idem_check +- name: Dump luks header + command: "cryptsetup luksDump {{ cryptfile_device }}" + become: true + register: luks_header_slot4_removed +- assert: + that: + - kill_luks_slot4_check is changed + - kill_luks_slot4 is changed + - kill_luks_slot4_idem is not changed + - kill_luks_slot4_idem_check is not changed + - "'Key Slot 4: DISABLED' in luks_header_slot4_removed.stdout or not '4: luks' in luks_header_slot4_removed.stdout" diff --git a/tests/integration/targets/luks_device/tasks/tests/keyslot-options.yml b/tests/integration/targets/luks_device/tasks/tests/keyslot-options.yml new file mode 100644 index 000000000..2a5feb2ab --- /dev/null +++ b/tests/integration/targets/luks_device/tasks/tests/keyslot-options.yml @@ -0,0 +1,86 @@ +- name: Check invalid slot (luks1, 8) + luks_device: + device: "{{ cryptfile_device }}" + state: present + type: luks1 + keyfile: "{{ remote_tmp_dir }}/keyfile1" + keyslot: 8 + pbkdf: + iteration_time: 0.1 + ignore_errors: true + become: true + register: create_luks1_slot8 +- name: Check invalid slot (luks2, 32) + luks_device: + device: "{{ cryptfile_device }}" + state: present + type: luks2 + keyfile: "{{ remote_tmp_dir }}/keyfile1" + keyslot: 32 + pbkdf: + iteration_time: 0.1 + ignore_errors: true + become: true + register: create_luks2_slot32 +- name: Check invalid slot (no luks type, 8) + luks_device: + device: "{{ cryptfile_device }}" + state: present + keyfile: "{{ remote_tmp_dir }}/keyfile1" + keyslot: 8 + pbkdf: + iteration_time: 0.1 + ignore_errors: true + become: true + register: create_luks_slot8 +- name: Check valid slot (luks2, 8) + luks_device: + device: "{{ cryptfile_device }}" + state: present + type: luks2 + keyfile: "{{ remote_tmp_dir }}/keyfile1" + keyslot: 8 + pbkdf: + iteration_time: 0.1 + become: true + register: create_luks2_slot8 +- name: Check add valid slot (no luks type, 10) + luks_device: + device: "{{ cryptfile_device }}" + state: present + keyfile: "{{ remote_tmp_dir }}/keyfile1" + new_keyfile: "{{ remote_tmp_dir }}/keyfile2" + new_keyslot: 10 + pbkdf: + iteration_time: 0.1 + become: true + register: create_luks_slot10 +- assert: + that: + - create_luks1_slot8 is failed + - create_luks2_slot32 is failed + - create_luks_slot8 is failed + - create_luks2_slot8 is changed + - create_luks_slot10 is changed + +- name: Check remove slot 8 without key + luks_device: + device: "{{ cryptfile_device }}" + state: absent + remove_keyslot: 8 + ignore_errors: true + become: true + register: kill_slot8_nokey +- name: Check remove slot 8 with slot 8 key + luks_device: + device: "{{ cryptfile_device }}" + state: absent + remove_keyslot: 8 + keyfile: "{{ remote_tmp_dir }}/keyfile1" + ignore_errors: true + become: true + register: kill_slot8_key_slot8 +- assert: + that: + - kill_slot8_nokey is failed + - kill_slot8_key_slot8 is failed \ No newline at end of file From 3090d697056681083a8ec68b2e1dc35c2f95b82d Mon Sep 17 00:00:00 2001 From: Steffen Gufler Date: Sun, 15 Oct 2023 20:49:51 +0200 Subject: [PATCH 2/8] luks_device: replace python3 format strings with python2 format strings, remove print statements --- plugins/modules/luks_device.py | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/plugins/modules/luks_device.py b/plugins/modules/luks_device.py index 2a113d121..5aceb22f3 100644 --- a/plugins/modules/luks_device.py +++ b/plugins/modules/luks_device.py @@ -589,8 +589,8 @@ def is_luks_slot_set(self, device, keyslot): luks_header = result = self._run_command([self._cryptsetup_bin, 'luksDump', device]) if result[RETURN_CODE] != 0: raise ValueError('Error while dumping LUKS header from %s' % (device, )) - result_luks1 = f'Key Slot {keyslot}: ENABLED' in luks_header[STDOUT] - result_luks2 = f'{keyslot}: luks2' in luks_header[STDOUT] + result_luks1 = 'Key Slot %d: ENABLED' % (keyslot) in luks_header[STDOUT] + result_luks2 = '%d: luks2' % (keyslot) in luks_header[STDOUT] return result_luks1 or result_luks2 def _add_pbkdf_options(self, options, pbkdf): @@ -777,7 +777,6 @@ def luks_test_key(self, device, keyfile, passphrase, keyslot=None): args.extend(['--key-slot', str(keyslot)]) result = self._run_command(args, data=data) - print(f'test_keyslot_result: {result}') if result[RETURN_CODE] == 0: return True for output in (STDOUT, STDERR): @@ -948,9 +947,9 @@ def run_module(): passphrase=dict(type='str', no_log=True), new_passphrase=dict(type='str', no_log=True), remove_passphrase=dict(type='str', no_log=True), - keyslot=dict(type='int'), - new_keyslot=dict(type='int'), - remove_keyslot=dict(type='int'), + keyslot=dict(type='int', no_log=False), + new_keyslot=dict(type='int', no_log=False), + remove_keyslot=dict(type='int', no_log=False), force_remove_last_key=dict(type='bool', default=False), keysize=dict(type='int'), label=dict(type='str'), @@ -1018,9 +1017,9 @@ def run_module(): for parameter in ['keyslot', 'new_keyslot', 'remove_keyslot']: if module.params[parameter] is not None and not validate_keyslot(module.params[parameter], luks_type): if luks_type == 'luks1': - module.fail_json(msg=f'{parameter} must be between 0 and 7 when using luks1') + module.fail_json(msg='%s must be between 0 and 7 when using luks1' % (parameter)) else: - module.fail_json(msg=f'{parameter} must be between 0 and 31 when using luks2') + module.fail_json(msg='%s must be between 0 and 31 when using luks2' % (parameter)) if conditions.luks_create() and module.params['keyslot'] is not None and module.params['type'] is None: keyslot = module.params['keyslot'] if validate_keyslot(keyslot, 'luks2') and not validate_keyslot(keyslot, 'luks1'): @@ -1118,7 +1117,6 @@ def run_module(): module.exit_json(**result) # luks remove key - print(f'cond_luks_remove_key: {conditions.luks_remove_key()}') if conditions.luks_remove_key(): if not module.check_mode: try: From d8944fab71aba070cf0e62f2ae6c469b8b147c00 Mon Sep 17 00:00:00 2001 From: Steffen Gufler Date: Sun, 15 Oct 2023 20:51:57 +0200 Subject: [PATCH 3/8] luks_device: add missing copyright information in keyslot integration test files --- .../luks_device/tasks/tests/keyslot-create-destroy.yml | 5 +++++ .../targets/luks_device/tasks/tests/keyslot-options.yml | 5 +++++ 2 files changed, 10 insertions(+) diff --git a/tests/integration/targets/luks_device/tasks/tests/keyslot-create-destroy.yml b/tests/integration/targets/luks_device/tasks/tests/keyslot-create-destroy.yml index 9eed59e28..9ac415aeb 100644 --- a/tests/integration/targets/luks_device/tasks/tests/keyslot-create-destroy.yml +++ b/tests/integration/targets/luks_device/tasks/tests/keyslot-create-destroy.yml @@ -1,3 +1,8 @@ +--- +# Copyright (c) Ansible Project +# GNU General Public License v3.0+ (see LICENSES/GPL-3.0-or-later.txt or https://www.gnu.org/licenses/gpl-3.0.txt) +# SPDX-License-Identifier: GPL-3.0-or-later + - name: Create luks with keyslot 4 (check) luks_device: device: "{{ cryptfile_device }}" diff --git a/tests/integration/targets/luks_device/tasks/tests/keyslot-options.yml b/tests/integration/targets/luks_device/tasks/tests/keyslot-options.yml index 2a5feb2ab..a763bc76b 100644 --- a/tests/integration/targets/luks_device/tasks/tests/keyslot-options.yml +++ b/tests/integration/targets/luks_device/tasks/tests/keyslot-options.yml @@ -1,3 +1,8 @@ +--- +# Copyright (c) Ansible Project +# GNU General Public License v3.0+ (see LICENSES/GPL-3.0-or-later.txt or https://www.gnu.org/licenses/gpl-3.0.txt) +# SPDX-License-Identifier: GPL-3.0-or-later + - name: Check invalid slot (luks1, 8) luks_device: device: "{{ cryptfile_device }}" From 7c203406cdfc31fc98009b8956ed50660b93e2db Mon Sep 17 00:00:00 2001 From: Steffen Gufler Date: Sun, 15 Oct 2023 21:53:39 +0200 Subject: [PATCH 4/8] luks_device: updated failing unit tests for keyslot support --- .../unit/plugins/modules/test_luks_device.py | 29 ++++++++++--------- 1 file changed, 15 insertions(+), 14 deletions(-) diff --git a/tests/unit/plugins/modules/test_luks_device.py b/tests/unit/plugins/modules/test_luks_device.py index c773640c6..f6353893a 100644 --- a/tests/unit/plugins/modules/test_luks_device.py +++ b/tests/unit/plugins/modules/test_luks_device.py @@ -148,16 +148,16 @@ def run_command_check(self, command): # device, remove_key, remove_passphrase, state, label, expected LUKS_REMOVE_KEY_DATA = ( - ("dummy", "key", None, "present", None, True), - (None, "key", None, "present", None, False), - (None, "key", None, "present", "labelName", True), - ("dummy", None, None, "present", None, False), - ("dummy", "key", None, "absent", None, "exception"), - ("dummy", None, "foo", "present", None, True), - (None, None, "foo", "present", None, False), - (None, None, "foo", "present", "labelName", True), - ("dummy", None, None, "present", None, False), - ("dummy", None, "foo", "absent", None, "exception")) + ("dummy", "key", None, None, "present", None, True), + (None, "key", None, None, "present", None, False), + (None, "key", None, None, "present", "labelName", True), + ("dummy", None, None, None, "present", None, False), + ("dummy", "key", None, None, "absent", None, "exception"), + ("dummy", None, "foo", None, "present", None, True), + (None, None, "foo", None, "present", None, False), + (None, None, "foo", None, "present", "labelName", True), + ("dummy", None, None, None, "present", None, False), + ("dummy", None, "foo", None, "absent", None, "exception")) @pytest.mark.parametrize("device, keyfile, passphrase, state, is_luks, " + @@ -291,17 +291,18 @@ def test_luks_add_key(device, keyfile, passphrase, new_keyfile, new_passphrase, assert expected == "exception" -@pytest.mark.parametrize("device, remove_keyfile, remove_passphrase, state, " + - "label, expected", - ((d[0], d[1], d[2], d[3], d[4], d[5]) +@pytest.mark.parametrize("device, remove_keyfile, remove_passphrase, remove_keyslot, " + + "state, label, expected", + ((d[0], d[1], d[2], d[3], d[4], d[5], d[6]) for d in LUKS_REMOVE_KEY_DATA)) -def test_luks_remove_key(device, remove_keyfile, remove_passphrase, state, +def test_luks_remove_key(device, remove_keyfile, remove_passphrase, remove_keyslot, state, label, expected, monkeypatch): module = DummyModule() module.params["device"] = device module.params["remove_keyfile"] = remove_keyfile module.params["remove_passphrase"] = remove_passphrase + module.params["remove_keyslot"] = remove_keyslot module.params["state"] = state module.params["label"] = label From 68f09f7b1391471e1389af811eb4ee2de5e0a89c Mon Sep 17 00:00:00 2001 From: Steffen Gufler Date: Mon, 16 Oct 2023 22:29:22 +0200 Subject: [PATCH 5/8] luks_device: improve detection of luks version --- plugins/modules/luks_device.py | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/plugins/modules/luks_device.py b/plugins/modules/luks_device.py index 5aceb22f3..b03e68c21 100644 --- a/plugins/modules/luks_device.py +++ b/plugins/modules/luks_device.py @@ -576,11 +576,13 @@ def get_luks_type(self, device): ''' get the luks type of a device ''' if self.is_luks(device): - result = self._run_command([self._cryptsetup_bin, 'luksDump', device]) - for line in result[STDOUT].splitlines(): - if 'Version' in line: - version = line.split()[1] - return 'luks2' if version == '2' else 'luks1' + with open(device, 'rb') as f: + f.seek(LUKS2_HEADER_OFFSETS[0]) + data = f.read(LUKS_HEADER_L) + if data == LUKS2_HEADER2: + return 'luks2' + else: + return 'luks1' return None def is_luks_slot_set(self, device, keyslot): From 8230609a9b7d8f53da21d84a1f5a4713adb9a8b3 Mon Sep 17 00:00:00 2001 From: Steffen Gufler Date: Sun, 22 Oct 2023 09:39:38 +0200 Subject: [PATCH 6/8] luks_device: Update documentation on keyslot parameters, minor code improvements --- plugins/modules/luks_device.py | 33 ++++++++++++++++++--------------- 1 file changed, 18 insertions(+), 15 deletions(-) diff --git a/plugins/modules/luks_device.py b/plugins/modules/luks_device.py index b03e68c21..07fbe9f48 100644 --- a/plugins/modules/luks_device.py +++ b/plugins/modules/luks_device.py @@ -84,11 +84,12 @@ - "Adds the O(keyfile) or O(passphrase) to a specific keyslot when creating a new container on O(device). Parameter value is the number of the keyslot." - - "NOTE that a device of O(type) luks1 supports the keyslot numbers - 0-7 and a device of O(type) luks2 supports the keyslot numbers - 0-32. In order to use the keyslots 8-31 when creating a new - container, setting O(type) to luks2 is required." + - "B(Note) that a device of O(type=luks1) supports the keyslot numbers + V(0)-V(7) and a device of O(type=luks2) supports the keyslot numbers + V(0)-V(32). In order to use the keyslots V(8)-V(31) when creating a new + container, setting O(type) to V(luks2) is required." type: int + version_added: '2.16.0' keysize: description: - "Sets the key size only if LUKS container does not exist." @@ -123,10 +124,11 @@ - "Adds the additional O(new_keyfile) or O(new_passphrase) to a specific keyslot on the given O(device). Parameter value is the number of the keyslot." - - "NOTE that a device of O(type) luks1 supports the keyslot numbers - 0-7 and a device of O(type) luks2 supports the keyslot numbers - 0-32." + - "B(Note) that a device of O(type=luks1) supports the keyslot numbers + V(0)-V(7) and a device of O(type=luks2) supports the keyslot numbers + V(0)-V(32)." type: int + version_added: '2.16.0' remove_keyfile: description: - "Removes given key from the container on O(device). Does not @@ -156,12 +158,13 @@ description: - "Removes the key in the given slot on O(device). Needs O(keyfile) or O(passphrase) for authorization." - - "NOTE that a device of O(type) luks1 supports the keyslot numbers - 0-7 and a device of O(type) luks2 supports the keyslot numbers - 0-32." - - "NOTE that the given O(keyfile) or O(passphrase) must not be - in the slot to be removed" + - "B(Note) that a device of O(type=luks1) supports the keyslot numbers + V(0)-V(7) and a device of O(type=luks2) supports the keyslot numbers + V(0)-V(32)." + - "B(Note) that the given O(keyfile) or O(passphrase) must not be + in the slot to be removed." type: int + version_added: '2.16.0' force_remove_last_key: description: - "If set to V(true), allows removing the last key from a container." @@ -588,11 +591,11 @@ def get_luks_type(self, device): def is_luks_slot_set(self, device, keyslot): ''' check if a keyslot is set ''' - luks_header = result = self._run_command([self._cryptsetup_bin, 'luksDump', device]) + result = self._run_command([self._cryptsetup_bin, 'luksDump', device]) if result[RETURN_CODE] != 0: raise ValueError('Error while dumping LUKS header from %s' % (device, )) - result_luks1 = 'Key Slot %d: ENABLED' % (keyslot) in luks_header[STDOUT] - result_luks2 = '%d: luks2' % (keyslot) in luks_header[STDOUT] + result_luks1 = 'Key Slot %d: ENABLED' % (keyslot) in result[STDOUT] + result_luks2 = ' %d: luks2' % (keyslot) in result[STDOUT] return result_luks1 or result_luks2 def _add_pbkdf_options(self, options, pbkdf): From ee501ecb45d0de116b80c02756e94148fd88540c Mon Sep 17 00:00:00 2001 From: Steffen Gufler Date: Sun, 22 Oct 2023 14:44:46 +0200 Subject: [PATCH 7/8] luks_device: improve validation of keyslot parameters, fix tests for systems that do not support luks2 --- plugins/modules/luks_device.py | 45 +++++++++---------- .../tasks/tests/keyslot-create-destroy.yml | 20 +++++++++ .../tasks/tests/keyslot-options.yml | 40 ++++++----------- 3 files changed, 56 insertions(+), 49 deletions(-) diff --git a/plugins/modules/luks_device.py b/plugins/modules/luks_device.py index 07fbe9f48..51d0cfd94 100644 --- a/plugins/modules/luks_device.py +++ b/plugins/modules/luks_device.py @@ -920,7 +920,7 @@ def luks_remove_key(self): result = self._crypthandler.luks_test_key(self.device, self._module.params['keyfile'], self._module.params['passphrase']) if self._crypthandler.luks_test_key(self.device, self._module.params['keyfile'], self._module.params['passphrase'], self._module.params['remove_keyslot']): - self._module.fail_json(msg='Cannot remove keyslot with keyfile or passphrase in same slot') + self._module.fail_json(msg='Cannot remove keyslot with keyfile or passphrase in same slot.') return result return self._crypthandler.luks_test_key(self.device, self._module.params['remove_keyfile'], self._module.params['remove_passphrase']) @@ -930,14 +930,18 @@ def luks_remove(self): self._module.params['state'] == 'absent' and self._crypthandler.is_luks(self.device)) + def validate_keyslot(self, param, luks_type): + if self._module.params[param] is not None: + if luks_type is None and param == 'keyslot': + if 8 <= self._module.params[param] <= 31: + self._module.fail_json(msg="You must specify type=luks2 when creating a new luks device to use keyslots 8-31.") + elif not (0 <= self._module.params[param] <= 7): + self._module.fail_json(msg="When not specifying a type, only the keyslots 0-7 are allowed.") -def validate_keyslot(keyslot, luks_type): - if luks_type == 'luks1' and not 0 <= keyslot <= 7: - return False - elif luks_type == 'luks2' and not 0 <= keyslot <= 31: - return False - else: - return True + if luks_type == 'luks1' and not 0 <= self._module.params[param] <= 7: + self._module.fail_json(msg="%s must be between 0 and 7 when using luks1." % self._module.params[param]) + elif luks_type == 'luks2' and not 0 <= self._module.params[param] <= 31: + self._module.fail_json(msg="%s must be between 0 and 31 when using luks2." % self._module.params[param]) def run_module(): @@ -1014,21 +1018,16 @@ def run_module(): if module.params['label'] is not None and module.params['type'] == 'luks1': module.fail_json(msg='You cannot combine type luks1 with the label option.') - if module.params['remove_keyslot'] is not None: - if module.params['keyfile'] is None and module.params['passphrase'] is None: - module.fail_json(msg='Removing a keyslot requires the passphrase or keyfile of another slot') - - luks_type = crypt.get_luks_type(conditions.get_device_name()) - for parameter in ['keyslot', 'new_keyslot', 'remove_keyslot']: - if module.params[parameter] is not None and not validate_keyslot(module.params[parameter], luks_type): - if luks_type == 'luks1': - module.fail_json(msg='%s must be between 0 and 7 when using luks1' % (parameter)) - else: - module.fail_json(msg='%s must be between 0 and 31 when using luks2' % (parameter)) - if conditions.luks_create() and module.params['keyslot'] is not None and module.params['type'] is None: - keyslot = module.params['keyslot'] - if validate_keyslot(keyslot, 'luks2') and not validate_keyslot(keyslot, 'luks1'): - module.fail_json(msg='You must specify type=luks2 when creating a new luks device to use keyslots 8-31') + if module.params['keyslot'] is not None or module.params['new_keyslot'] is not None or module.params['remove_keyslot'] is not None: + luks_type = crypt.get_luks_type(conditions.get_device_name()) + if luks_type is None and module.params['type'] is not None: + luks_type = module.params['type'] + for param in ['keyslot', 'new_keyslot', 'remove_keyslot']: + conditions.validate_keyslot(param, luks_type) + + for param in ['new_keyslot', 'remove_keyslot']: + if module.params[param] is not None and module.params['keyfile'] is None and module.params['passphrase'] is None: + module.fail_json(msg="Removing a keyslot requires the passphrase or keyfile of antoher slot.") # The conditions are in order to allow more operations in one run. # (e.g. create luks and add a key to it) diff --git a/tests/integration/targets/luks_device/tasks/tests/keyslot-create-destroy.yml b/tests/integration/targets/luks_device/tasks/tests/keyslot-create-destroy.yml index 9ac415aeb..238f42007 100644 --- a/tests/integration/targets/luks_device/tasks/tests/keyslot-create-destroy.yml +++ b/tests/integration/targets/luks_device/tasks/tests/keyslot-create-destroy.yml @@ -115,6 +115,26 @@ - add_luks_slot2_idem_check is not changed - "'Key Slot 2: ENABLED' in luks_header_slot2.stdout or '2: luks2' in luks_header_slot2.stdout" +- name: Check remove slot 4 without key + luks_device: + device: "{{ cryptfile_device }}" + remove_keyslot: 4 + ignore_errors: true + become: true + register: kill_slot4_nokey +- name: Check remove slot 4 with slot 4 key + luks_device: + device: "{{ cryptfile_device }}" + remove_keyslot: 4 + keyfile: "{{ remote_tmp_dir }}/keyfile1" + ignore_errors: true + become: true + register: kill_slot4_key_slot4 +- assert: + that: + - kill_slot4_nokey is failed + - kill_slot4_key_slot4 is failed + - name: Remove key in slot 4 (check) luks_device: device: "{{ cryptfile_device }}" diff --git a/tests/integration/targets/luks_device/tasks/tests/keyslot-options.yml b/tests/integration/targets/luks_device/tasks/tests/keyslot-options.yml index a763bc76b..8a1ca14b3 100644 --- a/tests/integration/targets/luks_device/tasks/tests/keyslot-options.yml +++ b/tests/integration/targets/luks_device/tasks/tests/keyslot-options.yml @@ -38,6 +38,12 @@ ignore_errors: true become: true register: create_luks_slot8 +- assert: + that: + - create_luks1_slot8 is failed + - create_luks2_slot32 is failed + - create_luks_slot8 is failed + - name: Check valid slot (luks2, 8) luks_device: device: "{{ cryptfile_device }}" @@ -48,7 +54,13 @@ pbkdf: iteration_time: 0.1 become: true + ignore_errors: true register: create_luks2_slot8 +- name: Make sure that the previous task only fails if LUKS2 is not supported + assert: + that: + - "'Unknown option --type' in create_luks2_slot8.msg" + when: create_luks2_slot8 is failed - name: Check add valid slot (no luks type, 10) luks_device: device: "{{ cryptfile_device }}" @@ -60,32 +72,8 @@ iteration_time: 0.1 become: true register: create_luks_slot10 + when: create_luks2_slot8 is changed - assert: that: - - create_luks1_slot8 is failed - - create_luks2_slot32 is failed - - create_luks_slot8 is failed - - create_luks2_slot8 is changed - create_luks_slot10 is changed - -- name: Check remove slot 8 without key - luks_device: - device: "{{ cryptfile_device }}" - state: absent - remove_keyslot: 8 - ignore_errors: true - become: true - register: kill_slot8_nokey -- name: Check remove slot 8 with slot 8 key - luks_device: - device: "{{ cryptfile_device }}" - state: absent - remove_keyslot: 8 - keyfile: "{{ remote_tmp_dir }}/keyfile1" - ignore_errors: true - become: true - register: kill_slot8_key_slot8 -- assert: - that: - - kill_slot8_nokey is failed - - kill_slot8_key_slot8 is failed \ No newline at end of file + when: create_luks2_slot8 is changed \ No newline at end of file From 8e5aae965882037dcedfbb69b0499df5b18ad94e Mon Sep 17 00:00:00 2001 From: Steffen Gufler Date: Sun, 22 Oct 2023 19:47:55 +0200 Subject: [PATCH 8/8] luks_device: correct spelling and errors in documentation and output, check all possible locations for LUKS2 header --- plugins/modules/luks_device.py | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/plugins/modules/luks_device.py b/plugins/modules/luks_device.py index 51d0cfd94..f51eddd23 100644 --- a/plugins/modules/luks_device.py +++ b/plugins/modules/luks_device.py @@ -86,7 +86,7 @@ number of the keyslot." - "B(Note) that a device of O(type=luks1) supports the keyslot numbers V(0)-V(7) and a device of O(type=luks2) supports the keyslot numbers - V(0)-V(32). In order to use the keyslots V(8)-V(31) when creating a new + V(0)-V(31). In order to use the keyslots V(8)-V(31) when creating a new container, setting O(type) to V(luks2) is required." type: int version_added: '2.16.0' @@ -126,7 +126,7 @@ of the keyslot." - "B(Note) that a device of O(type=luks1) supports the keyslot numbers V(0)-V(7) and a device of O(type=luks2) supports the keyslot numbers - V(0)-V(32)." + V(0)-V(31)." type: int version_added: '2.16.0' remove_keyfile: @@ -160,7 +160,7 @@ O(keyfile) or O(passphrase) for authorization." - "B(Note) that a device of O(type=luks1) supports the keyslot numbers V(0)-V(7) and a device of O(type=luks2) supports the keyslot numbers - V(0)-V(32)." + V(0)-V(31)." - "B(Note) that the given O(keyfile) or O(passphrase) must not be in the slot to be removed." type: int @@ -580,12 +580,12 @@ def get_luks_type(self, device): ''' if self.is_luks(device): with open(device, 'rb') as f: - f.seek(LUKS2_HEADER_OFFSETS[0]) - data = f.read(LUKS_HEADER_L) - if data == LUKS2_HEADER2: - return 'luks2' - else: - return 'luks1' + for offset in LUKS2_HEADER_OFFSETS: + f.seek(offset) + data = f.read(LUKS_HEADER_L) + if data == LUKS2_HEADER2: + return 'luks2' + return 'luks1' return None def is_luks_slot_set(self, device, keyslot): @@ -934,14 +934,14 @@ def validate_keyslot(self, param, luks_type): if self._module.params[param] is not None: if luks_type is None and param == 'keyslot': if 8 <= self._module.params[param] <= 31: - self._module.fail_json(msg="You must specify type=luks2 when creating a new luks device to use keyslots 8-31.") + self._module.fail_json(msg="You must specify type=luks2 when creating a new LUKS device to use keyslots 8-31.") elif not (0 <= self._module.params[param] <= 7): self._module.fail_json(msg="When not specifying a type, only the keyslots 0-7 are allowed.") if luks_type == 'luks1' and not 0 <= self._module.params[param] <= 7: - self._module.fail_json(msg="%s must be between 0 and 7 when using luks1." % self._module.params[param]) + self._module.fail_json(msg="%s must be between 0 and 7 when using LUKS1." % self._module.params[param]) elif luks_type == 'luks2' and not 0 <= self._module.params[param] <= 31: - self._module.fail_json(msg="%s must be between 0 and 31 when using luks2." % self._module.params[param]) + self._module.fail_json(msg="%s must be between 0 and 31 when using LUKS2." % self._module.params[param]) def run_module(): @@ -1027,7 +1027,7 @@ def run_module(): for param in ['new_keyslot', 'remove_keyslot']: if module.params[param] is not None and module.params['keyfile'] is None and module.params['passphrase'] is None: - module.fail_json(msg="Removing a keyslot requires the passphrase or keyfile of antoher slot.") + module.fail_json(msg="Removing a keyslot requires the passphrase or keyfile of another slot.") # The conditions are in order to allow more operations in one run. # (e.g. create luks and add a key to it)