Make service_intro_point_new() take a node instead of an extend_info

changed milestone to %Tor: 0.4.1.x-final

added 034-removed-20180328 034-triage-20180328 040-unreached-20190109 041-proposed actualpoints::2 component::core tor/tor fast-fix ipv6 milestone::Tor: 0.4.1.x-final owner::teor parent::23588 points::1 priority::medium prop224 resolution::fixed reviewer::dgoulet severity::normal single-onion status::closed tor-hs type::defect version::tor 0.3.2.1-alpha labels

These tickets aren't urgent, and they involve major refactoring. Deferring to 0.3.3

Trac:
Milestone: Tor: 0.3.2.x-final to Tor: 0.3.3.x-final

In my commit b4aa8fc3d9 in my branch bug23820_032 (#23820 (moved)), I ripped out IPv6 support in these functions. We'll need to revert that commit, and then fix the issues in the function.

    Remove buggy IPv6 support from pick_intro_point() and service_intro_point_new()
    
    The previous version of these functions had the following issues:
    * they can't supply both the IPv4 and IPv6 addresses in link specifiers,
    * they try to fall back to a 3-hop path when the address for a direct
      connection is unreachable, but this isn't supported by
      launch_rendezvous_point_circuit(), so it fails.
    But we can't fix these things in a bugfix release.
    
    Instead, always put IPv4 addresses in link specifiers.
    And if a v3 single onion service can't reach any intro points, fail.

Trac:
Owner: N/A to dgoulet
Status: new to assigned

Move 033 ticket I own to 034

Trac:
Milestone: Tor: 0.3.3.x-final to Tor: 0.3.4.x-final

Trac:
Keywords: N/A deleted, 034-triage-20180328 added

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

Trac:
Keywords: N/A deleted, 034-removed-20180328 added

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

Trac:
Milestone: Tor: 0.3.4.x-final to Tor: unspecified

We can implement this ticket easily by replacing the link specifier part of service_intro_point_new() with get_lspecs_from_node().

This refactor completes part of #23759 (moved).

Trac:
Milestone: Tor: unspecified to Tor: 0.3.5.x-final
Version: N/A to Tor: 0.3.2.1-alpha
Owner: dgoulet to teor

See my branch bug23576 on https://github.com/teor2345/tor.git

This refactor also puts intro IPv6 link specifiers in onion service descriptors (#26992 (moved)).

It passes make check and make test-network-all on my machine, but will probably fail Appveyor CI due to #26986 (moved).

Trac:
Status: assigned to needs_review

Trac:
Keywords: N/A deleted, fast-fix added

Pull request is: https://github.com/torproject/tor/pull/254

Trac:
Reviewer: N/A to asn

This looks good and I like the code simplification!

I left a few nitpicks on the PR that you might want to handle.

Also, should we try to rebase this so that we can see the appveyor green checkmark (now that #26986 (moved) got fixed)?

Trac:
Status: needs_review to needs_revision

Replying to asn:

This looks good and I like the code simplification!

I left a few nitpicks on the PR that you might want to handle.

See the PR for my changes.

Also, should we try to rebase this so that we can see the appveyor green checkmark (now that #26986 (moved) got fixed)?

See my branch bug23576-rebased:

I'd like to defer the merge to early in 0.3.6, for the same reasons as: https://trac.torproject.org/projects/tor/ticket/23588#comment:52

We're still missing #23818 (moved), and when we test in #27251 (moved), we might find we're missing other things as well. I'd like to have a full release to implement and test them.

Trac:
Milestone: Tor: 0.3.5.x-final to Tor: 0.3.6.x-final
Status: needs_revision to needs_review

LGTM for 036!

Trac:
Status: needs_review to merge_ready

I think this branch fails make test-network-all:

neel@megora:~/Code/Tor/back/teor2345-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.1535248896
Warning: Rejected vote from 127.0.0.1 ("Duplicate discarded"). Number: 1
PASS: bridges-min
PASS: hs-v2-min
FAIL: hs-v3-min
Detail: chutney/tools/warnings.sh /usr/home/neel/Code/Tor/chutney/net/nodes.1535249030
Warning: Rejected vote from 127.0.0.1 ("Duplicate discarded"). Number: 1
FAIL: single-onion-v23
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/back/teor2345-tor
neel@megora:~/Code/Tor/back/teor2345-tor

Is this branch failing because of missing code (that should get implemented), bugs in the code, or just my computer?

Trac:
Cc: N/A to neel@neelc.org

It's possible that this code is buggy. Since it's supposed to be a refactor, there shouldn't be any missing code.

But we've also fixed some issues in tor and chutney over the last few days.

Please try again with:

the latest version of chutney master, and
bug23576-rebased merged into tor master

If it still fails, I'll take a look at it some time next week.

Trac:
Status: merge_ready to needs_revision

Using this new merged branch also fails on both FreeBSD 12-ALPHA3 and Fedora 28 (the latter on LiveUSB).

On macOS 10.13 with tor bug23576-rebased and chutney 0ccad53 (or later) I see:

$ make test-network-all
 cd . && /bin/sh ./config.status Makefile depfiles
config.status: creating Makefile
config.status: executing depfiles commands
mkdir -p ./test_network_log
ping6 ::1 and ping ::1 failed, skipping 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: bridges+ipv6-min
SKIP: ipv6-exit-min
SKIP: hs-v23-ipv6-md
SKIP: single-onion-ipv6-md
SKIP: mixed+hs-v23
PASS: basic-min
PASS: bridges-min
PASS: hs-v2-min
FAIL: hs-v3-min
FAIL: single-onion-v23
Log and result files are available in ./test_network_log.
make: *** [test-network-all] Error 1
Exit 2

When I merge bug23576-rebased into master to pick up the latest fixes, I see similar output. I'll have a look into it later this week.

Tor 0.3.6.x has been renamed to 0.4.0.x.

Trac:
Milestone: Tor: 0.3.6.x-final to Tor: 0.4.0.x-final

These IPv6 tickets won't make it into 0.4.0, let's think about doing them in 0.4.1. (They're not on the roadmap, so we'll need to decide if diagnosing the underlying issues is worth the time.)

Trac:
Keywords: N/A deleted, 041-proposed, 040-unreached-20190109 added
Milestone: Tor: 0.4.0.x-final to Tor: unspecified

I rebased this branch on master, and dgoulet helped me find some bugs in it: https://github.com/torproject/tor/pull/673

I still need to tidy it up, add changes files, and find tickets for the new changes. But it seems to work well enough to run it on CI.

Trac:
Actualpoints: N/A to 2

The CI has two spurious errors:

circuitpadding/circuitpadding_circuitsetup_machine: [forking] 
  FAIL src/test/test_circuitpadding.c:1880: assert(n_client_cells OP_EQ 2): 3 vs 2
  [circuitpadding_circuitsetup_machine FAILED]

https://travis-ci.org/torproject/tor/jobs/486721893#L5750

error: failed to write /home/travis/build/torproject/tor/tor-0.4.0.1-alpha-dev/src/rust/Cargo.lock

Caused by:
  failed to open: /home/travis/build/torproject/tor/tor-0.4.0.1-alpha-dev/src/rust/Cargo.lock

Caused by:
  Permission denied (os error 13)

https://travis-ci.org/torproject/tor/jobs/486721901#L3634

Replying to teor:

The CI has two spurious errors: {{{ circuitpadding/circuitpadding_circuitsetup_machine: [forking] FAIL src/test/test_circuitpadding.c:1880: assert(n_client_cells OP_EQ 2): 3 vs 2 [circuitpadding_circuitsetup_machine FAILED] }}} https://travis-ci.org/torproject/tor/jobs/486721893#L5750

Reopened #29122 (moved).

{{{ error: failed to write /home/travis/build/torproject/tor/tor-0.4.0.1-alpha-dev/src/rust/Cargo.lock

Caused by: failed to open: /home/travis/build/torproject/tor/tor-0.4.0.1-alpha-dev/src/rust/Cargo.lock

Caused by: Permission denied (os error 13) }}} https://travis-ci.org/torproject/tor/jobs/486721901#L3634

Opened #29244 (moved).

Ok, the branch is ready with changes files and a cleaned-up series of commits: https://github.com/torproject/tor/pull/673

The branch fixes all the child tickets of this ticket. See the commit messages for details.

asn, if you don't want to review this branch, dgoulet reviewed the first two commits yesterday at the hackfest. There are 2 new commits that dgoulet has not seen: they remove hs_desc_link_specifier_t, and fix the unit tests.

Trac:
Status: needs_revision to needs_review