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.

Sign up for GitHub

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

Jump to bottom

Running kexec-save-key wiped the only slot remaining #645

Closed
EDmitry opened this issue Dec 28, 2019 · 7 comments
Closed

Running kexec-save-key wiped the only slot remaining #645

EDmitry opened this issue Dec 28, 2019 · 7 comments
Assignees
@tlaurion

Comments

@EDmitry
Copy link

EDmitry commented Dec 28, 2019

After running kexec-save-key -p /boot/ /dev/sda2 the only slot generated by Qubes installation was wiped, and the new key wasn't inserted. Verified with luksDump that all slots are disabled after running the command. I see that from the code it supposed to wipe the slot 1, but I think the reason my main key was in slot 1 is because I change the LUKS password before. That probably inserted a new key in slot 1 and deleted from 0.

I think kexec-seal-key should verify that it's not removing the only slot available.
@flammit
Copy link
Collaborator

flammit commented Dec 31, 2019

Yeah that's a good catch. We probably should be using luksRemoveKey instead.

@tlaurion
Copy link
Collaborator

tlaurion commented Jan 1, 2020 •
edited

Yeah that's a good catch. We probably should be using luksRemoveKey instead.

@flammit: instead of killslot?

Edit: in my attempts to play fair with crypsetup, I found that specifying the slots was mitigating those errors. Of course killslot doesn't pardon and will just delete the key slot (1 here, as specified).

luksKillSlot <device> <key slot number>

              Wipe the key-slot number <key slot> from the LUKS device. Except running in batch-mode (-q) a remaining passphrase must be supplied,  either  interac‐
              tively  or  via --key-file.  This command can remove the last remaining key-slot, but requires an interactive confirmation when doing so. Removing the
              last passphrase makes a LUKS container permanently inaccessible.


luksRemoveKey <device> [<key file with passphrase to be removed>]

              Removes  the  supplied passphrase from the LUKS device. The passphrase to be removed can be specified interactively, as the positional argument or via
              --key-file.

              <options> can be [--key-file, --keyfile-offset, --keyfile-size, --header, --disable-locks, --type]

              WARNING: If you read the passphrase from stdin (without further argument or with '-' as an argument to --key-file), batch-mode (-q) will be implicitly
              switched  on  and no warning will be given when you remove the last remaining passphrase from a LUKS container. Removing the last passphrase makes the
              LUKS container permanently inaccessible.

But if I understand well the problem of @EDmitry, it was caused by playing externally with cryptsetup, so using luksRemoveKey would not have prevented the disaster and would have complied also, or I might miss something.

@tlaurion
Copy link
Collaborator

tlaurion commented Jan 1, 2020 •
edited

After running kexec-save-key -p /boot/ /dev/sda2 the only slot generated by Qubes installation was wiped, and the new key wasn't inserted. Verified with luksDump that all slots are disabled after running the command. I see that from the code it supposed to wipe the slot 1, but I think the reason my main key was in slot 1 is because I change the LUKS password before. That probably inserted a new key in slot 1 and deleted from 0.

I think kexec-seal-key should verify that it's not removing the only slot available.

@EDmitry : When you change a passphrase for a keyslot without specifying which one to play with, cryptsetup creates a new slot and deletes the old one. Adding a keyslot leaves the old one in place.

So you had nothing in slot 0 anymore because of changing a passphrase, and save-key doesn't validate if slot 0 exists, just kills slot 1, and saves new key in 1 as instructed. So in this case, it killed slot 1, had nothing to validate functional decryption against old key (no more slots) and failed to insert new key in slot 1.

Any proposition to deal with this external error?

As a side note, adding key (additional passphrase to unlock disk) for slot, or changing slot decryption key (passphrase) doesn't reencrypt disk content. It just adds/change key to be able to decrypt same encrypted disk content. To reencrypt, cryptsetup-reencrypt needs to be used.

@EDmitry
Copy link
Author

EDmitry commented Jan 1, 2020

