[tor-commits] [tor/master] sendme: Validate v1 SENDMEs on both client and exit side

[nickm at torproject.org]

commit 69e0d5bfc7d52f223d686bcd87f629f01b03561a
Author: David Goulet <dgoulet at torproject.org>
Date:   Tue May 7 09:19:41 2019 -0400

    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 at torproject.org>
---
 src/core/or/sendme.c      | 17 ++++++++---------
 src/test/test_relaycell.c |  2 +-
 2 files changed, 9 insertions(+), 10 deletions(-)

diff --git a/src/core/or/sendme.c b/src/core/or/sendme.c
index 586d4d0ae..baa57f4f2 100644
--- 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 062358351..d6372d395 100644
--- 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);