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>

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

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;

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);
 }

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);

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. */