Bugginess since crypto changes

I'm looking over the tinc-1.1 branch again. I'm getting some errors that
I haven't been able to track down yet. tinc sometimes crashes either on
its own (I think after a timeout has fired?) or when I
hit ctrl-C. I've seen a few different behaviors in particular, as
reported by valgrind. Dumps below.

I suspected the bufferevent changes, but I haven't gotten any revision
before 1550 to crash. Looks like revisions 1546 and up started adding
new crypto code, but 1550 was the first to actually use it. 1550
definitely crashes.

How well-tested is this stuff? Have you seen crashes like this?

I'll keep looking for the problem. I'm working on {tincctl,control}.c
changes in another working copy, but I don't want to muddy the waters by
committing anything significant when there's still a crash going on.

crash 1:

==28913== Invalid read of size 8
==28913==    at 0x412150: list_unlink_node (list.c:97)
==28913==    by 0x412278: list_delete_node (list.c:111)
==28913==    by 0x407143: flush_queue (net_packet.c:451)
==28913==    by 0x40E2AE: ans_key_h (protocol_key.c:239)
==28913==    by 0x40BC58: receive_request (protocol.c:157)
==28913==    by 0x405B87: receive_meta (meta.c:138)
==28913==    by 0x406867: handle_meta_connection_data (net.c:225)
==28913==    by 0x4C0FAC0: event_base_loop (event.c:318)
==28913==    by 0x40601F: main_loop (net.c:374)
==28913==    by 0x411853: main (tincd.c:329)

Re: Bugginess since crypto changes

On Fri, Jul 20, 2007 at 10:45:59AM -0700, Scott Lamb wrote:

> I'm looking over the tinc-1.1 branch again. I'm getting some errors that
> I haven't been able to track down yet. tinc sometimes crashes either on
> its own (I think after a timeout has fired?) or when I
> hit ctrl-C. I've seen a few different behaviors in particular, as
> reported by valgrind. Dumps below.
> 
> I suspected the bufferevent changes, but I haven't gotten any revision
> before 1550 to crash. Looks like revisions 1546 and up started adding
> new crypto code, but 1550 was the first to actually use it. 1550
> definitely crashes.
> 
> How well-tested is this stuff? Have you seen crashes like this?

I must admit that it worked ok until the last commits. At the moment I
have no time to look at it in detail. If you want, you can join #tinc on
irc.oftc.net, there are other people trying to work with the 1.1 branch
(mjt is one). If you can't find the problem but if you want to keep on
developing in the 1.1 branch, feel free to back out the latest changes.

Thank you for working on tinc again! I hope I have some more time myself
in two or three weeks.

--

-- 
Met vriendelijke groet / with kind regards,
     Guus Sliepen <guus@...>

On Fri, Jul 20, 2007 at 10:45:59AM -0700, Scott Lamb wrote:

Re: Bugginess since crypto changes

Guus Sliepen wrote:
>> How well-tested is this stuff? Have you seen crashes like this?
> 
> I must admit that it worked ok until the last commits. At the moment I
> have no time to look at it in detail. If you want, you can join #tinc on
> irc.oftc.net, there are other people trying to work with the 1.1 branch
> (mjt is one). If you can't find the problem but if you want to keep on
> developing in the 1.1 branch, feel free to back out the latest changes.
> 
> Thank you for working on tinc again! I hope I have some more time myself
> in two or three weeks.

That's cool. I've got a lot of time for the next week, so I guess it
will be a relay race.

Haven't spent much time yet figuring out that crash, but I've got my
tincctl patches ready to apply once it's fixed.

Best regards,
Scott

--

-- 
Scott Lamb <http://www.slamb.org/>

tincctl patches

