Cell types and states for new OR handshake

Also, define all commands > 128 as variable-length when using
v3 or later link protocol.  Running into a var cell with an
unrecognized type is no longer a bug.

5 files changed, 61 insertions, 35 deletions

diff --git a/src/or/buffers.c b/src/or/buffers.c
index 1025cedad..c589fbaef 100644
--- a/src/or/buffers.c
+++ b/src/or/buffers.c
@@ -1005,6 +1005,32 @@ fetch_from_buf(char *string, size_t string_len, buf_t *buf)
   return (int)buf->datalen;
 }
 
+/** True iff the cell command <b>command</b> is one that implies a variable-length
+ * cell in Tor link protocol <b>linkproto</b>. */
+static inline int
+cell_command_is_var_length(uint8_t command, int linkproto)
+{
+  /* If linkproto is v2 (2), CELL_VERSIONS is the only variable-length cells work as
+   * implemented here. If it's 1, there are no variable-length cells.  Tor
+   * does not support other versions right now, and so can't negotiate them.
+   */
+  switch (linkproto) {
+  case 1:
+    /* Link protocol version 1 has no variable-length cells. */
+    return 0;
+  case 2:
+    /* In link protocol version 2, VERSIONS is the only variable-length cell */
+    return command == CELL_VERSIONS;
+  case 0:
+  case 3:
+  default:
+    /* In link protocol version 3 and later, and in version "unknown",
+     * commands 128 and higher indicate variable-length. VERSIONS is
+     * grandfathered in. */
+    return command == CELL_VERSIONS || command >= 128;
+  }
+}
+
 /** Check <b>buf</b> for a variable-length cell according to the rules of link
  * protocol version <b>linkproto</b>.  If one is found, pull it off the buffer
  * and assign a newly allocated var_cell_t to *<b>out</b>, and return 1.
@@ -1019,12 +1045,6 @@ fetch_var_cell_from_buf(buf_t *buf, var_cell_t **out, int linkproto)
   var_cell_t *result;
   uint8_t command;
   uint16_t length;
-  /* If linkproto is unknown (0) or v2 (2), variable-length cells work as
-   * implemented here. If it's 1, there are no variable-length cells.  Tor
-   * does not support other versions right now, and so can't negotiate them.
-   */
-  if (linkproto == 1)
-    return 0;
   check();
   *out = NULL;
   if (buf->datalen < VAR_CELL_HEADER_SIZE)
@@ -1032,7 +1052,7 @@ fetch_var_cell_from_buf(buf_t *buf, var_cell_t **out, int linkproto)
   peek_from_buf(hdr, sizeof(hdr), buf);
 
   command = get_uint8(hdr+2);
-  if (!(CELL_COMMAND_IS_VAR_LENGTH(command)))
+  if (!(cell_command_is_var_length(command, linkproto)))
     return 0;
 
   length = ntohs(get_uint16(hdr+3));
