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

Jump to bottom

crypto/x509: Verify failed on some valid certificates with SubjectAlternateNames #24293

Closed
ghost opened this issue Mar 7, 2018 · 4 comments
Closed

crypto/x509: Verify failed on some valid certificates with SubjectAlternateNames #24293

ghost opened this issue Mar 7, 2018 · 4 comments
Labels
FrozenDueToAge

Comments

@ghost
Copy link

ghost commented Mar 7, 2018

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

Using Docker 17.12. compiled with golang 1.9.4.

Does this issue reproduce with the latest release?

The causative lines of code are still present in the master branch. So i think yes.

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

x86 Centos 7 (as VM in VirtualBox on Windows 7)

What did you do?

I want to connect to a docker registry which has an server certificate. The certificate should be validated.

Our certificate has the following attributes:

Subject:
    CommonName: our.company.registry
SubjectAlternateNamens:
    IP-Address: 1.2.3.4
    IP-Address: 1.2.3.5

I don't know if this is a valid combination, but every other tool says this certificate is valid.
According to http://www.alvestrand.no/objectid/2.5.29.17.html SubjectAlternateNamens can have different types of items (e.g. IP-Addresses and DNS-Names)

What did you expect to see?

golang should verify this certificate as valid.

What did you see instead?

golang throws an error: "x509: certificate is not valid for any names, but wanted to match our.company.registry"

What is the cause?

Checking the hostname against the certificate causes the Problem:
(https://github.com/golang/go/blob/release-branch.go1.9/src/crypto/x509/verify.go)

...
90   if c.hasSANExtension() {
91       valid = strings.Join(c.DNSNames, ", ")
92   } else {
93       valid = c.Subject.CommonName
94   }
...
97   if len(valid) == 0 {
98       return "x509: certificate is not valid for any names, but wanted to match " + h.Host
99   }
...

Line 90 checks for presence of SubjectAlternateNamens extension (which is present). Line 91 will join all DNSNames, even if they are not present. So valid will be empty and the error will be thrown.

These lines of code have actually changed between golang 1.8 and golang 1.9. In golang 1.8, the following was executed:
(https://github.com/golang/go/blob/release-branch.go1.8/src/crypto/x509/verify.go)

...
90   if len(c.DNSNames) > 0 {
91       valid = strings.Join(c.DNSNames, ", ")
92   } else {
93       valid = c.Subject.CommonName
94   }
...
97   if len(valid) == 0 {
98       return "x509: certificate is not valid for any names, but wanted to match " + h.Host
99   }
...

Here, the if clause does not match, and the c.Subject.CommonName is used. (and is valid).

So we dont have problems using docker 17.09, wich was compiled against go1.8.3.

What could be the solution?

I can not imagine why the if clause has changed in that way. But in my opinion, before using c.DNSNames it should be checked if they are present.

...
90   if c.hasSANExtension() && len(c.DNSNames) > 0 {
91       valid = strings.Join(c.DNSNames, ", ")
92   } else {
93       valid = c.Subject.CommonName
94   }
...
@ghost
Copy link
Author

ghost commented Mar 7, 2018 •
edited by ghost

Found the corresponding commit:
630e93e

@agl your comment is:

The code previously tested only whether DNS-name SANs were present in a certificate which is only approximately correct. In fact, /any/ SAN extension, including one with no DNS names, should cause the CN to be ignored.

Why should CN be ignored, even if no DNS names where present?

As our certificate is validated correctly by most other tool, it is not clear to me which approach is correct.

@dmitris
Copy link
Contributor

dmitris commented Mar 7, 2018

I don't have a definitive RFC reference at hand, but https://www.digicert.com/subject-alternative-name-compatibility.htm says: "If a SSL Certificate has a Subject Alternative Name (SAN) field, then SSL clients are supposed to ignore the Common Name value and seek a match in the SAN list. This is why DigiCert always repeats the common name as the first SAN in our certificates."

I also find among StackExchange search hits the following -
https://security.stackexchange.com/a/175790:

Support for CN was deprecated for a long time (at least 17 years, see RFC 2818) and Chrome browser will not even look at the CN anymore so today you need to have the domain of the URL as a subject alternative name.

See also https://tools.ietf.org/html/rfc6125#section-6.4.4 "Checking of Common Names":

As noted, a client MUST NOT seek a match for a reference identifier
of CN-ID if the presented identifiers include a DNS-ID, SRV-ID,
URI-ID, or any application-specific identifier types supported by the
client.

Therefore, if and only if the presented identifiers do not include a
DNS-ID, SRV-ID, URI-ID, or any application-specific identifier types
supported by the client, then the client MAY as a last resort check
for a string whose form matches that of a fully qualified DNS domain
name in a Common Name field of the subject field (i.e., a CN-ID). If
the client chooses to compare a reference identifier of type CN-ID
against that string, it MUST follow the comparison rules for the DNS
domain name portion of an identifier of type DNS-ID, SRV-ID, or
URI-ID, as described under Section 6.4.1, Section 6.4.2, and
Section 6.4.3.

@ghost
Copy link
Author

ghost commented Mar 7, 2018

Hmm ok. Seems legit.

I didn't know that SubjectAlternateNames superseeds the CommonName.

Now i understand why our certificate in fact is valid, but will never match the real hostname.

Thanks!

Please close this issue!

@odeke-em
Copy link
Member

odeke-em commented Mar 7, 2018

Thank you @gibma for the question and @dmitris for the response, I'll close this issue as you've requested and as per @dmitris' response.

@odeke-em odeke-em closed this as completed Mar 7, 2018
@odeke-em odeke-em changed the title x509: Verify failed on some valid certificates with SubjectAlternateNames crypto/x509: Verify failed on some valid certificates with SubjectAlternateNames Mar 7, 2018
@diptadas diptadas mentioned this issue Jun 7, 2018
Closed
@Grejeru Grejeru mentioned this issue Jun 14, 2018
Closed
@golang golang locked and limited conversation to collaborators Mar 7, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@dmitris @odeke-em @gopherbot