[tor-bugs] #12535 [Pluggable transport]: goptlib should expose a SOCKS5 server instead of SOCKS4a.

[Tor Bug Tracker & Wiki]

#12535: goptlib should expose a SOCKS5 server instead of SOCKS4a.
-------------------------------------+----------------------------
     Reporter:  yawning              |      Owner:  yawning
         Type:  defect               |     Status:  needs_revision
     Priority:  normal               |  Milestone:
    Component:  Pluggable transport  |    Version:
   Resolution:                       |   Keywords:  goptlib, socks
Actual Points:                       |  Parent ID:  #12130
       Points:                       |
-------------------------------------+----------------------------

Comment (by dcf):

 Replying to [comment:8 yawning]:
 > Replying to [comment:7 dcf]:
 > > {{{
 > >  // Send a message to the proxy client that access to the given
 address is
 > > -// granted. If the IP field inside addr is not an IPv4 address, the
 IP portion
 > > -// of the response will be four zero bytes.
 > > +// granted.  For interface backwards compatibility reasons, this does
 not set
 > > +// BND.ADDR/BND.PORT correctly, however very few if any clients
 examine the
 > > +// values of this field.
 > >  func (conn *SocksConn) Grant(addr *net.TCPAddr) error {
 > > -   return sendSocks4aResponseGranted(conn, addr)
 > > +   // Addr in the SOCKS 4 code was the destination address, which is
 not sent
 > > +   // in SOCKS 5.
 > > +   return sendSocks5ResponseGranted(conn, nil)
 > >  }
 > > }}}
 > >
 > > I don't understand why addr is ignored? The comments don't make sense
 to me; isn't BND.ADDR/BND.PORT the destination address?
 >
 > Despite what certain things say (eg: Wikipedia), it's the local
 address/port of the outgoing socket from the SOCKS server to the
 destination.
 >
 > {{{
 >    In the reply to a CONNECT, BND.PORT contains the port number that the
 >    server assigned to connect to the target host, while BND.ADDR
 >    contains the associated IP address.  The supplied BND.ADDR is often
 >    different from the IP address that the client uses to reach the SOCKS
 >    server, since such servers are often multi-homed.
 > }}}
 >
 > It's presumably done this way because clients can't call `getsockname()`
 on connections through a proxy, but in practice not many SOCKS clients
 require or use this information.  We could go and modify the application
 code that is affected by the switch to send back the right address as
 well.

 Oh fascinating. That does seem pretty useless for our purposes. I think
 you made the right call with your design. In the function comment, state
 explicitly that addr is ignored. I think I would remove the special
 handling for addr == nil in sendSocks5Response and push emptyAddr into the
 callers, just for the sake of explicitness. And in fact, it looks like you
 could just hard-code an IPv4 emptyAddr in sendSocks5Response and remove
 the addr parameter, as there doesn't seem to be any external way to cause
 a non-nil addr to be set. You can make the call.

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