prop289: Keep the digest bytes, not the object

The digest object is as large as the entire internal digest object's state,
which is often much larger than the actual set of bytes you're transmitting.

This commit makes it that we keep the digest itself which is 20 bytes.

Part of #26288

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

diff --git a/src/core/crypto/relay_crypto.c b/src/core/crypto/relay_crypto.c
index d4116d47ab2a30739e40a6f59b29d167c46fedc2..94e90606513548d629445c702b45d6940c1db714 100644 (file)
--- a/src/core/crypto/relay_crypto.c
+++ b/src/core/crypto/relay_crypto.c
@@ -143,12 +143,9 @@ relay_decrypt_cell(circuit_t *circ, cell_t *cell,
             *recognized = 1;
             *layer_hint = thishop;
             /* Keep current digest of this cell for the possible SENDME. */
-            if (thishop->crypto.sendme_digest) {
-              crypto_digest_free(thishop->crypto.sendme_digest);
-            }
-            thishop->crypto.sendme_digest =
-              crypto_digest_dup(thishop->crypto.b_digest);
-
+            crypto_digest_get_digest(thishop->crypto.b_digest,
+                                     (char *) thishop->crypto.sendme_digest,
+                                     sizeof(thishop->crypto.sendme_digest));
             return 0;
           }
         }
@@ -220,10 +217,9 @@ relay_encrypt_cell_inbound(cell_t *cell,
 {
   relay_set_digest(or_circ->crypto.b_digest, cell);
   /* Keep a record of this cell, we might use it for validating the SENDME. */
-  if (or_circ->crypto.sendme_digest) {
-    crypto_digest_free(or_circ->crypto.sendme_digest);
-  }
-  or_circ->crypto.sendme_digest = crypto_digest_dup(or_circ->crypto.b_digest);
+  crypto_digest_get_digest(or_circ->crypto.b_digest,
+                           (char *) or_circ->crypto.sendme_digest,
+                           sizeof(or_circ->crypto.sendme_digest));
   /* encrypt one layer */
   relay_crypt_one_payload(or_circ->crypto.b_crypto, cell->payload);
 }
@@ -241,7 +237,6 @@ relay_crypto_clear(relay_crypto_t *crypto)
   crypto_cipher_free(crypto->b_crypto);
   crypto_digest_free(crypto->f_digest);
   crypto_digest_free(crypto->b_digest);
-  crypto_digest_free(crypto->sendme_digest);
 }
 
 /** Initialize <b>crypto</b> from the key material in key_data.

diff --git a/src/core/or/relay_crypto_st.h b/src/core/or/relay_crypto_st.h
index dbdf1599dc9d8137fa123b68fdeae4b183c15815..1f243ccdc8fb073ed669cb7e5f04013defec5b12 100644 (file)
--- a/src/core/or/relay_crypto_st.h
+++ b/src/core/or/relay_crypto_st.h
@@ -26,7 +26,7 @@ struct relay_crypto_t {
   struct crypto_digest_t *b_digest;
 
   /** Digest used for the next SENDME cell if any. */
-  struct crypto_digest_t *sendme_digest;
+  uint8_t sendme_digest[DIGEST_LEN];
 };
 #undef crypto_cipher_t
 

