Opened 6 years ago

Closed 3 years ago

#9067 closed defect (fixed)

Choice of address and match of fascist_firewall_allows_address* need to consider ipv6

Reported by: nickm Owned by: teor
Priority: Medium Milestone: Tor: 0.2.8.x-final
Component: Core Tor/Tor Version:
Severity: Normal Keywords: tor-client, 025-triaged, ipv6
Cc: ln5 Actual Points:
Parent ID: #17840 Points:
Reviewer: Sponsor:

Description

Right now, when we decide "can we reach node X", we only pass the node's primary address to fasist_firewall_allows_*.

Also, when we decide whether to use an primary or secondary address, we don't look at fascist_firewall_* as far as I know.

Child Tickets

Change History (15)

comment:1 Changed 6 years ago by nickm

Specifically, fasist_firewall_allows_or and fasist_firewall_allows_address_* should pretty much die outside of policies.c

comment:2 Changed 6 years ago by ln5

Cc: ln5 added

comment:3 Changed 5 years ago by andrea

Triage for 0.2.5.x: is this ever actually causing false negatives for reachability? If so, I think we should keep this for 0.2.5.

comment:4 Changed 5 years ago by nickm

Keywords: 025-triaged added

comment:5 Changed 5 years ago by nickm

Milestone: Tor: 0.2.5.x-finalTor: 0.2.6.x-final

Deferring to 0.2.6; fixing it has just as much risk for causing false positives. (For instance, if you have a reachableaddresses policy that is accurate for IPv4, but you have no idea what IPv6 stuff your firewall blocks.)

comment:6 Changed 5 years ago by nickm

Milestone: Tor: 0.2.6.x-finalTor: 0.2.???

comment:7 Changed 3 years ago by teor

Parent ID: #6027
Severity: Normal

comment:8 Changed 3 years ago by teor

Keywords: ipv6 added
Parent ID: #17811

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

Replying to nickm:

Deferring to 0.2.6; fixing it has just as much risk for causing false positives. (For instance, if you have a reachableaddresses policy that is accurate for IPv4, but you have no idea what IPv6 stuff your firewall blocks.)

I think we can fix the false positives, see #9068 for details.

comment:10 Changed 3 years ago by teor

Milestone: Tor: 0.2.???Tor: 0.2.8.x-final

This is fixed in #17840, it can close when #17840 closes.

comment:11 Changed 3 years ago by teor

Parent ID: #17811#17840

comment:12 Changed 3 years ago by teor

Owner: set to teor
Status: newaccepted

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

Replying to nickm:

Deferring to 0.2.6; fixing it has just as much risk for causing false positives. (For instance, if you have a reachableaddresses policy that is accurate for IPv4, but you have no idea what IPv6 stuff your firewall blocks.)

The only risk of false positives is if the policy contains "reject *".
(And I can't see any way we can work around that.)

Otherwise, fascist_firewall_allows_address_* eventually calls compare_known_tor_addr_to_addr_policy, where the default action is ADDR_POLICY_ACCEPTED.

comment:14 in reply to:  13 Changed 3 years ago by teor

Replying to teor:

Replying to nickm:

Deferring to 0.2.6; fixing it has just as much risk for causing false positives. (For instance, if you have a reachableaddresses policy that is accurate for IPv4, but you have no idea what IPv6 stuff your firewall blocks.)

The only risk of false positives is if the policy contains "reject *".
(And I can't see any way we can work around that.)

Ugh, options_validate() appends reject *:* to Reachable*Addresses.

And there's no way to work around that without either:

  • never connecting to IPv6 if Reachable*Addresses has no accept *:<port> entries
  • always connecting to IPv6 if Reachable*Addresses only has reject <ipv4>:* entries

So if ClientIPv6 is set and all Reachable*Addresses look IPv4-only, I'll warn the user they should review their Reachable*Addresses policies.

This ticket can close when #17840 closes.

comment:15 Changed 3 years ago by teor

Resolution: fixed
Status: acceptedclosed

We resolved this in #17840 by warning the user if they have ClientUseIPv[46] set, but their Reachable*Addresses rules block all IPv[46] connections.

See my branch feature17840-v11-squashed in #17840.

Note: See TracTickets for help on using tickets.