[Author Prev][Author Next][Thread Prev][Thread Next][Author Index][Thread Index]

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

#12585: Implement new option SocksSocket
-----------------------------+--------------------------------
     Reporter:  ioerror      |      Owner:
         Type:  enhancement  |     Status:  needs_revision
     Priority:  normal       |  Milestone:  Tor: 0.2.6.x-final
    Component:  Tor          |    Version:  Tor: unspecified
   Resolution:               |   Keywords:  026-triaged-1
Actual Points:               |  Parent ID:
       Points:               |
-----------------------------+--------------------------------
Changes (by nickm):

 * status:  needs_review => needs_revision


Comment:

 Throughout:

   * Looks nice!  Much simpler now.

   - This is probably gonna break on windows: I don't think they have
 AF_UNIX, and at least address.c uses AF_UNIX unconditionally. I can clean
 it up if you want, or you can if you've got mingw cross-compilation stuff
 installed.

   - Is "SocksSocket" really the right name for this?  I'm used to  it, but
 I bet it would confuse users some.

 address.c

   - Probably should document how to work with unix domain sockets;
     they don't actually fit in a tor_addr_t.

 config.c / tor.1.txt

   - Can we support SocksSocket on-by-default?  If so we'd need a way
     to turn it off.  "SocksSocket 0"?

 connection.c:

   - I wonder if we should try fchmod first, and then chmod.  fchmod
     is a bit safer, right?  (Not a new issue.)

   - If we chmod 0660 in the event of a group-writable socket, should
     we shmod 0600 in the event of a non-group-writable socket?
     (Not a new issue.)

   - The foosocketsgroupwriteable code in connection_listener_new
     seems to be nearly duplicated.

   - This comment is wrong:
 {{{
     /* For now only control ports can be Unix domain sockets
       * and listeners at the same time */
 }}}
 connection_edge.c:

   - What's the point of the modifications of
     connection_ap_get_begincell_flags and
     connection_ap_handshake_rewrite_and_attach ?  I think they might
     be wrong.   The flags that they adjust are about whether the
     destination address is IPv4/IPv6/etc, not whether the socksport
     address is IPv4 or IPv6.

 relay.c:

   - Same comment as comment above, wrt the change in
     connection_edge_process_relay_cell_not_open().


 Otherwise, looks okay.  Have we tested the latest version of this? :)

--
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/12585#comment:43>
Tor Bug Tracker & Wiki <https://trac.torproject.org/>
The Tor Project: anonymity online
_______________________________________________
tor-bugs mailing list
tor-bugs@xxxxxxxxxxxxxxxxxxxx
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs