Write server-side pluggable transport options to extra-info descriptor

To correctly deploy pluggable transports that require pre-shared-secrets, like scramblesuit, we need to be able to pass the shared-secret of each bridge to BridgeDB.

This is not currently implemented, and we need to implement the ARGS option of the SMETHOD line as detailed in 180-pluggable-transport.txt.

This is a ticket for the Tor side of this task. That is, parsing the SMETHOD line and writing stuff to the extra-info descriptors.

changed milestone to %Tor: 0.2.5.x-final

added component::core tor/tor milestone::Tor: 0.2.5.x-final priority::medium pt resolution::implemented status::closed tor-bridge type::task wants-mocking labels

Trac:
Cc: N/A to phw

Note to self: We should mention on the spec how the ARGS options and the extra-info line are escaped.

For the torspec side of this: Check branch bug8978 in https://git.torproject.org/user/asn/torspec.git.

It's mostly copy-paste from proposal 180, with some added information on how escaping should be done.

Please see bug8978 in https://git.torproject.org/user/asn/tor.git.

As you can see from the branch, I'm just picking up the ARGS provided by the pluggable transport and placing them directly to our extra-info descriptor. I'm not doing any validation that they are actually key=value strings for two reasons: a) Our threat model usually trusts the transport proxy. b) I didn't want to write an unescape function in C. We will have to write such a function anyway for BridgeDB in Python.

Trac:
Status: new to needs_review

This part looks okay, but it totally needs unit tests for the new format. (And the existing unit tests for parse_smethod_line() should really be checking the output for correctness too.)

(As a first approximation, each new or changed line of code in 0.2.5 should have test coverage. I'm not going to start actually trying to reach "100% of new lines" till after #8949 (moved) is merged, but we should try to close in on it, and not leave code untested when we could just as easily write the tests.)

Trac:
Status: needs_review to needs_revision

Trac:
Keywords: tor-bridge pt deleted, tor-bridge pt wants-mocking added

(And the existing unit tests for parse_smethod_line() should really be checking the output for correctness too.)

This is now #9265 (moved).

I'm rebasing onto master so I can do unit test with the new mocking/coverage stuff, in a new "bug8978_rebase" branch. My b472d964c6d8c9753393421f2fc61a3d7955e7a2 is your 642ea61bda70f2dafbd89e27aa91a5d452888358.

I'll comment again here once I've done the tests. (I'm doing tests myself on the theory that if I'm going to insist soon on test coverage, I should demonstrate that it's possible.)

Please look at the new tests in "bug8978_rebase". This requires some of the exposed stuff from #8929 (moved) to work, so it won't compile as-is, and we should merge it after merging that branch.

Trac:
Status: needs_revision to needs_review

Added tests look good.

However, managed_proxy_create() was still marked as static (even though it was used in test_pt.c) and that caused warnings during compilation. Check my branch nickm-bug8978_rebase in https://git.torproject.org/user/asn/tor.git for a fix.

Squashed as "bug8978_rebase_2" and merged. Thanks!

Trac:
Resolution: N/A to implemented
Status: needs_review to closed

Trac:
Cc: phw to phw, isis@torproject.org

closed

mentioned in issue #9013 (moved)

moved to tpo/core/tor#8978 (closed)