[tor-commits] [tor/master] Accurately identify client connections by their lack of peer authentication

[nickm at torproject.org]

commit f9af7e8bd01d4b7a70776a96c649a89cf1717dd1
Author: teor <teor2345 at gmail.com>
Date:   Wed Mar 1 16:23:02 2017 +1100

    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.
---
 changes/bug21406    |  5 +++++
 src/or/channel.h    |  4 ++--
 src/or/channeltls.c |  4 ++++
 src/or/command.c    | 10 ++++++++--
 4 files changed, 19 insertions(+), 4 deletions(-)

diff --git a/changes/bug21406 b/changes/bug21406
new file mode 100644
index 0000000..170e631
--- /dev/null
+++ b/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.
diff --git a/src/or/channel.h b/src/or/channel.h
index 26aa93b..748a099 100644
--- a/src/or/channel.h
+++ b/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) */
diff --git a/src/or/channeltls.c b/src/or/channeltls.c
index dbed95f..e734913 100644
--- a/src/or/channeltls.c
+++ b/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);
 
diff --git a/src/or/command.c b/src/or/command.c
index 5866c38..4d3830f 100644
--- a/src/or/command.c
+++ b/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));