Skip to content

Enable different key sizes and curves for EK and AK #846

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
Merged
merged 5 commits into from
May 27, 2025
Merged

Enable different key sizes and curves for EK and AK #846

merged 5 commits into from
May 27, 2025

Conversation

@THS-on
Copy link
Member

@THS-on THS-on commented Oct 1, 2024

This implements a selection of the different key sizes for RSA and curves for ECC for EK and AK using the backported changes in rust-tss-esapi.

Further this then also enables us to use EK certs for non default key types.

Note this is currently blocked until parallaxsecond/rust-tss-esapi#546 gets merged and we have a new release.

@THS-on
Copy link
Member Author

THS-on commented Oct 1, 2024 •
edited
Loading

Note that this now outputs in the quote rsa2048 instead of rsa, so we would probably need to update also something on the server side.

Edit: decided to make the string encoding for RSA 2024 just rsa

@THS-on THS-on force-pushed the ecc-ek-cert branch 4 times, most recently from cde261e to fdbdb4a Compare October 3, 2024 09:25
@codecov
Copy link

codecov bot commented Oct 3, 2024 •
edited
Loading

Codecov Report

Attention: Patch coverage is 56.25000% with 56 lines in your changes missing coverage. Please review.

Project coverage is 69.32%. Comparing base (36600c4) to head (a99bb40).
Report is 5 commits behind head on master.

