Browse Source

ssl-verify-host: fix verifying ip addresses containing zero's (#732)

* ssl-verify-host: fix verifying ip addresses containing zero's

If the subject alternate name contained an ip address with an zero
(like 10.42.0.1) it could not successfully verify.
It is because in c++ strings are null-terminated
and therefore strlen(name) would return a wrong result.
As I can not see why we can not trust the length returned by openssl,
lets drop this check.

* ssl-verify-host: add test case

lets try to validate against 127.0.0.1

Co-authored-by: Daniel Ottiger <[email protected]>
Daniel Ottiger 5 years ago
parent
commit
6e1879dfae
4 changed files with 38 additions and 10 deletions
  1. 8 10
      httplib.h
  2. 1 0
      test/Makefile
  3. 26 0
      test/test.cc
  4. 3 0
      test/test.conf

+ 8 - 10
httplib.h

@@ -6225,17 +6225,15 @@ SSLClient::verify_host_with_subject_alt_name(X509 *server_cert) const {
         auto name = (const char *)ASN1_STRING_get0_data(val->d.ia5);
         auto name = (const char *)ASN1_STRING_get0_data(val->d.ia5);
         auto name_len = (size_t)ASN1_STRING_length(val->d.ia5);
         auto name_len = (size_t)ASN1_STRING_length(val->d.ia5);
 
 
-        if (strlen(name) == name_len) {
-          switch (type) {
-          case GEN_DNS: dsn_matched = check_host_name(name, name_len); break;
-
-          case GEN_IPADD:
-            if (!memcmp(&addr6, name, addr_len) ||
-                !memcmp(&addr, name, addr_len)) {
-              ip_mached = true;
-            }
-            break;
+        switch (type) {
+        case GEN_DNS: dsn_matched = check_host_name(name, name_len); break;
+
+        case GEN_IPADD:
+          if (!memcmp(&addr6, name, addr_len) ||
+              !memcmp(&addr, name, addr_len)) {
+            ip_mached = true;
           }
           }
+          break;
         }
         }
       }
       }
     }
     }

+ 1 - 0
test/Makefile

@@ -24,6 +24,7 @@ test_proxy : test_proxy.cc ../httplib.h Makefile cert.pem
 cert.pem:
 cert.pem:
 	openssl genrsa 2048 > key.pem
 	openssl genrsa 2048 > key.pem
 	openssl req -new -batch -config test.conf -key key.pem | openssl x509 -days 3650 -req -signkey key.pem > cert.pem
 	openssl req -new -batch -config test.conf -key key.pem | openssl x509 -days 3650 -req -signkey key.pem > cert.pem
+	openssl req -x509 -config test.conf -key key.pem -sha256 -days 3650 -nodes -out cert2.pem -extensions SAN
 	openssl genrsa 2048 > rootCA.key.pem
 	openssl genrsa 2048 > rootCA.key.pem
 	openssl req -x509 -new -batch -config test.rootCA.conf -key rootCA.key.pem -days 1024 > rootCA.cert.pem
 	openssl req -x509 -new -batch -config test.rootCA.conf -key rootCA.key.pem -days 1024 > rootCA.cert.pem
 	openssl genrsa 2048 > client.key.pem
 	openssl genrsa 2048 > client.key.pem

+ 26 - 0
test/test.cc

@@ -7,6 +7,7 @@
 #include <thread>
 #include <thread>
 
 
 #define SERVER_CERT_FILE "./cert.pem"
 #define SERVER_CERT_FILE "./cert.pem"
+#define SERVER_CERT2_FILE "./cert2.pem"
 #define SERVER_PRIVATE_KEY_FILE "./key.pem"
 #define SERVER_PRIVATE_KEY_FILE "./key.pem"
 #define CA_CERT_FILE "./ca-bundle.crt"
 #define CA_CERT_FILE "./ca-bundle.crt"
 #define CLIENT_CA_CERT_FILE "./rootCA.cert.pem"
 #define CLIENT_CA_CERT_FILE "./rootCA.cert.pem"
@@ -3245,6 +3246,31 @@ TEST(SSLClientTest, ServerCertificateVerification3) {
   ASSERT_EQ(301, res->status);
   ASSERT_EQ(301, res->status);
 }
 }
 
 
+TEST(SSLClientTest, ServerCertificateVerification4) {
+  SSLServer svr(SERVER_CERT2_FILE, SERVER_PRIVATE_KEY_FILE);
+  ASSERT_TRUE(svr.is_valid());
+
+  svr.Get("/test", [&](const Request &req, Response &res) {
+    res.set_content("test", "text/plain");
+    svr.stop();
+    ASSERT_TRUE(true);
+  });
+
+  thread t = thread([&]() { ASSERT_TRUE(svr.listen("127.0.0.1", PORT)); });
+  std::this_thread::sleep_for(std::chrono::milliseconds(1));
+
+  SSLClient cli("127.0.0.1", PORT);
+  cli.set_ca_cert_path(SERVER_CERT2_FILE);
+  cli.enable_server_certificate_verification(true);
+  cli.set_connection_timeout(30);
+
+  auto res = cli.Get("/test");
+  ASSERT_TRUE(res);
+  ASSERT_EQ(200, res->status);
+
+  t.join();
+}
+
 TEST(SSLClientTest, WildcardHostNameMatch) {
 TEST(SSLClientTest, WildcardHostNameMatch) {
   SSLClient cli("www.youtube.com");
   SSLClient cli("www.youtube.com");
 
 

+ 3 - 0
test/test.conf

@@ -16,3 +16,6 @@ emailAddress           = [email protected]
 
 
 [req_attributes]
 [req_attributes]
 challengePassword              = 1234
 challengePassword              = 1234
+
+[SAN]
+subjectAltName=IP:127.0.0.1