r16409@catbus: nickm | 2007-11-05 10:38:25 -0500

 Split handshake state into its own structure.  Revise versions and netinfo code to use this structure.


svn:r12380

doc/TODO

@@ -24,11 +24,15 @@ Things we'd like to do in 0.2.0.x:
     D 118 if feasible and obvious
     D Maintain a skew estimate and use ftime consistently.
     - 105+TLS, if possible.
-      - Add a separate handshake structure that handles version negotiation,
+      o Add a separate handshake structure that handles version negotiation,
         and stores netinfo data until authentication is done.
-      - Revise versions and netinfo to use separate structure; make
+      o Revise versions and netinfo to use separate structure; make
         act-on-netinfo logic separate so it can get called _after_
         negotiation.
+      - Variable-length cells
+        - Add structure
+        - Add parse logic
+        - Make CERT and VERSIONS variable.
       - CERT cells
         - functions to parse x509 certs
         - functions to validate a single x509 cert against a TLS connection

src/or/command.c

@@ -396,12 +396,14 @@ command_process_versions_cell(cell_t *cell, or_connection_t *conn)
   int highest_supported_version = 0;
   const char *cp, *end;
   if (conn->link_proto != 0 ||
-      conn->_base.state != OR_CONN_STATE_WAITING_FOR_VERSIONS) {
+      conn->_base.state != OR_CONN_STATE_OR_HANDSHAKING ||
+      (conn->handshake_state && conn->handshake_state->received_versions)) {
     log_fn(LOG_PROTOCOL_WARN, LD_OR,
            "Received a VERSIONS cell on a connection with its version "
            "already set to %d; dropping", (int) conn->link_proto);
     return;
   }
+  tor_assert(conn->handshake_state);
   versionslen = ntohs(get_uint16(cell->payload));
   end = cell->payload + 2 + versionslen;
   if (end > cell->payload + CELL_PAYLOAD_SIZE)
@@ -416,12 +418,12 @@ command_process_versions_cell(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 or just break the connection?*/
+    /*XXXX020 just break the connection?*/
     conn->link_proto = 1;
     return;
   }
   conn->link_proto = highest_supported_version;
-  conn->_base.state = OR_CONN_STATE_OPEN;
+  conn->handshake_state->received_versions = 1;
 
   if (highest_supported_version >= 2)
     connection_or_send_netinfo(conn);
@@ -438,46 +440,42 @@ command_process_netinfo_cell(cell_t *cell, or_connection_t *conn)
   const char *cp, *end;
   uint8_t n_other_addrs;
   time_t now = time(NULL);
