Trac issueshttps://gitlab.torproject.org/legacy/trac/-/issues2020-06-13T15:53:49Zhttps://gitlab.torproject.org/legacy/trac/-/issues/34446TestingTorNetwork should not set AssumeReachable 12020-06-13T15:53:49ZNick MathewsonTestingTorNetwork should not set AssumeReachable 1For S55 we are trying to get self-tests and remote-tests working with IPv6. To make sure we've succeed, we need to try them with Chutney. But to try them with chutney, we need them to be enabled.
With #33583 we removed AssumeReachable...For S55 we are trying to get self-tests and remote-tests working with IPv6. To make sure we've succeed, we need to try them with Chutney. But to try them with chutney, we need them to be enabled.
With #33583 we removed AssumeReachable from the chutney configurations. But that didn't actually have any effect, since `AssumeReachable 1` is implicit in `TestingTorNetwork 1`.Tor: 0.4.5.x-finalNick MathewsonNick Mathewsonhttps://gitlab.torproject.org/legacy/trac/-/issues/34445Split assumereachable into self and authority components2020-06-13T15:53:48ZNick MathewsonSplit assumereachable into self and authority componentsThis option governs both "should I check if I, a relay, reachable" and "should I, an authority, check if relays are reachable"? These should really be separated.
Calling this S55-must since we need it for chutney-ipv6 stuff.This option governs both "should I check if I, a relay, reachable" and "should I, an authority, check if relays are reachable"? These should really be separated.
Calling this S55-must since we need it for chutney-ipv6 stuff.Tor: 0.4.5.x-finalNick MathewsonNick Mathewsonhttps://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/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/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/33905Adjust "large number of connections to other relays" warnings2020-06-13T15:53:08ZteorAdjust "large number of connections to other relays" warningsIn #33880, we adjusted the limits MIN_RELAY_CONNECTIONS_TO_WARN, and the default number of connections allowed per relay.
In #33048, we will allow relays to extend via IPv6 as well as IPv4. So we should make these changes:
* warn if any...In #33880, we adjusted the limits MIN_RELAY_CONNECTIONS_TO_WARN, and the default number of connections allowed per relay.
In #33048, we will allow relays to extend via IPv6 as well as IPv4. So we should make these changes:
* warn if any relay has more than 4 connections (that is, more than 2 sides multiplied by 2 IP addresses, if there is disagreement over canonical connections).
* ignore 2 connections per relay.
Clients should only extend between two relays that support the Relay=3 protocol version. So we shouldn't need to backport this change.Tor: unspecifiedhttps://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 Mathewsonhttps://gitlab.torproject.org/legacy/trac/-/issues/33880Confusing "Your relay has a very large number of connections to other relays"...2020-06-13T15:53:02ZRoger DingledineConfusing "Your relay has a very large number of connections to other relays" relay messageA new relay operator reports this complaint in their logs, showing up hourly:
```
Your relay has a very large number of connections to other relays. Is your
outbound address the same as your relay address? Found 13 connections to 8
relay...A new relay operator reports this complaint in their logs, showing up hourly:
```
Your relay has a very large number of connections to other relays. Is your
outbound address the same as your relay address? Found 13 connections to 8
relays. Found 13 current canonical connections, in 0 of which we were a
non-canonical peer. 5 relays had more than 1 connection, 0 had more than 2, and
0 had more than 4 connections.
```
I checked, and their outbound address was the same as their relay address.
Upon investigation, it looks like the redundant connections are to directory authorities.
My theory is that the change from #17592 (which went into 0.3.1.1-alpha, commit d5a151a0) is responsible: while before that canonical relay-to-relay connections would expire after either side first reached its randomized "15 to 22.5 minute" timeout, now they expire after either side reaches its "45 to 75 minute" timeout. And since directory authorities test reachability every 1280 seconds (around 21.3 minutes), that means it is expected that most relays will have duplicate canonical connections with directory authorities.
Possible fixes:
(A) Change the notice-level log to make it clearer that it's not scary, or at least it's not actionable. Maybe that means making it info-level so nobody will see it. Probably not the best option, assuming there *are* cases where we do want relay operators to hear it.
(B) In channel_check_for_duplicates(), change `MIN_RELAY_CONNECTIONS_TO_WARN 5` to a high enough number that even if we have 2 canonical conns per authority, plus a bit more, the log message still doesn't trigger.
(C) In channel_check_for_duplicates(), skip over connections to directory authorities in some way, since we know they will be special.
(D) Make connections to or from directory authorities expire quicker, on the theory that they don't really need the same level of padding protection as other connections.
(E) Your idea here?
I'd be fine with any of B,C,D. Whichever one can be done with an easy, short, and non-invasive patch is my favorite. Maybe that's "B, raise it to 30"? That would make the message trigger when we have connections to more than 30 relays and also we have more than 45 connections open. Or we could pick the more conservative "raise it to 40", on the theory that small numbers are more likely to have edge cases and less indicative of major network problems anyway.
And while we're at it, it might be smart to say in the log message what action we want the relay operator take, e.g. "Please report:".Tor: unspecifiedNick MathewsonNick Mathewsonhttps://gitlab.torproject.org/legacy/trac/-/issues/33817Perform Basic Relay IPv6 Extends2020-06-13T15:52:51ZteorPerform Basic Relay IPv6 ExtendsCurrently, tor checks that extend cells have IPv4 addresses in:
[x] some functions in circuitbuild_relay.c (a new file introduced by #33633)
[x] channel_get_for_extend() in channel.c
[x] check_extend_cell() in onion.c
[x] extend_cell_fr...Currently, tor checks that extend cells have IPv4 addresses in:
[x] some functions in circuitbuild_relay.c (a new file introduced by #33633)
[x] channel_get_for_extend() in channel.c
[x] check_extend_cell() in onion.c
[x] extend_cell_from_extend2_cell_body() in onion.c
[?] and possibly other functions.
We want to allow IPv6 in all these places.
And when we have an IPv4 and an IPv6 address, we want to choose between them at random.Tor: 0.4.4.x-finalteorteorhttps://gitlab.torproject.org/legacy/trac/-/issues/33633Move extend and reachability code to the relay module2020-06-13T15:52:17ZteorMove extend and reachability code to the relay moduleMost of the extend and reachability code is already in the relay module.
But some code was left behind in src/core/or/circuitbuild.c.Most of the extend and reachability code is already in the relay module.
But some code was left behind in src/core/or/circuitbuild.c.Tor: 0.4.4.x-finalteorteorhttps://gitlab.torproject.org/legacy/trac/-/issues/33285Make sure relay protocol lists are sorted2020-06-13T15:53:39ZteorMake sure relay protocol lists are sortedTor's directory specification says that protocols should be sorted by keyword:
https://gitweb.torproject.org/torspec.git/tree/dir-spec.txt#n802
But many relays current protocol lists are:
`Cons=1-2 ... Relay=1-2 Padding=2 FlowCtrl=1`
W...Tor's directory specification says that protocols should be sorted by keyword:
https://gitweb.torproject.org/torspec.git/tree/dir-spec.txt#n802
But many relays current protocol lists are:
`Cons=1-2 ... Relay=1-2 Padding=2 FlowCtrl=1`
We should sort these lists, and make sure they stay sorted.Tor: 0.4.4.x-finalteorteorhttps://gitlab.torproject.org/legacy/trac/-/issues/33230Prop 311: 6.1. Ask Relay Operators to Test IPv6 Reachability2020-06-13T15:50:58ZteorProp 311: 6.1. Ask Relay Operators to Test IPv6 ReachabilitySee #33229 for advice on testing stages.
Write an email to the tor-relays list, asking relay operators to help test the proposal 311 IPv6 ORPort Reachability and Extends changes.
Once these changes are merged, volunteer relay and bridg...See #33229 for advice on testing stages.
Write an email to the tor-relays list, asking relay operators to help test the proposal 311 IPv6 ORPort Reachability and Extends changes.
Once these changes are merged, volunteer relay and bridge operators will be able to test them by:
* compiling from source,
* running nightly builds, or
* running alpha releases.
See proposal 311, section 6.1, relay operator test part:
https://gitweb.torproject.org/torspec.git/tree/proposals/311-relay-ipv6-reachability.txt#n671Tor: unspecifiedhttps://gitlab.torproject.org/legacy/trac/-/issues/33229Prop 311: 6.1. Test IPv6 ORPort Reachability on the Tor Network2020-06-13T15:50:58ZteorProp 311: 6.1. Test IPv6 ORPort Reachability on the Tor NetworkTest the IPv6 ORPort Reachability and Extends changes in proposal 311, on the public tor network, with a small number of relays and bridges.
We can do public tests after each major feature is merged. We can
also wait for multiple featur...Test the IPv6 ORPort Reachability and Extends changes in proposal 311, on the public tor network, with a small number of relays and bridges.
We can do public tests after each major feature is merged. We can
also wait for multiple features to merge, and test them all.
Here are some useful features that we can test:
#33222 / #33226 IPv6 ORPort Reachability Self-Tests / Relay=3 Protocol
#33223 Don't Publish Descriptor if IPv6 ORPort is Unreachable
See proposal 311, section 6.1, initial public test part:
https://gitweb.torproject.org/torspec.git/tree/proposals/311-relay-ipv6-reachability.txt#n671Tor: unspecifiedhttps://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/33221Prop 311: 4. Ensure Relay and Bridge IPv6 ORPorts are Reachable2020-06-13T15:50:54ZteorProp 311: 4. Ensure Relay and Bridge IPv6 ORPorts are ReachableThis is a parent ticket.
We propose that relays (and bridges) check their own IPv6 ORPort
reachability.
To check IPv6 ORPort reachability, relays (and bridges) extend circuits via
other relays (but not other bridges), and back to their...This is a parent ticket.
We propose that relays (and bridges) check their own IPv6 ORPort
reachability.
To check IPv6 ORPort reachability, relays (and bridges) extend circuits via
other relays (but not other bridges), and back to their own IPv6 ORPort.
If IPv6 reachability checks fail, relays (and bridges) should refuse to
publish their descriptors, if they believe IPv6 reachability checks are
reliable, and their IPv6 address was explicitly configured. (See
[Proposal 312: Relay Auto IPv6 Address] for the ways relays can guess their
IPv6 addresses.)
Directory authorities always publish their descriptors.
From Proposal 311, section 4:
https://gitweb.torproject.org/torspec.git/tree/proposals/311-relay-ipv6-reachability.txt#n246teorteorhttps://gitlab.torproject.org/legacy/trac/-/issues/33220Prop 311: 3. Allow Relay IPv6 Extends2020-06-13T15:50:53ZteorProp 311: 3. Allow Relay IPv6 ExtendsRelays may make a new connection over IPv6 when:
* they have an IPv6 ORPort,
* there is no existing authenticated connection to the requested relay, and
* the extend cell contains an IPv6 ORPort.
If these conditions are satisfied,...Relays may make a new connection over IPv6 when:
* they have an IPv6 ORPort,
* there is no existing authenticated connection to the requested relay, and
* the extend cell contains an IPv6 ORPort.
If these conditions are satisfied, and the extend cell also contains an
IPv4 ORPort, we propose that the relay choose between an IPv4 and an IPv6
connection at random.
If the extend cell does not contain an IPv4 ORPort, we propose that the
relay connects over IPv6. (Relays should support IPv6-only extend cells,
even though they are not used to test relay reachability in this proposal.)
A successful IPv6 connection also requires that:
* the requested relay has an IPv6 ORPort.
But extending relays must not check the consensus for other relays' IPv6
information. Consensuses may be out of date, particularly when relays are
doing reachability checks for new IPv6 ORPorts.
From proposal 311, section 3:
https://gitweb.torproject.org/torspec.git/tree/proposals/311-relay-ipv6-reachability.txt#n112teorteor