Opened 14 months ago

Last modified 3 months ago

#23818 assigned defect

Make v3 single onion services retry failed connections with a 3-hop path

Reported by: teor Owned by: neel
Priority: Medium Milestone: Tor: unspecified
Component: Core Tor/Tor Version:
Severity: Normal Keywords: prop224, tor-hs, single-onion, ipv6, 034-triage-20180328, 034-removed-20180328
Cc: neel@… Actual Points:
Parent ID: #23493 Points:
Reviewer: Sponsor:

Description (last modified by teor)

This makes a single onion service connect via an entry that it can reach when connections fail.

Child Tickets

Change History (14)

comment:1 Changed 13 months ago by teor

Description: modified (diff)

I removed partial support for this feature in commit b4aa8fc3d9 in my bug23820_032 branch (#23820), because it was buggy. We'll need to revert that commit and build on it for 0.3.3.

comment:2 Changed 10 months ago by dgoulet

Milestone: Tor: 0.3.3.x-finalTor: 0.3.4.x-final

Moving a bunch of tickets from 033 to 034.

comment:3 Changed 8 months ago by nickm

Keywords: 034-triage-20180328 added

comment:4 Changed 8 months ago by nickm

Keywords: 034-removed-20180328 added

Per our triage process, these tickets are pending removal from 0.3.4.

comment:5 Changed 8 months ago by nickm

Milestone: Tor: 0.3.4.x-finalTor: unspecified

These tickets, tagged with 034-removed-*, are no longer in-scope for 0.3.4. We can reconsider any of them, if time permits.

comment:6 Changed 3 months ago by neel

Cc: neel@… added
Owner: set to neel
Status: newassigned

comment:7 Changed 3 months ago by neel

Owner: neel deleted

comment:8 Changed 3 months ago by neel

Status: assignednew

comment:9 Changed 3 months ago by neel

Owner: set to neel
Status: newassigned

comment:10 Changed 3 months ago by neel

I am interested in this patch.

Right now, I am looking at commit b4aa8fc3d9 (that you referred to in Comment 1) and saw that in pick_intro_point(), you replaced this line

info = extend_info_from_node(node, direct_conn);

with this:

info = extend_info_from_node(node, 0);

Should I revert this to the former line (the one with direct_conn)?

comment:11 in reply to:  10 Changed 3 months ago by teor

Replying to neel:

I am interested in this patch.

Right now, I am looking at commit b4aa8fc3d9 (that you referred to in Comment 1) and saw that in pick_intro_point(), you replaced this line

info = extend_info_from_node(node, direct_conn);

with this:

info = extend_info_from_node(node, 0);

Should I revert this to the former line (the one with direct_conn)?

We re-wrote a lot of this code in #23588 and #23576.

I suggest you:

  1. create a branch from the master branch, and merge the latest branches from #23588 and #23576
  2. test that the merged branch passes "make test-network-all", including all the IPv6 tests
  3. test that the merged branch fails the single-onion-v23-ipv6-md network using "chutney/tools/test-network.sh --flavour single-onion-v23-ipv6-md"

Please paste the test results into a ticket, to confirm that you're running the correct tests.

Then you can start writing a patch for this ticket based on those branches:

As you are writing the patch, you might find some of this removed code useful:
https://github.com/torproject/tor/commit/b4aa8fc3d918cc3aea375985c44abd086f91ae7a#diff-b2d25162bf60cfbeb456effc403d3282L1540
https://github.com/torproject/tor/commit/b4aa8fc3d918cc3aea375985c44abd086f91ae7a#diff-b2d25162bf60cfbeb456effc403d3282L1557

But remember, that old code is buggy and incomplete!

  1. write code that matches the v2 single onion service behaviour for intro point selection:

https://github.com/torproject/tor/blob/fe9f58514349c9d25b48ae29c87d8aaf065d0931/src/feature/rend/rendservice.c#L4133

  1. and intro point connection:

https://github.com/torproject/tor/blob/fe9f58514349c9d25b48ae29c87d8aaf065d0931/src/feature/rend/rendservice.c#L3031

  1. write code that matches the v2 single onion service behaviour for rend point connection:

https://github.com/torproject/tor/blob/fe9f58514349c9d25b48ae29c87d8aaf065d0931/src/feature/rend/rendservice.c#L2089

  1. write a comment that explains the single onion service behaviour for rend point retries (which just use a standard 3-hop connection):

https://github.com/torproject/tor/blob/422abd4fa3c2c9c10f4e7f83eced7416785e89c4/src/feature/rend/rendservice.c#L2994

  1. when you have implemented the 3-hop fallback feature, your code will pass on the single-onion-v23-ipv6-md network, and we can make that network part of the standard test suite (#27251).

I suggest that you open child tickets for steps 1-3, 4-5, and 6-8. Otherwise, this ticket will become very large and hard to read,

comment:12 Changed 3 months ago by neel

I merged the two branches for #23588 and #23576 here: https://github.com/neelchauhan/tor/tree/v3_sos

However, make test-network-all tests involving v3 onion services fail:

neel@megora:~/Code/Tor/tor % make test-network-all
mkdir -p ./test_network_log
ping6 ::1 or ping ::1 succeeded, running IPv6 flavors: bridges+ipv6-min ipv6-exit-min hs-v23-ipv6-md  single-onion-ipv6-md.
tor-stable not found, skipping mixed flavors: mixed+hs-v23.
SKIP: mixed+hs-v23
PASS: basic-min
Detail: chutney/tools/warnings.sh /usr/home/neel/Code/Tor/chutney/net/nodes.1535244496
Warning: Rejected vote from 127.0.0.1 ("Duplicate discarded"). Number: 1
PASS: bridges-min
PASS: hs-v2-min
FAIL: hs-v3-min
FAIL: single-onion-v23
Detail: chutney/tools/warnings.sh /usr/home/neel/Code/Tor/chutney/net/nodes.1535244735
Warning: Rejected vote from 127.0.0.1 ("Duplicate discarded"). Number: 2
PASS: bridges+ipv6-min
PASS: ipv6-exit-min
FAIL: hs-v23-ipv6-md
PASS: single-onion-ipv6-md
Log and result files are available in ./test_network_log.
*** Error code 1

Stop.
make: stopped in /usr/home/neel/Code/Tor/tor
neel@megora:~/Code/Tor/tor % 

It seems teor's branch for #23576 fails make test-network-all also.

comment:13 Changed 3 months ago by neel

For Steps 1-3 in Comment 11, after the buggy code gets fixed, should I make the branch based on teor's bug23576-rebased or just bug23576?

comment:14 in reply to:  13 Changed 3 months ago by teor

Replying to neel:

For Steps 1-3 in Comment 11, after the buggy code gets fixed, should I make the branch based on teor's bug23576-rebased or just bug23576?

Check #23576 for details. I'll probably do the fixes on the rebased branch.

Note: See TracTickets for help on using tickets.