Browse Source

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.
Nick Mathewson 6 years ago
parent
commit
34a6755b94
4 changed files with 24 additions and 1 deletions
  1. 6 0
      changes/bug22460_case1
  2. 5 1
      src/or/connection_or.c
  3. 6 0
      src/or/or.h
  4. 7 0
      src/test/test_link_handshake.c

+ 6 - 0
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.
 
 

+ 5 - 1
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,

+ 6 - 0
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.
    *

+ 7 - 0
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 {