commit b6d5fbba7d7e0e2cfa0c0cbb7f91e6039e3decf9 Author: Mike Perry <mikeperry-...@torproject.org> Date: Thu Nov 4 00:44:38 2021 +0000
Implement congestion control parameter negotiation --- src/core/crypto/onion_crypto.c | 200 +++++++++++++++++++++++++++++++++++------ src/core/crypto/onion_crypto.h | 10 ++- src/core/mainloop/cpuworker.c | 22 ++++- 3 files changed, 198 insertions(+), 34 deletions(-) diff --git a/src/core/crypto/onion_crypto.c b/src/core/crypto/onion_crypto.c index 1e9913539f..13f8f54b35 100644 --- a/src/core/crypto/onion_crypto.c +++ b/src/core/crypto/onion_crypto.c @@ -41,15 +41,17 @@ #include "lib/crypt_ops/crypto_dh.h" #include "lib/crypt_ops/crypto_util.h" #include "feature/relay/routerkeys.h" +#include "core/or/congestion_control_common.h" #include "core/or/circuitbuild.h" #include "core/or/crypt_path_st.h" #include "core/or/extend_info_st.h" +#include "trunnel/circ_params.h" /* TODO-324: Add this to the specification! */ -const uint8_t NTOR3_CIRC_VERIFICATION[] = "circuit extend"; -const size_t NTOR3_CIRC_VERIFICATION_LEN = 14; +static const uint8_t NTOR3_CIRC_VERIFICATION[] = "circuit extend"; +static const size_t NTOR3_CIRC_VERIFICATION_LEN = 14; #define NTOR3_VERIFICATION_ARGS \ NTOR3_CIRC_VERIFICATION, NTOR3_CIRC_VERIFICATION_LEN @@ -210,6 +212,93 @@ onion_skin_create(int type, return r; } +/** + * Takes a param request message from the client, compares it to our + * consensus parameters, and creates a reply message and output + * parameters. + * + * This function runs in a worker thread, so it can only inspect + * arguments and local variables. + * + * Returns 0 if successful. + * Returns -1 on parsing, parameter failure, or reply creation failure. + */ +static int +negotiate_v3_ntor_server_circ_params(const uint8_t *param_request_msg, + size_t param_request_len, + const circuit_params_t *our_ns_params, + circuit_params_t *params_out, + uint8_t **resp_msg_out, + size_t *resp_msg_len_out) +{ + circ_params_response_t *resp = NULL; + circ_params_request_t *param_request = NULL; + ssize_t resp_msg_len; + + if (circ_params_request_parse(¶m_request, param_request_msg, + param_request_len) < 0) { + return -1; + } + + /* CC is enabled if the client wants it, and our consensus paramers + * allow it. If both are true, its on. If either is false, it's off. */ + params_out->cc_enabled = + circ_params_request_get_cc_supported(param_request) && + our_ns_params->cc_enabled; + + resp = circ_params_response_new(); + + if (circ_params_response_set_version(resp, 0) < 0) { + circ_params_request_free(param_request); + circ_params_response_free(resp); + return -1; + } + + /* The relay always chooses its sendme_inc, and sends it to the client */ + params_out->sendme_inc_cells = our_ns_params->sendme_inc_cells; + + if (circ_params_response_set_sendme_inc_cells(resp, + our_ns_params->sendme_inc_cells) < 0) { + circ_params_request_free(param_request); + circ_params_response_free(resp); + return -1; + } + + /* Use the negotiated cc_enabled value to respond */ + if (circ_params_response_set_cc_enabled(resp, params_out->cc_enabled) < 0) { + circ_params_request_free(param_request); + circ_params_response_free(resp); + return -1; + } + + resp_msg_len = circ_params_response_encoded_len(resp); + + if (resp_msg_len < 0) { + circ_params_request_free(param_request); + circ_params_response_free(resp); + return -1; + } + + *resp_msg_out = tor_malloc_zero(resp_msg_len); + + resp_msg_len = circ_params_response_encode(*resp_msg_out, resp_msg_len, + resp); + if (resp_msg_len < 0) { + circ_params_request_free(param_request); + circ_params_response_free(resp); + + tor_free(*resp_msg_out); + return -1; + } + + *resp_msg_len_out = (size_t)resp_msg_len; + + circ_params_request_free(param_request); + circ_params_response_free(resp); + + return 0; +} + /* This is the maximum value for keys_out_len passed to * onion_skin_server_handshake, plus 16. We can make it bigger if needed: * It just defines how many bytes to stack-allocate. */ @@ -226,6 +315,7 @@ int onion_skin_server_handshake(int type, const uint8_t *onion_skin, size_t onionskin_len, const server_onion_keys_t *keys, + const circuit_params_t *our_ns_params, uint8_t *reply_out, size_t reply_out_maxlen, uint8_t *keys_out, size_t keys_out_len, @@ -233,7 +323,7 @@ onion_skin_server_handshake(int type, circuit_params_t *params_out) { int r = -1; - memset(params_out, 0, sizeof(*params_out)); // TODO-324: actually set this! + memset(params_out, 0, sizeof(*params_out)); switch (type) { case ONION_HANDSHAKE_TYPE_TAP: @@ -290,6 +380,9 @@ onion_skin_server_handshake(int type, uint8_t keys_tmp[MAX_KEYS_TMP_LEN]; uint8_t *client_msg = NULL; size_t client_msg_len = 0; + uint8_t *reply_msg = NULL; + size_t reply_msg_len = 0; + ntor3_server_handshake_state_t *state = NULL; if (onion_skin_ntor3_server_handshake_part1( @@ -303,25 +396,17 @@ onion_skin_server_handshake(int type, return -1; } - uint8_t reply_msg[1] = { 0 }; - size_t reply_msg_len = 1; - { - /* TODO-324, Okay, we have a message from the client trying to negotiate - * parameters. We need to decide whether the client's request is okay, - * what we're going to say in response, and what circuit parameters - * we've just negotiated - */ - - /* NOTE! DANGER, DANGER, DANGER! - - Remember that this function can be run in a worker thread, and so - therefore you can't access "global" state that isn't lock-protected. - - CAVEAT HAXX0R! - */ - + if (negotiate_v3_ntor_server_circ_params(client_msg, + client_msg_len, + our_ns_params, + params_out, + &reply_msg, + &reply_msg_len) < 0) { + ntor3_server_handshake_state_free(state); tor_free(client_msg); + return -1; } + tor_free(client_msg); uint8_t *server_handshake = NULL; size_t server_handshake_len = 0; @@ -331,12 +416,15 @@ onion_skin_server_handshake(int type, reply_msg, reply_msg_len, &server_handshake, &server_handshake_len, keys_tmp, keys_tmp_len) < 0) { - // XXX TODO-324 free some stuff + tor_free(reply_msg); + ntor3_server_handshake_state_free(state); return -1; } + tor_free(reply_msg); if (server_handshake_len > reply_out_maxlen) { - // XXX TODO-324 free that stuff + tor_free(server_handshake); + ntor3_server_handshake_state_free(state); return -1; } @@ -346,6 +434,7 @@ onion_skin_server_handshake(int type, memwipe(keys_tmp, 0, keys_tmp_len); memwipe(server_handshake, 0, server_handshake_len); tor_free(server_handshake); + ntor3_server_handshake_state_free(state); r = (int) server_handshake_len; } @@ -362,6 +451,61 @@ onion_skin_server_handshake(int type, return r; } +/** + * Takes a param response message from the exit, compares it to our + * consensus parameters for sanity, and creates output parameters + * if sane. + * + * Returns -1 on parsing or insane params, 0 if success. + */ +static int +negotiate_v3_ntor_client_circ_params(const uint8_t *param_response_msg, + size_t param_response_len, + circuit_params_t *params_out) +{ + circ_params_response_t *param_response = NULL; + bool cc_enabled; + uint8_t sendme_inc_cells; + + if (circ_params_response_parse(¶m_response, param_response_msg, + param_response_len) < 0) { + return -1; + } + + cc_enabled = + circ_params_response_get_cc_enabled(param_response); + + /* If congestion control came back enabled, but we didn't ask for it + * because the consensus said no, close the circuit */ + if (cc_enabled && !congestion_control_enabled()) { + circ_params_response_free(param_response); + return -1; + } + params_out->cc_enabled = cc_enabled; + + /* We will only accept this response (and this circuit) if sendme_inc + * is within a factor of 2 of our consensus value. We should not need + * to change cc_sendme_inc much, and if we do, we can spread out those + * changes over smaller increments once every 4 hours. Exits that + * violate this range should just not be used. */ +#define MAX_SENDME_INC_NEGOTIATE_FACTOR 2 + + sendme_inc_cells = + circ_params_response_get_sendme_inc_cells(param_response); + + if (sendme_inc_cells > + MAX_SENDME_INC_NEGOTIATE_FACTOR*congestion_control_sendme_inc() || + sendme_inc_cells < + congestion_control_sendme_inc()/MAX_SENDME_INC_NEGOTIATE_FACTOR) { + circ_params_response_free(param_response); + return -1; + } + params_out->sendme_inc_cells = sendme_inc_cells; + + circ_params_response_free(param_response); + return 0; +} + /** Perform the final (client-side) step of a circuit-creation handshake of * type <b>type</b>, using our state in <b>handshake_state</b> and the * server's response in <b>reply</b>. On success, generate <b>keys_out_len</b> @@ -382,7 +526,7 @@ onion_skin_client_handshake(int type, if (handshake_state->tag != type) return -1; - memset(params_out, 0, sizeof(*params_out)); // TODO-324: actually set this! + memset(params_out, 0, sizeof(*params_out)); switch (type) { case ONION_HANDSHAKE_TYPE_TAP: @@ -450,10 +594,12 @@ onion_skin_client_handshake(int type, return -1; } - { - // XXXX TODO-324: see what the server said, make sure it's okay, see what - // parameters it gave us, make sure we like them, and put them into - // `params_out` + if (negotiate_v3_ntor_client_circ_params(server_msg, + server_msg_len, + params_out) < 0) { + tor_free(keys_tmp); + tor_free(server_msg); + return -1; } tor_free(server_msg); diff --git a/src/core/crypto/onion_crypto.h b/src/core/crypto/onion_crypto.h index 45e8eeca0c..cb0188ff54 100644 --- a/src/core/crypto/onion_crypto.h +++ b/src/core/crypto/onion_crypto.h @@ -29,10 +29,11 @@ void onion_handshake_state_release(onion_handshake_state_t *state); * Parameters negotiated as part of a circuit handshake. */ typedef struct circuit_params_t { - /* placeholder field for congestion control algorithm. Right now this - * is always set to zero */ - int cc_algorithm; - int cc_window; + /** Is true if congestion control is enabled in consensus or param, + * as per congestion_control_enabled() result. */ + bool cc_enabled; + /** The number of cells in a sendme increment. Only used if cc_enabled=1. */ + uint8_t sendme_inc_cells; } circuit_params_t; int onion_skin_create(int type, @@ -43,6 +44,7 @@ int onion_skin_create(int type, int onion_skin_server_handshake(int type, const uint8_t *onion_skin, size_t onionskin_len, const server_onion_keys_t *keys, + const circuit_params_t *ns_params, uint8_t *reply_out, size_t reply_out_maxlen, uint8_t *keys_out, size_t key_out_len, diff --git a/src/core/mainloop/cpuworker.c b/src/core/mainloop/cpuworker.c index 7ca66a1c45..2cb667615d 100644 --- a/src/core/mainloop/cpuworker.c +++ b/src/core/mainloop/cpuworker.c @@ -21,6 +21,8 @@ #include "core/or/channel.h" #include "core/or/circuitlist.h" #include "core/or/connection_or.h" +#include "core/or/congestion_control_common.h" +#include "core/or/congestion_control_flow.h" #include "app/config/config.h" #include "core/mainloop/cpuworker.h" #include "lib/crypt_ops/crypto_rand.h" @@ -126,6 +128,11 @@ typedef struct cpuworker_request_t { /** A create cell for the cpuworker to process. */ create_cell_t create_cell; + /** + * A copy of this relay's consensus params that are relevant to + * the circuit, for use in negotiation. */ + circuit_params_t circ_ns_params; + /* Turn the above into a tagged union if needed. */ } cpuworker_request_t; @@ -381,6 +388,12 @@ cpuworker_onion_handshake_replyfn(void *work_) goto done_processing; } + /* If the client asked for congestion control, if our consensus parameter + * allowed it to negotiate as enabled, allocate a congestion control obj. */ + if (rpl.circ_params.cc_enabled) { + TO_CIRCUIT(circ)->ccontrol = congestion_control_new(&rpl.circ_params); + } + if (onionskin_answer(circ, &rpl.created_cell, (const char*)rpl.keys, sizeof(rpl.keys), @@ -390,9 +403,6 @@ cpuworker_onion_handshake_replyfn(void *work_) goto done_processing; } - /* TODO-324! We need to use rpl.circ_params here to initialize the congestion - control parameters of the circuit. */ - log_debug(LD_OR,"onionskin_answer succeeded. Yay."); done_processing: @@ -431,6 +441,7 @@ cpuworker_onion_handshake_threadfn(void *state_, void *work_) n = onion_skin_server_handshake(cc->handshake_type, cc->onionskin, cc->handshake_len, onion_keys, + &req.circ_ns_params, cell_out->reply, sizeof(cell_out->reply), rpl.keys, CPATH_KEY_MATERIAL_LEN, @@ -559,6 +570,11 @@ assign_onionskin_to_cpuworker(or_circuit_t *circ, if (should_time) tor_gettimeofday(&req.started_at); + /* Copy the current cached consensus params relevant to + * circuit negotiation into the CPU worker context */ + req.circ_ns_params.cc_enabled = congestion_control_enabled(); + req.circ_ns_params.sendme_inc_cells = congestion_control_sendme_inc(); + job = tor_malloc_zero(sizeof(cpuworker_job_t)); job->circ = circ; memcpy(&job->u.request, &req, sizeof(req)); _______________________________________________ tor-commits mailing list tor-commits@lists.torproject.org https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-commits