Skip to content
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: StartAuthSession bind auth trailing zeroes #2836

Conversation

throwException
Copy link
Contributor

When StartAuthSession is called with a bind entity with a auth value containing trailing zeroes, the HMAC or policy session computation of ESYS does not match the computation on the TPM2.

The fix is to remove trailing zeroes from the auth value according to the specification (TPM2 Architecture, 19.6.5, Note 2) before computation of the session key.

The fixed bug is especially tricky as a randomly generated auth value of the bind object can cause HMAC or policy session to fail occassionally.

@throwException throwException force-pushed the esys_startauthsession_bind_auth_trailing_zeroes branch 2 times, most recently from f7f97e8 to 818fdb1 Compare May 11, 2024 13:39
@tomoveu
Copy link
Contributor

tomoveu commented May 13, 2024

good find @throwException

*
* Remove tailing zeroes from the auth value
*/
while ((bindNode->auth.size > 0) &&
Copy link
Contributor

Choose a reason for hiding this comment

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

I think the auth.size > 0 check needs to be part of the original if statement and then use a working variable for the removal of trailing zeroes. It is more steps, but safer in my opinion.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@tomoveu You are right, the bindNode->auth shouldn't be changed. But since this is passed to iesys_compute_bound_entity later, the TPM2B_DIGEST structure needs to be copied into a working variable I name bindNodeAuth.

The computation of secret_size was off in the original merge request, which I corrected by moving the removal of trailing zeros.

Finally, some lines below can be simplied since bindNodeAuth.size is always valid and 0 if bindNode == NULL.

@throwException throwException force-pushed the esys_startauthsession_bind_auth_trailing_zeroes branch 2 times, most recently from d5af351 to 256e646 Compare May 13, 2024 12:19
When StartAuthSession is called with a bind entity with a auth value
containing trailing zeroes, the HMAC or policy session computation
of ESYS does not match the computation on the TPM2.

The fix is to remove trailing zeroes from the auth value according
to the specification (TPM2 Architecture, 19.6.5, Note 2) before
computation of the session key.

The fixed bug is especially tricky as a randomly generated auth value
of the bind object can cause HMAC or policy session to fail
occassionally.

Signed-off-by: Stefan Thöni <stefan.thoeni@gapfruit.com>
@throwException throwException force-pushed the esys_startauthsession_bind_auth_trailing_zeroes branch from 256e646 to 80f8733 Compare May 13, 2024 12:31
@AndreasFuchsTPM
Copy link
Member

I am wondering, why

iesys_strip_trailing_zeros(&esys_object->auth);

and
iesys_strip_trailing_zeros(auth_value);

are not effective in this case of bind ?

The idea was and is that the auth values carried in the metadata are always free of trailing zeros to begin with.
Thus I don't like the fix at this place but rather want to find out the call path that circumvented the previous cases.

@throwException
Copy link
Contributor Author

@AndreasFuchsTPM I'm sorry, this bug was already fixed in 4.0.2 by commit d3bcce8.

@AndreasFuchsTPM
Copy link
Member

No worries, PRs are always welcome !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants