From: Nick Mathewson <nickm@torproject.org>
Date: Thu, 5 Jun 2025 20:01:28 +0000 (-0400)
Subject: Make SENDME tag lengths defined in a single place.
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=a13e0dd1a38d9e12a7840906bdab41d447bc6b57;p=thirdparty%2Ftor.git

Make SENDME tag lengths defined in a single place.
---

diff --git a/src/core/crypto/relay_crypto.c b/src/core/crypto/relay_crypto.c
index 1c01b14c08..f9d72a1f48 100644
--- a/src/core/crypto/relay_crypto.c
+++ b/src/core/crypto/relay_crypto.c
@@ -42,10 +42,10 @@ relay_crypto_get_sendme_tag(relay_crypto_t *crypto,
   tor_assert(crypto);
   switch (crypto->kind) {
     case RCK_TOR1:
-      *len_out = DIGEST_LEN;
+      *len_out = SENDME_TAG_LEN_TOR1;
       return crypto->c.tor1.sendme_digest;
     case RCK_CGO:
-      *len_out = CGO_TAG_LEN;
+      *len_out = SENDME_TAG_LEN_CGO;
       return crypto->c.cgo.last_tag;
   }
   tor_assert_unreached();
@@ -65,7 +65,7 @@ relay_crypt_client_backward(relay_crypto_t *crypto, cell_t *cell)
       const uint8_t *tag = NULL;
       cgo_crypt_client_backward(crypto->c.cgo.back, cell, &tag);
       if (tag != NULL) {
-        memcpy(crypto->c.cgo.last_tag, tag, CGO_TAG_LEN);
+        memcpy(crypto->c.cgo.last_tag, tag, SENDME_TAG_LEN_CGO);
         return true;
       } else {
         return false;
@@ -89,7 +89,7 @@ relay_crypt_relay_forward(relay_crypto_t *crypto, cell_t *cell)
       const uint8_t *tag = NULL;
       cgo_crypt_relay_forward(crypto->c.cgo.fwd, cell, &tag);
       if (tag != NULL) {
-        memcpy(crypto->c.cgo.last_tag, tag, CGO_TAG_LEN);
+        memcpy(crypto->c.cgo.last_tag, tag, SENDME_TAG_LEN_CGO);
         return true;
       } else {
         return false;
@@ -198,7 +198,7 @@ relay_crypt_client_originate(relay_crypto_t *crypto, cell_t *cell)
       const uint8_t *tag = NULL;
       cgo_crypt_client_originate(crypto->c.cgo.fwd, cell, &tag);
       tor_assert(tag);
-      memcpy(crypto->c.cgo.last_tag, tag, CGO_TAG_LEN);
+      memcpy(crypto->c.cgo.last_tag, tag, SENDME_TAG_LEN_CGO);
       break;
     }
   }
@@ -262,7 +262,7 @@ relay_encrypt_cell_inbound(cell_t *cell,
       const uint8_t *tag = NULL;
       cgo_crypt_relay_originate(crypto->c.cgo.back, cell, &tag);
       tor_assert(tag);
-      memcpy(&crypto->c.cgo.last_tag, tag, CGO_TAG_LEN);
+      memcpy(&crypto->c.cgo.last_tag, tag, SENDME_TAG_LEN_CGO);
       break;
     }
   }
diff --git a/src/core/crypto/relay_crypto_cgo.c b/src/core/crypto/relay_crypto_cgo.c
index 9dbc6a65ff..9dd271a295 100644
--- a/src/core/crypto/relay_crypto_cgo.c
+++ b/src/core/crypto/relay_crypto_cgo.c
@@ -362,7 +362,7 @@ size_t
 cgo_key_material_len(int aesbits)
 {
   tor_assert(aesbits == 128 || aesbits == 192 || aesbits == 256);
-  size_t r = (cgo_uiv_keylen(aesbits) + CGO_TAG_LEN);
+  size_t r = (cgo_uiv_keylen(aesbits) + SENDME_TAG_LEN_CGO);
   tor_assert(r * 2 <= MAX_RELAY_KEY_MATERIAL_LEN);
   return r;
 }
@@ -391,8 +391,8 @@ cgo_crypt_new(cgo_mode_t mode, int aesbits, const uint8_t *keys, size_t keylen)
   r = cgo_uiv_init(&cgo->uiv, aesbits, encrypt, keys);
   tor_assert(r == 0);
   keys += cgo_uiv_keylen(aesbits);
-  memcpy(cgo->nonce, keys, CGO_TAG_LEN);
-  keys += CGO_TAG_LEN;
+  memcpy(cgo->nonce, keys, SENDME_TAG_LEN_CGO);
+  keys += SENDME_TAG_LEN_CGO;
   tor_assert(keys == end_of_keys);
 
   cgo->aes_bytes = aesbits / 8;
@@ -428,7 +428,7 @@ cgo_crypt_update(cgo_crypt_t *cgo, cgo_mode_t mode)
  * process an outbound cell from the client.
  *
  * If the cell is for this relay, set *'recognized_tag_out'
- * to point to a CGO_TAG_LEN value that should be used
+ * to point to a SENDME_TAG_LEN_CGO value that should be used
  * if we want to acknowledge this cell with an authenticated SENDME.
  *
  * The value of 'recognized_tag_out' will become invalid
@@ -445,10 +445,10 @@ cgo_crypt_relay_forward(cgo_crypt_t *cgo, cell_t *cell,
     .h = cgo->tprime,
     .cmd = cell->command,
   };
-  memcpy(cgo->last_tag_relay_fwd, cell->payload, CGO_TAG_LEN);
+  memcpy(cgo->last_tag_relay_fwd, cell->payload, SENDME_TAG_LEN_CGO);
   cgo_uiv_encrypt(&cgo->uiv, h, cell->payload);
-  memcpy(cgo->tprime, cell->payload, CGO_TAG_LEN);
-  if (tor_memeq(cell->payload, cgo->nonce, CGO_TAG_LEN)) {
+  memcpy(cgo->tprime, cell->payload, SENDME_TAG_LEN_CGO);
+  if (tor_memeq(cell->payload, cgo->nonce, SENDME_TAG_LEN_CGO)) {
     cgo_crypt_update(cgo, CGO_MODE_RELAY_FORWARD);
     *recognized_tag_out = cgo->last_tag_relay_fwd;
   } else {
@@ -468,7 +468,7 @@ cgo_crypt_relay_backward(cgo_crypt_t *cgo, cell_t *cell)
     .cmd = cell->command,
   };
   cgo_uiv_encrypt(&cgo->uiv, h, cell->payload);
-  memcpy(cgo->tprime, cell->payload, CGO_TAG_LEN);
+  memcpy(cgo->tprime, cell->payload, SENDME_TAG_LEN_CGO);
 }
 
 /**
@@ -476,7 +476,7 @@ cgo_crypt_relay_backward(cgo_crypt_t *cgo, cell_t *cell)
  * encrypt an inbound message that we are originating, for the client.
  *
  * The provided cell must have its command value set,
- * and should have the first CGO_TAG_LEN bytes of its payload unused.
+ * and should have the first SENDME_TAG_LEN_CGO bytes of its payload unused.
  *
  * Set '*tag_out' to a value that we should expect
  * if we want an authenticated SENDME for this cell.
@@ -493,12 +493,12 @@ cgo_crypt_relay_originate(cgo_crypt_t *cgo, cell_t *cell,
     .h = cgo->tprime,
     .cmd = cell->command,
   };
-  memcpy(cell->payload, cgo->nonce, CGO_TAG_LEN);
+  memcpy(cell->payload, cgo->nonce, SENDME_TAG_LEN_CGO);
   cgo_uiv_encrypt(&cgo->uiv, h, cell->payload);
-  memcpy(&cgo->tprime, cell->payload, CGO_TAG_LEN);
-  memcpy(&cgo->nonce, cell->payload, CGO_TAG_LEN);
+  memcpy(&cgo->tprime, cell->payload, SENDME_TAG_LEN_CGO);
+  memcpy(&cgo->nonce, cell->payload, SENDME_TAG_LEN_CGO);
   if (tag_out) {
-    // tor_assert(tor_memeq(cgo->tprime, cell->payload, CGO_TAG_LEN));
+    // tor_assert(tor_memeq(cgo->tprime, cell->payload, SENDME_TAG_LEN_CGO));
     *tag_out = cgo->tprime;
   }
   cgo_crypt_update(cgo, CGO_MODE_RELAY_BACKWARD);
@@ -511,14 +511,14 @@ cgo_crypt_relay_originate(cgo_crypt_t *cgo, cell_t *cell,
 void
 cgo_crypt_client_forward(cgo_crypt_t *cgo, cell_t *cell)
 {
-  uint8_t tprime_new[CGO_TAG_LEN];
-  memcpy(tprime_new, cell->payload, CGO_TAG_LEN);
+  uint8_t tprime_new[SENDME_TAG_LEN_CGO];
+  memcpy(tprime_new, cell->payload, SENDME_TAG_LEN_CGO);
   uiv_tweak_t h = {
     .h = cgo->tprime,
     .cmd = cell->command,
   };
   cgo_uiv_decrypt(&cgo->uiv, h, cell->payload);
-  memcpy(cgo->tprime, tprime_new, CGO_TAG_LEN);
+  memcpy(cgo->tprime, tprime_new, SENDME_TAG_LEN_CGO);
 }
 
 /**
@@ -526,7 +526,7 @@ cgo_crypt_client_forward(cgo_crypt_t *cgo, cell_t *cell)
  * originate a cell for a given target hop.
  *
  * The provided cell must have its command value set,
- * and should have the first CGO_TAG_LEN bytes of its payload unused.
+ * and should have the first SENDME_TAG_LEN_CGO bytes of its payload unused.
  *
  * Set '*tag_out' to a value that we should expect
  * if we want an authenticated SENDME for this cell.
@@ -539,7 +539,7 @@ void
 cgo_crypt_client_originate(cgo_crypt_t *cgo, cell_t *cell,
                            const uint8_t **tag_out)
 {
-  memcpy(cell->payload, cgo->nonce, CGO_TAG_LEN);
+  memcpy(cell->payload, cgo->nonce, SENDME_TAG_LEN_CGO);
   cgo_crypt_client_forward(cgo, cell);
   cgo_crypt_update(cgo, CGO_MODE_CLIENT_FORWARD);
   *tag_out = cell->payload;
@@ -550,7 +550,7 @@ cgo_crypt_client_originate(cgo_crypt_t *cgo, cell_t *cell,
  * process an inbound cell from a relay.
  *
  * If the cell originated from this this relay, set *'recognized_tag_out'
- * to point to a CGO_TAG_LEN value that should be used
+ * to point to a SENDME_TAG_LEN_CGO value that should be used
  * if we want to acknowledge this cell with an authenticated SENDME.
  *
  * The value of 'recognized_tag_out' will become invalid
@@ -567,15 +567,15 @@ cgo_crypt_client_backward(cgo_crypt_t *cgo, cell_t *cell,
     .h = cgo->tprime,
     .cmd = cell->command,
   };
-  uint8_t t_orig[CGO_TAG_LEN];
-  memcpy(t_orig, cell->payload, CGO_TAG_LEN);
+  uint8_t t_orig[SENDME_TAG_LEN_CGO];
+  memcpy(t_orig, cell->payload, SENDME_TAG_LEN_CGO);
 
   cgo_uiv_decrypt(&cgo->uiv, h, cell->payload);
-  memcpy(cgo->tprime, t_orig, CGO_TAG_LEN);
-  if (tor_memeq(cell->payload, cgo->nonce, CGO_TAG_LEN)) {
-    memcpy(cgo->nonce, t_orig, CGO_TAG_LEN);
+  memcpy(cgo->tprime, t_orig, SENDME_TAG_LEN_CGO);
+  if (tor_memeq(cell->payload, cgo->nonce, SENDME_TAG_LEN_CGO)) {
+    memcpy(cgo->nonce, t_orig, SENDME_TAG_LEN_CGO);
     cgo_crypt_update(cgo, CGO_MODE_CLIENT_BACKWARD);
-    // tor_assert(tor_memeq(cgo->tprime, t_orig, CGO_TAG_LEN));
+    // tor_assert(tor_memeq(cgo->tprime, t_orig, SENDME_TAG_LEN_CGO));
     *recognized_tag_out = cgo->tprime;
   } else {
     *recognized_tag_out = NULL;
diff --git a/src/core/crypto/relay_crypto_cgo.h b/src/core/crypto/relay_crypto_cgo.h
index ef1ffb0f8c..95762ae66f 100644
--- a/src/core/crypto/relay_crypto_cgo.h
+++ b/src/core/crypto/relay_crypto_cgo.h
@@ -29,13 +29,6 @@ typedef enum {
   CGO_MODE_RELAY_BACKWARD,
 } cgo_mode_t;
 
-/**
- * Length of a CGO cell tag.
- *
- * This is the value used for authenticated SENDMES.
- **/
-#define CGO_TAG_LEN 16
-
 struct cell_t;
 
 size_t cgo_key_material_len(int aesbits);
