Yeah, I was clearly mislead by that first quote and didn't do the full check. I think we should go for compatibility with how the browsers treat this and allow the IP match. Thanks @jay !

Ok. Another thing, if we are verifying an IP and there is at least one iPAddress field but no dNSName is it correct that if the match fails to default to CN like we are now? I haven't found much but this a python issue.

That's a very good question, but I would guess so. I also guess that as there are very few certs in use using iPAddress, there will be even less that actually put an IP address in the CN field so we might simply not hit this case in real life.

Firefox and Chrome don't default to CN when iPAddress is present but dNSName is not. To repro follow above but use this server.cnf:

[req]
distinguished_name = req_distinguished_name
req_extensions = v3_req
prompt = no

[req_distinguished_name]
CN = 127.0.0.1

[v3_req]
basicConstraints = CA:FALSE
keyUsage = digitalSignature, keyEncipherment
subjectAltName = @alt_names

[alt_names]
IP.1 = 8.8.8.8

Chrome shows This server could not prove that it is 127.0.0.1; its security certificate is from 127.0.0.1. NET::ERR_CERT_COMMON_NAME_INVALID

Firefox shows The certificate is only valid for 8.8.8.8 Error code: SSL_ERROR_BAD_CERT_DOMAIN

(Edit: Note the connection is also rejected under the same premises if I change the CN and URL from 127.0.0.1 to a hostname (eg CN foo and I connect to https://foo:4433/).)

So assuming that's correct the fix looks more like this:

diff --git a/lib/vtls/openssl.c b/lib/vtls/openssl.c
index 3027ca3..eedba0d 100644
--- a/lib/vtls/openssl.c
+++ b/lib/vtls/openssl.c
@@ -1083,6 +1083,7 @@ static CURLcode verifyhost(struct connectdata *conn, X509 *server_cert)
 #endif
   CURLcode result = CURLE_OK;
   bool dNSName = FALSE; /* if a dNSName field exists in the cert */
+  bool iPAddress = FALSE; /* if a iPAddress field exists in the cert */

 #ifdef ENABLE_IPV6
   if(conn->bits.ipv6_ip &&
@@ -1115,10 +1116,10 @@ static CURLcode verifyhost(struct connectdata *conn, X509 *server_cert)
       /* get a handle to alternative name number i */
       const GENERAL_NAME *check = sk_GENERAL_NAME_value(altnames, i);

-      /* If a subjectAltName extension of type dNSName is present, that MUST
-         be used as the identity. / RFC2818 section 3.1 */
       if(check->type == GEN_DNS)
         dNSName = TRUE;
+      else if(check->type == GEN_IPADD)
+        iPAddress = TRUE;

       /* only check alternatives of the same type the target is */
       if(check->type == target) {
@@ -1164,18 +1165,14 @@ static CURLcode verifyhost(struct connectdata *conn, X509 *server_cert)
     }
     GENERAL_NAMES_free(altnames);

-    if(dnsmatched || (!dNSName && ipmatched)) {
-      /* count as a match if the dnsname matched or if there was no dnsname
-         fields at all AND there was an IP field match */
+    if(dnsmatched || ipmatched)
       matched = TRUE;
-    }
   }

   if(matched)
     /* an alternative name matched */
     ;
-  else if(dNSName) {
-    /* an dNSName field existed, but didn't match and then we MUST fail */
+  else if(dNSName || (iPAddress && target == GEN_IPADD)) {
     infof(data, " subjectAltName does not match %s\n", conn->host.dispname);
     failf(data, "SSL: no alternative certificate subject name matches "
           "target host name '%s'", conn->host.dispname);

Great. And yes, going with mimicking the browsers' behavior seems like a good guidance here. 👍

I turned this into a PR with the final version. I made one more change that needs review, I have it fail now in the case of iPAddress present but no-match regardless of what type of target we have. I did some tests and that is consistent with the way Firefox and Chrome do it. If I set the CN to localhost but I have v3 iPAddress then it ignores the CN even if iPAddress doesn't match.

👍 excellent and yes, I think that update makes sense. LGTM as they say.

Landed in b6fcdc3

I have used curl and the regression seems to have been fixed; however, an original comment from databus23 in issue #875 is still not addressed. databus23 wrote "Newer versions of curl don't work with our certificates anymore which only contain the hostname in the CN." This is an issue for us as well - perfectly good server certificates with the server hostname in CN no longer work - in 7.50.2.

I would suggest that CN be used as a fallback match on hostname if dNSName is missing from subjectAltName. I do not see any security issues with this approach. Additionally, wget matches on hostname in CN.