[tor-bugs] #12585 [Tor]: Implement new option SocksSocket

[Tor Bug Tracker & Wiki]

#12585: Implement new option SocksSocket
-----------------------------+-----------------
     Reporter:  ioerror      |      Owner:
         Type:  enhancement  |     Status:  new
     Priority:  normal       |  Milestone:
    Component:  Tor          |    Version:
   Resolution:               |   Keywords:
Actual Points:               |  Parent ID:
       Points:               |
-----------------------------+-----------------

Comment (by andrea):

 Initial code review of ioerror's SOCKS-over-AF_UNIX patch:

  - In tor_addr_from_sockaddr(), you're calling a helper function
    tor_addr_make_af_unix(), and then returning -1.  The existing
 convention
    for that function (which is not documented in its comment, but should
 be),
    is to return -1 in the error case of an unknown address family, and 0
    in the success case.  The line after tor_addr_make_af_unix(a); should
    be return 0, I think.

  - The tor_addr_make_af_unix() helper itself looks okay to me, but this
    function is only called in one place, from within address.c.  It should
    be declared static at the top of address.c rather than in the global
    namespace in address.h.

  - The change to tor_addr_to_str() looks okay to me.

  - All the config.c changes look okay to me.

  - Changes to entry_connection_new() and connection_free_() look okay.

  - As far as I can tell, the new check_location_for_socks_unix_socket()
    function differs from the existing check_location_for_unix_socket()
    only in the text of the warning message emitted.  These should be
    refactored to avoid duplicated logic.

  - In connection_listener_new():
     - Perhaps is_tcp should be renamed, since obviously an AF_UNIX socket
 is
       not TCP.  Is the relevant property actually SOCK_STREAM (vs.
 SOCK_DGRAM
       in the CONN_TYPE_AP_DNS_LISTENER case)?

     - Why are you adding log_warn(LD_NET,"accept() fails after this...")
       when no actual error condition appears to be occurring in the
 AF_INET/
       AF_INET6 path?

     - The logic in the "if (listensockaddr->sa_family == AF_UNIX &&
       type != CONN_TYPE_CONTROL_LISTENER)" branch is very similar to the
       existing logic for the CONN_TYPE_CONTROL_LISTENER case.  They should
       be combined to avoid duplicated code.

  - Changes check_sock_addr() and connection_handle_listener_read() look
 okay.

  - All changes to main.c, or.h and relay.c look okay to me.

  - Changes to connection_edge.c look okay.

 I will now proceed to attempt to apply the patch against current master
 and test.

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