-
Notifications
You must be signed in to change notification settings - Fork 26
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 interface types for TimestampingAuthority and CertificateAuthority #300
Conversation
Signed-off-by: Cody Soyland <[email protected]>
Signed-off-by: Cody Soyland <[email protected]>
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.
Nice work!
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! Just a few comments about testing and some clean up.
Signed-off-by: Cody Soyland <[email protected]>
Signed-off-by: Cody Soyland <[email protected]>
Signed-off-by: Cody Soyland <[email protected]>
Signed-off-by: Cody Soyland <[email protected]>
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.
One small suggested change, otherwise LGTM!
@@ -564,7 +566,7 @@ func (v *SignedEntityVerifier) Verify(entity SignedEntity, pb PolicyBuilder) (*V | |||
// > Unless performing online verification (see §Alternative Workflows), the Verifier MUST extract the SignedCertificateTimestamp embedded in the leaf certificate, and verify it as in RFC 9162 §8.1.3, using the verification key from the Certificate Transparency Log. | |||
|
|||
if v.config.weExpectSCTs { | |||
err = VerifySignedCertificateTimestamp(leafCert, v.config.ctlogEntriesThreshold, v.trustedMaterial) | |||
err = VerifySignedCertificateTimestamp(chains, v.config.ctlogEntriesThreshold, v.trustedMaterial) |
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 love this change - that we're actually verifying the SCT against the chain the leaf cert verified with!
Signed-off-by: Cody Soyland <[email protected]>
Signed-off-by: Cody Soyland <[email protected]>
Co-authored-by: Zach Steindler <[email protected]> Signed-off-by: Cody Soyland <[email protected]>
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.
LGTM!
…rface-types Signed-off-by: Cody Soyland <[email protected]>
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.
Still LGTM!
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.
fantastic work!
This PR modifies the
TrustedMaterial
interface with interface types forCertificateAuthority
andTimestampingAuthority
. This decouples the originalCertificateAuthority
struct from the verifier and allows clients to have more control over the verification of certificates and timestamps.Additionally, this PR modifies the verifier to output the chains of certificates that were used to verify the certificate, which has the added benefit of improving performance of the SCT verifier by only attempting SCT verification on the chain that was used to verify the certificate.
This is a breaking change to the
TrustedMaterial
interface, so clients that directly modifyTrustedMaterial
must be updated to use the new interface types.Fixes #293
Summary
Release Note
Documentation