From: Nick Mathewson <nickm@torproject.org>
Date: Wed, 23 Apr 2025 15:27:07 +0000 (-0400)
Subject: CGO: Fix authenticated-sendme tag handling.
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=19e1afacab5027e5e9da4ed0b31bc0300d034c1b;p=thirdparty%2Ftor.git

CGO: Fix authenticated-sendme tag handling.

See discussion at torspec#328: it's important that our
SENDME authentication tag always be taken based on the
_encrypted_ cell.
---

diff --git a/src/core/crypto/relay_crypto_cgo.c b/src/core/crypto/relay_crypto_cgo.c
index 5ff8346b9f..079b867fbd 100644
--- a/src/core/crypto/relay_crypto_cgo.c
+++ b/src/core/crypto/relay_crypto_cgo.c
@@ -425,14 +425,12 @@ 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);
   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)) {
     cgo_crypt_update(cgo, CGO_MODE_RELAY_FORWARD);
-    // XXXX: Here and in Arti, we've used tprime as the value
-    // of our tag, but the proposal says to use T.  We should
-    // fix that, unless the CGO implementors say it's better!
-    *recognized_tag_out = cgo->tprime;
+    *recognized_tag_out = cgo->last_tag_relay_fwd;
   } else {
     *recognized_tag_out = NULL;
   }
@@ -480,9 +478,7 @@ cgo_crypt_relay_originate(cgo_crypt_t *cgo, cell_t *cell,
   memcpy(&cgo->tprime, cell->payload, CGO_TAG_LEN);
   memcpy(&cgo->nonce, cell->payload, CGO_TAG_LEN);
   if (tag_out) {
-    // XXXX: Here and elsewhere, we've used tprime as the value
-    // of our tag, but the proposal says to use T.  We should
-    // fix that, unless the CGO implementors say it's better!
+    // tor_assert(tor_memeq(cgo->tprime, cell->payload, CGO_TAG_LEN));
     *tag_out = cgo->tprime;
   }
   cgo_crypt_update(cgo, CGO_MODE_RELAY_BACKWARD);
@@ -526,10 +522,7 @@ cgo_crypt_client_originate(cgo_crypt_t *cgo, cell_t *cell,
   memcpy(cell->payload, cgo->nonce, CGO_TAG_LEN);
   cgo_crypt_client_forward(cgo, cell);
   cgo_crypt_update(cgo, CGO_MODE_CLIENT_FORWARD);
-  // XXXX: Here and elsewhere, we've used tprime as the value
-  // of our tag, but the proposal says to use T.  We should
-  // fix that, unless the CGO implementors say it's better!
-  *tag_out = cgo->tprime;
+  *tag_out = cell->payload;
 }
 
 /**
@@ -562,9 +555,7 @@ cgo_crypt_client_backward(cgo_crypt_t *cgo, cell_t *cell,
   if (tor_memeq(cell->payload, cgo->nonce, CGO_TAG_LEN)) {
     memcpy(cgo->nonce, t_orig, CGO_TAG_LEN);
     cgo_crypt_update(cgo, CGO_MODE_CLIENT_BACKWARD);
-    // XXXX: Here and elsewhere, we've used tprime as the value
-    // of our tag, but the proposal says to use T.  We should
-    // fix that, unless the CGO implementors say it's better!
+    // tor_assert(tor_memeq(cgo->tprime, t_orig, CGO_TAG_LEN));
     *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 fd3f89288f..208d7ef2bc 100644
--- a/src/core/crypto/relay_crypto_cgo.h
+++ b/src/core/crypto/relay_crypto_cgo.h
@@ -205,9 +205,13 @@ struct cgo_crypt_t {
   cgo_uiv_t uiv;
   uint8_t nonce[CGO_TAG_LEN];
   uint8_t tprime[CGO_TAG_LEN];
-  uint8_t aes_bytes;
-
+  /**
+   * 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 aes_bytes;
 };
 #endif