diff options
| -rw-r--r-- | src/core/or/sendme.c | 52 | ||||
| -rw-r--r-- | src/core/or/sendme.h | 16 | ||||
| -rw-r--r-- | src/test/test_sendme.c | 33 |
3 files changed, 72 insertions, 29 deletions
diff --git a/src/core/or/sendme.c b/src/core/or/sendme.c index baa57f4f25..80d6b7d165 100644 --- a/src/core/or/sendme.c +++ b/src/core/or/sendme.c @@ -25,20 +25,6 @@ #include "lib/ctime/di_ops.h" #include "trunnel/sendme.h" -/* The maximum supported version. Above that value, the cell can't be - * recognized as a valid SENDME. */ -#define SENDME_MAX_SUPPORTED_VERSION 1 - -/* The cell version constants for when emitting a cell. */ -#define SENDME_EMIT_MIN_VERSION_DEFAULT 0 -#define SENDME_EMIT_MIN_VERSION_MIN 0 -#define SENDME_EMIT_MIN_VERSION_MAX UINT8_MAX - -/* The cell version constants for when accepting a cell. */ -#define SENDME_ACCEPT_MIN_VERSION_DEFAULT 0 -#define SENDME_ACCEPT_MIN_VERSION_MIN 0 -#define SENDME_ACCEPT_MIN_VERSION_MAX UINT8_MAX - /* Return the minimum version given by the consensus (if any) that should be * used when emitting a SENDME cell. */ STATIC int @@ -123,30 +109,41 @@ cell_v1_is_valid(const sendme_cell_t *cell, const circuit_t *circ) /* Return true iff the given cell version can be handled or if the minimum * accepted version from the consensus is known to us. */ STATIC bool -cell_version_is_valid(uint8_t cell_version) +cell_version_can_be_handled(uint8_t cell_version) { int accept_version = get_accept_min_version(); - /* Can we handle this version? */ + /* We will first check if the consensus minimum accepted version can be + * handled by us and if not, regardless of the cell version we got, we can't + * continue. */ if (accept_version > SENDME_MAX_SUPPORTED_VERSION) { log_fn(LOG_PROTOCOL_WARN, LD_PROTOCOL, "Unable to accept SENDME version %u (from consensus). " - "We only support <= %d. Probably your tor is too old?", - accept_version, cell_version); + "We only support <= %u. Probably your tor is too old?", + accept_version, SENDME_MAX_SUPPORTED_VERSION); goto invalid; } - /* We only accept a SENDME cell from what the consensus tells us. */ + /* Then, is this version below the accepted version from the consensus? If + * yes, we must not handle it. */ if (cell_version < accept_version) { - log_info(LD_PROTOCOL, "Unacceptable SENDME version %d. Only " + log_info(LD_PROTOCOL, "Unacceptable SENDME version %u. Only " "accepting %u (from consensus). Closing circuit.", cell_version, accept_version); goto invalid; } - return 1; + /* Is this cell version supported by us? */ + if (cell_version > SENDME_MAX_SUPPORTED_VERSION) { + log_info(LD_PROTOCOL, "SENDME cell version %u is not supported by us. " + "We only support <= %u", + cell_version, SENDME_MAX_SUPPORTED_VERSION); + goto invalid; + } + + return true; invalid: - return 0; + return false; } /* Return true iff the encoded SENDME cell in cell_payload of length @@ -183,7 +180,7 @@ sendme_is_valid(const circuit_t *circ, const uint8_t *cell_payload, } /* Validate that we can handle this cell version. */ - if (!cell_version_is_valid(cell_version)) { + if (!cell_version_can_be_handled(cell_version)) { goto invalid; } @@ -195,10 +192,13 @@ sendme_is_valid(const circuit_t *circ, const uint8_t *cell_payload, } break; case 0x00: - /* Fallthrough. Version 0, there is no work to be done on the payload so - * it is necessarily valid if we pass the version validation. */ + /* Version 0, there is no work to be done on the payload so it is + * necessarily valid if we pass the version validation. */ + break; default: - /* Unknown version means we can't handle it so fallback to version 0. */ + log_warn(LD_PROTOCOL, "Unknown SENDME cell version %d received.", + cell_version); + tor_assert_nonfatal_unreached(); break; } diff --git a/src/core/or/sendme.h b/src/core/or/sendme.h index 2fb73f76d9..a71891996b 100644 --- a/src/core/or/sendme.h +++ b/src/core/or/sendme.h @@ -45,6 +45,20 @@ bool sendme_circuit_cell_is_next(int window); /* Private section starts. */ #ifdef SENDME_PRIVATE +/* The maximum supported version. Above that value, the cell can't be + * recognized as a valid SENDME. */ +#define SENDME_MAX_SUPPORTED_VERSION 1 + +/* The cell version constants for when emitting a cell. */ +#define SENDME_EMIT_MIN_VERSION_DEFAULT 0 +#define SENDME_EMIT_MIN_VERSION_MIN 0 +#define SENDME_EMIT_MIN_VERSION_MAX UINT8_MAX + +/* The cell version constants for when accepting a cell. */ +#define SENDME_ACCEPT_MIN_VERSION_DEFAULT 0 +#define SENDME_ACCEPT_MIN_VERSION_MIN 0 +#define SENDME_ACCEPT_MIN_VERSION_MAX UINT8_MAX + /* * Unit tests declaractions. */ @@ -53,7 +67,7 @@ bool sendme_circuit_cell_is_next(int window); STATIC int get_emit_min_version(void); STATIC int get_accept_min_version(void); -STATIC bool cell_version_is_valid(uint8_t cell_version); +STATIC bool cell_version_can_be_handled(uint8_t cell_version); STATIC ssize_t build_cell_payload_v1(const uint8_t *cell_digest, uint8_t *payload); diff --git a/src/test/test_sendme.c b/src/test/test_sendme.c index 463a0ec086..a36c904c28 100644 --- a/src/test/test_sendme.c +++ b/src/test/test_sendme.c @@ -122,9 +122,9 @@ test_v1_consensus_params(void *arg) smartlist_add(current_md_consensus->net_params, (void *) "sendme_accept_min_version=1"); /* Minimum acceptable value is 1. */ - tt_int_op(cell_version_is_valid(1), OP_EQ, true); + tt_int_op(cell_version_can_be_handled(1), OP_EQ, true); /* Minimum acceptable value is 1 so a cell version of 0 is refused. */ - tt_int_op(cell_version_is_valid(0), OP_EQ, false); + tt_int_op(cell_version_can_be_handled(0), OP_EQ, false); done: free_mock_consensus(); @@ -239,6 +239,33 @@ test_cell_payload_pad(void *arg) ; } +static void +test_cell_version_validation(void *arg) +{ + (void) arg; + + /* We currently only support up to SENDME_MAX_SUPPORTED_VERSION so we are + * going to test the boundaries there. */ + + tt_assert(cell_version_can_be_handled(SENDME_MAX_SUPPORTED_VERSION)); + + /* Version below our supported should pass. */ + tt_assert(cell_version_can_be_handled(SENDME_MAX_SUPPORTED_VERSION - 1)); + + /* Extra version from our supported should fail. */ + tt_assert(!cell_version_can_be_handled(SENDME_MAX_SUPPORTED_VERSION + 1)); + + /* Simple check for version 0. */ + tt_assert(cell_version_can_be_handled(0)); + + /* We MUST handle the default cell version that we emit or accept. */ + tt_assert(cell_version_can_be_handled(SENDME_EMIT_MIN_VERSION_DEFAULT)); + tt_assert(cell_version_can_be_handled(SENDME_ACCEPT_MIN_VERSION_DEFAULT)); + + done: + ; +} + struct testcase_t sendme_tests[] = { { "v1_record_digest", test_v1_record_digest, TT_FORK, NULL, NULL }, @@ -248,6 +275,8 @@ struct testcase_t sendme_tests[] = { NULL, NULL }, { "cell_payload_pad", test_cell_payload_pad, TT_FORK, NULL, NULL }, + { "cell_version_validation", test_cell_version_validation, TT_FORK, + NULL, NULL }, END_OF_TESTCASES }; |