diff --git a/src/core/or/sendme.c b/src/core/or/sendme.c
index 0a7b1cbc0225af4bd23e8a54116d884b349e03cb..3dcd9df08ececb368efa586f66975a8c60583478 100644 (file)
--- a/src/core/or/sendme.c
+++ b/src/core/or/sendme.c
@@ -215,7 +215,7 @@ sendme_is_valid(const circuit_t *circ, const uint8_t *cell_payload,
  * Return the size in bytes of the encoded cell in payload. A negative value
  * is returned on encoding failure. */
 STATIC ssize_t
-build_cell_payload_v1(crypto_digest_t *cell_digest, uint8_t *payload)
+build_cell_payload_v1(const uint8_t *cell_digest, uint8_t *payload)
 {
   ssize_t len = -1;
   sendme_cell_t *cell = NULL;
@@ -231,9 +231,8 @@ build_cell_payload_v1(crypto_digest_t *cell_digest, uint8_t *payload)
   sendme_cell_set_data_len(cell, TRUNNEL_SENDME_V1_DIGEST_LEN);
 
   /* Copy the digest into the data payload. */
-  crypto_digest_get_digest(cell_digest,
-                           (char *) sendme_cell_getarray_data_v1_digest(cell),
-                           sendme_cell_get_data_len(cell));
+  memcpy(sendme_cell_getarray_data_v1_digest(cell), cell_digest,
+         sendme_cell_get_data_len(cell));
 
   /* Finally, encode the cell into the payload. */
   len = sendme_cell_encode(payload, RELAY_PAYLOAD_SIZE, cell);
@@ -249,7 +248,7 @@ build_cell_payload_v1(crypto_digest_t *cell_digest, uint8_t *payload)
  * because we failed to send the cell on it. */
 static int
 send_circuit_level_sendme(circuit_t *circ, crypt_path_t *layer_hint,
-                          crypto_digest_t *cell_digest)
+                          const uint8_t *cell_digest)
 {
   uint8_t emit_version;
   uint8_t payload[RELAY_PAYLOAD_SIZE];
@@ -340,7 +339,7 @@ sendme_connection_edge_consider_sending(edge_connection_t *conn)
 void
 sendme_circuit_consider_sending(circuit_t *circ, crypt_path_t *layer_hint)
 {
-  crypto_digest_t *digest;
+  const uint8_t *digest;
 
   while ((layer_hint ? layer_hint->deliver_window : circ->deliver_window) <=
           CIRCWINDOW_START - CIRCWINDOW_INCREMENT) {
@@ -539,7 +538,7 @@ sendme_note_stream_data_packaged(edge_connection_t *conn)
 void
 sendme_note_cell_digest(circuit_t *circ)
 {
-  uint8_t *digest;
+  const uint8_t *digest;
 
   tor_assert(circ);
 
@@ -555,16 +554,10 @@ sendme_note_cell_digest(circuit_t *circ)
     return;
   }
 
-  /* Only note the digest if we actually have the digest of the previous cell
-   * recorded. It should never happen in theory as we always record the last
-   * digest for the v1 SENDME. */
-  if (TO_OR_CIRCUIT(circ)->crypto.sendme_digest) {
-    digest = tor_malloc_zero(TRUNNEL_SENDME_V1_DIGEST_LEN);
-    crypto_digest_get_digest(TO_OR_CIRCUIT(circ)->crypto.sendme_digest,
-                             (char *) digest, TRUNNEL_SENDME_V1_DIGEST_LEN);
-    if (circ->sendme_last_digests == NULL) {
-      circ->sendme_last_digests = smartlist_new();
-    }
-    smartlist_add(circ->sendme_last_digests, digest);
+  /* Add the digest to the last seen list in the circuit. */
+  digest = TO_OR_CIRCUIT(circ)->crypto.sendme_digest;
+  if (circ->sendme_last_digests == NULL) {
+    circ->sendme_last_digests = smartlist_new();
   }
+  smartlist_add(circ->sendme_last_digests, tor_memdup(digest, DIGEST_LEN));
 }

diff --git a/src/core/or/sendme.h b/src/core/or/sendme.h
index 2154a29f4a183eff4d911e25e544941de5c66363..71df9b6f07ec66af4465905ed6a15019fc924dc5 100644 (file)
--- a/src/core/or/sendme.h
+++ b/src/core/or/sendme.h
@@ -50,7 +50,7 @@ STATIC int get_accept_min_version(void);
 
 STATIC bool cell_version_is_valid(uint8_t cell_version);
 
-STATIC ssize_t build_cell_payload_v1(crypto_digest_t *cell_digest,
+STATIC ssize_t build_cell_payload_v1(const uint8_t *cell_digest,
                                      uint8_t *payload);
 STATIC bool sendme_is_valid(const circuit_t *circ,
                             const uint8_t *cell_payload,

diff --git a/src/test/test_relaycell.c b/src/test/test_relaycell.c
index c4ed215c7c6ef87144c9bf902a7e68c763472159..0623583511f8271437b7b2cb1ef53c61243e388f 100644 (file)
--- a/src/test/test_relaycell.c
+++ b/src/test/test_relaycell.c
@@ -705,7 +705,6 @@ test_circbw_relay(void *arg)
   circ = helper_create_origin_circuit(CIRCUIT_PURPOSE_C_GENERAL, 0);
   circ->cpath->state = CPATH_STATE_AWAITING_KEYS;
   circ->cpath->deliver_window = CIRCWINDOW_START;
-  circ->cpath->crypto.sendme_digest = crypto_digest_new();
 
   entryconn1 = fake_entry_conn(circ, 1);
   edgeconn = ENTRY_TO_EDGE_CONN(entryconn1);

diff --git a/src/test/test_sendme.c b/src/test/test_sendme.c
index 92f478df0e172a610e316896a79b11ed3a47788c..d6410a748828ee153813481b2438db83790f8d27 100644 (file)
--- a/src/test/test_sendme.c
+++ b/src/test/test_sendme.c
@@ -16,6 +16,8 @@
 #include "feature/nodelist/networkstatus.h"
 #include "feature/nodelist/networkstatus_st.h"
 
+#include "lib/crypt_ops/crypto_digest.h"
+
 #include "test/test.h"
 #include "test/log_test_helpers.h"
 
@@ -64,7 +66,6 @@ test_v1_note_digest(void *arg)
   circuit_free_(circ);
   /* Points it to the OR circuit now. */
   circ = TO_CIRCUIT(or_circ);
-  or_circ->crypto.sendme_digest = crypto_digest_new();
 
   /* The package window has to be a multiple of CIRCWINDOW_INCREMENT minus 1
    * in order to catched the CIRCWINDOW_INCREMENT-nth cell. Try something that
@@ -144,7 +145,7 @@ test_v1_consensus_params(void *arg)
 static void
 test_v1_build_cell(void *arg)
 {
-  uint8_t payload[RELAY_PAYLOAD_SIZE];
+  uint8_t payload[RELAY_PAYLOAD_SIZE], digest[DIGEST_LEN];
   ssize_t ret;
   crypto_digest_t *cell_digest = NULL;
   or_circuit_t *or_circ = NULL;
@@ -156,11 +157,12 @@ test_v1_build_cell(void *arg)
   circ = TO_CIRCUIT(or_circ);
 
   cell_digest = crypto_digest_new();
-  crypto_digest_add_bytes(cell_digest, "AAAAAAAAAAAAAAAAAAAA", 20);
   tt_assert(cell_digest);
+  crypto_digest_add_bytes(cell_digest, "AAAAAAAAAAAAAAAAAAAA", 20);
+  crypto_digest_get_digest(cell_digest, (char *) digest, sizeof(digest));
 
   /* SENDME v1 payload is 3 bytes + 20 bytes digest. See spec. */
-  ret = build_cell_payload_v1(cell_digest, payload);
+  ret = build_cell_payload_v1(digest, payload);
   tt_int_op(ret, OP_EQ, 23);
 
   /* Validation. */
@@ -183,7 +185,6 @@ test_v1_build_cell(void *arg)
   teardown_capture_of_logs();
 
   /* Note the wrong digest in the circuit, cell should fail validation. */
-  or_circ->crypto.sendme_digest = crypto_digest_new();
   circ->package_window = CIRCWINDOW_INCREMENT + 1;
   sendme_note_cell_digest(circ);
   tt_int_op(smartlist_len(circ->sendme_last_digests), OP_EQ, 1);
@@ -194,11 +195,8 @@ test_v1_build_cell(void *arg)
   expect_log_msg_containing("SENDME v1 cell digest do not match.");
   teardown_capture_of_logs();
 
-  /* Cleanup */
-  crypto_digest_free(or_circ->crypto.sendme_digest);
-
   /* Record the cell digest into the circuit, cell should validate. */
-  or_circ->crypto.sendme_digest = crypto_digest_dup(cell_digest);
+  memcpy(or_circ->crypto.sendme_digest, digest, sizeof(digest));
   circ->package_window = CIRCWINDOW_INCREMENT + 1;
   sendme_note_cell_digest(circ);
   tt_int_op(smartlist_len(circ->sendme_last_digests), OP_EQ, 1);