-
-  /*XXXX020 reject duplicate netinfos. */
-
-  if (conn->link_proto < 2 || conn->_base.state != OR_CONN_STATE_OPEN) {
+  if (conn->link_proto < 2) {
     log_fn(LOG_PROTOCOL_WARN, LD_OR,
            "Received a NETINFO cell on %s connection; dropping.",
            conn->link_proto == 0 ? "non-versioned" : "a v1");
     return;
   }
+  if (conn->_base.state != OR_CONN_STATE_OR_HANDSHAKING) {
+    log_fn(LOG_PROTOCOL_WARN, LD_OR,
+           "Received a NETINFO cell on a non-handshaking; dropping.");
+    return;
+  }
+  tor_assert(conn->handshake_state &&
+             conn->handshake_state->received_versions);
+  if (conn->handshake_state->received_netinfo) {
+    log_fn(LOG_PROTOCOL_WARN, LD_OR,
+           "Received a duplicate NETINFO cell; dropping.");
+    return;
+  }
   /* Decode the cell. */
   timestamp = ntohl(get_uint32(cell->payload));
+  if (abs(now - conn->handshake_state->sent_versions_at) < 180) {
+    conn->handshake_state->apparent_skew = now - timestamp;
+  }
+
   my_addr_type = (uint8_t) cell->payload[4];
   my_addr_len = (uint8_t) cell->payload[5];
   my_addr_ptr = cell->payload + 6;
-  /* Possibly learn my address. XXXX020 */
   end = cell->payload + CELL_PAYLOAD_SIZE;
   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! */
     return;
-  }
-
-  /*XXXX020 magic number 3600 */
-  if (abs(timestamp - now) > 3600 &&
-      router_get_by_digest(conn->identity_digest)) {
-    long delta = now - timestamp;
-    char dbuf[64];
-    /*XXXX020 not always warn!*/
-    format_time_interval(dbuf, sizeof(dbuf), delta);
-    log_fn(LOG_WARN, LD_HTTP, "Received NETINFO cell with skewed time from "
-           "server at %s:%d.  It seems that our clock is %s by %s, or "
-           "that theirs is %s. Tor requires an accurate clock to work: "
-           "please check your time and date settings.",
-           conn->_base.address, (int)conn->_base.port,
-           delta>0 ? "ahead" : "behind", dbuf,
-           delta>0 ? "behind" : "ahead");
-    control_event_general_status(LOG_WARN,
-                                 "CLOCK_SKEW SKEW=%ld SOURCE=OR:%s:%d",
-                                 delta, conn->_base.address, conn->_base.port);
+  } else if (my_addr_type == RESOLVED_TYPE_IPV4 && my_addr_len == 4) {
+    conn->handshake_state->my_apparent_addr = ntohl(get_uint32(my_addr_ptr));
   }
 
   n_other_addrs = (uint8_t) *cp++;
@@ -491,12 +489,50 @@ command_process_netinfo_cell(cell_t *cell, or_connection_t *conn)
     if (other_addr_type == RESOLVED_TYPE_IPV4 && other_addr_len == 4) {
       uint32_t addr = ntohl(get_uint32(cp));
       if (addr == conn->real_addr) {
-        conn->is_canonical = 1;
+        conn->handshake_state->apparently_canonical = 1;
         break;
       }
     }
     cp += other_addr_len;
     --n_other_addrs;
   }
+
+  conn->handshake_state->received_netinfo = 1;
 }
 
+/** DOCDOC Called when we're done authenticating; act on stuff we
+ * learned in netinfo. */
+void
+connection_or_act_on_netinfo(or_connection_t *conn)
+{
+  long delta;
+  if (!conn->handshake_state)
+    return;
+
+  tor_assert(conn->handshake_state->authenticated != 0);
+
+  delta = conn->handshake_state->apparent_skew;
+  /*XXXX020 magic number 3600 */
+  if (abs(delta) > 3600 &&
+      router_get_by_digest(conn->identity_digest)) {
+    char dbuf[64];
+    /*XXXX020 not always warn!*/
+    format_time_interval(dbuf, sizeof(dbuf), delta);
+    log_fn(LOG_WARN, LD_HTTP, "Received NETINFO cell with skewed time from "
+           "server at %s:%d.  It seems that our clock is %s by %s, or "
+           "that theirs is %s. Tor requires an accurate clock to work: "
+           "please check your time and date settings.",
+           conn->_base.address, (int)conn->_base.port,
+           delta>0 ? "ahead" : "behind", dbuf,
+           delta>0 ? "behind" : "ahead");
+    control_event_general_status(LOG_WARN,
+                                 "CLOCK_SKEW SKEW=%ld SOURCE=OR:%s:%d",
+                                 delta, conn->_base.address, conn->_base.port);
+  }
+
+  /* XXX020 possibly, learn my address from my_apparent_addr */
+
+  if (conn->handshake_state->apparently_canonical) {
+    conn->is_canonical = 1;
+  }
+}

src/or/connection.c

@@ -88,7 +88,8 @@ conn_state_to_string(int type, int state)
         case OR_CONN_STATE_CONNECTING: return "connect()ing";
         case OR_CONN_STATE_PROXY_FLUSHING: return "proxy flushing";
         case OR_CONN_STATE_PROXY_READING: return "proxy reading";
