luks_device - make add/removal of keyfile/passphrase idempotent (#168)

* Update documentation, adjust tests, add changelog fragment.

* Move module unit test to correct place.

* Implement keyfile / passphrase test.

changelogs/fragments/168-luks_device-add-remove-idempotence.yml

@@ -0,0 +1,2 @@
+minor_changes:
+- "luks_device - ``new_keyfile``, ``new_passphrase``, ``remove_keyfile`` and ``remove_passphrase`` are now idempotent (https://github.com/ansible-collections/community.crypto/issues/19, https://github.com/ansible-collections/community.crypto/pull/168)."

plugins/modules/luks_device.py

@@ -81,9 +81,10 @@ options:
               Needs I(keyfile) or I(passphrase) option for authorization.
               LUKS container supports up to 8 keyslots. Parameter value
               is the path to the keyfile with the passphrase."
-            - "NOTE that adding additional keys is *not idempotent*.
+            - "NOTE that adding additional keys is idempotent only since
-              A new keyslot will be used even if another keyslot already
+              community.crypto 1.4.0. For older versions, a new keyslot
-              exists for this keyfile."
+              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
@@ -93,9 +94,9 @@ options:
               Needs I(keyfile) or I(passphrase) option for authorization. LUKS
               container supports up to 8 keyslots. Parameter value is a string
               with the new passphrase."
-            - "NOTE that adding additional passphrase is *not idempotent*.  A
+            - "NOTE that adding additional passphrase is idempotent only since
-              new keyslot will be used even if another keyslot already exists
+              community.crypto 1.4.0. For older versions, a new keyslot will
-              for this passphrase."
+              be used even if another keyslot already exists for this passphrase."
         type: str
         version_added: '1.0.0'
     remove_keyfile:
@@ -103,7 +104,8 @@ 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."
-            - "NOTE that removing keys is *not idempotent*. Trying to remove
+            - "NOTE that removing keys is idempotent only since
+              community.crypto 1.4.0. For older versions, 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)."
@@ -114,9 +116,9 @@ options:
         description:
             - "Removes given passphrase from the container on I(device).
               Parameter value is a string with the passphrase to remove."
-            - "NOTE that removing passphrases is I(not
+            - "NOTE that removing passphrases is idempotent only since
-              idempotent). Trying to remove a passphrase which no longer
+              community.crypto 1.4.0. For older versions, trying to remove
-              exists results in an error."
+              a passphrase which no longer exists results in an error."
             - "NOTE that to remove the last keyslot from a LUKS
               container, the I(force_remove_last_key) option must be set
               to C(yes)."
@@ -520,6 +522,28 @@ class CryptHandler(Handler):
             raise ValueError('Error while removing LUKS key from %s: %s'
                              % (device, result[STDERR]))
 
+    def luks_test_key(self, device, keyfile, passphrase):
+        ''' Check whether the keyfile or passphrase works.
+            Raises ValueError when command fails.
+        '''
+        data = None
+        args = [self._cryptsetup_bin, 'luksOpen', '--test-passphrase', device]
+
+        if keyfile:
+            args.extend(['--key-file', keyfile])
+        else:
+            data = passphrase
+
+        result = self._run_command(args, data=data)
+        if result[RETURN_CODE] == 0:
+            return True
+        for output in (STDOUT, STDERR):
+            if 'No key available with this passphrase' in result[output]:
+                return False
+
+        raise ValueError('Error while testing whether keyslot exists on %s: %s'
+                         % (device, result[STDERR]))
+
 
 class ConditionsHandler(Handler):
 
@@ -627,7 +651,7 @@ class ConditionsHandler(Handler):
             self._module.fail_json(msg="Contradiction in setup: Asking to "
                                    "add a key to absent LUKS.")
 
