Opened 5 months ago

Last modified 5 weeks ago

#29801 needs_review enhancement

Write a proposal for IPv6 "Happy Eyeballs" on Tor clients

Reported by: neel Owned by: neel
Priority: Medium Milestone: Tor: 0.4.2.x-final
Component: Core Tor/Tor Version:
Severity: Normal Keywords: ipv6, prop299
Cc: neel Actual Points:
Parent ID: #17835 Points:
Reviewer: nickm Sponsor:

Description

Teor has asked for updates in https://github.com/torproject/torspec/pull/61, but this PR has already been merged.

I have created a new PR for his suggestions: https://github.com/torproject/torspec/pull/63

I don't believe you can add to a merged PR so that's why a new PR exists.

Child Tickets

TicketTypeStatusOwnerSummary
#31039taskneeds_revisionReview proposal 306: IPv6 "Happy Eyeballs" for Tor clients

Change History (20)

comment:1 Changed 5 months ago by teor

Status: assignedneeds_review

comment:2 Changed 5 months ago by teor

Please don't assign me as the reviewer for this proposal.
I have already reviewed it, and I would like another reviewer to review it.

comment:3 Changed 5 months ago by teor

I have reviewed the new changes.

Before you make more changes, I want to discuss the issues on the pull request. And I want a review from another person.

comment:4 Changed 5 months ago by asn

Reviewer: nickm

comment:5 Changed 5 months ago by nickm

Hi! Here are my initial questions on the proposal.

First off, I'd like to understand how this interacts with the guard-selection algorithms in guard-spec.txt. At what stage(s) during guard selection _exactly_ do we take these probabilities into account?

Second, I'd like to understand better why we are picking the constants we have here. Why are werounding everything to multiples of 1/4, and rounding up to 1/4. I understand that we don't want to reduce the probability to 0, but why 1/4 and not some other value? Similarly, why are we counting "no route" as twice as bad as a regular failure, instead of 1.5x or 3x or something?

Third, I'm assuming that we only do this algorithm when we detect that we are dual-stack. If we only have an ipv6 address or an ipv4 address, then we should just assume that's the only one we can use, right?

Fourth, the halving algorithm seems kind of complicated to me. In some other places, we halve things like this based on the passage of time, rather than on their totals. Would that make sense here?

Fifth, I don't understand section 7. Why would a client need to make 4 connections at once to a guard?

Sixth, what should the default be for ClientAutoIPv6ORPort?

comment:6 Changed 5 months ago by nickm

Status: needs_reviewneeds_revision

comment:7 in reply to:  5 Changed 5 months ago by teor

Replying to nickm:

Third, I'm assuming that we only do this algorithm when we detect that we are dual-stack. If we only have an ipv6 address or an ipv4 address, then we should just assume that's the only one we can use, right?

And what do we do if we can't detect any of our addresses?
(Some OSes allow administrators to block the APIs that Tor uses to detect local addresses. I know of at least one Tor user who does this.)

Using local addresses is covered by ticket #27492 - Try IPv4 or IPv6 more often based on public or private IP addresses. I think it makes sense to include all the children of #17835 in this proposal. (It's the only missing child that adds a new feature. All the rest are refactoring or bugs.)

Fourth, the halving algorithm seems kind of complicated to me. In some other places, we halve things like this based on the passage of time, rather than on their totals. Would that make sense here?

Fifth, I don't understand section 7. Why would a client need to make 4 connections at once to a guard?

You're right. This section is not very clear.

Tor limits the number of simultaneous connection attempts *when bootstrapping* using the option ClientBootstrapConsensusMaxInProgressTries. The default is 3, which is too low for networks that drop all IPv4 or all IPv6 packets.

Tor doesn't limit the number of simultaneous connection attempts once it has bootstrapped. Unless the DDoS code added a limit on the client side?

Sixth, what should the default be for ClientAutoIPv6ORPort?

0 until it has been tested and we are sure we want the feature on by default.
Similarly, each flag should be off by default, until we have tested it. Then we can decide if it should be on by default.

comment:8 Changed 5 months ago by neel

Status: needs_revisionneeds_review

I have pushed your suggestions as two new commits to https://github.com/torproject/torspec/pull/63. I hope I didn't miss anything.

Setting as "needs review".

comment:9 Changed 5 months ago by neel

Have you (the Core Tor team) been busy? I haven't gotten a review for this proposal in its current form yet.

Could one of you please review this proposal?

comment:10 in reply to:  9 Changed 5 months ago by teor

Replying to neel:

Have you (the Core Tor team) been busy? I haven't gotten a review for this proposal in its current form yet.

Yes, we have been very busy finishing off sbws, and fixing bugs in release 0.4.0.
We've also reviewed a bunch of your patches on other tickets.

Could one of you please review this proposal?

I don't have time to review this week, but maybe nickm does?

comment:11 Changed 4 months ago by teor

Status: needs_reviewneeds_information

Here is my review:

I am not sure if we should implement this proposal.

I think this proposal is really complex. It risks destabilising Tor's network code. It uses a lot of randomness, which has led to hard-to-diagnose network bugs in the past. (I suggested many of the ideas in the proposal, so this complexity and risk is my fault.)

