Loading changes/bug22460_case1 0 → 100644 +16 −0 Original line number Diff line number Diff line o Major bugfixes (relays, key management): - Regenerate link and authentication certificates whenever the key that signs them changes; also, regenerate link certificates whenever the signed key changes. Previously, these processes were only weakly coupled, and we relays could (for minutes to hours) wind up with an inconsistent set of keys and certificates, which other relays would not accept. Fixes two cases of bug 22460; bugfix on 0.3.0.1-alpha. - When sending an Ed25519 signing->link certificate in a CERTS cell, send the certificate that matches the x509 certificate that we used on the TLS connection. Previously, there was a race condition if the TLS context rotated after we began the TLS handshake but before we sent the CERTS cell. Fixes a case of bug 22460; bugfix on 0.3.0.1-alpha. 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 +24 −3 Original line number Diff line number Diff line Loading @@ -705,11 +705,13 @@ tor_x509_cert_new,(X509 *x509_cert)) return cert; } /** Return a copy of <b>cert</b> */ /** Return a new copy of <b>cert</b>. */ tor_x509_cert_t * tor_x509_cert_dup(const tor_x509_cert_t *cert) { return tor_x509_cert_new(X509_dup(cert->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>, Loading Loading @@ -2047,7 +2049,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 @@ -2059,6 +2062,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 +1 −0 Original line number Diff line number Diff line Loading @@ -219,6 +219,7 @@ 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); 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, time_t now, Loading src/or/connection_or.c +34 −17 Original line number Diff line number Diff line Loading @@ -1838,6 +1838,9 @@ connection_init_or_handshake_state(or_connection_t *conn, int started_here) s->started_here = started_here ? 1 : 0; s->digest_sent_data = 1; s->digest_received_data = 1; if (! started_here && get_current_link_cert_cert()) { s->own_link_cert = tor_cert_dup(get_current_link_cert_cert()); } s->certs = or_handshake_certs_new(); s->certs->started_here = s->started_here; return 0; Loading @@ -1852,6 +1855,7 @@ or_handshake_state_free(or_handshake_state_t *state) crypto_digest_free(state->digest_sent); crypto_digest_free(state->digest_received); or_handshake_certs_free(state->certs); tor_cert_free(state->own_link_cert); memwipe(state, 0xBE, sizeof(or_handshake_state_t)); tor_free(state); } Loading Loading @@ -2228,7 +2232,8 @@ add_certs_cell_cert_helper(certs_cell_t *certs_cell, /** Add an encoded X509 cert (stored as <b>cert_len</b> bytes at * <b>cert_encoded</b>) to the trunnel certs_cell_t object that we are * building in <b>certs_cell</b>. Set its type field to <b>cert_type</b>. */ * building in <b>certs_cell</b>. Set its type field to <b>cert_type</b>. * (If <b>cert</b> is NULL, take no action.) */ static void add_x509_cert(certs_cell_t *certs_cell, uint8_t cert_type, Loading @@ -2246,7 +2251,7 @@ add_x509_cert(certs_cell_t *certs_cell, /** Add an Ed25519 cert from <b>cert</b> to the trunnel certs_cell_t object * that we are building in <b>certs_cell</b>. Set its type field to * <b>cert_type</b>. */ * <b>cert_type</b>. (If <b>cert</b> is NULL, take no action.) */ static void add_ed25519_cert(certs_cell_t *certs_cell, uint8_t cert_type, Loading @@ -2259,12 +2264,19 @@ add_ed25519_cert(certs_cell_t *certs_cell, cert->encoded, cert->encoded_len); } #ifdef TOR_UNIT_TESTS int certs_cell_ed25519_disabled_for_testing = 0; #else #define certs_cell_ed25519_disabled_for_testing 0 #endif /** Send a CERTS cell on the connection <b>conn</b>. Return 0 on success, -1 * on failure. */ 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; tor_x509_cert_t *own_link_cert = NULL; var_cell_t *cell; certs_cell_t *certs_cell = NULL; Loading @@ -2277,21 +2289,26 @@ connection_or_send_certs_cell(or_connection_t *conn) const int conn_in_server_mode = ! conn->handshake_state->started_here; /* Get the encoded values of the X509 certificates */ 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_assert(link_cert); if (conn_in_server_mode) { own_link_cert = tor_tls_get_own_cert(conn->tls); } tor_assert(id_cert); certs_cell = certs_cell_new(); /* Start adding certs. First the link cert or auth1024 cert. */ if (conn_in_server_mode) { tor_assert_nonfatal(own_link_cert); add_x509_cert(certs_cell, OR_CERT_TYPE_TLS_LINK, link_cert); OR_CERT_TYPE_TLS_LINK, own_link_cert); } else { tor_assert(global_link_cert); add_x509_cert(certs_cell, OR_CERT_TYPE_AUTH_1024, link_cert); OR_CERT_TYPE_AUTH_1024, global_link_cert); } /* Next the RSA->RSA ID cert */ Loading @@ -2303,9 +2320,11 @@ connection_or_send_certs_cell(or_connection_t *conn) CERTTYPE_ED_ID_SIGN, get_master_signing_key_cert()); if (conn_in_server_mode) { tor_assert_nonfatal(conn->handshake_state->own_link_cert || certs_cell_ed25519_disabled_for_testing); add_ed25519_cert(certs_cell, CERTTYPE_ED_SIGN_LINK, get_current_link_cert_cert()); conn->handshake_state->own_link_cert); } else { add_ed25519_cert(certs_cell, CERTTYPE_ED_SIGN_AUTH, Loading Loading @@ -2338,6 +2357,7 @@ connection_or_send_certs_cell(or_connection_t *conn) connection_or_write_var_cell_to_buf(cell, conn); var_cell_free(cell); certs_cell_free(certs_cell); tor_x509_cert_free(own_link_cert); return 0; } Loading Loading @@ -2478,10 +2498,10 @@ connection_or_compute_authenticate_cell_body(or_connection_t *conn, memcpy(auth1_getarray_type(auth), authtype_str, 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 @@ -2536,13 +2556,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 %s data.", Loading @@ -2553,8 +2571,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 Loading
changes/bug22460_case1 0 → 100644 +16 −0 Original line number Diff line number Diff line o Major bugfixes (relays, key management): - Regenerate link and authentication certificates whenever the key that signs them changes; also, regenerate link certificates whenever the signed key changes. Previously, these processes were only weakly coupled, and we relays could (for minutes to hours) wind up with an inconsistent set of keys and certificates, which other relays would not accept. Fixes two cases of bug 22460; bugfix on 0.3.0.1-alpha. - When sending an Ed25519 signing->link certificate in a CERTS cell, send the certificate that matches the x509 certificate that we used on the TLS connection. Previously, there was a race condition if the TLS context rotated after we began the TLS handshake but before we sent the CERTS cell. Fixes a case of bug 22460; bugfix on 0.3.0.1-alpha.
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 +24 −3 Original line number Diff line number Diff line Loading @@ -705,11 +705,13 @@ tor_x509_cert_new,(X509 *x509_cert)) return cert; } /** Return a copy of <b>cert</b> */ /** Return a new copy of <b>cert</b>. */ tor_x509_cert_t * tor_x509_cert_dup(const tor_x509_cert_t *cert) { return tor_x509_cert_new(X509_dup(cert->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>, Loading Loading @@ -2047,7 +2049,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 @@ -2059,6 +2062,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 +1 −0 Original line number Diff line number Diff line Loading @@ -219,6 +219,7 @@ 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); 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, time_t now, Loading
src/or/connection_or.c +34 −17 Original line number Diff line number Diff line Loading @@ -1838,6 +1838,9 @@ connection_init_or_handshake_state(or_connection_t *conn, int started_here) s->started_here = started_here ? 1 : 0; s->digest_sent_data = 1; s->digest_received_data = 1; if (! started_here && get_current_link_cert_cert()) { s->own_link_cert = tor_cert_dup(get_current_link_cert_cert()); } s->certs = or_handshake_certs_new(); s->certs->started_here = s->started_here; return 0; Loading @@ -1852,6 +1855,7 @@ or_handshake_state_free(or_handshake_state_t *state) crypto_digest_free(state->digest_sent); crypto_digest_free(state->digest_received); or_handshake_certs_free(state->certs); tor_cert_free(state->own_link_cert); memwipe(state, 0xBE, sizeof(or_handshake_state_t)); tor_free(state); } Loading Loading @@ -2228,7 +2232,8 @@ add_certs_cell_cert_helper(certs_cell_t *certs_cell, /** Add an encoded X509 cert (stored as <b>cert_len</b> bytes at * <b>cert_encoded</b>) to the trunnel certs_cell_t object that we are * building in <b>certs_cell</b>. Set its type field to <b>cert_type</b>. */ * building in <b>certs_cell</b>. Set its type field to <b>cert_type</b>. * (If <b>cert</b> is NULL, take no action.) */ static void add_x509_cert(certs_cell_t *certs_cell, uint8_t cert_type, Loading @@ -2246,7 +2251,7 @@ add_x509_cert(certs_cell_t *certs_cell, /** Add an Ed25519 cert from <b>cert</b> to the trunnel certs_cell_t object * that we are building in <b>certs_cell</b>. Set its type field to * <b>cert_type</b>. */ * <b>cert_type</b>. (If <b>cert</b> is NULL, take no action.) */ static void add_ed25519_cert(certs_cell_t *certs_cell, uint8_t cert_type, Loading @@ -2259,12 +2264,19 @@ add_ed25519_cert(certs_cell_t *certs_cell, cert->encoded, cert->encoded_len); } #ifdef TOR_UNIT_TESTS int certs_cell_ed25519_disabled_for_testing = 0; #else #define certs_cell_ed25519_disabled_for_testing 0 #endif /** Send a CERTS cell on the connection <b>conn</b>. Return 0 on success, -1 * on failure. */ 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; tor_x509_cert_t *own_link_cert = NULL; var_cell_t *cell; certs_cell_t *certs_cell = NULL; Loading @@ -2277,21 +2289,26 @@ connection_or_send_certs_cell(or_connection_t *conn) const int conn_in_server_mode = ! conn->handshake_state->started_here; /* Get the encoded values of the X509 certificates */ 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_assert(link_cert); if (conn_in_server_mode) { own_link_cert = tor_tls_get_own_cert(conn->tls); } tor_assert(id_cert); certs_cell = certs_cell_new(); /* Start adding certs. First the link cert or auth1024 cert. */ if (conn_in_server_mode) { tor_assert_nonfatal(own_link_cert); add_x509_cert(certs_cell, OR_CERT_TYPE_TLS_LINK, link_cert); OR_CERT_TYPE_TLS_LINK, own_link_cert); } else { tor_assert(global_link_cert); add_x509_cert(certs_cell, OR_CERT_TYPE_AUTH_1024, link_cert); OR_CERT_TYPE_AUTH_1024, global_link_cert); } /* Next the RSA->RSA ID cert */ Loading @@ -2303,9 +2320,11 @@ connection_or_send_certs_cell(or_connection_t *conn) CERTTYPE_ED_ID_SIGN, get_master_signing_key_cert()); if (conn_in_server_mode) { tor_assert_nonfatal(conn->handshake_state->own_link_cert || certs_cell_ed25519_disabled_for_testing); add_ed25519_cert(certs_cell, CERTTYPE_ED_SIGN_LINK, get_current_link_cert_cert()); conn->handshake_state->own_link_cert); } else { add_ed25519_cert(certs_cell, CERTTYPE_ED_SIGN_AUTH, Loading Loading @@ -2338,6 +2357,7 @@ connection_or_send_certs_cell(or_connection_t *conn) connection_or_write_var_cell_to_buf(cell, conn); var_cell_free(cell); certs_cell_free(certs_cell); tor_x509_cert_free(own_link_cert); return 0; } Loading Loading @@ -2478,10 +2498,10 @@ connection_or_compute_authenticate_cell_body(or_connection_t *conn, memcpy(auth1_getarray_type(auth), authtype_str, 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 @@ -2536,13 +2556,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 %s data.", Loading @@ -2553,8 +2571,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