Merge branch 'bug22805_v2_squashed'

9 files changed, 39 insertions, 26 deletions

diff --git a/changes/bug22805 b/changes/bug22805
new file mode 100644
index 0000000000..2b0369da30
--- /dev/null
+++ b/changes/bug22805
@@ -0,0 +1,10 @@
+  o Minor features (relay):
+    - When choosing which circuits can be expired as unused, consider
+      circuits from clients even if those clients used regular CREATE
+      cells to make them; and do not consider circuits from relays even if
+      they were made with CREATE_FAST. Part of ticket 22805.
+
+  o Code simplification and refactoring:
+    - Remove various ways of testing circuits and connections for
+      "clientness"; instead, favor channel_is_client().
+      Part of ticket 22805.
diff --git a/src/or/channel.c b/src/or/channel.c
index faece55981..0b5a7fde90 100644
--- a/src/or/channel.c
+++ b/src/or/channel.c
@@ -4090,7 +4090,7 @@ channel_mark_bad_for_new_circs(channel_t *chan)
  */
 
 int
-channel_is_client(channel_t *chan)
+channel_is_client(const channel_t *chan)
 {
   tor_assert(chan);
 
@@ -4112,6 +4112,20 @@ channel_mark_client(channel_t *chan)
 }
 
 /**
+ * Clear the client flag
+ *
+ * Mark a channel as being _not_ from a client
+ */
+
+void
+channel_clear_client(channel_t *chan)
+{
+  tor_assert(chan);
+
+  chan->is_client = 0;
+}
+
+/**
  * Get the canonical flag for a channel
  *
  * This returns the is_canonical for a channel; this flag is determined by
diff --git a/src/or/channel.h b/src/or/channel.h
index 204b244293..32336fe1d2 100644
--- a/src/or/channel.h
+++ b/src/or/channel.h
@@ -666,11 +666,12 @@ int channel_is_bad_for_new_circs(channel_t *chan);
 void channel_mark_bad_for_new_circs(channel_t *chan);
 int channel_is_canonical(channel_t *chan);
 int channel_is_canonical_is_reliable(channel_t *chan);
-int channel_is_client(channel_t *chan);
+int channel_is_client(const channel_t *chan);
 int channel_is_local(channel_t *chan);
 int channel_is_incoming(channel_t *chan);
 int channel_is_outgoing(channel_t *chan);
 void channel_mark_client(channel_t *chan);
+void channel_clear_client(channel_t *chan);
 int channel_matches_extend_info(channel_t *chan, extend_info_t *extend_info);
 int channel_matches_target_addr_for_extend(channel_t *chan,
                                            const tor_addr_t *target);
diff --git a/src/or/channelpadding.c b/src/or/channelpadding.c
index ccaf5b4ec8..435436c45c 100644
--- a/src/or/channelpadding.c
+++ b/src/or/channelpadding.c
@@ -71,7 +71,7 @@ static int consensus_nf_pad_single_onion;
  *  its a client, use that. Then finally verify in the consensus).
  */
 #define CHANNEL_IS_CLIENT(chan, options) \
-  (!public_server_mode((options)) || (chan)->is_client || \
+  (!public_server_mode((options)) || channel_is_client(chan) || \
       !connection_or_digest_is_known_relay((chan)->identity_digest))
 
 /**
diff --git a/src/or/circuitbuild.c b/src/or/circuitbuild.c
index aa048f8c31..b36fed63b3 100644
--- a/src/or/circuitbuild.c
+++ b/src/or/circuitbuild.c
@@ -1612,12 +1612,12 @@ onionskin_answer(or_circuit_t *circ,
 
   memcpy(circ->rend_circ_nonce, rend_circ_nonce, DIGEST_LEN);
 
-  circ->is_first_hop = (created_cell->cell_type == CELL_CREATED_FAST);
+  int used_create_fast = (created_cell->cell_type == CELL_CREATED_FAST);
 
   append_cell_to_circuit_queue(TO_CIRCUIT(circ),
                                circ->p_chan, &cell, CELL_DIRECTION_IN, 0);
   log_debug(LD_CIRC,"Finished sending '%s' cell.",
-            circ->is_first_hop ? "created_fast" : "created");
+            used_create_fast ? "created_fast" : "created");
 
   /* Ignore the local bit when ExtendAllowPrivateAddresses is set:
    * it violates the assumption that private addresses are local.
diff --git a/src/or/circuituse.c b/src/or/circuituse.c
index e6f1e4df88..343ce3bcf5 100644
--- a/src/or/circuituse.c
+++ b/src/or/circuituse.c
@@ -1514,7 +1514,7 @@ circuit_expire_old_circuits_clientside(void)
 #define IDLE_ONE_HOP_CIRC_TIMEOUT 60
 
 /** Find each non-origin circuit that has been unused for too long,
- * has no streams on it, used a create_fast, and ends here: mark it
+ * has no streams on it, came from a client, and ends here: mark it
  * for close.
  */
 void
@@ -1530,9 +1530,9 @@ circuit_expire_old_circuits_serverside(time_t now)
     /* If the circuit has been idle for too long, and there are no streams
      * on it, and it ends here, and it used a create_fast, mark it for close.
      */
-    if (or_circ->is_first_hop && !circ->n_chan &&
+    if (or_circ->p_chan && channel_is_client(or_circ->p_chan) &&
+        !circ->n_chan &&
         !or_circ->n_streams && !or_circ->resolving_streams &&
-        or_circ->p_chan &&
         channel_when_last_xmit(or_circ->p_chan) <= cutoff) {
       log_info(LD_CIRC, "Closing circ_id %u (empty %d secs ago)",
                (unsigned)or_circ->p_circ_id,
diff --git a/src/or/command.c b/src/or/command.c
index 56134659c5..ae419ad068 100644
--- a/src/or/command.c
+++ b/src/or/command.c
@@ -331,7 +331,7 @@ command_process_create_cell(cell_t *cell, channel_t *chan)
     // Needed for chutney: Sometimes relays aren't in the consensus yet, and
     // get marked as clients. This resets their channels once they appear.
     // Probably useful for normal operation wrt relay flapping, too.
-    chan->is_client = 0;
+    channel_clear_client(chan);
   } else {
     channel_mark_client(chan);
   }
@@ -353,16 +353,6 @@ command_process_create_cell(cell_t *cell, channel_t *chan)
     int len;
     created_cell_t created_cell;
 
-    /* If the client used CREATE_FAST, it's probably a tor client or bridge
-     * relay, and we must not use it for EXTEND requests (in most cases, we
-     * won't have an authenticated peer ID for the extend).
-     * Public relays on 0.2.9 and later will use CREATE_FAST if they have no
-     * ntor onion key for this relay, but that should be a rare occurrence.
-     * Clients on 0.3.1 and later avoid using CREATE_FAST as much as they can,
-     * even during bootstrap, so the CREATE_FAST check is most accurate for
-     * earlier tor client versions. */
-    channel_mark_client(chan);
-
     memset(&created_cell, 0, sizeof(created_cell));
     len = onion_skin_server_handshake(ONION_HANDSHAKE_TYPE_FAST,
                                        create_cell->onionskin,
diff --git a/src/or/connection_edge.c b/src/or/connection_edge.c
index e413e5bc36..9098cb6908 100644
--- a/src/or/connection_edge.c
+++ b/src/or/connection_edge.c
@@ -3434,7 +3434,8 @@ connection_exit_begin_conn(cell_t *cell, circuit_t *circ)
     port = bcell.port;
 
     if (or_circ && or_circ->p_chan) {
-      if ((or_circ->is_first_hop ||
+      const int client_chan = channel_is_client(or_circ->p_chan);
+      if ((client_chan ||
            (!connection_or_digest_is_known_relay(
                 or_circ->p_chan->identity_digest) &&
           should_refuse_unknown_exits(options)))) {
@@ -3444,10 +3445,10 @@ connection_exit_begin_conn(cell_t *cell, circuit_t *circ)
         log_fn(LOG_PROTOCOL_WARN, LD_PROTOCOL,
                "Attempt by %s to open a stream %s. Closing.",
                safe_str(channel_get_canonical_remote_descr(or_circ->p_chan)),
-               or_circ->is_first_hop ? "on first hop of circuit" :
-                                       "from unknown relay");
+               client_chan ? "on first hop of circuit" :
+                             "from unknown relay");
         relay_send_end_cell_from_edge(rh.stream_id, circ,
-                                      or_circ->is_first_hop ?
+                                      client_chan ?
                                         END_STREAM_REASON_TORPROTOCOL :
                                         END_STREAM_REASON_MISC,
                                       NULL);
diff --git a/src/or/or.h b/src/or/or.h
index c924145568..10a2b57415 100644
--- a/src/or/or.h
+++ b/src/or/or.h
@@ -3473,9 +3473,6 @@ typedef struct or_circuit_t {
   /* We have already received an INTRODUCE1 cell on this circuit. */
   unsigned int already_received_introduce1 : 1;
 
-  /** True iff this circuit was made with a CREATE_FAST cell. */
-  unsigned int is_first_hop : 1;
-
   /** If set, this circuit carries HS traffic. Consider it in any HS
    *  statistics. */
   unsigned int circuit_carries_hs_traffic_stats : 1;