r16669@catbus: nickm | 2007-11-14 14:50:03 -0500

 When we complete an OR handshake, set up all the internal fields and mark the connection as open.


svn:r12495

doc/TODO

@@ -50,11 +50,10 @@ Things we'd like to do in 0.2.0.x:
         o Generate CERT cells
         o Keep copies of X509 certs around, not necessarily associated with
           connection.
-      . LINK_AUTH cells
+      o LINK_AUTH cells
         o Code to generate
           o Remember certificate digests from TLS
         o Code to parse and check
-        - Unit tests
       - Revised handshake: TLS
         - Server checks for new cipher types, and if it finds them, sends
           only one cert and does not ask for client certs.
@@ -65,9 +64,9 @@ Things we'd like to do in 0.2.0.x:
         o If in 'handshaking' state (since v2+ conn is in use), accept
           VERSIONS and NETINFO and CERT and LINK_AUTH.
         o After we send NETINFO, send CERT and LINK_AUTH if needed.
-        - Once we get a good LINK_AUTH, the connection is OPEN.
+        o Once we get a good LINK_AUTH, the connection is OPEN.
         - Ban most cell types on a non-OPEN connection.
-        - Close connections on handshake failure.
+        o Close connections on handshake failure.
       o Make code work right wrt TLS context rotation.
       - NETINFO fallout
         - Don't extend a circuit over a noncanonical connection with

src/common/tortls.c

