tpm: Hash qualifying data in TPM2_Certify for IAK-based AK certification#1239
Merged
Conversation
📝 WalkthroughWalkthroughThe PR changes agent registration to compute SHA‑256(agent_uuid) as TPM certify() qualifying_data (used for IAK/IDevID), derives attest/signature during register_agent, removes precomputed attest/signature fields, and updates API v2.6 support, tests, and CI environment pinning. ChangesIAK/IDevID Certification and Registration Flow
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related issues
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packit-ci.fmf`:
- Around line 9-10: The CI is pointing to a personal fork/branch; update
KEYLIME_UPSTREAM_URL and KEYLIME_UPSTREAM_BRANCH to use the canonical Keylime
repository and an immutable ref: set KEYLIME_UPSTREAM_URL to
"https://github.com/keylime/keylime.git" (or the official org URL) and set
KEYLIME_UPSTREAM_BRANCH to a pinned commit SHA or a stable tag that includes
verifier-side hash support (instead of "hash_before_certify"); ensure the chosen
tag/SHA is referenced in the CI config so the e2e dependency is reproducible and
stable.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 4dd81425-b0fa-4d0b-b961-a65d1d92985e
📒 Files selected for processing (4)
keylime-agent/src/main.rskeylime/src/config/base.rskeylime/src/tpm.rspackit-ci.fmf
Comment on lines
+9
to
+10
| KEYLIME_UPSTREAM_URL: "https://github.com/ansasaki/keylime.git" | ||
| KEYLIME_UPSTREAM_BRANCH: "hash_before_certify" |
There was a problem hiding this comment.
Use canonical upstream (preferably immutable ref) for e2e dependency source.
Pointing CI to a personal fork/feature branch makes gating non-reproducible and vulnerable to branch rewrites or disappearance. Please switch to the official Keylime repo and a stable ref (ideally a pinned commit/tag containing the verifier-side hash support).
Suggested change
- KEYLIME_UPSTREAM_URL: "https://github.com/ansasaki/keylime.git"
- KEYLIME_UPSTREAM_BRANCH: "hash_before_certify"
+ KEYLIME_UPSTREAM_URL: "https://github.com/keylime/keylime.git"
+ KEYLIME_UPSTREAM_BRANCH: "main"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| KEYLIME_UPSTREAM_URL: "https://github.com/ansasaki/keylime.git" | |
| KEYLIME_UPSTREAM_BRANCH: "hash_before_certify" | |
| KEYLIME_UPSTREAM_URL: "https://github.com/keylime/keylime.git" | |
| KEYLIME_UPSTREAM_BRANCH: "main" |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packit-ci.fmf` around lines 9 - 10, The CI is pointing to a personal
fork/branch; update KEYLIME_UPSTREAM_URL and KEYLIME_UPSTREAM_BRANCH to use the
canonical Keylime repository and an immutable ref: set KEYLIME_UPSTREAM_URL to
"https://github.com/keylime/keylime.git" (or the official org URL) and set
KEYLIME_UPSTREAM_BRANCH to a pinned commit SHA or a stable tag that includes
verifier-side hash support (instead of "hash_before_certify"); ensure the chosen
tag/SHA is referenced in the CI config so the e2e dependency is reproducible and
stable.
ansasaki
force-pushed
the
hash_before_certify
branch
from
c2b889c to
9368744
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
SHA-256 hash the agent ID before passing it as qualifyingData to TPM2_Certify when certifying the AK with the IAK. This produces a fixed 32-byte value that fits within TPM2B_DATA on all TPMs, including those that only support SHA-256 (where the limit is 34 bytes). Without this fix, UUID strings (36 bytes) and other agent IDs exceed the TPM2B_DATA capacity on SHA-256-only TPMs, causing TPM_RC_SIZE errors during registration. Bumps the pull-mode API version to 2.6. Fixes: keylime#1226 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
ansasaki
force-pushed
the
hash_before_certify
branch
from
9368744 to
81a9b9b
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (1)
packit-ci.fmf (1)
9-10:⚠️ Potential issue | 🟠 MajorCI dependency should use canonical repository and immutable reference.
As noted in previous review, pointing to a personal fork branch makes CI non-reproducible and vulnerable to branch rewrites or removal. Once the companion registrar PR supporting hashed qualifying data is merged upstream, update these to reference the official Keylime repository and a stable tag or commit SHA.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packit-ci.fmf` around lines 9 - 10, Replace the personal fork and mutable branch in the CI variables by pointing KEYLIME_UPSTREAM_URL to the official Keylime repository (e.g. https://github.com/keylime/keylime.git) and set KEYLIME_UPSTREAM_BRANCH to an immutable reference (a released tag or specific commit SHA) once the upstream registrar PR is merged; update the variables KEYLIME_UPSTREAM_URL and KEYLIME_UPSTREAM_BRANCH accordingly to ensure reproducible builds.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Duplicate comments:
In `@packit-ci.fmf`:
- Around line 9-10: Replace the personal fork and mutable branch in the CI
variables by pointing KEYLIME_UPSTREAM_URL to the official Keylime repository
(e.g. https://github.com/keylime/keylime.git) and set KEYLIME_UPSTREAM_BRANCH to
an immutable reference (a released tag or specific commit SHA) once the upstream
registrar PR is merged; update the variables KEYLIME_UPSTREAM_URL and
KEYLIME_UPSTREAM_BRANCH accordingly to ensure reproducible builds.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: c1717806-e85d-47b5-b723-5bfeb7a3f4f4
📒 Files selected for processing (8)
keylime-agent/src/api.rskeylime-agent/src/main.rskeylime-push-model-agent/src/registration.rskeylime/src/agent_registration.rskeylime/src/config/base.rskeylime/src/registrar_client.rskeylime/src/tpm.rspackit-ci.fmf
💤 Files with no reviewable changes (1)
- keylime-push-model-agent/src/registration.rs
sergio-correia
approved these changes
May 13, 2026
Contributor
sergio-correia
left a comment
sergio-correia
left a comment
There was a problem hiding this comment.
Thanks for the updates, LGTM
ansasaki
force-pushed
the
hash_before_certify
branch
from
81a9b9b to
6a6b104
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
tpm: Hash qualifying data in TPM2_Certify for IAK-based AK certification
Type of Change
(Select all that apply)
Related Issues
Resolves: #1226
See also: keylime/keylime#1892
Change Description
Concise Summary
Hash agent ID (SHA-256) before passing it as qualifying data to
TPM2_Certify.Technical Details
When the agent certifies the AK with the IAK using
TPM2_Certify, it passes the agent ID string directly asqualifyingData(TPM2B_DATA). The TPM 2.0 spec limitsTPM2B_DATAtosizeof(TPMT_HA)bytes, which depends on the TPM's supported hash algorithms. On SHA-256-only TPMs (e.g., NVIDIA Grace Hopper aarch64), this limit is 34 bytes — smaller than a standard UUID string (36 bytes). Since Keylime supports arbitrary strings as agent IDs, the qualifying data can be even longer.This PR hashes the agent ID with SHA-256 before using it as qualifying data, producing a fixed 32-byte value that fits within
TPM2B_DATAon any TPM. The qualifying data is arbitrary per the TPM 2.0 spec (Part 3, Section 18.2) — hashing it is fully compliant.Changes:
Hash agent ID in
main.rs: Usekeylime::crypto::hashwithMessageDigest::sha256()to hash the agent ID before passing it toTPM2_Certify. No new dependencies — uses the existingopensslcrate via the keylime crypto module.Update test in
tpm.rs:test_certify_credential_with_iaknow hashes the qualifying data to match the production code path.Bump pull-mode API version to 2.6 in
config/base.rs.Documentation Updates Required
(Check all that apply)
docs/)Documentation updates are included in the companion Python PR.
Verification Process
cargo fmt && cargo clippy --all-targets— no new warningscargo test— existing tests passChecklist
Additional Context
keylime::crypto::hashfunction backed byopenssl.Summary by CodeRabbit
New Features
Updates
Tests / CI