diff --git a/changelogs/fragments/168-luks_device-add-remove-idempotence.yml b/changelogs/fragments/168-luks_device-add-remove-idempotence.yml new file mode 100644 index 00000000..47b31537 --- /dev/null +++ b/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)." diff --git a/plugins/modules/luks_device.py b/plugins/modules/luks_device.py index 9dc3292b..a69c5597 100644 --- a/plugins/modules/luks_device.py +++ b/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*. - A new keyslot will be used even if another keyslot already - exists for this keyfile." + - "NOTE that adding additional keys is idempotent only since + community.crypto 1.4.0. For older versions, 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 @@ -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 - new keyslot will be used even if another keyslot already exists - for this passphrase." + - "NOTE that adding additional passphrase is idempotent only since + community.crypto 1.4.0. For older versions, a new keyslot will + 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 - idempotent). Trying to remove a passphrase which no longer - exists results in an error." + - "NOTE that removing passphrases is idempotent only since + community.crypto 1.4.0. For older versions, trying to remove + 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 diff --git a/tests/integration/targets/luks_device/tasks/tests/key-management.yml b/tests/integration/targets/luks_device/tasks/tests/key-management.yml index b89d505f..3e35d753 100644 --- a/tests/integration/targets/luks_device/tasks/tests/key-management.yml +++ b/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 diff --git a/tests/integration/targets/luks_device/tasks/tests/passphrase.yml b/tests/integration/targets/luks_device/tasks/tests/passphrase.yml index 56091525..16c7ef8d 100644 --- a/tests/integration/targets/luks_device/tasks/tests/passphrase.yml +++ b/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: diff --git a/tests/unit/plugins/modules/crypto/__init__.py b/tests/unit/plugins/modules/crypto/__init__.py deleted file mode 100644 index e69de29b..00000000 diff --git a/tests/unit/plugins/modules/crypto/test_luks_device.py b/tests/unit/plugins/modules/test_luks_device.py similarity index 97% rename from tests/unit/plugins/modules/crypto/test_luks_device.py rename to tests/unit/plugins/modules/test_luks_device.py index 7a12aaf5..d798b447 100644 --- a/tests/unit/plugins/modules/crypto/test_luks_device.py +++ b/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: