From 7b7fa06aa918c5b916a46d02aae4c72b51810912 Mon Sep 17 00:00:00 2001 From: Steffen Gufler Date: Thu, 8 Feb 2024 17:52:30 +0100 Subject: [PATCH 1/8] luks_device: fix remove_keyslot not working when set to 0 --- plugins/modules/luks_device.py | 2 +- .../tasks/tests/keyslot-create-destroy.yml | 28 +++++++++++++++++++ 2 files changed, 29 insertions(+), 1 deletion(-) diff --git a/plugins/modules/luks_device.py b/plugins/modules/luks_device.py index 64c498706..524059e40 100644 --- a/plugins/modules/luks_device.py +++ b/plugins/modules/luks_device.py @@ -923,7 +923,7 @@ 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 self._module.params['remove_keyslot'] is not None: 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']) 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 238f42007..51a3db362 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 @@ -176,3 +176,31 @@ - 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" + +- name: Add key in slot 0 + luks_device: + device: "{{ cryptfile_device }}" + state: present + keyfile: "{{ remote_tmp_dir }}/keyfile2" + new_keyfile: "{{ remote_tmp_dir }}/keyfile1" + new_keyslot: 0 + pbkdf: + iteration_time: 0.1 + become: true + register: add_luks_slot0 +- name: Remove key in slot 0 + luks_device: + device: "{{ cryptfile_device }}" + keyfile: "{{ remote_tmp_dir }}/keyfile2" + remove_keyslot: 0 + become: true + register: kill_luks_slot0 +- name: Dump luks header + command: "cryptsetup luksDump {{ cryptfile_device }}" + become: true + register: luks_header_slot0_removed +- assert: + that: + - add_luks_slot0 is changed + - kill_luks_slot0 is changed + - "'Key Slot 0: DISABLED' in luks_header_slot0_removed.stdout or not '0: luks' in luks_header_slot0_removed.stdout" From 305f7c990f2be6c54886c170df83012e71a19314 Mon Sep 17 00:00:00 2001 From: Steffen Gufler Date: Thu, 8 Feb 2024 22:55:47 +0100 Subject: [PATCH 2/8] luks_device: fix module outputting 'ok' when trying to add a key that is already present in another keyslot --- plugins/modules/luks_device.py | 9 +++- .../tasks/tests/keyslot-duplicate.yml | 44 +++++++++++++++++++ 2 files changed, 52 insertions(+), 1 deletion(-) create mode 100644 tests/integration/targets/luks_device/tasks/tests/keyslot-duplicate.yml diff --git a/plugins/modules/luks_device.py b/plugins/modules/luks_device.py index 524059e40..edef12060 100644 --- a/plugins/modules/luks_device.py +++ b/plugins/modules/luks_device.py @@ -909,7 +909,14 @@ def luks_add_key(self): self._module.fail_json(msg="Contradiction in setup: Asking to " "add a key to absent LUKS.") - return not self._crypthandler.luks_test_key(self.device, self._module.params['new_keyfile'], self._module.params['new_passphrase']) + key_present = self._crypthandler.luks_test_key(self.device, self._module.params['new_keyfile'], self._module.params['new_passphrase']) + if self._module.params['new_keyslot'] is not None: + key_present_slot = self._crypthandler.luks_test_key(self.device, self._module.params['new_keyfile'], self._module.params['new_passphrase'], + keyslot=self._module.params['new_keyslot']) + if key_present and not key_present_slot: + self._module.fail_json(msg="Trying to add key that is already present in another slot") + + return not key_present def luks_remove_key(self): if (self.device is None or diff --git a/tests/integration/targets/luks_device/tasks/tests/keyslot-duplicate.yml b/tests/integration/targets/luks_device/tasks/tests/keyslot-duplicate.yml new file mode 100644 index 000000000..bc181d0fc --- /dev/null +++ b/tests/integration/targets/luks_device/tasks/tests/keyslot-duplicate.yml @@ -0,0 +1,44 @@ +--- +# 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 new luks + luks_device: + device: "{{ cryptfile_device }}" + state: present + keyfile: "{{ remote_tmp_dir }}/keyfile1" + pbkdf: + iteration_time: 0.1 + become: true +- name: Add new keyslot with same keyfile + luks_device: + device: "{{ cryptfile_device }}" + state: present + new_keyslot: 1 + keyfile: "{{ remote_tmp_dir }}/keyfile1" + new_keyfile: "{{ remote_tmp_dir }}/keyfile1" + pbkdf: + iteration_time: 0.1 + become: true + ignore_errors: true + check_mode: true + register: keyslot_duplicate_check +- name: Add new keyslot with same keyfile + luks_device: + device: "{{ cryptfile_device }}" + state: present + new_keyslot: 1 + keyfile: "{{ remote_tmp_dir }}/keyfile1" + new_keyfile: "{{ remote_tmp_dir }}/keyfile1" + pbkdf: + iteration_time: 0.1 + become: true + ignore_errors: true + register: keyslot_duplicate +- assert: + that: + - keyslot_duplicate_check is failed + - "'Trying to add key that is already present in another slot' in keyslot_duplicate_check.msg" + - keyslot_duplicate is failed + - "'Trying to add key that is already present in another slot' in keyslot_duplicate.msg" From ffff75cba36e9d4debf046ff7e90bb1392a4bf1b Mon Sep 17 00:00:00 2001 From: Steffen Gufler Date: Thu, 8 Feb 2024 23:16:45 +0100 Subject: [PATCH 3/8] luks_device: fix breaking unit tests --- tests/unit/plugins/modules/test_luks_device.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/unit/plugins/modules/test_luks_device.py b/tests/unit/plugins/modules/test_luks_device.py index f6353893a..371001827 100644 --- a/tests/unit/plugins/modules/test_luks_device.py +++ b/tests/unit/plugins/modules/test_luks_device.py @@ -275,6 +275,7 @@ def test_luks_add_key(device, keyfile, passphrase, new_keyfile, new_passphrase, module.params["passphrase"] = passphrase module.params["new_keyfile"] = new_keyfile module.params["new_passphrase"] = new_passphrase + module.params["new_keyslot"] = None module.params["state"] = state module.params["label"] = label From b4426813ccad6012d6e8299e268c6878daeddb64 Mon Sep 17 00:00:00 2001 From: Steffen Gufler Date: Sat, 10 Feb 2024 11:31:08 +0100 Subject: [PATCH 4/8] luks_device: Duplicate key test case code cleanup --- plugins/modules/luks_device.py | 2 +- .../targets/luks_device/tasks/tests/keyslot-duplicate.yml | 6 +----- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/plugins/modules/luks_device.py b/plugins/modules/luks_device.py index edef12060..2ce1a00ae 100644 --- a/plugins/modules/luks_device.py +++ b/plugins/modules/luks_device.py @@ -912,7 +912,7 @@ def luks_add_key(self): key_present = self._crypthandler.luks_test_key(self.device, self._module.params['new_keyfile'], self._module.params['new_passphrase']) if self._module.params['new_keyslot'] is not None: key_present_slot = self._crypthandler.luks_test_key(self.device, self._module.params['new_keyfile'], self._module.params['new_passphrase'], - keyslot=self._module.params['new_keyslot']) + self._module.params['new_keyslot']) if key_present and not key_present_slot: self._module.fail_json(msg="Trying to add key that is already present in another slot") diff --git a/tests/integration/targets/luks_device/tasks/tests/keyslot-duplicate.yml b/tests/integration/targets/luks_device/tasks/tests/keyslot-duplicate.yml index bc181d0fc..cb9e559a1 100644 --- a/tests/integration/targets/luks_device/tasks/tests/keyslot-duplicate.yml +++ b/tests/integration/targets/luks_device/tasks/tests/keyslot-duplicate.yml @@ -11,15 +11,13 @@ pbkdf: iteration_time: 0.1 become: true -- name: Add new keyslot with same keyfile +- name: Add new keyslot with same keyfile (check) luks_device: device: "{{ cryptfile_device }}" state: present new_keyslot: 1 keyfile: "{{ remote_tmp_dir }}/keyfile1" new_keyfile: "{{ remote_tmp_dir }}/keyfile1" - pbkdf: - iteration_time: 0.1 become: true ignore_errors: true check_mode: true @@ -31,8 +29,6 @@ new_keyslot: 1 keyfile: "{{ remote_tmp_dir }}/keyfile1" new_keyfile: "{{ remote_tmp_dir }}/keyfile1" - pbkdf: - iteration_time: 0.1 become: true ignore_errors: true register: keyslot_duplicate From 434d8514ab6a815aa56e6a6cd3d8c701a1895f70 Mon Sep 17 00:00:00 2001 From: Steffen Gufler Date: Sat, 10 Feb 2024 13:22:43 +0100 Subject: [PATCH 5/8] luks_device: Fix testing of LUKS passphrases when only testing one key slot --- plugins/modules/luks_device.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/plugins/modules/luks_device.py b/plugins/modules/luks_device.py index 2ce1a00ae..e3668b16c 100644 --- a/plugins/modules/luks_device.py +++ b/plugins/modules/luks_device.py @@ -799,6 +799,11 @@ def luks_test_key(self, device, keyfile, passphrase, keyslot=None): if 'No usable keyslot is available.' in result[output]: return False + # This check is necessary due to cryptsetup in version 2.0.3 not printing 'No usable keyslot is available' + # when using the --key-slot parameter in combination with --test-passphrase + if result[RETURN_CODE] == 1 and keyslot is not None and result[STDOUT] == '' and result[STDERR] == '': + return True + raise ValueError('Error while testing whether keyslot exists on %s: %s' % (device, result[STDERR])) From 23d862f061770e423488e50b454e7f164e7a1581 Mon Sep 17 00:00:00 2001 From: Steffen Gufler Date: Sat, 10 Feb 2024 14:07:50 +0100 Subject: [PATCH 6/8] luks_device: Fix testing of LUKS passphrases when only testing one key slot --- plugins/modules/luks_device.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/modules/luks_device.py b/plugins/modules/luks_device.py index e3668b16c..5c41eddd3 100644 --- a/plugins/modules/luks_device.py +++ b/plugins/modules/luks_device.py @@ -802,7 +802,7 @@ def luks_test_key(self, device, keyfile, passphrase, keyslot=None): # This check is necessary due to cryptsetup in version 2.0.3 not printing 'No usable keyslot is available' # when using the --key-slot parameter in combination with --test-passphrase if result[RETURN_CODE] == 1 and keyslot is not None and result[STDOUT] == '' and result[STDERR] == '': - return True + return False raise ValueError('Error while testing whether keyslot exists on %s: %s' % (device, result[STDERR])) From b6fad10aaf9372473dee67dd5d5cb4df9561daac Mon Sep 17 00:00:00 2001 From: Steffen Gufler Date: Sun, 11 Feb 2024 08:58:26 +0100 Subject: [PATCH 7/8] luks_device: Add changelog fragment for PR #710 --- changelogs/fragments/710-luks_device-keyslot-fixes.yml | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 changelogs/fragments/710-luks_device-keyslot-fixes.yml diff --git a/changelogs/fragments/710-luks_device-keyslot-fixes.yml b/changelogs/fragments/710-luks_device-keyslot-fixes.yml new file mode 100644 index 000000000..7f6895fce --- /dev/null +++ b/changelogs/fragments/710-luks_device-keyslot-fixes.yml @@ -0,0 +1,4 @@ +bugfixes: + - "luks_device - Fixed module a bug that prevented using `remove_keyslot` with the value `0` (https://github.com/ansible-collections/community.crypto/pull/710)." + - "luks_device - Fixed module falsely outputting 'ok' when trying to add a new slot with a key that is already present in another slot. The module now rejects adding keys that are already present in another slot (https://github.com/ansible-collections/community.crypto/pull/710)." + - "luks_device - Fixed testing of LUKS passphrases in when specifying a keyslot for cryptsetup version 2.0.3. The output of this cryptsetup version slightly differs from later versions (https://github.com/ansible-collections/community.crypto/pull/710)." \ No newline at end of file From 7729dfa7dd09b55ec2f88dad963ce86ecc40b93e Mon Sep 17 00:00:00 2001 From: Steffen Gufler Date: Sun, 11 Feb 2024 11:53:05 +0100 Subject: [PATCH 8/8] luks_device: Update changlog fragment --- changelogs/fragments/710-luks_device-keyslot-fixes.yml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/changelogs/fragments/710-luks_device-keyslot-fixes.yml b/changelogs/fragments/710-luks_device-keyslot-fixes.yml index 7f6895fce..323922c08 100644 --- a/changelogs/fragments/710-luks_device-keyslot-fixes.yml +++ b/changelogs/fragments/710-luks_device-keyslot-fixes.yml @@ -1,4 +1,4 @@ bugfixes: - - "luks_device - Fixed module a bug that prevented using `remove_keyslot` with the value `0` (https://github.com/ansible-collections/community.crypto/pull/710)." - - "luks_device - Fixed module falsely outputting 'ok' when trying to add a new slot with a key that is already present in another slot. The module now rejects adding keys that are already present in another slot (https://github.com/ansible-collections/community.crypto/pull/710)." - - "luks_device - Fixed testing of LUKS passphrases in when specifying a keyslot for cryptsetup version 2.0.3. The output of this cryptsetup version slightly differs from later versions (https://github.com/ansible-collections/community.crypto/pull/710)." \ No newline at end of file + - "luks_device - fixed module a bug that prevented using ``remove_keyslot`` with the value ``0`` (https://github.com/ansible-collections/community.crypto/pull/710)." + - "luks_device - fixed module falsely outputting ``changed=false`` when trying to add a new slot with a key that is already present in another slot. The module now rejects adding keys that are already present in another slot (https://github.com/ansible-collections/community.crypto/pull/710)." + - "luks_device - fixed testing of LUKS passphrases in when specifying a keyslot for cryptsetup version 2.0.3. The output of this cryptsetup version slightly differs from later versions (https://github.com/ansible-collections/community.crypto/pull/710)." \ No newline at end of file