The proposal is also non-standard: it claims to be "Happy Eyeballs", but it does not implement RFC 8305. (The simplest version of RFC 8305 uses IPv4 and IPv6 addresses for the same machine. It tries IPv6, waits 250ms, then tries IPv4.)

I'd like to see an alternative proposal for implementing Happy Eyeballs in Tor. (Neel, you don't have to write that proposal.) Then we can decide which alternative to accept.

Here's a quick sketch of what a minimal Happy Eyeballs proposal would look like:

When selecting addresses:

  1. Modify extend_info_t so it contains an IPv4 and an IPv6 address
  2. When a bridge or relay has multiple addresses, add them both to the extend_info_t.

When connecting using an extend_info_t:

  1. If there is an existing authenticated connection, use it.
  2. If not, connect using the first available, allowed, and preferred address. (IPv4 by default.)
  3. Then, schedule a timer for connecting using the other address, if it is available and allowed. We should choose a timer value that is higher than most clients successful TLS authentication time.

When a connection successfully authenticates using TLS:

  1. Cancel any other connection timers
  2. Cancel any other in-progress connections

When all available and allowed connection attempts fail:

  1. Tell the rest of Tor that the connection has failed

comment:12 Changed 4 months ago by nickm

Milestone: Tor: unspecified

comment:13 in reply to:  11 Changed 4 months ago by neel

Sorry for the delay in responding.

Replying to teor:

Here is my review:

I am not sure if we should implement this proposal.

I think this proposal is really complex. It risks destabilising Tor's network code. It uses a lot of randomness, which has led to hard-to-diagnose network bugs in the past. (I suggested many of the ideas in the proposal, so this complexity and risk is my fault.)

The proposal is also non-standard: it claims to be "Happy Eyeballs", but it does not implement RFC 8305. (The simplest version of RFC 8305 uses IPv4 and IPv6 addresses for the same machine. It tries IPv6, waits 250ms, then tries IPv4.)

Makes sense.

I'd like to see an alternative proposal for implementing Happy Eyeballs in Tor. (Neel, you don't have to write that proposal.) Then we can decide which alternative to accept.

Sounds good.

I am happy to write a proposal if nobody else is willing to volunteer for that role. Whether or not I write the new proposal, I still plan to implement it and write the code for the new proposal.

Here's a quick sketch of what a minimal Happy Eyeballs proposal would look like:

When selecting addresses:

  1. Modify extend_info_t so it contains an IPv4 and an IPv6 address
  2. When a bridge or relay has multiple addresses, add them both to the extend_info_t.

When connecting using an extend_info_t:

  1. If there is an existing authenticated connection, use it.
  2. If not, connect using the first available, allowed, and preferred address. (IPv4 by default.)
  3. Then, schedule a timer for connecting using the other address, if it is available and allowed. We should choose a timer value that is higher than most clients successful TLS authentication time.

When a connection successfully authenticates using TLS:

  1. Cancel any other connection timers
  2. Cancel any other in-progress connections

When all available and allowed connection attempts fail:

  1. Tell the rest of Tor that the connection has failed

Sounds good.

comment:14 Changed 3 months ago by neel

If nobody else has volunteered, I plan to write the "new" proposal based on teor's feedback in Comment 11.

comment:15 Changed 8 weeks ago by neel

Status: needs_informationneeds_review

I have a revised proposal based on Comment 11. The torspec PR is here: https://github.com/torproject/torspec/pull/86

Sorry for the 3-month delay in this, I was really busy.

comment:16 in reply to:  15 Changed 8 weeks ago by teor

Status: needs_reviewneeds_revision

Replying to neel:

I have a revised proposal based on Comment 11. The torspec PR is here: https://github.com/torproject/torspec/pull/86

Sorry for the 3-month delay in this, I was really busy.

Thanks for this proposal!

We review proposals on the tor-dev mailing list, before they are merged to the repository:
https://gitweb.torproject.org/torspec.git/tree/proposals/001-process.txt#n69

Please send a copy of the proposal to the list.
Then wait a week or two for comments.
Then make any necessary revisions, and re-submit the proposal to the list.

Once that process is complete, we'll assign your proposal a number, and merge it into the torspec repository.
(It can't be proposal 302, because the most recent proposal is proposal 305.)
https://gitweb.torproject.org/torspec.git/tree/proposals

comment:17 Changed 8 weeks ago by neel

I have submitted the proposal (the number is 306) to the mailing list.

comment:18 Changed 8 weeks ago by teor

Parent ID: #27491#17835
Summary: Add teor's suggestions for Prop#299 (referring IPv4 or IPv6 based on IP Version Failure Count)Write a proposal for IPv6 "Happy Eyeballs" on Tor clients

comment:19 Changed 6 weeks ago by neel

Status: needs_revisionneeds_review

I have an revised proposal (same GitHub PR) based on the reviews from the tor-dev@ mailing list.

https://lists.torproject.org/pipermail/tor-dev/2019-July/013918.html
https://lists.torproject.org/pipermail/tor-dev/2019-July/013919.html

comment:20 Changed 5 weeks ago by nickm

Milestone: Tor: unspecifiedTor: 0.4.2.x-final
Note: See TracTickets for help on using tickets.