Merge pull request #2464 from libgit2/cmn/host-cert-info

Provide a callback for certificate validation

9 files changed, 225 insertions, 77 deletions

diff --git a/src/netops.c b/src/netops.c
index fceb4fb74..43b8c5311 100644
--- a/src/netops.c
+++ b/src/netops.c
@@ -384,10 +384,10 @@ on_error:
 cert_fail_name:
 	OPENSSL_free(peer_cn);
 	giterr_set(GITERR_SSL, "hostname does not match certificate");
-	return -1;
+	return GIT_ECERTIFICATE;
 }
 
-static int ssl_setup(gitno_socket *socket, const char *host, int flags)
+static int ssl_setup(gitno_socket *socket, const char *host)
 {
 	int ret;
 
@@ -406,9 +406,6 @@ static int ssl_setup(gitno_socket *socket, const char *host, int flags)
 	if ((ret = SSL_connect(socket->ssl.ssl)) <= 0)
 		return ssl_set_error(&socket->ssl, ret);
 
-	if (GITNO_CONNECT_SSL_NO_CHECK_CERT & flags)
-		return 0;
-
 	return verify_server_cert(&socket->ssl, host);
 }
 #endif
@@ -494,8 +491,9 @@ int gitno_connect(gitno_socket *s_out, const char *host, const char *port, int f
 	p_freeaddrinfo(info);
 
 #ifdef GIT_SSL
-	if ((flags & GITNO_CONNECT_SSL) && ssl_setup(s_out, host, flags) < 0)
-		return -1;
+	if ((flags & GITNO_CONNECT_SSL) &&
+	    (ret = ssl_setup(s_out, host)) < 0)
+		return ret;
 #else
 	/* SSL is not supported */
 	if (flags & GITNO_CONNECT_SSL) {
diff --git a/src/netops.h b/src/netops.h
index dfb4ab7b4..beb0e0760 100644
--- a/src/netops.h
+++ b/src/netops.h
@@ -47,10 +47,6 @@ typedef struct gitno_buffer gitno_buffer;
 enum {
 	/* Attempt to create an SSL connection. */
 	GITNO_CONNECT_SSL = 1,
-
-	/* Valid only when GITNO_CONNECT_SSL is also specified.
-	 * Indicates that the server certificate should not be validated. */
-	GITNO_CONNECT_SSL_NO_CHECK_CERT = 2,
 };
 
 /**
diff --git a/src/remote.c b/src/remote.c
index b0b5ff9d8..dfad946d5 100644
--- a/src/remote.c
+++ b/src/remote.c
@@ -80,6 +80,8 @@ static int ensure_remote_name_is_valid(const char *name)
 	return error;
 }
 
+#if 0
+/* We could export this as a helper */
 static int get_check_cert(int *out, git_repository *repo)
 {
 	git_config *cfg;
@@ -105,6 +107,7 @@ static int get_check_cert(int *out, git_repository *repo)
 	*out = git_config__get_bool_force(cfg, "http.sslverify", 1);
 	return 0;
 }
+#endif
 
 static int create_internal(git_remote **out, git_repository *repo, const char *name, const char *url, const char *fetch)
 {
@@ -121,9 +124,6 @@ static int create_internal(git_remote **out, git_repository *repo, const char *n
 	remote->repo = repo;
 	remote->update_fetchhead = 1;
 
-	if (get_check_cert(&remote->check_cert, repo) < 0)
-		goto on_error;
-
 	if (git_vector_init(&remote->refs, 32, NULL) < 0)
 		goto on_error;
 
@@ -274,7 +274,6 @@ int git_remote_dup(git_remote **dest, git_remote *source)
 	remote->transport_cb_payload = source->transport_cb_payload;
 	remote->repo = source->repo;
 	remote->download_tags = source->download_tags;
-	remote->check_cert = source->check_cert;
 	remote->update_fetchhead = source->update_fetchhead;
 
 	if (git_vector_init(&remote->refs, 32, NULL) < 0 ||
@@ -369,9 +368,6 @@ int git_remote_load(git_remote **out, git_repository *repo, const char *name)
 	remote->name = git__strdup(name);
 	GITERR_CHECK_ALLOC(remote->name);
 
-	if ((error = get_check_cert(&remote->check_cert, repo)) < 0)
-		goto cleanup;
-
 	if (git_vector_init(&remote->refs, 32, NULL) < 0 ||
 	    git_vector_init(&remote->refspecs, 2, NULL) < 0 ||
 	    git_vector_init(&remote->active_refspecs, 2, NULL) < 0) {
@@ -673,12 +669,9 @@ int git_remote_connect(git_remote *remote, git_direction direction)
 		return error;
 
 	if (t->set_callbacks &&
-		(error = t->set_callbacks(t, remote->callbacks.sideband_progress, NULL, remote->callbacks.payload)) < 0)
+	    (error = t->set_callbacks(t, remote->callbacks.sideband_progress, NULL, remote->callbacks.certificate_check, remote->callbacks.payload)) < 0)
 		goto on_error;
 
-	if (!remote->check_cert)
-		flags |= GIT_TRANSPORTFLAGS_NO_CHECK_CERT;
-
 	if ((error = t->connect(t, url, remote->callbacks.credentials, remote->callbacks.payload, direction, flags)) != 0)
 		goto on_error;
 
@@ -1248,13 +1241,6 @@ int git_remote_list(git_strarray *remotes_list, git_repository *repo)
 	return 0;
 }
 
-void git_remote_check_cert(git_remote *remote, int check)
-{
-	assert(remote);
-
-	remote->check_cert = check;
-}
-
 int git_remote_set_callbacks(git_remote *remote, const git_remote_callbacks *callbacks)
 {
 	assert(remote && callbacks);
@@ -1267,6 +1253,7 @@ int git_remote_set_callbacks(git_remote *remote, const git_remote_callbacks *cal
 		return remote->transport->set_callbacks(remote->transport,
 			remote->callbacks.sideband_progress,
 			NULL,
+			remote->callbacks.certificate_check,
 			remote->callbacks.payload);
 
 	return 0;
diff --git a/src/remote.h b/src/remote.h
index c471756b8..f88601e9b 100644
--- a/src/remote.h
+++ b/src/remote.h
@@ -31,7 +31,6 @@ struct git_remote {
 	git_transfer_progress stats;
 	unsigned int need_pack;
 	git_remote_autotag_option_t download_tags;
-	int check_cert;
 	int update_fetchhead;
 };
 
diff --git a/src/transports/http.c b/src/transports/http.c
index f9df53b71..7ef0b519c 100644
--- a/src/transports/http.c
+++ b/src/transports/http.c
@@ -19,6 +19,10 @@ git_http_auth_scheme auth_schemes[] = {
 	{ GIT_AUTHTYPE_BASIC, "Basic", GIT_CREDTYPE_USERPASS_PLAINTEXT, git_http_auth_basic },
 };
 
+#ifdef GIT_SSL
+# include <openssl/x509v3.h>
+#endif
+
 static const char *upload_pack_service = "upload-pack";
 static const char *upload_pack_ls_service_url = "/info/refs?service=git-upload-pack";
 static const char *upload_pack_service_url = "/git-upload-pack";
@@ -524,7 +528,7 @@ static int write_chunk(gitno_socket *socket, const char *buffer, size_t len)
 
 static int http_connect(http_subtransport *t)
 {
-	int flags = 0;
+	int flags = 0, error;
 
 	if (t->connected &&
 		http_should_keep_alive(&t->parser) &&
@@ -541,13 +545,55 @@ static int http_connect(http_subtransport *t)
 			return -1;
 
 		flags |= GITNO_CONNECT_SSL;
-
-		if (GIT_TRANSPORTFLAGS_NO_CHECK_CERT & tflags)
-			flags |= GITNO_CONNECT_SSL_NO_CHECK_CERT;
 	}
 
-	if (gitno_connect(&t->socket, t->connection_data.host, t->connection_data.port, flags) < 0)
-		return -1;
+	error = gitno_connect(&t->socket, t->connection_data.host, t->connection_data.port, flags);
+
+#ifdef GIT_SSL
+	if ((!error || error == GIT_ECERTIFICATE) && t->owner->certificate_check_cb != NULL) {
+                X509 *cert = SSL_get_peer_certificate(t->socket.ssl.ssl);
+		git_cert_x509 cert_info;
+                int len, is_valid;
+		unsigned char *guard, *encoded_cert;
+
+		/* Retrieve the length of the certificate first */
+		len = i2d_X509(cert, NULL);
+		if (len < 0) {
+			giterr_set(GITERR_NET, "failed to retrieve certificate information");
+			return -1;
+		}
+
+
+		encoded_cert = git__malloc(len);
+		GITERR_CHECK_ALLOC(encoded_cert);
+		/* i2d_X509 makes 'copy' point to just after the data */
+		guard = encoded_cert;
+
+		len = i2d_X509(cert, &guard);
+		if (len < 0) {
+			git__free(encoded_cert);
+			giterr_set(GITERR_NET, "failed to retrieve certificate information");
+			return -1;
+		}
+
+		giterr_clear();
+		is_valid = error != GIT_ECERTIFICATE;
+		cert_info.cert_type = GIT_CERT_X509;
+		cert_info.data = encoded_cert;
+		cert_info.len = len;
+                error = t->owner->certificate_check_cb((git_cert *) &cert_info, is_valid, t->owner->message_cb_payload);
+		git__free(encoded_cert);
+
+		if (error < 0) {
+			if (!giterr_last())
+				giterr_set(GITERR_NET, "user cancelled certificate check");
+
+			return error;
+		}
+	}
+#endif
+	if (error < 0)
+		return error;
 
 	t->connected = 1;
 	return 0;
@@ -608,6 +654,7 @@ replay:
 
 	while (!*bytes_read && !t->parse_finished) {
 		size_t data_offset;
+		int error;
 
 		/*
 		 * Make the parse_buffer think it's as full of data as
@@ -654,8 +701,8 @@ replay:
 		if (PARSE_ERROR_REPLAY == t->parse_error) {
 			s->sent_request = 0;
 
-			if (http_connect(t) < 0)
-				return -1;
+			if ((error = http_connect(t)) < 0)
+				return error;
 
 			goto replay;
 		}
@@ -907,8 +954,8 @@ static int http_action(
 		 (ret = gitno_connection_data_from_url(&t->connection_data, url, NULL)) < 0)
 		return ret;
 
-	if (http_connect(t) < 0)
-		return -1;
+	if ((ret = http_connect(t)) < 0)
+		return ret;
 
 	switch (action) {
 	case GIT_SERVICE_UPLOADPACK_LS:
diff --git a/src/transports/smart.c b/src/transports/smart.c
index a5c3e82dc..d0f9c90e8 100644
--- a/src/transports/smart.c
+++ b/src/transports/smart.c
@@ -53,12 +53,14 @@ static int git_smart__set_callbacks(
 	git_transport *transport,
 	git_transport_message_cb progress_cb,
 	git_transport_message_cb error_cb,
+	git_transport_certificate_check_cb certificate_check_cb,
 	void *message_cb_payload)
 {
 	transport_smart *t = (transport_smart *)transport;
 
 	t->progress_cb = progress_cb;
 	t->error_cb = error_cb;
+	t->certificate_check_cb = certificate_check_cb;
 	t->message_cb_payload = message_cb_payload;
 
 	return 0;
diff --git a/src/transports/smart.h b/src/transports/smart.h
index b29faae44..44e241adc 100644
--- a/src/transports/smart.h
+++ b/src/transports/smart.h
@@ -137,6 +137,7 @@ typedef struct {
 	int flags;
 	git_transport_message_cb progress_cb;
 	git_transport_message_cb error_cb;
+	git_transport_certificate_check_cb certificate_check_cb;
 	void *message_cb_payload;
 	git_smart_subtransport *wrapped;
 	git_smart_subtransport_stream *current_stream;
diff --git a/src/transports/ssh.c b/src/transports/ssh.c
index a57d27d31..15a45ca86 100644
--- a/src/transports/ssh.c
+++ b/src/transports/ssh.c
@@ -473,6 +473,46 @@ static int _git_ssh_setup_conn(
 		GITERR_CHECK_ALLOC(port);
 	}
 
+	if ((error = gitno_connect(&s->socket, host, port, 0)) < 0)
+		goto on_error;
+
+	if ((error = _git_ssh_session_create(&session, s->socket)) < 0)
+		goto on_error;
+
+	if (t->owner->certificate_check_cb != NULL) {
+		git_cert_hostkey cert = { 0 };
+		const char *key;
+
+		cert.cert_type = GIT_CERT_HOSTKEY_LIBSSH2;
+
+		key = libssh2_hostkey_hash(session, LIBSSH2_HOSTKEY_HASH_SHA1);
+		if (key != NULL) {
+			cert.type |= GIT_CERT_SSH_SHA1;
+			memcpy(&cert.hash_sha1, key, 20);
+		}
+
+		key = libssh2_hostkey_hash(session, LIBSSH2_HOSTKEY_HASH_MD5);
+		if (key != NULL) {
+			cert.type |= GIT_CERT_SSH_MD5;
+			memcpy(&cert.hash_md5, key, 16);
+		}
+
+		if (cert.type == 0) {
+			giterr_set(GITERR_SSH, "unable to get the host key");
+			return -1;
+		}
+
+		/* We don't currently trust any hostkeys */
+		giterr_clear();
+                error = t->owner->certificate_check_cb((git_cert *) &cert, 0, t->owner->message_cb_payload);
+		if (error < 0) {
+			if (!giterr_last())
+				giterr_set(GITERR_NET, "user cancelled hostkey check");
+
+			goto on_error;
+		}
+        }
+
 	/* we need the username to ask for auth methods */
 	if (!user) {
 		if ((error = request_creds(&cred, t, NULL, GIT_CREDTYPE_USERNAME)) < 0)
@@ -488,12 +528,6 @@ static int _git_ssh_setup_conn(
 			goto on_error;
 	}
 
-	if ((error = gitno_connect(&s->socket, host, port, 0)) < 0)
-		goto on_error;
-
-	if ((error = _git_ssh_session_create(&session, s->socket)) < 0)
-		goto on_error;
-
 	if ((error = list_auth_methods(&auth_methods, session, user)) < 0)
 		goto on_error;
 
@@ -602,10 +636,8 @@ static int ssh_receivepack_ls(
 {
 	const char *cmd = t->cmd_receivepack ? t->cmd_receivepack : cmd_receivepack;
 
-	if (_git_ssh_setup_conn(t, url, cmd, stream) < 0)
-		return -1;
 
-	return 0;
+	return _git_ssh_setup_conn(t, url, cmd, stream);
 }
 
 static int ssh_receivepack(
diff --git a/src/transports/winhttp.c b/src/transports/winhttp.c
index 6523086da..8aef63193 100644
--- a/src/transports/winhttp.c
+++ b/src/transports/winhttp.c
@@ -16,6 +16,8 @@
 #include "remote.h"
 #include "repository.h"
 
+#include <wincrypt.h>
+#pragma comment(lib, "crypt32")
 #include <winhttp.h>
 #pragma comment(lib, "winhttp")
 
@@ -203,6 +205,39 @@ static int fallback_cred_acquire_cb(
 	return error;
 }
 
+static int certificate_check(winhttp_stream *s, int valid)
+{
+	int error;
+	winhttp_subtransport *t = OWNING_SUBTRANSPORT(s);
+	PCERT_CONTEXT cert_ctx;
+	DWORD cert_ctx_size = sizeof(cert_ctx);
+	git_cert_x509 cert;
+
+	/* If there is no override, we should fail if WinHTTP doesn't think it's fine */
+	if (t->owner->certificate_check_cb == NULL && !valid)
+		return GIT_ECERTIFICATE;
+
+	if (t->owner->certificate_check_cb == NULL || !t->connection_data.use_ssl)
+		return 0;
+
+	if (!WinHttpQueryOption(s->request, WINHTTP_OPTION_SERVER_CERT_CONTEXT, &cert_ctx, &cert_ctx_size)) {
+		giterr_set(GITERR_OS, "failed to get server certificate");
+		return -1;
+	}
+
+	giterr_clear();
+	cert.cert_type = GIT_CERT_X509;
+	cert.data = cert_ctx->pbCertEncoded;
+	cert.len = cert_ctx->cbCertEncoded;
+	error = t->owner->certificate_check_cb((git_cert *) &cert, valid, t->owner->cred_acquire_payload);
+	CertFreeCertificateContext(cert_ctx);
+
+	if (error < 0 && !giterr_last())
+		giterr_set(GITERR_NET, "user cancelled certificate check");
+
+	return error;
+}
+
 static int winhttp_stream_connect(winhttp_stream *s)
 {
 	winhttp_subtransport *t = OWNING_SUBTRANSPORT(s);
@@ -353,13 +388,6 @@ static int winhttp_stream_connect(winhttp_stream *s)
 
 		if (t->owner->parent.read_flags(&t->owner->parent, &flags) < 0)
 			goto on_error;
-
-		if ((GIT_TRANSPORTFLAGS_NO_CHECK_CERT & flags) &&
-			!WinHttpSetOption(s->request, WINHTTP_OPTION_SECURITY_FLAGS,
-			(LPVOID)&no_check_cert_flags, sizeof(no_check_cert_flags))) {
-			giterr_set(GITERR_OS, "Failed to set options to ignore cert errors");
-			goto on_error;
-		}
 	}
 
 	/* If we have a credential on the subtransport, apply it to the request */
@@ -527,6 +555,74 @@ on_error:
 	return error;
 }
 
+static int do_send_request(winhttp_stream *s, size_t len, int ignore_length)
+{
+	int request_failed = 0, cert_valid = 1, error = 0;
+
+	if (ignore_length) {
+		if (!WinHttpSendRequest(s->request,
+			WINHTTP_NO_ADDITIONAL_HEADERS, 0,
+			WINHTTP_NO_REQUEST_DATA, 0,
+			WINHTTP_IGNORE_REQUEST_TOTAL_LENGTH, 0)) {
+			return -1;
+		}
+	} else {
+		if (!WinHttpSendRequest(s->request,
+			WINHTTP_NO_ADDITIONAL_HEADERS, 0,
+			WINHTTP_NO_REQUEST_DATA, 0,
+			len, 0)) {
+			return -1;
+		}
+	}
+
+	return 0;
+}
+
+static int send_request(winhttp_stream *s, size_t len, int ignore_length)
+{
+	int request_failed = 0, cert_valid = 1, error = 0;
+	DWORD ignore_flags;
+
+	if ((error = do_send_request(s, len, ignore_length)) < 0)
+		request_failed = 1;
+
+	if (request_failed) {
+		if (GetLastError() != ERROR_WINHTTP_SECURE_FAILURE) {
+			giterr_set(GITERR_OS, "failed to send request");
+			return -1;
+		} else {
+			cert_valid = 0;
+		}
+	}
+
+	giterr_clear();
+	if ((error = certificate_check(s, cert_valid)) < 0) {
+		if (!giterr_last())
+			giterr_set(GITERR_OS, "user cancelled certificate check");
+
+		return error;
+	}
+
+	/* if neither the request nor the certificate check returned errors, we're done */
+	if (!request_failed)
+		return 0;
+
+	ignore_flags =
+		SECURITY_FLAG_IGNORE_CERT_CN_INVALID |
+		SECURITY_FLAG_IGNORE_CERT_DATE_INVALID |
+		SECURITY_FLAG_IGNORE_UNKNOWN_CA;
+	
+	if (!WinHttpSetOption(s->request, WINHTTP_OPTION_SECURITY_FLAGS, &ignore_flags, sizeof(ignore_flags))) {
+		giterr_set(GITERR_OS, "failed to set security options");
+		return -1;
+	}
+
+	if ((error = do_send_request(s, len, ignore_length)) < 0)
+		giterr_set(GITERR_OS, "failed to send request");
+
+	return error;
+}
+
 static int winhttp_stream_read(
 	git_smart_subtransport_stream *stream,
 	char *buffer,
@@ -537,6 +633,7 @@ static int winhttp_stream_read(
 	winhttp_subtransport *t = OWNING_SUBTRANSPORT(s);
 	DWORD dw_bytes_read;
 	char replay_count = 0;
+	int error;
 
 replay:
 	/* Enforce a reasonable cap on the number of replays */
@@ -553,15 +650,12 @@ replay:
 		DWORD status_code, status_code_length, content_type_length, bytes_written;
 		char expected_content_type_8[MAX_CONTENT_TYPE_LEN];
 		wchar_t expected_content_type[MAX_CONTENT_TYPE_LEN], content_type[MAX_CONTENT_TYPE_LEN];
+		int request_failed = 0, cert_valid = 1;
 
 		if (!s->sent_request) {
-			if (!WinHttpSendRequest(s->request,
-				WINHTTP_NO_ADDITIONAL_HEADERS, 0,
-				WINHTTP_NO_REQUEST_DATA, 0,
-				s->post_body_len, 0)) {
-				giterr_set(GITERR_OS, "Failed to send request");
-				return -1;
-			}
+
+			if ((error = send_request(s, s->post_body_len, 0)) < 0)
+				return error;
 
 			s->sent_request = 1;
 		}
@@ -815,6 +909,7 @@ static int winhttp_stream_write_single(
 	winhttp_stream *s = (winhttp_stream *)stream;
 	winhttp_subtransport *t = OWNING_SUBTRANSPORT(s);
 	DWORD bytes_written;
+	int error;
 
 	if (!s->request && winhttp_stream_connect(s) < 0)
 		return -1;
@@ -825,13 +920,8 @@ static int winhttp_stream_write_single(
 		return -1;
 	}
 
-	if (!WinHttpSendRequest(s->request,
-			WINHTTP_NO_ADDITIONAL_HEADERS, 0,
-			WINHTTP_NO_REQUEST_DATA, 0,
-			(DWORD)len, 0)) {
-		giterr_set(GITERR_OS, "Failed to send request");
-		return -1;
-	}
+	if ((error = send_request(s, len, 0)) < 0)
+		return error;
 
 	s->sent_request = 1;
 
@@ -954,6 +1044,7 @@ static int winhttp_stream_write_chunked(
 {
 	winhttp_stream *s = (winhttp_stream *)stream;
 	winhttp_subtransport *t = OWNING_SUBTRANSPORT(s);
+	int error;
 
 	if (!s->request && winhttp_stream_connect(s) < 0)
 		return -1;
@@ -967,13 +1058,8 @@ static int winhttp_stream_write_chunked(
 			return -1;
 		}
 
-		if (!WinHttpSendRequest(s->request,
-			WINHTTP_NO_ADDITIONAL_HEADERS, 0,
-			WINHTTP_NO_REQUEST_DATA, 0,
-			WINHTTP_IGNORE_REQUEST_TOTAL_LENGTH, 0)) {
-			giterr_set(GITERR_OS, "Failed to send request");
-			return -1;
-		}
+		if ((error = send_request(s, 0, 1)) < 0)
+			return error;
 
 		s->sent_request = 1;
 	}