Guus Sliepen [Tue, 27 Feb 2018 20:08:57 +0000 (21:08 +0100)]
Work around a GCC bug that causes inet_checksum() to give wrong results.
Valgrind reports the following bug:
==24877== Conditional jump or move depends on uninitialised value(s)
==24877== at 0x12283E: inet_checksum (route.c:80)
==24877== by 0x12283E: route_ipv6_unreachable (route.c:315)
==24877== by 0x1236AC: route_ipv6 (route.c:751)
==24877== by 0x1236AC: route (route.c:1160)
==24877== by 0x113DE0: receive_tcppacket (net_packet.c:493)
==24877== by 0x1119D4: receive_meta (meta.c:315)
==24877== by 0x113288: handle_meta_connection_data (net.c:287)
==24877== by 0x11A091: handle_meta_io (net_socket.c:491)
==24877== by 0x10FB0C: event_loop (event.c:370)
==24877== by 0x11362E: main_loop (net.c:489)
==24877== by 0x10CACA: main (tincd.c:551)
Clearing the variable pseudo in route_ipv6_unreachable removes this error,
but the resulting checksum is still bad. If one instead adds a dummy
write that depends on checksum, the error goes away and the checksum is
correct.
Guus Sliepen [Mon, 26 Feb 2018 21:19:43 +0000 (22:19 +0100)]
Unconditionally remove timeouts from the queue before calling the callback.
We are going to unlink the timeout from the splay tree anyway, so do it
unconditionally before the callback, instead of waiting until after the
callback to check whether or not to remove it based on its expiration
time.
Todd C. Miller [Thu, 22 Feb 2018 21:27:37 +0000 (14:27 -0700)]
In device_handle_read() we need to reset the read event on error or
it will keep firing. This is easy to reproduce by suspending the
machine while tinc is running.
Todd C. Miller [Wed, 21 Feb 2018 03:18:38 +0000 (20:18 -0700)]
Fix heap corruption on Windows exposed by the use-after free fix.
reset_address_cache() could call free_known_addresses() on a struct
addrinfo * that was returned by getaddrinfo(). It seems safest to just
make a copy of the addresses returned by getaddrinfo() so we can always
use free_known_addresses() instead of trying to determine whether or
not we need to use freeaddrinfo().
Guus Sliepen [Sun, 18 Feb 2018 15:51:06 +0000 (16:51 +0100)]
Reduce memory allocations due to HMAC() and EVP_MD_*().
HMAC() allocates a temporary buffer on the heap each time it is called.
Similarly, we called EVP_MD_CTX_create() every time we wanted to
calculate a hash. Use HMAC_CTX and EVP_MD_CTX variables to store the
state so no (re)allocations are necessary. HMAC() was called for every
legacy packet sent and received.
Guus Sliepen [Sun, 18 Feb 2018 14:38:12 +0000 (15:38 +0100)]
Reduce memory allocations due to zlib's uncompress().
Everytime uncompress() is called, zlib allocates some buffer on the heap
and frees it again. When compression is enabled, this is the biggest source
of memory allocations in tinc. Instead of using this function, use
inflate(), which can store its state in a z_stream variable, which avoids
(re)allocating memory for every packet received.
Guus Sliepen [Sun, 18 Feb 2018 14:33:36 +0000 (15:33 +0100)]
Add code coverage testing support.
Allows configure to be run with the --enable-code-coverage flag, allowing
one to run make check-code-coverage, which runs the test suite and produces
a code coverage report.
Todd C. Miller [Fri, 16 Feb 2018 21:17:39 +0000 (14:17 -0700)]
Fix a use-after-free bug in get_recent_address() and two related issues.
1) The sockaddr_t * returned may be part of memory freed by the call to
freeaddrinfo().
2) The sockaddr_t * returned from a recently seen address not in the
cache was cast from struct addrinfo *ai, not the struct sockaddr *
inside of it.
3) In do_outgoing_connection(), when filling in the address in the
connection_t, there is a buffer overflow (read, not write) if
the sa returned by get_recent_address() didn't come from the
cache of recently seen addresses. That is, it was really a
struct sockaddr * and not a sockaddr_t *. This last was
found by building tinc with address sanitizer.
Todd C. Miller [Wed, 31 Jan 2018 21:55:20 +0000 (14:55 -0700)]
In device_issue_read() there is no need to reset Offset and OffsetHigh
to 0; they are only used for seekable files (not sockets).
Reset the write event before the call to WriteFile(). This is
consistent with how the read event is reset before ReadFile().
Clear device_write_packet.len() if WriteFile() fails with an error
other than ERROR_IO_PENDING; otherwise write_packet() will call
GetOverlappedResult() the next time it is run even though there is
no write in progress.
Todd C. Miller [Tue, 23 Jan 2018 22:57:58 +0000 (15:57 -0700)]
WSAEVENT is a pointer, so we cannot simply return the different of two
events in io_compare(), which returns an int. This can return the wrong
result for 64-bit executables.
Etienne Dechamps [Wed, 17 Jan 2018 19:37:53 +0000 (19:37 +0000)]
Move ResetEvent() call before ReadFile().
Commit 313a752 changed the Windows device code such that ResetEvent() is
called on the read OVERLAPPED structure before GetOverlappedResult(), as
opposed to before ReadFile(). In [1] Guus pointed out that this doesn't
make a ton of sense, and I agree with him; it must have been an
oversight on my part when I wrote this code.
Surprisingly, none of this makes any difference in my testing, at least
with the standard TAP 9.0.0.9 driver. Nevertheless, this code is
probably wrong and fixing it will make me sleep better at night.
Guus Sliepen [Fri, 5 Jan 2018 21:49:30 +0000 (22:49 +0100)]
Add a cache of recently seen addresses.
This maintains a cache file for each host we have communicated with, either
via TCP or UDP. The cache is used when trying to make outgoing connections,
and is updated whenever a successful TCP or UDP connection is established.
Up to 8 addresses are stored in the cache.
Currently, the cache is stored in /etc/tinc/NETNAME/cache. The directory
has to be manually created to opt in to this feature for now.
Guus Sliepen [Mon, 6 Nov 2017 21:35:28 +0000 (22:35 +0100)]
Support autoconf's --runstatedir option.
Put the PID file in @runstatedir@ instead of @localstatedir@/run. This
requires autoconf 2.70, which is not released yet, so add a fallback to
use @localstatedir@/run if @runstatedir@ is not set.
Guus Sliepen [Wed, 25 Oct 2017 19:08:29 +0000 (21:08 +0200)]
Only forward SPTPS packets if Forwarding = internal.
This tries to match what is done for packets using the legacy protocol.
However, since SPTPS is end-to-end encrypted, Forwarding = kernel cannot
be implemented. In that case, we also drop the packets.
Guus Sliepen [Sat, 7 Oct 2017 15:47:19 +0000 (17:47 +0200)]
Convert sizeof foo to sizeof(foo).
While technically sizeof is an operator and doesn't need the parentheses
around expressions it operates on, except if they are type names, code
formatters don't seem to handle this very well.
Allow forcing either IPv4 or IPv6 for sptps_test, and use IPv4 for the
sptps-basic test. Since sptps_test is only opening a single listening
socket, and you cannot control which address family it uses, this gets
around a problem where the listening side is using a different address
family than the one connecting to it.
Guus Sliepen [Tue, 22 Aug 2017 18:51:44 +0000 (20:51 +0200)]
Make autoconnect try to heal network splits.
When we have less than three connections, we greedily try to connect to any
viable node. However, once we have three connections, try to connect to
nodes that we know of but that aren't reachable.
We also make sure that if there are 100 reachable nodes, and 1 unreachable
one, that not all 100 reachable nodes try to connect to the unreachable
at the same time.
On Linux network restart, Tinc can get into a loop writing millions of error messages "Error while reading from Linux tun/tap device (tun mode) /dev/net/tun: File descriptor in bad state" to the log. https://github.com/NixOS/nixpkgs/pull/27675
It should be somehow aborted.
Here is my quick hack.
Guus Sliepen [Sun, 28 May 2017 10:48:32 +0000 (12:48 +0200)]
Set KillMode=mixed in the systemd service file.
This ensures only the main process is sent the SIGTERM, and not anything
else that might have started in the same control group, including the
tinc-down script.
Guus Sliepen [Tue, 7 Mar 2017 18:19:19 +0000 (19:19 +0100)]
Use free_known_addresses() to free memory allocated by get_known_addresses().
We know what struct addrinfo looks like, but the standard says nothing
about how it is allocated. So we cannot trust freeaddrinfo() to work
correctly on the struct addrinfo list we allocated ourselves in
get_known_addresses(). To make a distinction by allocations from the
latter and from str2addrinfo(), we keep two pointers (*ai and *kai) in
struct outgoing, and use the freeing function that is appropriate for
each.