sendme: Properly record SENDMEs on both edges

Turns out that we were only recording the "b_digest" but to have
bidirectionnal authenticated SENDMEs, we need to use the "f_digest" in the
forward cell situation.

Because of the cpath refactoring, this commit plays with the crypt_path_ and
relay_crypto_t API a little bit in order to respect the abstractions.

Previously, we would record the cell digest as the SENDME digest in the
decrypt cell function but to avoid code duplication (both directions needs to
record), we now do that right after iff the cell is recognized (at the edge).
It is now done in circuit_receive_relay_cell() instead.

We now also record the cell digest as the SENDME digest in both relay cell
encryption functions since they are split depending on the direction.
relay_encrypt_cell_outbound() and relay_encrypt_cell_inbound() need to
consider recording the cell digest depending on their direction (f vs b
digest).

Fixes #30428

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

diff --git a/scripts/maint/practracker/exceptions.txt b/scripts/maint/practracker/exceptions.txt
index a83bb62bb72b54984fe8ce5830b6657da404478b..2671f723ba756bfeb143c61bb0c0a4aa17a9a93c 100644 (file)
--- a/scripts/maint/practracker/exceptions.txt
+++ b/scripts/maint/practracker/exceptions.txt
@@ -121,7 +121,7 @@ problem file-size /src/core/or/policies.c 3249
 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:circuit_receive_relay_cell() 127
 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

diff --git a/src/core/crypto/relay_crypto.c b/src/core/crypto/relay_crypto.c
index 74cccd22239bbb542aa68fdfe25c838310cb2fbc..8a285131a8b8ce290c4325bc940d1ecab902a0c8 100644 (file)
--- a/src/core/crypto/relay_crypto.c
+++ b/src/core/crypto/relay_crypto.c
@@ -100,12 +100,22 @@ relay_crypto_get_sendme_digest(relay_crypto_t *crypto)
   return crypto->sendme_digest;
 }
 
-/** Record the b_digest from <b>crypto</b> and put it in the sendme_digest. */
+/** Record the cell digest, indicated by is_foward_digest or not, as the
+ * SENDME cell digest. */
 void
