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

Make it possible to report headers of which LUKSes to be unlocked via TPM change #1093

Closed

Conversation

root-hardenedvault
Copy link
Contributor

Copy link
Collaborator

@tlaurion tlaurion left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@root-hardenedvault quick review!
Analysis given here

@@ -51,6 +51,8 @@ tpm extend -ix 4 -ic generic \

# Check to continue
if [ "$unseal_failed" = "y" ]; then
diff "$(dirname $INITRD)/kexec_lukshdr_hash.txt" /tmp/luksDump.txt \
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@root-hardenedvault : wouldn't be preferable to inform the user if the hashes differ? (! diff)

@@ -51,6 +51,8 @@ tpm extend -ix 4 -ic generic \

# Check to continue
if [ "$unseal_failed" = "y" ]; then
diff "$(dirname $INITRD)/kexec_lukshdr_hash.txt" /tmp/luksDump.txt \
&& echo "Headers of LUKSes to be unlocked via TPM do not change."
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Otherwise, tense of verb seems wrong, since they were previously measured.
"Headers of LUKSes to be unlocked via TPM do not change." -> "Headers of LUKSes to be unlocked via TPM did not change."

But as said in previous comment at line 54, it would be preferable for end user to know that it changed, not that it didn't change.

@tlaurion
Copy link
Collaborator

@root-hardenedvault ping?

@tlaurion
Copy link
Collaborator

tlaurion commented Feb 2, 2022

@tlaurion
Copy link
Collaborator

tlaurion commented Mar 7, 2022

@root-hardenedvault Would it be possibleto address review here?

@tlaurion
Copy link
Collaborator

tlaurion commented Mar 11, 2022

Testing header change (by cryptsetup-reencrypt).

Prior of reencryption of device, trying to unseal TPM encryption key with bad TPM disk unlock key passphrase, the code doesn't go through changed code to show "Headers of LUKSes to be unlocked via TPM do not change." there.

Instead, and as expected, since the header is part of what is measured and sealed, we only get a "bad TPM unseal password", which happens from tpm binary, and given by errors at https://github.com/osresearch/heads/blob/14c76d062c199f17ff6369091a80d2885cb54914/initrd/bin/kexec-unseal-key#L34-L38
It outputs: "Error Authentication failed (Incorrect Password) from TPM_Unseal" since not successful.

On reboot, after having reencrypted encrypted drive, attempting to boot the default option prompts to type the TPM disk unlock key passphrase and gives: "Unable to unseal disk encryption key":
https://github.com/osresearch/heads/blob/14c76d062c199f17ff6369091a80d2885cb54914/initrd/bin/kexec-unseal-key#L46

also not running through modified code from this PR. Intuition is that the inverse diff should happen just there.

It seems that the diff in question and associated message never shows. Am I missing something @root-hardenedvault ?

@tlaurion
Copy link
Collaborator

@root-hardenedvault

The following works in both case: when the user enters a bad TPM disk encryption key passphrase, and when he enters a good one but the LUKS headers changed:

diff --git a/initrd/bin/kexec-unseal-key b/initrd/bin/kexec-unseal-key
index e016f5bd..b410f706 100755
--- a/initrd/bin/kexec-unseal-key
+++ b/initrd/bin/kexec-unseal-key
@@ -44,6 +44,9 @@ for tries in 1 2 3; do
 
        pcrs
        warn "Unable to unseal disk encryption key"
+       if ! diff /boot/kexec_lukshdr_hash.txt /tmp/luksDump.txt > /dev/null 2>&1; then
+               warn "Encrypted LUKS(es) headers changed since they were measured and sealed in TPM when you configured a disk unlock key. You might want to investigate."
+       fi
 done
 
 die "Retry count exceeded..."

I was never able to trigger the message in your PR. Please update.

@tlaurion
Copy link
Collaborator

Ok got it.
So without code addition above, with ed1c23a the user is actually hitting your code if he types his TPM disk unlock unlock code 3 times, and receives a raw diff output, without a warning in the case the LUKS Header changed.

