Merge pull request #2300 from libgit2/cmn/match-host-tests

Some improvements to the cert checking

src/netops.c

@@ -207,7 +207,7 @@ static int gitno_ssl_teardown(gitno_ssl *ssl)
 }
 
 /* Match host names according to RFC 2818 rules */
-static int match_host(const char *pattern, const char *host)
+int gitno__match_host(const char *pattern, const char *host)
 {
 	for (;;) {
 		char c = tolower(*pattern++);
@@ -230,9 +230,9 @@ static int match_host(const char *pattern, const char *host)
 			while(*host) {
 				char h = tolower(*host);
 				if (c == h)
-					return match_host(pattern, host++);
+					return gitno__match_host(pattern, host++);
 				if (h == '.')
-					return match_host(pattern, host);
+					return gitno__match_host(pattern, host);
 				host++;
 			}
 			return -1;
@@ -250,7 +250,7 @@ static int check_host_name(const char *name, const char *host)
 	if (!strcasecmp(name, host))
 		return 0;
 
-	if (match_host(name, host) < 0)
+	if (gitno__match_host(name, host) < 0)
 		return -1;
 
 	return 0;
@@ -287,6 +287,10 @@ static int verify_server_cert(gitno_ssl *ssl, const char *host)
 
 
 	cert = SSL_get_peer_certificate(ssl->ssl);
+	if (!cert) {
+		giterr_set(GITERR_SSL, "the server did not provide a certificate");
+		return -1;
+	}
 
 	/* Check the alternative names */
 	alts = X509_get_ext_d2i(cert, NID_subject_alt_name, NULL, NULL);
@@ -321,7 +325,7 @@ static int verify_server_cert(gitno_ssl *ssl, const char *host)
 	GENERAL_NAMES_free(alts);
 
 	if (matched == 0)
-		goto cert_fail;
+		goto cert_fail_name;
 
 	if (matched == 1)
 		return 0;
@@ -358,11 +362,11 @@ static int verify_server_cert(gitno_ssl *ssl, const char *host)
 		int size = ASN1_STRING_to_UTF8(&peer_cn, str);
 		GITERR_CHECK_ALLOC(peer_cn);
 		if (memchr(peer_cn, '\0', size))
-			goto cert_fail;
+			goto cert_fail_name;
 	}
 
 	if (check_host_name((char *)peer_cn, host) < 0)
-		goto cert_fail;
+		goto cert_fail_name;
 
 	OPENSSL_free(peer_cn);
 
@@ -372,9 +376,9 @@ on_error:
 	OPENSSL_free(peer_cn);
 	return ssl_set_error(ssl, 0);
 
-cert_fail:
+cert_fail_name:
 	OPENSSL_free(peer_cn);
-	giterr_set(GITERR_SSL, "Certificate host name check failed");
+	giterr_set(GITERR_SSL, "hostname does not match certificate");
 	return -1;
 }
 

src/netops.h

@@ -54,6 +54,19 @@ enum {
 	GITNO_CONNECT_SSL_NO_CHECK_CERT = 2,
 };
 
+/**
+ * Check if the name in a cert matches the wanted hostname
+ *
+ * Check if a pattern from a certificate matches the hostname we
+ * wanted to connect to according to RFC2818 rules (which specifies
+ * HTTP over TLS). Mainly, an asterisk matches anything, but is
+ * limited to a single url component.
+ *
+ * Note that this does not set an error message. It expects the user
+ * to provide the message for the user.
+ */
+int gitno__match_host(const char *pattern, const char *host);
+
 void gitno_buffer_setup(gitno_socket *t, gitno_buffer *buf, char *data, size_t len);
 void gitno_buffer_setup_callback(gitno_socket *t, gitno_buffer *buf, char *data, size_t len, int (*recv)(gitno_buffer *buf), void *cb_data);
 int gitno_recv(gitno_buffer *buf);

tests/network/matchhost.c

@@ -0,0 +1,13 @@
+#include "clar_libgit2.h"
+#include "netops.h"
+
+void test_network_matchhost__match(void)
+{
+	cl_git_pass(gitno__match_host("*.example.org", "www.example.org"));
+	cl_git_pass(gitno__match_host("*.foo.example.org", "www.foo.example.org"));
+	cl_git_fail(gitno__match_host("*.foo.example.org", "foo.example.org"));
+	cl_git_fail(gitno__match_host("*.foo.example.org", "www.example.org"));
+	cl_git_fail(gitno__match_host("*.example.org", "example.org"));
+	cl_git_fail(gitno__match_host("*.example.org", "www.foo.example.org"));
+	cl_git_fail(gitno__match_host("*.example.org", "blah.www.www.example.org"));
+}