@tlaurion yes, the password change was needed because of a suspicion that my password was exposed to couch surfing. The drive didn’t have to be re-encrypted.

I think just checking the amount of slots remaining should be enough. The script should abort if there is only one slot remaining at #1.

@tlaurion tlaurion self-assigned this Mar 9, 2020
@Jetba
Copy link

Jetba commented Jan 14, 2021 •
edited

Came across this as well while setting up my T430.
After installing Debian testing with defaults using LUKS+LVM and setting up TPM Disk encryption keys I noticed that there is no support for LUKS version 2. Downgrading LUKS to version 1 also moved my key to slot 1, which caused this issue during setup.

A rare occurance but got me confused for a second 😄 One fast Debian reinstall later and making sure I was using slot 0 solved everything.
External issue I guess as I was playing with cryptsetup to downgrade the LUKS version, but the proposition by EDmitry would have prevented this.

tlaurion added a commit to tlaurion/heads that referenced this issue Aug 29, 2023
@tlaurion
WiP : Optimize kexec-save-defaults to use proposed LUKS/encrypted LVMs
fbd98c7 
Non related outside of reviewing current code:
- Addition of TRACE and DEBUG statements to troubleshoot actual vs expected behavior while coding

close issue linuxboot#645, checking if slot1 is the only slot prior of wiping it

Currently rebased on top of https://github.com/tlaurion/stenghten_entropy_sources_with_jitter_and_TPM to test qemu at the same time.
TODO: rebase on top of osresearch/master prior of merging
tlaurion added a commit to tlaurion/heads that referenced this issue Aug 29, 2023
@tlaurion
WiP : Optimize kexec-save-defaults to use proposed LUKS/encrypted LVMs
b9f836c 
Non related outside of reviewing current code:
- Addition of TRACE and DEBUG statements to troubleshoot actual vs expected behavior while coding

close issue linuxboot#645, checking if slot1 is the only slot prior of wiping it

Currently rebased on top of https://github.com/tlaurion/stenghten_entropy_sources_with_jitter_and_TPM to test qemu at the same time.
TODO: rebase on top of osresearch/master prior of merging
tlaurion added a commit to tlaurion/heads that referenced this issue Aug 30, 2023
@tlaurion
Optimize kexec-save-defaults to use proposed LUKS/encrypted LVMs
607a2cd 
Non related outside of reviewing current code:
- Addition of TRACE and DEBUG statements to troubleshoot actual vs expected behavior while coding

close issue linuxboot#645, checking if slot1 is the only slot prior of wiping it

Tested:
- on top of https://github.com/tlaurion/stenghten_entropy_sources_with_jitter_and_TPM to test qemu at the same time.
- On top of master

Boards:
- qemu-coreboot-whiptail-tpm1: success
- qemu-coreboot-whiptail-tpm2 : fails
tlaurion added a commit to tlaurion/heads that referenced this issue Aug 30, 2023
@tlaurion
Optimize kexec-save-defaults to use proposed LUKS/encrypted LVMs
b5d52d6 
Non related outside of reviewing current code:
- Addition of TRACE and DEBUG statements to troubleshoot actual vs expected behavior while coding

close issue linuxboot#645, checking if slot1 is the only slot prior of wiping it

Tested:
- on top of https://github.com/tlaurion/stenghten_entropy_sources_with_jitter_and_TPM to test qemu at the same time.
- On top of master

Boards:
- qemu-coreboot-whiptail-tpm1: success
- qemu-coreboot-whiptail-tpm2 : fails
tlaurion added a commit to tlaurion/heads that referenced this issue Aug 30, 2023
@tlaurion
TPM Disk Unlock Key sealing/renewal cleanup (Triggered automatically …
4daff3e 
…when resealing TOTP)

Changes:
- As per master: when TOTP cannot unseal TOTP, user is prompted to either reset or regenerate TOTP
- Now, when either is done and a previous TPM Disk Unlock Key was setuped, the user is guided into:
  - Regenerating checksums and signing them
  - Regenerating TPM disk Unlock Key and resealing TPM disk Unlock Key with passphrase into TPM
  - LUKS header being modified, user is asked to resign kexec.sig one last time prior of being able to default boot
