Opened 6 years ago

Closed 6 years ago

#8929 closed task (implemented)

Pass server-side pluggable transport parameters to proxies

Reported by: asn Owned by:
Priority: Medium Milestone: Tor: 0.2.5.x-final
Component: Core Tor/Tor Version:
Severity: Keywords: tor-bridge pt wants-mocking
Cc: Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

On the client-side, we pass pluggable transport parameters to the transport proxies using the SOCKS handshake (and using the username/password as a covert channel).

On the server-side, we don't have such a mechanism in place. We should spec out how this should work out. Maybe use the environment variables like this:
TOR_PT_PARAMETERS="obfs2:shared-secret=asdsada;obfs3:shared-secret=paa\;ssword

Child Tickets

Change History (15)

comment:1 Changed 6 years ago by asn

Assuming that we use environment variables to do the parameter passing on the server-side, it's annoying that we can't use the same encoding as the one we are using for SOCKS parameters on the client-side.

Specifically, on the client-side SOCKS parameters look like:
shared-secret=blabla;ticket-file=/ticket.file

while on the server-side, the names of the pluggable transports must be specified, which makes the encoding look like:
obfs2:shared-secret=blabla;obfs2:ticket-file=/ticket.file;scramblesuit:shared-secret=hoho

On the client-side we have to escape semicolons and backslashes, but on the server side we have to escape colons, semicolons and backslashes. (Actually now that I look at it, we also have to escape the equal signs on both cases.)

Nick, any opinions on the encoding?

(We could also use environment variables like TOR_PT_PARAMETERS_OBFS2=shared-secret=blabla;ticket-file=/ticket.file so that we use the same encoding, but playing with environment variable names seems a bit silly.)

comment:2 Changed 6 years ago by asn

We might also need to add another torrc option (ServerTransportOptions <transport_name> <options...> or something), to be able to pass options to server-side pluggable transports since we won't be able to fit them in the ServerTransportPlugin line (the ServerTransportPlugin line ends with a variable number of command line options to be passed to the transport program).

comment:3 Changed 6 years ago by asn

How about branch bug8929 in https://git.torproject.org/user/asn/torspec.git. Should we proceed with this?

comment:4 Changed 6 years ago by nickm

17:23 < nickm> asn_: it looks good, except that we shouldn't really edit 
               proposals once they're done, and proposal 180 is closed.
17:24 < nickm> asn_: also, when you say that such-and-such should be escaped, 
               you should say how.

comment:5 in reply to:  4 Changed 6 years ago by asn

Replying to nickm:

17:23 < nickm> asn_: it looks good, except that we shouldn't really edit 
               proposals once they're done, and proposal 180 is closed.
17:24 < nickm> asn_: also, when you say that such-and-such should be escaped, 
               you should say how.

OK. Check out branch bug8929_take2. This one changes only pt-spec.txt and it also specifies that escapes must happen with a backslash.

comment:6 Changed 6 years ago by nickm

Looks fine. Let's merge that when we merge the corresponding feature.

comment:7 Changed 6 years ago by asn

Status: newneeds_review

Please check branch bug8929 in https://git.torproject.org/user/asn/tor.git.

comment:8 Changed 6 years ago by asn

Also, please re-fetch the bug8929_take2 torspec branch. I forgot to specify that if no transports need to be passed to transports, the environment variable can be omitted from the environment.

comment:9 Changed 6 years ago by nickm

Milestone: Tor: 0.2.5.x-final

comment:10 Changed 6 years ago by nickm

Status: needs_reviewneeds_revision

Here are my notes; I'm happy to make these changes myself before merging. I plan to do them myself once #8949 is merged. I'm just copying them here so I don't lose them.

  • In tor_escape_str_for_pt_args: document what "escape" means.
  • In get_options_from_transport_options_line():
    • add_strdup, not add_asprintf("%s")
    • escape option before logging.
    • Use option_sl_idx, not i.
  • get_transport_options_for_server_proxy: Document that it can return NULL.
  • fix memory leaks in tests
  • Make sure all new functions have tests.

comment:11 Changed 6 years ago by nickm

Keywords: wants-mocking added

comment:12 Changed 6 years ago by nickm

I've started on a branch in "bug8929_rebase" based on your branch but rebased to master so I can use the #8949 stuff to write tests. My 16a907564d2fd344bc69230af88c6fe50f8e3a06 corresponds to your 9d400abdf65173a3a4c167c0f45ff5504f445fd9.

It's not done yet; I'll post when it is.

comment:13 Changed 6 years ago by nickm

Status: needs_revisionneeds_review

Okay, please have a look at branch "bug8929_rebase" in my public repository.

comment:14 Changed 6 years ago by asn

I agree with your changes!

comment:15 Changed 6 years ago by nickm

Resolution: implemented
Status: needs_reviewclosed

Squashed as "bug8929_rebase_2" and merged. Thanks!

Note: See TracTickets for help on using tickets.