Browse Source

Ticket #25573: Count TRUNCATED cells.

TRUNCATED cells were ignored while in path bias. Now they are obeyed, and
cause us to tear down the circuit. The actual impact is minimal, since we
would just wait around for a probe that would never arrive before.

This commit changes client behavior.
Mike Perry 5 years ago
parent
commit
ce894e20b5
5 changed files with 38 additions and 5 deletions
  1. 10 0
      src/or/circpathbias.c
  2. 1 2
      src/or/circuitbuild.c
  3. 1 2
      src/or/circuitbuild.h
  4. 8 1
      src/or/relay.c
  5. 18 0
      src/test/test_relaycell.c

+ 10 - 0
src/or/circpathbias.c

@@ -929,6 +929,16 @@ pathbias_count_valid_cells(circuit_t *circ, const cell_t *cell)
   /* Check to see if this is a cell from a previous connection,
    * or is a request to close the circuit. */
   switch (rh.command) {
+    case RELAY_COMMAND_TRUNCATED:
+      /* Truncated cells can arrive on path bias circs. When they do,
+       * just process them. This closes the circ, but it was junk anyway.
+       * No reason to wait for the probe. */
+      circuit_read_valid_data(ocirc, rh.length);
+      circuit_truncated(TO_ORIGIN_CIRCUIT(circ),
+                        get_uint8(cell->payload + RELAY_HEADER_SIZE));
+
+      break;
+
     case RELAY_COMMAND_END:
       if (connection_half_edge_is_valid_end(ocirc->half_streams,
                                              rh.stream_id)) {

+ 1 - 2
src/or/circuitbuild.c

@@ -1419,13 +1419,12 @@ circuit_finish_handshake(origin_circuit_t *circ,
  * just give up: force circ to close, and return 0.
  */
 int
-circuit_truncated(origin_circuit_t *circ, crypt_path_t *layer, int reason)
+circuit_truncated(origin_circuit_t *circ, int reason)
 {
 //  crypt_path_t *victim;
 //  connection_t *stream;
 
   tor_assert(circ);
-  tor_assert(layer);
 
   /* XXX Since we don't send truncates currently, getting a truncated
    *     means that a connection broke or an extend failed. For now,

+ 1 - 2
src/or/circuitbuild.h

@@ -37,8 +37,7 @@ int circuit_init_cpath_crypto(crypt_path_t *cpath,
 struct created_cell_t;
 int circuit_finish_handshake(origin_circuit_t *circ,
                              const struct created_cell_t *created_cell);
-int circuit_truncated(origin_circuit_t *circ, crypt_path_t *layer,
-                      int reason);
+int circuit_truncated(origin_circuit_t *circ, int reason);
 int onionskin_answer(or_circuit_t *circ,
                      const struct created_cell_t *created_cell,
                      const char *keys, size_t keys_len,

+ 8 - 1
src/or/relay.c

@@ -1748,7 +1748,14 @@ connection_edge_process_relay_cell(cell_t *cell, circuit_t *circ,
                "'truncated' unsupported at non-origin. Dropping.");
         return 0;
       }
-      circuit_truncated(TO_ORIGIN_CIRCUIT(circ), layer_hint,
+
+      /* Count the truncated as valid, for completeness. The
+       * circuit is being torn down anyway, though.  */
+      if (CIRCUIT_IS_ORIGIN(circ)) {
+        circuit_read_valid_data(TO_ORIGIN_CIRCUIT(circ),
+                                rh.length);
+      }
+      circuit_truncated(TO_ORIGIN_CIRCUIT(circ),
                         get_uint8(cell->payload + RELAY_HEADER_SIZE));
       return 0;
     case RELAY_COMMAND_CONNECTED:

+ 18 - 0
src/test/test_relaycell.c

@@ -115,6 +115,16 @@ mock_connection_mark_unattached_ap_(entry_connection_t *conn, int endreason,
   conn->edge_.end_reason = endreason;
 }
 
+static void
+mock_mark_circ_for_close(circuit_t *circ, int reason, int line,
+                          const char *file)
+{
+  (void)reason; (void)line; (void)file;
+
+  circ->marked_for_close = 1;
+  return;
+}
+
 static void
 mock_mark_for_close(connection_t *conn,
                         int line, const char *file)
@@ -694,6 +704,7 @@ test_circbw_relay(void *arg)
   MOCK(connection_start_reading, mock_start_reading);
   MOCK(connection_mark_for_close_internal_, mock_mark_for_close);
   MOCK(relay_send_command_from_edge_, mock_send_command);
+  MOCK(circuit_mark_for_close_, mock_mark_circ_for_close);
 
   circ = helper_create_origin_circuit(CIRCUIT_PURPOSE_C_GENERAL, 0);
   circ->cpath->state = CPATH_STATE_AWAITING_KEYS;
@@ -856,11 +867,18 @@ test_circbw_relay(void *arg)
   if (!subtest_circbw_halfclosed(circ, 6))
     goto done;
 
+  /* Path bias: truncated */
+  tt_int_op(circ->base_.marked_for_close, OP_EQ, 0);
+  PACK_CELL(0, RELAY_COMMAND_TRUNCATED, "Data1234");
+  pathbias_count_valid_cells(circ, &cell);
+  tt_int_op(circ->base_.marked_for_close, OP_EQ, 1);
+
  done:
   UNMOCK(connection_start_reading);
   UNMOCK(connection_mark_unattached_ap_);
   UNMOCK(connection_mark_for_close_internal_);
   UNMOCK(relay_send_command_from_edge_);
+  UNMOCK(circuit_mark_for_close_);
   circuit_free_(TO_CIRCUIT(circ));
   connection_free_minimal(ENTRY_TO_CONN(entryconn1));
 }