@@ -933,6 +933,7 @@ tor_tls_verify_certs_v2(int severity, tor_tls_t *tls,
                         const char *id_cert_str, size_t id_cert_len,
                         crypto_pk_env_t **cert_key_out,
                         char *conn_cert_digest_out,
+                        crypto_pk_env_t **id_key_out,
                         char *id_digest_out)
 {
   X509 *cert = NULL, *id_cert = NULL;
@@ -942,6 +943,7 @@ tor_tls_verify_certs_v2(int severity, tor_tls_t *tls,
 
   tor_assert(cert_key_out);
   tor_assert(conn_cert_digest_out);
+  tor_assert(id_key_out);
   tor_assert(id_digest_out);
 
   *cert_key_out = NULL;
@@ -996,13 +998,9 @@ tor_tls_verify_certs_v2(int severity, tor_tls_t *tls,
     goto done;
   }
 
-  {
-    crypto_pk_env_t *i = _crypto_new_pk_env_evp_pkey(id_pkey);
-    if (!i)
-      goto done;
-    crypto_pk_get_digest(i, id_digest_out);
-    crypto_free_pk_env(i);
-  }
+  if (!(*id_key_out = _crypto_new_pk_env_evp_pkey(id_pkey)))
+    goto done;
+  crypto_pk_get_digest(*id_key_out, id_digest_out);
   if (!(cert_pkey = X509_get_pubkey(cert)))
     goto done;
   if (!(*cert_key_out = _crypto_new_pk_env_evp_pkey(cert_pkey)))

src/common/tortls.h

@@ -67,6 +67,7 @@ int tor_tls_verify_certs_v2(int severity, tor_tls_t *tls,
                             const char *id_cert_str, size_t id_cert_len,
                             crypto_pk_env_t **cert_key_out,
                             char *conn_cert_digest_out,
+                            crypto_pk_env_t **id_key_out,
                             char *id_digest_out);
 int tor_tls_check_lifetime(tor_tls_t *tls, int tolerance);
 int tor_tls_read(tor_tls_t *tls, char *cp, size_t len);

src/or/command.c

@@ -475,18 +475,20 @@ command_process_versions_cell(var_cell_t *cell, or_connection_t *conn)
   }
   if (!highest_supported_version) {
     log_fn(LOG_PROTOCOL_WARN, LD_OR,
-           "Couldn't find a version in common; defaulting to v1.");
-    /*XXXX020 just break the connection! */
-    conn->link_proto = 1;
+           "Couldn't find a version in common between my version list and the "
+           "list in the VERSIONS cell; closing connection.");
+    connection_mark_for_close(TO_CONN(conn));
     return;
   }
   conn->link_proto = highest_supported_version;
   conn->handshake_state->received_versions = 1;
 
   if (highest_supported_version >= 2) {
-    /*XXXX020 check return values. */
-    connection_or_send_netinfo(conn);
-    connection_or_send_cert(conn);
+    if (connection_or_send_netinfo(conn) < 0 ||
+        connection_or_send_cert(conn) < 0) {
+      connection_mark_for_close(TO_CONN(conn));
+      return;
+    }
     if (conn->handshake_state->started_here)
       connection_or_send_link_auth(conn);
   } else {
@@ -536,8 +538,8 @@ command_process_netinfo_cell(cell_t *cell, or_connection_t *conn)
   cp = cell->payload + 6 + my_addr_len;
   if (cp >= end) {
     log_fn(LOG_PROTOCOL_WARN, LD_OR,
-           "Address too long in netinfo cell; dropping.");
-    /*XXXX020 reject and break OR conn! */
+           "Addresses too long in netinfo cell; closing connection.");
+    connection_mark_for_close(TO_CONN(conn));
     return;
   } else if (my_addr_type == RESOLVED_TYPE_IPV4 && my_addr_len == 4) {
     conn->handshake_state->my_apparent_addr = ntohl(get_uint32(my_addr_ptr));
@@ -549,8 +551,12 @@ command_process_netinfo_cell(cell_t *cell, or_connection_t *conn)
      * "canonical." */
     uint8_t other_addr_type = (uint8_t) *cp++;
     uint8_t other_addr_len = (uint8_t) *cp++;
-    if (cp + other_addr_len >= end)
-      break; /*XXXX020 protocol warn. */
+    if (cp + other_addr_len >= end) {
+      log_fn(LOG_PROTOCOL_WARN, LD_OR,
+             "Address too long in netinfo cell; closing connection.");
+      connection_mark_for_close(TO_CONN(conn));
+      return;
+    }
     if (other_addr_type == RESOLVED_TYPE_IPV4 && other_addr_len == 4) {
       uint32_t addr = ntohl(get_uint32(cp));
       if (addr == conn->real_addr) {
@@ -568,12 +574,12 @@ command_process_netinfo_cell(cell_t *cell, or_connection_t *conn)
 /*XXXX020 move to connection_or.c */
 /** DOCDOC Called when we're done authenticating; act on stuff we
  * learned in netinfo. */
-void
+int
 connection_or_act_on_netinfo(or_connection_t *conn)
 {
   long delta;
   if (!conn->handshake_state)
-    return;
+    return -1;
 
   tor_assert(conn->handshake_state->authenticated != 0);
 
@@ -601,6 +607,8 @@ connection_or_act_on_netinfo(or_connection_t *conn)
   if (conn->handshake_state->apparently_canonical) {
     conn->is_canonical = 1;
   }
+
+  return 0;
 }
 
 /*DOCDOC*/
@@ -611,16 +619,25 @@ command_process_cert_cell(var_cell_t *cell, or_connection_t *conn)
   uint16_t conn_cert_len = 0, id_cert_len = 0;
   const char *conn_cert = NULL, *id_cert = NULL;
   const char *cp, *end;
-  int authenticated = 0;
+  int done = 0;
 
-  /*XXXX020 log messages*/
-  if (conn->_base.state != OR_CONN_STATE_OR_HANDSHAKING)
-    goto err;
+  if (conn->_base.state != OR_CONN_STATE_OR_HANDSHAKING) {
+    log_fn(LOG_PROTOCOL_WARN, LD_OR, "Got CERT cell when not handshaking. "
+           "Ignoring.");
+    return;
+  }
   tor_assert(conn->handshake_state);
   if (!conn->handshake_state->received_versions ||
-      !conn->handshake_state->received_netinfo ||
-      conn->handshake_state->received_certs)
+      !conn->handshake_state->received_netinfo) {
+    log_fn(LOG_PROTOCOL_WARN, LD_OR, "Got CERT cell before VERSIONS and "
+           "NETINFO. Closing the connection.");
+    goto err;
+  }
+  if (conn->handshake_state->received_certs) {
+    log_fn(LOG_PROTOCOL_WARN, LD_OR, "Got duplicate CERT cell. "
+           "Closing the connection.");
     goto err;
+  }
 
   cp = cell->payload;
   end = cell->payload + cell->payload_len;
@@ -651,6 +668,7 @@ command_process_cert_cell(var_cell_t *cell, or_connection_t *conn)
   /* Now we have 0, 1, or 2 certs. */
   if (n_certs == 0) {
     /* The other side is unauthenticated. */
+    done = 1;
   } else {
     int r;
     r = tor_tls_verify_certs_v2(LOG_PROTOCOL_WARN, conn->tls,
@@ -660,23 +678,27 @@ command_process_cert_cell(var_cell_t *cell, or_connection_t *conn)
                                 (conn->handshake_state->started_here ?
                                  conn->handshake_state->server_cert_digest :
                                  conn->handshake_state->client_cert_digest),
+                                &conn->handshake_state->identity_key,
                                 conn->handshake_state->cert_id_digest);
     if (r < 0)
       goto err;
-    if (r == 1)
-      authenticated = 1;
+    if (r == 1) {
+      done = 1;
+      conn->handshake_state->authenticated = 1;
+    }
   }
 
   conn->handshake_state->received_certs = 1;
-  if (authenticated) {
-    /* XXXX020 make the connection open. */
+  if (done) {
+    if (connection_or_finish_or_handshake(conn) < 0)
+      goto err;
   }
   if (! conn->handshake_state->signing_key)
     goto err;
 
   return;
  err:
-  /*XXXX020 close the connection */;
+  connection_mark_for_close(TO_CONN(conn));
 }
 
 #define LINK_AUTH_STRING "Tor initiator certificate verification"
@@ -746,11 +768,12 @@ command_process_link_auth_cell(cell_t *cell, or_connection_t *conn)
     goto err;
   }
 
-  /* Okay, we're authenticated. */
   s->authenticated = 1;
 
-  /* XXXX020 act on being authenticated: Open the connection. */
+  if (connection_or_finish_or_handshake(conn)<0)
+    goto err;
 
+  tor_free(checked);
   return;
  err:
   tor_free(checked);

src/or/connection_or.c

@@ -614,6 +614,8 @@ connection_or_nonopen_was_started_here(or_connection_t *conn)
   tor_assert(conn->_base.type == CONN_TYPE_OR);
   if (!conn->tls)
     return 1; /* it's still in proxy states or something */
+  if (conn->handshake_state)
+    return conn->handshake_state->started_here;
   return !tor_tls_is_server(conn->tls);
 }
 
