We have two conflicting notions of channel_is_client()
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.