-
Notifications
You must be signed in to change notification settings - Fork 12
Fix PreIssuer detection #554
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
Conversation
6a9cb06 to
1e18ded
Compare
|
Since it seems really unfortunate that |
|
Technically RFC 6962 specifies that a Precertificate Signing Certificate needs to also have CA:true. I'm assuming that's already checked when the whole chain is validated, but maybe it's a good idea to check it in |
5b8afe8 to
9ed34cc
Compare
internal/x509util/ct_test.go
Outdated
| `) | ||
|
|
||
| var preIssuerExt = func() pkix.Extension { | ||
| func EKUOIDToExt(ekus []asn1.ObjectIdentifier) pkix.Extension { |
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.
ekuExtWithOIDs?
internal/x509util/ct_test.go
Outdated
| }() | ||
| } | ||
|
|
||
| var preIssuerExt = EKUOIDToExt([]asn1.ObjectIdentifier{rfc6962.OIDExtKeyUsageCertificateTransparency}) |
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.
How about preIssuerEKUExt?
Probably worth a comment on here about why this rather than using x509 support directly.
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.
Done. Note that we might be able to use x509 support when we build the certificates in tests... Test might even be useful to detect if the x509 library ever changes its behavior. But I decided not to pursue this option, to stay away from any trouble, and to stay consistent with the base implementation.
Fixes #553