Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged

Conversation

zemdreg
Copy link
Contributor

@zemdreg zemdreg commented Feb 8, 2024

SUMMARY
  • Fixed task failure when specifying remove_keyslot: 0.
  • Fixed module falsely outputting when trying to add a new slot with a key that is already present in another slot by rejecting duplicate keys
  • Fixed testing LUKS passphrases when using keyslots in cryptsetup version 2.0.3
ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

luks_device

ADDITIONAL INFORMATION

Tasks to reproduce the first issue:

- name: Add key in slot 0
  luks_device:
    device: "{{ device }}"
    state: present
    keyfile: "{{ keyfile1 }}"
    new_keyfile: "{{ keyfile2 }}"
    new_keyslot: 0
- name: Remove key in slot 0
  luks_device:
    device: "{{ device }}"
    keyfile: "{{ keyfile1 }}"
    remove_keyslot: 0

Tasks to reproduce the second issue, the second tasks outputs 'ok' but does not actually not add a new key to slot 1.

- name: Create new luks
  luks_device:
    device: "{{ device }}"
    state: present
    keyfile: "{{ keyfile 1}}"
- name: Add new keyslot with same keyfile
  luks_device:
    device: "{{ device }}"
    state: present
    new_keyslot: 1
    keyfile: "{{ keyfile1 }}"
    new_keyfile: "{{ keyfile1 }}"

The change in the luks_test_key method is necessary due to differing behavior cryptsetup 2.0.3 when using the parameters --test-passphrase and --key-slot at the same time.
Output with cryptsetup 2.0.3 (tested in a CentOS 7.9 container):

[root@21d89bdad3da /]# cryptsetup --version
cryptsetup 2.0.3
[root@21d89bdad3da /]# dd if=/dev/zero of=/tmp/test bs=1M count=1000
1000+0 records in
1000+0 records out
1048576000 bytes (1.0 GB) copied, 0.871568 s, 1.2 GB/s
[root@21d89bdad3da /]# dd if=/dev/urandom of=/tmp/keyfile bs=1M count=1
1+0 records in
1+0 records out
1048576 bytes (1.0 MB) copied, 0.00456403 s, 230 MB/s
[root@21d89bdad3da /]# cryptsetup luksFormat --key-file /tmp/keyfile /tmp/test

WARNING!
========
This will overwrite data on /tmp/test irrevocably.

Are you sure? (Type uppercase yes): YES
[root@21d89bdad3da /]# cryptsetup open --test-passphrase --key-file /tmp/keyfile /tmp/test
[root@21d89bdad3da /]# echo $?
0
[root@21d89bdad3da /]# cryptsetup open --test-passphrase --key-file /tmp/keyfile --key-slot 1 /tmp/test
[root@21d89bdad3da /]# echo $?
1

Output with cryptsetup 2.6.1 (tested on Manjaro):

[ansible-dev ~]# cryptsetup --version
cryptsetup 2.6.1 flags: UDEV BLKID KEYRING KERNEL_CAPI 
[ansible-dev ~]# dd if=/dev/zero of=/tmp/test bs=1M count=1000
1000+0 records in
1000+0 records out
1048576000 bytes (1,0 GB, 1000 MiB) copied, 0,202097 s, 5,2 GB/s
[ansible-dev ~]# dd if=/dev/urandom of=/tmp/keyfile bs=1M count=1
1+0 records in
1+0 records out
1048576 bytes (1,0 MB, 1,0 MiB) copied, 0,0112751 s, 93,0 MB/s
[ansible-dev ~]# cryptsetup luksFormat --key-file /tmp/keyfile /tmp/test

WARNING!
========
This will overwrite data on /tmp/test irrevocably.

Are you sure? (Type 'yes' in capital letters): YES
[ansible-dev ~]# cryptsetup open --test-passphrase --key-file /tmp/keyfile /tmp/test
[ansible-dev ~]# echo $?
0
[ansible-dev ~]# cryptsetup open --test-passphrase --key-file /tmp/keyfile --key-slot 1 /tmp/test
No usable keyslot is available.
[ansible-dev ~]# echo $?
1

@zemdreg zemdreg changed the title WIP: luks_device: fix remove_keyslot not working when set to 0 [WIP]: luks_device: fix remove_keyslot not working when set to 0 Feb 8, 2024
@zemdreg zemdreg marked this pull request as draft February 8, 2024 17:02
@zemdreg zemdreg changed the title [WIP]: luks_device: fix remove_keyslot not working when set to 0 [WIP] luks_device: fix remove_keyslot not working when set to 0 Feb 8, 2024
@zemdreg zemdreg changed the title [WIP] luks_device: fix remove_keyslot not working when set to 0 [WIP] luks_device: fix remove_keyslot not working when set to 0 and duplicate keys Feb 8, 2024
@zemdreg zemdreg marked this pull request as ready for review February 10, 2024 13:38
@zemdreg zemdreg changed the title [WIP] luks_device: fix remove_keyslot not working when set to 0 and duplicate keys luks_device: fix remove_keyslot not working when set to 0 and duplicate keys Feb 10, 2024
@felixfontein
Copy link
Contributor

Thanks for your contribution! The code looks good to me. Can you please add a changelog fragment? Thanks :)

@felixfontein felixfontein merged commit 5159189 into ansible-collections:main Feb 11, 2024
128 checks passed
@felixfontein
Copy link
Contributor

@sgufler thanks a lot for improving the module!

@zemdreg zemdreg deleted the luks_device_fix_remove_keyslot0 branch February 11, 2024 11:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants