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.

diff --git a/src/or/circpathbias.c b/src/or/circpathbias.c
index 923941e5b6a9821a9fc1cb8f26ff1d6ac2d362a7..3cdd16261ab2fecb3dc60a8e8b0442724a9705a7 100644 (file)
--- a/src/or/circpathbias.c
+++ b/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)) {

diff --git a/src/or/circuitbuild.c b/src/or/circuitbuild.c
index 3d1c9c1abf5649ef77b1826b23e73081728641bf..8f17f278682981cf19702673b6a013c0d716d116 100644 (file)
--- a/src/or/circuitbuild.c
+++ b/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,

diff --git a/src/or/circuitbuild.h b/src/or/circuitbuild.h
index 0184898e29f189c6ee818d423709943ae513bc69..e3d30c110452ed87a0c0a6ab5df774c9c9e797b0 100644 (file)
--- a/src/or/circuitbuild.h
+++ b/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,

diff --git a/src/or/relay.c b/src/or/relay.c
index 13f2b56bc80fcffa30f61bb2fe8dcbfa86940f7a..81bb94d5aaeaba683df02c1492deb9076f021e84 100644 (file)
--- a/src/or/relay.c
+++ b/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:

diff --git a/src/test/test_relaycell.c b/src/test/test_relaycell.c
index 4c406a9b764e32aeaede8290b77e5600a5ca0733..3f84ee830323d0ac52e802a421387cb8bf625d84 100644 (file)
--- a/src/test/test_relaycell.c
+++ b/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));
 }