Opened 7 years ago

Closed 7 years ago

Last modified 7 years ago

#5535 closed enhancement (implemented)

Make clients use "a" lines in network status documents

Reported by: ln5 Owned by: ln5
Priority: Medium Milestone: Tor: 0.2.4.x-final
Component: Core Tor/Tor Version:
Severity: Keywords: ipv6 tor-client
Cc: arma Actual Points:
Parent ID: #4564 Points:
Reviewer: Sponsor:

Description

As per prop 186, a client which can use IPv6 should use should be
added to router status entry "a" lines.

Child Tickets

Change History (25)

comment:1 Changed 7 years ago by arma

Summary: Mak clients use "a" lines in network status documentsMake clients use "a" lines in network status documents

comment:2 in reply to:  description Changed 7 years ago by ln5

Owner: set to ln5
Parent ID: #4563#4564
Status: newaccepted

Parse that sentence does not! It should probably say

"As per prop 186, a client which can use IPv6 should use router status
entry "a" lines."

But in fact, that's not what prop 186 says. IPv6-only clients not
running with bridges are the only ones to use "a" lines.

Moving this ticket off #4563.

comment:3 Changed 7 years ago by ln5

Keywords: ipv6 added

We don't want clients to use IPv6 OR ports before before some
substantial fraction of the clients have upgraded.

Let's add a configuration option UseIPv6Guards defaulting to 0 which
when set to 1 makes clients connect to guards over IPv6.

comment:4 Changed 7 years ago by nickm

Seems good to me.

comment:5 Changed 7 years ago by ln5

Input from #tor-dev:

<armadev> best solved by a consensus param. ideally one that starts out in
          place saying "no not yet", and then we stop saying it and forget it
          ever existed when we want to transition.

Nick, do you have an opinion?

comment:6 Changed 7 years ago by ln5

onion_extend_cpath() (calling extend_info_from_node()) determines
whether the "preferred" (i.e. IPv4 or IPv6) or the "primary"
(i.e. IPv4) address is to be used.

We need #4620 to be fixed in order to make it possible to pick the
preferred OR port based on routerstatus + microdescriptor. This is
important both for clients using microdescriptors and doing ordinary
three-hop circuits as well as for one-hop BEGIN_DIR circuits (for
fetching directory info, like server descriptors).

comment:7 Changed 7 years ago by ln5

When should a client prefer the IPv6 or port of a (non bridge) entry
node?

For bridges, the IPv6 OR port is preferred if the last bridge
descriptor seen (by rewrite_node_address_for_bridge()) has an IPv6
address.

Let's try this: Prefer IPv6 if there is an IPv6 OR port but make sure
we fall back to its IPv4 port if we fail connecting. I wonder how hard
it will be to implement the fall back thing. Lat time I looked (late
2011) it didn't seem too tempting.

comment:8 Changed 7 years ago by nickm

Nick, do you have an opinion?

arma: Why have this be a consensus param? I don't see the utility, especially given the not implausible possibility that the first versions of the client code to use "a" lines will do so buggily, turning such a consensus param into a param which must never be set.

When should a client prefer the IPv6 or port of a (non bridge) entry

node?

My first thought would be either "never" or "only when told to do so" at first. My rationale is that lots of clients will have IPv6 addresses that leak their MAC addresses for a while, and we don't (now) have code to detect that yet.

Let's try this: Prefer IPv6 if there is an IPv6 OR port but make sure we fall back to its IPv4 port if we fail connecting.

I'm not saying that's wrong, but I'd like to hear your rationale.

comment:9 in reply to:  8 Changed 7 years ago by ln5

Cc: arma added

When should a client prefer the IPv6 or port of a (non bridge) entry

node?

My first thought would be either "never" or "only when told to do so" at first. My rationale is that lots of clients will have IPv6 addresses that leak their MAC addresses for a while, and we don't (now) have code to detect that yet.

Let's try this: Prefer IPv6 if there is an IPv6 OR port but make sure we fall back to its IPv4 port if we fail connecting.

I'm not saying that's wrong, but I'd like to hear your rationale.

I'm afraid my rationale is based more on functionality and
connectivity than MAC protection. It boils down to two points:

  • Using IPv6 would (supposedly) be good for connectivity when running on networks without IPv6 blocking.
  • I have become less worried about MAC address leaking lately, mostly because my (highly nonscientific) list of systems with privacy extensions for SLAAC (RFC 4941) enabled by default now includes most of what we care about (Ubuntu, Debian, Fedora, OS X and Windows).

