luks_device: fix remove_keyslot not working when set to 0 and duplicate keys (#710)

* luks_device: fix remove_keyslot not working when set to 0

* luks_device: fix module outputting 'ok' when trying to add a key that is already present in another keyslot

* luks_device: fix breaking unit tests

* luks_device: Duplicate key test case code cleanup

* luks_device: Fix testing of LUKS passphrases when only testing one key slot

* luks_device: Fix testing of LUKS passphrases when only testing one key slot

* luks_device: Add changelog fragment for PR #710

* luks_device: Update changlog fragment

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 ``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)."

plugins/modules/luks_device.py

@@ -799,6 +799,11 @@ class CryptHandler(Handler):
             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 False
+
         raise ValueError('Error while testing whether keyslot exists on %s: %s'
                          % (device, result[STDERR]))
 
@@ -909,7 +914,14 @@ class ConditionsHandler(Handler):
             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'],
+                                                                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
@@ -923,7 +935,7 @@ class ConditionsHandler(Handler):
             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'])

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"

tests/integration/targets/luks_device/tasks/tests/keyslot-duplicate.yml

@@ -0,0 +1,40 @@
+---
+# 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 (check)
+  luks_device:
+    device: "{{ cryptfile_device }}"
+    state: present
+    new_keyslot: 1
+    keyfile: "{{ remote_tmp_dir }}/keyfile1"
+    new_keyfile: "{{ remote_tmp_dir }}/keyfile1"
+  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"
+  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"

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