[tor-bugs] #24898 [Core Tor/Tor]: We have two conflicting notions of channel_is_client()

[Tor Bug Tracker & Wiki]

#24898: We have two conflicting notions of channel_is_client()
------------------------------+--------------------------------
     Reporter:  arma          |      Owner:  (none)
         Type:  defect        |     Status:  new
     Priority:  Medium        |  Milestone:  Tor: 0.3.3.x-final
    Component:  Core Tor/Tor  |    Version:
     Severity:  Normal        |   Keywords:
Actual Points:                |  Parent ID:
       Points:                |   Reviewer:
      Sponsor:                |
------------------------------+--------------------------------
 In channel_tls_process_netinfo_cell() we have a section that says
 {{{
       if (!(chan->conn->handshake_state->authenticated)) {
         tor_assert(tor_digest_is_zero(
                   (const char*)(chan->conn->handshake_state->
                       authenticated_rsa_peer_id)));
         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));
 }}}

 That's great and awesome -- it's a clear solid definition of client: a
 client is somebody who did the client handshake with us, not the relay
 handshake.

 But then in command.c we have this weird clause:
 {{{
   if (connection_or_digest_is_known_relay(chan->identity_digest)) {
 rep_hist_note_circuit_handshake_requested(create_cell->handshake_type);
     // 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.
     channel_clear_client(chan);
   } else {
     channel_mark_client(chan);
   }
 }}}

 This definition of client is much murkier: a client is anybody who, at
 this moment of receiving a create cell, had earlier presented an identity
 digest that isn't in our current consensus and that we don't have a cached
 descriptor for.

 I think we should standardize on one definition, and I think we should
 pick the first one: it's clean and not full of false positives.

--
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/24898>
Tor Bug Tracker & Wiki <https://trac.torproject.org/>
The Tor Project: anonymity online