[Author Prev][Author Next][Thread Prev][Thread Next][Author Index][Thread Index]

Re: [tor-dev] Prop 311: Relay IPv6 Reachability



Hi Nick,

Thanks for your proposal review.

I've made most of the changes you've suggested, you can see the latest
version of the proposal here:
https://github.com/torproject/torspec/pull/103/files

I'll send another full text to the list when all the reviews are done.

I've also responded to your comments below individually:

> On 25 Jan 2020, at 06:09, Nick Mathewson <nickm@xxxxxxxxxxxxxx> wrote:
> 
> On Fri, Jan 24, 2020 at 1:31 AM teor <teor@xxxxxxxxxx> wrote:
>> 
>> 
>> Here is an initial draft of Proposal 311: Relay IPv6 Reachability.
>> 
>> This proposal includes:
>>  * relay IPv6 ORPort extends, and
>>  * relay IPv6 ORPort reachability checks.
>> 
>> This is the first of 3 proposals:
>> * Proposal 311: Relay IPv6 Reachability
>> * Proposal 312: Automatic Relay IPv6 Addresses
>> * Proposal 313: Relay IPv6 Statistics
>> (I haven't written the others yet!)
>> 
>> I also want to make some minor changes to Proposal 306, so that bridge
>> IPv6 behaviour stays in sync with client IPv6 behaviour.
>> 
>> The full text is included below, and it is also available as a GitHub
>> pull request:
>> https://github.com/torproject/torspec/pull/103
>> 
>> The related tickets are #24404 (proposal) and #24403 (implementation):
>> https://trac.torproject.org/projects/tor/ticket/24404
>> https://trac.torproject.org/projects/tor/ticket/24403
>> 
>> Please feel free to reply on this list, or via GitHub pull request
>> comments.
> 
> This proposal seems like a good start: It's solid, readable, and I
> think it would work.  There are few places where I have questions or
> suggested clarifications, and a few where I think we should consider
> other designs as well.
> 
> I'm going to only quote the parts I'm commenting on.
> 
>> 3.2.1. Making IPv6 ORPort Extend Connections
>> 
>>   Relays can make a new connection over IPv6 when:
>>     * there is no existing authenticated connection to the requested relay,
>>       and
>>     * the extend cell contains an IPv6 ORPort.
> 
> Also we should probably have the condition, "The relay itself supports
> IPv6, or thinks that it does"?

Thanks, I included that condition later in the proposal, but missed it here:
https://github.com/torproject/torspec/pull/103/commits/9204293f9b1e106ca73c4726703c3560f81cdeb8

>>   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.
> 
> [...]
>> 3.3. Alternative Extend Designs
>> 
>>   We briefly mention some potential extend designs, and the reasons that
>>   they were not used in this proposal.
>> 
>>   (Some designs may be proposed for future Tor versions, but are not necessary
>>   at this time.)
> 
> Let me sketch another alternative design here:
> 
> Suppose that a relay's extend cell contains the IPv4 address and the
> IPv6 address in their _preferred order_.  So if the party generating
> the extend cell would prefer an IPv4 connection, it puts the IPv4
> addess first; if it would prefer an IPv6 connection, it puts the IPv6
> address first.
> 
> The relay that receives the extend cell could respond in several ways:
>   * One possibility (similar to your 3.2.1) is to choose at random,
> with a higher probability given to the first option.
>   * One possibility (similar to your 3.3.1) is to try the first, and
> then try the second if the first one fails.
> 
> I think this scheme has some advantage, in that it lets the
> self-testing relay say "please try IPv6 if you can" or "please try
> IPv4 if you can" in a reliable way, and lets us migrate from the
> current behavior to the 3.3.1 behavior down the road.

I've added this alternate design as section 3.3.3, and renumbered the
previous section 3.3.3 to 3.4:
https://github.com/torproject/torspec/pull/103/commits/654bcb16815c0c57d5c551de53a79116a6d0188a

Apart from relay reachability self-tests, I'm not sure if there are any
other use cases where preferring IPv4 or IPv6 extends is necessary. Most
clients shouldn't care, they just want to get to an exit safely. (And
most clients can't care, because existing connections may be used, and
they may be over IPv4 or IPv6.)

But I wanted to include this design in the proposal, in case we find that
it is necessary during testing.

> [...]
>> 4.3. Refuse to Publish Descriptor if IPv6 ORPort is Unreachable
>> 
>>   If an IPv6 ORPort reachability check fails, relays (and bridges) should log
>>   a warning.
>> 
>>   In addition, if IPv6ORPort reachability checks are reliable, based on the
>>   number of relays in the network that support IPv6 extends, relays should
>>   refuse to publish their descriptor.
> 
> Is this the behavior we want?  Another behavior would be to omit our
> IPv6 orport from our descriptor if it is not reachable.  That way,
> relay operators could configure their relays to just listen on IPv6
> unconditionally, and let the relay figure out whether it should
> advertise IPv6 or not.  This behavior  seems preferable to me, since
> it has less chance of breaking a relay that would otherwise "work".

