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

Add ctx.get_time #551

Merged
merged 2 commits into from
Nov 1, 2024
Merged

Add ctx.get_time #551

merged 2 commits into from
Nov 1, 2024

Conversation

nilsb99
Copy link

@nilsb99 nilsb99 commented Oct 17, 2024

For a time-based remote attestation project in a uni class, we needed this function and wanted to contribute it to upstream.

We followed the existing implementation of ctx.quote() to write ctx.get_time() and its test.

Copy link
Collaborator

@Superhepper Superhepper left a comment

Choose a reason for hiding this comment

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

Thank you so much for the PR. I had some comments.

Superhepper
Superhepper previously approved these changes Oct 23, 2024
Copy link
Member

@ionut-arm ionut-arm left a comment

Choose a reason for hiding this comment

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

Awesome, thanks for the patch, it looks pretty good! Amazing that you've worked with something like this in Uni :)

Comment on lines 415 to 416
self.optional_session_1(),
self.optional_session_2(),
Copy link
Member

Choose a reason for hiding this comment

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

I think these two are actually required, the first one for the endorsement, the second for the signing key. Looking at quote above I wonder if that's a bug, AFAICT the spec asks for one authorization session at least there.

Copy link
Collaborator

Choose a reason for hiding this comment

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

TCG TSS 2.0 Enhanced System API (ESAPI) Specification, Version 1.00, Revision 14, October 1, 2021

11.3.45 Esys_GetTime Functions
TSS2_DLL_EXPORT TSS2_RC Esys_GetTime_Async(
ESYS_CONTEXT *esysContext,
ESYS_TR privacyAdminHandle,
ESYS_TR signHandle,
ESYS_TR privacyAdminHandleSession1,
ESYS_TR signHandleSession2,
ESYS_TR optionalSession3,
TPM2B_DATA const *qualifyingData,
TPMT_SIG_SCHEME const *inScheme);

I think you are right only the third session is optional.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Regarding Esys_Quote.

TCG TSS 2.0 Enhanced System API (ESAPI) Specification, Version 1.00, Revision 14, October 1, 2021

11.3.42 Esys_Quote Functions
TSS2_DLL_EXPORT TSS2_RC Esys_Quote_Async(
ESYS_CONTEXT *esysContext,
ESYS_TR signHandle,
ESYS_TR signHandleSession1,
ESYS_TR optionalSession2,
ESYS_TR optionalSession3,
TPM2B_DATA const *qualifyingData,
TPMT_SIG_SCHEME const *inScheme,
TPML_PCR_SELECTION const *PCRselect);

So it seems Quote is wrong as well because the first session should be required.

Nils Bourcarde and others added 2 commits October 30, 2024 18:26
Signed-off-by: Nils Bourcarde <[email protected]>
Signed-off-by: Maurice Blattmann <[email protected]>
@Superhepper
Copy link
Collaborator

@nilsb99 I did not ask this before. But do you need this in an official release? Because if you do I suggest you cherry-pick this to the 7.x.y branch in a new PR.

Copy link
Member

@ionut-arm ionut-arm left a comment

Choose a reason for hiding this comment

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

🚀

@ionut-arm ionut-arm merged commit 2dd2308 into parallaxsecond:main Nov 1, 2024
12 checks passed
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.

4 participants