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

Try to use the public keys pcrs if tpm2 pcrs are blank #32960

Closed
wants to merge 3 commits into from

Conversation

Itxaka
Copy link

@Itxaka Itxaka commented May 21, 2024

Fixes #32946

If you want to bind a policy to PCR11 and especify the tpm2-pcr flag with and empty value, the bank calculation will fail as it tries to use a non valid value for the calculation of hash.

This works around it by trying to use the public keys pcr values if they are set and if the usual tpm2 pcrs banks are empty as to not fail

From issue systemd#32946

If you want to bind a policy to PCR11 and especify the tpm2-pcr
flag with and empty value, the bank calculation will fail as it tries to
use a non valid value for the calculation of hash.

This works around it by trying to use the public keys pcr values if
they are set and if the usual tpm2 pcrs banks are empty as to not fail

Signed-off-by: Itxaka <itxaka@kairos.io>
@github-actions github-actions bot added the please-review PR is ready for (re-)review by a maintainer label May 21, 2024
Copy link

Important

An -rc1 tag has been created and a release is being prepared, so please note that PRs introducing new features and APIs will be held back until the new version has been released.

// Otherwise if user sets the tpm2-pcrs option to blank to not bind them, it will fail to calculate the bank
if (arg_tpm2_hash_pcr_values == NULL && arg_tpm2_n_hash_pcr_values == 0 && arg_tpm2_key_pcr_values != NULL && arg_tpm2_n_key_pcr_values > 0) {
arg_tpm2_hash_pcr_values = arg_tpm2_key_pcr_values;
arg_tpm2_n_hash_pcr_values = arg_tpm2_n_key_pcr_values;
Copy link
Contributor

Choose a reason for hiding this comment

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

This will force PCR policy for the public key PCRs in addition to the signed policy without any way to disable it.

Copy link
Author

Choose a reason for hiding this comment

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

will it?

I tested it and it worked as expected, because int he case the arg_tpm2_hash_pcr_values is NULL, using the arg_tpm2_key_pcr_values will turn out to just use 1 set and becuase both are set to the same value it ends up working as expected

with --tpm2-public-key-pcrs=11 --tpm2-pcrs=

TPM2 PCR bank sha1 has fewer than 24 PCR bits enabled, ignoring.
Reading PCR selection: [sha256(11)]
Read PCR selection: [sha256(11)]
PCR value: 11:sha256=cd4a21cb128034894805634efa3de7bf7ad64bab838831a22d525a33b6afe133
TPM2 device supports SHA256 PCR bank and SHA256 PCRs are valid, yay!
Reading PCR selection: [sha256(11)]
Read PCR selection: [sha256(11)]
PCR value: 11:sha256=cd4a21cb128034894805634efa3de7bf7ad64bab838831a22d525a33b6afe133
Calculated public key name: 000bad2a981c0eed83d41d9544750be2bc8f80b39f8af1ab3463bd7dbefb6576da3e
PolicyAuthorize calculated digest: 513c0686708c06d99dd558564d916e0cfd3ed03040e91cc6de5a8ac89d5b02b6
PolicyPCR calculated digest: 7b1583483ab680be0156633c05dc59b9758e53d56083144c64b28aab6d072fa9
Not adding TPM2 entropy to the kernel random pool again.

with --tpm2-public-key-pcrs=11 --tpm2-pcrs=3

Building sealing policy.
Reading PCR selection: [sha256(3+11)]
Read PCR selection: [sha256(3+11)]
PCR value: 3:sha256=3d458cfe55cc03ea1f443f1562beec8df51c75e14a9fcf9a7234a13f198e7969
PCR value: 11:sha256=cd4a21cb128034894805634efa3de7bf7ad64bab838831a22d525a33b6afe133
Adding PCR signature policy.
Loading external key into TPM.
Object name: 000bad2a981c0eed83d41d9544750be2bc8f80b39f8af1ab3463bd7dbefb6576da3e
Submitting PCR hash policy.
Acquiring policy digest.
Session policy digest: bd34d71c240e83bcfc49b2f6e5becf220c5f1f7380d57838e7b65aad1d963b23
Acquiring policy digest.
Session policy digest: 513c0686708c06d99dd558564d916e0cfd3ed03040e91cc6de5a8ac89d5b02b6
Submitting PCR hash policy.
Acquiring policy digest.
Session policy digest: 7f9fa4e54d3d064b06b5f614d7e134ae6167e29036ec5fcaf3d1318ec3636a95
Acquiring policy digest.
Session policy digest: 7f9fa4e54d3d064b06b5f614d7e134ae6167e29036ec5fcaf3d1318ec3636a95
Unsealing HMAC key.
Completed TPM2 key unsealing in 254.055ms.
Adding token text <{"type":"systemd-tpm2","keyslots":["2"],"tpm2-blob":"AJ4AIMQU9xxD07rg0JMC5H7PbaYZXriHNIxLsBfntHwyUaqrABCEBhHvzrYEEE22RlYK2pPZwm11YA/VxgmfB3gNaTlBDUIGx/LXdGIZwrwnUzXtxsBpRGRWRWfyCWkv7aZ4tI4v2XYUL6Vmb2f2sdZfjEHE4g3viMk3rLwkC+z5Zrw5IgLYB9fzLYlwtGOH0MP1/5fh241ttdEmhSmL0gBOAAgACwAABBIAIH+fpOVNPQZLBrX2FNfhNK5hZ+KQNuxfyvPRMY7DY2qVABAAIPEyW47z/mdFrFxSml4G1H5gdxEjoTbLPGadgNwGeEvU","tpm2-pcrs":[3],"tpm2-pcr-bank":"sha256","tpm2-policy-hash":"7f9fa4e54d3d064b06b5f614d7e134ae6167e29036ec5fcaf3d1318ec3636a95","tpm2-pin":false,"tpm2_pcrlock":false,"tpm2_pubkey_pcrs":[11],"tpm2_pubkey":"LS0tLS1CRUdJTiBQVUJMSUMgS0VZLS0tLS0KTUlJQklqQU5CZ2txaGtpRzl3MEJBUUVGQUFPQ0FROEFNSUlCQ2dLQ0FRRUExTldTU3RWNDRIQVowdkdGaFBuUAp3eHJZbUhHRU1NeHVzUXRPVnBXNVFiLzN2dHRDMFpieG05MGdMQ2w0UjYzNUl2WGVwb2I1NmZhR2YyL2l5TTJyCnA2MlRHSnpLdFNLODFGNjFLQnlZRjdaS2pWeWFNTmMzSjdoMVBmQjdlT0oyZ1EzUGdybWpLM0lKdUkxT2hGMzcKUi9maWt3cFYvZ2VTU2diUGxqZ3IyUmw1b1d4VFluNXFDaC9CQ3dmbDVZTnp0N0lPOEVFdEUwTThycklvTHpTMQp4UnhmaWdoOTNQaEltdG5tbDN2ZDlnd1FIcnJPN2NzNGVsQVkrOFhSdW5kRVZCYmd1TTd6QzBIUlRocDRZYWh0Cmw4bVREQ1YwaGFZOVVvSWp0ZGJkcG10K3NEMmZyYllEVlk5Zis3VTJSZDdZcDFkaDhNc1BHMkJ5M1RVQ2l3a1EKYVFJREFRQUIKLS0tLS1FTkQgUFVCTElDIEtFWS0tLS0tCg==","tpm2_srk":"gQAAAQAiAAtFmtcOfTaTsqkzrcwxonwYAc9HacRwvfKkGhbjR9uaLAAAAAEAWgAjAAsAAwRyAAAABgCAAEMAEAADABAAICn301ozNM+qA0RKpa/ptgcIYxLjBPYKJvcfxSpaCILlACBkbJ6JNOT36Spcm1s3hFSP+RVLj0JG+/ZQPRDTF+0jmA=="}>

with --tpm2-public-key-pcrs=11 --tpm2-pcrs=11

Reading PCR selection: [sha256(11)]
Read PCR selection: [sha256(11)]
PCR value: 11:sha256=cd4a21cb128034894805634efa3de7bf7ad64bab838831a22d525a33b6afe133
Adding PCR signature policy.
Loading external key into TPM.
Object name: 000bad2a981c0eed83d41d9544750be2bc8f80b39f8af1ab3463bd7dbefb6576da3e
Submitting PCR hash policy.
Acquiring policy digest.
Session policy digest: bd34d71c240e83bcfc49b2f6e5becf220c5f1f7380d57838e7b65aad1d963b23
Acquiring policy digest.
Session policy digest: 513c0686708c06d99dd558564d916e0cfd3ed03040e91cc6de5a8ac89d5b02b6
Submitting PCR hash policy.
Acquiring policy digest.
Session policy digest: 7b1583483ab680be0156633c05dc59b9758e53d56083144c64b28aab6d072fa9
Acquiring policy digest.
Session policy digest: 7b1583483ab680be0156633c05dc59b9758e53d56083144c64b28aab6d072fa9
Unsealing HMAC key.
Completed TPM2 key unsealing in 285.262ms.
Adding token text <{"type":"systemd-tpm2","keyslots":["1"],"tpm2-blob":"AJ4AILb2BY+vkX4Be/rKjbtsLHjhzKLLISmdo/maoXxsu09EABBRj9QPl45QvQdc4yW7z17kPyA3ZXaamwJtUKV39sYpepDTxF0z6GoldGtyNV4ons3n3Mz6+wPRu1Su+XxD8FCBXdcKDbn/28g6X6Gk/E2NK/Kec615Xj6TYSQu4VsmXSZeKcvrzuyaRJv4BfX3/LZxQ6Mk0Ekt0+CbmwBOAAgACwAABBIAIHsVg0g6toC+AVZjPAXcWbl1jlPVYIMUTGSyiqttBy+pABAAIHjq3PTSEV7WtlW2AhAYTimQrEEoaj/5nkinsnUJfq5K","tpm2-pcrs":[11],"tpm2-pcr-bank":"sha256","tpm2-policy-hash":"7b1583483ab680be0156633c05dc59b9758e53d56083144c64b28aab6d072fa9","tpm2-pin":false,"tpm2_pcrlock":false,"tpm2_pubkey_pcrs":[11],"tpm2_pubkey":"LS0tLS1CRUdJTiBQVUJMSUMgS0VZLS0tLS0KTUlJQklqQU5CZ2txaGtpRzl3MEJBUUVGQUFPQ0FROEFNSUlCQ2dLQ0FRRUExTldTU3RWNDRIQVowdkdGaFBuUAp3eHJZbUhHRU1NeHVzUXRPVnBXNVFiLzN2dHRDMFpieG05MGdMQ2w0UjYzNUl2WGVwb2I1NmZhR2YyL2l5TTJyCnA2MlRHSnpLdFNLODFGNjFLQnlZRjdaS2pWeWFNTmMzSjdoMVBmQjdlT0oyZ1EzUGdybWpLM0lKdUkxT2hGMzcKUi9maWt3cFYvZ2VTU2diUGxqZ3IyUmw1b1d4VFluNXFDaC9CQ3dmbDVZTnp0N0lPOEVFdEUwTThycklvTHpTMQp4UnhmaWdoOTNQaEltdG5tbDN2ZDlnd1FIcnJPN2NzNGVsQVkrOFhSdW5kRVZCYmd1TTd6QzBIUlRocDRZYWh0Cmw4bVREQ1YwaGFZOVVvSWp0ZGJkcG10K3NEMmZyYllEVlk5Zis3VTJSZDdZcDFkaDhNc1BHMkJ5M1RVQ2l3a1EKYVFJREFRQUIKLS0tLS1FTkQgUFVCTElDIEtFWS0tLS0tCg==","tpm2_srk":"gQAAAQAiAAtFmtcOfTaTsqkzrcwxonwYAc9HacRwvfKkGhbjR9uaLAAAAAEAWgAjAAsAAwRyAAAABgCAAEMAEAADABAAICn301ozNM+qA0RKpa/ptgcIYxLjBPYKJvcfxSpaCILlACBkbJ6JNOT36Spcm1s3hFSP+RVLj0JG+/ZQPRDTF+0jmA=="}>

with only --tpm2-public-key-pcrs=11

Building sealing policy.
Reading PCR selection: [sha256(7+11)]
Read PCR selection: [sha256(7+11)]
PCR value: 7:sha256=4f30c898e3fdbd7b4e165c2d6724bd16469371a5fb2496f8b94b822edf458767
PCR value: 11:sha256=cd4a21cb128034894805634efa3de7bf7ad64bab838831a22d525a33b6afe133
Adding PCR signature policy.
Loading external key into TPM.
Object name: 000bad2a981c0eed83d41d9544750be2bc8f80b39f8af1ab3463bd7dbefb6576da3e
Submitting PCR hash policy.
Acquiring policy digest.
Session policy digest: bd34d71c240e83bcfc49b2f6e5becf220c5f1f7380d57838e7b65aad1d963b23
Acquiring policy digest.
Session policy digest: 513c0686708c06d99dd558564d916e0cfd3ed03040e91cc6de5a8ac89d5b02b6
Submitting PCR hash policy.
Acquiring policy digest.
Session policy digest: 23ae8c0b5f9a41773c6bf6b46b5634e45b2b55be35d1446dca8700426e72664e
Acquiring policy digest.
Session policy digest: 23ae8c0b5f9a41773c6bf6b46b5634e45b2b55be35d1446dca8700426e72664e
Unsealing HMAC key.
Completed TPM2 key unsealing in 281.578ms.
Adding token text <{"type":"systemd-tpm2","keyslots":["2"],"tpm2-blob":"AJ4AIGF3glHWpuX2ZYk0JxQypQsbUQMAVyoiJa3BAt1pID50ABCUzlhdyB8Ws0AigJYBm9TRtEZNJc+bEb6OgcLh5PiYqoTMzb6+d0UexHkxDbdJ9/XtEg4/HkJY7tt3+PR9oonK6bAoqfp7qTVHUfZs+8vhxT7VtUY69UFdiakaQEdzDILKqm612wYRANkNPrw98O1BQ7glo/GS5fdBmQBOAAgACwAABBIAICOujAtfmkF3PGv2tGtWNORbK1W+NdFEbcqHAEJucmZOABAAIK2YjRBY/HBdnkaZy1LBraEYBzXrmbNSc/J9OBZiJ5IA","tpm2-pcrs":[7],"tpm2-pcr-bank":"sha256","tpm2-policy-hash":"23ae8c0b5f9a41773c6bf6b46b5634e45b2b55be35d1446dca8700426e72664e","tpm2-pin":false,"tpm2_pcrlock":false,"tpm2_pubkey_pcrs":[11],"tpm2_pubkey":"LS0tLS1CRUdJTiBQVUJMSUMgS0VZLS0tLS0KTUlJQklqQU5CZ2txaGtpRzl3MEJBUUVGQUFPQ0FROEFNSUlCQ2dLQ0FRRUExTldTU3RWNDRIQVowdkdGaFBuUAp3eHJZbUhHRU1NeHVzUXRPVnBXNVFiLzN2dHRDMFpieG05MGdMQ2w0UjYzNUl2WGVwb2I1NmZhR2YyL2l5TTJyCnA2MlRHSnpLdFNLODFGNjFLQnlZRjdaS2pWeWFNTmMzSjdoMVBmQjdlT0oyZ1EzUGdybWpLM0lKdUkxT2hGMzcKUi9maWt3cFYvZ2VTU2diUGxqZ3IyUmw1b1d4VFluNXFDaC9CQ3dmbDVZTnp0N0lPOEVFdEUwTThycklvTHpTMQp4UnhmaWdoOTNQaEltdG5tbDN2ZDlnd1FIcnJPN2NzNGVsQVkrOFhSdW5kRVZCYmd1TTd6QzBIUlRocDRZYWh0Cmw4bVREQ1YwaGFZOVVvSWp0ZGJkcG10K3NEMmZyYllEVlk5Zis3VTJSZDdZcDFkaDhNc1BHMkJ5M1RVQ2l3a1EKYVFJREFRQUIKLS0tLS1FTkQgUFVCTElDIEtFWS0tLS0tCg==","tpm2_srk":"gQAAAQAiAAtFmtcOfTaTsqkzrcwxonwYAc9HacRwvfKkGhbjR9uaLAAAAAEAWgAjAAsAAwRyAAAABgCAAEMAEAADABAAICn301ozNM+qA0RKpa/ptgcIYxLjBPYKJvcfxSpaCILlACBkbJ6JNOT36Spcm1s3hFSP+RVLj0JG+/ZQPRDTF+0jmA=="}>

so it seems to work as expected. The only time this hits its when you set --tpm2-pcrs to an empty value while also setting --tpm2-public-key-pcrs

I do agree this is probably not the best way of fixing it, I tried my best :D

Copy link
Contributor

Choose a reason for hiding this comment

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

will it?

Yes.

PolicyPCR calculated digest: 7b1583483ab680be0156633c05dc59b9758e53d56083144c64b28aab6d072fa9

Copy link
Author

Choose a reason for hiding this comment

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

I dont understand it as its a bit complex so no idea, but I trust you.

@arvidjaar do you think its useful to go this route or should I drop this instead?

@arvidjaar
Copy link
Contributor

After closer look, current situation is a real mess.

  1. Documentation claims that --tpm2-public-key-pcrs= follows the same format as --tpm2-pcrs=, but in reality, both bank indication and the actual values are completely ignored. systemd-cryptenroll computes PCR mask straight from the PCR indices, but PCR mask is meaningless without knowing which bank will be used. Maybe bank should be part of the PCR mask object.
  2. While the actual values are useless anyway (they are never used for the signed policy), the bank indication is used and must match the content of --tpm2-signature= parameter. But as bank from --tpm2-public-key-pcrs= is ignored, systemd-cryptenroll is implicitly using the bank from --tpm2-pcrs=.
  3. Logically, if no --tpm2-pcrs= are present (explicitly or implicitly), enrollment fails.

So, the following

systemd-measure --bank=sha384 ...
systemd-cryptenroll --tpm2-public-key-pcrs=11:sha384 ...

will never work, because systemd-cryptenroll will encode the default SHA256 bank which will be missing in the file generated by systemd-measure.

What is needed to fix it

  1. The bank for --tpm2-public-key-pcrs= must be computed separately, similar to how it is done for --tpm2-pcrs=.
  2. This bank must be explicitly passed down to enroll_tpm2 and further down where appropriate,

Once this is in place your problem is trivially fixed by splitting tpm2_pcr_mask_good(c, pcr_bank, hash_pcr_mask|pubkey_pcr_mask) into two invocations for hash_pcr_mask and pubkey_pcr_mask separately using the correct bank in each case. And of course, in all other places where these two PCR sets are possibly combined.

Nice bonus would be documentation fix explaining that values in --tpm2-public-key-pcrs= are ignored and additional warning when they are present.

@poettering
Copy link
Member

My alternative is here: #32993. It goes a different route and just picks a PCR bank automatically from the used ones if we cannot derive any from the input because the user disabled literal PCR policies.

@poettering
Copy link
Member

  1. PCR mask is meaningless without knowing which bank will be used. Maybe bank should be part of the PCR mask object.

If you pass literal PCR values we'll derive the bank from that. If you don't we automatically pick a suitable one from the ones we see used on the TPM currently.

@poettering
Copy link
Member

2. While the actual values are useless anyway (they are never used for the signed policy),

Yeah, the current tpm2_parse_pcr_argument_to_mask() parser being based on tpm2_parse_pcr_argument() is a bit stupid: i.e. it parses hash algs/values and then throws them out of the window, is nonsensical. It should refuse them.

I added a TODO list item to clean this up.

the bank indication is used and must match the content of --tpm2-signature= parameter. But as bank from --tpm2-public-key-pcrs= is ignored, systemd-cryptenroll is implicitly using the bank from --tpm2-pcrs=.

yeah we stick to a single bank per enrollment. If you specify literal hash values, then we use the bank this implies. And otherwise we were supposed to pick the bank automatically as suitable, but this was broken. #32993 fixes that part.

And I am very sure we should not make the bank explicitly configurable. This should work automatically, there's no reason to make it configurable, it's just a knob no smart person could reasonably answer. Hence: determine it automatically unless user gives us some other indirect indication. Which is what #32993 does.

@arvidjaar
Copy link
Contributor

And I am very sure we should not make the bank explicitly configurable.

It is explicitly configurable in systemd-measure. Why? Then drop this part too and always store all possible banks.

@poettering
Copy link
Member

And I am very sure we should not make the bank explicitly configurable.

It is explicitly configurable in systemd-measure. Why? Then drop this part too and always store all possible banks.

That's different. In systemd-measure we select the banks to pre-calculate for, which typically should be a large number, because we don't know the hardware/system of the target device yet, we operate in "offline" fashion after all. In systemd-cryptenroll we operate locally generally, and "online", hence we do know the system, and we only need a single bank, not many. Hence there we pick automatically.

@bluca bluca added replaced-by-newer-pr and removed please-review PR is ready for (re-)review by a maintainer labels May 23, 2024
@bluca
Copy link
Member

bluca commented May 23, 2024

replaced by #32993

@bluca bluca closed this May 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

[UKI] Cant unlock luks device after adding new certs to EFI
4 participants