Browse Source

Accurately identify client connections by their lack of peer authentication

This means that we bail out earlier if asked to extend to a client.

Follow-up to 21407.
Fixes bug 21406; bugfix on 0.2.4.23.
teor 7 years ago
parent
commit
f9af7e8bd0
4 changed files with 19 additions and 4 deletions
  1. 5 0
      changes/bug21406
  2. 2 2
      src/or/channel.h
  3. 4 0
      src/or/channeltls.c
  4. 8 2
      src/or/command.c

+ 5 - 0
changes/bug21406

@@ -0,0 +1,5 @@
+  o Minor bugfixes (code correctness):
+    - Accurately identify client connections using their lack of peer
+      authentication. This means that we bail out earlier if asked to extend
+      to a client. Follow-up to 21407.
+      Fixes bug 21406; bugfix on 0.2.4.23.

+ 2 - 2
src/or/channel.h

@@ -214,8 +214,8 @@ struct channel_s {
   unsigned int is_bad_for_new_circs:1;
 
   /** True iff we have decided that the other end of this connection
-   * is a client.  Channels with this flag set should never be used
-   * to satisfy an EXTEND request.  */
+   * is a client or bridge relay.  Connections with this flag set should never
+   * be used to satisfy an EXTEND request. */
   unsigned int is_client:1;
 
   /** Set if the channel was initiated remotely (came from a listener) */

+ 4 - 0
src/or/channeltls.c

@@ -1654,6 +1654,10 @@ channel_tls_process_netinfo_cell(cell_t *cell, channel_tls_t *chan)
         tor_assert(tor_mem_is_zero(
                   (const char*)(chan->conn->handshake_state->
                                 authenticated_ed25519_peer_id.pubkey), 32));
+        /* If the client never authenticated, it's a tor client or bridge
+         * relay, and we must not use it for EXTEND requests (nor could we, as
+         * there are no authenticated peer IDs) */
+        channel_mark_client(TLS_CHAN_TO_BASE(chan));
         channel_set_circid_type(TLS_CHAN_TO_BASE(chan), NULL,
                chan->conn->link_proto < MIN_LINK_PROTO_FOR_WIDE_CIRC_IDS);
 

+ 8 - 2
src/or/command.c

@@ -344,8 +344,14 @@ command_process_create_cell(cell_t *cell, channel_t *chan)
     int len;
     created_cell_t created_cell;
 
-    /* Make sure we never try to use the OR connection on which we
-     * received this cell to satisfy an EXTEND request,  */
+    /* 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));