[tor-bugs] #8402 [Tor]: Tor should help its transport proxy use a proxy, if needed.

[Tor Bug Tracker & Wiki]

#8402: Tor should help its transport proxy use a proxy, if needed.
------------------------+--------------------------------------
     Reporter:  asn     |      Owner:
         Type:  defect  |     Status:  needs_revision
     Priority:  normal  |  Milestone:  Tor: 0.2.6.x-final
    Component:  Tor     |    Version:
   Resolution:          |   Keywords:  tor-bridge pt flashproxy
Actual Points:          |  Parent ID:
       Points:          |
------------------------+--------------------------------------

Comment (by asn):

 Replying to [comment:16 yawning]:
 > Replying to [comment:14 asn]:
 > > A few comments from a preliminary review. I would like to review it
 once again:
 > >
 > > - As we discussed in IRC, unit tests for the non-trivial additions
 would be great.
 >
 > Will do.
 >
 > > - Maybe we could functionify the new duplicate code in
 `get_proxy_addrport()`. I know that Nick hates duplicate code, and I share
 his sentiments.
 >
 > Ditto.  I would have thought of something clever to do, but I was
 focused on getting an initial revision that worked.
 >
 > > - This is more of a comment to the original proposal, but isn't `PROXY
 true` a bit off in a protocol that doesn't have any other `false`/`true`
 strings? Maybe `PROXY DONE` is more appropriate? Maybe not.
 >
 > I would be ok with this, and agree that it makes sense (I considered
 doing it when I was writing, but decided to implement that portion of the
 spec as is).
 >

 Hm, if you also like the suggestion, let's do it then?

 > > - `acked_proxy` is a bit of a deceiving name. Maybe we should change
 the variable name to imply some connection to the proxy?
 >
 > Hmmm, I was going to change it to proxy_acked to match proxy_uri as far
 as naming goes, if there is something better for "the pt claims it will
 use the specified proxy", then I'm open to suggestions.
 >

 Hm, not sure. How about `supports_proxy`? So that you can do `if
 (mp.supports_proxy) {`?
 Doesn't look terribly bad. Probably can be improved.

 > (Is what I said in comment #11 correct?  The proposal should be changed
 to not require the pluggable transport to verify that the proxy is
 actually usable during the config right?  Apart from trying to connect to
 something, that's not possible with SOCKS4 or 5 in a way that isn't at
 least somewhat suspicious.)

 Yes, I think that's reasonable. The `PROXY true` message is there so that
 Tor is assured that the PT understands that it has to use a proxy for any
 outgoing connections. It doesn't seem necessary to test the proxy for
 correctness at configure time.

--
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/8402#comment:17>
Tor Bug Tracker & Wiki <https://trac.torproject.org/>
The Tor Project: anonymity online