-
Notifications
You must be signed in to change notification settings - Fork 166
Add TPM2_HMAC_Start implementation #351
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
Conversation
nckrss
commented
Feb 16, 2024
- See definition in Part 3, Commands, section 17.2
- See definition in Part 3, Commands, section 17.2
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have only some very minor requests while the Cirrus CI re-runs. Thank you for sending this change!
tpm2/constants.go
Outdated
// Context Handle Values come from table 211 | ||
const ( | ||
// an ordinary transient object | ||
TPMIDHSSavedTransient TPMIDHSaved = 0x80000000 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: These symbols don't have a name in the spec. I like your suggestions except for the extra S: could you name them TPMIDHSavedTransient
, TPMIDHSavedSequence
, etc.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated in f3f6077
tpm2/structures.go
Outdated
switch (byte)(h >> 24) { | ||
case 0x00, 0x02, 0x03, 0x40: | ||
switch (TPMHT)(h >> 24) { | ||
case TPMHTPCR, TPMHTHMACSession, TPMHTPolicySession, TPMHTPermanent, TPMHTTransient: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are you sure it's right to add TPMHTTransient
here? A transient object's name is not just the big-endian byte array of its handle, unlike with PCR, HMAC and Policy sessions, and Permanent entities.
(Thank you for the cleanup though, putting the actual symbols here!)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for calling this out, I mis-interpreted Part 2 Structures section 14.6.2 savedHandle.
Based on the table from Part 1 Architecture section 16 Table 3, I changed the handling in 0d43555 to address the case of a sequence object and to allow for Name computation of Transient and Persistent Objects in future implementations.
I first thought of changing the AuthHandle{}
KnownName()
method because passing in an empty AuthHandle.Name
1 would default to AuthHandle.Handle.KnownName()
being called, which will always be nil
because the Name associated with a sequenceHandle is an empty buffer1. The nil
Name value would then error in cmdNames
.
Rather than modify the AuthHandle
struct method I opted to extend the Name computation since it is defined for both Transient and Persistent Objects in Part Architecture section 16 Table 3.
Concerns or suggestions?
1 Part 1: Architecture, section 32.4.5
I updated the |
Thanks @nckrss for the change! |
If you would like to do this, I would love to review the PR! Thanks in advance! |
Opened #353 |