[tor-bugs] #15435 [Tor]: Tor should not close stdin on child processes.

[Tor Bug Tracker & Wiki]

#15435: Tor should not close stdin on child processes.
---------------------------+-----------------------------------------------
     Reporter:  yawning    |      Owner:
         Type:             |     Status:  needs_review
  enhancement              |  Milestone:  Tor: 0.2.7.x-final
     Priority:  normal     |    Version:  Tor: unspecified
    Component:  Tor        |   Keywords:  tor-pt, needs-spec-patch pt-wants
   Resolution:             |  Parent ID:
Actual Points:             |
       Points:             |
---------------------------+-----------------------------------------------

Comment (by dcf):

 Let's think about how this might be implemented in pyptlib or goptlib. I
 left signal handling out of goptlib (instead demonstrating it in the dummy
 transports) because it's a bit complicated and it requires knowledge of
 how many connections are in progress—whatever that may mean to the
 transport.

 The signal-handling code in [https://gitweb.torproject.org/pluggable-
 transports/goptlib.git/tree/examples/dummy-client/dummy-
 client.go?id=0.4#n109 dummy-client] and [https://gitweb.torproject.org
 /pluggable-transports/goptlib.git/tree/examples/dummy-server/dummy-
 server.go?id=0.4#n106 dummy-server] is the same, and looks like this in
 pseudocode:
 {{{
 numHandlers = 0
 while true {
     event = wait_for_event()
     if event is "connection begin" {
         numHandlers++
     } else if event is "connection end" {
         numHandlers--
     } else if event is SIGINT or SIGTERM {
         break
     }
 }

 close_all_listeners()

 if event is SIGTERM {
     exit()
 }

 // event was SIGINT, wait until second signal or no more handlers
 while numHandlers > 0 {
     event = wait_for_event()
     if event is "connection end" {
         numHandlers--
     } else if event is SIGINT or SIGTERM {
         break
     }
 }
 }}}
 It's only about 30 lines, so not terrible to copy-paste, but if we add
 interaction with stdin (especially if it's switched through
 TOR_PT_EXIT_ON_STDIN_CLOSE), we should think of a way to librarify it, to
 avoid mistakes. Basically, we want to treat "stdin eof" the same as
 SIGTERM everywhere.

 The reason the above code is not in goptlib is that it requires knowing
 how a "connection begin" and "connection end" happen, and how to do
 close_all_listeners. I didn't think of a nice universal abstraction for
 those. A "connection" may be ill-defined; for example, meek-server treats
 an in-progress HTTP request as a "connection" for the purpose of signal
 handling. I also wasn't sure if close_all_listeners is all any transport
 will have to do.

 For comparison, here's what [https://gitweb.torproject.org/pluggable-
 transports/meek.git/tree/meek-client-torbrowser/meek-client-
 torbrowser.go?id=0.16#n185 meek-client-torbrowser] (the first program to
 test the close-on-stdin-eof idea) does:
 {{{
 event = wait_for_event()
 if event is "stdin eof" {
     // pretend it was a SIGTERM
     event = SIGTERM
 }
 // pass the signal to meek-client so it can react
 send_signal_to_child(event)
 if event == SIGINT {
     wait_for_event()
 }
 }}}

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