Files with missing lines Patch % Lines
keylime/src/algorithms.rs 50.00% 26 Missing ⚠️
keylime/src/tpm.rs 68.65% 21 Missing ⚠️
keylime-push-model-agent/src/registration.rs 0.00% 5 Missing ⚠️
keylime-agent/src/config.rs 0.00% 4 Missing ⚠️
Additional details and impacted files
Flag Coverage Δ
e2e-testsuite 69.32% <56.25%> (-0.35%) ⬇️
upstream-unit-tests 69.32% <56.25%> (-0.35%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
keylime-agent/src/agent_handler.rs 58.69% <ø> (ø)
keylime-agent/src/common.rs 33.89% <ø> (ø)
keylime-agent/src/main.rs 19.33% <ø> (ø)
keylime-agent/src/config.rs 81.39% <0.00%> (-0.28%) ⬇️
keylime-push-model-agent/src/registration.rs 6.81% <0.00%> (-0.50%) ⬇️
keylime/src/tpm.rs 72.99% <68.65%> (-0.63%) ⬇️
keylime/src/algorithms.rs 66.16% <50.00%> (-10.85%) ⬇️

... and 3 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@THS-on
Copy link
Member Author

THS-on commented Oct 17, 2024

This is now mostly blocked on a new stable release of the tss-esapi create.

There is also the issue that the parsing of the EK TPM structure goes somewhere wrong, as the verifier says that the EK public key in the TSS structure and the one in the EK certificate don't match. I'll investigate where this goes wrong.

@THS-on
Copy link
Member Author

THS-on commented Oct 21, 2024

This PR is also blocked on parallaxsecond/rust-tss-esapi#552 being backported, otherwise the EK verification will always fail.

@THS-on
Copy link
Member Author

THS-on commented Nov 12, 2024

@ansasaki this is now fully working. We still need to wait for a new release of tss-esapi bindings to actually use it.

@ansasaki ansasaki mentioned this pull request Nov 26, 2024
Closed
28 tasks
@THS-on THS-on force-pushed the ecc-ek-cert branch 5 times, most recently from 9a9506e to 5219bae Compare December 23, 2024 14:51
@THS-on
Copy link
Member Author

THS-on commented Dec 23, 2024

Registration and EK verification using swtpm with the Nist384 curve works. Unfortunatly something with is broken when trying to verify the actual quote:

2024-12-23 16:16:17.166 - keylime.tpm - ERROR - Error verifying quote: 
2024-12-23 16:16:17.166 - keylime.tpm - ERROR - 
Traceback (most recent call last):
  File "/usr/local/lib/python3.13/site-packages/keylime/tpm/tpm_main.py", line 176, in _tpm2_checkquote
    pcrs_dict = tpm_util.checkquote(aikFromRegistrar, nonce, sigblob, quoteblob, pcrblob, hash_alg)
  File "/usr/local/lib/python3.13/site-packages/keylime/tpm/tpm_util.py", line 249, in checkquote
    verify(pubkey, signature, quote_digest, hashfunc)
    ~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.13/site-packages/keylime/tpm/tpm_util.py", line 82, in verify
    pubkey.verify(sig, digest, ec.ECDSA(Prehashed(hashfunc)))
    ~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
cryptography.exceptions.InvalidSignature
2024-12-23 16:16:17.167 - keylime.tpm - WARNING - Quote for agent d432fbb3-d2f1-4a97-9ef7-75bd81c00000 does not contain any PCRs. Make sure that the TPM supports sha256 PCR banks
2024-12-23 16:16:17.167 - keylime.tpm - ERROR - Binding PCR #16 was not included in the quote (from agent d432fbb3-d2f1-4a97-9ef7-75bd81c00000), but is required
2024-12-23 16:16:17.167 - keylime.tenant - ERROR - TPM Quote from Agent d432fbb3-d2f1-4a97-9ef7-75bd81c00000 (127.0.0.1:9002) is invalid for nonce: aY76KkOhWwK53FlNocI1

Might be an padding issue or something else that we missed.

@ansasaki ansasaki mentioned this pull request Jan 15, 2025
Closed
28 tasks
@ansasaki ansasaki mentioned this pull request Feb 25, 2025
Closed
30 tasks
@THS-on THS-on mentioned this pull request Mar 3, 2025
Merged
@THS-on THS-on requested a review from ansasaki March 3, 2025 16:04
@THS-on THS-on marked this pull request as ready for review March 4, 2025 12:51
@THS-on
Copy link
Member Author

THS-on commented Mar 4, 2025

This code is tested against latest Keylime version + keylime/keylime#1746

@THS-on THS-on requested review from sarroutbi and stefanberger March 5, 2025 08:33
@ansasaki ansasaki mentioned this pull request Mar 24, 2025
Closed
30 tasks
sarroutbi
sarroutbi reviewed Apr 8, 2025
View reviewed changes
Copy link
Contributor

@sarroutbi sarroutbi left a comment

Choose a reason for hiding this comment

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

I am quite newbie regarding this, so, in general, changes LGTM.

If possible, Code Coverage should be extended to avoid CI failing scenarios.

@ansasaki ansasaki mentioned this pull request Apr 23, 2025
Closed
30 tasks
@ansasaki
Update to tss-esapi v7.6.0
dd8fc41 
Signed-off-by: Thore Sommer <[email protected]>
Signed-off-by: Anderson Toshiyuki Sasaki <[email protected]>
ansasaki and others added 4 commits May 27, 2025 08:58
@ansasaki
config: Use next_back() instead of last() for iterators
d526499 
This fixes a warning generated by clippy.

Signed-off-by: Anderson Toshiyuki Sasaki <[email protected]>
@THS-on @ansasaki
Enable non standard key sizes and curves for EK and AK
6fd02de 
Add the missing argument when calling create_ak() to the push model
prototype

Signed-off-by: Thore Sommer <[email protected]>
Signed-off-by: Anderson Toshiyuki Sasaki <[email protected]>
@THS-on @ansasaki
tpm: add policy auth for EK to activate crendential
88884a5 
Signed-off-by: Thore Sommer <[email protected]>
Signed-off-by: Anderson Toshiyuki Sasaki <[email protected]>
@THS-on @ansasaki
keylime-agent.conf: add all accepted TPM encryption algs
a99bb40 
Signed-off-by: Thore Sommer <[email protected]>
ansasaki
ansasaki approved these changes May 27, 2025
View reviewed changes
Copy link
Contributor

@ansasaki ansasaki left a comment

Choose a reason for hiding this comment

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

I had to rebase and adjust the original patchset to cleanly merge into master.
The only failing test is about the code coverage, I guess we need to extend the test cases to include ECC keys with various curves

@ansasaki ansasaki merged commit 8c26b4e into keylime:master May 27, 2025
11 of 12 checks passed
@ansasaki ansasaki mentioned this pull request May 28, 2025
Closed
30 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@sarroutbi sarroutbi sarroutbi left review comments

@ansasaki ansasaki ansasaki approved these changes

@sergio-correia sergio-correia Awaiting requested review from sergio-correia

@ueno ueno Awaiting requested review from ueno

@lkatalin lkatalin Awaiting requested review from lkatalin

@stefanberger stefanberger Awaiting requested review from stefanberger

Assignees

@ueno ueno

@sarroutbi sarroutbi

@stefanberger stefanberger

@lkatalin lkatalin

@ansasaki ansasaki

@sergio-correia sergio-correia

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@THS-on @sarroutbi @ansasaki @ueno @stefanberger @lkatalin @sergio-correia