- When no previous Disk Unlock Key was setuped, the user is guided into:
  - The above, plus
    - Detection of LUKS containers,suggesting only relevant partitions

- Addition of TRACE and DEBUG statements to troubleshoot actual vs expected behavior while coding
  - Were missing under TPM Disk Unlock Key setup codepaths

- Fixes for linuxboot#645 : We now check if only one slots exists and we do not use it if its slot1.
  - Also shows in DEBUG traces now

Unrelated staged changes
- ash_functions: warn and die now contains proper spacing and eye attaction
- all warn and die calls modified if containing warnings and too much punctuation
- unify usage of term TPM Disk Unlock Key and Disk Recovery Key
tlaurion added a commit to tlaurion/heads that referenced this issue Aug 30, 2023
@tlaurion
TPM Disk Unlock Key sealing/renewal cleanup (Triggered automatically …
2eeeb0b 
…when resealing TOTP)

Changes:
- As per master: when TOTP cannot unseal TOTP, user is prompted to either reset or regenerate TOTP
- Now, when either is done and a previous TPM Disk Unlock Key was setuped, the user is guided into:
  - Regenerating checksums and signing them
  - Regenerating TPM disk Unlock Key and resealing TPM disk Unlock Key with passphrase into TPM
  - LUKS header being modified, user is asked to resign kexec.sig one last time prior of being able to default boot
- When no previous Disk Unlock Key was setuped, the user is guided into:
  - The above, plus
    - Detection of LUKS containers,suggesting only relevant partitions

- Addition of TRACE and DEBUG statements to troubleshoot actual vs expected behavior while coding
  - Were missing under TPM Disk Unlock Key setup codepaths

- Fixes for linuxboot#645 : We now check if only one slots exists and we do not use it if its slot1.
  - Also shows in DEBUG traces now

Unrelated staged changes
- ash_functions: warn and die now contains proper spacing and eye attaction
- all warn and die calls modified if containing warnings and too much punctuation
- unify usage of term TPM Disk Unlock Key and Disk Recovery Key
tlaurion added a commit to tlaurion/heads that referenced this issue Aug 30, 2023
@tlaurion
TPM Disk Unlock Key sealing/renewal cleanup (Triggered automatically …
4910c11 
…when resealing TOTP)

Changes:
- As per master: when TOTP cannot unseal TOTP, user is prompted to either reset or regenerate TOTP
- Now, when either is done and a previous TPM Disk Unlock Key was setuped, the user is guided into:
  - Regenerating checksums and signing them
  - Regenerating TPM disk Unlock Key and resealing TPM disk Unlock Key with passphrase into TPM
  - LUKS header being modified, user is asked to resign kexec.sig one last time prior of being able to default boot
- When no previous Disk Unlock Key was setuped, the user is guided into:
  - The above, plus
    - Detection of LUKS containers,suggesting only relevant partitions

- Addition of TRACE and DEBUG statements to troubleshoot actual vs expected behavior while coding
  - Were missing under TPM Disk Unlock Key setup codepaths

- Fixes for linuxboot#645 : We now check if only one slots exists and we do not use it if its slot1.
  - Also shows in DEBUG traces now

Unrelated staged changes
- ash_functions: warn and die now contains proper spacing and eye attaction
- all warn and die calls modified if containing warnings and too much punctuation
- unify usage of term TPM Disk Unlock Key and Disk Recovery Key
@tlaurion
Copy link
Collaborator

Not sure why I cannot tag this issue from #1482, but that PR fixes this. Closing prematurely otherwise won't be closed automatically (Bad Github)

@tlaurion tlaurion mentioned this issue Aug 30, 2023
Merged
1 task
@tlaurion
Copy link
Collaborator

tlaurion commented Sep 1, 2023

@EDmitry @Jetba fixed under #1482
2023-09-01-173124

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@tlaurion tlaurion

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@EDmitry @tlaurion @Jetba @flammit