Commit 17491f6e5660b82cb8163eea58805df6398af16e
2014-08-29T17:18:23
transport: always call the certificate check callback We should let the user decide whether to cancel the connection or not regardless of whether our checks have decided that the certificate is fine. We provide our own assessment to the callback to let the user fall back to our checks if they so desire.
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 53 54 55 56 57 58 59 60 61 62 63 64 65 66 67 68 69 70 71 72 73 74 75 76 77 78 79 80 81 82 83 84 85 86 87 88 89 90 91 92 93 94 95 96 97 98 99 100 101 102 103 104 105 106 107 108 109 110 111 112 113 114 115 116 117 118 119 120 121 122 123 124 125 126 127 128 129 130 131 132 133 134 135 136 137 138 139 140 141 142 143
diff --git a/include/git2/types.h b/include/git2/types.h
index b574d29..51f0588 100644
--- a/include/git2/types.h
+++ b/include/git2/types.h
@@ -262,9 +262,11 @@ typedef enum git_cert_t git_cert_t;
* @param type The type of certificate or host info, SSH or X.509
* @param data The data for the certificate or host info
* @param len The size of the certificate or host info
+ * @param valid Whether the libgit2 checks (OpenSSL or WinHTTP) think
+ * this certificate is valid
* @param payload Payload provided by the caller
*/
-typedef int (*git_transport_certificate_check_cb)(git_cert_t type, void *data, size_t len, void *payload);
+typedef int (*git_transport_certificate_check_cb)(git_cert_t type, void *data, size_t len, int valid, void *payload);
/**
* Opaque structure representing a submodule.
diff --git a/src/transports/http.c b/src/transports/http.c
index ab47754..f49242e 100644
--- a/src/transports/http.c
+++ b/src/transports/http.c
@@ -553,9 +553,9 @@ static int http_connect(http_subtransport *t)
error = gitno_connect(&t->socket, t->connection_data.host, t->connection_data.port, flags);
#ifdef GIT_SSL
- if (error == GIT_ECERTIFICATE && t->owner->certificate_check_cb != NULL) {
+ if ((!error || error == GIT_ECERTIFICATE) && t->owner->certificate_check_cb != NULL) {
X509 *cert = SSL_get_peer_certificate(t->socket.ssl.ssl);
- int allow, len;
+ int allow, len, is_valid;
unsigned char *guard, *encoded_cert;
/* Retrieve the length of the certificate first */
@@ -578,7 +578,8 @@ static int http_connect(http_subtransport *t)
return -1;
}
- allow = t->owner->certificate_check_cb(GIT_CERT_X509, encoded_cert, len, t->owner->message_cb_payload);
+ is_valid = error != GIT_ECERTIFICATE;
+ allow = t->owner->certificate_check_cb(GIT_CERT_X509, encoded_cert, len, is_valid, t->owner->message_cb_payload);
git__free(encoded_cert);
if (allow < 0) {
@@ -589,10 +590,9 @@ static int http_connect(http_subtransport *t)
error = 0;
}
}
-#else
+#endif
if (error < 0)
return error;
-#endif
t->connected = 1;
return 0;
@@ -653,6 +653,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
@@ -699,8 +700,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;
}
@@ -952,8 +953,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/ssh.c b/src/transports/ssh.c
index ba549ff..8344714 100644
--- a/src/transports/ssh.c
+++ b/src/transports/ssh.c
@@ -541,7 +541,8 @@ static int _git_ssh_setup_conn(
return -1;
}
- allow = t->owner->certificate_check_cb(GIT_CERT_HOSTKEY_LIBSSH2, &cert, certlen, t->owner->message_cb_payload);
+ /* We don't currently trust any hostkeys */
+ allow = t->owner->certificate_check_cb(GIT_CERT_HOSTKEY_LIBSSH2, &cert, certlen, 0, t->owner->message_cb_payload);
if (allow < 0) {
error = allow;
goto on_error;
diff --git a/tests/online/clone.c b/tests/online/clone.c
index 2e8db2b..0e9a176 100644
--- a/tests/online/clone.c
+++ b/tests/online/clone.c
@@ -470,14 +470,15 @@ void test_online_clone__url_with_no_path_returns_EINVALIDSPEC(void)
GIT_EINVALIDSPEC);
}
-static int fail_certificate_check(git_cert_t type, void *data, size_t len, void *payload)
+static int fail_certificate_check(git_cert_t type, void *data, size_t len, int valid, void *payload)
{
GIT_UNUSED(type);
GIT_UNUSED(data);
GIT_UNUSED(len);
+ GIT_UNUSED(valid);
GIT_UNUSED(payload);
- return GIT_EUSER;
+ return 0;
}
void test_online_clone__certificate_invalid(void)
@@ -485,17 +486,18 @@ void test_online_clone__certificate_invalid(void)
g_options.remote_callbacks.certificate_check = fail_certificate_check;
cl_git_fail_with(git_clone(&g_repo, "http://github.com/libgit2/TestGitRepository", "./foo", &g_options),
- GIT_EUSER);
+ GIT_ECERTIFICATE);
}
-static int succeed_certificate_check(git_cert_t type, void *data, size_t len, void *payload)
+static int succeed_certificate_check(git_cert_t type, void *data, size_t len, int valid, void *payload)
{
GIT_UNUSED(type);
GIT_UNUSED(data);
GIT_UNUSED(len);
+ GIT_UNUSED(valid);
GIT_UNUSED(payload);
- return 0;
+ return 1;
}
void test_online_clone__certificate_valid(void)