summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
Diffstat
-rw-r--r--src/core/or/sendme.c70
-rw-r--r--src/test/test_relaycell.c5
-rw-r--r--src/test/test_sendme.c4
3 files changed, 52 insertions, 27 deletions
diff --git a/src/core/or/sendme.c b/src/core/or/sendme.c
index d914ba5e2e..bfbb65e851 100644
--- a/src/core/or/sendme.c
+++ b/src/core/or/sendme.c
@@ -47,47 +47,46 @@ get_accept_min_version(void)
SENDME_ACCEPT_MIN_VERSION_MAX);
}
-/* Return true iff the given cell digest matches the first digest in the
- * circuit sendme list. */
-static bool
-v1_digest_matches(const circuit_t *circ, const uint8_t *cell_digest)
+/* Pop the first cell digset on the given circuit from the SENDME last digests
+ * list. NULL is returned if the list is uninitialized or empty.
+ *
+ * The caller gets ownership of the returned digest thus is responsible for
+ * freeing the memory. */
+static uint8_t *
+pop_first_cell_digest(const circuit_t *circ)
{
- bool ret = false;
- uint8_t *circ_digest = NULL;
+ uint8_t *circ_digest;
tor_assert(circ);
- tor_assert(cell_digest);
- /* 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 ||
smartlist_len(circ->sendme_last_digests) == 0) {
- log_fn(LOG_PROTOCOL_WARN, LD_PROTOCOL,
- "We received a SENDME but we have no cell digests to match. "
- "Closing circuit.");
- goto no_match;
+ return NULL;
}
- /* Pop the first element that was added (FIFO) and compare it. */
circ_digest = smartlist_get(circ->sendme_last_digests, 0);
smartlist_del_keeporder(circ->sendme_last_digests, 0);
+ return circ_digest;
+}
+
+/* Return true iff the given cell digest matches the first digest in the
+ * circuit sendme list. */
+static bool
+v1_digest_matches(const uint8_t *circ_digest, const uint8_t *cell_digest)
+{
+ tor_assert(circ_digest);
+ tor_assert(cell_digest);
/* 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;
+ return false;
}
- /* 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;
+ /* Digests matches! */
+ return true;
}
/* Return true iff the given decoded SENDME version 1 cell is valid and
@@ -97,13 +96,13 @@ v1_digest_matches(const circuit_t *circ, const uint8_t *cell_digest)
* 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)
+cell_v1_is_valid(const sendme_cell_t *cell, const uint8_t *circ_digest)
{
tor_assert(cell);
- tor_assert(circ);
+ tor_assert(circ_digest);
const uint8_t *cell_digest = sendme_cell_getconstarray_data_v1_digest(cell);
- return v1_digest_matches(circ, cell_digest);
+ return v1_digest_matches(circ_digest, cell_digest);
}
/* Return true iff the given cell version can be handled or if the minimum
@@ -160,6 +159,7 @@ sendme_is_valid(const circuit_t *circ, const uint8_t *cell_payload,
size_t cell_payload_len)
{
uint8_t cell_version;
+ uint8_t *circ_digest = NULL;
sendme_cell_t *cell = NULL;
tor_assert(circ);
@@ -184,10 +184,24 @@ sendme_is_valid(const circuit_t *circ, const uint8_t *cell_payload,
goto invalid;
}
+ /* Pop the first element that was added (FIFO). We do that regardless of the
+ * version so we don't accumulate on the circuit if v0 is used by the other
+ * end point. */
+ circ_digest = pop_first_cell_digest(circ);
+ if (circ_digest == NULL) {
+ /* 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. */
+ log_fn(LOG_PROTOCOL_WARN, LD_PROTOCOL,
+ "We received a SENDME but we have no cell digests to match. "
+ "Closing circuit.");
+ goto invalid;
+ }
+
/* Validate depending on the version now. */
switch (cell_version) {
case 0x01:
- if (!cell_v1_is_valid(cell, circ)) {
+ if (!cell_v1_is_valid(cell, circ_digest)) {
goto invalid;
}
break;
@@ -204,9 +218,11 @@ sendme_is_valid(const circuit_t *circ, const uint8_t *cell_payload,
/* Valid cell. */
sendme_cell_free(cell);
+ tor_free(circ_digest);
return true;
invalid:
sendme_cell_free(cell);
+ tor_free(circ_digest);
return false;
}
diff --git a/src/test/test_relaycell.c b/src/test/test_relaycell.c
index d6372d3956..c65279fb25 100644
--- a/src/test/test_relaycell.c
+++ b/src/test/test_relaycell.c
@@ -17,6 +17,7 @@
#include "core/or/circuitbuild.h"
#include "core/or/circuitlist.h"
#include "core/or/connection_edge.h"
+#include "core/or/sendme.h"
#include "core/or/relay.h"
#include "test/test.h"
#include "test/log_test_helpers.h"
@@ -813,6 +814,10 @@ test_circbw_relay(void *arg)
/* Sendme on circuit with non-full window: counted */
PACK_CELL(0, RELAY_COMMAND_SENDME, "");
+ /* Recording a cell, the window is updated after decryption so off by one in
+ * order to record and then we process it with the proper window. */
+ circ->cpath->package_window = 901;
+ sendme_record_cell_digest_on_circ(TO_CIRCUIT(circ), circ->cpath);
circ->cpath->package_window = 900;
connection_edge_process_relay_cell(&cell, TO_CIRCUIT(circ), edgeconn,
circ->cpath);
diff --git a/src/test/test_sendme.c b/src/test/test_sendme.c
index a36c904c28..fa5ae115ac 100644
--- a/src/test/test_sendme.c
+++ b/src/test/test_sendme.c
@@ -143,11 +143,13 @@ test_v1_build_cell(void *arg)
or_circ = or_circuit_new(1, NULL);
circ = TO_CIRCUIT(or_circ);
+ circ->sendme_last_digests = smartlist_new();
cell_digest = crypto_digest_new();
tt_assert(cell_digest);
crypto_digest_add_bytes(cell_digest, "AAAAAAAAAAAAAAAAAAAA", 20);
crypto_digest_get_digest(cell_digest, (char *) digest, sizeof(digest));
+ smartlist_add(circ->sendme_last_digests, tor_memdup(digest, sizeof(digest)));
/* SENDME v1 payload is 3 bytes + 20 bytes digest. See spec. */
ret = build_cell_payload_v1(digest, payload);
@@ -157,6 +159,8 @@ test_v1_build_cell(void *arg)
/* An empty payload means SENDME version 0 thus valid. */
tt_int_op(sendme_is_valid(circ, payload, 0), OP_EQ, true);
+ /* Current phoney digest should have been popped. */
+ tt_int_op(smartlist_len(circ->sendme_last_digests), OP_EQ, 0);
/* An unparseable cell means invalid. */
setup_full_capture_of_logs(LOG_INFO);
7apu2bq3rhoylwmucxizk54afw5jyda7pho4j5zdcqpwkufke7zdzwad.onion