Opened 6 years ago

Closed 5 years ago

#11674 closed defect (fixed)

Document TOR_PT_PROXY in 'pt-spec.txt'.

Reported by: asn Owned by:
Priority: Medium Milestone: Tor: 0.2.6.x-final
Component: Core Tor/Tor Version:
Severity: Keywords: tor-spec tor-pt
Cc: yawning Actual Points:
Parent ID: #8402 Points:
Reviewer: Sponsor:

Description

#8403 was great, but since we decided to implement the PT-outgoing-proxy feature we should also mention how it works in pt-spec.txt (since that's where people who want to develop PTs look at)

Child Tickets

Change History (10)

comment:1 Changed 5 years ago by asn

Status: newneeds_review

Please see bug11674 at https://git.torproject.org/user/asn/torspec.git :
https://gitweb.torproject.org/user/asn/torspec.git/commitdiff/daf64f31a5dafbf452d85e8e7b58efad388dc1b7

comment:2 Changed 5 years ago by dcf

"If the pluggable transport proxy detects that the TOR_PT_PROXY environment variable is set, it attempts connecting to it." I think anyone who has tried implementing this proposal has ignored the "attempts connecting to it" part. See for example comment:4:ticket:12125.

Trying to connect first doesn't make sense in many situations. For instance, tor may start up multiple transport plugins but only use one of them--they all shouldn't be doing a test probe to the same proxy. Not all transports use a single long-lived connection. meek-client is passing its proxy configuration on to the browser extension, and it is the browser extension that actually connects to the proxy--there's no way to "connect" to the proxy at startup.

I propose wording like

If the value of the TOR_PT_PROXY environment variable is a proxy URL
usable by the pluggable transport plugin, the plugin writes to stdout:
  PROXY DONE
Otherwise, it writes
  PROXY-ERROR <errormessage>

comment:3 Changed 5 years ago by asn

And there is also the suggestion at comment:29:ticket:8402.

comment:4 Changed 5 years ago by nickm

asn -- if you think that's accurate, then could somebody turn it into a diff I can apply?

comment:5 Changed 5 years ago by nickm

Milestone: Tor: 0.2.6.x-final

comment:6 Changed 5 years ago by asn

I suggest we fix this as part of #12434.

Please review my latest commit in #12434, it adds a paragraph about TOR_PT_PROXY to the new tor-pt-spec.txt. You can see it here:
https://gitweb.torproject.org/user/asn/torspec.git/commitdiff/063504d6f1ccaf24995b37afefa11c8b0193425f

Let's close this ticket when #12434 gets merged.

comment:7 Changed 5 years ago by yawning

If the client PT finds out that the TOR_PT_PROXY environment
variable is set, the PT is expected to use that as an outgoing
proxy.

I would prefer something like:

If the client PT is provided with a TOR_PT_PROXY environment
variable, it MUST make all outgoing network connections via the
supplied proxy.  If it is unable to do so for any reason (eg:
malformed value, unsupported proxy type, unreachable proxy
address), it MUST return a `PROXY_ERROR` and terminate.

Adjust language to taste. The hard-failure requirement should explicitly be listed in the pt-spec instead of being buried in a proposal (as a MUST).

comment:8 Changed 5 years ago by asn

Thanks for the feedback. Fixed and pushed your wording, please check again.

comment:9 Changed 5 years ago by nickm

Is this ok now? Can we close it?

comment:10 Changed 5 years ago by nickm

Resolution: fixed
Status: needs_reviewclosed

Being bold and guessing "yes".

Note: See TracTickets for help on using tickets.