Following ed1c23a, current PR shows to the user, only after 3 bad TPM disk unlock passphrase:

  • uncontextualized diff output if mismatch
  • No message if mismatch (message requires a positive diff exit (no change) to show the message (&&) that there was no change (there was change).

See picture:
signal-2022-03-12-165352

tlaurion added a commit to tlaurion/heads that referenced this pull request May 3, 2022
…aling of TPM Disk Unlock Key.

Fixes linuxboot#1092.
Supersedes linuxboot#1093
- Cherry-picks ed1c23a (credit to @hardened-vault) thank you!)
- Addresses and correct self-review under linuxboot#1093 (@hardened-vault: you don't answer often here!)
  - kexec-unseal-key: Warn a user who attempts to default boot while his Disk Unlock Key passphrase fails to unseal because LUKS headers changed.
    (linuxboot#1093 (comment))
  - kexec-seal-key: Identical as in ed1c23a
  - kexec-add-key: Tell the user that the Headers did not change when changing TPM released Disk Unlock Key
    (Through changing default boot at Options->Boot Options -> Show OS boot options: select a new boot option
    and set a Disk Unlock Key in TPM, accept to modify disk and sign /boot options)
    - Here, we cancel the diff output shown on screen linuxboot#1093 (comment)
    - And we change the warning given to the user to past tense "Headers of LUKS containers to be unlocked via TPM Disk Unlock Key passphrase did not change."
tlaurion added a commit to tlaurion/heads that referenced this pull request Mar 26, 2024
…sk Unlock Key.

Fixes linuxboot#1092.
Supersedes linuxboot#1093

- Cherry-picks ed1c23a (credit to @hardened-vault) thank you!)
- Addresses and correct self-review under linuxboot#1093 (@hardened-vault: you don't answer often here!)
  - kexec-unseal-key: Warn a user who attempts to default boot while his Disk Unlock Key passphrase fails to unseal because LUKS headers changed.
    (linuxboot#1093 (comment))
  - kexec-seal-key: Identical as in ed1c23a
  - kexec-add-key: Tell the user that the Headers did not change when changing TPM released Disk Unlock Key
    (Through changing default boot at Options->Boot Options -> Show OS boot options: select a new boot option
    and set a Disk Unlock Key in TPM, accept to modify disk and sign /boot options)
    - Here, we cancel the diff output shown on screen linuxboot#1093 (comment)
    - And we change the warning given to the user to past tense "Headers of LUKS containers to be unlocked via TPM Disk Unlock Key passphrase did not change."

Signed-off-by: Thierry Laurion <[email protected]>
@tlaurion
Copy link
Collaborator

Superseeded by #1625

@tlaurion tlaurion closed this Mar 27, 2024
tlaurion added a commit to tlaurion/heads that referenced this pull request Apr 2, 2024
…sk Unlock Key.

Fixes linuxboot#1092.
Supersedes linuxboot#1093

- Cherry-picks ed1c23a (credit to @hardened-vault) thank you!)
- Addresses and correct self-review under linuxboot#1093 (@hardened-vault: you don't answer often here!)
  - kexec-unseal-key: Warn a user who attempts to default boot while his Disk Unlock Key passphrase fails to unseal because LUKS headers changed.
    (linuxboot#1093 (comment))
  - kexec-seal-key: Identical as in ed1c23a
  - kexec-add-key: Tell the user that the Headers did not change when changing TPM released Disk Unlock Key
    (Through changing default boot at Options->Boot Options -> Show OS boot options: select a new boot option
    and set a Disk Unlock Key in TPM, accept to modify disk and sign /boot options)
    - Here, we cancel the diff output shown on screen linuxboot#1093 (comment)
    - And we change the warning given to the user to past tense "Headers of LUKS containers to be unlocked via TPM Disk Unlock Key passphrase did not change."

Signed-off-by: Thierry Laurion <[email protected]>
tlaurion added a commit to tlaurion/heads that referenced this pull request Apr 11, 2024
…sk Unlock Key.

Fixes linuxboot#1092.
Supersedes linuxboot#1093

- Cherry-picks ed1c23a (credit to @hardened-vault) thank you!)
- Addresses and correct self-review under linuxboot#1093 (@hardened-vault: you don't answer often here!)
  - kexec-unseal-key: Warn a user who attempts to default boot while his Disk Unlock Key passphrase fails to unseal because LUKS headers changed.
    (linuxboot#1093 (comment))
  - kexec-seal-key: Identical as in ed1c23a
  - kexec-add-key: Tell the user that the Headers did not change when changing TPM released Disk Unlock Key
    (Through changing default boot at Options->Boot Options -> Show OS boot options: select a new boot option
    and set a Disk Unlock Key in TPM, accept to modify disk and sign /boot options)
    - Here, we cancel the diff output shown on screen linuxboot#1093 (comment)
    - And we change the warning given to the user to past tense "Headers of LUKS containers to be unlocked via TPM Disk Unlock Key passphrase did not change."

Signed-off-by: Thierry Laurion <[email protected]>
tlaurion added a commit to tlaurion/heads that referenced this pull request Apr 26, 2024
…sk Unlock Key.

Fixes linuxboot#1092.
Supersedes linuxboot#1093

- Cherry-picks ed1c23a (credit to @hardened-vault) thank you!)
- Addresses and correct self-review under linuxboot#1093 (@hardened-vault: you don't answer often here!)
  - kexec-unseal-key: Warn a user who attempts to default boot while his Disk Unlock Key passphrase fails to unseal because LUKS headers changed.
    (linuxboot#1093 (comment))
  - kexec-seal-key: Identical as in ed1c23a
  - kexec-add-key: Tell the user that the Headers did not change when changing TPM released Disk Unlock Key
    (Through changing default boot at Options->Boot Options -> Show OS boot options: select a new boot option
    and set a Disk Unlock Key in TPM, accept to modify disk and sign /boot options)
    - Here, we cancel the diff output shown on screen linuxboot#1093 (comment)
    - And we change the warning given to the user to past tense "Headers of LUKS containers to be unlocked via TPM Disk Unlock Key passphrase did not change."

Signed-off-by: Thierry Laurion <[email protected]>
tlaurion added a commit to tlaurion/heads that referenced this pull request May 2, 2024
…sk Unlock Key.

Fixes linuxboot#1092.
Supersedes linuxboot#1093

- Cherry-picks ed1c23a (credit to @hardened-vault) thank you!)
- Addresses and correct self-review under linuxboot#1093 (@hardened-vault: you don't answer often here!)
  - kexec-unseal-key: Warn a user who attempts to default boot while his Disk Unlock Key passphrase fails to unseal because LUKS headers changed.
    (linuxboot#1093 (comment))
  - kexec-seal-key: Identical as in ed1c23a
  - kexec-add-key: Tell the user that the Headers did not change when changing TPM released Disk Unlock Key
    (Through changing default boot at Options->Boot Options -> Show OS boot options: select a new boot option
    and set a Disk Unlock Key in TPM, accept to modify disk and sign /boot options)
    - Here, we cancel the diff output shown on screen linuxboot#1093 (comment)
    - And we change the warning given to the user to past tense "Headers of LUKS containers to be unlocked via TPM Disk Unlock Key passphrase did not change."

Signed-off-by: Thierry Laurion <[email protected]>
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