sendme: Record cell digest on both client and exit

It turns out that only the exit side is validating the authenticated SENDME v1
logic and never the client side. Which means that if a client ever uploaded
data towards an exit, the authenticated SENDME logic wouldn't apply.

For this to work, we have to record the cell digest client side as well which
introduced a new function that supports both type of edges.

This also removes a test that is not valid anymore which was that we didn't
allow cell recording on an origin circuit (client).

Part of #30428

Signed-off-by: David Goulet <dgoulet@torproject.org>

diff --git a/scripts/maint/practracker/exceptions.txt b/scripts/maint/practracker/exceptions.txt
index cba3ddc1b3a67f5a9a4c50c561bfe68c1137abcf..a83bb62bb72b54984fe8ce5830b6657da404478b 100644 (file)
--- a/scripts/maint/practracker/exceptions.txt
+++ b/scripts/maint/practracker/exceptions.txt
@@ -122,7 +122,7 @@ problem function-size /src/core/or/policies.c:policy_summarize() 107
 problem function-size /src/core/or/protover.c:protover_all_supported() 117
 problem file-size /src/core/or/relay.c 3173
 problem function-size /src/core/or/relay.c:circuit_receive_relay_cell() 123
-problem function-size /src/core/or/relay.c:relay_send_command_from_edge_() 112
+problem function-size /src/core/or/relay.c:relay_send_command_from_edge_() 116
 problem function-size /src/core/or/relay.c:connection_ap_process_end_not_open() 194
 problem function-size /src/core/or/relay.c:connection_edge_process_relay_cell_not_open() 139
 problem function-size /src/core/or/relay.c:connection_edge_process_relay_cell() 520

diff --git a/src/core/or/relay.c b/src/core/or/relay.c
index 8b3a1be18eb13a65b5758c85bcbdf18b014bcb8e..91236e5fec339363ac440c5749ff5cb862fbec54 100644 (file)
--- a/src/core/or/relay.c
+++ b/src/core/or/relay.c
@@ -701,7 +701,7 @@ relay_send_command_from_edge_,(streamid_t stream_id, circuit_t *circ,
    * we need to. This call needs to be after the circuit_package_relay_cell()
    * because the cell digest is set within that function. */
   if (relay_command == RELAY_COMMAND_DATA) {
-    sendme_record_cell_digest(circ);
+    sendme_record_cell_digest_on_circ(circ, cpath_layer);
   }
 
   return 0;

diff --git a/src/core/or/sendme.c b/src/core/or/sendme.c
index e7c65d99e23357e0116696668f954859702930fa..586d4d0ae03775e14807d06bca9a04f5090d9455 100644 (file)
--- a/src/core/or/sendme.c
+++ b/src/core/or/sendme.c
@@ -287,6 +287,21 @@ send_circuit_level_sendme(circuit_t *circ, crypt_path_t *layer_hint,
   return 0;
 }
 
+/* Record the cell digest only if the next cell is expected to be a SENDME. */
+static void
+record_cell_digest_on_circ(circuit_t *circ, const uint8_t *sendme_digest)
+{
+  tor_assert(circ);
+  tor_assert(sendme_digest);
+
+  /* Add the digest to the last seen list in the circuit. */
+  if (circ->sendme_last_digests == NULL) {
+    circ->sendme_last_digests = smartlist_new();
+  }
+  smartlist_add(circ->sendme_last_digests,
+                tor_memdup(sendme_digest, DIGEST_LEN));
+}
+
 /*
  * Public API
  */
@@ -571,31 +586,37 @@ sendme_note_stream_data_packaged(edge_connection_t *conn)
   return --conn->package_window;
 }
 
-/* Note the cell digest in the circuit sendme last digests FIFO if applicable.
- * It is safe to pass a circuit that isn't meant to track those digests. */
+/* Record the cell digest into the circuit sendme digest list depending on
+ * which edge we are. The digest is recorded only if we expect the next cell
+ * that we will receive is a SENDME so we can match the digest. */
 void