In general, when I've asked similar questions before, relay operators and
directory authority operators have wanted relays to fail fast, rather than
continuing to work with a partly-broken config.

The last time I asked, it was about directory authority IPv6 ORPort
reachability checks to relays. The directory authority operators told me
that relays which fail IPv6 reachability checks should not be in the
consensus, so relay operators notice, and fix their relays.

But in this case, I think we should make an exception for relays that
automatically guess their IPv6 address (rather than having it configured
explicitly by the operator). Since IPv6 address guesses will be a new
feature, and they happen automatically when operators upgrade tor, we
should just warn if a relay appears to have an IPv6 address, but it is
not actually reachable.

Relays that explicitly configure an IPv6 address should continue to fail
fast and fail loudly (in their logs). They were always going to fail anyway,
once they published their IPv6 address, and the directory authorities found
them unreachable.

> As a third possibility, we could have an option to say whether we
> should publish a descriptor if some of our addresses are not
> reachable.

With the new design above, I don't think that's necessary: relay operators
can simply remove their configured IPv6 ORPort address, and tor will start
publishing its descriptor.

See the changes here:
https://github.com/torproject/torspec/pull/103/commits/2e82cbd45679e9e17390ec9183c8bcb49c77a081

> [...]
>> 4.3.2. Add AssumeIPv6Reachable Option
>> 
>>   We add an AssumeIPv6Reachable torrc option and consensus parameter.
>> 
>>   If IPv6 ORPort checks have bugs that impact the health of the network,
>>   they can be disabled by setting AssumeIPv6Reachable=1 in the consensus
>>   parameters.
>> 
>>   If IPv6 ORPort checks have bugs that impact a particular relay (or bridge),
>>   they can be disabled by setting "AssumeIPv6Reachable 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.
>> 
>>   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 "AssumeIPv6Reachable",
>>   disabling both IPv4 and IPv6 ORPort reachability checks.
> 
> Should we have some way to AssumeReachable our IPv4 address only, and
> not our IPv6 address?  Or is that too much?

There's one configuration where it might be needed:
* a reachable IPv4 address,
* a reachable IPv6 address,
* but the IPv6 address is unsuitable for an ORPort.

In that case, tor would automatically discover both addresses, publish them
both, and the relay operator could not turn off IPv6.

So I think a better way to fix this issue is to turn off guessing the IPv6
address. I've added a new AddressDisableIPv6 as an required part of
proposal 312:
https://github.com/torproject/torspec/pull/105/files#diff-28c992d72bedaa9378a4f3627afb8694R361

>> 4.4. Optional Efficiency and Reliability Changes
>> 
>>   We propose some optional changes for efficiency and reliability, and
>>   describe their impact.
>> 
>>   Some of these changes may be more appropriate in future releases, or
>>   along with other proposed features.
>> 
> 
> 
> [...]
>> 4.4.3. Accurately Identifying Test Circuits
>> 
>>   The testing relay (or bridge) may confirm that the create cells it is
>>   receiving are from its own test circuits, and that test circuits are
>>   capable of returning create cells to the origin.
>> 
>>   Currently, relays confirm reachability if any create cell is received on
>>   any inbound connection (see section 4.1). Relays do not check that the
>>   circuit is a reachability test circuit, and they do not wait to receive the
>>   return created cell. This behaviour has resulted in difficult to diagnose
>>   bugs on some rare relay configurations.
>> 
>>   We propose these optional changes, to improve the efficiency of IPv4 and
>>   IPv6 ORPort reachability checks:
> 
> If we make these changes, I think we should track both whether we are
> "maybe reachable" (under the current definition of 'reachable') and
> "definitely reachable" (based on the new definition).   We should log
> different messages depending on whether we are "maybe reachable" but
> these new tests fail, or whether we are completely unreachable.

Done in:
https://github.com/torproject/torspec/pull/103/commits/6dceff0658e122ac00713f3fb25be29cedea0359

>>   Testing relays may:
>>     * check that the create cell is received from a test circuit
>>       (by comparing the received cell to the cells sent by test circuits),
>>     * check that the create cell is received on an inbound connection
>>       (this is existing behaviour),
>>     * if the create cell from a test circuit is received on an outbound
>>       connection, destroy the circuit (rather than returning a created cell),
>>       and
>>     * check that the created cell is returned to the relay on a test circuit
>>       (by comparing the remote address of the final hop on the circuit, to
>>       the local IPv4 and IPv6 ORPort addresses).
>> 
>>   TODO: work out how to efficiently match inbound create cells to test
>>         circuits.
> 
> One possibility: we could store a set of our test circuits' extend
> cells g^X values, and then check incoming cells create cells against
> that set.