-relay_crypto_record_sendme_digest(relay_crypto_t *crypto)
+relay_crypto_record_sendme_digest(relay_crypto_t *crypto,
+                                  bool is_foward_digest)
 {
+  struct crypto_digest_t *digest;
+
   tor_assert(crypto);
-  crypto_digest_get_digest(crypto->b_digest, (char *) crypto->sendme_digest,
+
+  digest = crypto->b_digest;
+  if (is_foward_digest) {
+    digest = crypto->f_digest;
+  }
+
+  crypto_digest_get_digest(digest, (char *) crypto->sendme_digest,
                            sizeof(crypto->sendme_digest));
 }
 
@@ -161,11 +171,6 @@ relay_decrypt_cell(circuit_t *circ, cell_t *cell,
           if (relay_digest_matches(cpath_get_incoming_digest(thishop), cell)) {
             *recognized = 1;
             *layer_hint = thishop;
-            /* This cell is for us. Keep a record of this cell because we will
-             * use it in the next SENDME cell. */
-            if (sendme_circuit_cell_is_next(thishop->deliver_window)) {
-              cpath_sendme_circuit_record_inbound_cell(thishop);
-            }
             return 0;
           }
         }
@@ -213,6 +218,9 @@ relay_encrypt_cell_outbound(cell_t *cell,
   crypt_path_t *thishop; /* counter for repeated crypts */
   cpath_set_cell_forward_digest(layer_hint, cell);
 
+  /* Record cell digest as the SENDME digest if need be. */
+  sendme_record_sending_cell_digest(TO_CIRCUIT(circ), layer_hint);
+
   thishop = layer_hint;
   /* moving from farthest to nearest hop */
   do {
@@ -237,11 +245,8 @@ relay_encrypt_cell_inbound(cell_t *cell,
 {
   relay_set_digest(or_circ->crypto.b_digest, cell);
 
-  /* We are about to send this cell outbound on the circuit. Keep a record of
-   * this cell if we are expecting that the next cell is a SENDME. */
-  if (sendme_circuit_cell_is_next(TO_CIRCUIT(or_circ)->package_window)) {
-    sendme_circuit_record_outbound_cell(or_circ);
-  }
+  /* Record cell digest as the SENDME digest if need be. */
+  sendme_record_sending_cell_digest(TO_CIRCUIT(or_circ), NULL);
 
   /* encrypt one layer */
   relay_crypt_one_payload(or_circ->crypto.b_crypto, cell->payload);

diff --git a/src/core/crypto/relay_crypto.h b/src/core/crypto/relay_crypto.h
index 7f09219c7f66116855808e04207b639b4f30cf1e..9478f8d3593ead6bc0dd6d130927bf9ede694c8c 100644 (file)
--- a/src/core/crypto/relay_crypto.h
+++ b/src/core/crypto/relay_crypto.h
@@ -28,7 +28,10 @@ void relay_crypto_clear(relay_crypto_t *crypto);
 void relay_crypto_assert_ok(const relay_crypto_t *crypto);
 
 uint8_t *relay_crypto_get_sendme_digest(relay_crypto_t *crypto);
-void relay_crypto_record_sendme_digest(relay_crypto_t *crypto);
+
+void relay_crypto_record_sendme_digest(relay_crypto_t *crypto,
+                                       bool is_foward_digest);
+
 void
 relay_crypt_one_payload(crypto_cipher_t *cipher, uint8_t *in);
 

diff --git a/src/core/or/crypt_path.c b/src/core/or/crypt_path.c
index a4b7190e21b48ebbb9975e632281ea2ba726fd11..6d5245510fef53324993e05ddac60dab837ac7d5 100644 (file)
--- a/src/core/or/crypt_path.c
+++ b/src/core/or/crypt_path.c
@@ -204,15 +204,6 @@ cpath_set_cell_forward_digest(crypt_path_t *cpath, cell_t *cell)
 
 /************ cpath sendme API ***************************/
 
-/** Keep the current inbound cell digest for the next SENDME digest. This part
- * is only done by the client as the circuit came back from the Exit. */
-void
-cpath_sendme_circuit_record_inbound_cell(crypt_path_t *cpath)
-{
-  tor_assert(cpath);
-  relay_crypto_record_sendme_digest(&cpath->pvt_crypto);
-}
-
 /** Return the sendme_digest of this <b>cpath</b>. */
 uint8_t *
 cpath_get_sendme_digest(crypt_path_t *cpath)
@@ -220,6 +211,15 @@ cpath_get_sendme_digest(crypt_path_t *cpath)
   return relay_crypto_get_sendme_digest(&cpath->pvt_crypto);
 }
 
+/** Record the cell digest, indicated by is_foward_digest or not, as the
+ * SENDME cell digest. */
+void
+cpath_sendme_record_cell_digest(crypt_path_t *cpath, bool is_foward_digest)
+{
+  tor_assert(cpath);
+  relay_crypto_record_sendme_digest(&cpath->pvt_crypto, is_foward_digest);
+}
+
 /************ other cpath functions ***************************/
 
 /** Return the first non-open hop in cpath, or return NULL if all

diff --git a/src/core/or/crypt_path.h b/src/core/or/crypt_path.h
index 30c14b3dce4a12f331e538edf8d0f39014899509..9850610ef7d65cb2c13dc4d4844706033ebaba8e 100644 (file)
--- a/src/core/or/crypt_path.h
+++ b/src/core/or/crypt_path.h
@@ -27,6 +27,9 @@ cpath_crypt_cell(const crypt_path_t *cpath, uint8_t *payload, bool is_decrypt);
 struct crypto_digest_t *
 cpath_get_incoming_digest(const crypt_path_t *cpath);
 
+void cpath_sendme_record_cell_digest(crypt_path_t *cpath,
+                                     bool is_foward_digest);
+
 void
 cpath_set_cell_forward_digest(crypt_path_t *cpath, cell_t *cell);
 

diff --git a/src/core/or/relay.c b/src/core/or/relay.c
index 91236e5fec339363ac440c5749ff5cb862fbec54..beff225af430f9a9eaed58be1de5e0dd25b38ca7 100644 (file)
--- a/src/core/or/relay.c
+++ b/src/core/or/relay.c
@@ -247,6 +247,10 @@ circuit_receive_relay_cell(cell_t *cell, circuit_t *circ,
   if (recognized) {
     edge_connection_t *conn = NULL;
 
+    /* Recognized cell, the cell digest has been updated, we'll record it for
+     * the SENDME if need be. */
+    sendme_record_received_cell_digest(circ, layer_hint);
+
     if (circ->purpose == CIRCUIT_PURPOSE_PATH_BIAS_TESTING) {
       if (pathbias_check_probe_response(circ, cell) == -1) {
         pathbias_count_valid_cells(circ, cell);

diff --git a/src/core/or/sendme.c b/src/core/or/sendme.c
index 80d6b7d16528b407c418a7a269df9bae0983e2b7..eba7c37cdb9c29fade5176b2af5721a8833b82c3 100644 (file)
--- a/src/core/or/sendme.c
+++ b/src/core/or/sendme.c
@@ -306,15 +306,6 @@ record_cell_digest_on_circ(circuit_t *circ, const uint8_t *sendme_digest)
  * Public API
  */
 
-/** Keep the current inbound cell digest for the next SENDME digest. This part
- * is only done by the client as the circuit came back from the Exit. */
-void
-sendme_circuit_record_outbound_cell(or_circuit_t *or_circ)
-{
-  tor_assert(or_circ);
-  relay_crypto_record_sendme_digest(&or_circ->crypto);
-}
-
 /** Return true iff the next cell for the given cell window is expected to be
  * a SENDME.
  *
@@ -582,7 +573,8 @@ int
 sendme_note_stream_data_packaged(edge_connection_t *conn)
 {
   tor_assert(conn);
-  return --conn->package_window;
+  log_debug(LD_APP, "Stream package_window now %d.", --conn->package_window);
+  return conn->package_window;
 }
 
 /* Record the cell digest into the circuit sendme digest list depending on
@@ -619,3 +611,52 @@ sendme_record_cell_digest_on_circ(circuit_t *circ, crypt_path_t *cpath)
 
   record_cell_digest_on_circ(circ, sendme_digest);
 }
+
+/* Called once we decrypted a cell and recognized it. Record the cell digest
+ * as the next sendme digest only if the next cell we'll send on the circuit
+ * is expected to be a SENDME. */
+void
+sendme_record_received_cell_digest(circuit_t *circ, crypt_path_t *cpath)
+{
+  tor_assert(circ);
+
+  /* Only record if the next cell is expected to be a SENDME. */
+  if (!sendme_circuit_cell_is_next(cpath ? cpath->deliver_window :
+                                           circ->deliver_window)) {
+    return;
+  }
+
+  if (cpath) {
+    /* Record incoming digest. */
+    cpath_sendme_record_cell_digest(cpath, false);
+  } else {
+    /* Record foward digest. */
+    relay_crypto_record_sendme_digest(&TO_OR_CIRCUIT(circ)->crypto, true);
+  }
+}
+
+/* Called once we encrypted a cell. Record the cell digest as the next sendme
+ * digest only if the next cell we expect to receive is a SENDME so we can
+ * match the digests. */
+void
+sendme_record_sending_cell_digest(circuit_t *circ, crypt_path_t *cpath)
+{
+  tor_assert(circ);
+
+  /* Only record if the next cell is expected to be a SENDME. */
+  if (!sendme_circuit_cell_is_next(cpath ? cpath->package_window:
+                                           circ->package_window)) {
+    goto end;
+  }
+
+  if (cpath) {
+    /* Record the forward digest. */
+    cpath_sendme_record_cell_digest(cpath, true);
+  } else {
+    /* Record the incoming digest. */
+    relay_crypto_record_sendme_digest(&TO_OR_CIRCUIT(circ)->crypto, false);
+  }
+
+ end:
+  return;
+}

diff --git a/src/core/or/sendme.h b/src/core/or/sendme.h
index a71891996bebaaa0437b1945f3fd29c144ddb86c..847fcdd67269bef48a4388e025a4e170de996d76 100644 (file)
--- a/src/core/or/sendme.h
+++ b/src/core/or/sendme.h
@@ -34,10 +34,11 @@ int sendme_note_circuit_data_packaged(circuit_t *circ,
                                       crypt_path_t *layer_hint);
 int sendme_note_stream_data_packaged(edge_connection_t *conn);
 
-/* Track cell digest. */
-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);
+/* Record cell digest as the SENDME digest. */
+void sendme_record_received_cell_digest(circuit_t *circ, crypt_path_t *cpath);
+void sendme_record_sending_cell_digest(circuit_t *circ, crypt_path_t *cpath);
 
 /* Circuit level information. */
 bool sendme_circuit_cell_is_next(int window);