[tor-commits] [tor/master] hs-v3: Don't BUG() if the RP node_t is invalid client side
nickm at torproject.org
nickm at torproject.org
Fri Sep 21 17:02:17 UTC 2018
commit 79265a6fb606e416529f5a1dd31c94f15edec91b
Author: David Goulet <dgoulet at torproject.org>
Date: Thu Sep 20 09:32:13 2018 -0400
hs-v3: Don't BUG() if the RP node_t is invalid client side
When sending the INTRODUCE1 cell, we acquire the needed data for the cell but
if the RP node_t has invalid data, we'll fail the send and completely kill the
SOCKS connection.
Instead, close the rendezvous circuit and return a transient error meaning
that Tor can recover by selecting a new rendezvous point. We'll also do the
same when we are unable to encode the INTRODUCE1 cell for which at that point,
we'll simply take another shot at a new rendezvous point.
Fixes #27774
Signed-off-by: David Goulet <dgoulet at torproject.org>
---
changes/ticket27774 | 4 ++++
src/feature/hs/hs_circuit.c | 45 ++++++++++++++++++++++++++++++---------------
src/feature/hs/hs_client.c | 19 +++++++++++++++----
3 files changed, 49 insertions(+), 19 deletions(-)
diff --git a/changes/ticket27774 b/changes/ticket27774
new file mode 100644
index 000000000..2598c4055
--- /dev/null
+++ b/changes/ticket27774
@@ -0,0 +1,4 @@
+ o Minor bugfixes (hidden service v3):
+ - Client side would dump a stack trace if tor doesn't have the descriptor
+ for the intro point it is trying to connect to. Fixes bug 27774; bugfix
+ on 0.3.2.1-alpha.
diff --git a/src/feature/hs/hs_circuit.c b/src/feature/hs/hs_circuit.c
index 541b165dd..70760e013 100644
--- a/src/feature/hs/hs_circuit.c
+++ b/src/feature/hs/hs_circuit.c
@@ -646,16 +646,16 @@ get_lspecs_from_node(const node_t *node, smartlist_t *lspecs)
* already allocated intro1_data object with the needed key material and link
* specifiers.
*
- * If rp_node has an invalid primary address, intro1_data->link_specifiers
- * will be an empty list. Otherwise, this function can't fail. The ip
- * MUST be a valid object containing the needed keys and authentication
- * method. */
-static void
+ * Return 0 on success or a negative value if we couldn't properly filled the
+ * introduce1 data from the RP node. In other word, it means the RP node is
+ * unusable to use in the introduction. */
+static int
setup_introduce1_data(const hs_desc_intro_point_t *ip,
const node_t *rp_node,
const uint8_t *subcredential,
hs_cell_introduce1_data_t *intro1_data)
{
+ int ret = -1;
smartlist_t *rp_lspecs;
tor_assert(ip);
@@ -667,6 +667,11 @@ setup_introduce1_data(const hs_desc_intro_point_t *ip,
* circuit that we've picked previously. */
rp_lspecs = smartlist_new();
get_lspecs_from_node(rp_node, rp_lspecs);
+ if (smartlist_len(rp_lspecs) == 0) {
+ /* We can't rendezvous without link specifiers. */
+ smartlist_free(rp_lspecs);
+ goto end;
+ }
/* Populate the introduce1 data object. */
memset(intro1_data, 0, sizeof(hs_cell_introduce1_data_t));
@@ -677,8 +682,17 @@ setup_introduce1_data(const hs_desc_intro_point_t *ip,
intro1_data->auth_pk = &ip->auth_key_cert->signed_key;
intro1_data->enc_pk = &ip->enc_key;
intro1_data->subcredential = subcredential;
- intro1_data->onion_pk = node_get_curve25519_onion_key(rp_node);
intro1_data->link_specifiers = rp_lspecs;
+ intro1_data->onion_pk = node_get_curve25519_onion_key(rp_node);
+ if (intro1_data->onion_pk == NULL) {
+ /* We can't rendezvous without the curve25519 onion key. */
+ goto end;
+ }
+ /* Success, we have valid introduce data. */
+ ret = 0;
+
+ end:
+ return ret;
}
/* ========== */
@@ -1130,14 +1144,13 @@ hs_circ_send_introduce1(origin_circuit_t *intro_circ,
"Failing.", TO_CIRCUIT(intro_circ)->n_circ_id);
goto done;
}
- setup_introduce1_data(ip, exit_node, subcredential, &intro1_data);
- /* If we didn't get any link specifiers, it's because our node was
- * bad. */
- if (BUG(!intro1_data.link_specifiers) ||
- !smartlist_len(intro1_data.link_specifiers)) {
- log_warn(LD_REND, "Unable to get link specifiers for INTRODUCE1 cell on "
- "circuit %u.", TO_CIRCUIT(intro_circ)->n_circ_id);
- goto done;
+
+ /* We should never select an invalid rendezvous point in theory but if we
+ * do, this function will fail to populate the introduce data. */
+ if (setup_introduce1_data(ip, exit_node, subcredential, &intro1_data) < 0) {
+ log_warn(LD_REND, "Unable to setup INTRODUCE1 data. The chosen rendezvous "
+ "point is unusable. Closing circuit.");
+ goto close;
}
/* Final step before we encode a cell, we setup the circuit identifier which
@@ -1154,7 +1167,7 @@ hs_circ_send_introduce1(origin_circuit_t *intro_circ,
* into payload which is then ready to be sent as is. */
payload_len = hs_cell_build_introduce1(&intro1_data, payload);
if (BUG(payload_len < 0)) {
- goto done;
+ goto close;
}
if (relay_send_command_from_edge(CONTROL_CELL_ID, TO_CIRCUIT(intro_circ),
@@ -1171,6 +1184,8 @@ hs_circ_send_introduce1(origin_circuit_t *intro_circ,
ret = 0;
goto done;
+ close:
+ circuit_mark_for_close(TO_CIRCUIT(rend_circ), END_CIRC_REASON_INTERNAL);
done:
hs_cell_introduce1_data_clear(&intro1_data);
memwipe(payload, 0, sizeof(payload));
diff --git a/src/feature/hs/hs_client.c b/src/feature/hs/hs_client.c
index a6384b87a..441edc324 100644
--- a/src/feature/hs/hs_client.c
+++ b/src/feature/hs/hs_client.c
@@ -576,10 +576,21 @@ send_introduce1(origin_circuit_t *intro_circ,
/* Send the INTRODUCE1 cell. */
if (hs_circ_send_introduce1(intro_circ, rend_circ, ip,
desc->subcredential) < 0) {
- /* Unable to send the cell, the intro circuit has been marked for close so
- * this is a permanent error. */
- tor_assert_nonfatal(TO_CIRCUIT(intro_circ)->marked_for_close);
- goto perm_err;
+ if (TO_CIRCUIT(intro_circ)->marked_for_close) {
+ /* If the introduction circuit was closed, we were unable to send the
+ * cell for some reasons. In any case, the intro circuit has to be
+ * closed by the above function. We'll return a transient error so tor
+ * can recover and pick a new intro point. To avoid picking that same
+ * intro point, we'll note down the intro point failure so it doesn't
+ * get reused. */
+ hs_cache_client_intro_state_note(service_identity_pk,
+ &intro_circ->hs_ident->intro_auth_pk,
+ INTRO_POINT_FAILURE_GENERIC);
+ }
+ /* It is also possible that the rendezvous circuit was closed due to being
+ * unable to use the rendezvous point node_t so in that case, we also want
+ * to recover and let tor pick a new one. */
+ goto tran_err;
}
/* Cell has been sent successfully. Copy the introduction point
More information about the tor-commits
mailing list