(Second try to send this. I wonder if the first one gotten eaten by a
spam filter; I'll link to patches instead of attaching them.)

Here are the tincctl patches I've been working on. They apply to
http://www.tinc-vpn.org/svn/tinc/branches/1.1 <at> 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.

* I put in a binary protocol for sending request/responses. Maybe
overkill, but I wanted something that would convey error status and
message boundaries.

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

http://www.slamb.org/tmp/tincctl-patches/0001-Update-documentation-to-match-tincctl-changes.patch
http://www.slamb.org/tmp/tincctl-patches/0002-Fancier-protocol-for-control-socket.patch
http://www.slamb.org/tmp/tincctl-patches/0003-Avoid-Linux-only-credential-based-pid-passing.patch
http://www.slamb.org/tmp/tincctl-patches/0004-Dump-through-control-socket.patch
http://www.slamb.org/tmp/tincctl-patches/0005-Purge-through-the-control-socket.patch
http://www.slamb.org/tmp/tincctl-patches/0006-Alter-debugging-levels-through-control-socket.patch
http://www.slamb.org/tmp/tincctl-patches/0007-Retry-connections-through-control-socket.patch
http://www.slamb.org/tmp/tincctl-patches/0008-Reload-configuration-through-control-socket.patch

tincctl patches

Here are the tincctl patches I've been working on. They apply to
http://www.tinc-vpn.org/svn/tinc/branches/1.1 <at> 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.

* I put in a binary protocol for sending request/responses. Maybe
overkill, but I wanted something that would convey error status and
message boundaries.

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

Best regards,
Scott

--

-- 
Scott Lamb <http://www.slamb.org/>

>From edfdd938ae878ad3f6a0b5942b1493189e29bf22 Mon Sep 17 00:00:00 2001
From: Scott Lamb <slamb@...>
Date: Sat, 21 Jul 2007 12:20:54 -0700
Subject: [PATCH] Dump through control socket

Re: tincctl patches

On Sat, Jul 21, 2007 at 01:50:07PM -0700, Scott Lamb wrote:

> (Second try to send this. I wonder if the first one gotten eaten by a
> spam filter; I'll link to patches instead of attaching them.)

It was held for moderator approval because the mail was larger than 40
MB.

-- 
Met vriendelijke groet / with kind regards,
     Guus Sliepen <guus@...>

On Sat, Jul 21, 2007 at 01:50:07PM -0700, Scott Lamb wrote:

> (Second try to send this. I wonder if the first one gotten eaten by a
> spam filter; I'll link to patches instead of attaching them.)

It was held for moderator approval because the mail was larger than 40
MB.

--

-- 
Met vriendelijke groet / with kind regards,
     Guus Sliepen <guus@...>

Re: tincctl patches

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 <at> 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:

packet default route

Hi All,

I have two queries to be clarified. Any response would be appreciated.

1. If I setup a default route to a particular network to go to the tun
device which in turn is picked up by the tinc daemon how does the
subsequent UDP/TCP connection made by the tinc daemon to the peer
daemon go through the network to the other end and not go to the local
tun device again ( by the virtue of the default route that we setup
earlier ).I am curious how tinc handles this.

2. If a default is route is set up in such a way that all the packets
can be intercepted by the tinc, then how does tinc handle the case of
an individual packet being forwarded to a particular host. Would the
setup be bypassed in that case.Once we setup the network so that *all*
the packets being sent out or being received _should_ go through the
daemon ( iptables etc.) would there be any case where this setup is
not used.

Thanks
Ashwin Ganti.

--

-- 
"There are two ways of constructing a software design: One way is to
make it so simple that there are obviously no deficiencies and the
other way is to make it so complicated that there are no obvious
deficiencies." -- C.A.R. Hoare, The 1980 ACM Turing Award Lecture
www.cs.uic.edu/~aganti
--

Re: packet default route

On Mon, Jul 23, 2007 at 02:22:18PM -0500, Ashwin Ganti wrote:

> 1. If I setup a default route to a particular network to go to the tun
> device which in turn is picked up by the tinc daemon how does the
> subsequent UDP/TCP connection made by the tinc daemon to the peer
> daemon go through the network to the other end and not go to the local
> tun device again ( by the virtue of the default route that we setup
> earlier ).I am curious how tinc handles this.

Tinc does not handle this. You are responsible to add another route so
that tinc's own UDP and TCP traffic does not loop back to the tun
device.  This is easy if you don't use the IP addresses of the hosts
running the tinc daemons on the VPN itself. But if you do, you can use
techniques like source based routing or firewall techniques (for
example, netfilter's ROUTE target) to divert only tinc's traffic to a
real interface.

> 2. If a default is route is set up in such a way that all the packets
> can be intercepted by the tinc, then how does tinc handle the case of
> an individual packet being forwarded to a particular host. Would the
> setup be bypassed in that case.Once we setup the network so that *all*
> the packets being sent out or being received _should_ go through the
> daemon ( iptables etc.) would there be any case where this setup is
> not used.

On the host that sends a packet, the kernel will first use the routing
table to see to which interface it has to send the packet. If it goes to
tinc's tun interface, then tinc will encrypt and forward the packet. If
the routing table says the packet can be sent directly, the kernel will
send it to a real interface, thus bypassing tinc. A default route always

Re: tincctl patches

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