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

proposal: crypto/x509: Add support for OtherName SAN #55897

Closed
haydentherapper opened this issue Sep 27, 2022 · 7 comments
Closed

proposal: crypto/x509: Add support for OtherName SAN #55897

haydentherapper opened this issue Sep 27, 2022 · 7 comments
Labels
FrozenDueToAge Proposal Proposal-Crypto Proposal related to crypto packages or other security issues
Milestone

Comments

@haydentherapper
Copy link

haydentherapper commented Sep 27, 2022

Context: I want to create a code-signing certificate whose SAN is set to an OtherName. I'm also happy to propose adding support for all GeneralName fields, although I recognize some are more esoteric.

It is currently possible to construct a certificate with a GeneralName that is not a DNS/email/IP address/URI by constructing the SAN extension and including it in ExtraExtensions before calling x509.CertificateCertificate. This is doable, but not ideal, as it requires some knowledge of ASN.1 encoding.

However, it is possible to construct a certificate that is not verifiable after parsing, if the SAN extension is set to critical and a GeneralName is set that is not a DNS/email/IP address/URI. When parsing the SAN, if no supported GeneralName is found, then the verifier will fail due to an unhandled critical extension.

RFC5280 states that "If the subject field contains an empty sequence, then the issuing CA MUST include a subjectAltName extension that is marked as critical." If I want to issue a certificate with an unsupported GeneralName, then I must mark the SAN extension as non-critical and not conform to RFC5280.

I propose adding support for OtherName, and if desired, for all GeneralName fields. I would prefer that we add support for both creating and parsing certificates with OtherName. If it's preferred to not support creating certificates with OtherName (since you could still by adding the SAN to ExtraExtensions), I would propose to add support for parsing only then, adding a field to the Certificate struct that's only written to when parsing.

@gopherbot gopherbot added this to the Proposal milestone Sep 27, 2022
@haydentherapper
Copy link
Author

I've found a workaround, removing the SAN OID from cert.UnhandledCriticalExtensions before calling cert.Verify.

For justification for adding support, there is an example of OtherName being used for smart card certificates.

@seankhliao seankhliao changed the title Proposal: crypto/x509: Add support for OtherName SAN proposal: crypto/x509: Add support for OtherName SAN Oct 3, 2022
@seankhliao seankhliao added the Proposal-Crypto Proposal related to crypto packages or other security issues label Oct 3, 2022
@seankhliao
Copy link
Member

cc @golang/security

@rsc
Copy link
Contributor

rsc commented Oct 6, 2022

This proposal has been added to the active column of the proposals project
and will now be reviewed at the weekly proposal review meetings.
— rsc for the proposal review group

@rsc
Copy link
Contributor

rsc commented Nov 9, 2022

Talked to @rolandshoemaker. OtherName isn't allowed for web PKI (nor are the other name types), and we generally treat crypto/x509 as supporting what we need for the web, not for all of X.509. What is the reason we should add OtherName? And what constraints should it imply?

@haydentherapper
Copy link
Author

OtherName is primarily used for smart card certificates, though I've found it useful for encoding non-email subjects for code-signing certificates. Agreed that it's not used within web PKI though, so if it's that bar, then I assume it'll need to remain outside the standard library.

@rsc
Copy link
Contributor

rsc commented Nov 16, 2022

Based on the discussion above, this proposal seems like a likely decline.
— rsc for the proposal review group

@rsc
Copy link
Contributor

rsc commented Nov 30, 2022

No change in consensus, so declined.
— rsc for the proposal review group

@rsc rsc closed this as completed Nov 30, 2022
@golang golang locked and limited conversation to collaborators Nov 30, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge Proposal Proposal-Crypto Proposal related to crypto packages or other security issues
Projects
None yet
Development

No branches or pull requests

4 participants