commit 72aa23a9fd1772de214ea8a22e1371c2d026dc7c Author: George Kadianakis desnacked@riseup.net Date: Thu Jul 22 15:02:13 2021 +0300

circpad: Be smarter on when to send back STOP cells. --- changes/bug40435 | 4 ++++ src/core/or/circuitpadding.c | 19 +++++++++++++++---- src/test/test_circuitpadding.c | 6 ++---- 3 files changed, 21 insertions(+), 8 deletions(-)

diff --git a/changes/bug40435 b/changes/bug40435 new file mode 100644 index 0000000000..76d0a687eb --- /dev/null +++ b/changes/bug40435 @@ -0,0 +1,4 @@ + o Minor bugfixes (circuit padding): + - Don't send STOP circuit padding cells when the other side has already + shut down the corresponding padding machine. Fixes bug 40435; bugfix on + 0.4.0.1-alpha. \ No newline at end of file diff --git a/src/core/or/circuitpadding.c b/src/core/or/circuitpadding.c index 6dfe94de01..99dc5f9d83 100644 --- a/src/core/or/circuitpadding.c +++ b/src/core/or/circuitpadding.c @@ -2967,6 +2967,8 @@ signed_error_t circpad_handle_padding_negotiate(circuit_t *circ, cell_t *cell) { int retval = 0; + /* Should we send back a STOP cell? */ + bool respond_with_stop = true; circpad_negotiate_t *negotiate;

if (CIRCUIT_IS_ORIGIN(circ)) { @@ -2992,6 +2994,12 @@ circpad_handle_padding_negotiate(circuit_t *circ, cell_t *cell) negotiate->machine_type, negotiate->machine_ctr); goto done; } + + /* If we reached this point we received a STOP command from an old or + unknown machine. Don't reply with our own STOP since there is no one to + handle it on the other end */ + respond_with_stop = false; + if (negotiate->machine_ctr <= circ->padding_machine_ctr) { log_info(LD_CIRC, "Received STOP command for old machine %u, ctr %u", negotiate->machine_type, negotiate->machine_ctr); @@ -3023,10 +3031,13 @@ circpad_handle_padding_negotiate(circuit_t *circ, cell_t *cell) retval = -1;

done: - circpad_padding_negotiated(circ, negotiate->machine_type, - negotiate->command, - (retval == 0) ? CIRCPAD_RESPONSE_OK : CIRCPAD_RESPONSE_ERR, - negotiate->machine_ctr); + if (respond_with_stop) { + circpad_padding_negotiated(circ, negotiate->machine_type, + negotiate->command, + (retval == 0) ? CIRCPAD_RESPONSE_OK : CIRCPAD_RESPONSE_ERR, + negotiate->machine_ctr); + } + circpad_negotiate_free(negotiate);

return retval; diff --git a/src/test/test_circuitpadding.c b/src/test/test_circuitpadding.c index 86baf54f40..6ced3f4111 100644 --- a/src/test/test_circuitpadding.c +++ b/src/test/test_circuitpadding.c @@ -1367,7 +1367,7 @@ test_circuitpadding_wronghop(void *arg) tt_ptr_op(client_side->padding_info[0], OP_NE, NULL); tt_ptr_op(relay_side->padding_machine[0], OP_NE, NULL); tt_ptr_op(relay_side->padding_info[0], OP_NE, NULL); - tt_int_op(n_relay_cells, OP_EQ, 3); + tt_int_op(n_relay_cells, OP_EQ, 2); tt_int_op(n_client_cells, OP_EQ, 2);

/* 6. Sending negotiated command to relay does nothing */ @@ -1396,11 +1396,9 @@ test_circuitpadding_wronghop(void *arg) /* verify no padding was negotiated */ tt_ptr_op(relay_side->padding_machine[0], OP_EQ, NULL); tt_ptr_op(client_side->padding_machine[0], OP_EQ, NULL); - tt_int_op(n_relay_cells, OP_EQ, 3); - tt_int_op(n_client_cells, OP_EQ, 2);

/* verify no echo was sent */ - tt_int_op(n_relay_cells, OP_EQ, 3); + tt_int_op(n_relay_cells, OP_EQ, 2); tt_int_op(n_client_cells, OP_EQ, 2);

/* Finish circuit */