[tor-commits] [tor/master] Fix ed25519 link certificate race on tls context rotation

[nickm at torproject.org]

commit 34a6755b94015fcbc838b46b54667899c238ac04
Author: Nick Mathewson <nickm at 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 {