-
Notifications
You must be signed in to change notification settings - Fork 2k
acme/autocert: make host of TLS certificate to be obtained always Punycode #85
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
Conversation
This PR (HEAD: 7b1b4fe) has been imported to Gerrit for code review. Please visit https://go-review.googlesource.com/c/crypto/+/171997 to see it. Tip: You can toggle comments from me using the |
Message from Gobot Gobot: Patch Set 1: Congratulations on opening your first change. Thank you for your contribution! Next steps: Most changes in the Go project go through a few rounds of revision. This can be During May-July and Nov-Jan the Go project is in a code freeze, during which Please don’t reply on this GitHub thread. Visit golang.org/cl/171997. |
Message from Brad Fitzpatrick: Patch Set 1: Run-TryBot+1 Could you add a test? What's the ClientHello.ServerName for an IDN/punycode domain name? Is it Unicode here or punycode? Please don’t reply on this GitHub thread. Visit golang.org/cl/171997. |
Message from Gobot Gobot: Patch Set 1: TryBots beginning. Status page: https://farmer.golang.org/try?commit=7687650f Please don’t reply on this GitHub thread. Visit golang.org/cl/171997. |
Message from Gobot Gobot: Patch Set 1: TryBot-Result+1 TryBots are happy. Please don’t reply on this GitHub thread. Visit golang.org/cl/171997. |
This PR (HEAD: a22ba07) has been imported to Gerrit for code review. Please visit https://go-review.googlesource.com/c/crypto/+/171997 to see it. Tip: You can toggle comments from me using the |
Message from Aofei Sheng: Patch Set 2:
Hi Brad, I think the Please don’t reply on this GitHub thread. Visit golang.org/cl/171997. |
Message from Aofei Sheng: Patch Set 2:
Sorry, I didn't test enough, I just sent the request via cURL (it's Unicode here). When I sent the request through the browser (Safari), I got a Punycode And BTW, I'm wondering, should we do something special for this kind of bare request (no implicit conversion to host) from cURL? Maybe an explicit conversion? Because if I send a request with an IDN via cURL now, I will get the "Invalid character in DNS name" error. Please don’t reply on this GitHub thread. Visit golang.org/cl/171997. |
Message from Aofei Sheng: Patch Set 2:
It can be like this: name, err := idna.ToASCII(strings.ToLower(hello.ServerName))
if err != nil {
return nil, err
} If the Please don’t reply on this GitHub thread. Visit golang.org/cl/171997. |
Message from Aofei Sheng: Patch Set 2:
Or this: name, err := idna.Lookup.ToASCII(hello.ServerName) // It also handles mixedcase hosts well
if err != nil {
return nil, err
} I also found the "σςΣ" problem described in here. So if we want to use the However, it seems that the performance of the Seems that there is a time difference between us. Anyway, I'll stay up late because of some other things tonight, so I'm waiting for your reply. :-) Please don’t reply on this GitHub thread. Visit golang.org/cl/171997. |
This PR (HEAD: 4e9f76b) has been imported to Gerrit for code review. Please visit https://go-review.googlesource.com/c/crypto/+/171997 to see it. Tip: You can toggle comments from me using the |
Message from Aofei Sheng: Patch Set 3: I just pushed the new code so that you can understand my ideas more easily. :-) Please don’t reply on this GitHub thread. Visit golang.org/cl/171997. |
This PR (HEAD: 6e606d9) has been imported to Gerrit for code review. Please visit https://go-review.googlesource.com/c/crypto/+/171997 to see it. Tip: You can toggle comments from me using the |
This PR (HEAD: 381032f) has been imported to Gerrit for code review. Please visit https://go-review.googlesource.com/c/crypto/+/171997 to see it. Tip: You can toggle comments from me using the |
Message from Aofei Sheng: Patch Set 7: Any ideas? Please don’t reply on this GitHub thread. Visit golang.org/cl/171997. |
Message from Brad Fitzpatrick: Patch Set 7: Run-TryBot+1 (1 comment) Please don’t reply on this GitHub thread. Visit golang.org/cl/171997. |
Message from Gobot Gobot: Patch Set 7: TryBots beginning. Status page: https://farmer.golang.org/try?commit=6b486319 Please don’t reply on this GitHub thread. Visit golang.org/cl/171997. |
Message from Gobot Gobot: Patch Set 7: TryBot-Result+1 TryBots are happy. Please don’t reply on this GitHub thread. Visit golang.org/cl/171997. |
This PR (HEAD: 6c12694) has been imported to Gerrit for code review. Please visit https://go-review.googlesource.com/c/crypto/+/171997 to see it. Tip: You can toggle comments from me using the |
Message from Aofei Sheng: Patch Set 8:
Typo has been corrected. Please don’t reply on this GitHub thread. Visit golang.org/cl/171997. |
Message from Aofei Sheng: Patch Set 8: (1 comment) Please don’t reply on this GitHub thread. Visit golang.org/cl/171997. |
…ycode According to the [RFC 5280, section 4.2.1.6](https://tools.ietf.org/html/rfc5280#section-4.2.1.6): > ... > When the subjectAltName extension contains a domain name system > label, the domain name MUST be stored in the dNSName (an IA5String). > The name MUST be in the "preferred name syntax", as specified by > Section 3.5 of [RFC1034] and as modified by Section 2.1 of > [RFC1123]. Note that while uppercase and lowercase letters are > allowed in domain names, no significance is attached to the case. > ... And the [RFC 1034, section 3.5](https://tools.ietf.org/html/rfc1034#section-3.5): > ... > Note that while upper and lower case letters are allowed in domain > names, no significance is attached to the case. That is, two names with > the same spelling but different case are to be treated as if identical. > ... We should return the same TLS certificate for both `example.com` and `EXAMPLE.COM`. But the `autocert.Manager.GetCertificate` treats the two as totally different, it signs and returns two different TLS certificates. Moreover, now the `autocert.Manager.GetCertificate` and the `autocert.HostWhitelist` can only handle Punycode IDN. If the client sends a Unicode IDN to `autocert.Manager.GetCertificate` (cURL is doing this), the "Invalid character in DNS name" error will be triggered. This PR corrects these problems by converting the host of the TLS certificate to be obtained to Punycode via `idna.Lookup.ToASCII`. Change-Id: I993821b3a6ae532a53772e2db00524479ef111af GitHub-Last-Rev: 6c12694 GitHub-Pull-Request: #85 Reviewed-on: https://go-review.googlesource.com/c/crypto/+/171997 Reviewed-by: Brad Fitzpatrick <[email protected]>
This PR is being closed because golang.org/cl/171997 has been merged. |
…ycode According to the [RFC 5280, section 4.2.1.6](https://tools.ietf.org/html/rfc5280#section-4.2.1.6): > ... > When the subjectAltName extension contains a domain name system > label, the domain name MUST be stored in the dNSName (an IA5String). > The name MUST be in the "preferred name syntax", as specified by > Section 3.5 of [RFC1034] and as modified by Section 2.1 of > [RFC1123]. Note that while uppercase and lowercase letters are > allowed in domain names, no significance is attached to the case. > ... And the [RFC 1034, section 3.5](https://tools.ietf.org/html/rfc1034#section-3.5): > ... > Note that while upper and lower case letters are allowed in domain > names, no significance is attached to the case. That is, two names with > the same spelling but different case are to be treated as if identical. > ... We should return the same TLS certificate for both `example.com` and `EXAMPLE.COM`. But the `autocert.Manager.GetCertificate` treats the two as totally different, it signs and returns two different TLS certificates. Moreover, now the `autocert.Manager.GetCertificate` and the `autocert.HostWhitelist` can only handle Punycode IDN. If the client sends a Unicode IDN to `autocert.Manager.GetCertificate` (cURL is doing this), the "Invalid character in DNS name" error will be triggered. This PR corrects these problems by converting the host of the TLS certificate to be obtained to Punycode via `idna.Lookup.ToASCII`. Change-Id: I993821b3a6ae532a53772e2db00524479ef111af GitHub-Last-Rev: 6c12694574b6ac71a920da8a8bdb130c0b6e0272 GitHub-Pull-Request: golang/crypto#85 Reviewed-on: https://go-review.googlesource.com/c/crypto/+/171997 Reviewed-by: Brad Fitzpatrick <[email protected]>
…ycode According to the [RFC 5280, section 4.2.1.6](https://tools.ietf.org/html/rfc5280#section-4.2.1.6): > ... > When the subjectAltName extension contains a domain name system > label, the domain name MUST be stored in the dNSName (an IA5String). > The name MUST be in the "preferred name syntax", as specified by > Section 3.5 of [RFC1034] and as modified by Section 2.1 of > [RFC1123]. Note that while uppercase and lowercase letters are > allowed in domain names, no significance is attached to the case. > ... And the [RFC 1034, section 3.5](https://tools.ietf.org/html/rfc1034#section-3.5): > ... > Note that while upper and lower case letters are allowed in domain > names, no significance is attached to the case. That is, two names with > the same spelling but different case are to be treated as if identical. > ... We should return the same TLS certificate for both `example.com` and `EXAMPLE.COM`. But the `autocert.Manager.GetCertificate` treats the two as totally different, it signs and returns two different TLS certificates. Moreover, now the `autocert.Manager.GetCertificate` and the `autocert.HostWhitelist` can only handle Punycode IDN. If the client sends a Unicode IDN to `autocert.Manager.GetCertificate` (cURL is doing this), the "Invalid character in DNS name" error will be triggered. This PR corrects these problems by converting the host of the TLS certificate to be obtained to Punycode via `idna.Lookup.ToASCII`. Change-Id: I993821b3a6ae532a53772e2db00524479ef111af GitHub-Last-Rev: 6c12694574b6ac71a920da8a8bdb130c0b6e0272 GitHub-Pull-Request: golang/crypto#85 Reviewed-on: https://go-review.googlesource.com/c/crypto/+/171997 Reviewed-by: Brad Fitzpatrick <[email protected]>
…ycode According to the [RFC 5280, section 4.2.1.6](https://tools.ietf.org/html/rfc5280#section-4.2.1.6): > ... > When the subjectAltName extension contains a domain name system > label, the domain name MUST be stored in the dNSName (an IA5String). > The name MUST be in the "preferred name syntax", as specified by > Section 3.5 of [RFC1034] and as modified by Section 2.1 of > [RFC1123]. Note that while uppercase and lowercase letters are > allowed in domain names, no significance is attached to the case. > ... And the [RFC 1034, section 3.5](https://tools.ietf.org/html/rfc1034#section-3.5): > ... > Note that while upper and lower case letters are allowed in domain > names, no significance is attached to the case. That is, two names with > the same spelling but different case are to be treated as if identical. > ... We should return the same TLS certificate for both `example.com` and `EXAMPLE.COM`. But the `autocert.Manager.GetCertificate` treats the two as totally different, it signs and returns two different TLS certificates. Moreover, now the `autocert.Manager.GetCertificate` and the `autocert.HostWhitelist` can only handle Punycode IDN. If the client sends a Unicode IDN to `autocert.Manager.GetCertificate` (cURL is doing this), the "Invalid character in DNS name" error will be triggered. This PR corrects these problems by converting the host of the TLS certificate to be obtained to Punycode via `idna.Lookup.ToASCII`. Change-Id: I993821b3a6ae532a53772e2db00524479ef111af GitHub-Last-Rev: 6c12694574b6ac71a920da8a8bdb130c0b6e0272 GitHub-Pull-Request: golang/crypto#85 Reviewed-on: https://go-review.googlesource.com/c/crypto/+/171997 Reviewed-by: Brad Fitzpatrick <[email protected]>
…ycode According to the [RFC 5280, section 4.2.1.6](https://tools.ietf.org/html/rfc5280#section-4.2.1.6): > ... > When the subjectAltName extension contains a domain name system > label, the domain name MUST be stored in the dNSName (an IA5String). > The name MUST be in the "preferred name syntax", as specified by > Section 3.5 of [RFC1034] and as modified by Section 2.1 of > [RFC1123]. Note that while uppercase and lowercase letters are > allowed in domain names, no significance is attached to the case. > ... And the [RFC 1034, section 3.5](https://tools.ietf.org/html/rfc1034#section-3.5): > ... > Note that while upper and lower case letters are allowed in domain > names, no significance is attached to the case. That is, two names with > the same spelling but different case are to be treated as if identical. > ... We should return the same TLS certificate for both `example.com` and `EXAMPLE.COM`. But the `autocert.Manager.GetCertificate` treats the two as totally different, it signs and returns two different TLS certificates. Moreover, now the `autocert.Manager.GetCertificate` and the `autocert.HostWhitelist` can only handle Punycode IDN. If the client sends a Unicode IDN to `autocert.Manager.GetCertificate` (cURL is doing this), the "Invalid character in DNS name" error will be triggered. This PR corrects these problems by converting the host of the TLS certificate to be obtained to Punycode via `idna.Lookup.ToASCII`. Change-Id: I993821b3a6ae532a53772e2db00524479ef111af GitHub-Last-Rev: 6c12694574b6ac71a920da8a8bdb130c0b6e0272 GitHub-Pull-Request: golang/crypto#85 Reviewed-on: https://go-review.googlesource.com/c/crypto/+/171997 Reviewed-by: Brad Fitzpatrick <[email protected]>
…ycode According to the [RFC 5280, section 4.2.1.6](https://tools.ietf.org/html/rfc5280#section-4.2.1.6): > ... > When the subjectAltName extension contains a domain name system > label, the domain name MUST be stored in the dNSName (an IA5String). > The name MUST be in the "preferred name syntax", as specified by > Section 3.5 of [RFC1034] and as modified by Section 2.1 of > [RFC1123]. Note that while uppercase and lowercase letters are > allowed in domain names, no significance is attached to the case. > ... And the [RFC 1034, section 3.5](https://tools.ietf.org/html/rfc1034#section-3.5): > ... > Note that while upper and lower case letters are allowed in domain > names, no significance is attached to the case. That is, two names with > the same spelling but different case are to be treated as if identical. > ... We should return the same TLS certificate for both `example.com` and `EXAMPLE.COM`. But the `autocert.Manager.GetCertificate` treats the two as totally different, it signs and returns two different TLS certificates. Moreover, now the `autocert.Manager.GetCertificate` and the `autocert.HostWhitelist` can only handle Punycode IDN. If the client sends a Unicode IDN to `autocert.Manager.GetCertificate` (cURL is doing this), the "Invalid character in DNS name" error will be triggered. This PR corrects these problems by converting the host of the TLS certificate to be obtained to Punycode via `idna.Lookup.ToASCII`. Change-Id: I993821b3a6ae532a53772e2db00524479ef111af GitHub-Last-Rev: 6c12694 GitHub-Pull-Request: golang#85 Reviewed-on: https://go-review.googlesource.com/c/crypto/+/171997 Reviewed-by: Brad Fitzpatrick <[email protected]>
…ycode According to the [RFC 5280, section 4.2.1.6](https://tools.ietf.org/html/rfc5280#section-4.2.1.6): > ... > When the subjectAltName extension contains a domain name system > label, the domain name MUST be stored in the dNSName (an IA5String). > The name MUST be in the "preferred name syntax", as specified by > Section 3.5 of [RFC1034] and as modified by Section 2.1 of > [RFC1123]. Note that while uppercase and lowercase letters are > allowed in domain names, no significance is attached to the case. > ... And the [RFC 1034, section 3.5](https://tools.ietf.org/html/rfc1034#section-3.5): > ... > Note that while upper and lower case letters are allowed in domain > names, no significance is attached to the case. That is, two names with > the same spelling but different case are to be treated as if identical. > ... We should return the same TLS certificate for both `example.com` and `EXAMPLE.COM`. But the `autocert.Manager.GetCertificate` treats the two as totally different, it signs and returns two different TLS certificates. Moreover, now the `autocert.Manager.GetCertificate` and the `autocert.HostWhitelist` can only handle Punycode IDN. If the client sends a Unicode IDN to `autocert.Manager.GetCertificate` (cURL is doing this), the "Invalid character in DNS name" error will be triggered. This PR corrects these problems by converting the host of the TLS certificate to be obtained to Punycode via `idna.Lookup.ToASCII`. Change-Id: I993821b3a6ae532a53772e2db00524479ef111af GitHub-Last-Rev: 6c12694574b6ac71a920da8a8bdb130c0b6e0272 GitHub-Pull-Request: golang/crypto#85 Reviewed-on: https://go-review.googlesource.com/c/crypto/+/171997 Reviewed-by: Brad Fitzpatrick <[email protected]>
According to the RFC 5280, section 4.2.1.6:
And the RFC 1034, section 3.5:
We should return the same TLS certificate for both
example.com
andEXAMPLE.COM
. But theautocert.Manager.GetCertificate
treats the two as totally different, it signs and returns two different TLS certificates.Moreover, now the
autocert.Manager.GetCertificate
and theautocert.HostWhitelist
can only handle Punycode IDN. If the client sends a Unicode IDN toautocert.Manager.GetCertificate
(cURL is doing this), the "Invalid character in DNS name" error will be triggered.This PR corrects these problems by converting the host of the TLS certificate to be obtained to Punycode via
idna.Lookup.ToASCII
.