From cacfd82c8dcc74ef8d96c0051649b5eae3e31d61 Mon Sep 17 00:00:00 2001 From: David Goulet Date: Thu, 10 Nov 2016 16:04:04 -0500 Subject: [PATCH 1/7] cell: Add a control cell ID for semantic Signed-off-by: David Goulet --- src/or/relay.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/or/relay.h b/src/or/relay.h index e15551ca51..26b594ece6 100644 --- a/src/or/relay.h +++ b/src/or/relay.h @@ -24,6 +24,8 @@ int relay_send_command_from_edge_(streamid_t stream_id, circuit_t *circ, uint8_t relay_command, const char *payload, size_t payload_len, crypt_path_t *cpath_layer, const char *filename, int lineno); +/* Indicates to relay_send_command_from_edge() that it is a control cell. */ +#define CONTROL_CELL_ID 0 #define relay_send_command_from_edge(stream_id, circ, relay_command, payload, \ payload_len, cpath_layer) \ relay_send_command_from_edge_((stream_id), (circ), (relay_command), \ From e1497744c8887c883541d3cb6d3d63d9ab51f9ad Mon Sep 17 00:00:00 2001 From: David Goulet Date: Thu, 10 Nov 2016 16:04:23 -0500 Subject: [PATCH 2/7] prop224: Add INTRODUCE1 cell relay support Closes #20029 Signed-off-by: David Goulet --- src/or/hs_intropoint.c | 338 ++++++++++++++++++++++++++++++++-- src/or/hs_intropoint.h | 14 +- src/or/rendcommon.c | 2 +- src/test/test_hs_intropoint.c | 3 +- 4 files changed, 335 insertions(+), 22 deletions(-) diff --git a/src/or/hs_intropoint.c b/src/or/hs_intropoint.c index bfc7ec3876..42b20552c6 100644 --- a/src/or/hs_intropoint.c +++ b/src/or/hs_intropoint.c @@ -9,6 +9,7 @@ #define HS_INTROPOINT_PRIVATE #include "or.h" +#include "config.h" #include "circuitlist.h" #include "circuituse.h" #include "config.h" @@ -16,27 +17,52 @@ #include "rendmid.h" #include "rephist.h" -#include "hs/cell_establish_intro.h" +/* Trunnel */ +#include "ed25519_cert.h" #include "hs/cell_common.h" +#include "hs/cell_establish_intro.h" +#include "hs/cell_introduce1.h" + #include "hs_circuitmap.h" #include "hs_intropoint.h" #include "hs_common.h" -/** Extract the authentication key from an ESTABLISH_INTRO cell and - * place it in auth_key_out. */ +/** Extract the authentication key from an ESTABLISH_INTRO or INTRODUCE1 using + * the given cell_type from cell and place it in + * auth_key_out. */ STATIC void -get_auth_key_from_establish_intro_cell(ed25519_public_key_t *auth_key_out, - const hs_cell_establish_intro_t *cell) +get_auth_key_from_cell(ed25519_public_key_t *auth_key_out, + unsigned int cell_type, const void *cell) { + size_t auth_key_len; + const uint8_t *key_array; + tor_assert(auth_key_out); + tor_assert(cell); - const uint8_t *key_array = - hs_cell_establish_intro_getconstarray_auth_key(cell); + switch (cell_type) { + case RELAY_COMMAND_ESTABLISH_INTRO: + { + const hs_cell_establish_intro_t *c_cell = cell; + key_array = hs_cell_establish_intro_getconstarray_auth_key(c_cell); + auth_key_len = hs_cell_establish_intro_getlen_auth_key(c_cell); + break; + } + case RELAY_COMMAND_INTRODUCE1: + { + const hs_cell_introduce1_t *c_cell = cell; + key_array = hs_cell_introduce1_getconstarray_auth_key(cell); + auth_key_len = hs_cell_introduce1_getlen_auth_key(c_cell); + break; + } + default: + /* Getting here is really bad as it means we got a unknown cell type from + * this file where every call has an hardcoded value. */ + tor_assert(0); + } tor_assert(key_array); - tor_assert(hs_cell_establish_intro_getlen_auth_key(cell) == - sizeof(auth_key_out->pubkey)); - - memcpy(auth_key_out->pubkey, key_array, cell->auth_key_len); + tor_assert(auth_key_len == sizeof(auth_key_out->pubkey)); + memcpy(auth_key_out->pubkey, key_array, auth_key_len); } /** We received an ESTABLISH_INTRO cell. Verify its signature and MAC, @@ -79,7 +105,7 @@ verify_establish_intro_cell(const hs_cell_establish_intro_t *cell, memcpy(sig_struct.sig, sig_array, cell->sig_len); ed25519_public_key_t auth_key; - get_auth_key_from_establish_intro_cell(&auth_key, cell); + get_auth_key_from_cell(&auth_key, RELAY_COMMAND_ESTABLISH_INTRO, cell); const size_t sig_msg_len = cell->end_sig_fields - msg; int sig_mismatch = ed25519_checksig_prefixed(&sig_struct, @@ -154,7 +180,8 @@ handle_verified_establish_intro_cell(or_circuit_t *circ, { /* Get the auth key of this intro point */ ed25519_public_key_t auth_key; - get_auth_key_from_establish_intro_cell(&auth_key, parsed_cell); + get_auth_key_from_cell(&auth_key, RELAY_COMMAND_ESTABLISH_INTRO, + parsed_cell); /* Then notify the hidden service that the intro point is established by sending an INTRO_ESTABLISHED cell */ @@ -232,26 +259,38 @@ handle_establish_intro(or_circuit_t *circ, const uint8_t *request, return retval; } -/* Return True if circuit is suitable for becoming an intro circuit. */ -int -hs_intro_circuit_is_suitable(const or_circuit_t *circ) +/* Return True if circuit is suitable for being an intro circuit. */ +static int +circuit_is_suitable_intro_point(const or_circuit_t *circ, + const char *log_cell_type_str) { + tor_assert(circ); + tor_assert(log_cell_type_str); + /* Basic circuit state sanity checks. */ if (circ->base_.purpose != CIRCUIT_PURPOSE_OR) { log_fn(LOG_PROTOCOL_WARN, LD_PROTOCOL, - "Rejecting ESTABLISH_INTRO on non-OR circuit."); + "Rejecting %s on non-OR circuit.", log_cell_type_str); return 0; } if (circ->base_.n_chan) { log_fn(LOG_PROTOCOL_WARN, LD_PROTOCOL, - "Rejecting ESTABLISH_INTRO on non-edge circuit."); + "Rejecting %s on non-edge circuit.", log_cell_type_str); return 0; } + /* Suitable. */ return 1; } +/* Return True if circuit is suitable for becoming an intro circuit. */ +int +hs_intro_circuit_is_suitable(const or_circuit_t *circ) +{ + return circuit_is_suitable_intro_point(circ, "ESTABLISH_INTRO"); +} + /* We just received an ESTABLISH_INTRO cell in circ. Figure out of it's * a legacy or a next gen cell, and pass it to the appropriate handler. */ int @@ -286,3 +325,266 @@ hs_intro_received_establish_intro(or_circuit_t *circ, const uint8_t *request, return -1; } +/* Send an INTRODUCE_ACK cell onto the circuit circ with the status + * value in status. Depending on the status, it can be ACK or a NACK. + * Return 0 on success else a negative value on error which will close the + * circuit. */ +static int +send_introduce_ack_cell(or_circuit_t *circ, hs_intro_ack_status_t status) +{ + int ret = -1; + uint8_t *encoded_cell = NULL; + ssize_t encoded_len, result_len; + hs_cell_introduce_ack_t *cell; + cell_extension_t *ext; + + tor_assert(circ); + + /* Setup the INTRODUCE_ACK cell. We have no extensions so the N_EXTENSIONS + * field is set to 0 by default with a new object. */ + cell = hs_cell_introduce_ack_new(); + ret = hs_cell_introduce_ack_set_status(cell, status); + /* We have no cell extensions in an INTRODUCE_ACK cell. */ + ext = cell_extension_new(); + cell_extension_set_num(ext, 0); + hs_cell_introduce_ack_set_extensions(cell, ext); + /* A wrong status is a very bad code flow error as this value is controlled + * by the code in this file and not an external input. This means we use a + * code that is not known by the trunnel ABI. */ + tor_assert(ret == 0); + /* Encode the payload. We should never fail to get the encoded length. */ + encoded_len = hs_cell_introduce_ack_encoded_len(cell); + tor_assert(encoded_len > 0); + encoded_cell = tor_malloc_zero(encoded_len); + result_len = hs_cell_introduce_ack_encode(encoded_cell, encoded_len, cell); + tor_assert(encoded_len == result_len); + + ret = relay_send_command_from_edge(CONTROL_CELL_ID, TO_CIRCUIT(circ), + RELAY_COMMAND_INTRODUCE_ACK, + (char *) encoded_cell, encoded_len, + NULL); + /* On failure, the above function will close the circuit. */ + hs_cell_introduce_ack_free(cell); + tor_free(encoded_cell); + return ret; +} + +/* Validate a parsed INTRODUCE1 cell. Return 0 if valid or else a + * negative value for an invalid cell that should be NACKed. */ +static int +validate_introduce1_parsed_cell(const hs_cell_introduce1_t *cell) +{ + size_t legacy_key_id_len; + const uint8_t *legacy_key_id; + + tor_assert(cell); + + /* This code path SHOULD NEVER be reached if the cell is a legacy type so + * safety net here. The legacy ID must be zeroes in this case. */ + legacy_key_id_len = hs_cell_introduce1_getlen_legacy_key_id(cell); + legacy_key_id = hs_cell_introduce1_getconstarray_legacy_key_id(cell); + if (BUG(!tor_mem_is_zero((char *) legacy_key_id, legacy_key_id_len))) { + goto invalid; + } + + /* The auth key of an INTRODUCE1 should be of type ed25519 thus leading to a + * known fixed length as well. */ + if (hs_cell_introduce1_get_auth_key_type(cell) != + HS_INTRO_AUTH_KEY_TYPE_ED25519) { + log_fn(LOG_PROTOCOL_WARN, LD_PROTOCOL, + "Rejecting invalid INTRODUCE1 cell auth key type. " + "Responding with NACK."); + goto invalid; + } + if (hs_cell_introduce1_get_auth_key_len(cell) != ED25519_PUBKEY_LEN || + hs_cell_introduce1_getlen_auth_key(cell) != ED25519_PUBKEY_LEN) { + log_fn(LOG_PROTOCOL_WARN, LD_PROTOCOL, + "Rejecting invalid INTRODUCE1 cell auth key length. " + "Responding with NACK."); + goto invalid; + } + if (hs_cell_introduce1_getlen_encrypted(cell) == 0) { + log_fn(LOG_PROTOCOL_WARN, LD_PROTOCOL, + "Rejecting invalid INTRODUCE1 cell encrypted length. " + "Responding with NACK."); + goto invalid; + } + + return 0; + invalid: + return -1; +} + +/* We just received a non legacy INTRODUCE1 cell on client_circ with + * the payload in request of size request_len. Return 0 if + * everything went well, or -1 if an error occured. This function is in charge + * of sending back an INTRODUCE_ACK cell and will close client_circ on error. + */ +static int +handle_introduce1(or_circuit_t *client_circ, const uint8_t *request, + size_t request_len) +{ + int ret = -1; + or_circuit_t *service_circ; + hs_cell_introduce1_t *parsed_cell; + hs_intro_ack_status_t status = HS_INTRO_ACK_STATUS_SUCCESS; + + tor_assert(client_circ); + tor_assert(request); + + /* Parse cell. Note that we can only parse the non encrypted section for + * which we'll use the authentication key to find the service introduction + * circuit and relay the cell on it. */ + ssize_t cell_size = hs_cell_introduce1_parse(&parsed_cell, request, + request_len); + if (cell_size < 0) { + log_fn(LOG_PROTOCOL_WARN, LD_PROTOCOL, + "Rejecting %s INTRODUCE1 cell. Responding with NACK.", + cell_size == -1 ? "invalid" : "truncated"); + /* Inform client that the INTRODUCE1 has a bad format. */ + status = HS_INTRO_ACK_STATUS_BAD_FORMAT; + goto send_ack; + } + + /* Once parsed, validate the cell expected format once parsed. */ + if (validate_introduce1_parsed_cell(parsed_cell) < 0) { + /* Inform client that the INTRODUCE1 has bad format. */ + status = HS_INTRO_ACK_STATUS_BAD_FORMAT; + goto send_ack; + } + + /* Find introduction circuit through our circuit map. */ + { + ed25519_public_key_t auth_key; + get_auth_key_from_cell(&auth_key, RELAY_COMMAND_INTRODUCE1, parsed_cell); + service_circ = hs_circuitmap_get_intro_circ_v3(&auth_key); + if (service_circ == NULL) { + char b64_key[ED25519_BASE64_LEN + 1]; + ed25519_public_to_base64(b64_key, &auth_key); + log_info(LD_REND, "No intro circuit found for INTRODUCE1 cell " + "with auth key %s from circuit %" PRIu32 ". " + "Responding with NACK.", + safe_str(b64_key), client_circ->p_circ_id); + /* Inform the client that we don't know the requested service ID. */ + status = HS_INTRO_ACK_STATUS_UNKNOWN_ID; + goto send_ack; + } + } + + /* Relay the cell to the service on its intro circuit with an INTRODUCE2 + * cell which is the same exact payload. */ + if (relay_send_command_from_edge(CONTROL_CELL_ID, TO_CIRCUIT(service_circ), + RELAY_COMMAND_INTRODUCE2, + (char *) request, request_len, NULL)) { + log_warn(LD_REND, "Unable to send INTRODUCE2 cell to the service."); + /* Inform the client that we can't relay the cell. */ + status = HS_INTRO_ACK_STATUS_CANT_RELAY; + goto send_ack; + } + + /* Success! Send an INTRODUCE_ACK success status onto the client circuit. */ + status = HS_INTRO_ACK_STATUS_SUCCESS; + ret = 0; + + send_ack: + /* Send the INTRODUCE_ACK cell to the client with a specific status. */ + if (send_introduce_ack_cell(client_circ, status) < 0) { + log_warn(LD_REND, "Unable to send an INTRODUCE ACK status %d to client.", + status); + /* Circuit has been closed on failure of transmission. */ + goto done; + } + if (status != HS_INTRO_ACK_STATUS_SUCCESS) { + /* We just sent a NACK that is a non success status code so close the + * circuit because it's not useful to keep it open. Remember, a client can + * only send one INTRODUCE1 cell on a circuit. */ + circuit_mark_for_close(TO_CIRCUIT(client_circ), END_CIRC_REASON_INTERNAL); + } + done: + hs_cell_introduce1_free(parsed_cell); + return ret; +} + +/* Identify if the encoded cell we just received is a legacy one or not. The + * request should be at least DIGEST_LEN bytes long. */ +static int +introduce1_cell_is_legacy(const uint8_t *request) +{ + tor_assert(request); + + /* If the first 20 bytes of the cell (DIGEST_LEN) are NOT zeroes, it + * indicates a legacy cell (v2). */ + if (!tor_mem_is_zero((const char *) request, DIGEST_LEN)) { + /* Legacy cell. */ + return 1; + } + /* Not a legacy cell. */ + return 0; +} + +/* Return true iff the circuit circ is suitable for receiving an + * INTRODUCE1 cell. */ +static int +circuit_is_suitable_for_introduce1(const or_circuit_t *circ) +{ + tor_assert(circ); + + /* First of all, do we have a valid circuit to be an introduction point? */ + if (!circuit_is_suitable_intro_point(circ, "INTRODUCE1")) { + return 0; + } + + if (circ->already_received_introduce1) { + log_fn(LOG_PROTOCOL_WARN, LD_REND, + "Blocking multiple introductions on the same circuit. " + "Someone might be trying to attack a hidden service through " + "this relay."); + return 0; + } + + return 1; +} + +/* We just received an INTRODUCE1 cell on circ. Figure out which type + * it is and pass it to the appropriate handler. Return 0 on success else a + * negative value and the circuit is closed. */ +int +hs_intro_received_introduce1(or_circuit_t *circ, const uint8_t *request, + size_t request_len) +{ + int ret; + + tor_assert(circ); + tor_assert(request); + + /* A cell that can't hold a DIGEST_LEN is invalid as we need to check if + * it's a legacy cell or not using the first DIGEST_LEN bytes. */ + if (request_len < DIGEST_LEN) { + log_fn(LOG_PROTOCOL_WARN, LD_PROTOCOL, "Invalid INTRODUCE1 cell length."); + goto err; + } + + /* Make sure we have a circuit that can have an INTRODUCE1 cell on it. */ + if (!circuit_is_suitable_for_introduce1(circ)) { + /* We do not send a NACK because the circuit is not suitable for any kind + * of response or transmission as it's a violation of the protocol. */ + goto err; + } + /* Mark the circuit that we got this cell. None are allowed after this as a + * DoS mitigation since one circuit with one client can hammer a service. */ + circ->already_received_introduce1 = 1; + + /* We are sure here to have at least DIGEST_LEN bytes. */ + if (introduce1_cell_is_legacy(request)) { + /* Handle a legacy cell. */ + ret = rend_mid_introduce(circ, request, request_len); + } else { + /* Handle a non legacy cell. */ + ret = handle_introduce1(circ, request, request_len); + } + return ret; + + err: + circuit_mark_for_close(TO_CIRCUIT(circ), END_CIRC_REASON_TORPROTOCOL); + return -1; +} diff --git a/src/or/hs_intropoint.h b/src/or/hs_intropoint.h index 46d8d6f7de..e1bad47fc8 100644 --- a/src/or/hs_intropoint.h +++ b/src/or/hs_intropoint.h @@ -16,9 +16,19 @@ enum hs_intro_auth_key_type { HS_INTRO_AUTH_KEY_TYPE_ED25519 = 0x02, }; +/* INTRODUCE_ACK status code. */ +typedef enum { + HS_INTRO_ACK_STATUS_SUCCESS = 0x0000, + HS_INTRO_ACK_STATUS_UNKNOWN_ID = 0x0001, + HS_INTRO_ACK_STATUS_BAD_FORMAT = 0x0002, + HS_INTRO_ACK_STATUS_CANT_RELAY = 0x0003, +} hs_intro_ack_status_t; + int hs_intro_received_establish_intro(or_circuit_t *circ, const uint8_t *request, size_t request_len); +int hs_intro_received_introduce1(or_circuit_t *circ, const uint8_t *request, + size_t request_len); MOCK_DECL(int, hs_intro_send_intro_established_cell,(or_circuit_t *circ)); @@ -33,8 +43,8 @@ verify_establish_intro_cell(const hs_cell_establish_intro_t *out, size_t circuit_key_material_len); STATIC void -get_auth_key_from_establish_intro_cell(ed25519_public_key_t *auth_key_out, - const hs_cell_establish_intro_t *cell); +get_auth_key_from_cell(ed25519_public_key_t *auth_key_out, + unsigned int cell_type, const void *cell); #endif /* HS_INTROPOINT_PRIVATE */ diff --git a/src/or/rendcommon.c b/src/or/rendcommon.c index def51b7986..bc53762fb6 100644 --- a/src/or/rendcommon.c +++ b/src/or/rendcommon.c @@ -771,7 +771,7 @@ rend_process_relay_cell(circuit_t *circ, const crypt_path_t *layer_hint, break; case RELAY_COMMAND_INTRODUCE1: if (or_circ) - r = rend_mid_introduce(or_circ,payload,length); + r = hs_intro_received_introduce1(or_circ,payload,length); break; case RELAY_COMMAND_INTRODUCE2: if (origin_circ) diff --git a/src/test/test_hs_intropoint.c b/src/test/test_hs_intropoint.c index 608988ba9a..7741cffa62 100644 --- a/src/test/test_hs_intropoint.c +++ b/src/test/test_hs_intropoint.c @@ -294,7 +294,8 @@ test_intro_point_registration(void *arg) the_hs_circuitmap = get_hs_circuitmap(); tt_assert(the_hs_circuitmap); tt_int_op(1, ==, HT_SIZE(the_hs_circuitmap)); - get_auth_key_from_establish_intro_cell(&auth_key, establish_intro_cell); + get_auth_key_from_cell(&auth_key, RELAY_COMMAND_ESTABLISH_INTRO, + establish_intro_cell); returned_intro_circ = hs_circuitmap_get_intro_circ_v3(&auth_key); tt_ptr_op(intro_circ, ==, returned_intro_circ); } From 9d7505a62aebaaf2fdf0e3a8178b80be17047e7b Mon Sep 17 00:00:00 2001 From: David Goulet Date: Tue, 15 Nov 2016 14:18:48 -0500 Subject: [PATCH 3/7] prop224: Rename hs_intro_circuit_is_suitable() Adds a better semantic and it also follows the same interface for the INTRODUCE1 API which is circuit_is_suitable_for_introduce1(). Signed-off-by: David Goulet --- src/or/hs_intropoint.c | 4 ++-- src/or/hs_intropoint.h | 2 +- src/or/rendmid.c | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/or/hs_intropoint.c b/src/or/hs_intropoint.c index 42b20552c6..2da3ee9243 100644 --- a/src/or/hs_intropoint.c +++ b/src/or/hs_intropoint.c @@ -215,7 +215,7 @@ handle_establish_intro(or_circuit_t *circ, const uint8_t *request, circ->p_circ_id); /* Check that the circuit is in shape to become an intro point */ - if (!hs_intro_circuit_is_suitable(circ)) { + if (!hs_intro_circuit_is_suitable_for_establish_intro(circ)) { goto err; } @@ -286,7 +286,7 @@ circuit_is_suitable_intro_point(const or_circuit_t *circ, /* Return True if circuit is suitable for becoming an intro circuit. */ int -hs_intro_circuit_is_suitable(const or_circuit_t *circ) +hs_intro_circuit_is_suitable_for_establish_intro(const or_circuit_t *circ) { return circuit_is_suitable_intro_point(circ, "ESTABLISH_INTRO"); } diff --git a/src/or/hs_intropoint.h b/src/or/hs_intropoint.h index e1bad47fc8..08b3470bca 100644 --- a/src/or/hs_intropoint.h +++ b/src/or/hs_intropoint.h @@ -33,7 +33,7 @@ int hs_intro_received_introduce1(or_circuit_t *circ, const uint8_t *request, MOCK_DECL(int, hs_intro_send_intro_established_cell,(or_circuit_t *circ)); /* also used by rendservice.c */ -int hs_intro_circuit_is_suitable(const or_circuit_t *circ); +int hs_intro_circuit_is_suitable_for_establish_intro(const or_circuit_t *circ); #ifdef HS_INTROPOINT_PRIVATE diff --git a/src/or/rendmid.c b/src/or/rendmid.c index 3319a639b9..b873ad7cca 100644 --- a/src/or/rendmid.c +++ b/src/or/rendmid.c @@ -38,7 +38,7 @@ rend_mid_establish_intro_legacy(or_circuit_t *circ, const uint8_t *request, "Received a legacy ESTABLISH_INTRO request on circuit %u", (unsigned) circ->p_circ_id); - if (!hs_intro_circuit_is_suitable(circ)) { + if (!hs_intro_circuit_is_suitable_for_establish_intro(circ)) { reason = END_CIRC_REASON_TORPROTOCOL; goto err; } From db77a38da2f661d81ea2c8df1b62e5de64a32488 Mon Sep 17 00:00:00 2001 From: David Goulet Date: Tue, 15 Nov 2016 14:23:25 -0500 Subject: [PATCH 4/7] hs: Remove useless code in rend_mid_introduce() With the previous commit, we validate the circuit _before_ calling rend_mid_introduce() which handles the INTRODUCE1 payload. Signed-off-by: David Goulet --- src/or/rendmid.c | 24 ++++-------------------- 1 file changed, 4 insertions(+), 20 deletions(-) diff --git a/src/or/rendmid.c b/src/or/rendmid.c index b873ad7cca..89d2482c61 100644 --- a/src/or/rendmid.c +++ b/src/or/rendmid.c @@ -143,26 +143,10 @@ rend_mid_introduce(or_circuit_t *circ, const uint8_t *request, log_info(LD_REND, "Received an INTRODUCE1 request on circuit %u", (unsigned)circ->p_circ_id); - if (circ->base_.purpose != CIRCUIT_PURPOSE_OR || circ->base_.n_chan) { - log_warn(LD_PROTOCOL, - "Rejecting INTRODUCE1 on non-OR or non-edge circuit %u.", - (unsigned)circ->p_circ_id); - goto err; - } - - /* We have already done an introduction on this circuit but we just - received a request for another one. We block it since this might - be an attempt to DoS a hidden service (#15515). */ - if (circ->already_received_introduce1) { - log_fn(LOG_PROTOCOL_WARN, LD_REND, - "Blocking multiple introductions on the same circuit. " - "Someone might be trying to attack a hidden service through " - "this relay."); - circuit_mark_for_close(TO_CIRCUIT(circ), END_CIRC_REASON_TORPROTOCOL); - return -1; - } - - circ->already_received_introduce1 = 1; + /* At this point, we know that the circuit is valid for an INTRODUCE1 + * because the validation has been made before calling this function. */ + tor_assert(circ->base_.purpose == CIRCUIT_PURPOSE_OR); + tor_assert(!circ->base_.n_chan); /* We could change this to MAX_HEX_NICKNAME_LEN now that 0.0.9.x is * obsolete; however, there isn't much reason to do so, and we're going From 5208085be1deff4711b634b108052970aed6a127 Mon Sep 17 00:00:00 2001 From: David Goulet Date: Tue, 15 Nov 2016 14:29:02 -0500 Subject: [PATCH 5/7] hs: Rename rend_mid_introduce() with legacy semantic Signed-off-by: David Goulet --- src/or/hs_intropoint.c | 2 +- src/or/rendmid.c | 4 ++-- src/or/rendmid.h | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/or/hs_intropoint.c b/src/or/hs_intropoint.c index 2da3ee9243..b7dae7d3c4 100644 --- a/src/or/hs_intropoint.c +++ b/src/or/hs_intropoint.c @@ -577,7 +577,7 @@ hs_intro_received_introduce1(or_circuit_t *circ, const uint8_t *request, /* We are sure here to have at least DIGEST_LEN bytes. */ if (introduce1_cell_is_legacy(request)) { /* Handle a legacy cell. */ - ret = rend_mid_introduce(circ, request, request_len); + ret = rend_mid_introduce_legacy(circ, request, request_len); } else { /* Handle a non legacy cell. */ ret = handle_introduce1(circ, request, request_len); diff --git a/src/or/rendmid.c b/src/or/rendmid.c index 89d2482c61..57c8cfac92 100644 --- a/src/or/rendmid.c +++ b/src/or/rendmid.c @@ -133,8 +133,8 @@ rend_mid_establish_intro_legacy(or_circuit_t *circ, const uint8_t *request, * INTRODUCE2 cell. */ int -rend_mid_introduce(or_circuit_t *circ, const uint8_t *request, - size_t request_len) +rend_mid_introduce_legacy(or_circuit_t *circ, const uint8_t *request, + size_t request_len) { or_circuit_t *intro_circ; char serviceid[REND_SERVICE_ID_LEN_BASE32+1]; diff --git a/src/or/rendmid.h b/src/or/rendmid.h index 374f2b7d66..347d745853 100644 --- a/src/or/rendmid.h +++ b/src/or/rendmid.h @@ -14,8 +14,8 @@ int rend_mid_establish_intro_legacy(or_circuit_t *circ, const uint8_t *request, size_t request_len); -int rend_mid_introduce(or_circuit_t *circ, const uint8_t *request, - size_t request_len); +int rend_mid_introduce_legacy(or_circuit_t *circ, const uint8_t *request, + size_t request_len); int rend_mid_establish_rendezvous(or_circuit_t *circ, const uint8_t *request, size_t request_len); int rend_mid_rendezvous(or_circuit_t *circ, const uint8_t *request, From 50cfc9834094acf33bd5afff6a2a36f7f17e70e0 Mon Sep 17 00:00:00 2001 From: David Goulet Date: Tue, 15 Nov 2016 15:09:27 -0500 Subject: [PATCH 6/7] prop224: Add unit tests for INTRODUCE1 support Signed-off-by: David Goulet --- src/or/hs_intropoint.c | 14 +- src/or/hs_intropoint.h | 9 + src/or/relay.c | 10 +- src/or/relay.h | 5 +- src/test/test_hs_intropoint.c | 328 ++++++++++++++++++++++++++++++++++ 5 files changed, 354 insertions(+), 12 deletions(-) diff --git a/src/or/hs_intropoint.c b/src/or/hs_intropoint.c index b7dae7d3c4..25721e93c6 100644 --- a/src/or/hs_intropoint.c +++ b/src/or/hs_intropoint.c @@ -58,7 +58,7 @@ get_auth_key_from_cell(ed25519_public_key_t *auth_key_out, default: /* Getting here is really bad as it means we got a unknown cell type from * this file where every call has an hardcoded value. */ - tor_assert(0); + tor_assert(0); /* LCOV_EXCL_LINE */ } tor_assert(key_array); tor_assert(auth_key_len == sizeof(auth_key_out->pubkey)); @@ -371,7 +371,7 @@ send_introduce_ack_cell(or_circuit_t *circ, hs_intro_ack_status_t status) /* Validate a parsed INTRODUCE1 cell. Return 0 if valid or else a * negative value for an invalid cell that should be NACKed. */ -static int +STATIC int validate_introduce1_parsed_cell(const hs_cell_introduce1_t *cell) { size_t legacy_key_id_len; @@ -420,7 +420,7 @@ validate_introduce1_parsed_cell(const hs_cell_introduce1_t *cell) * everything went well, or -1 if an error occured. This function is in charge * of sending back an INTRODUCE_ACK cell and will close client_circ on error. */ -static int +STATIC int handle_introduce1(or_circuit_t *client_circ, const uint8_t *request, size_t request_len) { @@ -476,10 +476,12 @@ handle_introduce1(or_circuit_t *client_circ, const uint8_t *request, if (relay_send_command_from_edge(CONTROL_CELL_ID, TO_CIRCUIT(service_circ), RELAY_COMMAND_INTRODUCE2, (char *) request, request_len, NULL)) { + /* LCOV_EXCL_START */ log_warn(LD_REND, "Unable to send INTRODUCE2 cell to the service."); /* Inform the client that we can't relay the cell. */ status = HS_INTRO_ACK_STATUS_CANT_RELAY; goto send_ack; + /* LCOV_EXCL_STOP */ } /* Success! Send an INTRODUCE_ACK success status onto the client circuit. */ @@ -489,10 +491,12 @@ handle_introduce1(or_circuit_t *client_circ, const uint8_t *request, send_ack: /* Send the INTRODUCE_ACK cell to the client with a specific status. */ if (send_introduce_ack_cell(client_circ, status) < 0) { + /* LCOV_EXCL_START */ log_warn(LD_REND, "Unable to send an INTRODUCE ACK status %d to client.", status); /* Circuit has been closed on failure of transmission. */ goto done; + /* LCOV_EXCL_STOP */ } if (status != HS_INTRO_ACK_STATUS_SUCCESS) { /* We just sent a NACK that is a non success status code so close the @@ -507,7 +511,7 @@ handle_introduce1(or_circuit_t *client_circ, const uint8_t *request, /* Identify if the encoded cell we just received is a legacy one or not. The * request should be at least DIGEST_LEN bytes long. */ -static int +STATIC int introduce1_cell_is_legacy(const uint8_t *request) { tor_assert(request); @@ -524,7 +528,7 @@ introduce1_cell_is_legacy(const uint8_t *request) /* Return true iff the circuit circ is suitable for receiving an * INTRODUCE1 cell. */ -static int +STATIC int circuit_is_suitable_for_introduce1(const or_circuit_t *circ) { tor_assert(circ); diff --git a/src/or/hs_intropoint.h b/src/or/hs_intropoint.h index 08b3470bca..e6024a858f 100644 --- a/src/or/hs_intropoint.h +++ b/src/or/hs_intropoint.h @@ -37,6 +37,9 @@ int hs_intro_circuit_is_suitable_for_establish_intro(const or_circuit_t *circ); #ifdef HS_INTROPOINT_PRIVATE +#include "hs/cell_establish_intro.h" +#include "hs/cell_introduce1.h" + STATIC int verify_establish_intro_cell(const hs_cell_establish_intro_t *out, const uint8_t *circuit_key_material, @@ -46,6 +49,12 @@ STATIC void get_auth_key_from_cell(ed25519_public_key_t *auth_key_out, unsigned int cell_type, const void *cell); +STATIC int introduce1_cell_is_legacy(const uint8_t *request); +STATIC int handle_introduce1(or_circuit_t *client_circ, + const uint8_t *request, size_t request_len); +STATIC int validate_introduce1_parsed_cell(const hs_cell_introduce1_t *cell); +STATIC int circuit_is_suitable_for_introduce1(const or_circuit_t *circ); + #endif /* HS_INTROPOINT_PRIVATE */ #endif /* TOR_HS_INTRO_H */ diff --git a/src/or/relay.c b/src/or/relay.c index 8d48239e47..2e76a8ec36 100644 --- a/src/or/relay.c +++ b/src/or/relay.c @@ -595,11 +595,11 @@ relay_command_to_string(uint8_t command) * If you can't send the cell, mark the circuit for close and return -1. Else * return 0. */ -int -relay_send_command_from_edge_(streamid_t stream_id, circuit_t *circ, - uint8_t relay_command, const char *payload, - size_t payload_len, crypt_path_t *cpath_layer, - const char *filename, int lineno) +MOCK_IMPL(int, +relay_send_command_from_edge_,(streamid_t stream_id, circuit_t *circ, + uint8_t relay_command, const char *payload, + size_t payload_len, crypt_path_t *cpath_layer, + const char *filename, int lineno)) { cell_t cell; relay_header_t rh; diff --git a/src/or/relay.h b/src/or/relay.h index 26b594ece6..3acf3ee0e3 100644 --- a/src/or/relay.h +++ b/src/or/relay.h @@ -20,10 +20,11 @@ int circuit_receive_relay_cell(cell_t *cell, circuit_t *circ, void relay_header_pack(uint8_t *dest, const relay_header_t *src); void relay_header_unpack(relay_header_t *dest, const uint8_t *src); -int relay_send_command_from_edge_(streamid_t stream_id, circuit_t *circ, +MOCK_DECL(int, +relay_send_command_from_edge_,(streamid_t stream_id, circuit_t *circ, uint8_t relay_command, const char *payload, size_t payload_len, crypt_path_t *cpath_layer, - const char *filename, int lineno); + const char *filename, int lineno)); /* Indicates to relay_send_command_from_edge() that it is a control cell. */ #define CONTROL_CELL_ID 0 #define relay_send_command_from_edge(stream_id, circ, relay_command, payload, \ diff --git a/src/test/test_hs_intropoint.c b/src/test/test_hs_intropoint.c index 7741cffa62..f8f4460e4a 100644 --- a/src/test/test_hs_intropoint.c +++ b/src/test/test_hs_intropoint.c @@ -13,11 +13,16 @@ #include "test.h" #include "crypto.h" +#include "log_test_helpers.h" #include "or.h" #include "ht.h" +/* Trunnel. */ #include "hs/cell_establish_intro.h" +#include "hs/cell_introduce1.h" +#include "hs/cell_common.h" + #include "hs_service.h" #include "hs_circuitmap.h" #include "hs_intropoint.h" @@ -25,6 +30,7 @@ #include "circuitlist.h" #include "circuituse.h" #include "rendservice.h" +#include "relay.h" /* Mock function to avoid networking in unittests */ static int @@ -34,6 +40,80 @@ mock_send_intro_established_cell(or_circuit_t *circ) return 0; } +static int +mock_relay_send_command_from_edge(streamid_t stream_id, circuit_t *circ, + uint8_t relay_command, const char *payload, + size_t payload_len, crypt_path_t *cpath_layer, + const char *filename, int lineno) +{ + (void) stream_id; + (void) circ; + (void) relay_command; + (void) payload; + (void) payload_len; + (void) cpath_layer; + (void) filename; + (void) lineno; + return 0; +} + +static or_circuit_t * +helper_create_intro_circuit(void) +{ + or_circuit_t *circ = or_circuit_new(0, NULL); + tt_assert(circ); + circuit_change_purpose(TO_CIRCUIT(circ), CIRCUIT_PURPOSE_OR); + done: + return circ; +} + +static hs_cell_introduce1_t * +helper_create_introduce1_cell(void) +{ + hs_cell_introduce1_t *cell = NULL; + ed25519_keypair_t auth_key_kp; + + /* Generate the auth_key of the cell. */ + if (ed25519_keypair_generate(&auth_key_kp, 0) < 0) { + goto err; + } + + cell = hs_cell_introduce1_new(); + tt_assert(cell); + + /* Set the auth key. */ + { + size_t auth_key_len = sizeof(auth_key_kp.pubkey); + hs_cell_introduce1_set_auth_key_type(cell, + HS_INTRO_AUTH_KEY_TYPE_ED25519); + hs_cell_introduce1_set_auth_key_len(cell, auth_key_len); + hs_cell_introduce1_setlen_auth_key(cell, auth_key_len); + uint8_t *auth_key_ptr = hs_cell_introduce1_getarray_auth_key(cell); + memcpy(auth_key_ptr, auth_key_kp.pubkey.pubkey, auth_key_len); + } + + /* Set the cell extentions to none. */ + { + cell_extension_t *ext = cell_extension_new(); + cell_extension_set_num(ext, 0); + hs_cell_introduce1_set_extensions(cell, ext); + } + + /* Set the encrypted section to some data. */ + { + size_t enc_len = 128; + hs_cell_introduce1_setlen_encrypted(cell, enc_len); + uint8_t *enc_ptr = hs_cell_introduce1_getarray_encrypted(cell); + memset(enc_ptr, 'a', enc_len); + } + + return cell; + err: + done: + hs_cell_introduce1_free(cell); + return NULL; +} + /* Try sending an ESTABLISH_INTRO cell on a circuit that is already an intro * point. Should fail. */ static void @@ -341,6 +421,242 @@ test_intro_point_registration(void *arg) UNMOCK(hs_intro_send_intro_established_cell); } +static void +test_introduce1_suitable_circuit(void *arg) +{ + int ret; + or_circuit_t *circ = NULL; + + (void) arg; + + /* Valid suitable circuit. */ + { + circ = or_circuit_new(0, NULL); + circuit_change_purpose(TO_CIRCUIT(circ), CIRCUIT_PURPOSE_OR); + ret = circuit_is_suitable_for_introduce1(circ); + circuit_free(TO_CIRCUIT(circ)); + tt_int_op(ret, OP_EQ, 1); + } + + /* Test if the circuit purpose safeguard works correctly. */ + { + circ = or_circuit_new(0, NULL); + circuit_change_purpose(TO_CIRCUIT(circ), CIRCUIT_PURPOSE_INTRO_POINT); + ret = circuit_is_suitable_for_introduce1(circ); + circuit_free(TO_CIRCUIT(circ)); + tt_int_op(ret, OP_EQ, 0); + } + + /* Test the non-edge circuit safeguard works correctly. */ + { + circ = or_circuit_new(0, NULL); + circuit_change_purpose(TO_CIRCUIT(circ), CIRCUIT_PURPOSE_OR); + /* Bogus pointer, the check is against NULL on n_chan. */ + circ->base_.n_chan = (channel_t *) circ; + ret = circuit_is_suitable_for_introduce1(circ); + circuit_free(TO_CIRCUIT(circ)); + tt_int_op(ret, OP_EQ, 0); + } + + /* Mangle the circuit a bit more so see if our only one INTRODUCE1 cell + * limit works correctly. */ + { + circ = or_circuit_new(0, NULL); + circuit_change_purpose(TO_CIRCUIT(circ), CIRCUIT_PURPOSE_OR); + circ->already_received_introduce1 = 1; + ret = circuit_is_suitable_for_introduce1(circ); + circuit_free(TO_CIRCUIT(circ)); + tt_int_op(ret, OP_EQ, 0); + } + + done: + ; +} + +static void +test_introduce1_is_legacy(void *arg) +{ + int ret; + uint8_t request[256]; + + (void) arg; + + /* For a cell to be considered legacy, according to the specification, the + * first 20 bytes MUST BE non-zero else it's a v3 cell. */ + memset(request, 'a', DIGEST_LEN); + memset(request + DIGEST_LEN, 0, sizeof(request) - DIGEST_LEN); + ret = introduce1_cell_is_legacy(request); + tt_int_op(ret, OP_EQ, 1); + + /* This is a NON legacy cell. */ + memset(request, 0, DIGEST_LEN); + memset(request + DIGEST_LEN, 'a', sizeof(request) - DIGEST_LEN); + ret = introduce1_cell_is_legacy(request); + tt_int_op(ret, OP_EQ, 0); + + done: + ; +} + +static void +test_introduce1_validation(void *arg) +{ + int ret; + hs_cell_introduce1_t *cell = NULL; + + (void) arg; + + /* Create our decoy cell that we'll modify as we go to test the validation + * function of that parsed cell. */ + cell = helper_create_introduce1_cell(); + + /* It should NOT be a legacy cell which will trigger a BUG(). */ + memset(cell->legacy_key_id, 'a', sizeof(cell->legacy_key_id)); + tor_capture_bugs_(1); + ret = validate_introduce1_parsed_cell(cell); + tor_end_capture_bugs_(); + tt_int_op(ret, OP_EQ, -1); + /* Reset legacy ID and make sure it's correct. */ + memset(cell->legacy_key_id, 0, sizeof(cell->legacy_key_id)); + ret = validate_introduce1_parsed_cell(cell); + tt_int_op(ret, OP_EQ, 0); + + /* Non existing auth key type. */ + cell->auth_key_type = 42; + ret = validate_introduce1_parsed_cell(cell); + tt_int_op(ret, OP_EQ, -1); + /* Reset is to correct value and make sure it's correct. */ + cell->auth_key_type = HS_INTRO_AUTH_KEY_TYPE_ED25519; + ret = validate_introduce1_parsed_cell(cell); + tt_int_op(ret, OP_EQ, 0); + + /* Really bad key length. */ + cell->auth_key_len = 0; + ret = validate_introduce1_parsed_cell(cell); + tt_int_op(ret, OP_EQ, -1); + cell->auth_key_len = UINT16_MAX; + ret = validate_introduce1_parsed_cell(cell); + tt_int_op(ret, OP_EQ, -1); + /* Correct size, let's try that. */ + cell->auth_key_len = sizeof(ed25519_public_key_t); + ret = validate_introduce1_parsed_cell(cell); + tt_int_op(ret, OP_EQ, 0); + /* Set an invalid size of the auth key buffer. */ + hs_cell_introduce1_setlen_auth_key(cell, 3); + ret = validate_introduce1_parsed_cell(cell); + tt_int_op(ret, OP_EQ, -1); + /* Reset auth key buffer and make sure it works. */ + hs_cell_introduce1_setlen_auth_key(cell, sizeof(ed25519_public_key_t)); + ret = validate_introduce1_parsed_cell(cell); + tt_int_op(ret, OP_EQ, 0); + + /* Empty encrypted section. */ + hs_cell_introduce1_setlen_encrypted(cell, 0); + ret = validate_introduce1_parsed_cell(cell); + tt_int_op(ret, OP_EQ, -1); + /* Reset it to some non zero bytes and validate. */ + hs_cell_introduce1_setlen_encrypted(cell, 1); + ret = validate_introduce1_parsed_cell(cell); + tt_int_op(ret, OP_EQ, 0); + + done: + hs_cell_introduce1_free(cell); +} + +static void +test_received_introduce1_handling(void *arg) +{ + int ret; + uint8_t *request = NULL, buf[128]; + hs_cell_introduce1_t *cell = NULL; + or_circuit_t *circ = NULL; + + (void) arg; + + MOCK(relay_send_command_from_edge_, mock_relay_send_command_from_edge); + + hs_circuitmap_init(); + + /* Too small request length. An INTRODUCE1 expect at the very least a + * DIGEST_LEN size. */ + { + circ = helper_create_intro_circuit(); + ret = hs_intro_received_introduce1(circ, buf, DIGEST_LEN - 1); + tt_int_op(ret, OP_EQ, -1); + circuit_free(TO_CIRCUIT(circ)); + } + + /* We have a unit test only for the suitability of a circuit to receive an + * INTRODUCE1 cell so from now on we'll only test the handling of a cell. */ + + /* Bad request. */ + { + circ = helper_create_intro_circuit(); + uint8_t test[2]; /* Too small request. */ + ret = handle_introduce1(circ, test, sizeof(test)); + tor_free(circ->p_chan); + circuit_free(TO_CIRCUIT(circ)); + tt_int_op(ret, OP_EQ, -1); + } + + /* Valid case. */ + { + cell = helper_create_introduce1_cell(); + size_t request_len = hs_cell_introduce1_encoded_len(cell); + tt_size_op(request_len, OP_GT, 0); + request = tor_malloc_zero(request_len); + ssize_t encoded_len = hs_cell_introduce1_encode(request, request_len, cell); + tt_size_op(encoded_len, OP_GT, 0); + + circ = helper_create_intro_circuit(); + or_circuit_t *service_circ = helper_create_intro_circuit(); + circuit_change_purpose(TO_CIRCUIT(service_circ), CIRCUIT_PURPOSE_INTRO_POINT); + /* Register the circuit in the map for the auth key of the cell. */ + ed25519_public_key_t auth_key; + const uint8_t *cell_auth_key = + hs_cell_introduce1_getconstarray_auth_key(cell); + memcpy(auth_key.pubkey, cell_auth_key, ED25519_PUBKEY_LEN); + hs_circuitmap_register_intro_circ_v3(service_circ, &auth_key); + ret = hs_intro_received_introduce1(circ, request, request_len); + circuit_free(TO_CIRCUIT(circ)); + circuit_free(TO_CIRCUIT(service_circ)); + tt_int_op(ret, OP_EQ, 0); + } + + /* Valid legacy cell. */ + { + tor_free(request); + hs_cell_introduce1_free(cell); + cell = helper_create_introduce1_cell(); + uint8_t *legacy_key_id = hs_cell_introduce1_getarray_legacy_key_id(cell); + memset(legacy_key_id, 'a', DIGEST_LEN); + /* Add an arbitrary amount of data for the payload of a v2 cell. */ + size_t request_len = hs_cell_introduce1_encoded_len(cell) + 256; + tt_size_op(request_len, OP_GT, 0); + request = tor_malloc_zero(request_len + 256); + ssize_t encoded_len = hs_cell_introduce1_encode(request, request_len, cell); + tt_size_op(encoded_len, OP_GT, 0); + + circ = helper_create_intro_circuit(); + or_circuit_t *service_circ = helper_create_intro_circuit(); + circuit_change_purpose(TO_CIRCUIT(service_circ), CIRCUIT_PURPOSE_INTRO_POINT); + /* Register the circuit in the map for the auth key of the cell. */ + uint8_t token[REND_TOKEN_LEN]; + memcpy(token, legacy_key_id, sizeof(token)); + hs_circuitmap_register_intro_circ_v2(service_circ, token); + ret = hs_intro_received_introduce1(circ, request, request_len); + circuit_free(TO_CIRCUIT(circ)); + circuit_free(TO_CIRCUIT(service_circ)); + tt_int_op(ret, OP_EQ, 0); + } + + done: + hs_cell_introduce1_free(cell); + tor_free(request); + hs_circuitmap_free_all(); + UNMOCK(relay_send_command_from_edge_); +} + struct testcase_t hs_intropoint_tests[] = { { "intro_point_registration", test_intro_point_registration, TT_FORK, NULL, NULL }, @@ -357,6 +673,18 @@ struct testcase_t hs_intropoint_tests[] = { { "receive_establish_intro_wrong_sig", test_establish_intro_wrong_sig, TT_FORK, NULL, NULL }, + { "introduce1_suitable_circuit", + test_introduce1_suitable_circuit, TT_FORK, NULL, NULL }, + + { "introduce1_is_legacy", + test_introduce1_is_legacy, TT_FORK, NULL, NULL }, + + { "introduce1_validation", + test_introduce1_validation, TT_FORK, NULL, NULL }, + + { "received_introduce1_handling", + test_received_introduce1_handling, TT_FORK, NULL, NULL }, + END_OF_TESTCASES }; From d6c14915cd3c6379973340afe3fd7f4e3f069166 Mon Sep 17 00:00:00 2001 From: George Kadianakis Date: Sun, 8 Jan 2017 15:06:39 +0200 Subject: [PATCH 7/7] Improve a few comments. - Also remove LCOV marks from blocks of code that can be reachable by tests if we mock relay_send_command_from_edge(). Signed-off-by: David Goulet --- src/or/hs_intropoint.c | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/src/or/hs_intropoint.c b/src/or/hs_intropoint.c index 25721e93c6..902b2e30f8 100644 --- a/src/or/hs_intropoint.c +++ b/src/or/hs_intropoint.c @@ -284,7 +284,7 @@ circuit_is_suitable_intro_point(const or_circuit_t *circ, return 1; } -/* Return True if circuit is suitable for becoming an intro circuit. */ +/* Return True if circuit is suitable for being service-side intro circuit. */ int hs_intro_circuit_is_suitable_for_establish_intro(const or_circuit_t *circ) { @@ -446,7 +446,7 @@ handle_introduce1(or_circuit_t *client_circ, const uint8_t *request, goto send_ack; } - /* Once parsed, validate the cell expected format once parsed. */ + /* Once parsed validate the cell format. */ if (validate_introduce1_parsed_cell(parsed_cell) < 0) { /* Inform client that the INTRODUCE1 has bad format. */ status = HS_INTRO_ACK_STATUS_BAD_FORMAT; @@ -476,12 +476,10 @@ handle_introduce1(or_circuit_t *client_circ, const uint8_t *request, if (relay_send_command_from_edge(CONTROL_CELL_ID, TO_CIRCUIT(service_circ), RELAY_COMMAND_INTRODUCE2, (char *) request, request_len, NULL)) { - /* LCOV_EXCL_START */ log_warn(LD_REND, "Unable to send INTRODUCE2 cell to the service."); /* Inform the client that we can't relay the cell. */ status = HS_INTRO_ACK_STATUS_CANT_RELAY; goto send_ack; - /* LCOV_EXCL_STOP */ } /* Success! Send an INTRODUCE_ACK success status onto the client circuit. */ @@ -489,14 +487,12 @@ handle_introduce1(or_circuit_t *client_circ, const uint8_t *request, ret = 0; send_ack: - /* Send the INTRODUCE_ACK cell to the client with a specific status. */ + /* Send INTRODUCE_ACK or INTRODUCE_NACK to client */ if (send_introduce_ack_cell(client_circ, status) < 0) { - /* LCOV_EXCL_START */ log_warn(LD_REND, "Unable to send an INTRODUCE ACK status %d to client.", status); /* Circuit has been closed on failure of transmission. */ goto done; - /* LCOV_EXCL_STOP */ } if (status != HS_INTRO_ACK_STATUS_SUCCESS) { /* We just sent a NACK that is a non success status code so close the @@ -533,7 +529,7 @@ circuit_is_suitable_for_introduce1(const or_circuit_t *circ) { tor_assert(circ); - /* First of all, do we have a valid circuit to be an introduction point? */ + /* Is this circuit an intro point circuit? */ if (!circuit_is_suitable_intro_point(circ, "INTRODUCE1")) { return 0; }