GitLab is used only for code review, issue tracking and project management. Canonical locations for source code are still https://gitweb.torproject.org/ https://git.torproject.org/ and git-rw.torproject.org.

Commit 19302a1d authored by George Kadianakis's avatar George Kadianakis

Merge remote-tracking branch 'tor-gitlab/mr/148' into master

parents f7adf365 cb4cedae
Pipeline #1403 passed with stage
in 55 minutes and 39 seconds
o Minor features (bootstrap reporting):
- When reporting bootstrapping status on a relay, do not consider
connections that have never been the target of an origin circuit.
Previously, all connection failures were treated as potential
bootstrapping failures, including those that had been opened because of
client requests. Closes ticket 25061.
......@@ -2395,12 +2395,16 @@ channel_is_better(channel_t *a, channel_t *b)
* *msg_out to a message describing the channel's state and our next action,
* and set *launch_out to a boolean indicated whether the caller should try to
* launch a new channel with channel_connect().
*
* If `for_origin_circ` is set, mark the channel as interesting for origin
* circuits, and therefore interesting for our bootstrapping reports.
*/
MOCK_IMPL(channel_t *,
channel_get_for_extend,(const char *rsa_id_digest,
const ed25519_public_key_t *ed_id,
const tor_addr_t *target_ipv4_addr,
const tor_addr_t *target_ipv6_addr,
bool for_origin_circ,
const char **msg_out,
int *launch_out))
{
......@@ -2440,8 +2444,15 @@ channel_get_for_extend,(const char *rsa_id_digest,
if (!CHANNEL_IS_OPEN(chan)) {
/* If the address matches, don't launch a new connection for this
* circuit. */
if (matches_target)
if (matches_target) {
++n_inprogress_goodaddr;
if (for_origin_circ) {
/* We were looking for a connection for an origin circuit; this one
* matches, so we'll note that we decided to use it for an origin
* circuit. */
channel_mark_as_used_for_origin_circuit(chan);
}
}
continue;
}
......
......@@ -526,6 +526,7 @@ void channel_mark_for_close(channel_t *chan);
int channel_write_packed_cell(channel_t *chan, packed_cell_t *cell);
void channel_listener_mark_for_close(channel_listener_t *chan_l);
void channel_mark_as_used_for_origin_circuit(channel_t *chan);
/* Channel callback registrations */
......@@ -661,6 +662,7 @@ MOCK_DECL(channel_t *, channel_get_for_extend,(
const struct ed25519_public_key_t *ed_id,
const tor_addr_t *target_ipv4_addr,
const tor_addr_t *target_ipv6_addr,
bool for_origin_circ,
const char **msg_out,
int *launch_out));
......
......@@ -360,6 +360,31 @@ channel_tls_handle_incoming(or_connection_t *orconn)
return chan;
}
/**
* Set the `potentially_used_for_bootstrapping` flag on the or_connection_t
* corresponding to the provided channel.
*
* This flag indicates that if the connection fails, it might be interesting
* to the bootstrapping subsystem. (The bootstrapping system only cares about
* channels that we have tried to use for our own circuits. Other channels
* may have been launched in response to EXTEND cells from somebody else, and
* if they fail, it won't necessarily indicate a bootstrapping problem.)
**/
void
channel_mark_as_used_for_origin_circuit(channel_t *chan)
{
if (BUG(!chan))
return;
if (chan->magic != TLS_CHAN_MAGIC)
return;
channel_tls_t *tlschan = channel_tls_from_base(chan);
if (BUG(!tlschan))
return;
if (tlschan->conn)
tlschan->conn->potentially_used_for_bootstrapping = 1;
}
/*********
* Casts *
********/
......
......@@ -574,6 +574,7 @@ circuit_handle_first_hop(origin_circuit_t *circ)
&firsthop->extend_info->ed_identity,
orport4 ? &orport4->addr : NULL,
orport6 ? &orport6->addr : NULL,
true,
&msg,
&should_launch);
......@@ -590,6 +591,11 @@ circuit_handle_first_hop(origin_circuit_t *circ)
log_info(LD_CIRC,"connect to firsthop failed. Closing.");
return -END_CIRC_REASON_CONNECTFAILED;
}
/* We didn't find a channel, but we're launching one for an origin
* circuit. (If we decided not to launch a channel, then we found at
* least one once good in-progress channel use for this circuit, and
* marked it in channel_get_for_extend().) */
channel_mark_as_used_for_origin_circuit(n_chan);
circuit_chan_publish(circ, n_chan);
}
......@@ -602,6 +608,8 @@ circuit_handle_first_hop(origin_circuit_t *circ)
} else { /* it's already open. use it. */
tor_assert(!circ->base_.n_hop);
circ->base_.n_chan = n_chan;
/* We found a channel, and we're using it for an origin circuit. */
channel_mark_as_used_for_origin_circuit(n_chan);
circuit_chan_publish(circ, n_chan);
log_debug(LD_CIRC,"Conn open for %s. Delivering first onion skin.",
safe_str_client(extend_info_describe(firsthop->extend_info)));
......
......@@ -74,6 +74,11 @@ struct or_connection_t {
unsigned int is_outgoing:1;
unsigned int proxy_type:3; /**< One of PROXY_NONE...PROXY_HAPROXY */
unsigned int wide_circ_ids:1;
/** True iff a failure on this connection indicates a posssible
* bootstrapping problem. We set this as true if we notice that this
* connection could handle a pending origin circuit, or if we launch it to
* handle an origin circuit. */
unsigned int potentially_used_for_bootstrapping:1;
/** True iff this connection has had its bootstrap failure logged with
* control_event_bootstrap_problem. */
unsigned int have_noted_bootstrap_problem:1;
......
......@@ -348,6 +348,18 @@ control_event_bootstrap_prob_or, (const char *warn, int reason,
{
int dowarn = 0;
if (! or_conn->potentially_used_for_bootstrapping) {
/* We never decided that this channel was a good match for one of our
* origin_circuit_t objects. That means that we probably launched it
* for somebody else, most likely in response to an EXTEND cell.
*
* Since EXTEND cells can contain arbitrarily broken descriptions of
* relays, a failure on this connection here won't necessarily indicate a
* bootstrapping problem.
*/
return;
}
if (or_conn->have_noted_bootstrap_problem)
return;
......
......@@ -475,6 +475,7 @@ circuit_extend(struct cell_t *cell, struct circuit_t *circ)
&ec.ed_pubkey,
ipv4_valid ? &ec.orport_ipv4.addr : NULL,
ipv6_valid ? &ec.orport_ipv6.addr : NULL,
false,
&msg,
&should_launch);
......
......@@ -1382,7 +1382,7 @@ test_channel_for_extend(void *arg)
/* The expected result is chan2 because it is older than chan1. */
ret_chan = channel_get_for_extend(digest, &ed_id, &ipv4_addr, &ipv6_addr,
&msg, &launch);
false, &msg, &launch);
tt_assert(ret_chan);
tt_ptr_op(ret_chan, OP_EQ, chan2);
tt_int_op(launch, OP_EQ, 0);
......@@ -1391,7 +1391,7 @@ test_channel_for_extend(void *arg)
/* Switch that around from previous test. */
chan2->timestamp_created = chan1->timestamp_created + 1;
ret_chan = channel_get_for_extend(digest, &ed_id, &ipv4_addr, &ipv6_addr,
&msg, &launch);
false, &msg, &launch);
tt_assert(ret_chan);
tt_ptr_op(ret_chan, OP_EQ, chan1);
tt_int_op(launch, OP_EQ, 0);
......@@ -1401,7 +1401,7 @@ test_channel_for_extend(void *arg)
* channel 2 should be picked due to how channel_is_better() works. */
chan2->timestamp_created = chan1->timestamp_created;
ret_chan = channel_get_for_extend(digest, &ed_id, &ipv4_addr, &ipv6_addr,
&msg, &launch);
false, &msg, &launch);
tt_assert(ret_chan);
tt_ptr_op(ret_chan, OP_EQ, chan1);
tt_int_op(launch, OP_EQ, 0);
......@@ -1413,7 +1413,7 @@ test_channel_for_extend(void *arg)
/* Condemned the older channel. */
chan1->state = CHANNEL_STATE_CLOSING;
ret_chan = channel_get_for_extend(digest, &ed_id, &ipv4_addr, &ipv6_addr,
&msg, &launch);
false, &msg, &launch);
tt_assert(ret_chan);
tt_ptr_op(ret_chan, OP_EQ, chan2);
tt_int_op(launch, OP_EQ, 0);
......@@ -1423,7 +1423,7 @@ test_channel_for_extend(void *arg)
/* Make the older channel a client one. */
channel_mark_client(chan1);
ret_chan = channel_get_for_extend(digest, &ed_id, &ipv4_addr, &ipv6_addr,
&msg, &launch);
false, &msg, &launch);
tt_assert(ret_chan);
tt_ptr_op(ret_chan, OP_EQ, chan2);
tt_int_op(launch, OP_EQ, 0);
......@@ -1435,7 +1435,7 @@ test_channel_for_extend(void *arg)
memset(&dumb_ed_id, 0, sizeof(dumb_ed_id));
ret_chan = channel_get_for_extend(digest, &dumb_ed_id,
&ipv4_addr, &ipv6_addr,
&msg, &launch);
false, &msg, &launch);
tt_assert(!ret_chan);
tt_str_op(msg, OP_EQ, "Not connected. Connecting.");
tt_int_op(launch, OP_EQ, 1);
......@@ -1445,7 +1445,7 @@ test_channel_for_extend(void *arg)
chan1->state = CHANNEL_STATE_OPENING;
chan2->state = CHANNEL_STATE_OPENING;
ret_chan = channel_get_for_extend(digest, &ed_id, &ipv4_addr, &ipv6_addr,
&msg, &launch);
false, &msg, &launch);
tt_assert(!ret_chan);
tt_str_op(msg, OP_EQ, "Connection in progress; waiting.");
tt_int_op(launch, OP_EQ, 0);
......@@ -1455,7 +1455,7 @@ test_channel_for_extend(void *arg)
/* Mark channel 1 as bad for circuits. */
channel_mark_bad_for_new_circs(chan1);
ret_chan = channel_get_for_extend(digest, &ed_id, &ipv4_addr, &ipv6_addr,
&msg, &launch);
false, &msg, &launch);
tt_assert(ret_chan);
tt_ptr_op(ret_chan, OP_EQ, chan2);
tt_int_op(launch, OP_EQ, 0);
......@@ -1466,7 +1466,7 @@ test_channel_for_extend(void *arg)
channel_mark_bad_for_new_circs(chan1);
channel_mark_bad_for_new_circs(chan2);
ret_chan = channel_get_for_extend(digest, &ed_id, &ipv4_addr, &ipv6_addr,
&msg, &launch);
false, &msg, &launch);
tt_assert(!ret_chan);
tt_str_op(msg, OP_EQ, "Connections all too old, or too non-canonical. "
" Launching a new one.");
......@@ -1478,7 +1478,7 @@ test_channel_for_extend(void *arg)
test_chan_should_be_canonical = 0;
test_chan_should_match_target = 0;
ret_chan = channel_get_for_extend(digest, &ed_id, &ipv4_addr, &ipv6_addr,
&msg, &launch);
false, &msg, &launch);
tt_assert(!ret_chan);
tt_str_op(msg, OP_EQ, "Connections all too old, or too non-canonical. "
" Launching a new one.");
......
......@@ -1214,6 +1214,7 @@ mock_channel_get_for_extend(const char *rsa_id_digest,
const ed25519_public_key_t *ed_id,
const tor_addr_t *target_ipv4_addr,
const tor_addr_t *target_ipv6_addr,
bool for_origin_circ,
const char **msg_out,
int *launch_out)
{
......@@ -1221,6 +1222,7 @@ mock_channel_get_for_extend(const char *rsa_id_digest,
(void)ed_id;
(void)target_ipv4_addr;
(void)target_ipv6_addr;
(void)for_origin_circ;
/* channel_get_for_extend() requires non-NULL arguments */
tt_ptr_op(msg_out, OP_NE, NULL);
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment