Add flag for whether an OR conn "counts" for bootstrap tracking

We set this flag if we've launched the connection in order to
satisfy an origin circuit, or when we decide the connection _would_
satisfy an origin circuit.  These are the only or_connections we
want to consider for bootstrapping: other or_connections are opened
because of client EXTEND requests, and they may succeed or fail
because of the clients' confusion or misconfiguration.

Closes #25061.

changes/ticket25061

@@ -0,0 +1,6 @@
+  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.

src/core/or/channel.c

@@ -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,12 @@ 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) {
+          channel_mark_as_used_for_origin_circuit(chan);
+        }
+      }
       continue;
     }
 

src/core/or/channel.h

@@ -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));
 

src/core/or/channeltls.c

@@ -360,6 +360,28 @@ 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.
+ **/
+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 *
  ********/

src/core/or/circuitbuild.c

@@ -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,7 @@ circuit_handle_first_hop(origin_circuit_t *circ)
         log_info(LD_CIRC,"connect to firsthop failed. Closing.");
         return -END_CIRC_REASON_CONNECTFAILED;
       }
+      channel_mark_as_used_for_origin_circuit(n_chan);
       circuit_chan_publish(circ, n_chan);
     }
 
@@ -602,6 +604,7 @@ 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;
+    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)));

src/core/or/or_connection_st.h

@@ -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;

src/feature/control/control_bootstrap.c

@@ -348,6 +348,9 @@ control_event_bootstrap_prob_or, (const char *warn, int reason,
 {
   int dowarn = 0;
 
+  if (! or_conn->potentially_used_for_bootstrapping)
+    return;
+
   if (or_conn->have_noted_bootstrap_problem)
     return;
 

src/feature/relay/circuitbuild_relay.c

@@ -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);
 

src/test/test_channel.c

@@ -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.");

src/test/test_circuitbuild.c

@@ -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);