sendme: Validate v1 SENDMEs on both client and exit side

The validation of the SENDME cell is now done as the very first thing when
receiving it for both client and exit. On failure to validate, the circuit is
closed as detailed in the specification.

Part of #30428

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

diff --git a/src/core/or/sendme.c b/src/core/or/sendme.c
index 586d4d0ae03775e14807d06bca9a04f5090d9455..baa57f4f258145eefb7559ed8a9d871ce0e6b3db 100644 (file)
--- a/src/core/or/sendme.c
+++ b/src/core/or/sendme.c
@@ -204,10 +204,10 @@ sendme_is_valid(const circuit_t *circ, const uint8_t *cell_payload,
 
   /* Valid cell. */
   sendme_cell_free(cell);
-  return 1;
+  return true;
  invalid:
   sendme_cell_free(cell);
-  return 0;
+  return false;
 }
 
 /* Build and encode a version 1 SENDME cell into payload, which must be at
@@ -424,6 +424,12 @@ sendme_process_circuit_level(crypt_path_t *layer_hint,
   tor_assert(circ);
   tor_assert(cell_payload);
 
+  /* Validate the SENDME cell. Depending on the version, different validation
+   * can be done. An invalid SENDME requires us to close the circuit. */
+  if (!sendme_is_valid(circ, cell_payload, cell_payload_len)) {
+    return -END_CIRC_REASON_TORPROTOCOL;
+  }
+
   /* If we are the origin of the circuit, we are the Client so we use the
    * layer hint (the Exit hop) for the package window tracking. */
   if (CIRCUIT_IS_ORIGIN(circ)) {
@@ -448,13 +454,6 @@ sendme_process_circuit_level(crypt_path_t *layer_hint,
      * are rate limited. */
     circuit_read_valid_data(TO_ORIGIN_CIRCUIT(circ), cell_payload_len);
   } else {
-    /* Validate the SENDME cell. Depending on the version, different
-     * validation can be done. An invalid SENDME requires us to close the
-     * circuit. It is only done if we are the Exit of the circuit. */
-    if (!sendme_is_valid(circ, cell_payload, cell_payload_len)) {
-      return -END_CIRC_REASON_TORPROTOCOL;
-    }
-
     /* We aren't the origin of this circuit so we are the Exit and thus we
      * track the package window with the circuit object. */
     if ((circ->package_window + CIRCWINDOW_INCREMENT) >

diff --git a/src/test/test_relaycell.c b/src/test/test_relaycell.c
index 0623583511f8271437b7b2cb1ef53c61243e388f..d6372d3956ff8cd309a98322fd4e04e6a93cab62 100644 (file)
--- a/src/test/test_relaycell.c
+++ b/src/test/test_relaycell.c
@@ -812,7 +812,7 @@ test_circbw_relay(void *arg)
   ASSERT_UNCOUNTED_BW();
 
   /* Sendme on circuit with non-full window: counted */
-  PACK_CELL(0, RELAY_COMMAND_SENDME, "Data1234");
+  PACK_CELL(0, RELAY_COMMAND_SENDME, "");
   circ->cpath->package_window = 900;
   connection_edge_process_relay_cell(&cell, TO_CIRCUIT(circ), edgeconn,
                                      circ->cpath);