-
Notifications
You must be signed in to change notification settings - Fork 17.9k
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() does not preserve correct order #52462
Comments
@rolandshoemaker per owners |
See #40876 for a previous discussion of these issues. |
I think you need to update the documentation to better describe the behavior of the library. Saying "roughly following the RFC 2253 Distinguished Names syntax" is far too vague and, frankly, quite a stretch. As you see, there are quite many tickets related to this (I checked all open ones before filing this one, but did not see #40876 since it is closed), and this is most likely due to the misleading documentation. The documentation should contain a warning that if you want RFC 2253 (or RFC 4515, which has replaced RFC2253) syntax, then you are better off writing your own library. The way it goes now is that the Go user trusts that Go library works according to the standards (despite the "roughly"). Then you start noticing quite many things are off. Then you start debugging, and eventually spend time submitting a bug report. This then also unnecessarily wastes the Go maintainers time. If the documentation was more accurate a lot of time would be saved. I repeat myself, It really should contain a warning. Regarding not wanting to fix this in the fear of breaking implementations relying on the wrong output, I see that the output has been changed before in #39924. Please also respond in #33093 that you are not going to fix the bug (instead of insisting on that current behavior is correct) at the same time when the documentation is updated. Is there a feature request to make a new RFC 2253/RFC 4514 compatible Go library in the future? |
In your defence, though, the pkix.Name documentation says:
which is good, but that also could contain further detail on what that approximation is. It's when you read the Name.String() documentation that you get distracted by the "roughly RFC 2253 syntax". It should say something along the lines of:
I did not yet fully verify that the above description is accurate, but it should be something along those lines. |
What version of Go are you using (
go version
)?Does this issue reproduce with the latest release?
Yes, produces with go version go1.18.1
What operating system and processor architecture are you using (
go env
)?go env
OutputWhat did you do?
Create three ceritificates with openssl:
Read in and print the subject name of each certificates with the following program:
What did you expect to see?
cert1.crt:
CN=foo,O=bar,C=FI
cert2.crt:
O=bar,CN=foo,C=FI
cert3.crt:
CN=foo+0.9.2342.19200300.100.1.1=#130431323334,O=bar,C=FI
(Or actually for cert3.crt:
CN=foo+0.9.2342.19200300.100.1.1=1234,O=bar,C=FI
, but that is another issue, see #33093.)What did you see instead?
cert1.crt:
CN=foo,O=bar,C=FI
(as expected)cert2.crt:
CN=foo,O=bar,C=FI
(order is not maintained)cert3.crt :
CN=foo,O=bar,C=FI,0.9.2342.19200300.100.1.1=#130431323334
(order is not mainainted, multi value RDN lost)To be fair, the documentation for pkix.Name.String() says:
But I think the "roughly" is a little too liberally interpretted.
RFC2253 2.1 says:
So the ordering should be mainainted.
Looking at #39924 I suspect there is a misinterpretation where @FiloSottile says:
However, the quoted text is related to the individual RDN parts, so both the following would be correct for cert3.crt:
But not:
Further and for clarity, looking at the ASN.1 structure of the Subject DN of cert3.crt:
You can see that each RDN is in a sequence (order should be maintained) but the multivalue RDN is a set (order does not matter. you can pick any order). The same goes for cert2.crt where the String() method throws the RDNs around in a fixed orderd and places unrecognized RDNs last.
In addition to this, it looks the String() method prints the sequence in reverse to what RFC 2253 says "starting with the last element of the sequence and moving backwards toward the first", which would suggest cert1.crt should actually print as (contrary to what I said in "What did you expect to see?"):
countryName is the last element and commonName is the first. RFC 2253 shows examples where the countryName is last, but it does not show the corresponding ASN.1 structure, so that cannot be used as a reference. The ASN.1 sequence in RFC 2253 is probably in reverse to what this example shows.
The text was updated successfully, but these errors were encountered: