commit 0cad83bea4019b3c448756d718f5b65718e81f6e Author: David Goulet <dgoulet@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@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; } }