Skip to content

Stop modifying addr on connections, and delete real_addr

In connection_or_check_canonicity(), we overwrite conn.addr with the canonical address of the relay in the consensus. That makes accurate logging impossible.

And so we also need channel.real_addr, to store the actual address of the remote peer. And for some reason, we also have conn.address, a string copy of the peer's canonical address and port.

See: https://github.com/torproject/tor/blob/7f9eaec538b7d01e0d1b130dc4cf2ec634252d46/src/core/or/connection_or.c#L920

This is... a mess.

Here's the high-level interface I'd like to see:

  • use a function to format a connection or channel addresses for loogging
  • use exactly as many address and port variables as needed in connection and channel (no extras!)
  • qualify each address and port variable's name with its purpose

For example, here's one possible design:

  • delete addr, port, address, and real_addr
  • add remote_ap, a tor_addr_port_t that is the remote address and port of the TCP connection to the remote peer
  • implement connection_describe(), which:
    • formats remote_ap,
    • formats is_canonical (and any other useful info), and
    • calls node_describe() to format the canonical IPv4 and IPv6 addresses and ports of the remote peer.

We may also need separate fields for reverse proxied addresses, see the comment at: https://github.com/torproject/tor/blob/7517e1b5d31aada1f594c2594737a231d9d8e116/src/core/or/channeltls.c#L1339

If we need separate variables or functions for channels, we can use a similar design. (But ideally, re-using as many functions and variables as possible.)

This is important for Sponsor 55: getting accurate connection information will make diagnosing bugs much easier.

To upload designs, you'll need to enable LFS and have an admin enable hashed storage. More information