@@ -1101,8 +1121,6 @@ fetch_var_cell_from_evbuffer(struct evbuffer *buf, var_cell_t **out,
   uint16_t cell_length;
   var_cell_t *cell;
   int result = 0;
-  if (linkproto == 1)
-    return 0;
 
   *out = NULL;
   buf_len = evbuffer_get_length(buf);
@@ -1113,7 +1131,7 @@ fetch_var_cell_from_evbuffer(struct evbuffer *buf, var_cell_t **out,
   tor_assert(n >= VAR_CELL_HEADER_SIZE);
 
   command = get_uint8(hdr+2);
-  if (!(CELL_COMMAND_IS_VAR_LENGTH(command))) {
+  if (!(cell_command_is_var_length(command, linkproto))) {
     goto done;
   }
 
diff --git a/src/or/command.c b/src/or/command.c
index d24373eec..72d8cd7cf 100644
--- a/src/or/command.c
+++ b/src/or/command.c
@@ -93,7 +93,7 @@ command_time_process_cell(cell_t *cell, or_connection_t *conn, int *time,
 void
 command_process_cell(cell_t *cell, or_connection_t *conn)
 {
-  int handshaking = (conn->_base.state == OR_CONN_STATE_OR_HANDSHAKING);
+  int handshaking = (conn->_base.state == OR_CONN_STATE_OR_HANDSHAKING_V2);
 #ifdef KEEP_TIMING_STATS
   /* how many of each cell have we seen so far this second? needs better
    * name. */
@@ -207,7 +207,7 @@ command_process_var_cell(var_cell_t *cell, or_connection_t *conn)
 #endif
 
   /* reject all when not handshaking. */
-  if (conn->_base.state != OR_CONN_STATE_OR_HANDSHAKING)
+  if (conn->_base.state != OR_CONN_STATE_OR_HANDSHAKING_V2)
     return;
 
   switch (cell->command) {
@@ -216,10 +216,9 @@ command_process_var_cell(var_cell_t *cell, or_connection_t *conn)
       PROCESS_CELL(versions, cell, conn);
       break;
     default:
-      log_warn(LD_BUG,
+      log_fn(LOG_INFO, LD_PROTOCOL,
                "Variable-length cell of unknown type (%d) received.",
                cell->command);
-      tor_fragile_assert();
       break;
   }
 }
@@ -506,7 +505,7 @@ command_process_versions_cell(var_cell_t *cell, or_connection_t *conn)
   int highest_supported_version = 0;
   const uint8_t *cp, *end;
   if (conn->link_proto != 0 ||
-      conn->_base.state != OR_CONN_STATE_OR_HANDSHAKING ||
+      conn->_base.state != OR_CONN_STATE_OR_HANDSHAKING_V2 ||
       (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 "
@@ -572,7 +571,7 @@ command_process_netinfo_cell(cell_t *cell, or_connection_t *conn)
            conn->link_proto == 0 ? "non-versioned" : "a v1");
     return;
   }
-  if (conn->_base.state != OR_CONN_STATE_OR_HANDSHAKING) {
+  if (conn->_base.state != OR_CONN_STATE_OR_HANDSHAKING_V2) {
     log_fn(LOG_PROTOCOL_WARN, LD_OR,
            "Received a NETINFO cell on non-handshaking connection; dropping.");
     return;
diff --git a/src/or/connection.c b/src/or/connection.c
index 45a12715a..2bd2d07e9 100644
--- a/src/or/connection.c
+++ b/src/or/connection.c
@@ -140,10 +140,13 @@ conn_state_to_string(int type, int state)
         case OR_CONN_STATE_PROXY_HANDSHAKING: return "handshaking (proxy)";
         case OR_CONN_STATE_TLS_HANDSHAKING: return "handshaking (TLS)";
         case OR_CONN_STATE_TLS_CLIENT_RENEGOTIATING:
-          return "renegotiating (TLS)";
+          return "renegotiating (TLS, v2 handshake)";
         case OR_CONN_STATE_TLS_SERVER_RENEGOTIATING:
-          return "waiting for renegotiation (TLS)";
-        case OR_CONN_STATE_OR_HANDSHAKING: return "handshaking (Tor)";
+          return "waiting for renegotiation or V3 handshake";
+        case OR_CONN_STATE_OR_HANDSHAKING_V2:
+          return "handshaking (Tor, v2 handshake)";
+        case OR_CONN_STATE_OR_HANDSHAKING_V3:
+          return "handshaking (Tor, v3 handshake)";
         case OR_CONN_STATE_OPEN: return "open";
       }
       break;
diff --git a/src/or/connection_or.c b/src/or/connection_or.c
index 29f0f8de7..b146efb41 100644
--- a/src/or/connection_or.c
+++ b/src/or/connection_or.c
@@ -387,7 +387,7 @@ connection_or_process_inbuf(or_connection_t *conn)
       /* fall through. */
 #endif
     case OR_CONN_STATE_OPEN:
-    case OR_CONN_STATE_OR_HANDSHAKING:
+    case OR_CONN_STATE_OR_HANDSHAKING_V2:
       return connection_or_process_cells_from_inbuf(conn);
     default:
       return 0; /* don't do anything */
@@ -439,7 +439,7 @@ connection_or_finished_flushing(or_connection_t *conn)
   switch (conn->_base.state) {
     case OR_CONN_STATE_PROXY_HANDSHAKING:
     case OR_CONN_STATE_OPEN:
-    case OR_CONN_STATE_OR_HANDSHAKING:
+    case OR_CONN_STATE_OR_HANDSHAKING_V2:
       break;
     default:
       log_err(LD_BUG,"Called in unexpected state %d.", conn->_base.state);
@@ -1476,7 +1476,7 @@ connection_tls_finish_handshake(or_connection_t *conn)
     tor_tls_block_renegotiation(conn->tls);
     return connection_or_set_state_open(conn);
   } else {
-    conn->_base.state = OR_CONN_STATE_OR_HANDSHAKING;
+    conn->_base.state = OR_CONN_STATE_OR_HANDSHAKING_V2;
     if (connection_init_or_handshake_state(conn, started_here) < 0)
       return -1;
     if (!started_here) {
diff --git a/src/or/or.h b/src/or/or.h
index d6eaeb6f1..ef4ddbdb8 100644
--- a/src/or/or.h
+++ b/src/or/or.h
@@ -283,22 +283,27 @@ typedef enum {
 #define OR_CONN_STATE_CONNECTING 1
 /** State for a connection to an OR: waiting for proxy handshake to complete */
 #define OR_CONN_STATE_PROXY_HANDSHAKING 2
-/** State for a connection to an OR or client: SSL is handshaking, not done
+/** State for an OR connection client: SSL is handshaking, not done
  * yet. */
 #define OR_CONN_STATE_TLS_HANDSHAKING 3
 /** State for a connection to an OR: We're doing a second SSL handshake for
- * renegotiation purposes. */
+ * renegotiation purposes. (V2 handshake only.) */
 #define OR_CONN_STATE_TLS_CLIENT_RENEGOTIATING 4
 /** State for a connection at an OR: We're waiting for the client to
- * renegotiate. */
+ * renegotiate (to indicate a v2 handshake) or send a versions cell (to
+ * indicate a v3 handshake) */
 #define OR_CONN_STATE_TLS_SERVER_RENEGOTIATING 5
-/** State for a connection to an OR: We're done with our SSL handshake, but we
- * haven't yet negotiated link protocol versions and sent a netinfo cell.
- */
-#define OR_CONN_STATE_OR_HANDSHAKING 6
-/** State for a connection to an OR: Ready to send/receive cells. */
-#define OR_CONN_STATE_OPEN 7
-#define _OR_CONN_STATE_MAX 7
+/** State for an OR connection: We're done with our SSL handshake, we've done
+ * renegotiation, but we haven't yet negotiated link protocol versions and
+ * sent a netinfo cell. */
+#define OR_CONN_STATE_OR_HANDSHAKING_V2 6
+/** State for an OR connection: We're done with our SSL handshake, but we
+ * haven't yet negotiated link protocol versions, done a V3 handshake, and
+ * sent a netinfo cell. */
+#define OR_CONN_STATE_OR_HANDSHAKING_V3 7
+/** State for an OR connection:: Ready to send/receive cells. */
+#define OR_CONN_STATE_OPEN 8
+#define _OR_CONN_STATE_MAX 8
 
 #define _EXIT_CONN_STATE_MIN 1
 /** State for an exit connection: waiting for response from DNS farm. */
@@ -820,9 +825,10 @@ typedef enum {
 #define CELL_NETINFO 8
 #define CELL_RELAY_EARLY 9
 
-/** True iff the cell command <b>x</b> is one that implies a variable-length
- * cell. */
-#define CELL_COMMAND_IS_VAR_LENGTH(x) ((x) == CELL_VERSIONS)
+#define CELL_VPADDING 128
+#define CELL_CERT 129
+#define CELL_AUTH_CHALLENGE 130
+#define CELL_AUTHENTICATE 131
 
 /** How long to test reachability before complaining to the user. */
 #define TIMEOUT_UNTIL_UNREACHABILITY_COMPLAINT (20*60)