Loading changes/bug22460_case2 0 → 100644 +8 −0 Original line number Diff line number Diff line o Major bugfixes (relay, link handshake): - When performing the v3 link handshake on a TLS connection, report that we have the x509 certificate that we actually used on that connection, even if we have changed certificates since that connection was first opened. Previously, we would claim to have used our most recent x509 link certificate, which would sometimes make the link handshake fail. Fixes one case of bug 22460; bugfix on 0.2.3.6-alpha. src/common/tortls.c +29 −1 Original line number Diff line number Diff line Loading @@ -677,6 +677,15 @@ MOCK_IMPL(STATIC tor_x509_cert_t *, return cert; } /** Return a new copy of <b>cert</b>. */ tor_x509_cert_t * tor_x509_cert_dup(const tor_x509_cert_t *cert) { tor_assert(cert); X509 *x509 = cert->cert; return tor_x509_cert_new(X509_dup(x509)); } /** Read a DER-encoded X509 cert, of length exactly <b>certificate_len</b>, * from a <b>certificate</b>. Return a newly allocated tor_x509_cert_t on * success and NULL on failure. */ Loading Loading @@ -2009,7 +2018,8 @@ tor_tls_peer_has_cert(tor_tls_t *tls) return 1; } /** Return the peer certificate, or NULL if there isn't one. */ /** Return a newly allocated copy of the peer certificate, or NULL if there * isn't one. */ MOCK_IMPL(tor_x509_cert_t *, tor_tls_get_peer_cert,(tor_tls_t *tls)) { Loading @@ -2021,6 +2031,24 @@ tor_tls_get_peer_cert,(tor_tls_t *tls)) return tor_x509_cert_new(cert); } /** Return a newly allocated copy of the cerficate we used on the connection, * or NULL if somehow we didn't use one. */ MOCK_IMPL(tor_x509_cert_t *, tor_tls_get_own_cert,(tor_tls_t *tls)) { X509 *cert = SSL_get_certificate(tls->ssl); tls_log_errors(tls, LOG_WARN, LD_HANDSHAKE, "getting own-connection certificate"); if (!cert) return NULL; /* Fun inconsistency: SSL_get_peer_certificate increments the reference * count, but SSL_get_certificate does not. */ X509 *duplicate = X509_dup(cert); if (BUG(duplicate == NULL)) return NULL; return tor_x509_cert_new(duplicate); } /** Warn that a certificate lifetime extends through a certain range. */ static void log_cert_lifetime(int severity, const X509 *cert, const char *problem) Loading src/common/tortls.h +2 −0 Original line number Diff line number Diff line Loading @@ -197,7 +197,9 @@ void tor_tls_set_renegotiate_callback(tor_tls_t *tls, int tor_tls_is_server(tor_tls_t *tls); void tor_tls_free(tor_tls_t *tls); int tor_tls_peer_has_cert(tor_tls_t *tls); tor_x509_cert_t *tor_x509_cert_dup(const tor_x509_cert_t *cert); MOCK_DECL(tor_x509_cert_t *,tor_tls_get_peer_cert,(tor_tls_t *tls)); MOCK_DECL(tor_x509_cert_t *,tor_tls_get_own_cert,(tor_tls_t *tls)); int tor_tls_verify(int severity, tor_tls_t *tls, crypto_pk_t **identity); int tor_tls_check_lifetime(int severity, tor_tls_t *tls, int past_tolerance, Loading src/or/connection_or.c +18 −12 Original line number Diff line number Diff line Loading @@ -2137,7 +2137,9 @@ connection_or_send_netinfo,(or_connection_t *conn)) int connection_or_send_certs_cell(or_connection_t *conn) { const tor_x509_cert_t *link_cert = NULL, *id_cert = NULL; const tor_x509_cert_t *global_link_cert = NULL, *id_cert = NULL, *using_link_cert = NULL; tor_x509_cert_t *own_link_cert = NULL; const uint8_t *link_encoded = NULL, *id_encoded = NULL; size_t link_len, id_len; var_cell_t *cell; Loading @@ -2149,9 +2151,15 @@ connection_or_send_certs_cell(or_connection_t *conn) if (! conn->handshake_state) return -1; const int conn_in_server_mode = ! conn->handshake_state->started_here; if (tor_tls_get_my_certs(conn_in_server_mode, &link_cert, &id_cert) < 0) if (tor_tls_get_my_certs(conn_in_server_mode, &global_link_cert, &id_cert) < 0) return -1; tor_x509_cert_get_der(link_cert, &link_encoded, &link_len); if (conn_in_server_mode) { using_link_cert = own_link_cert = tor_tls_get_own_cert(conn->tls); } else { using_link_cert = global_link_cert; } tor_x509_cert_get_der(using_link_cert, &link_encoded, &link_len); tor_x509_cert_get_der(id_cert, &id_encoded, &id_len); cell_len = 1 /* 1 byte: num certs in cell */ + Loading Loading @@ -2179,6 +2187,7 @@ connection_or_send_certs_cell(or_connection_t *conn) connection_or_write_var_cell_to_buf(cell, conn); var_cell_free(cell); tor_x509_cert_free(own_link_cert); return 0; } Loading Loading @@ -2258,10 +2267,10 @@ connection_or_compute_authenticate_cell_body(or_connection_t *conn, memcpy(auth1_getarray_type(auth), "AUTH0001", 8); { const tor_x509_cert_t *id_cert=NULL, *link_cert=NULL; const tor_x509_cert_t *id_cert=NULL; const common_digests_t *my_digests, *their_digests; const uint8_t *my_id, *their_id, *client_id, *server_id; if (tor_tls_get_my_certs(server, &link_cert, &id_cert)) if (tor_tls_get_my_certs(server, NULL, &id_cert)) goto err; my_digests = tor_x509_cert_get_id_digests(id_cert); their_digests = Loading Loading @@ -2300,13 +2309,11 @@ connection_or_compute_authenticate_cell_body(or_connection_t *conn, { /* Digest of cert used on TLS link : 32 octets. */ const tor_x509_cert_t *cert = NULL; tor_x509_cert_t *freecert = NULL; tor_x509_cert_t *cert = NULL; if (server) { tor_tls_get_my_certs(1, &cert, NULL); cert = tor_tls_get_own_cert(conn->tls); } else { freecert = tor_tls_get_peer_cert(conn->tls); cert = freecert; cert = tor_tls_get_peer_cert(conn->tls); } if (!cert) { log_warn(LD_OR, "Unable to find cert when making AUTH1 data."); Loading @@ -2316,8 +2323,7 @@ connection_or_compute_authenticate_cell_body(or_connection_t *conn, memcpy(auth->scert, tor_x509_cert_get_cert_digests(cert)->d[DIGEST_SHA256], 32); if (freecert) tor_x509_cert_free(freecert); tor_x509_cert_free(cert); } /* HMAC of clientrandom and serverrandom using master key : 32 octets */ Loading src/test/test_link_handshake.c +27 −1 Original line number Diff line number Diff line Loading @@ -66,6 +66,14 @@ mock_send_authenticate(or_connection_t *conn, int type) return 0; } static tor_x509_cert_t *mock_own_cert = NULL; static tor_x509_cert_t * mock_get_own_cert(tor_tls_t *tls) { (void)tls; return tor_x509_cert_dup(mock_own_cert); } /* Test good certs cells */ static void test_link_handshake_certs_ok(void *arg) Loading @@ -84,6 +92,7 @@ test_link_handshake_certs_ok(void *arg) MOCK(tor_tls_cert_matches_key, mock_tls_cert_matches_key); MOCK(connection_or_write_var_cell_to_buf, mock_write_var_cell); MOCK(connection_or_send_netinfo, mock_send_netinfo); MOCK(tor_tls_get_own_cert, mock_get_own_cert); key1 = pk_generate(2); key2 = pk_generate(3); Loading @@ -94,6 +103,12 @@ test_link_handshake_certs_ok(void *arg) tt_int_op(tor_tls_context_init(TOR_TLS_CTX_IS_PUBLIC_SERVER, key1, key2, 86400), ==, 0); { const tor_x509_cert_t *link = NULL; tt_assert(!tor_tls_get_my_certs(1, &link, NULL)); mock_own_cert = tor_x509_cert_dup(link); } c1->base_.state = OR_CONN_STATE_OR_HANDSHAKING_V3; c1->link_proto = 3; tt_int_op(connection_init_or_handshake_state(c1, 1), ==, 0); Loading Loading @@ -174,6 +189,9 @@ test_link_handshake_certs_ok(void *arg) UNMOCK(tor_tls_cert_matches_key); UNMOCK(connection_or_write_var_cell_to_buf); UNMOCK(connection_or_send_netinfo); UNMOCK(tor_tls_get_own_cert); tor_x509_cert_free(mock_own_cert); mock_own_cert = NULL; memset(c1->identity_digest, 0, sizeof(c1->identity_digest)); memset(c2->identity_digest, 0, sizeof(c2->identity_digest)); connection_free_(TO_CONN(c1)); Loading Loading @@ -656,11 +674,12 @@ AUTHCHALLENGE_FAIL(nonzero_circid, d->cell->circ_id = 1337) static tor_x509_cert_t *mock_peer_cert = NULL; static tor_x509_cert_t * mock_get_peer_cert(tor_tls_t *tls) { (void)tls; return mock_peer_cert; return tor_x509_cert_dup(mock_peer_cert); } static int Loading Loading @@ -694,6 +713,7 @@ authenticate_data_cleanup(const struct testcase_t *test, void *arg) (void) test; UNMOCK(connection_or_write_var_cell_to_buf); UNMOCK(tor_tls_get_peer_cert); UNMOCK(tor_tls_get_own_cert); UNMOCK(tor_tls_get_tlssecrets); UNMOCK(connection_or_close_for_error); UNMOCK(channel_set_circid_type); Loading @@ -710,7 +730,10 @@ authenticate_data_cleanup(const struct testcase_t *test, void *arg) crypto_pk_free(d->key2); tor_free(d); } tor_x509_cert_free(mock_peer_cert); tor_x509_cert_free(mock_own_cert); mock_peer_cert = NULL; mock_own_cert = NULL; return 1; } Loading @@ -724,6 +747,7 @@ authenticate_data_setup(const struct testcase_t *test) MOCK(connection_or_write_var_cell_to_buf, mock_write_var_cell); MOCK(tor_tls_get_peer_cert, mock_get_peer_cert); MOCK(tor_tls_get_own_cert, mock_get_own_cert); MOCK(tor_tls_get_tlssecrets, mock_get_tlssecrets); MOCK(connection_or_close_for_error, mock_close_for_err); MOCK(channel_set_circid_type, mock_set_circid_type); Loading Loading @@ -773,6 +797,8 @@ authenticate_data_setup(const struct testcase_t *test) tor_x509_cert_get_der(link_cert, &der, &sz); mock_peer_cert = tor_x509_cert_decode(der, sz); tt_assert(mock_peer_cert); mock_own_cert = tor_x509_cert_decode(der, sz); tt_assert(mock_own_cert); tt_assert(! tor_tls_get_my_certs(0, &auth_cert, &id_cert)); tor_x509_cert_get_der(auth_cert, &der, &sz); d->c2->handshake_state->auth_cert = tor_x509_cert_decode(der, sz); Loading Loading
changes/bug22460_case2 0 → 100644 +8 −0 Original line number Diff line number Diff line o Major bugfixes (relay, link handshake): - When performing the v3 link handshake on a TLS connection, report that we have the x509 certificate that we actually used on that connection, even if we have changed certificates since that connection was first opened. Previously, we would claim to have used our most recent x509 link certificate, which would sometimes make the link handshake fail. Fixes one case of bug 22460; bugfix on 0.2.3.6-alpha.
src/common/tortls.c +29 −1 Original line number Diff line number Diff line Loading @@ -677,6 +677,15 @@ MOCK_IMPL(STATIC tor_x509_cert_t *, return cert; } /** Return a new copy of <b>cert</b>. */ tor_x509_cert_t * tor_x509_cert_dup(const tor_x509_cert_t *cert) { tor_assert(cert); X509 *x509 = cert->cert; return tor_x509_cert_new(X509_dup(x509)); } /** Read a DER-encoded X509 cert, of length exactly <b>certificate_len</b>, * from a <b>certificate</b>. Return a newly allocated tor_x509_cert_t on * success and NULL on failure. */ Loading Loading @@ -2009,7 +2018,8 @@ tor_tls_peer_has_cert(tor_tls_t *tls) return 1; } /** Return the peer certificate, or NULL if there isn't one. */ /** Return a newly allocated copy of the peer certificate, or NULL if there * isn't one. */ MOCK_IMPL(tor_x509_cert_t *, tor_tls_get_peer_cert,(tor_tls_t *tls)) { Loading @@ -2021,6 +2031,24 @@ tor_tls_get_peer_cert,(tor_tls_t *tls)) return tor_x509_cert_new(cert); } /** Return a newly allocated copy of the cerficate we used on the connection, * or NULL if somehow we didn't use one. */ MOCK_IMPL(tor_x509_cert_t *, tor_tls_get_own_cert,(tor_tls_t *tls)) { X509 *cert = SSL_get_certificate(tls->ssl); tls_log_errors(tls, LOG_WARN, LD_HANDSHAKE, "getting own-connection certificate"); if (!cert) return NULL; /* Fun inconsistency: SSL_get_peer_certificate increments the reference * count, but SSL_get_certificate does not. */ X509 *duplicate = X509_dup(cert); if (BUG(duplicate == NULL)) return NULL; return tor_x509_cert_new(duplicate); } /** Warn that a certificate lifetime extends through a certain range. */ static void log_cert_lifetime(int severity, const X509 *cert, const char *problem) Loading
src/common/tortls.h +2 −0 Original line number Diff line number Diff line Loading @@ -197,7 +197,9 @@ void tor_tls_set_renegotiate_callback(tor_tls_t *tls, int tor_tls_is_server(tor_tls_t *tls); void tor_tls_free(tor_tls_t *tls); int tor_tls_peer_has_cert(tor_tls_t *tls); tor_x509_cert_t *tor_x509_cert_dup(const tor_x509_cert_t *cert); MOCK_DECL(tor_x509_cert_t *,tor_tls_get_peer_cert,(tor_tls_t *tls)); MOCK_DECL(tor_x509_cert_t *,tor_tls_get_own_cert,(tor_tls_t *tls)); int tor_tls_verify(int severity, tor_tls_t *tls, crypto_pk_t **identity); int tor_tls_check_lifetime(int severity, tor_tls_t *tls, int past_tolerance, Loading
src/or/connection_or.c +18 −12 Original line number Diff line number Diff line Loading @@ -2137,7 +2137,9 @@ connection_or_send_netinfo,(or_connection_t *conn)) int connection_or_send_certs_cell(or_connection_t *conn) { const tor_x509_cert_t *link_cert = NULL, *id_cert = NULL; const tor_x509_cert_t *global_link_cert = NULL, *id_cert = NULL, *using_link_cert = NULL; tor_x509_cert_t *own_link_cert = NULL; const uint8_t *link_encoded = NULL, *id_encoded = NULL; size_t link_len, id_len; var_cell_t *cell; Loading @@ -2149,9 +2151,15 @@ connection_or_send_certs_cell(or_connection_t *conn) if (! conn->handshake_state) return -1; const int conn_in_server_mode = ! conn->handshake_state->started_here; if (tor_tls_get_my_certs(conn_in_server_mode, &link_cert, &id_cert) < 0) if (tor_tls_get_my_certs(conn_in_server_mode, &global_link_cert, &id_cert) < 0) return -1; tor_x509_cert_get_der(link_cert, &link_encoded, &link_len); if (conn_in_server_mode) { using_link_cert = own_link_cert = tor_tls_get_own_cert(conn->tls); } else { using_link_cert = global_link_cert; } tor_x509_cert_get_der(using_link_cert, &link_encoded, &link_len); tor_x509_cert_get_der(id_cert, &id_encoded, &id_len); cell_len = 1 /* 1 byte: num certs in cell */ + Loading Loading @@ -2179,6 +2187,7 @@ connection_or_send_certs_cell(or_connection_t *conn) connection_or_write_var_cell_to_buf(cell, conn); var_cell_free(cell); tor_x509_cert_free(own_link_cert); return 0; } Loading Loading @@ -2258,10 +2267,10 @@ connection_or_compute_authenticate_cell_body(or_connection_t *conn, memcpy(auth1_getarray_type(auth), "AUTH0001", 8); { const tor_x509_cert_t *id_cert=NULL, *link_cert=NULL; const tor_x509_cert_t *id_cert=NULL; const common_digests_t *my_digests, *their_digests; const uint8_t *my_id, *their_id, *client_id, *server_id; if (tor_tls_get_my_certs(server, &link_cert, &id_cert)) if (tor_tls_get_my_certs(server, NULL, &id_cert)) goto err; my_digests = tor_x509_cert_get_id_digests(id_cert); their_digests = Loading Loading @@ -2300,13 +2309,11 @@ connection_or_compute_authenticate_cell_body(or_connection_t *conn, { /* Digest of cert used on TLS link : 32 octets. */ const tor_x509_cert_t *cert = NULL; tor_x509_cert_t *freecert = NULL; tor_x509_cert_t *cert = NULL; if (server) { tor_tls_get_my_certs(1, &cert, NULL); cert = tor_tls_get_own_cert(conn->tls); } else { freecert = tor_tls_get_peer_cert(conn->tls); cert = freecert; cert = tor_tls_get_peer_cert(conn->tls); } if (!cert) { log_warn(LD_OR, "Unable to find cert when making AUTH1 data."); Loading @@ -2316,8 +2323,7 @@ connection_or_compute_authenticate_cell_body(or_connection_t *conn, memcpy(auth->scert, tor_x509_cert_get_cert_digests(cert)->d[DIGEST_SHA256], 32); if (freecert) tor_x509_cert_free(freecert); tor_x509_cert_free(cert); } /* HMAC of clientrandom and serverrandom using master key : 32 octets */ Loading
src/test/test_link_handshake.c +27 −1 Original line number Diff line number Diff line Loading @@ -66,6 +66,14 @@ mock_send_authenticate(or_connection_t *conn, int type) return 0; } static tor_x509_cert_t *mock_own_cert = NULL; static tor_x509_cert_t * mock_get_own_cert(tor_tls_t *tls) { (void)tls; return tor_x509_cert_dup(mock_own_cert); } /* Test good certs cells */ static void test_link_handshake_certs_ok(void *arg) Loading @@ -84,6 +92,7 @@ test_link_handshake_certs_ok(void *arg) MOCK(tor_tls_cert_matches_key, mock_tls_cert_matches_key); MOCK(connection_or_write_var_cell_to_buf, mock_write_var_cell); MOCK(connection_or_send_netinfo, mock_send_netinfo); MOCK(tor_tls_get_own_cert, mock_get_own_cert); key1 = pk_generate(2); key2 = pk_generate(3); Loading @@ -94,6 +103,12 @@ test_link_handshake_certs_ok(void *arg) tt_int_op(tor_tls_context_init(TOR_TLS_CTX_IS_PUBLIC_SERVER, key1, key2, 86400), ==, 0); { const tor_x509_cert_t *link = NULL; tt_assert(!tor_tls_get_my_certs(1, &link, NULL)); mock_own_cert = tor_x509_cert_dup(link); } c1->base_.state = OR_CONN_STATE_OR_HANDSHAKING_V3; c1->link_proto = 3; tt_int_op(connection_init_or_handshake_state(c1, 1), ==, 0); Loading Loading @@ -174,6 +189,9 @@ test_link_handshake_certs_ok(void *arg) UNMOCK(tor_tls_cert_matches_key); UNMOCK(connection_or_write_var_cell_to_buf); UNMOCK(connection_or_send_netinfo); UNMOCK(tor_tls_get_own_cert); tor_x509_cert_free(mock_own_cert); mock_own_cert = NULL; memset(c1->identity_digest, 0, sizeof(c1->identity_digest)); memset(c2->identity_digest, 0, sizeof(c2->identity_digest)); connection_free_(TO_CONN(c1)); Loading Loading @@ -656,11 +674,12 @@ AUTHCHALLENGE_FAIL(nonzero_circid, d->cell->circ_id = 1337) static tor_x509_cert_t *mock_peer_cert = NULL; static tor_x509_cert_t * mock_get_peer_cert(tor_tls_t *tls) { (void)tls; return mock_peer_cert; return tor_x509_cert_dup(mock_peer_cert); } static int Loading Loading @@ -694,6 +713,7 @@ authenticate_data_cleanup(const struct testcase_t *test, void *arg) (void) test; UNMOCK(connection_or_write_var_cell_to_buf); UNMOCK(tor_tls_get_peer_cert); UNMOCK(tor_tls_get_own_cert); UNMOCK(tor_tls_get_tlssecrets); UNMOCK(connection_or_close_for_error); UNMOCK(channel_set_circid_type); Loading @@ -710,7 +730,10 @@ authenticate_data_cleanup(const struct testcase_t *test, void *arg) crypto_pk_free(d->key2); tor_free(d); } tor_x509_cert_free(mock_peer_cert); tor_x509_cert_free(mock_own_cert); mock_peer_cert = NULL; mock_own_cert = NULL; return 1; } Loading @@ -724,6 +747,7 @@ authenticate_data_setup(const struct testcase_t *test) MOCK(connection_or_write_var_cell_to_buf, mock_write_var_cell); MOCK(tor_tls_get_peer_cert, mock_get_peer_cert); MOCK(tor_tls_get_own_cert, mock_get_own_cert); MOCK(tor_tls_get_tlssecrets, mock_get_tlssecrets); MOCK(connection_or_close_for_error, mock_close_for_err); MOCK(channel_set_circid_type, mock_set_circid_type); Loading Loading @@ -773,6 +797,8 @@ authenticate_data_setup(const struct testcase_t *test) tor_x509_cert_get_der(link_cert, &der, &sz); mock_peer_cert = tor_x509_cert_decode(der, sz); tt_assert(mock_peer_cert); mock_own_cert = tor_x509_cert_decode(der, sz); tt_assert(mock_own_cert); tt_assert(! tor_tls_get_my_certs(0, &auth_cert, &id_cert)); tor_x509_cert_get_der(auth_cert, &der, &sz); d->c2->handshake_state->auth_cert = tor_x509_cert_decode(der, sz); Loading