commit 4efe9d653aa1d375d77d6dca83ca63787d6599d7 Author: David Goulet <dgoulet@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@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