Also done in:
https://github.com/torproject/torspec/pull/103/commits/6dceff0658e122ac00713f3fb25be29cedea0359

> There are probably fancier designs based on EC trickery or tweaks to
> the ntor protocol, but I think we'd best avoid them.

I think you're right.

>> 4.4.4. Allowing More Relay IPv6 Extends
>> 
>>   Currently, clients, relays, and bridges do not include IPv6 ORPorts in their
>>   extend cells.
>> 
>>   In this proposal, we only make relays (and bridges) extend over IPv6 on
>>   the final hop of test circuits. This limited use of IPv6 extends means that
>>   IPv6 connections will still be uncommon.
>> 
>>   We propose these optional changes, to increase the number of IPv6
>>   connections between relays:
>> 
>>   To increase the number of IPv6 connections, relays that support IPv6
>>   extends may want to use them for all hops of their own circuits. Relays
>>   make their own circuits for reachability tests, bandwidth tests, and
>>   ongoing preemptive circuits. (Bridges can not change their behaviour,
>>   because they try to imitate clients.)
>> 
>>   We propose a torrc option and consensus parameter SendIPv6CircuitExtends,
>>   which is only supported on relays (and not bridges or clients). This option
>>   makes relays send IPv4 and IPv6 ORPorts in all their extend cells, when
>>   supported by the extending and receiving relay. (See section 3.2.1.)
>> 
>>   TODO: Is there a shorter name, that isn't easily confused with enabling
>>         support for other nodes to perform IPv6 extends via this relay?
> 
> I have another concern here: if this option is truly relay-only, it
> should probably have a name that indicates the fact.

Changed to RelaySendIPv6Extends in:
https://github.com/torproject/torspec/pull/103/commits/f874424ebc9dafb7c721ed1893c93bb990f4f2c9