-        return True
+        return not self._crypthandler.luks_test_key(self.device, self._module.params['new_keyfile'], self._module.params['new_passphrase'])
 
     def luks_remove_key(self):
         if (self.device is None or
@@ -640,7 +664,7 @@ class ConditionsHandler(Handler):
             self._module.fail_json(msg="Contradiction in setup: Asking to "
                                    "remove a key from absent LUKS.")
 
-        return True
+        return self._crypthandler.luks_test_key(self.device, self._module.params['remove_keyfile'], self._module.params['remove_passphrase'])
 
     def luks_remove(self):
         return (self.device is not None and

tests/integration/targets/luks_device/tasks/tests/key-management.yml

@@ -44,6 +44,21 @@
     keyfile: "{{ role_path }}/files/keyfile1"
     new_keyfile: "{{ role_path }}/files/keyfile2"
   become: yes
+  register: result_1
+
+- name: Give access to keyfile2 (idempotent)
+  luks_device:
+    device: "{{ cryptfile_device }}"
+    state: closed
+    keyfile: "{{ role_path }}/files/keyfile1"
+    new_keyfile: "{{ role_path }}/files/keyfile2"
+  become: yes
+  register: result_2
+
+- assert:
+    that:
+      - result_1 is changed
+      - result_2 is not changed
 
 # Access: keyfile1 and keyfile2
 
@@ -75,6 +90,21 @@
     keyfile: "{{ role_path }}/files/keyfile1"
     remove_keyfile: "{{ role_path }}/files/keyfile1"
   become: yes
+  register: result_1
+
+- name: Remove access from keyfile1 (idempotent)
+  luks_device:
+    device: "{{ cryptfile_device }}"
+    state: closed
+    keyfile: "{{ role_path }}/files/keyfile1"
+    remove_keyfile: "{{ role_path }}/files/keyfile1"
+  become: yes
+  register: result_2
+
+- assert:
+    that:
+      - result_1 is changed
+      - result_2 is not changed
 
 # Access: keyfile2
 

tests/integration/targets/luks_device/tasks/tests/passphrase.yml

@@ -56,6 +56,21 @@
     passphrase: "{{ cryptfile_passphrase1 }}"
     new_passphrase: "{{ cryptfile_passphrase2 }}"
   become: yes
+  register: result_1
+
+- name: Give access to passphrase2 (idempotent)
+  luks_device:
+    device: "{{ cryptfile_device }}"
+    state: closed
+    passphrase: "{{ cryptfile_passphrase1 }}"
+    new_passphrase: "{{ cryptfile_passphrase2 }}"
+  become: yes
+  register: result_2
+
+- assert:
+    that:
+      - result_1 is changed
+      - result_2 is not changed
 
 - name: Open with passphrase2
   luks_device:
@@ -130,6 +145,20 @@
     state: closed
     remove_passphrase: "{{ cryptfile_passphrase1 }}"
   become: yes
+  register: result_1
+
+- name: Remove access for passphrase1 (idempotent)
+  luks_device:
+    device: "{{ cryptfile_device }}"
+    state: closed
+    remove_passphrase: "{{ cryptfile_passphrase1 }}"
+  become: yes
+  register: result_2
+
+- assert:
+    that:
+      - result_1 is changed
+      - result_2 is not changed
 
 - name: Try to open with passphrase1
   luks_device:

tests/unit/plugins/modules/test_luks_device.py

@@ -275,9 +275,12 @@ def test_luks_add_key(device, keyfile, passphrase, new_keyfile, new_passphrase,
 
     monkeypatch.setattr(luks_device.Handler, "get_device_by_label",
                         lambda x, y: [0, "/dev/dummy", ""])
+    monkeypatch.setattr(luks_device.CryptHandler, "luks_test_key",
+                        lambda x, y, z, w: False)
 
+    crypt = luks_device.CryptHandler(module)
     try:
-        conditions = luks_device.ConditionsHandler(module, module)
+        conditions = luks_device.ConditionsHandler(module, crypt)
         assert conditions.luks_add_key() == expected
     except ValueError:
         assert expected == "exception"
@@ -301,6 +304,8 @@ def test_luks_remove_key(device, remove_keyfile, remove_passphrase, state,
                         lambda x, y: [0, "/dev/dummy", ""])
     monkeypatch.setattr(luks_device.Handler, "_run_command",
                         lambda x, y: [0, device, ""])
+    monkeypatch.setattr(luks_device.CryptHandler, "luks_test_key",
+                        lambda x, y, z, w: True)
 
     crypt = luks_device.CryptHandler(module)
     try: