tincctl patches

[Guus Sliepen]

On Sat, Jul 21, 2007 at 12:21:34PM -0700, Scott Lamb wrote:

> Here are the tincctl patches I've been working on. They apply to
> http://www.tinc-vpn.org/svn/tinc/branches/1.1@1545. I intend to commit
> them once the crypto stuff's fixed. Since they're basically done, I'm
> emailing them now for review and in case I lose my hard drive or something.
> 
> They implement a pretty full set of tincctl operations. A few notes:
> 
> * 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 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.

> * I also removed the GraphDumpFile configuration option. I think now it
> makes more sense to do this sort of thing with a cron job based on
> "tincctl -n NET dump graph".

I totally agree. 

I think most of the patches are OK. However:

> 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.

Again, the rest of the patches is OK!

-- 
Met vriendelijke groet / with kind regards,
     Guus Sliepen <guus at tinc-vpn.org>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
Url : http://brouwer.uvt.nl/pipermail/tinc-devel/attachments/20070723/020debcb/attachment.pgp