>> 4.5. Alternate Reachability Designs
>> 
>>   We briefly mention some potential reachability designs, and the reasons that
>>   they were not used in this proposal.
>> 
>> 4.5.1. Removing IPv4 ORPorts from Extend Cells
>> 
>>   We avoid designs that only include IPv6 ORPorts in extend cells, and remove
>>   IPv4 ORPorts.
>> 
>>   Only including the IPv6 ORPort would provide slightly more specific
>>   reachability check circuits. However, we don't need IPv6-only designs,
>>   because relays continue trying different reachability circuits until they
>>   confirm reachability.
>> 
>>   IPv6-only designs also make it easy to distinguish relay reachability extend
>>   cells from other extend cells. This distinguisher will become more of an
>>   issue as IPv6 extends become more common in the network (see sections 4.2.2
>>   and 4.4.4).
>> 
>>   Removing the IPv4 ORPort also provides no fallback, if the IPv6 ORPort is
>>   actually unreachable. IPv6-only failures do not affect reachability checks,
>>   but they will become important in the future, as other circuit types start
>>   using IPv6 extends.
>> 
>>   IPv6-only reachability designs also increase the number of special cases in
>>   the implementation. (And the likelihood of subtle bugs.)
>> 
>>   These designs may be appropriate in future, when there are IPv6-only bridges
>>   or relays.
> 
> As an aside: I think in the long run, the future is for clients to
> treat relays' link specifiers as completely opaque when they are
> making extend cells.
> 
> That is, if a client is extending to some relay X, it should know
> "here is a blob that you use as X's link specifiers" -- it can be the
> authorities' job to decide which IP addresses and which identity keys
> such a blob needs to contain.
> 
> (Of course, a client still needs to know a relay's address when making
> a connection itself, but that's more in prop306 territory.)

Seems fine to me, but I don't think we're looking that far into the
future in this proposal.

>> 5. New Relay Subprotocol Version
>> 
>>   We reserve Tor subprotocol "Relay=3" for:
>>     * relays that support for IPv6 extends, and
>>     * relays and bridges that support IPv6 ORPort reachability checks,
>>   as described in this proposal.
> 
> I don't fully understand; this reads as ambiguous to me.  Does
> "Relay=3" mean "IF I have an IPv6 address, I support IPv6 extends and
> checks"?  Or does it mean "I have an IPv6 address, and I support IPv6
> extends and checks"?
> 
> I would argue for the former interpretation, since no other
> subprotocol version is specified to depend on the relay's network
> configuration.

It was always meant to be configuration-independent. It was documented
as configuration-independent in section 5.1, I've now made those
changes in the introduction to section 5 as well:
https://github.com/torproject/torspec/pull/103/commits/2662c688170696fbed2ba7e4dfa7f33ccf702435

>> 5.1. Tor Specification Changes
>> 
>>   We propose the following changes to the [Tor Specification], once this
>>   proposal is implemented.
>> 
>>   Adding a new Relay subprotocol version lets testing relays identify other
>>   relays that support IPv6 extends. It also allows us to eventually recommend
>>   or require support for IPv6 extends on all relays (and bridges).
>> 
>>   Append to the Relay version 2 subprotocol specification:
>> 
>>          Relay=2 has limited IPv6 support:
>>            * Clients do not include IPv6 ORPorts in EXTEND2 cells.
> 
> I think that this is saying too much; we should leave it out.  Clients
> on the current Tor network MAY include IPv6 ORPorts, after all: we
> don't want to retroactively specify that any implementations that _do_
> include them are noncompliant with subprotocol v2.

I made these useful features "may not" in:
https://github.com/torproject/torspec/pull/103/commits/2662c688170696fbed2ba7e4dfa7f33ccf702435

>>            * Relays do not not initiate IPv6 connections in response to
>>              EXTEND2 cells containing IPv6 ORPorts.
> 
> Is this always true?   Would "may not" or "do not always" be more correct?

It's always true of the tor implementation that we maintain. I made these
useful features "may not" in:
https://github.com/torproject/torspec/pull/103/commits/2662c688170696fbed2ba7e4dfa7f33ccf702435

>>          However, relays accept inbound connections to their IPv6 ORPorts,
>>          and will extend circuits via those connections.
>> 
>>   "3" -- relays support extending over IPv6 connections in response to an
>>          EXTEND2 cell containing an IPv6 ORPort. Relays and bridges perform
>>          IPv6 ORPort reachability checks. Client behaviour does not change.
> 
> Should this be "self-checks" instead of "checks" for clarity?
> 
> Also, I wonder whether having self-checking be part of the relay
> protocol is correct.

I don't think the rest of the network cares about self-checks, so I removed
them from this section:
https://github.com/torproject/torspec/pull/103/commits/2662c688170696fbed2ba7e4dfa7f33ccf702435

I also reviewed the rest of the proposal, it's obvious when we're talking
about self-checks, and when we're talking about authority checks.

>>          This subprotocol is advertised by all relays and bridges, regardless
>>          of their configured ORPorts.
> 
> This "all" should probably be removed or clarified.  How about  "all
> relays and bridges running software that supports it, whether or not
> they have IPv6 ORPorts configured."

I clarified that the protocol applies to some tor versions, regardless of
their current configuration:
https://github.com/torproject/torspec/pull/103/commits/2662c688170696fbed2ba7e4dfa7f33ccf702435

>>                                                           But relays without a configured IPv6
>>          ORPort may refuse to extend over IPv6. And bridges always refuse to
>>          extend over IPv6, because they try to imitate client behaviour.
> 
> This last seems like it's saying that bridges SHOULD or MUST refuse to
> extend over IPv6; it might be better to say that they MAY, so that if
> client behavior changes, bridges can change too.

I clarified that bridges may not extend:
https://github.com/torproject/torspec/pull/103/commits/2662c688170696fbed2ba7e4dfa7f33ccf702435

Although, we'll need bridge clients to start sending IPv6 ORPorts in
EXTEND2 cells, before bridges can extend over IPv6. But these changes
may not need a new protocol version. (We could use a consensus parameter
instead.)

>>          A successful IPv6 extend requires:
>>            * Relay subprotocol version 3 (or later) and an IPv6 ORPort on the
>>              extending relay,
>>            * an IPv6 ORPort in the EXTEND2 cell, and
>>            * an IPv6 ORPort on the accepting relay.
>>          (Because different tor instances can have different views of the
>>          network, these checks should be done when the path is selected.
>>          Extending relays should only check local IPv6 information, before
>>          attempting the extend.)
>> 
>>          This subprotocol version is described in proposal 311, and
>>          implemented in Tor 0.4.4.1-alpha. (TODO: check version after code is
>>          merged).
> 
> I think that the "pick a random ORPort" behavior should be specified
> here too, as part of relay subprotocol v3, if we keep it.


I added the random selection, and added a TODO to check the final
implementation after we merge:
https://github.com/torproject/torspec/pull/103/commits/2662c688170696fbed2ba7e4dfa7f33ccf702435

T

Attachment: signature.asc
Description: Message signed with OpenPGP

_______________________________________________
tor-dev mailing list
tor-dev@xxxxxxxxxxxxxxxxxxxx
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-dev