-
Notifications
You must be signed in to change notification settings - Fork 369
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
esys: remove trailing zeros in auth value. #2665
esys: remove trailing zeros in auth value. #2665
Conversation
9ef067d
to
1ba1f5b
Compare
1ba1f5b
to
1cbf7a3
Compare
@JuergenReppSIT we updated the bug tracker #2660 with complete trigger examples, any way you could respin this to get it to fail? I backed your patch out and ran the test and couldn't get a failure. Perhaps I didn't do something properly locally. Don't we also need to handle the auth values when we create objects (Esys_Create, Esys_CreateLoaded, Esys_CreatePrimary, Esys_NV_DefineSpace) since they get remembered? |
1cbf7a3
to
361a436
Compare
@williamcroberts sorry I forgot, that Esys_Create does not create an esys object. The object will be created by Esys_Load. So the call of Esys_TR_SetAuth is needed for keys.
|
dea8a31
to
1badbb8
Compare
store_input_parameters(esysContext, authHandle, newAuth); | ||
iesys_strip_trailing_zeros(&esysContext->in.HierarchyChangeAuth.newAuth); |
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.
any reason to not do this in store_input_parameters?
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.
A good hint. The spec states:
This command does not change the authorization of the TPM-resident object on which it operates.
Therefore, the old authValue (of the TPM-resident object) is used when generating the response HMAC
key if required.
So it makes no sense to store the auth value in the esys object. I will remove this change.
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 think @williamcroberts wants the "iesys_strip_trailing_zeros" command to be implemented in the "store_input_parameters" function. This makes also sense to me.
@JuergenReppSIT: Could you do this?
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.
@williamcroberts @cplappert Sorry my comment was related to Esys_ObjectChangeAuth. I will do it in store_input_parameters.
/* Retrieve the metadata objects for provided handles */ | ||
r = esys_GetResourceObject(esysContext, objectHandle, &objectHandleNode); | ||
return_state_if_error(r, _ESYS_STATE_INIT, "objectHandle unknown."); | ||
r = esys_GetResourceObject(esysContext, parentHandle, &parentHandleNode); | ||
return_state_if_error(r, _ESYS_STATE_INIT, "parentHandle unknown."); | ||
|
||
if (objectHandleNode->rsrc.rsrcType == IESYSC_KEY_RSRC) { |
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.
what about NV objects?
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.
The spec states:
NOTE 3 If an NV Index is to have a new authorization, it is done with TPM2_NV_ChangeAuth().
2ad640f
to
f32bda0
Compare
0f4e2c8
to
26068c4
Compare
When TPM Calulates the HMAC trailing zeros are rmoved. Therfore the trailing zeros are removed in Esys_TR_SetAuth, Esys_Create, Esys_CreateLoded, Esys_NV_DefineSpace, and Esys_CreatePrimary. The removing is added to the function iesys_hash_long_auth_value. Therefore this function is renamed to iesys_adapt_auth_value. An integration test which uses trailing zeros in auth values is added. The auth value handling in ChangeAuth programs is fixed: * The trailing zeros are now removed in these programs. * The new auth value now is stored in objects where the auth value is changed with Esys_ObjectChangeAuth. * the integration test which checkd trailing zeros is extend. Fixes: tpm2-software#2664 Signed-off-by: Juergen Repp <[email protected]>
26068c4
to
d103e84
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #2665 +/- ##
==========================================
+ Coverage 82.57% 82.58% +0.01%
==========================================
Files 368 368
Lines 42985 43011 +26
==========================================
+ Hits 35495 35522 +27
+ Misses 7490 7489 -1 ☔ View full report in Codecov by Sentry. |
When TPM Calulates the HMAC trailing zeros are rmoved. Therfore the
trailing zeros are removed in Esys_TR_SetAuth, Esys_Create, Esys_CreateLoded,
Esys_NV_DefineSpace, and Esys_CreatePrimary.
The auth value is also adapted in the ChangeAuth programs
Addresses #2664
Signed-off-by: Juergen Repp [email protected]