diff --git a/src/transports/http.c b/src/transports/http.c index ad28c5889..155fd7b30 100644 --- a/src/transports/http.c +++ b/src/transports/http.c @@ -624,13 +624,12 @@ static int http_connect(http_subtransport *t) if ((!error || error == GIT_ECERTIFICATE) && t->owner->certificate_check_cb != NULL && git_stream_is_encrypted(t->io)) { git_cert *cert; - int is_valid; + int is_valid = (error == GIT_OK); if ((error = git_stream_certificate(&cert, t->io)) < 0) return error; giterr_clear(); - is_valid = error != GIT_ECERTIFICATE; error = t->owner->certificate_check_cb(cert, is_valid, t->connection_data.host, t->owner->message_cb_payload); if (error < 0) { diff --git a/src/transports/smart_pkt.c b/src/transports/smart_pkt.c index 2297cc94f..e05196cd8 100644 --- a/src/transports/smart_pkt.c +++ b/src/transports/smart_pkt.c @@ -427,15 +427,23 @@ int git_pkt_parse_line( if (bufflen > 0 && bufflen < (size_t)len) return GIT_EBUFS; + /* + * The length has to be exactly 0 in case of a flush + * packet or greater than PKT_LEN_SIZE, as the decoded + * length includes its own encoded length of four bytes. + */ + if (len != 0 && len < PKT_LEN_SIZE) + return GIT_ERROR; + line += PKT_LEN_SIZE; /* - * TODO: How do we deal with empty lines? Try again? with the next - * line? + * The Git protocol does not specify empty lines as part + * of the protocol. Not knowing what to do with an empty + * line, we should return an error upon hitting one. */ if (len == PKT_LEN_SIZE) { - *head = NULL; - *out = line; - return 0; + giterr_set_str(GITERR_NET, "Invalid empty packet"); + return GIT_ERROR; } if (len == 0) { /* Flush pkt */ diff --git a/src/transports/smart_protocol.c b/src/transports/smart_protocol.c index 53c0b089e..db6a8b9c8 100644 --- a/src/transports/smart_protocol.c +++ b/src/transports/smart_protocol.c @@ -763,14 +763,6 @@ static int add_push_report_sideband_pkt(git_push *push, git_pkt_data *data_pkt, line_len -= (line_end - line); line = line_end; - /* When a valid packet with no content has been - * read, git_pkt_parse_line does not report an - * error, but the pkt pointer has not been set. - * Handle this by skipping over empty packets. - */ - if (pkt == NULL) - continue; - error = add_push_report_pkt(push, pkt); git_pkt_free(pkt); @@ -825,9 +817,6 @@ static int parse_report(transport_smart *transport, git_push *push) error = 0; - if (pkt == NULL) - continue; - switch (pkt->type) { case GIT_PKT_DATA: /* This is a sideband packet which contains other packets */ diff --git a/tests/online/badssl.c b/tests/online/badssl.c index 66b090df4..aa4c24d9c 100644 --- a/tests/online/badssl.c +++ b/tests/online/badssl.c @@ -10,37 +10,71 @@ static bool g_has_ssl = true; static bool g_has_ssl = false; #endif +static int cert_check_assert_invalid(git_cert *cert, int valid, const char* host, void *payload) +{ + GIT_UNUSED(cert); GIT_UNUSED(host); GIT_UNUSED(payload); + + cl_assert_equal_i(0, valid); + + return GIT_ECERTIFICATE; +} + void test_online_badssl__expired(void) { + git_clone_options opts = GIT_CLONE_OPTIONS_INIT; + opts.fetch_opts.callbacks.certificate_check = cert_check_assert_invalid; + if (!g_has_ssl) cl_skip(); cl_git_fail_with(GIT_ECERTIFICATE, git_clone(&g_repo, "https://expired.badssl.com/fake.git", "./fake", NULL)); + + cl_git_fail_with(GIT_ECERTIFICATE, + git_clone(&g_repo, "https://expired.badssl.com/fake.git", "./fake", &opts)); } void test_online_badssl__wrong_host(void) { + git_clone_options opts = GIT_CLONE_OPTIONS_INIT; + opts.fetch_opts.callbacks.certificate_check = cert_check_assert_invalid; + if (!g_has_ssl) cl_skip(); cl_git_fail_with(GIT_ECERTIFICATE, git_clone(&g_repo, "https://wrong.host.badssl.com/fake.git", "./fake", NULL)); + cl_git_fail_with(GIT_ECERTIFICATE, + git_clone(&g_repo, "https://wrong.host.badssl.com/fake.git", "./fake", &opts)); } void test_online_badssl__self_signed(void) { + git_clone_options opts = GIT_CLONE_OPTIONS_INIT; + opts.fetch_opts.callbacks.certificate_check = cert_check_assert_invalid; + if (!g_has_ssl) cl_skip(); cl_git_fail_with(GIT_ECERTIFICATE, git_clone(&g_repo, "https://self-signed.badssl.com/fake.git", "./fake", NULL)); + cl_git_fail_with(GIT_ECERTIFICATE, + git_clone(&g_repo, "https://self-signed.badssl.com/fake.git", "./fake", &opts)); } void test_online_badssl__old_cipher(void) { + git_clone_options opts = GIT_CLONE_OPTIONS_INIT; + opts.fetch_opts.callbacks.certificate_check = cert_check_assert_invalid; + + /* FIXME: we don't actually reject RC4 anywhere, figure out what to tweak */ + cl_skip(); + if (!g_has_ssl) cl_skip(); - cl_git_fail(git_clone(&g_repo, "https://rc4.badssl.com/fake.git", "./fake", NULL)); + cl_git_fail_with(GIT_ECERTIFICATE, + git_clone(&g_repo, "https://rc4.badssl.com/fake.git", "./fake", NULL)); + cl_git_fail_with(GIT_ECERTIFICATE, + git_clone(&g_repo, "https://rc4.badssl.com/fake.git", "./fake", &opts)); }