Opened 2 years ago

Last modified 5 months ago

#21816 needs_revision enhancement

Add support for Pluggable Transports 2.0

Reported by: chelseakomlo Owned by: dasyatid1
Priority: Medium Milestone: Tor: unspecified
Component: Core Tor/Tor Version: Tor: unspecified
Severity: Normal Keywords: tor-client, tor-pt, tor-bridge, design, pt2, review-group-23, review-group-24, 034-triage-20180328, 034-removed-20180328
Cc: iang, darkk, blanu, dcf, dasyatid1, mcs, ahf, Samdney Actual Points:
Parent ID: Points:
Reviewer: nickm Sponsor: SponsorM-can

Description

In the Pluggable Transport 2.0 specification, the IPC specification changes the way per-connection params are sent from Tor to the managed proxy. In order to support this new change, we need to change which Socks Auth method is used to convey parameters (this allows backwards compatibility).

The current issue is that per connection parameters are overriding the authentication parameters, and we want to allow for longer auth params.

Ian took a look at the code, and identified some places where we need changes:

  • connection.c, lines 2225
  • connection.c, case statement at 2402

The IPC framework will need to declare which version it is, and Tor will need to parse this information.

We will also need to upgrade the managed proxy which is bundled with Tor in order to support the PT 2.0 IPC method. We will open a separate ticket for this.

Child Tickets

Change History (23)

comment:1 Changed 2 years ago by chelseakomlo

Summary: Add support for Pluggable Transport 2.0Add support for Pluggable Transports 2.0

comment:2 Changed 2 years ago by chelseakomlo

Cc: darkk blanu added

comment:3 Changed 2 years ago by iang

Sitting here in the session with darkk. We think this is what needs changing:

  • transports.c already negotiates the PT protocol version number, and sticks it in a managed_proxy_t (field conf_protocol); however, transports.c:998 hardcodes that only version 1 is currently accepted. Also transports.c:1324 is where tor tells the transport what versions tor supports, so that will need to be changed to support both 1 and 2.
  • In connection.c, around line 2225, tor calls get_socks_args_by_bridge_addrport to get the socks args, but that doesn't give us a handle to the managed_proxy_t
  • So we should call find_transport_name_by_bridge_addrport to get the transport name
  • Then we'll need a new function in transports.c that's very much like get_managed_proxy_by_argv_and_type, except it's get_managed_proxy_by_transport_name (it searches the managed_proxy_list, and for each entry, searches the transports list looking for a match, presumably using smartlist_contains_string(mp->transports, transport_name)). It also needs to be non-static.

Then we'd be able to access the conf_protocol field in connection.c, and do the small changes to use the alternate SOCKS 5 authentication method specified in the PT 2.0 spec.

Last edited 2 years ago by iang (previous) (diff)

comment:4 Changed 2 years ago by nickm_mobile

And in parallel: We all review the thing on tor-dev and make sure that it's what we want to implement :)

comment:5 Changed 2 years ago by dcf

Cc: dcf added

comment:6 Changed 21 months ago by nickm

Keywords: tor-client tor-pt tor-bridge design pt2 added

comment:7 Changed 19 months ago by dasyatid1

Cc: dasyatid1 added
Owner: set to dasyatid1
Status: newassigned

As I posted on tor-dev a little while ago, I've been working on a patchset for this as part of work I've been doing with blanu / Operator. This is available at Bitbucket (Git, Web).

With commit 887352263569, I've completed successful circuits through the "rtt2017" branch of shapeshifter-dispatcher and associated shapeshifter-ipc using PT2 configuration protocol (there are changes there I still want to upstream), and through obfs4proxy using PT1, each acting as an obfs4 client for connecting via a bridge.

I'd like to hear what would be necessary to merge a version of this. Currently on my radar: unit tests for JSON/RFC1929 encoding functions, 'changes' documentation, possibly commit-level cleanup, possibly pushing on PT spec and/or Shapeshifter upstream to lock them down. I'm probably missing something, though. There are some other details in my original post to tor-dev.

comment:8 Changed 19 months ago by nickm

Milestone: Tor: unspecifiedTor: 0.3.2.x-final
Status: assignedneeds_review

comment:9 Changed 19 months ago by nickm

Keywords: review-group-23 added

Put 0.3.2 needs_review and merge_ready tickets into review-group-23.

comment:10 Changed 18 months ago by nickm

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

comment:11 Changed 17 months ago by mcs

Cc: mcs added

comment:12 Changed 17 months ago by dcf

Here's a link to view the diff resulting from dasyatid's patch:

https://bitbucket.org/DasyatidPrime/tor-rtt2017-21816/branches/compare/rtt2017%0Dmaster#diff

One of the changes in the PT 2.0 spec draft 3 is that error responses such as ENV-ERROR, VERSION-ERROR, etc. are now written to stderr, not stdout. But I don't see anything the patch series that accounts for that?

comment:13 Changed 17 months ago by nickm

Keywords: review-group-24 added

review-group-24 is now open.

comment:14 Changed 17 months ago by nickm

Sponsor: SponsorM-can

comment:15 Changed 17 months ago by nickm

Reviewer: nickm

comment:16 Changed 17 months ago by nickm

Starting review. I've opened a code review ticket at https://oniongit.eu/nickm/tor/merge_requests/8 so I can comment on the commits. Please let me know if you have any problems reading them :)

comment:17 Changed 17 months ago by nickm

Status: needs_reviewneeds_revision

Okay, I've left comments on the ticket. It looks like a good start!

comment:18 Changed 14 months ago by nickm

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

ping -- is anyone still working for this?

comment:19 Changed 12 months ago by nickm

Keywords: 034-triage-20180328 added

comment:20 Changed 12 months ago by nickm

Keywords: 034-removed-20180328 added

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

comment:21 Changed 12 months ago by ahf

Cc: ahf added

comment:22 Changed 11 months ago by nickm

Milestone: Tor: 0.3.4.x-finalTor: unspecified

These needs_revision, tickets, tagged with 034-removed-*, are no longer in-scope for 0.3.4. We can reconsider any of them, if somebody does the necessary revision.

comment:23 Changed 5 months ago by Samdney

Cc: Samdney added
Note: See TracTickets for help on using tickets.