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/pkix: Name.String() rfc2253 AttributeTypes #41750

Open
karlovskiy opened this issue Oct 2, 2020 · 9 comments
Open

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

karlovskiy opened this issue Oct 2, 2020 · 9 comments
Labels
help wanted NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@karlovskiy
Copy link

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 dmitshur added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Oct 13, 2020
@dmitshur dmitshur added this to the Backlog milestone Oct 13, 2020
@dmitshur
Copy link
Contributor

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

@FiloSottile FiloSottile added NeedsFix The path to resolution is known, but the work has not been done. help wanted and removed NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Oct 15, 2020
@FiloSottile
Copy link
Contributor

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

@gopherbot
Copy link

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

@karlovskiy
Copy link
Author

@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
Copy link
Contributor

@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
Copy link
Author

karlovskiy commented Oct 17, 2020

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
Copy link

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().

#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
Copy link

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

@DieTime
Copy link

DieTime commented Dec 23, 2022

@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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants