tinc - Oct 1999 - Observing nasty crashes

Hi,

This weekend  I changed to faster machines, some very old Ethernet card
and re-tried
Tinc... I stumbled into some nasty bugs.

Packets that are held in the receive queue become corrupted. I observed
lengths (ie, the
len field in the packet structure) that are ok when add_queue() inserts
them in the queue, and way to large lengths or even negative when the
queues get flushed.

Wanna reproduce the problem? Choose 30 seconds for key expiry and 300
seconds
for ping/pong. Do not sent data over the tunnel continuously as with
ping
etc. Just open a telnet session, wait for 5 minutes and type something.
This will almost
certainly trigger a key refresh, which will have a side effect of
flushing the queues.


If the len gets corrupted, the crypting will crash the program (segm
fault).

In general, I think the program needs a checksum to see that it does not
decrypt
corrupted packets. Since it uses UDP, the packets can get corrupted.

After that, I had a look to the add_queue() and flush_queue() funcs.
Need to so some
more work here. flush_queue() will call recv() for more packets that are
in the queue.
It sometimes ends up with calls for packets that do not exist or have
negative lengths.

I do not trust the pointer fiddling in flush_queue(). If it is flushed,
where do q->header
and q->tail point to? Why do the queue elements also have a prev pointer
or where
are these prev pointers used? add_queue() sometimes ends up with a queue
that has only
one packet, with a next pointer pointing to itself.

Letting tincd restart itself will not always work. Its better to run an
outside scheduler that
restarts it when it does not see it anymore in the ps table or when it
detects connection failure. I did this (just let it crash) so that I
also could have the core dumps and see
where it crashes.

Would like to have some more comments in the code. It costs a lot of
time if you
are new to this code and want to find out how it works.

I also was wondering if it is possible to have a stable version and a
developers version.





-
Tinc:         Discussion list about the tinc VPN daemon
Archive:      http://mail.nl.linux.org/lists/
Tinc site:    http://ftp.nl.linux.org/pub/linux/tinc/

Hi Hans,

I am fully aware of the bug that causes tinc to crash after the keys
have expired. Several people have been trying to track down the cause
for this, but to no avail.

And the send/receive queue is not the only thing that is corrupted.
The problem is much bigger than that, I suspect that sometimes, the
stack gets overwritten with garbage. That would be why tinc crashes
without a trace, eating memory 'till the kernel decides to kill it.
[ On a side note, you can send a USR2 signal to tincd to force it to
regenerate the key. ]

As for the checksum part, I'm working on a HMAC (Hashed Message
Authentication Code) implementation for tinc. This is like a checksum,
with the added functionality of tinc being able to confirm that the
packet really comes from the other party; and that it was not altered
somewhere on the road.

Doing this right, however, requires a more advanced way of dealing
with the basic protocol negotiation, what if, for example, a newer
version of tinc supports a superior hashing algorithm, and it is
talking to an older version which does not have this. There has to be
a way of choosing the best option, while being as objective as
possible.

> I do not trust the pointer fiddling in flush_queue(). If it is flushed,
> where do q->header
> and q->tail point to? Why do the queue elements also have a prev pointer
> or where
> are these prev pointers used? add_queue() sometimes ends up with a queue
> that has only
> one packet, with a next pointer pointing to itself.
You got me there, I can't remember why i put in the prev pointer in
the first place (probably some obsolete remnant of a weird braindead
implementation error :P )

> Would like to have some more comments in the code. It costs a lot of
> time if you
> are new to this code and want to find out how it works.
You are so right about this. Sometimes I find myself wondering what a
particular function was meant to do, or re-writing the same algorithm
three times just because I was not sure that it worked the first time
(often re-introducing old bugs).

> I also was wondering if it is possible to have a stable version and a
> developers version.
Yes. I'm not sure which convention to follow, but I was thinking of
something along the lines of how the Linux kernel version numbers are
treated, i.e. even minor version numbers are stable, odd is unstable/
development.

I am going to make the changes to 0.3 public as 0.3.1, and then I'll
put out a snapshot of my development directory as 0.5.0, to fall into
the rhythm of the aforementioned scheme.

Maybe it would be an idea to put up a CVS server for tinc, Rik van
Riel already kindly offered to do so.


-- 
Ivo Timmermans

You are just jealous because the little voices are talking to me.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 232 bytes
Desc: not available
Url :
http://brouwer.uvt.nl/pipermail/tinc/attachments/19991018/003bf837/attachment.pgp