Opened 3 years ago

Last modified 13 months ago

#19919 new defect

If ORPort address is publicly routable, use it to guess Address

Reported by: teor Owned by:
Priority: Medium Milestone: Tor: unspecified
Component: Core Tor/Tor Version: Tor: 0.2.5.10
Severity: Normal Keywords: address-guessing config torrc bootsrap relay needs-analysis tarpit
Cc: s7r Actual Points:
Parent ID: #24403 Points: 1
Reviewer: Sponsor:

Description

Splitting this off from #13953: we'll warn in that ticket, and make the change to the address resolution order in this one.

Child Tickets

Change History (16)

comment:1 Changed 3 years ago by s7r

Cc: s7r added

While we're at it we might want to make a patch that will also cover IPv6. While it's quite possible to have multiple public IPv4 addresses on the same box, for IPv6 cases where we have just one public IPv6 address are rare. We cannot currently publish a descriptor that has only IPv6 address, but are we testing the IPv6 addresses for reachability?

comment:2 in reply to:  1 Changed 3 years ago by teor

Replying to s7r:

While we're at it we might want to make a patch that will also cover IPv6. While it's quite possible to have multiple public IPv4 addresses on the same box, for IPv6 cases where we have just one public IPv6 address are rare. We cannot currently publish a descriptor that has only IPv6 address, but are we testing the IPv6 addresses for reachability?

No. #6939.

The IPv6 address in the descriptor is the address from the first IPv6 ORPort. Address is not used.

comment:3 Changed 3 years ago by s7r

I think this ticket can fix the behavior permanently by assuming Address = The first *Advertised* ORPort (and DirPort?)of course publicly routable, unless otherwise explicitly set by the user in torrc. Also, I think it's fine to also assume OutboundBindAddress is the same IP address (first publicly routable Advertised ORPort), unless otherwise explicitly set in torrc. This can apply to both IPv4 and IPv6 without any problems.

If there's no IP address set with ORPort, and Address is not set, maintain the current behavior to guess Address and build the descriptor - the current behavior to guess Address is not broken in any way, to the contrary it works good, we just need to add this improvement for boxes with multiple public IP addresses and/or multiple Tor instances.

comment:4 in reply to:  3 ; Changed 3 years ago by teor

Replying to s7r:

I think this ticket can fix the behavior permanently by assuming Address = The first *Advertised* ORPort (and DirPort?)of course publicly routable, unless otherwise explicitly set by the user in torrc.

Let's just use ORPort for consistency between IPv4 and IPv6.
If the user sets a different DirPort address, #13953 in 0.2.9 will warn them.
And if they set the same one, that's perfectly ok.

Also, I think it's fine to also assume OutboundBindAddress is the same IP address (first publicly routable Advertised ORPort), unless otherwise explicitly set in torrc. This can apply to both IPv4 and IPv6 without any problems.

Not when the relay is behind a NAT: OutboundBindAddress is the *internal* address.
And if we use this default, there's no way to specify "your default interface", which is the current default behaviour. Unless there's a specific issue here that's causing confusion, let's just leave this alone. Or tackle it in a separate ticket.

If there's no IP address set with ORPort, and Address is not set, maintain the current behavior to guess Address and build the descriptor - the current behavior to guess Address is not broken in any way, to the contrary it works good, we just need to add this improvement for boxes with multiple public IP addresses and/or multiple Tor instances.

We can do this, it just involves adding a small amount of code to resolve_my_address(), right before we use get_interface_address() to do the guess, we should instead use the first advertised IPv4 ORPort address from get_first_advertised_addr_by_type_af(CONN_TYPE_OR_LISTENER, AF_INET);

(There's some duplicate code in resolve_my_address(), let's refactor it out before changing it.)

The code might be similar to router_check_descriptor_address_port_consistency(), but will likely be much simpler.

comment:5 in reply to:  4 ; Changed 3 years ago by s7r

Replying to teor:

Let's just use ORPort for consistency between IPv4 and IPv6.
If the user sets a different DirPort address, #13953 in 0.2.9 will warn them.
And if they set the same one, that's perfectly ok.

Agreed.

Also, I think it's fine to also assume OutboundBindAddress is the same IP address (first publicly routable Advertised ORPort), unless otherwise explicitly set in torrc. This can apply to both IPv4 and IPv6 without any problems.

Not when the relay is behind a NAT: OutboundBindAddress is the *internal* address.
And if we use this default, there's no way to specify "your default interface", which is the current default behaviour. Unless there's a specific issue here that's causing confusion, let's just leave this alone. Or tackle it in a separate ticket.

Also true. So we need more cases:

  1. Simple ORPort (where user just enters ORPort 9001)
  2. Flagged ORPort (where user enters ORPort 9001 NoListen OR NoAdvertise)
  3. Explicit ORPort (where user enters ORPort public.ip:9001)
  4. Explicit flagged ORPort (where user enters ORPort public.ip:9001 NoListen OR NoAdvertise)
  5. Explicit NAT ORPort (where user enters ORPort nat.ip:9001)
  6. Explicit flagged NAT ORPort (where user enters ORPort nat.ip:9001 NoListen or NoAdvertise)

Obviously for cases 1,2,5,6 and 4 if flagged NoListen we cannot make any assumption about OutboundBindAddress and that should remain as it is set now (equal to Address guessed by Tor via current methods).

But for case 3 and case 4 if it's not NoListen, we can assume that IP address is also Address and also OutboundBindAddress (it's publicly routable and it's being listened on).

If there's no IP address set with ORPort, and Address is not set, maintain the current behavior to guess Address and build the descriptor - the current behavior to guess Address is not broken in any way, to the contrary it works good, we just need to add this improvement for boxes with multiple public IP addresses and/or multiple Tor instances.

We can do this, it just involves adding a small amount of code to resolve_my_address(), right before we use get_interface_address() to do the guess, we should instead use the first advertised IPv4 ORPort address from get_first_advertised_addr_by_type_af(CONN_TYPE_OR_LISTENER, AF_INET);

(There's some duplicate code in resolve_my_address(), let's refactor it out before changing it.)

The code might be similar to router_check_descriptor_address_port_consistency(), but will likely be much simpler.

Neat ! Sounds like a clean and simple approach that will do the job just fine. resolve_my_address() should check if there's anything publicly routable manually configured by user with ORPort before starting the current algorithm to guess Address, that's all.

comment:6 in reply to:  5 ; Changed 3 years ago by teor

Replying to s7r:

Replying to teor:

Also, I think it's fine to also assume OutboundBindAddress is the same IP address (first publicly routable Advertised ORPort), unless otherwise explicitly set in torrc. This can apply to both IPv4 and IPv6 without any problems.

Not when the relay is behind a NAT: OutboundBindAddress is the *internal* address.
And if we use this default, there's no way to specify "your default interface", which is the current default behaviour. Unless there's a specific issue here that's causing confusion, let's just leave this alone. Or tackle it in a separate ticket.

Also true. So we need more cases:

  1. Simple ORPort (where user just enters ORPort 9001)
  2. Flagged ORPort (where user enters ORPort 9001 NoListen OR NoAdvertise)
  3. Explicit ORPort (where user enters ORPort public.ip:9001)
  4. Explicit flagged ORPort (where user enters ORPort public.ip:9001 NoListen OR NoAdvertise)
  5. Explicit NAT ORPort (where user enters ORPort nat.ip:9001)
  6. Explicit flagged NAT ORPort (where user enters ORPort nat.ip:9001 NoListen or NoAdvertise)

Obviously for cases 1,2,5,6 and 4 if flagged NoListen we cannot make any assumption about OutboundBindAddress and that should remain as it is set now (equal to Address guessed by Tor via current methods).

But for case 3 and case 4 if it's not NoListen, we can assume that IP address is also Address and also OutboundBindAddress (it's publicly routable and it's being listened on).

Have there ever been any issues reported by relay operators about OutboundBindAddress being wrong? If not, let's leave it as an advanced option - the default seems fine for almost all relay operators. And there's the risk that any automatic guessing gets it wrong, causing inexplicable failures for some operators, where before it worked for them.

Otherwise, I'm all for changing Address selection to be more robust.

comment:7 in reply to:  6 ; Changed 3 years ago by s7r

Replying to teor:

Have there ever been any issues reported by relay operators about OutboundBindAddress being wrong? If not, let's leave it as an advanced option - the default seems fine for almost all relay operators. And there's the risk that any automatic guessing gets it wrong, causing inexplicable failures for some operators, where before it worked for them.

Otherwise, I'm all for changing Address selection to be more robust.

Not that I am aware of. But it makes sense for cases 3 and 4 if not NoListen to assume OutboundBindAddress == Address == first publicly routable explicitly configured ORPort that we listen on. The logic here is that usually an explicit ORPort listening on public IP is configured on boxes with multiple public IP addresses, and the user wants to assign one of them for the relay (or run multiple different Tor instances/relays) case in which having the outgoing address the same with the one we receive traffic on is reasonable.

If it's just few more lines of code to also add this algorithm for OutboundBindAddress only for cases 3 and 4 if not NoListen, it could payoff (there is no risk, if it's configured as ORPort obviously it's an IP to be used with Tor, so making sure we also use it for outgoing shouldn't be catastrophic).

If it's not so easy then yes, Address is of course the most important part of the problem.

comment:8 in reply to:  7 Changed 3 years ago by teor

Replying to s7r:

Replying to teor:

Have there ever been any issues reported by relay operators about OutboundBindAddress being wrong? If not, let's leave it as an advanced option - the default seems fine for almost all relay operators. And there's the risk that any automatic guessing gets it wrong, causing inexplicable failures for some operators, where before it worked for them.

Otherwise, I'm all for changing Address selection to be more robust.

Not that I am aware of. But it makes sense for cases 3 and 4 if not NoListen to assume OutboundBindAddress == Address == first publicly routable explicitly configured ORPort that we listen on. The logic here is that usually an explicit ORPort listening on public IP is configured on boxes with multiple public IP addresses, and the user wants to assign one of them for the relay (or run multiple different Tor instances/relays) case in which having the outgoing address the same with the one we receive traffic on is reasonable.

If it's just few more lines of code to also add this algorithm for OutboundBindAddress only for cases 3 and 4 if not NoListen, it could payoff (there is no risk, if it's configured as ORPort obviously it's an IP to be used with Tor, so making sure we also use it for outgoing shouldn't be catastrophic).

If it's not so easy then yes, Address is of course the most important part of the problem.

It is more complex: Tor never modifies the current OutboundBindAddress, and has no mechanism for guessing that address. Please open a separate ticket for OutboundBindAddress.

comment:9 Changed 3 years ago by s7r

Correct. We should leave OutboundBindAddress for the time being. This tor-dev thread confirms adding the algorithm to OutboundBindAdress would not work for at least this use case:

https://lists.torproject.org/pipermail/tor-dev/2016-September/011429.html

While applying the algorithm to Address as described in this ticket would fix it, and plenty more use cases. So, we'll leave OutboundBindAddress aside currently. I don't know, does OutboundBindAddress == Address unless configured otherwise?

comment:10 in reply to:  9 ; Changed 3 years ago by teor

Replying to s7r:

Correct. We should leave OutboundBindAddress for the time being. This tor-dev thread confirms adding the algorithm to OutboundBindAdress would not work for at least this use case:

https://lists.torproject.org/pipermail/tor-dev/2016-September/011429.html

While applying the algorithm to Address as described in this ticket would fix it, and plenty more use cases. So, we'll leave OutboundBindAddress aside currently. I don't know, does OutboundBindAddress == Address unless configured otherwise?

No, the outbound address is left unspecified by Tor unless configured explicitly by the user. So the OS determines the outbound address, which can vary depending on the destination (routing table) on multi-homed systems.

comment:11 in reply to:  10 Changed 3 years ago by s7r

Replying to teor:

No, the outbound address is left unspecified by Tor unless configured explicitly by the user. So the OS determines the outbound address, which can vary depending on the destination (routing table) on multi-homed systems.

Sounds good enough. Probably most users who need this setting take care of it at upstream level such as routing table or firewall rules - so this should be fine. Patching Address as you said is the way to go.

comment:12 Changed 2 years ago by teor

Milestone: Tor: 0.2.???Tor: 0.3.???

Milestone renamed

comment:13 Changed 2 years ago by nickm

Keywords: tor-03-unspecified-201612 added
Milestone: Tor: 0.3.???Tor: unspecified

Finally admitting that 0.3.??? was a euphemism for Tor: unspecified all along.

comment:14 Changed 2 years ago by nickm

Keywords: tor-03-unspecified-201612 removed

Remove an old triaging keyword.

comment:15 Changed 22 months ago by nickm

Keywords: address-guessing config torrc bootsrap relay needs-analysis tarpit added; 030-proposed removed

comment:16 Changed 13 months ago by teor

Parent ID: #24403

This bug will be easier to resolve after we do IPv6 address autodetection on relays.

Note: See TracTickets for help on using tickets.