golang / go

The Go programming language
https://go.dev
BSD 3-Clause "New" or "Revised" License
124.37k stars 17.71k forks source link

crypto/x509/pkix: Name.String() rfc2253 AttributeTypes #41750

Open karlovskiy opened 4 years ago

karlovskiy commented 4 years ago

What version of Go are you using (go version)?

1.15.2

Does this issue reproduce with the latest release?

Yes

What operating system and processor architecture are you using (go env)?

linux, amd64

What did you do?

Print the issuer (or subject) dn from an x509 certificate:

cert.Issuer.String()

According to https://tools.ietf.org/html/rfc2253#section-2.3 we have this table of


                    String  X.500 AttributeType
                    ------------------------------
                    CN      commonName
                    L       localityName
                    ST      stateOrProvinceName
                    O       organizationName
                    OU      organizationalUnitName
                    C       countryName
                    STREET  streetAddress
                    DC      domainComponent
                    UID     userid

But currently in crypto/x509/pkix we have this

var attributeTypeNames = map[string]string{
    "2.5.4.6":  "C",
    "2.5.4.10": "O",
    "2.5.4.11": "OU",
    "2.5.4.3":  "CN",
    "2.5.4.5":  "SERIALNUMBER",
    "2.5.4.7":  "L",
    "2.5.4.8":  "ST",
    "2.5.4.9":  "STREET",
    "2.5.4.17": "POSTALCODE",
}

Could you please add "DC" and "UID" ? Or maybe could we have special method to pass additional attributeTypeNames for custom converting x509name to the string.

What did you expect to see?

CN=TEST-DESK,DC=test-ca,DC=acm,DC=us

What did you see instead?

CN=TEST-DESK,0.9.2342.19200300.100.1.25=#1307746573742d6361,0.9.2342.19200300.100.1.25=#130361636d,0.9.2342.19200300.100.1.25=#13027573
dmitshur commented 4 years ago

CC @FiloSottile, @agl, @rsc per owners.

FiloSottile commented 4 years ago

This sounds good, we'd take a PR for it.

gopherbot commented 4 years ago

Change https://golang.org/cl/263177 mentions this issue: crypto/x509/pkix: add styled string conversion of PKIX names

karlovskiy commented 4 years ago

@FiloSottile i've added the new method StyledString so we can now use custom style (attributes map). Also, i've added DC and UID to be compliant with rfc2253 and T, DN and E in addition to existing SERIALNUMBER and POSTALCODE to extend common attributes.

FiloSottile commented 4 years ago

@FiloSottile i've added the new method StyledString so we can now use custom style (attributes map).

Adding an API would require a proposal, and since this is already implementing an obsolete RFC, I don't think we'll want to do that.

Also, i've added DC and UID to be compliant with rfc2253

Sounds good, thanks! It would be useful for review to have a reference in the CL or in the comments of where the OIDs come from.

and T, DN and E in addition to existing SERIALNUMBER and POSTALCODE to extend common attributes.

This might be ok, but is there a clear line to draw to explain why support these attributes and not more? I would like not to start maintaining an ever-growing registry. DC and UID had "because they are in RFC 2253".

karlovskiy commented 4 years ago

Adding an API would require a proposal, and since this is already implementing an obsolete RFC, I don't think we'll want to do that.

Sorry didn't know about that. I thought we need the new method because as you said "I would like not to start maintaining an ever-growing registry" and it's absolute OK, but i think it's a good idea if API give pass custom attributes map with using all power converting rdn sequence that general String method already has.

Also, i've added DC and UID to be compliant with rfc2253

They come from https://tools.ietf.org/html/rfc2253#section-2.3, currently we have 7 from 9 attributes from rfc2253 and two custom: SERIALNUMBER and POSTALCODE.

This might be ok, but is there a clear line to draw to explain why support these attributes and not more? I would like not to start maintaining an ever-growing registry.

My thoughts and concept of style (that's why i called with StyledString ) was about BC library https://github.com/bcgit/bc-java/blob/master/core/src/main/java/org/bouncycastle/asn1/x509/X509Name.java#L263-L294 Another one is jdk's table here https://github.com/frohoff/jdk8u-jdk/blob/master/src/share/classes/sun/security/x509/AVA.java#L1331

I think that we could add DC and UID in the current attributeTypeNames, to be complient with rfc 2253. In addition, could we in some way extend API and give users the possibility to iterate in reverse order (by rfc2253) with already implemented algorithm in String method of RDNSequence but with custom attributes map ?

We could use this new method like this if current String is not suitable:

myAttributes := map[string]string {
    ...
}
var name pkix.RDNSequence
asn1.Unmarshal(asn1Name, &name)
custom := name.StyledString(myAttributes)

What do you think ?

mbartosch commented 3 years ago

The current RFC 5280 section 4.1.2.4 states

Standard sets of attributes have been defined in the X.500 series of
specifications [X.520].  Implementations of this specification MUST
be prepared to receive the following standard attribute types in
issuer and subject (Section 4.1.2.6) names:

      * country,
      * organization,
      * organizational unit,
      * distinguished name qualifier,
      * state or province name,
      * common name (e.g., "Susan Housley"), and
      * serial number.

   In addition, implementations of this specification SHOULD be prepared
   to receive the following standard attribute types in issuer and
   subject names:

      * locality,
      * title,
      * surname,
      * given name,
      * initials,
      * pseudonym, and
      * generation qualifier (e.g., "Jr.", "3rd", or "IV").

It also explicitly demands that

In addition, implementations of this specification MUST be prepared
to receive the domainComponent attribute, as defined in [RFC4519].

In particular the domainComponent RDN is quite important for designing modern PKIs, so I suggest to include the listed attributes, including the DC in the attributeTypeNames map which will result in proper representation within String().

https://github.com/golang/go/pull/44536 was raised to address this. However, this does not include the UID RDN, as this is not an OID that is required by RFC 5280. It may thus be useful to additionally include "0.9.2342.19200300.100.1.1": "UID" // userid

gopherbot commented 3 years ago

Change https://golang.org/cl/295391 mentions this issue: crypto/x509/pkix/pkix: add missing RFC 5280 RDN OIDs

DieTime commented 1 year ago

@FiloSottile StyledString support would be a very handy API to not have to maintain a list of OIDs all the time. I encountered the same problem the other day, that the email address was unreadable. How likely is it that this proposal will be accepted?

twuyts commented 1 month ago

What is holding golang.org/cl/295391 back to getting merged? It's been open for over 3 years?