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.

Sign up for GitHub

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

Jump to bottom

Allow platform to choose Signer Identifier and Cert validity #292

Merged
merged 2 commits into from
Jan 9, 2024
Merged

Allow platform to choose Signer Identifier and Cert validity #292

merged 2 commits into from
Jan 9, 2024

Conversation

sree-revoori1
Copy link
Contributor

fixes #291
fixes #280

@sree-revoori1 sree-revoori1 requested a review from jhand2 December 19, 2023 16:55
@sree-revoori1 sree-revoori1 force-pushed the sid branch 2 times, most recently from 19f59d1 to b0c901a Compare December 19, 2023 17:14
@sree-revoori1 sree-revoori1 marked this pull request as draft December 19, 2023 17:59
@sree-revoori1 sree-revoori1 marked this pull request as ready for review January 1, 2024 19:45
jhand2
jhand2 reviewed Jan 2, 2024
View reviewed changes
jhand2
jhand2 reviewed Jan 8, 2024
View reviewed changes
Copy link
Collaborator

@jhand2 jhand2 left a comment

Choose a reason for hiding this comment

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

Looks good! Does the verification test succeed now if you remove this?

caliptra-dpe/verification/testing/certifyKey.go

Lines 173 to 178 in 2e4110b

// This library expects the issuer of the cert to match the issuer of the CMS.
//
// The last cert in the cert chain would be signed by the preceding cert
// so its issuer is not what the library expects. The DPE leaf cert is signed
// by the alias key so it's issuer will match the issuer of the CMS.
lastCertInCertChain.RawIssuer = leafCert.RawIssuer

esmusick and others added 2 commits January 9, 2024 03:21
@esmusick @sree-revoori1
Minor updates to RustCrypto impl
73805f0 
* Remove redundant `use` in crypto
* Reorganize default platform parsing options
@sree-revoori1
Allow platform to choose Signer Identifier and Cert validity
c50547b
@sree-revoori1
Copy link
Contributor Author

Looks good! Does the verification test succeed now if you remove this?

caliptra-dpe/verification/testing/certifyKey.go

Lines 173 to 178 in 2e4110b

// This library expects the issuer of the cert to match the issuer of the CMS.
//
// The last cert in the cert chain would be signed by the preceding cert
// so its issuer is not what the library expects. The DPE leaf cert is signed
// by the alias key so it's issuer will match the issuer of the CMS.
lastCertInCertChain.RawIssuer = leafCert.RawIssuer

Yes, it passes now.

@jhand2 jhand2 merged commit 11c9260 into chipsalliance:main Jan 9, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jhand2 jhand2 jhand2 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Let platform choose cert validity Fix IssuerAndSerialNumber in CMS Content message
3 participants
@sree-revoori1 @jhand2 @esmusick