-sendme_record_cell_digest(circuit_t *circ)
+sendme_record_cell_digest_on_circ(circuit_t *circ, crypt_path_t *cpath)
 {
-  const uint8_t *digest;
+  int package_window;
+  uint8_t *sendme_digest;
 
   tor_assert(circ);
 
-  /* We only keep the cell digest if we are the Exit on that circuit and if
-   * this cell is the last one before the client should send a SENDME. */
-  if (CIRCUIT_IS_ORIGIN(circ)) {
-    return;
+  package_window = circ->package_window;
+  if (cpath) {
+    package_window = cpath->package_window;
   }
+
   /* Is this the last cell before a SENDME? The idea is that if the
    * package_window reaches a multiple of the increment, after this cell, we
    * should expect a SENDME. */
-  if (!sendme_circuit_cell_is_next(circ->package_window)) {
+  if (!sendme_circuit_cell_is_next(package_window)) {
     return;
   }
 
-  /* Add the digest to the last seen list in the circuit. */
-  digest = relay_crypto_get_sendme_digest(&TO_OR_CIRCUIT(circ)->crypto);
-  if (circ->sendme_last_digests == NULL) {
-    circ->sendme_last_digests = smartlist_new();
+  /* Getting the digest is expensive so we only do it once we are certain to
+   * record it on the circuit. */
+  if (cpath) {
+    sendme_digest = cpath_get_sendme_digest(cpath);
+  } else {
+    sendme_digest =
+      relay_crypto_get_sendme_digest(&TO_OR_CIRCUIT(circ)->crypto);
   }
-  smartlist_add(circ->sendme_last_digests, tor_memdup(digest, DIGEST_LEN));
+
+  record_cell_digest_on_circ(circ, sendme_digest);
 }

diff --git a/src/core/or/sendme.h b/src/core/or/sendme.h
index ac18bbdd31939db2f3c0ec5821a8480571babb3c..2fb73f76d9658c7d6bd2bfce2cb96fac899eb330 100644 (file)
--- a/src/core/or/sendme.h
+++ b/src/core/or/sendme.h
@@ -35,8 +35,9 @@ int sendme_note_circuit_data_packaged(circuit_t *circ,
 int sendme_note_stream_data_packaged(edge_connection_t *conn);
 
 /* Track cell digest. */
-void sendme_record_cell_digest(circuit_t *circ);
 void sendme_circuit_record_outbound_cell(or_circuit_t *or_circ);
+/* Record cell digest on circuit. */
+void sendme_record_cell_digest_on_circ(circuit_t *circ, crypt_path_t *cpath);
 
 /* Circuit level information. */
 bool sendme_circuit_cell_is_next(int window);

diff --git a/src/test/test_sendme.c b/src/test/test_sendme.c
index d40fbaf862fd8a3fe4277a01879991125a00acad..463a0ec086d74e339a546bd2422cd44c1b3c0183 100644 (file)
--- a/src/test/test_sendme.c
+++ b/src/test/test_sendme.c
@@ -46,26 +46,12 @@ static void
 test_v1_record_digest(void *arg)
 {
   or_circuit_t *or_circ = NULL;
-  origin_circuit_t *orig_circ = NULL;
   circuit_t *circ = NULL;
 
   (void) arg;
 
-  /* Create our dummy circuits. */
-  orig_circ = origin_circuit_new();
-  tt_assert(orig_circ);
+  /* Create our dummy circuit. */
   or_circ = or_circuit_new(1, NULL);
-
-  /* Start by pointing to the origin circuit. */
-  circ = TO_CIRCUIT(orig_circ);
-  circ->purpose = CIRCUIT_PURPOSE_S_REND_JOINED;
-
-  /* We should never note SENDME digest on origin circuit. */
-  sendme_record_cell_digest(circ);
-  tt_assert(!circ->sendme_last_digests);
-  /* We do not need the origin circuit for now. */
-  orig_circ = NULL;
-  circuit_free_(circ);
   /* Points it to the OR circuit now. */
   circ = TO_CIRCUIT(or_circ);
 
@@ -73,23 +59,23 @@ test_v1_record_digest(void *arg)
    * in order to catched the CIRCWINDOW_INCREMENT-nth cell. Try something that
    * shouldn't be noted. */
   circ->package_window = CIRCWINDOW_INCREMENT;
-  sendme_record_cell_digest(circ);
+  sendme_record_cell_digest_on_circ(circ, NULL);
   tt_assert(!circ->sendme_last_digests);
 
   /* This should work now. Package window at CIRCWINDOW_INCREMENT + 1. */
   circ->package_window++;
-  sendme_record_cell_digest(circ);
+  sendme_record_cell_digest_on_circ(circ, NULL);
   tt_assert(circ->sendme_last_digests);
   tt_int_op(smartlist_len(circ->sendme_last_digests), OP_EQ, 1);
 
   /* Next cell in the package window shouldn't do anything. */
   circ->package_window++;
-  sendme_record_cell_digest(circ);
+  sendme_record_cell_digest_on_circ(circ, NULL);
   tt_int_op(smartlist_len(circ->sendme_last_digests), OP_EQ, 1);
 
   /* The next CIRCWINDOW_INCREMENT should add one more digest. */
   circ->package_window = (CIRCWINDOW_INCREMENT * 2) + 1;
-  sendme_record_cell_digest(circ);
+  sendme_record_cell_digest_on_circ(circ, NULL);
   tt_int_op(smartlist_len(circ->sendme_last_digests), OP_EQ, 2);
 
  done:
@@ -188,7 +174,7 @@ test_v1_build_cell(void *arg)
 
   /* Note the wrong digest in the circuit, cell should fail validation. */
   circ->package_window = CIRCWINDOW_INCREMENT + 1;
-  sendme_record_cell_digest(circ);
+  sendme_record_cell_digest_on_circ(circ, NULL);
   tt_int_op(smartlist_len(circ->sendme_last_digests), OP_EQ, 1);
   setup_full_capture_of_logs(LOG_INFO);
   tt_int_op(sendme_is_valid(circ, payload, sizeof(payload)), OP_EQ, false);
@@ -200,7 +186,7 @@ test_v1_build_cell(void *arg)
   /* Record the cell digest into the circuit, cell should validate. */
   memcpy(or_circ->crypto.sendme_digest, digest, sizeof(digest));
   circ->package_window = CIRCWINDOW_INCREMENT + 1;
-  sendme_record_cell_digest(circ);
+  sendme_record_cell_digest_on_circ(circ, NULL);
   tt_int_op(smartlist_len(circ->sendme_last_digests), OP_EQ, 1);
   tt_int_op(sendme_is_valid(circ, payload, sizeof(payload)), OP_EQ, true);
   /* After a validation, the last digests is always popped out. */