@@ -198,14 +191,14 @@ STATIC void cgo_uiv_clear(cgo_uiv_t *uiv);
 
 struct cgo_crypt_t {
   cgo_uiv_t uiv;
-  uint8_t nonce[CGO_TAG_LEN];
-  uint8_t tprime[CGO_TAG_LEN];
+  uint8_t nonce[SENDME_TAG_LEN_CGO];
+  uint8_t tprime[SENDME_TAG_LEN_CGO];
   /**
    * Stored version of the last incoming cell tag.
    * Only used for cgo_crypt_relay_fwd, where this information is not
    * otherwise available after encryption.
    */
-  uint8_t last_tag_relay_fwd[CGO_TAG_LEN];
+  uint8_t last_tag_relay_fwd[SENDME_TAG_LEN_CGO];
   uint8_t aes_bytes;
 };
 #endif
diff --git a/src/core/crypto/relay_crypto_st.h b/src/core/crypto/relay_crypto_st.h
index a7fcbcb081..71a5933e36 100644
--- a/src/core/crypto/relay_crypto_st.h
+++ b/src/core/crypto/relay_crypto_st.h
@@ -26,7 +26,7 @@ typedef struct cgo_pair_t {
   cgo_crypt_t *fwd;
   cgo_crypt_t *back;
   /* The last tag that we got when originating or recognizing a message */
-  uint8_t last_tag[CGO_TAG_LEN];
+  uint8_t last_tag[SENDME_TAG_LEN_CGO];
 } cgo_pair_t;
 
 struct relay_crypto_t {
diff --git a/src/core/or/or.h b/src/core/or/or.h
index 0d8f63da02..24c427453c 100644
--- a/src/core/or/or.h
+++ b/src/core/or/or.h
@@ -451,6 +451,11 @@ typedef enum {
 /** Amount to increment a stream window when we get a stream SENDME. */
 #define STREAMWINDOW_INCREMENT 50
 
+/** Length for authenticated sendme tag with tor1 encryption. */
+#define SENDME_TAG_LEN_TOR1 20
+/** Length for authenticated sendme tag with cgo encryption. */
+#define SENDME_TAG_LEN_CGO 16
+
 /** Maximum number of queued cells on a circuit for which we are the
  * midpoint before we give up and kill it.  This must be >= circwindow
  * to avoid killing innocent circuits, and >= circwindow*2 to give
diff --git a/src/core/or/sendme.c b/src/core/or/sendme.c
index 77d9518048..07b2f3984b 100644
--- a/src/core/or/sendme.c
+++ b/src/core/or/sendme.c
@@ -27,16 +27,13 @@
 #include "lib/ctime/di_ops.h"
 #include "trunnel/sendme_cell.h"
 
-#define SHORT_TAG_LEN 16
-#define LONG_TAG_LEN 20
-
 /**
  * Return true iff tag_len is some length we recognize.
  */
 static inline bool
 tag_len_ok(size_t tag_len)
 {
-  return tag_len == SHORT_TAG_LEN || tag_len == LONG_TAG_LEN;
+  return tag_len == SENDME_TAG_LEN_CGO || tag_len == SENDME_TAG_LEN_TOR1;
 }
 
 /* Return the minimum version given by the consensus (if any) that should be
@@ -346,12 +343,12 @@ record_cell_digest_on_circ(circuit_t *circ,
   // We always allocate the largest possible tag here to
   // make sure we don't have heap overflow bugs.
   uint8_t *tag;
-  if (tag_len == SHORT_TAG_LEN) {
-    tag = tor_malloc_zero(LONG_TAG_LEN);
+  if (tag_len == SENDME_TAG_LEN_CGO) {
+    tag = tor_malloc_zero(SENDME_TAG_LEN_TOR1);
     memcpy(tag, sendme_tag, tag_len);
     // (The final bytes were initialized to zero.)
-  } else if (tag_len == LONG_TAG_LEN) {
-    tag = tor_memdup(sendme_tag, LONG_TAG_LEN);
+  } else if (tag_len == SENDME_TAG_LEN_TOR1) {
+    tag = tor_memdup(sendme_tag, SENDME_TAG_LEN_TOR1);
   } else {
     tor_assert_unreached();
   }
diff --git a/src/test/test_crypto_cgo.c b/src/test/test_crypto_cgo.c
index 3e4c44d3c5..8bc55e16ce 100644
--- a/src/test/test_crypto_cgo.c
+++ b/src/test/test_crypto_cgo.c
@@ -335,7 +335,7 @@ test_crypto_cgo_fwd(void *arg)
     for (int trial = 0; trial < 64; ++trial) {
       int target_hop = crypto_rand_int(3);
       cell_t cell, cell_orig;
-      uint8_t tag_client[CGO_TAG_LEN];
+      uint8_t tag_client[SENDME_TAG_LEN_CGO];
       const uint8_t *tagp = NULL;
 
       memset(&cell, 0, sizeof(cell));
@@ -344,14 +344,14 @@ test_crypto_cgo_fwd(void *arg)
       } else {
         cell.command = CELL_RELAY_EARLY;
       }
-      crypto_rand((char*) cell.payload+CGO_TAG_LEN,
-                  sizeof(cell.payload)-CGO_TAG_LEN);
+      crypto_rand((char*) cell.payload+SENDME_TAG_LEN_CGO,
+                  sizeof(cell.payload)-SENDME_TAG_LEN_CGO);
       memcpy(&cell_orig, &cell, sizeof(cell));
 
       // First the client encrypts the cell...
       cgo_crypt_client_originate(client[target_hop], &cell, &tagp);
       tt_assert(tagp);
-      memcpy(tag_client, tagp, CGO_TAG_LEN);
+      memcpy(tag_client, tagp, SENDME_TAG_LEN_CGO);
       for (int i = target_hop - 1; i >= 0; --i) {
         cgo_crypt_client_forward(client[i], &cell);
       }
@@ -363,16 +363,16 @@ test_crypto_cgo_fwd(void *arg)
         cgo_crypt_relay_forward(relays[i], &cell, &tagp);
         if (tagp) {
           tt_int_op(i, OP_EQ, target_hop);
-          tt_mem_op(tagp, OP_EQ, tag_client, CGO_TAG_LEN);
+          tt_mem_op(tagp, OP_EQ, tag_client, SENDME_TAG_LEN_CGO);
           cell_recognized = true;
           break;
         }
       }
       tt_assert(cell_recognized);
       tt_int_op(cell.command, OP_EQ, cell_orig.command);
-      tt_mem_op(cell.payload + CGO_TAG_LEN, OP_EQ,
-                cell_orig.payload + CGO_TAG_LEN,
-                sizeof(cell.payload) - CGO_TAG_LEN);
+      tt_mem_op(cell.payload + SENDME_TAG_LEN_CGO, OP_EQ,
+                cell_orig.payload + SENDME_TAG_LEN_CGO,
+                sizeof(cell.payload) - SENDME_TAG_LEN_CGO);
     }
     for (int i = 0; i < N_HOPS; ++i) {
       cgo_crypt_free(client[i]);
@@ -416,19 +416,19 @@ test_crypto_cgo_rev(void *arg)
     for (int trial = 0; trial < 64; ++trial) {
       int origin_hop = crypto_rand_int(3);
       cell_t cell, cell_orig;
-      uint8_t tag_relay[CGO_TAG_LEN];
+      uint8_t tag_relay[SENDME_TAG_LEN_CGO];
       const uint8_t *tagp = NULL;
 
       memset(&cell, 0, sizeof(cell));
       cell.command = CELL_RELAY;
-      crypto_rand((char*) cell.payload+CGO_TAG_LEN,
-                  sizeof(cell.payload)-CGO_TAG_LEN);
+      crypto_rand((char*) cell.payload+SENDME_TAG_LEN_CGO,
+                  sizeof(cell.payload)-SENDME_TAG_LEN_CGO);
       memcpy(&cell_orig, &cell, sizeof(cell));
 
       // First the specified relay encrypts the cell...
       cgo_crypt_relay_originate(relays[origin_hop], &cell, &tagp);
       tt_assert(tagp);
-      memcpy(tag_relay, tagp, CGO_TAG_LEN);
+      memcpy(tag_relay, tagp, SENDME_TAG_LEN_CGO);
       for (int i = origin_hop - 1; i >= 0; --i) {
         cgo_crypt_relay_backward(relays[i], &cell);
       }
@@ -440,16 +440,16 @@ test_crypto_cgo_rev(void *arg)
         cgo_crypt_client_backward(client[i], &cell, &tagp);
         if (tagp) {
           tt_int_op(i, OP_EQ, origin_hop);
-          tt_mem_op(tagp, OP_EQ, tag_relay, CGO_TAG_LEN);
+          tt_mem_op(tagp, OP_EQ, tag_relay, SENDME_TAG_LEN_CGO);
           cell_recognized = true;
           break;
         }
       }
       tt_assert(cell_recognized);
       tt_int_op(cell.command, OP_EQ, cell_orig.command);
-      tt_mem_op(cell.payload + CGO_TAG_LEN, OP_EQ,
-                cell_orig.payload + CGO_TAG_LEN,
-                sizeof(cell.payload) - CGO_TAG_LEN);
+      tt_mem_op(cell.payload + SENDME_TAG_LEN_CGO, OP_EQ,
+                cell_orig.payload + SENDME_TAG_LEN_CGO,
+                sizeof(cell.payload) - SENDME_TAG_LEN_CGO);
     }
     for (int i = 0; i < N_HOPS; ++i) {
       cgo_crypt_free(client[i]);
@@ -474,7 +474,8 @@ test_crypto_cgo_relay_testvec(void *arg)
   for (int i = 0; i < (int)ARRAY_LENGTH(CGO_RELAY_TESTVECS); ++i) {
     const struct cgo_relay_testvec *tv = &CGO_RELAY_TESTVECS[i];
     const int aesbits = 128;
-    uint8_t keys[80], expect_keys[80], expect_tprime[CGO_TAG_LEN], cmd[1];
+    uint8_t keys[80], expect_keys[80], expect_tprime[SENDME_TAG_LEN_CGO],
+      cmd[1];
     cell_t cell;
     cell_t expect_cell;
     tt_int_op(sizeof(keys), OP_EQ, cgo_key_material_len(aesbits));
@@ -527,7 +528,8 @@ test_crypto_cgo_relay_originate_testvec(void *arg)
     const struct cgo_relay_originate_testvec *tv =
       &CGO_RELAY_ORIGINATE_TESTVECS[i];
     const int aesbits = 128;
-    uint8_t keys[80], expect_keys[80], expect_tprime[CGO_TAG_LEN], cmd[1];
+    uint8_t keys[80], expect_keys[80], expect_tprime[SENDME_TAG_LEN_CGO],
+      cmd[1];
     cell_t cell;
     cell_t expect_cell;
     tt_int_op(sizeof(keys), OP_EQ, cgo_key_material_len(aesbits));
@@ -575,7 +577,8 @@ test_crypto_cgo_client_originate_testvec(void *arg)
     const struct cgo_client_originate_testvec *tv =
       &CGO_CLIENT_ORIGINATE_TESTVECS[tv_i];
     const int aesbits = 128;
-    uint8_t keys[80], expect_keys[80], expect_tprime[CGO_TAG_LEN], cmd[1];
+    uint8_t keys[80], expect_keys[80], expect_tprime[SENDME_TAG_LEN_CGO],
+      cmd[1];
     cell_t cell;
     cell_t expect_cell;
     for (int i = 0; i < 3; ++i) {