@@ -651,8 +653,15 @@ connection_or_check_valid_tls_handshake(or_connection_t *conn,
     started_here ? conn->_base.address : safe_str(conn->_base.address);
   const char *conn_type = started_here ? "outgoing" : "incoming";
   int has_cert = 0, has_identity = 0;
+  int v1 = (conn->link_proto == 1);
 
   check_no_tls_errors();
+  if (v1) {
+    has_cert = tor_tls_peer_has_cert(conn->tls);
+  } else {
+    tor_assert(conn->handshake_state);
+    has_cert = !tor_digest_is_zero(conn->handshake_state->cert_id_digest);
+  }
   has_cert = tor_tls_peer_has_cert(conn->tls);
   if (started_here && !has_cert) {
     log_info(LD_PROTOCOL,"Tried connecting to router at %s:%d, but it didn't "
@@ -665,28 +674,34 @@ connection_or_check_valid_tls_handshake(or_connection_t *conn,
   }
   check_no_tls_errors();
 
-  if (has_cert) {
-    int v = tor_tls_verify_v1(started_here?severity:LOG_INFO,
-                              conn->tls, &identity_rcvd);
-    if (started_here && v<0) {
-      log_fn(severity,LD_OR,"Tried connecting to router at %s:%d: It"
-             " has a cert but it's invalid. Closing.",
-             safe_address, conn->_base.port);
-      return -1;
-    } else if (v<0) {
-      log_info(LD_PROTOCOL,"Incoming connection gave us an invalid cert "
-               "chain; ignoring.");
-    } else {
-      log_debug(LD_OR,"The certificate seems to be valid on %s connection "
-                "with %s:%d", conn_type, safe_address, conn->_base.port);
+  if (v1) {
+    if (has_cert) {
+      int v = tor_tls_verify_v1(started_here?severity:LOG_INFO,
+                                conn->tls, &identity_rcvd);
+      if (started_here && v<0) {
+        log_fn(severity,LD_OR,"Tried connecting to router at %s:%d: It"
+               " has a cert but it's invalid. Closing.",
+               safe_address, conn->_base.port);
+        return -1;
+      } else if (v<0) {
+        log_info(LD_PROTOCOL,"Incoming connection gave us an invalid cert "
+                 "chain; ignoring.");
+      } else {
+        log_debug(LD_OR,"The certificate seems to be valid on %s connection "
+                  "with %s:%d", conn_type, safe_address, conn->_base.port);
+      }
+      check_no_tls_errors();
+    }
+  } else {
+    if (conn->handshake_state->authenticated &&
+        conn->handshake_state->identity_key) {
+      identity_rcvd = crypto_pk_dup_key(conn->handshake_state->identity_key);
     }
-    check_no_tls_errors();
   }
 
   if (identity_rcvd) {
     has_identity=1;
     crypto_pk_get_digest(identity_rcvd, digest_rcvd_out);
-
     if (crypto_pk_cmp_keys(get_identity_key(), identity_rcvd)<0) {
       conn->circ_id_type = CIRC_ID_TYPE_LOWER;
     } else {
@@ -744,6 +759,29 @@ connection_or_check_valid_tls_handshake(or_connection_t *conn,
   return 0;
 }
 
+/** DOCDOC */
+int
+connection_or_finish_or_handshake(or_connection_t *conn)
+{
+  char id_digest[DIGEST_LEN];
+  tor_assert(conn);
+  tor_assert(conn->handshake_state);
+  tor_assert(conn->link_proto >= 2);
+  tor_assert(conn->handshake_state->received_versions != 0);
+  tor_assert(conn->handshake_state->received_netinfo != 0);
+  tor_assert(conn->handshake_state->received_certs != 0);
+
+  if (connection_or_check_valid_tls_handshake(conn,
+                                  conn->handshake_state->started_here,
+                                              id_digest) < 0)
+    return -1;
+  connection_or_init_conn_from_address(conn, conn->_base.addr,
+                                       conn->_base.port, id_digest, 0);
+  if (connection_or_act_on_netinfo(conn)<0)
+    return -1;
+  return connection_or_set_state_open(conn);
+}
+
 /** The tls handshake is finished.
  *
  * Make sure we are happy with the person we just handshaked with.
@@ -815,6 +853,8 @@ or_handshake_state_free(or_handshake_state_t *state)
   tor_assert(state);
   if (state->signing_key)
     crypto_free_pk_env(state->signing_key);
+  if (state->identity_key)
+    crypto_free_pk_env(state->identity_key);
   memset(state, 0xBE, sizeof(or_handshake_state_t));
   tor_free(state);
 }
@@ -836,6 +876,10 @@ connection_or_set_state_open(or_connection_t *conn)
     }
     router_set_status(conn->identity_digest, 1);
   }
+  if (conn->handshake_state) {
+    or_handshake_state_free(conn->handshake_state);
+    conn->handshake_state = NULL;
+  }
   connection_watch_events(TO_CONN(conn), EV_READ);
   circuit_n_conn_done(conn, 1); /* send the pending creates, if any. */
 
@@ -1120,8 +1164,6 @@ connection_or_send_link_auth(or_connection_t *conn)
 
   connection_or_write_cell_to_buf(&cell, conn);
 
-  /* XXXX020 at this point, as a client, we can consider ourself
-   * authenticated. */
   return 0;
 }
 

src/or/or.h

@@ -894,6 +894,7 @@ typedef struct or_handshake_state_t {
   /* from certs */
   char cert_id_digest[DIGEST_LEN];
   crypto_pk_env_t *signing_key;
+  crypto_pk_env_t *identity_key;
 } or_handshake_state_t;
 
 /** Subtype of connection_t for an "OR connection" -- that is, one that speaks
@@ -2561,7 +2562,7 @@ int connection_ap_handshake_attach_circuit(edge_connection_t *conn);
 
 void command_process_cell(cell_t *cell, or_connection_t *conn);
 void command_process_var_cell(var_cell_t *cell, or_connection_t *conn);
-void connection_or_act_on_netinfo(or_connection_t *conn);
+int connection_or_act_on_netinfo(or_connection_t *conn);
 
 extern uint64_t stats_n_padding_cells_processed;
 extern uint64_t stats_n_create_cells_processed;
@@ -2781,6 +2782,7 @@ int connection_or_process_inbuf(or_connection_t *conn);
 int connection_or_flushed_some(or_connection_t *conn);
 int connection_or_finished_flushing(or_connection_t *conn);
 int connection_or_finished_connecting(or_connection_t *conn);
+int connection_or_finish_or_handshake(or_connection_t *conn);
 
 or_connection_t *connection_or_connect(uint32_t addr, uint16_t port,
                                     const char *id_digest);