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

crypto/x509: Provide a mechanism for accessing SRVNames #21789

Open
SamWhited opened this issue Sep 7, 2017 · 9 comments
Open

crypto/x509: Provide a mechanism for accessing SRVNames #21789

SamWhited opened this issue Sep 7, 2017 · 9 comments
Labels
Proposal Proposal-Accepted Proposal-Crypto Proposal related to crypto packages or other security issues
Milestone

Comments

@SamWhited
Copy link
Member

The SRVName field of an X.509 certificate defined by RFC 4985 allows a certificate to be used to verify that a service is managed by a particular entity without giving the same entity control over the entire domain or subdomain (eg. if DNSName or CommonName were used).

I would like to be able to access the SRVNames from a certificate parsed using x509.ParseCertificate and the like. I can see two ways of doing this, depending on how accessible this information needs to be. The easiest would be to add a field to the Certificate struct alongside the existing SAN fields:

type Certificate struct {
    …
    // Subject Alternate Name values
    DNSNames       []string
    SRVNames       []string
    EmailAddresses []string
    IPAddresses    []net.IP
    …
}

and parse the SRVNames at the same time we parse DNSNames. This is easy to use, but it may not be desirable to pollute the struct with fields for more otherName values that aren't as widely used (especially when issues like #15196 may already cause it to balloon).

Alternatively, the raw SAN field can already be pulled out of the Extensions []pkix.Extension field. A more extensible approach might be to create a raw SAN field similar to Extensions that contains the raw map of OIDs and their values. Something like:

type Certificate struct {
    …
    // Extensions contains raw X.509 extensions. When parsing certificates,
    // this can be used to extract non-critical extensions that are not
    // parsed by this package. When marshaling certificates, the Extensions
    // field is ignored, see ExtraExtensions.
    Extensions []pkix.Extension

    // SAN contains raw X.509 extensions that were not parsed into the DNSNames,
    // EmailAddresses, or IPAddresses fields..
    SAN []pkix.Extension
    …
}

which would make it possible for users to get ahold of arbitrary fields from the SAN without requiring that they pull the blob out of Extensions and re-parse it again.
Note that this would have to be in addition to the existing raw SAN field from Extensions (we probably can't remove it since code might already be pulling the SAN field out of Extensions). I have not included an ExtraExtensions (for marshalling) equivalent for this reason (what happens when both exist and you go to create a certificate?).

/cc @agl

@gopherbot gopherbot added this to the Proposal milestone Sep 7, 2017
@agl agl self-assigned this Sep 7, 2017
@SamWhited
Copy link
Member Author

Here's what I'm doing temporarily to solve my problem (lots of code copied and modified from the standard library). If one of the solutions (or something vaguely similar) proposed in this issue is accepted I can adapt it back to crypto/x509: https://godoc.org/mellium.im/xmpp/x509

@rsc
Copy link
Contributor

rsc commented Oct 16, 2017

CL 62693 is adding more constraint checking but doesn't have SRVNames. I commented there to find out if it should be added.

@SamWhited
Copy link
Member Author

This did not end up being addressed by CL 62693; now that the 1.11 tree is open, would either of the above proposed APIs be acceptable? If so I will commit to submitting a CL during this cycle. Thanks!

@rsc
Copy link
Contributor

rsc commented Feb 26, 2018

/cc @FiloSottile @agl

@FiloSottile
Copy link
Contributor

I wouldn't object to just adding SRVNames, x509.Certificate is way past the ergonomic API surface territory. I'm more wary of adding APIs with overlapping meaning and complex extensibility, causing issues like the one you identified with Extensions.

Do you think you'll also need XMPPAddresses?

@SamWhited
Copy link
Member Author

SamWhited commented Feb 26, 2018 via email

@gopherbot
Copy link

Change https://golang.org/cl/97376 mentions this issue: crypto/x509: add SRV and XMPP fields

@SamWhited
Copy link
Member Author

SamWhited commented Feb 28, 2018

Update: I've added parsing and marshaling to the CL for certificates and certificate requests, but am waiting on the results of https://golang.org/cl/96378 before doing any sort of validation since that will probably require a more complicated rebase if I do it now.

@ianlancetaylor
Copy link
Contributor

Accepted for SRVName per @FiloSottile 's comment above.

@FiloSottile FiloSottile added the Proposal-Crypto Proposal related to crypto packages or other security issues label Nov 22, 2019
@rsc rsc unassigned agl Jun 23, 2022
@rsc rsc changed the title proposal: crypto/x509: Provide a mechanism for accessing SRVNames crypto/x509: Provide a mechanism for accessing SRVNames Aug 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Proposal Proposal-Accepted Proposal-Crypto Proposal related to crypto packages or other security issues
Projects
None yet
Development

No branches or pull requests

6 participants