Updates for SAN parsing #229
Conversation
There was a problem hiding this comment.
One minor comment / request for a explanatory code comment.
Otherwise I think this is good! This is a breaking change, but it's pretty easy for clients to update and remove that argument from the few places they call things like NewSANMatcher() or NewShortCertificateIdentity(). For example, in https://github.com/cli/cli/blob/trunk/pkg/cmd/attestation/verify/policy.go#L27-L43 we just pass an empty string for the sanType currently.
| if len(leafCert.UnhandledCriticalExtensions) > 0 { | ||
| var unhandledExts []asn1.ObjectIdentifier | ||
| for _, oid := range leafCert.UnhandledCriticalExtensions { | ||
| if !oid.Equal(cryptoutils.SANOID) { | ||
| unhandledExts = append(unhandledExts, oid) | ||
| } | ||
| } | ||
| leafCert.UnhandledCriticalExtensions = unhandledExts | ||
| } |
There was a problem hiding this comment.
Minor: this is probably worth a comment, as it took me some time to figure out! Maybe something like:
We handle the Subject Alternative Name (`SANOID`) in pkg/fulcio/certificate, so we remove it from the unhandled critical extensions list here, otherwise x.509 verification will fail
Feel free to edit / improve on that suggested wording!
There was a problem hiding this comment.
Done, thanks for pointing this out
Added a release note to the PR description |
Simplify certificate identities by removing the Type attribute from the SubjectAlternativeName type. The CA is expected to handle validating that SAN values match their designated types. Signed-off-by: Colleen Murphy <colleenmurphy@google.com>
Add support for parsing and verifying a Fulcio certificate with a username identity issued from an OIDC provider. See [1]. [1] https://github.com/sigstore/fulcio/blob/main/docs/oid-info.md#1361415726417--othername-san Signed-off-by: Colleen Murphy <colleenmurphy@google.com>
|
@steiza I went ahead and rebased to remove the merge commit |
|
Thank you! |
Remove Type for SubjectAlternativeName
Simplify certificate identities by removing the Type attribute from the
SubjectAlternativeName type. The CA is expected to handle validating
that SAN values match their designated types.
Fixes #175
Support OtherName SAN in Fulcio cert
Add support for parsing and verifying a Fulcio certificate with a
username identity issued from an OIDC provider. See [1].
[1] https://github.com/sigstore/fulcio/blob/main/docs/oid-info.md#1361415726417--othername-san
Fixes #55
Summary
Release Note
API change -
github.com/sigstore/sigstore-go/pkg/fulcio/certificate: removed theSubjectAlternativeNameTypetype and changed the type of theSubjectAlternativeNamefield in theSummarystruct to be a string instead of a struct.API change -
github.com/sigstore/sigstore-go/pkg/verify: changed theSubjectAlternativeNamefield of theSubjectAlternativeNameMatcherstrcut to be a string instead of a struct. Changed the signature for functionsNewSANMatcherandNewShortCertificateIdentity.Feature: added support for verifying Fulcio certificates that use the OtherName GeneralName (RFC 5280 4.2.1.6) as a certificate identity.
Documentation