tpm2: Update the policy for the PCR policy counter index #370
Conversation
This file contains 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
This adds a new branch which permits using the TPM2_PolicyNV command against the index without any authorization, much like there is already a branch that permits TPM2_NV_Read. This will make it compatible with automatic branch selection when the tpm2 package starts to make use of Policy.Execute from the github.com/canonical/go-tpm2/policyutil packagevin the future, without having to retrospectively modify the policy of NV indexes on existing devices.
chrisccoulson
force-pushed
the
tpm2-update-nv-index-policy
branch
from
8114797 to
b7a7bd3
Compare
|
Note that most of the new code in this PR is a clone of the code in |
pedronis
reviewed
Jan 28, 2025
tpm2/policy_v3.go
Outdated
| // pcrPolicyData_v3 represents version 3 of the PCR policy metadata for | ||
| // executing a policy session, and can be updated. It has the same format | ||
| // as version 2. | ||
| type pcrPolicyData_v3 = pcrPolicyData_v2 | ||
| type pcrPolicyData_v3 struct { | ||
| Selection tpm2.PCRSelectionList |
There was a problem hiding this comment.
couldn't this just embed pcrPolicyData_v2 and replace below only the methods that have changes? it's very hard to spot what are difference between the methods are atm, are they mostly in executeRevocationCheck ?
There was a problem hiding this comment.
Yes, the changes are only in executeRevocationCheck. I'll try what you suggested.
There was a problem hiding this comment.
Ok, I've done this now, although it did require a bit more work in the unit tests.
Rather than fully reimplement pcrPolicyData_v3, which only has minor differences to pcrPolicyData_v2, instead, just embed pcrPolicyData_v2 as an anonymous member of the new pcrPolicyData_v3 and add an override for the method that is different.
pedronis
reviewed
Jan 28, 2025
Comment on lines
+179
to
+200
| // XXX(chrisccoulson): This is the 3rd session we open here, with all 3 loaded | ||
| // on the TPM. PC Client TPMs are only guaranteed to support 3 loaded sessions, | ||
| // so we're pushing things a bit close to the wire here. This code does execute | ||
| // during early boot, but it might be better to make the HMAC session associated | ||
| // with the TPM connection unloaded by default (context saved), and only context | ||
| // load it when needed. | ||
| revocationCheckSession, err := tpm.StartAuthSession(nil, nil, tpm2.SessionTypePolicy, nil, counter.Name().Algorithm()) | ||
| if err != nil { | ||
| return fmt.Errorf("cannot create policy session for revocation check: %w", err) | ||
| } | ||
| defer tpm.FlushContext(revocationCheckSession) | ||
|
|
||
| authPolicies, err := computeV3PcrPolicyCounterAuthPolicies(counter.Name().Algorithm(), updateKey) | ||
| if err != nil { | ||
| return policyDataError{fmt.Errorf("cannot compute auth policies for PCR policy counter: %w", err)} | ||
| } | ||
| if err := tpm.PolicyCommandCode(revocationCheckSession, tpm2.CommandPolicyNV); err != nil { | ||
| return err | ||
| } | ||
| if err := tpm.PolicyOR(revocationCheckSession, authPolicies); err != nil { | ||
| return err | ||
| } |
There was a problem hiding this comment.
it would be good to have a comment here that explain why we create a new session and what we do with it
There was a problem hiding this comment.
I've added a comment now.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This adds a new branch which permits using the
TPM2_PolicyNVcommandagainst the index without any authorization, much like there is already
a branch that permits
TPM2_NV_Read.This will make it compatible with automatic branch selection when the
tpm2package starts to make use ofPolicy.Executefrom thegithub.com/canonical/go-tpm2/policyutilpackage in the future, withouthaving to retrospectively modify the policy of NV indexes on existing
devices.