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

clevis ignores PCR policy #102

Closed
dnoliver opened this issue Jun 4, 2019 · 5 comments
Closed

clevis ignores PCR policy #102

dnoliver opened this issue Jun 4, 2019 · 5 comments

Comments

@dnoliver
Copy link

dnoliver commented Jun 4, 2019

Clevis is ignoring the pcr policy configured with bind.
The following test demonstrate the behaviour.

Use clevis to create a key slot in an already created LUKS container.
Select the PCR sha1:16 to enforce the policy.
By default, sha1:16 contains zeroes.

sudo tpm2_pcrlist --sel-list sha1:16 > pcr-26-initial.yaml
sudo clevis luks bind -d /dev/sda1 tpm2 '{"pcr_bank":"sha1","pcr_ids":["16"]}'

Extend the PCR sha1:16 with any value.

sudo tpm2_pcrextend 16:sha1=a3d2744ea1acb343f49fe2a6441c0b057a0ac64c
sudo tpm2_pcrlist --sel-list sha1:16 > pcr-26-final.yaml

Check the PCR sha1:16 state

diff pcr-26-initial.yaml pcr-26-final.yaml

2c2
<   16 : 0000000000000000000000000000000000000000
---
>   16 : 232e11deb60a45b0c4055f5b2fbae1340e5e8932

Unlock the LUKS container using clevis

sudo clevis luks unlock -d /dev/sda1 -n c1

Expected: Clevis should not be able to unlock the volume, given that the policy should not be satisfied with the modified PCR value
Actual: Clevis unlocks the volume

The impact of this is that, if you used clevis with an PCR policy, the volume can be unlocked regardless of the platform integrity state.

NOTE: This originally happened with the PCR-9 as well. I am using the PCR-16 for testing purposes, given that it is the one that you can reset.

@martinezjavier is this expected behaviour?

@martinezjavier
Copy link
Contributor

@martinezjavier is this expected behaviour?

No, this is not the expected behavior. What clevis and tpm2-tools version are you using? On my Fedora 30 machine:

$ rpm -q clevis tpm2-tools
clevis-11-5.fc30.x86_64
tpm2-tools-3.1.4-1.fc30.x86_64

$ export TPM2TOOLS_TCTI_NAME=device
$ export TPM2TOOLS_DEVICE_FILE=/dev/tpmrm0

$ tpm2_pcrlist --sel-list sha1:16 > pcr-16-initial.yaml
$ cat pcr-16-initial.yaml 
sha1 :
  16 : 0000000000000000000000000000000000000000

$ echo foobar | clevis encrypt tpm2 '{"pcr_bank":"sha1","pcr_ids":"16"}' > secret.jwe
$ clevis decrypt < secret.jwe
foobar

$ tpm2_pcrextend 16:sha1=a3d2744ea1acb343f49fe2a6441c0b057a0ac64c
$ tpm2_pcrlist --sel-list sha1:16 > pcr-16-final.yaml

$ diff pcr-16-initial.yaml pcr-16-final.yaml 
2c2
<   16 : 0000000000000000000000000000000000000000
---
>   16 : 232e11deb60a45b0c4055f5b2fbae1340e5e8932

$ clevis decrypt < secret.jwe
Unsealing jwk from TPM failed!

@martinezjavier
Copy link
Contributor

sudo clevis luks bind -d /dev/sda1 tpm2 '{"pcr_bank":"sha1","pcr_ids":["16"]}'

The problem is in the jose project that's not able to parse {"pcr_bank":"sha1","pcr_ids":["16"]} that as fas as I can tell is a valid JSON. It does work though with {"pcr_bank":"sha1","pcr_ids":"16"}:

$ jose fmt -j- -Og pcr_ids -u- <<< '{"pcr_bank":"sha1","pcr_ids":"16"}'
16
$ echo $?
0

$ jose fmt -j- -Og pcr_ids -u- <<< '{"pcr_bank":"sha1","pcr_ids":["16"]}'
$ echo $?
4

So you should file an issue in the jose project for this.

clevis currently only takes into account the configuration fields if these are parsed correctly by jose, so in your case the pcr_ids field is ignored and that's why the object is unsealed regardless of your PCR state.

Probably that's not correct and should take into account when the field is key is present but its value couldn't be parsed.

@dnoliver
Copy link
Author

dnoliver commented Jun 5, 2019

What clevis and tpm2-tools version are you using?

Sorry I forgot to post my versions! find them below:

cat /etc/redhat-release
Fedora release 29 (Twenty Nine)

uname -a
Linux localhost.localdomain 5.0.16 #1 SMP Tue May 21 10:40:27 PDT 2019 x86_64 x86_64 x86_64 GNU/Linux

rpm -qa tpm2-tools tpm2-tss
tpm2-tools-3.1.4-1.fc29.x86_64
tpm2-tss-2.1.2-1.fc29.x86_64

rpm -qa clevis clevis-luks jose
clevis-luks-11-4.fc29.x86_64
jose-10-3.fc29.x86_64
clevis-11-4.fc29.x86_64

The problem is in the jose project that's not able to parse {"pcr_bank":"sha1","pcr_ids":["16"]}

Originally, I was using "pcr_ids": ["0", "1", "2", "3", "4", "5", "6", "7", "8", "9"] as a policy, so in the test, I kept using the array notation for the setting.

So you should file an issue in the jose project for this.

Done latchset/jose#68

Probably that's not correct and should take into account when the field is key is present but its value couldn't be parsed.

I originally thought that I was passing an invalid configuration, given that I was sing "pcr_banks": "sha1" instead of the "pcr_bank" key. As you said, it makes sense that clevis helps you in that area, and fails with invalid/unparseable configurations, instead of ignoring them and giving you a false sense of security :D. Do you think I should file another issue for this?

@martinezjavier
Copy link
Contributor

Originally, I was using "pcr_ids": ["0", "1", "2", "3", "4", "5", "6", "7", "8", "9"] as a policy,

Right, the clevis tpm2 pin supports a list of PCR for the policy as a coma separated list, i.e: "pcr_ids":"0,1,2,3,4,5,6,7,8,9" as explained in the clevis-encrypt-tpm2` man page.

But yes, probably jose should parse the array notation correctly.

Do you think I should file another issue for this?

Yes, please file another issue for that.

I think this issue can be closed now, unless you want to keep it open until jose is fixed for reference if other users face the same.

@dnoliver
Copy link
Author

dnoliver commented Jun 6, 2019

Done, created new issue for Jose settings parsing state validation #103.

I am closing this, it is now clear which is the setting I should have used to not run into this error.

Thank you @martinezjavier!

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

No branches or pull requests

2 participants