[tor-commits] [tor/master] sendme: Add non fatal asserts for extra safety

[nickm at torproject.org]

commit 0cad83bea4019b3c448756d718f5b65718e81f6e
Author: David Goulet <dgoulet at torproject.org>
Date:   Wed May 22 11:05:36 2019 -0400

    sendme: Add non fatal asserts for extra safety
    
    Two non fatal asserts are added in this commit. First one is to see if the
    SENDME digest list kept on the circuit for validation ever grows bigger than
    the maximum number of expected SENDME on a circuit (currently 10).
    
    The second one is to know if we ever send more than one SENDME at a time on a
    circuit. In theory, we shouldn't but if we ever do, the v1 implementation
    wouldn't work because we only keep one single cell digest (the previous cell
    to the SENDME) on the circuit/cpath. Thus, sending two SENDME consecutively
    will lead to a mismatch on the other side because the same cell digest would
    be use and thus the circuit would collapse.
    
    Finally, add an extra debug log in case we emit a v0 which also includes the
    consensus emit version in that case.
    
    Part of #30428
    
    Signed-off-by: David Goulet <dgoulet at torproject.org>
---
 src/core/or/sendme.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/src/core/or/sendme.c b/src/core/or/sendme.c
index bfbb65e85..98ed63994 100644
--- a/src/core/or/sendme.c
+++ b/src/core/or/sendme.c
@@ -64,6 +64,13 @@ pop_first_cell_digest(const circuit_t *circ)
     return NULL;
   }
 
+  /* More cell digest than the SENDME window is never suppose to happen. The
+   * cell should have been rejected before reaching this point due to its
+   * package_window down to 0 leading to a circuit close. Scream loudly but
+   * still pop the element so we don't memory leak. */
+  tor_assert_nonfatal(smartlist_len(circ->sendme_last_digests) <=
+                      CIRCWINDOW_START_MAX / CIRCWINDOW_INCREMENT);
+
   circ_digest = smartlist_get(circ->sendme_last_digests, 0);
   smartlist_del_keeporder(circ->sendme_last_digests, 0);
   return circ_digest;
@@ -290,6 +297,8 @@ send_circuit_level_sendme(circuit_t *circ, crypt_path_t *layer_hint,
   default:
     /* Unknown version, fallback to version 0 meaning no payload. */
     payload_len = 0;
+    log_debug(LD_PROTOCOL, "Emitting SENDME version 0 cell. "
+                           "Consensus emit version is %d", emit_version);
     break;
   }
 
@@ -408,6 +417,7 @@ sendme_connection_edge_consider_sending(edge_connection_t *conn)
 void
 sendme_circuit_consider_sending(circuit_t *circ, crypt_path_t *layer_hint)
 {
+  bool sent_one_sendme = false;
   const uint8_t *digest;
 
   while ((layer_hint ? layer_hint->deliver_window : circ->deliver_window) <=
@@ -423,6 +433,12 @@ sendme_circuit_consider_sending(circuit_t *circ, crypt_path_t *layer_hint)
     if (send_circuit_level_sendme(circ, layer_hint, digest) < 0) {
       return; /* The circuit's closed, don't continue */
     }
+    /* Current implementation is not suppose to send multiple SENDME at once
+     * because this means we would use the same relay crypto digest for each
+     * SENDME leading to a mismatch on the other side and the circuit to
+     * collapse. Scream loudly if it ever happens so we can address it. */
+    tor_assert_nonfatal(!sent_one_sendme);
+    sent_one_sendme = true;
   }
 }