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

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   }
...

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]

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.

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]

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.