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.
This commit is contained in:
Carlos Martín Nieto 2014-08-29 17:18:23 +02:00
parent 85acc56262
commit 17491f6e56
4 changed files with 22 additions and 16 deletions

View File

@ -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 type The type of certificate or host info, SSH or X.509
* @param data The data for the certificate or host info * @param data The data for the certificate or host info
* @param len The size of 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 * @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. * Opaque structure representing a submodule.

View File

@ -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); error = gitno_connect(&t->socket, t->connection_data.host, t->connection_data.port, flags);
#ifdef GIT_SSL #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); X509 *cert = SSL_get_peer_certificate(t->socket.ssl.ssl);
int allow, len; int allow, len, is_valid;
unsigned char *guard, *encoded_cert; unsigned char *guard, *encoded_cert;
/* Retrieve the length of the certificate first */ /* Retrieve the length of the certificate first */
@ -578,7 +578,8 @@ static int http_connect(http_subtransport *t)
return -1; 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); git__free(encoded_cert);
if (allow < 0) { if (allow < 0) {
@ -589,10 +590,9 @@ static int http_connect(http_subtransport *t)
error = 0; error = 0;
} }
} }
#else #endif
if (error < 0) if (error < 0)
return error; return error;
#endif
t->connected = 1; t->connected = 1;
return 0; return 0;
@ -653,6 +653,7 @@ replay:
while (!*bytes_read && !t->parse_finished) { while (!*bytes_read && !t->parse_finished) {
size_t data_offset; size_t data_offset;
int error;
/* /*
* Make the parse_buffer think it's as full of data as * Make the parse_buffer think it's as full of data as
@ -699,8 +700,8 @@ replay:
if (PARSE_ERROR_REPLAY == t->parse_error) { if (PARSE_ERROR_REPLAY == t->parse_error) {
s->sent_request = 0; s->sent_request = 0;
if (http_connect(t) < 0) if ((error = http_connect(t)) < 0)
return -1; return error;
goto replay; goto replay;
} }
@ -952,8 +953,8 @@ static int http_action(
(ret = gitno_connection_data_from_url(&t->connection_data, url, NULL)) < 0) (ret = gitno_connection_data_from_url(&t->connection_data, url, NULL)) < 0)
return ret; return ret;
if (http_connect(t) < 0) if ((ret = http_connect(t)) < 0)
return -1; return ret;
switch (action) { switch (action) {
case GIT_SERVICE_UPLOADPACK_LS: case GIT_SERVICE_UPLOADPACK_LS:

View File

@ -541,7 +541,8 @@ static int _git_ssh_setup_conn(
return -1; 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) { if (allow < 0) {
error = allow; error = allow;
goto on_error; goto on_error;

View File

@ -470,14 +470,15 @@ void test_online_clone__url_with_no_path_returns_EINVALIDSPEC(void)
GIT_EINVALIDSPEC); 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(type);
GIT_UNUSED(data); GIT_UNUSED(data);
GIT_UNUSED(len); GIT_UNUSED(len);
GIT_UNUSED(valid);
GIT_UNUSED(payload); GIT_UNUSED(payload);
return GIT_EUSER; return 0;
} }
void test_online_clone__certificate_invalid(void) 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; 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), 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(type);
GIT_UNUSED(data); GIT_UNUSED(data);
GIT_UNUSED(len); GIT_UNUSED(len);
GIT_UNUSED(valid);
GIT_UNUSED(payload); GIT_UNUSED(payload);
return 0; return 1;
} }
void test_online_clone__certificate_valid(void) void test_online_clone__certificate_valid(void)