Split connection_edge_process_relay_cell() in two functions.

One function does the validation, the other does the handling.

1 files changed, 124 insertions, 101 deletions

diff --git a/src/core/or/relay.c b/src/core/or/relay.c
index 9e691a02b5..84fc587871 100644
--- a/src/core/or/relay.c
+++ b/src/core/or/relay.c
@@ -1576,83 +1576,27 @@ process_sendme_cell(const relay_header_t *rh, const cell_t *cell,
   return 0;
 }
 
-/** An incoming relay cell has arrived on circuit <b>circ</b>. If
- * <b>conn</b> is NULL this is a control cell, else <b>cell</b> is
- * destined for <b>conn</b>.
- *
- * If <b>layer_hint</b> is defined, then we're the origin of the
- * circuit, and it specifies the hop that packaged <b>cell</b>.
+/** A helper for connection_edge_process_relay_cell(): Actually handles the
+ *  cell that we received on the connection.
  *
- * Return -reason if you want to warn and tear down the circuit, else 0.
+ *  The arguments are the same as in the parent function
+ *  connection_edge_process_relay_cell(), plus the relay header <b>rh</b> as
+ *  unpacked by the parent function, and <b>optimistic_data</b> as set by the
+ *  parent function.
  */
-STATIC int
-connection_edge_process_relay_cell(cell_t *cell, circuit_t *circ,
-                                   edge_connection_t *conn,
-                                   crypt_path_t *layer_hint)
+static int
+handle_relay_command(cell_t *cell, circuit_t *circ,
+                     edge_connection_t *conn,
+                     crypt_path_t *layer_hint,
+                     relay_header_t *rh,
+                     int optimistic_data)
 {
-  static int num_seen=0;
-  relay_header_t rh;
   unsigned domain = layer_hint?LD_APP:LD_EXIT;
   int reason;
-  int optimistic_data = 0; /* Set to 1 if we receive data on a stream
-                            * that's in the EXIT_CONN_STATE_RESOLVING
-                            * or EXIT_CONN_STATE_CONNECTING states. */
-
-  tor_assert(cell);
-  tor_assert(circ);
-
-  relay_header_unpack(&rh, cell->payload);
-//  log_fn(LOG_DEBUG,"command %d stream %d", rh.command, rh.stream_id);
-  num_seen++;
-  log_debug(domain, "Now seen %d relay cells here (command %d, stream %d).",
-            num_seen, rh.command, rh.stream_id);
 
-  if (rh.length > RELAY_PAYLOAD_SIZE) {
-    log_fn(LOG_PROTOCOL_WARN, LD_PROTOCOL,
-           "Relay cell length field too long. Closing circuit.");
-    return - END_CIRC_REASON_TORPROTOCOL;
-  }
-
-  if (rh.stream_id == 0) {
-    switch (rh.command) {
-      case RELAY_COMMAND_BEGIN:
-      case RELAY_COMMAND_CONNECTED:
-      case RELAY_COMMAND_END:
-      case RELAY_COMMAND_RESOLVE:
-      case RELAY_COMMAND_RESOLVED:
-      case RELAY_COMMAND_BEGIN_DIR:
-        log_fn(LOG_PROTOCOL_WARN, LD_PROTOCOL, "Relay command %d with zero "
-               "stream_id. Dropping.", (int)rh.command);
-        return 0;
-      default:
-        ;
-    }
-  }
-
-  /* Tell circpad that we've received a recognized cell */
-  circpad_deliver_recognized_relay_cell_events(circ, rh.command, layer_hint);
-
-  /* either conn is NULL, in which case we've got a control cell, or else
-   * conn points to the recognized stream. */
-  if (conn && !connection_state_is_open(TO_CONN(conn))) {
-    if (conn->base_.type == CONN_TYPE_EXIT &&
-        (conn->base_.state == EXIT_CONN_STATE_CONNECTING ||
-         conn->base_.state == EXIT_CONN_STATE_RESOLVING) &&
-        rh.command == RELAY_COMMAND_DATA) {
-      /* Allow DATA cells to be delivered to an exit node in state
-       * EXIT_CONN_STATE_CONNECTING or EXIT_CONN_STATE_RESOLVING.
-       * This speeds up HTTP, for example. */
-      optimistic_data = 1;
-    } else if (rh.stream_id == 0 && rh.command == RELAY_COMMAND_DATA) {
-      log_warn(LD_BUG, "Somehow I had a connection that matched a "
-               "data cell with stream ID 0.");
-    } else {
-      return connection_edge_process_relay_cell_not_open(
-               &rh, cell, circ, conn, layer_hint);
-    }
-  }
+  tor_assert(rh);
 
-  switch (rh.command) {
+  switch (rh->command) {
     case RELAY_COMMAND_DROP:
       /* Already examined in circpad_deliver_recognized_relay_cell_events */
       return 0;
@@ -1661,7 +1605,7 @@ connection_edge_process_relay_cell(cell_t *cell, circuit_t *circ,
       return 0;
     case RELAY_COMMAND_PADDING_NEGOTIATED:
       if (circpad_handle_padding_negotiated(circ, cell, layer_hint) == 0)
-        circuit_read_valid_data(TO_ORIGIN_CIRCUIT(circ), rh.length);
+        circuit_read_valid_data(TO_ORIGIN_CIRCUIT(circ), rh->length);
       return 0;
   }
 
@@ -1694,7 +1638,7 @@ connection_edge_process_relay_cell(cell_t *cell, circuit_t *circ,
                "Begin cell for known stream. Dropping.");
         return 0;
       }
-      if (rh.command == RELAY_COMMAND_BEGIN_DIR &&
+      if (rh->command == RELAY_COMMAND_BEGIN_DIR &&
           circ->purpose != CIRCUIT_PURPOSE_S_REND_JOINED) {
         /* Assign this circuit and its app-ward OR connection a unique ID,
          * so that we can measure download times. The local edge and dir
@@ -1721,7 +1665,7 @@ connection_edge_process_relay_cell(cell_t *cell, circuit_t *circ,
       /* Consider sending a circuit-level SENDME cell. */
       sendme_circuit_consider_sending(circ, layer_hint);
 
-      if (rh.stream_id == 0) {
+      if (rh->stream_id == 0) {
         log_fn(LOG_PROTOCOL_WARN, LD_PROTOCOL, "Relay data cell with zero "
                "stream_id. Dropping.");
         return 0;
@@ -1729,16 +1673,16 @@ connection_edge_process_relay_cell(cell_t *cell, circuit_t *circ,
         if (CIRCUIT_IS_ORIGIN(circ)) {
           origin_circuit_t *ocirc = TO_ORIGIN_CIRCUIT(circ);
           if (connection_half_edge_is_valid_data(ocirc->half_streams,
-                                                 rh.stream_id)) {
-            circuit_read_valid_data(ocirc, rh.length);
+                                                 rh->stream_id)) {
+            circuit_read_valid_data(ocirc, rh->length);
             log_info(domain,
                      "data cell on circ %u valid on half-closed "
-                     "stream id %d", ocirc->global_identifier, rh.stream_id);
+                     "stream id %d", ocirc->global_identifier, rh->stream_id);
           }
         }
 
         log_info(domain,"data cell dropped, unknown stream (streamid %d).",
-                 rh.stream_id);
+                 rh->stream_id);
         return 0;
       }
 
@@ -1753,13 +1697,13 @@ connection_edge_process_relay_cell(cell_t *cell, circuit_t *circ,
         return -END_CIRC_REASON_TORPROTOCOL;
       }
       /* Total all valid application bytes delivered */
-      if (CIRCUIT_IS_ORIGIN(circ) && rh.length > 0) {
-        circuit_read_valid_data(TO_ORIGIN_CIRCUIT(circ), rh.length);
+      if (CIRCUIT_IS_ORIGIN(circ) && rh->length > 0) {
+        circuit_read_valid_data(TO_ORIGIN_CIRCUIT(circ), rh->length);
       }
 
-      stats_n_data_bytes_received += rh.length;
+      stats_n_data_bytes_received += rh->length;
       connection_buf_add((char*)(cell->payload + RELAY_HEADER_SIZE),
-                              rh.length, TO_CONN(conn));
+                              rh->length, TO_CONN(conn));
 
 #ifdef MEASUREMENTS_21206
       /* Count number of RELAY_DATA cells received on a linked directory
@@ -1780,20 +1724,20 @@ connection_edge_process_relay_cell(cell_t *cell, circuit_t *circ,
 
       return 0;
     case RELAY_COMMAND_END:
-      reason = rh.length > 0 ?
+      reason = rh->length > 0 ?
         get_uint8(cell->payload+RELAY_HEADER_SIZE) : END_STREAM_REASON_MISC;
       if (!conn) {
         if (CIRCUIT_IS_ORIGIN(circ)) {
           origin_circuit_t *ocirc = TO_ORIGIN_CIRCUIT(circ);
           if (connection_half_edge_is_valid_end(ocirc->half_streams,
-                                                rh.stream_id)) {
+                                                rh->stream_id)) {
 
-            circuit_read_valid_data(ocirc, rh.length);
+            circuit_read_valid_data(ocirc, rh->length);
             log_info(domain,
                      "end cell (%s) on circ %u valid on half-closed "
                      "stream id %d",
                      stream_end_reason_to_string(reason),
-                     ocirc->global_identifier, rh.stream_id);
+                     ocirc->global_identifier, rh->stream_id);
             return 0;
           }
         }
@@ -1825,7 +1769,7 @@ connection_edge_process_relay_cell(cell_t *cell, circuit_t *circ,
 
         /* Total all valid application bytes delivered */
         if (CIRCUIT_IS_ORIGIN(circ)) {
-          circuit_read_valid_data(TO_ORIGIN_CIRCUIT(circ), rh.length);
+          circuit_read_valid_data(TO_ORIGIN_CIRCUIT(circ), rh->length);
         }
       }
       return 0;
@@ -1833,7 +1777,7 @@ connection_edge_process_relay_cell(cell_t *cell, circuit_t *circ,
     case RELAY_COMMAND_EXTEND2: {
       static uint64_t total_n_extend=0, total_nonearly=0;
       total_n_extend++;
-      if (rh.stream_id) {
+      if (rh->stream_id) {
         log_fn(LOG_PROTOCOL_WARN, domain,
                "'extend' cell received for non-zero stream. Dropping.");
         return 0;
@@ -1874,9 +1818,9 @@ connection_edge_process_relay_cell(cell_t *cell, circuit_t *circ,
       log_debug(domain,"Got an extended cell! Yay.");
       {
         extended_cell_t extended_cell;
-        if (extended_cell_parse(&extended_cell, rh.command,
+        if (extended_cell_parse(&extended_cell, rh->command,
                         (const uint8_t*)cell->payload+RELAY_HEADER_SIZE,
-                        rh.length)<0) {
+                        rh->length)<0) {
           log_warn(LD_PROTOCOL,
                    "Can't parse EXTENDED cell; killing circuit.");
           return -END_CIRC_REASON_TORPROTOCOL;
@@ -1894,7 +1838,7 @@ connection_edge_process_relay_cell(cell_t *cell, circuit_t *circ,
       }
       /* Total all valid bytes delivered. */
       if (CIRCUIT_IS_ORIGIN(circ)) {
-        circuit_read_valid_data(TO_ORIGIN_CIRCUIT(circ), rh.length);
+        circuit_read_valid_data(TO_ORIGIN_CIRCUIT(circ), rh->length);
       }
       return 0;
     case RELAY_COMMAND_TRUNCATE:
@@ -1938,7 +1882,7 @@ connection_edge_process_relay_cell(cell_t *cell, circuit_t *circ,
        * circuit is being torn down anyway, though.  */
       if (CIRCUIT_IS_ORIGIN(circ)) {
         circuit_read_valid_data(TO_ORIGIN_CIRCUIT(circ),
-                                rh.length);
+                                rh->length);
       }
       circuit_truncated(TO_ORIGIN_CIRCUIT(circ),
                         get_uint8(cell->payload + RELAY_HEADER_SIZE));
@@ -1953,11 +1897,11 @@ connection_edge_process_relay_cell(cell_t *cell, circuit_t *circ,
       if (CIRCUIT_IS_ORIGIN(circ)) {
         origin_circuit_t *ocirc = TO_ORIGIN_CIRCUIT(circ);
         if (connection_half_edge_is_valid_connected(ocirc->half_streams,
-                                                    rh.stream_id)) {
-          circuit_read_valid_data(ocirc, rh.length);
+                                                    rh->stream_id)) {
+          circuit_read_valid_data(ocirc, rh->length);
           log_info(domain,
                    "connected cell on circ %u valid on half-closed "
-                   "stream id %d", ocirc->global_identifier, rh.stream_id);
+                   "stream id %d", ocirc->global_identifier, rh->stream_id);
           return 0;
         }
       }
@@ -1965,10 +1909,10 @@ connection_edge_process_relay_cell(cell_t *cell, circuit_t *circ,
       log_info(domain,
                "'connected' received on circid %u for streamid %d, "
                "no conn attached anymore. Ignoring.",
-               (unsigned)circ->n_circ_id, rh.stream_id);
+               (unsigned)circ->n_circ_id, rh->stream_id);
       return 0;
     case RELAY_COMMAND_SENDME:
-      return process_sendme_cell(&rh, cell, circ, conn, layer_hint, domain);
+      return process_sendme_cell(rh, cell, circ, conn, layer_hint, domain);
     case RELAY_COMMAND_RESOLVE:
       if (layer_hint) {
         log_fn(LOG_PROTOCOL_WARN, LD_APP,
@@ -1996,11 +1940,11 @@ connection_edge_process_relay_cell(cell_t *cell, circuit_t *circ,
       if (CIRCUIT_IS_ORIGIN(circ)) {
         origin_circuit_t *ocirc = TO_ORIGIN_CIRCUIT(circ);
         if (connection_half_edge_is_valid_resolved(ocirc->half_streams,
-                                                    rh.stream_id)) {
-          circuit_read_valid_data(ocirc, rh.length);
+                                                    rh->stream_id)) {
+          circuit_read_valid_data(ocirc, rh->length);
           log_info(domain,
                    "resolved cell on circ %u valid on half-closed "
-                   "stream id %d", ocirc->global_identifier, rh.stream_id);
+                   "stream id %d", ocirc->global_identifier, rh->stream_id);
           return 0;
         }
       }
@@ -2018,17 +1962,96 @@ connection_edge_process_relay_cell(cell_t *cell, circuit_t *circ,
     case RELAY_COMMAND_INTRO_ESTABLISHED:
     case RELAY_COMMAND_RENDEZVOUS_ESTABLISHED:
       rend_process_relay_cell(circ, layer_hint,
-                              rh.command, rh.length,
+                              rh->command, rh->length,
                               cell->payload+RELAY_HEADER_SIZE);
       return 0;
   }
   log_fn(LOG_PROTOCOL_WARN, LD_PROTOCOL,
          "Received unknown relay command %d. Perhaps the other side is using "
          "a newer version of Tor? Dropping.",
-         rh.command);
+         rh->command);
   return 0; /* for forward compatibility, don't kill the circuit */
 }
 
+/** An incoming relay cell has arrived on circuit <b>circ</b>. If
+ * <b>conn</b> is NULL this is a control cell, else <b>cell</b> is
+ * destined for <b>conn</b>.
+ *
+ * If <b>layer_hint</b> is defined, then we're the origin of the
+ * circuit, and it specifies the hop that packaged <b>cell</b>.
+ *
+ * Return -reason if you want to warn and tear down the circuit, else 0.
+ */
+STATIC int
+connection_edge_process_relay_cell(cell_t *cell, circuit_t *circ,
+                                   edge_connection_t *conn,
+                                   crypt_path_t *layer_hint)
+{
+  static int num_seen=0;
+  relay_header_t rh;
+  unsigned domain = layer_hint?LD_APP:LD_EXIT;
+  int optimistic_data = 0; /* Set to 1 if we receive data on a stream
+                            * that's in the EXIT_CONN_STATE_RESOLVING
+                            * or EXIT_CONN_STATE_CONNECTING states. */
+
+  tor_assert(cell);
+  tor_assert(circ);
+
+  relay_header_unpack(&rh, cell->payload);
+//  log_fn(LOG_DEBUG,"command %d stream %d", rh.command, rh.stream_id);
+  num_seen++;
+  log_debug(domain, "Now seen %d relay cells here (command %d, stream %d).",
+            num_seen, rh.command, rh.stream_id);
+
+  if (rh.length > RELAY_PAYLOAD_SIZE) {
+    log_fn(LOG_PROTOCOL_WARN, LD_PROTOCOL,
+           "Relay cell length field too long. Closing circuit.");
+    return - END_CIRC_REASON_TORPROTOCOL;
+  }
+
+  if (rh.stream_id == 0) {
+    switch (rh.command) {
+      case RELAY_COMMAND_BEGIN:
+      case RELAY_COMMAND_CONNECTED:
+      case RELAY_COMMAND_END:
+      case RELAY_COMMAND_RESOLVE:
+      case RELAY_COMMAND_RESOLVED:
+      case RELAY_COMMAND_BEGIN_DIR:
+        log_fn(LOG_PROTOCOL_WARN, LD_PROTOCOL, "Relay command %d with zero "
+               "stream_id. Dropping.", (int)rh.command);
+        return 0;
+      default:
+        ;
+    }
+  }
+
+  /* Tell circpad that we've received a recognized cell */
+  circpad_deliver_recognized_relay_cell_events(circ, rh.command, layer_hint);
+
+  /* either conn is NULL, in which case we've got a control cell, or else
+   * conn points to the recognized stream. */
+  if (conn && !connection_state_is_open(TO_CONN(conn))) {
+    if (conn->base_.type == CONN_TYPE_EXIT &&
+        (conn->base_.state == EXIT_CONN_STATE_CONNECTING ||
+         conn->base_.state == EXIT_CONN_STATE_RESOLVING) &&
+        rh.command == RELAY_COMMAND_DATA) {
+      /* Allow DATA cells to be delivered to an exit node in state
+       * EXIT_CONN_STATE_CONNECTING or EXIT_CONN_STATE_RESOLVING.
+       * This speeds up HTTP, for example. */
+      optimistic_data = 1;
+    } else if (rh.stream_id == 0 && rh.command == RELAY_COMMAND_DATA) {
+      log_warn(LD_BUG, "Somehow I had a connection that matched a "
+               "data cell with stream ID 0.");
+    } else {
+      return connection_edge_process_relay_cell_not_open(
+               &rh, cell, circ, conn, layer_hint);
+    }
+  }
+
+  return handle_relay_command(cell, circ, conn, layer_hint,
+                              &rh, optimistic_data);
+}
+
 /** How many relay_data cells have we built, ever? */
 uint64_t stats_n_data_cells_packaged = 0;
 /** How many bytes of data have we put in relay_data cells have we built,