-        case OR_CONN_STATE_HANDSHAKING: return "handshaking";
+        case OR_CONN_STATE_TLS_HANDSHAKING: return "handshaking (TLS)";
+        case OR_CONN_STATE_OR_HANDSHAKING: return "handshaking (Tor)";
         case OR_CONN_STATE_OPEN: return "open";
       }
       break;
@@ -314,7 +315,10 @@ _connection_free(connection_t *conn)
       tor_tls_free(or_conn->tls);
       or_conn->tls = NULL;
     }
-
+    if (or_conn->handshake_state) {
+      or_handshake_state_free(or_conn->handshake_state);
+      or_conn->handshake_state = NULL;
+    }
     tor_free(or_conn->nickname);
   }
   if (CONN_IS_EDGE(conn)) {
@@ -1879,7 +1883,7 @@ connection_read_to_buf(connection_t *conn, int *max_to_read)
       conn->state > OR_CONN_STATE_PROXY_READING) {
     int pending;
     or_connection_t *or_conn = TO_OR_CONN(conn);
-    if (conn->state == OR_CONN_STATE_HANDSHAKING) {
+    if (conn->state == OR_CONN_STATE_TLS_HANDSHAKING) {
       /* continue handshaking even if global token bucket is empty */
       return connection_tls_continue_handshake(or_conn);
     }
@@ -2100,7 +2104,7 @@ connection_handle_write(connection_t *conn, int force)
   if (connection_speaks_cells(conn) &&
       conn->state > OR_CONN_STATE_PROXY_READING) {
     or_connection_t *or_conn = TO_OR_CONN(conn);
-    if (conn->state == OR_CONN_STATE_HANDSHAKING) {
+    if (conn->state == OR_CONN_STATE_TLS_HANDSHAKING) {
       connection_stop_writing(conn);
       if (connection_tls_continue_handshake(or_conn) < 0) {
         /* Don't flush; connection is dead. */

src/or/connection_or.c

@@ -524,7 +524,7 @@ connection_or_connect(uint32_t addr, uint16_t port, const char *id_digest)
 int
 connection_tls_start_handshake(or_connection_t *conn, int receiving)
 {
-  conn->_base.state = OR_CONN_STATE_HANDSHAKING;
+  conn->_base.state = OR_CONN_STATE_TLS_HANDSHAKING;
   conn->tls = tor_tls_new(conn->_base.s, receiving);
   if (!conn->tls) {
     log_warn(LD_BUG,"tor_tls_new failed. Closing.");
@@ -739,12 +739,22 @@ connection_tls_finish_handshake(or_connection_t *conn)
     conn->link_proto = 1;
     return connection_or_set_state_open(conn);
   } else {
-    /*XXXX020 actually, we'll need to send some kind of authentication. */
-    conn->_base.state = OR_CONN_STATE_WAITING_FOR_VERSIONS;
+    conn->_base.state = OR_CONN_STATE_OR_HANDSHAKING;
+    conn->handshake_state = tor_malloc_zero(sizeof(or_handshake_state_t));
     return connection_or_send_versions(conn);
   }
 }
 
+/** DOCDOC */
+void
+or_handshake_state_free(or_handshake_state_t *state)
+{
+  tor_assert(state);
+  if (state->signing_key)
+    crypto_free_pk_env(state->signing_key);
+  tor_free(state);
+}
+
 /**DOCDOC*/
 int
 connection_or_set_state_open(or_connection_t *conn)
@@ -858,6 +868,8 @@ connection_or_send_versions(or_connection_t *conn)
   uint8_t versions[] = { 1 };
   int n_versions = sizeof(versions) / sizeof(uint8_t);
   int i;
+  tor_assert(conn->handshake_state &&
+             !conn->handshake_state->sent_versions_at);
   memset(&cell, 0, sizeof(cell_t));
   cell.command = CELL_VERSIONS;
   set_uint16(cell.payload, htons(n_versions));
@@ -868,6 +880,7 @@ connection_or_send_versions(or_connection_t *conn)
   }
 
   connection_or_write_cell_to_buf(&cell, conn);
+  conn->handshake_state->sent_versions_at = time(NULL);
 
   return 0;
 }

src/or/or.h

@@ -235,10 +235,11 @@ typedef enum {
 /** State for a connection to an OR: waiting for proxy response. */
 #define OR_CONN_STATE_PROXY_READING 3
 /** State for a connection to an OR: SSL is handshaking, not done yet. */
-#define OR_CONN_STATE_HANDSHAKING 4
-/** State for a connection to an OR: We sent a VERSIONS cell and want one back
+#define OR_CONN_STATE_TLS_HANDSHAKING 4
+/** State for a connection to an OR: We're done with our SSL handshake, but we
+ * haven't yet negotiated link protocol versions and finished authenticating.
  */
-#define OR_CONN_STATE_WAITING_FOR_VERSIONS 5
+#define OR_CONN_STATE_OR_HANDSHAKING 5
 /** State for a connection to an OR: Ready to send/receive cells. */
 #define OR_CONN_STATE_OPEN 6
 #define _OR_CONN_STATE_MAX 6
@@ -857,6 +858,29 @@ typedef struct connection_t {
 
 } connection_t;
 
+/** DOCDOC */
+typedef struct or_handshake_state_t {
+  time_t sent_versions_at;
+  unsigned int received_versions : 1;
+  unsigned int received_netinfo : 1;
+  unsigned int received_certs : 1;
+  unsigned int authenticated : 1;
+
+  /* from tls */
+  char client_random[32];
+  char server_random[32];
+
+  /* from netinfo */
+  long apparent_skew;
+  uint32_t my_apparent_addr;
+  unsigned int apparently_canonical;
+
+  /* from certs */
+  char cert_id_digest[DIGEST_LEN];
+  crypto_pk_env_t *signing_key;
+
+} or_handshake_state_t;
+
 /** Subtype of connection_t for an "OR connection" -- that is, one that speaks
  * cells over TLS. */
 typedef struct or_connection_t {
@@ -878,13 +902,14 @@ typedef struct or_connection_t {
   circ_id_type_t circ_id_type:2; /**< When we send CREATE cells along this
                                   * connection, which half of the space should
                                   * we use? */
-  unsigned int is_canonical; /**< DOCDOC */
+  unsigned int is_canonical:1; /**< DOCDOC */
   uint8_t link_proto; /**< What protocol version are we using? 0 for
                        * "none negotiated yet." */
   uint16_t next_circ_id; /**< Which circ_id do we try to use next on
                           * this connection?  This is always in the
                           * range 0..1<<15-1. */
 
+  or_handshake_state_t *handshake_state;/**< DOCDOC */
   time_t timestamp_lastempty; /**< When was the outbuf last completely empty?*/
 
   /* bandwidth* and read_bucket only used by ORs in OPEN state: */
@@ -2517,6 +2542,7 @@ int connection_ap_handshake_attach_circuit(edge_connection_t *conn);
 /********************************* command.c ***************************/
 
 void command_process_cell(cell_t *cell, or_connection_t *conn);
+void 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;
@@ -2743,6 +2769,7 @@ or_connection_t *connection_or_connect(uint32_t addr, uint16_t port,
 int connection_tls_start_handshake(or_connection_t *conn, int receiving);
 int connection_tls_continue_handshake(or_connection_t *conn);
 
+void or_handshake_state_free(or_handshake_state_t *state);
 int connection_or_set_state_open(or_connection_t *conn);
 void connection_or_write_cell_to_buf(const cell_t *cell,
                                      or_connection_t *conn);