[tor-commits] [tor/master] prop289: Move digest matching in its own function

[asn at torproject.org]

commit 4efe9d653aa1d375d77d6dca83ca63787d6599d7
Author: David Goulet <dgoulet at torproject.org>
Date:   Thu Mar 7 12:01:58 2019 -0500

    prop289: Move digest matching in its own function
    
    No behavior change but code had to be refactored a bit. Also, the tor_memcmp()
    was changed to tor_memneq().
    
    Part of #26288
    
    Signed-off-by: David Goulet <dgoulet at torproject.org>
---
 src/core/or/sendme.c | 70 ++++++++++++++++++++++++++++++----------------------
 1 file changed, 40 insertions(+), 30 deletions(-)

diff --git a/src/core/or/sendme.c b/src/core/or/sendme.c
index 16ff5bcb8..0a7b1cbc0 100644
--- a/src/core/or/sendme.c
+++ b/src/core/or/sendme.c
@@ -59,23 +59,18 @@ get_accept_min_version(void)
                                  SENDME_ACCEPT_MIN_VERSION_MAX);
 }
 
-/* Return true iff the given decoded SENDME version 1 cell is valid and
- * matches the expected digest on the circuit.
- *
- * Validation is done by comparing the digest in the cell from the previous
- * cell we saw which tells us that the other side has in fact seen that cell.
- * See proposal 289 for more details. */
+/* Return true iff the given cell digest matches the first digest in the
+ * circuit sendme list. */
 static bool
-cell_v1_is_valid(const sendme_cell_t *cell, const circuit_t *circ)
+v1_digest_matches(const circuit_t *circ, const uint8_t *cell_digest)
 {
-  const uint8_t *cell_digest = NULL;
+  bool ret = false;
+  uint8_t *circ_digest = NULL;
 
-  tor_assert(cell);
   tor_assert(circ);
+  tor_assert(cell_digest);
 
-  cell_digest = sendme_cell_getconstarray_data_v1_digest(cell);
-
-  /* We shouldn't have received this SENDME if we have no digests. Log at
+  /* We shouldn't have received a SENDME if we have no digests. Log at
    * protocol warning because it can be tricked by sending many SENDMEs
    * without prior data cell. */
   if (circ->sendme_last_digests == NULL ||
@@ -83,29 +78,44 @@ cell_v1_is_valid(const sendme_cell_t *cell, const circuit_t *circ)
     log_fn(LOG_PROTOCOL_WARN, LD_PROTOCOL,
            "We received a SENDME but we have no cell digests to match. "
            "Closing circuit.");
-    goto invalid;
+    goto no_match;
   }
 
   /* Pop the first element that was added (FIFO) and compare it. */
-  {
-    uint8_t *digest = smartlist_get(circ->sendme_last_digests, 0);
-    smartlist_del_keeporder(circ->sendme_last_digests, 0);
-
-    /* Compare the digest with the one in the SENDME. This cell is invalid
-     * without a perfect match. */
-    if (tor_memcmp(digest, cell_digest, TRUNNEL_SENDME_V1_DIGEST_LEN)) {
-      tor_free(digest);
-      log_fn(LOG_PROTOCOL_WARN, LD_PROTOCOL,
-             "SENDME v1 cell digest do not match.");
-      goto invalid;
-    }
-    tor_free(digest);
+  circ_digest = smartlist_get(circ->sendme_last_digests, 0);
+  smartlist_del_keeporder(circ->sendme_last_digests, 0);
+
+  /* Compare the digest with the one in the SENDME. This cell is invalid
+   * without a perfect match. */
+  if (tor_memneq(circ_digest, cell_digest, TRUNNEL_SENDME_V1_DIGEST_LEN)) {
+    log_fn(LOG_PROTOCOL_WARN, LD_PROTOCOL,
+           "SENDME v1 cell digest do not match.");
+    goto no_match;
   }
+  /* Digests matches! */
+  ret = true;
+
+ no_match:
+  /* This digest was popped from the circuit list. Regardless of what happens,
+   * we have no more use for it. */
+  tor_free(circ_digest);
+  return ret;
+}
 
-  /* Validated SENDME v1 cell. */
-  return 1;
- invalid:
-  return 0;
+/* Return true iff the given decoded SENDME version 1 cell is valid and
+ * matches the expected digest on the circuit.
+ *
+ * Validation is done by comparing the digest in the cell from the previous
+ * cell we saw which tells us that the other side has in fact seen that cell.
+ * See proposal 289 for more details. */
+static bool
+cell_v1_is_valid(const sendme_cell_t *cell, const circuit_t *circ)
+{
+  tor_assert(cell);
+  tor_assert(circ);
+
+  const uint8_t *cell_digest = sendme_cell_getconstarray_data_v1_digest(cell);
+  return v1_digest_matches(circ, cell_digest);
 }
 
 /* Return true iff the given cell version can be handled or if the minimum