Opened 6 years ago

Closed 4 years ago

#11150 closed defect (implemented)

Remove client code for connecting to and using 0.2.2 servers

Reported by: nickm Owned by: TvdW
Priority: Medium Milestone: Tor: 0.2.8.x-final
Component: Core Tor/Tor Version:
Severity: Normal Keywords: tor-client, 026-triaged-1, 027-triaged-1-out, 028-triaged
Cc: TvdW Actual Points:
Parent ID: #9476 Points: small
Reviewer: Sponsor:

Description

We can finally drop support for the client-side V2 link handshake!

Child Tickets

Change History (22)

comment:1 Changed 5 years ago by nickm

Milestone: Tor: 0.2.5.x-finalTor: 0.2.6.x-final

comment:2 Changed 5 years ago by nickm

Keywords: 026-triaged-1 added

comment:3 Changed 5 years ago by rl1987

Owner: set to rl1987
Status: newaccepted

comment:4 Changed 5 years ago by TvdW

Cc: TvdW added
Owner: changed from rl1987 to TvdW
Status: acceptedassigned

Initial work on this has been done :

https://github.com/TvdW/tor/commit/75b5d94eb976ee4998189dc69582c62511dde9eb (stops supporting v1 and drops support for parsing pre-0.2.3.17 handshakes)
https://github.com/TvdW/tor/commit/c5edd3f4deb0bfbf8d6e61b4723ddb563402431b (stops hacking around openssl's client certificates)

I also revised the spec a little :

https://github.com/TvdW/torspec/commit/88457e2b824f9088c7a92902fcfa4effc90ed620

Version 0, edited 5 years ago by TvdW (next)

comment:5 Changed 5 years ago by nickm

I'm fine with killing pre-0.2.3.19-alpha clients, modulo the testing issues.

How's the testing coming?

comment:6 Changed 5 years ago by TvdW

I have a working branch at https://github.com/TvdW/tor/commits/remove-v1v2 and am testing it now (looks good so far). Next step is to see how older clients behave with these patches in place.

comment:7 Changed 5 years ago by nickm

Is the removal of always_accept_verify_cb correct?

comment:8 Changed 5 years ago by TvdW

Should be. V1 support required sometimes sending certificates, sometimes not. This had to be overridden if we saw V2, to make sure we don't send a chain. Instead of doing all this, we now just do :

SSL_CTX_set_verify(result->ctx, SSL_VERIFY_NONE, NULL);

Which has the same result.

comment:9 Changed 5 years ago by TvdW

We noticed that 0.2.2 clients (maint-0.2.2 branch) could retry failed handshakes after each consensus download, which could potentially turn into each 0.2.2 client attempting to connect to each 0.2.6 server in the network (assuming the network only has 0.2.2 clients and 0.2.6 servers). Maybe bad.

Last edited 5 years ago by TvdW (previous) (diff)

comment:10 Changed 5 years ago by nickm

Milestone: Tor: 0.2.6.x-finalTor: 0.2.7.x-final

Being a coward, pushing to 0.2.7. We need to figure out the issue with the handful of remaining zombie 0.2.2 clients turning from slow zombies into fast zombies when the network stops supporting them.

comment:11 Changed 5 years ago by nickm

Status: assignedneeds_review

comment:12 Changed 5 years ago by TvdW

The patch is currently broken, no need to review it just yet. I'm planning to fix it asap.

comment:13 Changed 5 years ago by TvdW

I've got an updated branch on https://github.com/TvdW/tor/tree/remove-v1v2-squash that should fix the merge conflicts.

comment:14 Changed 4 years ago by nickm

Keywords: 027-triaged-1-out added

Marking triaged-out items from first round of 0.2.7 triage.

comment:15 Changed 4 years ago by nickm

Milestone: Tor: 0.2.7.x-finalTor: 0.2.8.x-final

comment:16 Changed 4 years ago by nickm

Keywords: 028-triaged added

comment:17 Changed 4 years ago by nickm

Points: small

comment:18 Changed 4 years ago by nickm

I've made a client-only version of this that removes client code to connect to Tor < 0.2.3.6-alpha, as branch ticket11150_client_only. Moving mention of TvdW's work above to another ticket which covers the server side too.

comment:19 Changed 4 years ago by TvdW

Patch looks good to me, but please double-check by connecting v1 and v2 clients to a relay running this code. Also, thanks for crediting!

comment:20 Changed 4 years ago by nickm

Severity: Normal

Testing:

  • 0.1.2 needs a network with v2 authorities, but the testing network didn't have any. More testing would be needed.
  • 0.2.0 connects but doesn't build circuits to the testing network, after some hacking. Must double-check whether behavior is same with un-patched master.
  • 0.2.1 bootstraps nice and happily!
  • 0.2.2 bootstraps nice and happily!

(In all cases, I had to hack these versions up a little to make them actually willing to look at the consensus from a test network and use it.)

comment:21 in reply to:  20 Changed 4 years ago by nickm

Replying to nickm:

Testing:

  • 0.2.0 connects but doesn't build circuits to the testing network, after some hacking. Must double-check whether behavior is same with un-patched master.

Double-checked. 0.2.0 has the same problem with master on a testing network that it does with the patch on a testing network.

(0.2.0 also doesn't bootstrap on the regular network.)

comment:22 Changed 4 years ago by nickm

Resolution: implemented
Status: needs_reviewclosed

Okay; looks good to tvdw and passes tests as above. Merged & closing!

Note: See TracTickets for help on using tickets.