From af53e4bd1c9aa655ce4faacb957ad7603b9c4825 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Tue, 10 Jun 2014 22:41:13 -0400 Subject: Move circuit-id-in-use check for CREATE cells to before all other checks This means that we never send a DESTROY cell in response to an attempt to CREATE an existing circuit. Fixes bug 12191. --- changes/bug12191 | 6 ++++++ src/or/command.c | 34 +++++++++++++++++----------------- 2 files changed, 23 insertions(+), 17 deletions(-) create mode 100644 changes/bug12191 diff --git a/changes/bug12191 b/changes/bug12191 new file mode 100644 index 000000000..7fe1c550b --- /dev/null +++ b/changes/bug12191 @@ -0,0 +1,6 @@ + o Minor bugfixes: + + - We now drop CREATE cells for already-existent circuit IDs, regardless + of other factors that might otherwise have called for DESTROY cells. + Fixes bug 12191; bugfix on 0.2.??? + diff --git a/src/or/command.c b/src/or/command.c index 105bdc637..51fb64044 100644 --- a/src/or/command.c +++ b/src/or/command.c @@ -227,6 +227,23 @@ command_process_create_cell(cell_t *cell, channel_t *chan) (unsigned)cell->circ_id, U64_PRINTF_ARG(chan->global_identifier), chan); + if (circuit_id_in_use_on_channel(cell->circ_id, chan)) { + const node_t *node = node_get_by_id(chan->identity_digest); + log_fn(LOG_PROTOCOL_WARN, LD_PROTOCOL, + "Received CREATE cell (circID %u) for known circ. " + "Dropping (age %d).", + (unsigned)cell->circ_id, + (int)(time(NULL) - channel_when_created(chan))); + if (node) { + char *p = esc_for_log(node_get_platform(node)); + log_fn(LOG_PROTOCOL_WARN, LD_PROTOCOL, + "Details: router %s, platform %s.", + node_describe(node), p); + tor_free(p); + } + return; + } + if (we_are_hibernating()) { log_info(LD_OR, "Received create cell but we're shutting down. Sending back " @@ -274,23 +291,6 @@ command_process_create_cell(cell_t *cell, channel_t *chan) return; } - if (circuit_id_in_use_on_channel(cell->circ_id, chan)) { - const node_t *node = node_get_by_id(chan->identity_digest); - log_fn(LOG_PROTOCOL_WARN, LD_PROTOCOL, - "Received CREATE cell (circID %u) for known circ. " - "Dropping (age %d).", - (unsigned)cell->circ_id, - (int)(time(NULL) - channel_when_created(chan))); - if (node) { - char *p = esc_for_log(node_get_platform(node)); - log_fn(LOG_PROTOCOL_WARN, LD_PROTOCOL, - "Details: router %s, platform %s.", - node_describe(node), p); - tor_free(p); - } - return; - } - circ = or_circuit_new(cell->circ_id, chan); circ->base_.purpose = CIRCUIT_PURPOSE_OR; circuit_set_state(TO_CIRCUIT(circ), CIRCUIT_STATE_ONIONSKIN_PENDING); -- cgit v1.2.3 From f9f450d688cc74403393dbe579ca9d92d98bfe13 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Wed, 11 Jun 2014 09:33:20 -0400 Subject: Also raise the check for 0 circuit ID in created cell. And add a comment about why conditions that would cause us to drop a cell should get checked before actions that would cause us to send a destroy cell. Spotted by 'cypherpunks'. And note that these issues have been present since 0.0.8pre1 (commit 0da256ef), where we added a "shutting down" state, and started responding to all create cells with DESTROY when shutting down. --- changes/bug12191 | 7 ++++--- src/or/command.c | 19 +++++++++++-------- 2 files changed, 15 insertions(+), 11 deletions(-) diff --git a/changes/bug12191 b/changes/bug12191 index 7fe1c550b..77589ab31 100644 --- a/changes/bug12191 +++ b/changes/bug12191 @@ -1,6 +1,7 @@ o Minor bugfixes: - - We now drop CREATE cells for already-existent circuit IDs, regardless - of other factors that might otherwise have called for DESTROY cells. - Fixes bug 12191; bugfix on 0.2.??? + - We now drop CREATE cells for already-existent circuit IDs and + for zero-valued circuit IDs, regardless of other factors that + might otherwise have called for DESTROY cells. Fixes bug 12191; + bugfix on 0.0.8pre1. diff --git a/src/or/command.c b/src/or/command.c index 51fb64044..fa2a0e74e 100644 --- a/src/or/command.c +++ b/src/or/command.c @@ -227,6 +227,17 @@ command_process_create_cell(cell_t *cell, channel_t *chan) (unsigned)cell->circ_id, U64_PRINTF_ARG(chan->global_identifier), chan); + /* We check for the conditions that would make us drop the cell before + * we check for the conditions that would make us send a DESTROY back, + * since those conditions would make a DESTROY nonsensical. */ + if (cell->circ_id == 0) { + log_fn(LOG_PROTOCOL_WARN, LD_PROTOCOL, + "Received a create cell (type %d) from %s with zero circID; " + " ignoring.", (int)cell->command, + channel_get_actual_remote_descr(chan)); + return; + } + if (circuit_id_in_use_on_channel(cell->circ_id, chan)) { const node_t *node = node_get_by_id(chan->identity_digest); log_fn(LOG_PROTOCOL_WARN, LD_PROTOCOL, @@ -265,14 +276,6 @@ command_process_create_cell(cell_t *cell, channel_t *chan) return; } - if (cell->circ_id == 0) { - log_fn(LOG_PROTOCOL_WARN, LD_PROTOCOL, - "Received a create cell (type %d) from %s with zero circID; " - " ignoring.", (int)cell->command, - channel_get_actual_remote_descr(chan)); - return; - } - /* If the high bit of the circuit ID is not as expected, close the * circ. */ if (chan->wide_circ_ids) -- cgit v1.2.3