This fixes the following compiler warning when building for Windows:
mingw/device.c: In function ‘setup_device’:
mingw/device.c:92:9: error: unused variable ‘thread’ [-Werror=unused-variable]
HANDLE thread;
^
This fixes the following compiler warning when building for Windows:
mingw/device.c: In function ‘setup_device’:
mingw/device.c:186:2: error: passing argument 2 of ‘io_add_event’ from incompatible pointer type [-Werror]
io_add_event(&device_read_io, device_handle_read, NULL, CreateEvent(NULL, TRUE, FALSE, NULL));
^
In file included from mingw/../net.h:27:0,
from mingw/../subnet.h:24,
from mingw/../conf.h:34,
from mingw/device.c:26:
mingw/../event.h:61:13: note: expected ‘io_cb_t’ but argument is of type ‘void (*)(void *)’
extern void io_add_event(io_t *io, io_cb_t cb, void* data, WSAEVENT event);
This fixes the following compiler warning when building for Windows:
script.c: In function ‘execute_script’:
script.c:52:5: error: value computed is not used [-Werror=unused-value]
*q++;
^
This fixes the following compiler warning when building for Windows:
net_packet.c: In function ‘send_udppacket’:
net_packet.c:633:6: error: unused variable ‘origpriority’ [-Werror=unused-variable]
int origpriority = origpkt->priority;
^
This is so the positions of the other bits don't change, making it easier to
debug problems with different versions of tinc.
Also fix the padding so connection_status_t is exactly 32 bits.
The only places where connection_t::status.active is modified is in
ack_h() and terminate_connection(). In both cases, connection_t::edge
is added and removed at the same time, and that's the only places
connection_t::edge is set. Therefore, the following is true at all
times:
!c->status.active == !c->edge
This commit removes the redundant state information by getting rid of
connection_t::status.active, and using connection_t::edge instead.
in receive_udppacket(), we initialize outpkt to a default value but the
value is never read anywhere, as every read is preceded by a write.
This issue was found by the clang static analyzer tool:
http://clang-analyzer.llvm.org/
If choose_local_address() is unable to find a local address (e.g.
because of old nodes that don't send their local address information),
then send_sptps_data() ends up using uninitialized variables for the
socket and address.
This regression was introduced in
4159108971. The commit took care of
handling that case in send_udppacket() but was missing the same fix
for send_sptps_data().
This bug was found by the clang static analyzer tool:
http://clang-analyzer.llvm.org/
Based on a patch from Etienne Dechamps. We avoid the use of %hhx, since even
though it is C99, not all compilers support it yet. We use %x instead, since
it's guaranteed that the minimum size of function arguments on the stack or in
registers is that of an int.
On Windows, the event loop io tree uses the Windows Event handle to
differentiate between io_t objects. Unfortunately, there is a bug in
the io_add_event() function (introduced in
2f9a1d4ab5) as it sets the event after
inserting the object into the tree, resulting in objects appearing in
io_tree out of order.
This can lead to crashes on Windows as the event loop is unable to
determine which events fired.
Setting the Port configuration variable to zero can be used to make tinc
listen on a system-assigned port. Unfortunately, in this scenario myport
will be zero, which means that tinc won't transmit its actual UDP
listening port to other nodes. This breaks UDP hole punching and local
discovery.
Commit 611217c96e introduced a regression
because it accidentally reordered the timeout handler calls and the
fdset setup code. This means that any io_add(), io_del() or io_set()
calls in timeout handlers would be ignored in the current event loop
iteration, resulting in erratic behavior.
The most visible symptom is when a metaconnection timeout occurs and the
connection is closed; the timeout handler closes the socket but it still
ends up in the select() call, typically resulting in the following
crash:
Error while waiting for input: Bad file descriptor
Currently we don't do any shortening on IPv6 addresses (aside from
removing trailing zeroes) before printing them. This commit makes
textual addresses smaller by shortening them according to the rules
described in RFC 5952. This is also the canonical textual representation
for IPv6 addresses, thus making them easier to compare.
This commit suppresses subnet prefix length output (/xx) for subnets
that only contain one address (/32 for IPv4, /128 for IPv6). It also
suppresses weight information if the subnet is using the default
weight. This improves readability of net2str() output in the majority
of cases.
tinc currently prints MAC addresses without trailing zeroes, for example:
1:2:3:4:5:6
This looks weird and is inconsistent with how MAC addresses are
displayed everywhere else. This commit adds trailing zeroes, so the
above address will be printed as the following:
01:02:03:04:05:06
This is a complete rewrite of the str2net() function. Besides
refactoring duplicate code, this new code brings the following fixes
and improvements:
- Fixes handling of leading/trailing double colon in IPv6 addresses.
For example, with the previous code the address
2001:0db8:85a3:0000:0000:8a2e:0370:: is interpreted as a MAC address,
and ::0db8:85a3:0000:0000:8a2e:0370:7334 is rejected.
- Catches more invalid cases, such as garbage at the end of the string.
- Adds support for dotted quad notation in IPv6 (e.g. ::1.2.3.4).
See RFC 4291, section 2.2 for details on the textual format of IPv6
addresses.
Instead of using a hardcoded version number in configure.ac, this makes
tinc use the live version reported by "git describe", queried on-the-fly
during the build process and regenerated for every build.
This provides several advantages:
- Less redundancy: git is now the source of truth for version
information, no need to store it in the repository itself.
- Simpler release process: just creating a git tag automatically
updates the version. No need to change files.
- More useful version information: tinc will now display the number of
commits since the last tag as well as the commit the binary is built
from, following the format described in git-describe(1).
Here's an example of tincd --version output:
tinc version release-1.1pre10-48-gc149315 (built Jun 29 2014 15:21:10, protocol 17.3)
When building directly from a release tag, this would like the following:
tinc version release-1.1pre10 (built Jun 29 2014 15:21:10, protocol 17.3)
(Note that the format is slightly different - because of the way the
tags are named, it says "release-1.1pre10" instead of just "1.1pre10")
This prevents the date and time shown in version information from
getting stale because of partial builds. With these changes, date and
time information is written to a dedicated object file that gets rebuilt
every time make is run, even if there are no changes.
This adds a new option, BroadcastSubnet, that allows the user to
declare broadcast subnets, i.e. subnets which are considered broadcast
addresses by the tinc routing layer. Previously only the global IPv4
and IPv6 broadcast addresses were supported by virtue of being
hardcoded.
This is useful when using tinc in router mode with Ethernet virtual
devices, as it can be used to provide broadcast support for a local
broadcast address (e.g. 10.42.255.255) instead of just the global
address (255.255.255.255).
This is implemented by removing hardcoded broadcast addresses and
introducing "broadcast subnets", which are subnets with a NULL owner.
By default, behavior is unchanged; this is accomplished by adding
the global broadcast addresses for Ethernet, IPv4 and IPv6 at start
time.
Implementation of sptps_verify_datagram() was left as a TODO. This
causes problems when using SPTPS in tinc, because this function is
used in try_mac(), which itself is used in try_harder() to locate
nodes sending UDP packets from unexpected addresses. In the current
state this function always returns true, resulting in UDP addresses
of random nodes getting changed which makes UDP communication
fragile and unreliable. In addition, this makes UDP communication
impossible through port translation and local discovery.
This commit adds the missing implementation, thus fixing the issue.
Recent improvements to the local discovery mechanism makes it cheaper,
more network-friendly, and now it cannot make things worse (as opposed
to the old mechanism). Thus there is no reason not to enable it by
default.
The new local address based local discovery mechanism is technically
superior to the old broadcast-based one. In fact, the old algorithm
can technically make things worse by e.g. sending broadcasts over the
VPN itself and then selecting the VPN address as the node's UDP
address. This cannot happen with the new mechanism.
Note that this means old nodes that don't send their local addresses in
ADD_EDGE messages can't be discovered, because there is no address to
send discovery packets to. Old nodes can still discover new nodes by
sending them broadcasts, though.
This introduces a new way of doing local discovery: when tinc has
local address information for the recipient node, it will send local
discovery packets directly to the local address of that node, instead
of using broadcast packets.
This new way of doing local discovery provides numerous advantages compared to
using broadcasts:
- No broadcast packets "polluting" the local network;
- Reliable even if the sending host has multiple network interfaces (in
contrast, broadcasts will only be sent through one unpredictable
interface)
- Works even if the two hosts are not on the same broadcast domain. One
example is a large LAN where the two hosts might be on different local
subnets. In fact, thanks to UDP hole punching this might even work if
there is a NAT sitting in the middle of the LAN between the two nodes!
- Sometimes a node is reachable through its "normal" address, and via a
local subnet as well. One might think the local subnet is the best route
to the node in this case, but more often than not it's actually worse -
one example is where the local segment is a third party VPN running in
parallel, or ironically it can be the local segment formed by the tinc
VPN itself! Because this new algorithm only checks the addresses for
which an edge is already established, it is less likely to fall into
these traps.
In addition to the remote address, each edge now stores the local address from
the point of view of the "from" node. This information is then made available
to other nodes through a backwards-compatible extension to ADD_EDGE messages.
This information can be used in future code to improve packet routing.
tinc is using a separate thread to read from the TAP device on Windows.
The rationale was that the notification mechanism for packets arriving
on the virtual network device is based on Win32 events, and the event
loop did not support listening to these events.
Thanks to recent improvements, this event loop limitation has been
lifted. Therefore we can get rid of the separate thread and simply add
the Win32 "incoming packet" event to the event loop, just like a socket.
The result is cleaner code that's easier to reason about.
Currently, when the tinc service handler callback (which runs in a
separate thread) receives a service shutdown request, it calls
event_exit() to request the event loop to exit.
This approach has a few issues:
- The event loop will only notice the exit request when the next event
fires. This slows down tinc service shutdown. In some extreme cases
(DeviceStandby enabled, long PingTimeout and no connections),
shutdown can take ages.
- Strictly speaking, because of the absence of memory barriers, there
is no guarantee that the event loop will even notice an exit request
coming from another thread. I suppose marking the "running" variable
as "volatile" is supposed to alleviate that, but it's unclear whether
that provides any guarantees with modern systems and compilers.
This commit fixes the issue by leveraging the new event loop Windows
interface, using a custom Windows event that is manually set when
shutdown is requested.
This commit changes the event loop to use WSAEventSelect() and
WSAWaitForMultipleEvents() on Windows. This paves the way for making the
event loop more flexible on Windows by introducing the required
infrastructure to make the event loop wait on any Win32 event.
This commit only affects the internal implementation of the event
module. Externally visible behavior remains strictly unchanged (for
now).
Commit 86a99c6b99 changed the way we
handle connection events to protect against spurious event loop
callbacks. Unfortunately, it turns out that calling connect() twice on
the same socket results in different behaviors depending on the platform
(even though it seems well defined in POSIX). On Windows this resulted
in the connection handling code being unable to react to connection
errors (such as connection refused), always hitting the timeout; on
Linux this resulted in spurious error messages about connect() returning
success.
In POSIX and on Linux, using connect() on a socket where the previous
attempt failed will attempt to connect again, resulting in unnecessary
network activity. Using getsockopt(SO_ERROR) before connect() solves
that, but introduces a race condition if a connection failure happens
between the two calls.
For this reason, this commit switches from connect() to a zero-sized
send() call, which is more consistent (though not completely, see the
truth table in the comments) and simpler to use for that purpose. Note
that Windows explictly support empty send() calls; POSIX says nothing
on the subject, but testing shows it works at least on Linux.
(Surprisingly enough, Windows seems more POSIX-compliant than Linux on
this one!)
The event loop does not guarantee that spurious write I/O events do not
happen; in fact, they are guaranteed to happen on Windows when
event_flush_output() is called. Because handle_meta_io() does not check
for spurious events, a metaconnection socket might appear connected even
though it's not, and will fail immediately when sending the ID request.
This commit fixes this issue by making handle_meta_io() check the
connection status before assuming the socket is connected. It seems that
the only reliable way to do that is to try to call connect() again and
look at the error code, which will be EISCONN if the socket is
connected, or EALREADY if it's not.
When using socket functions, "sockerrno" is supposed to be used to
retrieve the error code as opposed to "errno", so that it is translated
to the correct call on Windows (WSAGetLastError() - Windows does not
update errno on socket errors). Unfortunately, the use of sockerrno is
inconsistent throughout the tinc codebase, as errno is often used
incorrectly on socket-related calls.
This commit fixes these oversights, which improves socket error
handling on Windows.
These Windows include lines are capitalized, which causes the build to fail
when cross-compiling from Linux to Windows using MinGW as the MinGW headers
are entirely lower case.
Besides controlling when tinc-up and tinc-down get called, this commit makes
DeviceStandby control when the virtual network interface "cable" is "plugged"
on Windows. This is more user-friendly as the status of the tinc network can
be seen just by looking at the state of the network interface, and it makes
Windows behave better when isolated.
This adds a new DeviceStandby option; when it is disabled (the default),
behavior is unchanged. If it is enabled, tinc-up will not be called during
tinc initialization, but will instead be deferred until the first node is
reachable, and it will be closed as soon as no nodes are reachable.
This is useful because it means the device won't be set up until we are fairly
sure there is something listening on the other side. This is more user-friendly,
as one can check on the status of the tinc network connection just by checking
the status of the network interface. Besides, it prevents the OS from thinking
it is connected to some network when it is in fact completely isolated.
