tincctl patches

[Scott Lamb]

Guus Sliepen wrote:
>> * I removed most of the weirder signal handlers - didn't see much use
>> for them once this was added.
> 
> Great. Signal handlers in libevent seemed to incur a lot of syscall
> overhead, the control socket can replace all the signals.

I don't think it's too bad for as often as those signals are passed, but
I like having clean names, acknowledgment with success or failure, and
the option to pass a lot more data back and forth.

>> * I put in a binary protocol for sending request/responses. Maybe
>> overkill, but I wanted something that would convey error status and
>> message boundaries.
> 
> You can do that with a purely textual protocol as well. But I admit that
> having the length of the message in a fixed size message header makes
> life a lot easier.

Yeah, I considered text processing but thought this would be easier -
particularly when sending big blocks of text back. Maybe that will
change if sending big chunks of more structured data around. In any
case, I tried to isolate the protocol to only one function each in
tincctl.c and control.c.

>> From: Scott Lamb <slamb at slamb.org>
>> Date: Sat, 21 Jul 2007 12:20:54 -0700
>> Subject: [PATCH] Avoid Linux-only credential-based pid passing
>>
>> In particular, *BSD's closest equivalent (LOCAL_PEERCRED / struct xucred)
>> does not support pids.
> 
> You want to send a greeting message instead where the daemon sends its
> own PID back. However, if a malicious user can create a control socket
> when the real tinc daemon isn't running, then it can send back a
> different PID than its own, which may cause unintended things to happen.
> If the administrator does kill -9 `tincctl -n vpn getpid`, then the
> malicious user could get the administrator to kill the wrong process. If
> the getpeercred() functionality is not portable, we should probably do
> specific implementations for each OS instead of a generic implementation
> that can be subverted.

Hmm, I guess the default location is /var/run, which is 755 by default.
So a malicious user couldn't create a socket there, but if tinc has
already created one then died, they could rebind to it. (I believe UNIX
domain sockets don't support permissions on most platforms.) Yeah,
you're right, this patch is no good (at least by itself).

What about putting the control socket in a dedicated subdirectory with
locked-down permissions (700)? I believe this is the approach taken by
several other daemon/client pairs, including saslauthd/libsasl2. The
malicious user then couldn't bind to the socket at all, which seems
preferable to just not being able to lie about their pid.

Best regards,
Scott