I have to admit that the value of the first point and the risk
mitigation given by the last point are both hard to quantify.

New proposed solution is to revisit #4455 and expand the discussion
beyond bridges, taking MAC privacy into account.

comment:10 Changed 7 years ago by ln5

I've pushed new code to bug5535 in my repo.
I'm currently testing it (in branch task4564).
It's not ready for merging, needs more testing.
Sloppy review and comments are ofc welcome!

comment:11 Changed 7 years ago by ln5

Branch bug5535_2 in my repo contains what's needed for clients to
connect to v6 public relays.

Still no use of microdescriptors but please review what's there.

comment:12 Changed 7 years ago by nickm

Status: acceptedneeds_revision

+ /* Cheap implementation of config option ClientUseIPv6 -- simply
+ don't prefer IPv6 when ClientUseIPv6 is not set. See #4455 for
+ more on this subject. */
+ if (get_options()->ClientUseIPv6 == 1 && node_ipv6_preferred(node))

So, "Client" here doesn't mean client at all; relays do this too?

comment:13 in reply to:  12 Changed 7 years ago by ln5

Replying to nickm:

So, "Client" here doesn't mean client at all; relays do this too?

Correct. Since relays shouldn't connect to IPv6 OR ports right now it
shouldn't matter as far as I can tell.

We might want to move the test for ClientUseIPv6 to where we actually
set up the outgoing connection but for now I think this is OK.

Or is it too misleading?

comment:14 Changed 7 years ago by ln5

Status: needs_revisionaccepted

Pushed a fix and a clarification to bug5535_2.

comment:15 Changed 7 years ago by ln5

Status: acceptedneeds_review

Pleae see bug6363_5535 in my repo. It's based on bug6363_for_review.

These are the related committs:

23a733ab * linus/bug6363_5535 bug6363_5535 Take microdesc into account when deciding about preferred OR port.
2761da10 * Make node_ipv6_preferred() take microdescs into account.
0df25451 * Clear the ipv6_preferred flag like the others.
15aa828e * Take microdesc IPv6 address into account when setting node->ipv6_preferred.
58995980 * Fix a comment.
e1fa7f99 * Use preferred OR for nodes with routerstatus and microdesc too.
3608b72a * Add IPv6 OR port to microdesc_t and populate it.
fa4812c0 * Merge branch 'bug5535_2_rebased' into bug6363_5535

|\

ace9b435 | * bug5535_2_rebased Fix whitespace.
84b3950e | * Clarify the use of ClientUseIPv6 in node_get_pref_orport().
43f2eeec | * Have only non-servers pick an IPv6 address for the first hop.
d1c1a7e8 | * Clients connect to public relays over IPv6.

comment:16 Changed 7 years ago by ln5

rewrite_node_address_for_bridge() sets node->ipv6_preferred but it
should possibly be cleared if things change.

comment:17 Changed 7 years ago by nickm

Other than that this basically looked okay to me when I reviewed it.

comment:18 Changed 7 years ago by nickm

I squashed and reordered the branch "bug6363_5535" to make "bug6363_5535_rebase_p3" in my public repository. I then extracted the bug5535 commits into a branch "bug5535_only". I'm planning to look for other commits that might squash well on that branch, then push another branch, then re-review and merge.

comment:19 Changed 7 years ago by nickm

I cherry-picked two commits so far from 5535 as c76a332887c2f79 and d6ad00a01f703a ; they seem unrelated to the actual work of 5535 and/or like bugfixes on already existing code.

comment:20 Changed 7 years ago by nickm

my branch "bug5535_only_rebased" now has a rebased and squashed version of only the bug5535 commits on that series. Next steps are to review both branches, and confirm I didn't lose any changes.

comment:21 Changed 7 years ago by nickm

I did lose a change, but cherry-picked it back to master as 730dd9a6d0c8

comment:22 Changed 7 years ago by ln5

Looks good to me.

comment:23 Changed 7 years ago by nickm

Resolution: implemented
Status: needs_reviewclosed

Merging it onto master.

comment:24 Changed 7 years ago by nickm

Keywords: tor-client added

comment:25 Changed 7 years ago by nickm

Component: Tor ClientTor
Note: See TracTickets for help on using tickets.