Opened 11 months ago

Last modified 6 weeks ago

#8908 needs_revision enhancement

Tor systemd socket activation support

Reported by: cypherpunks Owned by: intgr
Priority: normal Milestone: Tor: 0.2.6.x-final
Component: Tor Version: Tor: unspecified
Keywords: tor-relay systemd Cc: steven@…
Actual Points: Parent ID:
Points:

Description

Allow Tor to be started on-demand on Linux under systemd.

WIP patch posted here: https://lists.torproject.org/pipermail/tor-dev/2013-May/004885.html

More info about socket activation: http://0pointer.de/blog/projects/socket-activation.html

Child Tickets

Change History (11)

comment:1 Changed 11 months ago by arma

  • Priority changed from minor to normal

comment:2 Changed 11 months ago by nickm

  • Milestone set to Tor: 0.2.5.x-final

comment:3 Changed 11 months ago by nickm

  • Status changed from new to needs_review

comment:4 Changed 11 months ago by nickm

This actually looks reasonably solid. Here are a few points we should think about:

  • Does it actually work to only start Tor when Tor receives a SocksPort or ControlPort request? When Tor first starts after significant downtime, it needs to download a pretty big amount of directory data, and build enough circuits for user traffic. Does that happen fast enough to answer the request that made systemd launch Tor?
  • A Tor is _supposed_ to actually turn itself nearly off when it sees no user traffic. Does your need for this feature mean that feature is not working?
  • If this is incompatible with hibernation, shouldn't we detect its use along with hibernation, and warn the user?
  • It appears that we do systemd socket discovery by default, unconditionally. Can that be right?
  • I would like if the implementation in Tor (that is, the parts outside of the sd-daemon.c code) were not systemd-specific. For example, it would also be nice to have the ability to write a little launcher program that bound to some low ports, then did a setuid(tor-daemon) and exec()d Tor. That program might want to pass a list of fds via the command line. For such a program, it would make sense to reuse 90% of this code, but have the fds come from some source other than sd-daemon.c.
  • There are no unit tests for this code at all. Is there anything we can do to make this tested? I would love for 0.2.5 to be the release where we stop adding untested code.

Minor nitpicks:

  • The new C code in Tor doesn't follow K&R indentation style like the rest of Tor.
  • I bet when we go to compile this on windows, we'll find at least one or two more methods that should have been wrapped in #ifndef _WIN32.

I can look at the code in more detail later, but I figured a rapid review here would be of benefit.

comment:5 Changed 11 months ago by intgr

  • Owner changed from cypherpunks to intgr
  • Priority changed from normal to minor
  • Status changed from needs_review to assigned

This actually looks reasonably solid

Thanks. It wasn't meant to be ready for merging yet, but it works well enough for me and I wanted to get some feedback before I invest more time into it.

When Tor first starts after significant downtime, it needs to download a pretty big amount of directory data, and build enough circuits for user traffic. Does that happen fast enough to answer the request that made systemd launch Tor?

I tried it by deleting /var/lib/tor and running "time torify wget http://httpbin.org/ip -o/dev/null -O-"

Over five tries I got 27 seconds, 11s, 9s, 67s, 12s. I guess some applications might hit timeouts at 67 seconds, but wget and Firefox don't seem to have a problem.

If this is incompatible with hibernation, shouldn't we detect its use along with hibernation, and warn the user?

Well it should be fixed one way or another before merging. As mentioned in the mailing list thread, we could keep the listening sockets open, but simply reject new incoming connections.

It appears that we do systemd socket discovery by default, unconditionally. Can that be right?

I think that's fine. If it isn't started by systemd then the environment vars will be unset and it will behave just like before.

For example, it would also be nice to have the ability to write a little launcher program that bound to some low ports, then did a setuid(tor-daemon) and exec()d Tor

The interface used by systemd to pass in sockets is pretty generic (sets 2 environment vars, LISTEN_PID and LISTEN_FDS). I think it could be re-used for that. TODO: documentation.

Is there anything we can do to make this tested?

Sure, I'll try to figure something out.

comment:6 Changed 11 months ago by intgr

  • Priority changed from minor to normal
  • Status changed from assigned to needs_revision

Oops, I messed up the fields (dunno how).

comment:7 Changed 9 months ago by nickm

  • Keywords tor-relay added

comment:8 Changed 9 months ago by stebalien

  • Cc steven@… added

comment:9 Changed 8 weeks ago by nickm

  • Keywords systemd added

comment:10 Changed 8 weeks ago by misc

So looking at the patch (quickly), I have a few remark :

  • the use and inclusion of sd-daemon.c is deprecated. I would also recommend against bundling if possible.
  • in systemd_adopt_socket, why not use the sd_is_socket function ?
  • in connection_listener_new, why do you close the tor_close_socket if listen fail ( ie, this is not present in the original code, and that's already done in the cleanup label code, after the label 'err')

Otherwise, that's quite a interesting approach ( comparing the exiting config with the systemd socket ), so I will take a bit more time to look in more details.

comment:11 Changed 6 weeks ago by nickm

  • Milestone changed from Tor: 0.2.5.x-final to Tor: 0.2.6.x-final

Moving nearly all needs_revision tickets into 0.2.6, as now untimely for 0.2.5.

Note: See TracTickets for help on using tickets.