Trac issueshttps://gitlab.torproject.org/legacy/trac/-/issues2020-07-02T19:46:44Zhttps://gitlab.torproject.org/legacy/trac/-/issues/33226Prop 311: 5. Declare Support for Subprotocol Version "Relay=3"2020-07-02T19:46:44ZteorProp 311: 5. Declare Support for Subprotocol Version "Relay=3"This ticket depends on relay IPv6 extends in #33220.
We reserve Tor subprotocol "Relay=3" for tor versions where:
* relays may perform IPv6 extends, and
* bridges might not perform IPv6 extends,
as described in this proposal.
See p...This ticket depends on relay IPv6 extends in #33220.
We reserve Tor subprotocol "Relay=3" for tor versions where:
* relays may perform IPv6 extends, and
* bridges might not perform IPv6 extends,
as described in this proposal.
See proposal 311, section 5:
https://gitweb.torproject.org/torspec.git/tree/proposals/311-relay-ipv6-reachability.txt#n601Tor: 0.4.4.x-finalteorteorhttps://gitlab.torproject.org/legacy/trac/-/issues/33583Stop setting AssumeReachable on chutney relays and bridges2020-06-13T15:53:49ZteorStop setting AssumeReachable on chutney relays and bridgesWe need to stop setting AssumeReachable on relays and bridges in chutney networks, so they do reachability self-tests.
We should continue to set AssumeReachable on authorities (including the bridge authority), because it disables author...We need to stop setting AssumeReachable on relays and bridges in chutney networks, so they do reachability self-tests.
We should continue to set AssumeReachable on authorities (including the bridge authority), because it disables authority to relay reachability tests. (These tests are still performed on a half-hourly schedule, even in chutney networks. And they are out of scope for Sponsor 55.)
After we implement this change, we should also be able to implement #33581.teorteorhttps://gitlab.torproject.org/legacy/trac/-/issues/34412Additonal unit tests for functions related to ipv6 self-test2020-06-13T15:53:47ZNick MathewsonAdditonal unit tests for functions related to ipv6 self-testSee #33222 and #34200 for lists of functions that did not get unit tests as part of the #34200 merge.See #33222 and #34200 for lists of functions that did not get unit tests as part of the #34200 merge.Tor: 0.4.5.x-finalhttps://gitlab.torproject.org/legacy/trac/-/issues/34200Refactor tor's circuit path node selection checks2020-06-13T15:53:32ZteorRefactor tor's circuit path node selection checksIn #33222, we added an extra "can extend over IPv6" check to tor's circuit path node selection code.
To make sure it's applied consistently, I did a refactor of that code, so all those checks are in one function.In #33222, we added an extra "can extend over IPv6" check to tor's circuit path node selection code.
To make sure it's applied consistently, I did a refactor of that code, so all those checks are in one function.Tor: 0.4.5.x-finalteorteorhttps://gitlab.torproject.org/legacy/trac/-/issues/34137Make sure inform_testing_reachability() reports the correct ports2020-06-13T15:53:29ZteorMake sure inform_testing_reachability() reports the correct portsIn #33222, we added IPv6 ORPort support to inform_testing_reachability(). But that function checks the ORPorts and DirPort itself, rather than logging the reachability checks that were actually launched.
We'd like to pass flags so that ...In #33222, we added IPv6 ORPort support to inform_testing_reachability(). But that function checks the ORPorts and DirPort itself, rather than logging the reachability checks that were actually launched.
We'd like to pass flags so that it logs the actual reachability tests that are being run. (Rather than re-checking the relay's own routerinfo, which may have changed since the most recent reachability checks were launched.)
But inform_testing_reachability() is called when:
* the first circuit finishes building, or
* tor is reconfigured, and some circuits have already finished building.
So we need to do a bit of a refactor.
The refactor should preserve this behaviour:
* don't log until after the first circuit has finished building (rather than logging as soon as we start building reachability circuits)
And introduce this new behaviour:
* log the ports that were tested recently, rather than the ports that are currently available.
As an alternative, we could move some of the logging into the functions that actually launch the checks. And elevate some of those logs to notice level. (Note that these checks can be launched from at least 4 different locations in tor's code.)Tor: unspecifiedhttps://gitlab.torproject.org/legacy/trac/-/issues/34069Allow extend_info to contain both IPv4 and IPv6 ORPorts2020-06-13T15:53:20ZteorAllow extend_info to contain both IPv4 and IPv6 ORPortsTo send dual-stack EXTEND2 cells in #33222, we need to have IPv4 and IPv6 addresses in extend_info_t.To send dual-stack EXTEND2 cells in #33222, we need to have IPv4 and IPv6 addresses in extend_info_t.Tor: 0.4.5.x-finalhttps://gitlab.torproject.org/legacy/trac/-/issues/34068Decide how to handle control port events for IPv6 reachability self-tests2020-06-13T15:53:19ZteorDecide how to handle control port events for IPv6 reachability self-testsThe control spec has two reachability self-test events.
Here is how they are specified:
```
CHECKING_REACHABILITY
"ORADDRESS=IP:port"
"DIRADDRESS=IP:port"
We're going to start testing the reachability of our extern...The control spec has two reachability self-test events.
Here is how they are specified:
```
CHECKING_REACHABILITY
"ORADDRESS=IP:port"
"DIRADDRESS=IP:port"
We're going to start testing the reachability of our external OR port
or directory port.
REACHABILITY_SUCCEEDED
"ORADDRESS=IP:port"
"DIRADDRESS=IP:port"
We successfully verified the reachability of our external OR port or
directory port (depending on which of ORADDRESS or DIRADDRESS is
given.)
```
And here is what tor actually sends:
```
CHECKING_REACHABILITY ORADDRESS=IPv4:port
CHECKING_REACHABILITY DIRADDRESS=IPv4:port
REACHABILITY_SUCCEEDED ORADDRESS=IPv4:port
REACHABILITY_SUCCEEDED DIRADDRESS=IPv4:port
```
When we add IPv6 reachability events, we could break some (buggy) control parsers with:
```
CHECKING_REACHABILITY ORADDRESS=[IPv6]:port
REACHABILITY_SUCCEEDED ORADDRESS=[IPv6]:port
```
How should we handle this change?Tor: unspecifiedhttps://gitlab.torproject.org/legacy/trac/-/issues/33222Prop 311: 4.2. Implement IPv6 ORPort Reachability Self-Tests2020-06-13T15:53:19ZteorProp 311: 4.2. Implement IPv6 ORPort Reachability Self-Tests4.2. Checking IPv6 ORPort Reachability
We propose that testing relays (and bridges) select some IPv6 extend-capable
relays for their reachability circuits, and include their own IPv4 and IPv6
ORPorts in the final extend cells on those c...4.2. Checking IPv6 ORPort Reachability
We propose that testing relays (and bridges) select some IPv6 extend-capable
relays for their reachability circuits, and include their own IPv4 and IPv6
ORPorts in the final extend cells on those circuits.
The final extending relay will extend to the testing relay:
* using an existing authenticated connection to the testing relay
(which may be over IPv4 or IPv6), or
* over a new connection via the IPv4 or IPv6 ORPort in the extend cell.
The testing relay will confirm that test circuits can extend to both its
IPv4 and IPv6 ORPorts.
4.2.1. Selecting the Final Extending Relay
IPv6 ORPort reachability checks require an IPv6 extend-capable relay as
the second-last hop of reachability circuits. (The testing relay is the
last hop.)
IPv6-extend capable relays must have:
* Relay subprotocol version 3 (or later), and
* an IPv6 ORPort.
(See section 5.1 for the definition of Relay subprotocol version 3.)
The other relays in the path do not require any particular protocol
versions.
4.2.2. Extending from the Second-Last Hop
IPv6 ORPort reachability circuits should put the IPv4 and IPv6 ORPorts in
the extend cell for the final extend in reachability circuits.
Supplying both ORPorts makes these extend cells indistinguishable from
future client extend cells.
If reachability succeeds, the testing relay (or bridge) will accept the
final extend on one of its ORPorts, selected at random by the extending
relay (see section 3.2.1).
4.2.3. Separate IPv4 and IPv6 Reachability Flags
Testing relays (and bridges) will record reachability separately for IPv4
and IPv6 ORPorts, based on the ORPort that the test circuit was received on.
Here is a reliable way to do reachability self-tests for each ORPort:
1. Check for create cells on inbound ORPort connections from other relays
Check for a cell on any IPv4 and any IPv6 ORPort. (We can't know which listener(s) correspond to the advertised ORPorts, particularly when using port forwarding.) Make sure the cell was received on an inbound OR connection, that is authenticated to another relay.
2. Check for created cells from testing circuits on outbound OR connections
Check for a returned created cell on our IPv4 and IPv6 self-test circuits. Make sure those circuits were on outbound OR connections.
By combining these tests, we confirm that we can:
* reach our own ORPorts with testing circuits
* send and receive cells via inbound OR connections from other relays to our own ORPorts
* send and receive cells via outbound OR connections to other relays' ORPorts
This isn't a perfect test, there are a few false positives:
* relays with multiple IPv4 or IPv6 ORPorts, where only some ports are reachable:
* (this configuration is uncommon, but multiple ORPorts are supported)
* the final extend cell contains the advertised IPv6 address of the self-testing relay
* if the extending relay already has a connection to an old but working ORPort, it may use that connection instead
* so these tests can pass, even when the advertised ORPort is unreachable
* relays whose keys have been copied from another relay in the consensus, for similar reasons
* this configuration is rare and unsupported
From proposal 311, section 4.2:
https://gitweb.torproject.org/torspec.git/tree/proposals/311-relay-ipv6-reachability.txt#n283Tor: 0.4.5.x-finalteorteorhttps://gitlab.torproject.org/legacy/trac/-/issues/34067Separate tor's IPv4 and IPv6 reachability flags2020-06-13T15:53:19ZteorSeparate tor's IPv4 and IPv6 reachability flagsIn #33222, I make tor build circuits to IPv4 and IPv6 ORPorts.
But we also need to detect IPv4 and IPv6 reachability separately.In #33222, I make tor build circuits to IPv4 and IPv6 ORPorts.
But we also need to detect IPv4 and IPv6 reachability separately.Tor: 0.4.5.x-finalteorteorhttps://gitlab.torproject.org/legacy/trac/-/issues/34065Make routerset_contains_router() support IPv62020-06-13T15:53:18ZteorMake routerset_contains_router() support IPv6In router_should_check_reachability(), we test to see if the current relay is in ExcludeNodes.
But ExcludeNodes doesn't support IPv6, so if a user excludes our IPv6 address, we'll just ignore that setting.
This is not a required task.
...In router_should_check_reachability(), we test to see if the current relay is in ExcludeNodes.
But ExcludeNodes doesn't support IPv6, so if a user excludes our IPv6 address, we'll just ignore that setting.
This is not a required task.
But we should probably be explicit about ignoring IPv6 in the man page.Tor: 0.4.4.x-finalNeel Chauhanneel@neelc.orgNeel Chauhanneel@neelc.orghttps://gitlab.torproject.org/legacy/trac/-/issues/33224Prop 311: 4.3.2. Add AssumeReachableIPv6 Option and Consensus Parameter2020-06-13T15:53:17ZteorProp 311: 4.3.2. Add AssumeReachableIPv6 Option and Consensus ParameterWe add an AssumeReachableIPv6 torrc option and consensus parameter.
If IPv6 ORPort checks have bugs that impact the health of the network,
they can be disabled by setting AssumeReachableIPv6=1 in the consensus
parameters.
If IPv6 ORPor...We add an AssumeReachableIPv6 torrc option and consensus parameter.
If IPv6 ORPort checks have bugs that impact the health of the network,
they can be disabled by setting AssumeReachableIPv6=1 in the consensus
parameters.
If IPv6 ORPort checks have bugs that impact a particular relay (or bridge),
they can be disabled by setting "AssumeReachableIPv6 1" in the relay's
torrc.
This option disables IPv6 ORPort reachability checks, so relays publish
their descriptors if their IPv4 ORPort reachability checks succeed.
(Unlike AssumeReachable, AssumeReachableIPv6 has no effect on the existing
dirauth IPv6 reachability checks, which connect directly to relay ORPorts.)
The default for the torrc option is "auto", which checks the consensus
parameter. If the consensus parameter is not set, the default is "0".
"AssumeReachable 1" overrides all values of "AssumeReachableIPv6",
disabling both IPv4 and IPv6 ORPort reachability checks. Tor should warn if
AssumeReachable is 1, but AssumeReachableIPv6 is 0. (On directory
authorities, "AssumeReachable 1" also disables dirauth IPv4 and IPv6
reachability checks, which connect directly to relay ORPorts.
AssumeReachableIPv6 does not disable directory authority to relay IPv6
checks.)
See proposal 311, section 4.3.2:
https://gitweb.torproject.org/torspec.git/tree/proposals/311-relay-ipv6-reachability.txt#n403https://gitlab.torproject.org/legacy/trac/-/issues/34064Add an AssumeReachable consensus parameter2020-06-13T15:53:17ZteorAdd an AssumeReachable consensus parameterLike #33224, we want a network-wide consensus parameter that makes relays and bridges assume they are reachable.
We'll be modifying the IPv4 and IPv6 reachability code, so we need to be able to respond quickly to IPv4 reachability bugs ...Like #33224, we want a network-wide consensus parameter that makes relays and bridges assume they are reachable.
We'll be modifying the IPv4 and IPv6 reachability code, so we need to be able to respond quickly to IPv4 reachability bugs as well.Tor: unspecifiedhttps://gitlab.torproject.org/legacy/trac/-/issues/33956Define and use TOR_ADDRPORT_BUF_LEN2020-06-13T15:53:13ZteorDefine and use TOR_ADDRPORT_BUF_LENIn #33918, we discovered a bug where IPv6 addresses were being truncated in logs.
During the fix, we noticed that we had a TOR_ADDR_BUF_LEN, but no equivalent constant for addresses and ports. The new TOR_ADDRPORT_BUF_LEN should allow s...In #33918, we discovered a bug where IPv6 addresses were being truncated in logs.
During the fix, we noticed that we had a TOR_ADDR_BUF_LEN, but no equivalent constant for addresses and ports. The new TOR_ADDRPORT_BUF_LEN should allow space for:
* TOR_ADDR_BUF_LEN
* IPv6 brackets (2, if not included in TOR_ADDR_BUF_LEN already)
* the port separator (1)
* the port (5)
We should check for other truncation errors while making this change.Tor: 0.4.4.x-finalNeel Chauhanneel@neelc.orgNeel Chauhanneel@neelc.orghttps://gitlab.torproject.org/legacy/trac/-/issues/33919Write unit tests for channel_matches_target_addr_for_extend()2020-06-13T15:53:10ZteorWrite unit tests for channel_matches_target_addr_for_extend()In #33817, we modify channel_matches_target_addr_for_extend() to take an IPv4 and an IPv6 address.
We also modify channel_get_for_extend() to take an IPv4 and an IPv6 address. It has existing tests, but they ignore IP addresses. (And mo...In #33817, we modify channel_matches_target_addr_for_extend() to take an IPv4 and an IPv6 address.
We also modify channel_get_for_extend() to take an IPv4 and an IPv6 address. It has existing tests, but they ignore IP addresses. (And mock the matches_target() method, which is called by channel_matches_target_addr_for_extend().)
It would be useful to have tests that check that a channel matches, if its IP address matches the IPv4 or IPv6 address. But it's not urgent. (And the actual changes to the function are pretty trivial.)
The setup for these tasks is a bit tricky, we need to set up a channel_tls_t and an associated connection_t with a real_addr. (Note that #33898 may change the name of real_addr.)
This task is not urgent or important. Anyone can do this task.Tor: 0.4.4.x-finalhttps://gitlab.torproject.org/legacy/trac/-/issues/33918Stop truncating IPv6 addresses in channel logs2020-06-13T15:53:10ZteorStop truncating IPv6 addresses in channel logschannel_tls_get_remote_desc_method() formats IP addresses and ports, but its buffer is only 32 characters. IPv6 addresses can be up to 41 characters long, and the port is an extra 6 characters.channel_tls_get_remote_desc_method() formats IP addresses and ports, but its buffer is only 32 characters. IPv6 addresses can be up to 41 characters long, and the port is an extra 6 characters.Tor: 0.4.4.x-finalteorteorhttps://gitlab.torproject.org/legacy/trac/-/issues/33917Make IF_BUG_ONCE() support ALL_BUGS_ARE_FATAL2020-06-13T15:53:09ZteorMake IF_BUG_ONCE() support ALL_BUGS_ARE_FATAL... and DISABLE_ASSERTS_IN_UNIT_TESTS.
It's the only assertion/bug macro that doesn't support these debugging modes. Let's fix that.... and DISABLE_ASSERTS_IN_UNIT_TESTS.
It's the only assertion/bug macro that doesn't support these debugging modes. Let's fix that.Tor: 0.4.4.x-finalteorteorhttps://gitlab.torproject.org/legacy/trac/-/issues/33901Allow IPv6 extend cells2020-06-13T15:53:08ZteorAllow IPv6 extend cellsAs part of #33817, we want to allow IPv6 extend cells.
We want clients and relays to send:
* dual-stack EXTEND2 cells
* IPv6-only EXTEND2 cells
We want relays to parse:
* dual-stack EXTEND2 cells
* IPv6-only EXTEND2 cellsAs part of #33817, we want to allow IPv6 extend cells.
We want clients and relays to send:
* dual-stack EXTEND2 cells
* IPv6-only EXTEND2 cells
We want relays to parse:
* dual-stack EXTEND2 cells
* IPv6-only EXTEND2 cellsTor: 0.4.4.x-finalteorteorhttps://gitlab.torproject.org/legacy/trac/-/issues/33900Check for invalid zero IPv4 addresses and ports in extend cells2020-06-13T15:53:07ZteorCheck for invalid zero IPv4 addresses and ports in extend cellsWhen we send and parse extend cells, we check that their IPv4 address field is not AF_UNSPEC.
But we should also check for zero IPv4 addresses and zero ports. (Which are both invalid.)
Code and points in #33817, I just needed a separat...When we send and parse extend cells, we check that their IPv4 address field is not AF_UNSPEC.
But we should also check for zero IPv4 addresses and zero ports. (Which are both invalid.)
Code and points in #33817, I just needed a separate bug number.Tor: 0.4.4.x-finalteorteorhttps://gitlab.torproject.org/legacy/trac/-/issues/33899Allow IPv6 addresses to be canonical2020-06-13T15:53:06ZteorAllow IPv6 addresses to be canonicalIn #17604, we allowed IPv6 connections to set is_canonical_to_peer. But we did not let them set is_canonical.
So channel_tls_process_netinfo_cell() handles IPv6 addresses, but connection_or_check_canonicity() does not.
This inconsisten...In #17604, we allowed IPv6 connections to set is_canonical_to_peer. But we did not let them set is_canonical.
So channel_tls_process_netinfo_cell() handles IPv6 addresses, but connection_or_check_canonicity() does not.
This inconsistency could be one cause of the logging issues in #24841.
We should allow IPv6 connections to be canonical. We should also do some cleanup of the related connection address logging in #33898.Tor: 0.4.4.x-finalteorteorhttps://gitlab.torproject.org/legacy/trac/-/issues/33898Stop modifying addr on connections, and delete real_addr2020-06-13T15:53:06ZteorStop modifying addr on connections, and delete real_addrIn 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 remo...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.Tor: 0.4.5.x-finalNick MathewsonNick Mathewson