Re: Integrating libnatpnp and miniupnpc into Tor

[Nick Mathewson <nickm@xxxxxxxxxxxxx>]

On Mon, May 24, 2010 at 6:04 PM, Steven J. Murdoch
<tor+Steven.Murdoch@xxxxxxxxxxxx> wrote:
 [...]
> The first is how the library will be called by Tor. One option is to
> call it from the Tor main thread. This is simple, but has the
> disadvantage that a bug in the library could cause Tor to crash. This
> would annoy the user, but on the other hand a Tor bridge without port
> forwarding being set up isn't much use.
>
> Another option is to fork() off a new process. This would insulate Tor
> from crash bugs, but isn't as robust security-wise because the
> subprocess would have read-only access to Tor's memory (unless we
> fiddle with page table permissions).

Also, on Windows, you can't fork().  You can CreateProcess (which is a
little like vfork+exec), but you need an .exe to call for that.

> Yet another option would be to fork() and exec(). This might be tricky
> to get right because Tor would need to find where the helper program
> lives. It would give some security isolation, but not much because the
> subprocess would still be running under the same user ID.

I prefer this option.  Calling a helper program is a solved problem,
after all, and once they're isolated at a process level, distributors
(or others, or maybe we) could use an OS's existing capabilities
system to isolate it from the rest of Tor.  (For Linux, this would
mean somebody doing the standard selinux magic.  Elsewhere, it might
be running  suid upnp_user.)

At the vary least, running it in a separate process makes it easier to
debug. I don't like the argument that "Tor without a forwarded port
(when needed) is not so useful; therefore it's not so bad for Tor to
just crash whenever there's a crash bug in the UPnP layer."
Remember, an immediate, traceable crash is not the only kind of crash
there is.  There are also lingering crashes that show up in unrelated
parts of the codebase with no apparent relation to their actual cause,
or crashes that show up unanticipated because of shared process
resources.   Pulling these libraries into the Tor process would means
that we'd no longer be able to clearly distinguish all Tor crashes
from port-forwarder crashes, which would make debugging work more than
just a little harder.

> Another choice we have is how to manage the build process. Neither
> miniupnpc or libnatpmp come with binary packages for major Linux
> distributions. Asking people to build these before building Tor might
> be too much to ask (especially given that they don't use autoconf).
> Also we might want to keep the version we use under our own control
> because in the past the library has broken API compatibility and their
> build scripts currently don't work on all platforms (notably the BSD
> family).

I really don't like pulling things into our source-trees except as a
last resort.  Forks are a contingency plan if Tor needs something from
these libraries that we cannot convince the developers to do, but
forks are not free, easy, fun, or polite.  If we truly need to fork,
we should fork cleanly, with the goal being an eventual re-merge,
pushing as many patches as we can upstream, and we should manage the
libnatpnp and miniupnpc code in their own repositories with that goal
in mind.

Even if that re-merge never happens, we should do our forked versions
in separate repositories from Tor so that other people can use them
too.  Otherwise, Tor's version will diverge from Vidalia's, which will
diverge from every other version under the sun.

If possible, building with these libraries should be optional.  Also,
we should get these things packaged for as many OSs as we can.  If Tor
strongly recommends them, that'd probably help.

yrs,
-- 
Nick