commit 34a6755b94015fcbc838b46b54667899c238ac04 Author: Nick Mathewson <nickm@torproject.org> Date: Thu Jun 1 09:26:24 2017 -0400 Fix ed25519 link certificate race on tls context rotation Whenever we rotate our TLS context, we change our Ed25519 Signing->Link certificate. But if we've already started a TLS connection, then we've already sent the old X509 link certificate, so the new Ed25519 Signing->Link certificate won't match it. To fix this, we now store a copy of the Signing->Link certificate when we initialize the handshake state, and send that certificate as part of our CERTS cell. Fixes one case of bug22460; bugfix on 0.3.0.1-alpha. --- changes/bug22460_case1 | 6 ++++++ src/or/connection_or.c | 6 +++++- src/or/or.h | 6 ++++++ src/test/test_link_handshake.c | 7 +++++++ 4 files changed, 24 insertions(+), 1 deletion(-) diff --git a/changes/bug22460_case1 b/changes/bug22460_case1 index 9aef46b..cfe78ad 100644 --- a/changes/bug22460_case1 +++ b/changes/bug22460_case1 @@ -6,5 +6,11 @@ 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. diff --git a/src/or/connection_or.c b/src/or/connection_or.c index cefe42c..0966ec8 100644 --- a/src/or/connection_or.c +++ b/src/or/connection_or.c @@ -1855,6 +1855,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; @@ -1869,6 +1872,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); } @@ -2311,7 +2315,7 @@ connection_or_send_certs_cell(or_connection_t *conn) if (conn_in_server_mode) { 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, diff --git a/src/or/or.h b/src/or/or.h index 0db9f23..50e6e3e 100644 --- a/src/or/or.h +++ b/src/or/or.h @@ -1449,6 +1449,12 @@ typedef struct or_handshake_state_t { /* True iff we have sent a netinfo cell */ unsigned int sent_netinfo : 1; + /** The signing->ed25519 link certificate corresponding to the x509 + * certificate we used on the TLS connection (if this is a server-side + * connection). We make a copy of this here to prevent a race condition + * caused by TLS context rotation. */ + struct tor_cert_st *own_link_cert; + /** True iff we should feed outgoing cells into digest_sent and * digest_received respectively. * diff --git a/src/test/test_link_handshake.c b/src/test/test_link_handshake.c index 421f3aa..d1b9c65 100644 --- a/src/test/test_link_handshake.c +++ b/src/test/test_link_handshake.c @@ -892,6 +892,11 @@ test_link_handshake_send_authchallenge(void *arg) or_connection_t *c1 = or_connection_new(CONN_TYPE_OR, AF_INET); var_cell_t *cell1=NULL, *cell2=NULL; + crypto_pk_t *rsa0 = pk_generate(0), *rsa1 = pk_generate(1); + tt_int_op(tor_tls_context_init(TOR_TLS_CTX_IS_PUBLIC_SERVER, + rsa0, rsa1, 86400), ==, 0); + init_mock_ed_keys(rsa0); + MOCK(connection_or_write_var_cell_to_buf, mock_write_var_cell); tt_int_op(connection_init_or_handshake_state(c1, 0), ==, 0); @@ -917,6 +922,8 @@ test_link_handshake_send_authchallenge(void *arg) connection_free_(TO_CONN(c1)); tor_free(cell1); tor_free(cell2); + crypto_pk_free(rsa0); + crypto_pk_free(rsa1); } typedef struct authchallenge_data_s {