oweals/tinc.git
9 years agoUse a smarter algorithm for choosing MTU discovery probe sizes.
Etienne Dechamps [Tue, 30 Dec 2014 16:34:48 +0000 (16:34 +0000)]
Use a smarter algorithm for choosing MTU discovery probe sizes.

Currently, tinc uses a naive algorithm for choosing MTU discovery probe
sizes, picking a size at random between minmtu and maxmtu.

This is of course suboptimal - since the behavior of probes is
deterministic (assuming no packet loss), it seems likely that using a
non-deterministic discovery algorithm will not yield the best results.
Furthermore, the randomness introduces a lot of variation in convergence
times.

The random solution also suffers from pathological cases - since it's
using a uniform distribution, it doesn't take into account the fact that
it's often more interesting to send small probes rather than large ones,
because getting replies is the only way we can make progress (assuming
the worst case scenario in which the OS doesn't know anything, therefore
keeping maxmtu constant). This can lead to absurd situations where the
discovery algorithm is close to the real MTU, but can't get to it
because the random number generator keeps generating numbers that are
past it.

The algorithm implemented in this patch aims to improve on the naive
random algorithm. It is organized around "cycles" of 8 probes; the sizes
of the probes decrease as we go through the cycle, thus making sure the
algorithm can cover lots of ground quickly (in case we're far from
actual MTU), but also examining the local area (in case we're close to
actual MTU). Using cycles ensures that the algorithm will "go back" to
large probes to better cover the new interval and to protect against
packet loss.

For the probe size itself, various mathematical models were simulated in
an attempt to find the one that converges the fastest; it has been
determined that using an exponential based on the size of the remaining
interval was the most effective option. The exponential is adjusted with
a magic multiplier fine-tuned to make tinc jump to the "most
interesting" (i.e. 1400+) section as soon as discovery starts.

Simulations indicate that assuming no packet loss and no help from the
OS (i.e. maxmtu stays constant), this algorithm will typically converge
to the *exact* MTU value in less than 10 probes, and will get within 8
bytes in less than 5 probes, for actual MTUs between 1417 and ~1450
(which is the range the algorithm is fine-tuned for). In contrast, the
previous algorithm gives results all over the place, sometimes taking
30+ probes to get in the ballpark. Because of the issues with the
distribution, the previous algorithm sometimes never gets to the precise
MTU value within any reasonable amount of time - in contrast, the new
algorithm will always get to the precise value in less than 30 probes,
even if the actual MTU is completely outside the optimized range.

9 years agoRemove bandwidth estimation code.
Etienne Dechamps [Tue, 30 Dec 2014 10:47:56 +0000 (10:47 +0000)]
Remove bandwidth estimation code.

tinc bandwidth estimation has always been quite unreliable (at least in
my experience), but there's no chance of it working anymore since the
last changes to MTU discovery code, because packets are not sent in
batches of three anymore.

This commit removes the dead code - fortunately, nothing depends on this
estimation (it's not even shown in node info). We probably need be
smarter about this if we do want this estimation back.

9 years agoSend one MTU probe at a time.
Etienne Dechamps [Tue, 30 Dec 2014 10:16:32 +0000 (10:16 +0000)]
Send one MTU probe at a time.

Currently, tinc sends MTU probes in batches of three every second. This
commit changes that to send one packet every 333 milliseconds instead.

This change brings two benefits:

 - It makes MTU probing faster, because MTU probe lengths are calculated
   based on minmtu, and minmtu is adjusted based on the replies. When
   sending batches of three packets, all three packets are based on the
   same minmtu estimation; in contrast, by sending one packet more
   frequently, each subsequent packet can benefit from the replies that
   have been received since the last packet was sent. As a result, MTU
   discovery converges much faster (2-3 times as fast, typically).

 - It reduces network spikiness - it's more network-friendly to send
   one packet from time to time as opposed to sending bursts.

9 years agoUse -1 to identify the post-initial MTU discovery state.
Etienne Dechamps [Thu, 1 Jan 2015 16:04:08 +0000 (16:04 +0000)]
Use -1 to identify the post-initial MTU discovery state.

This is a minor cosmetic nit to emphasise the distinction between the
initial MTU discovery phase, and the post-initial phase (i.e. maxmtu
checking).

Furthermore, this is an improvement with regard to the DRY (Don't
Repeat Yourself) principle, as the maximum mtuprobes value is only
written once.

9 years agoFix MTU as soon as possible.
Etienne Dechamps [Thu, 1 Jan 2015 10:32:14 +0000 (10:32 +0000)]
Fix MTU as soon as possible.

If a probe reply is received that makes minmtu equal to maxmtu, we
have to wait until try_mtu() runs to realize that. Since try_mtu()
runs after a packet is sent, this means there is at least one packet
(possibly more, depending on timing) that won't benefit from the
fixed MTU. This also happens when maxmtu is updated from the send()
path.

This commit fixes that by making sure we check whether the MTU can be
fixed every time minmtu or maxmtu is touched.

9 years agoMove try_mtu() closer to try_tx().
Etienne Dechamps [Mon, 29 Dec 2014 17:05:19 +0000 (17:05 +0000)]
Move try_mtu() closer to try_tx().

This moves related functions together, and is a pure cut-and-paste
change. The reason it was not done in the previous commit is because it
would have made the diff harder to review.

9 years agoMove PMTU discovery code into the TX path.
Etienne Dechamps [Mon, 29 Dec 2014 16:47:49 +0000 (16:47 +0000)]
Move PMTU discovery code into the TX path.

Currently, the PMTU discovery code is run by a timeout callback,
independently of tunnel activity. This commit moves it into the TX
path, meaning that send_mtu_probe_handler() is only called if a
packet is about to be sent. Consequently, it has been renamed to
try_mtu() for consistency with try_tx(), try_udp() and try_sptps().

Running PMTU discovery code only as part of the TX path prevents
PMTU discovery from generating unreasonable amounts of traffic when
the "real" traffic is negligible. One extreme example is sending one
real packet and then going silent: in the current code this one little
packet will result in the entire PMTU discovery algorithm being run
from start to finish, resulting in absurd write traffic amplification.
With this patch, PMTU discovery stops as soon as "real" packets stop
flowing, and will be no more aggressive than the underlying traffic.

Furthermore, try_mtu() only runs if there is confirmed UDP
connectivity as per the UDP discovery mechanism. This prevents
unnecessary network chatter - previously, the PMTU discovery code
would send bursts of (potentially large) probe packets every second
even if there was nothing on the other side. With this patch, the
PMTU code only does that if something replied to the lightweight UDP
discovery pings.

These inefficiencies were made even worse when the node is not a
direct neighbour, as tinc will use PMTU discovery both on the
destination node *and* the relay. UDP discovery is more lightweight for
this purpose.

As a bonus, this code simplifies overall code somewhat - state is
easier to manage when code is run in predictable contexts as opposed
to "surprise callbacks". In addition, there is no need to call PMTU
discovery code outside of net_packet.c anymore, thereby simplifying
module boundaries.

9 years agoRemove PMTU discovery code redundant with UDP discovery.
Etienne Dechamps [Mon, 29 Dec 2014 16:11:04 +0000 (16:11 +0000)]
Remove PMTU discovery code redundant with UDP discovery.

This is a rewrite of the send_mtu_probe_handler() function to make it
focus on the actual discovery of PMTU. In particular, the PMTU
discovery code doesn't care about tunnel state anymore - it only cares
about doing the initial PMTU discovery, and once that's done, making
sure PMTU did not increase by checking it from time to time. All other
duties have already been rewritten in the UDP discovery code.

As a result, the send_mtu_probe_handler(), which previously implemented
a nightmarish state machine which was very difficult to follow and
understand, has been massively simplified. We moved from four persistent
states to only two - initial discovery and steady state.

Furthermore, a side effect is that network chatter is reduced: instead
of sending bursts of three minmtu-sized packets in the steady state,
there is only one such packet that's sent from the UDP discovery code.
However, that introduces a slight regression in the bandwidth estimation
code, which relies on three-packet bursts in order to function.
Considering that this estimation is extremely unreliable (in my
experience) and isn't relied on by anything, this seems like an
acceptable regression.

9 years agoMove responsibility for local discovery to UDP discovery.
Etienne Dechamps [Mon, 29 Dec 2014 15:40:55 +0000 (15:40 +0000)]
Move responsibility for local discovery to UDP discovery.

Since UDP discovery is the place where UDP feasibility is checked, it
makes sense to test for local connectivity as well. This was previously
done as part of PMTU discovery.

9 years agoAdd UDP discovery mechanism.
Etienne Dechamps [Mon, 29 Dec 2014 10:34:39 +0000 (10:34 +0000)]
Add UDP discovery mechanism.

This adds a new mechanism by which tinc can determine if a node is
reachable via UDP. The new mechanism is currently redundant with the
PMTU discovery mechanism - that will be fixed in a future commit.

Conceptually, the UDP discovery mechanism works similarly to PMTU
discovery: it sends UDP probes (of minmtu size, to make sure the tunnel
is fully usable), and assumes UDP is usable if it gets replies. It
assumes UDP is broken if too much time has passed since the last reply.

The big difference with the current PMTU discovery mechanism, however,
is that UDP discovery probes are only triggered as part of the
packet TX path (through try_tx()). This is quite interesting, because
it means tinc will never send UDP pings more often than normal packets,
and most importantly, it will automatically stop sending pings as soon
as packets stop flowing, thereby nicely reducing network chatter.

Of course, there are small drawbacks in some edge cases: for example,
if a node only sends one packet every minute to another node, these
packets will only be sent over TCP, because the interval between packets
is too long for tinc to maintain the UDP tunnel. I consider this a
feature, not a bug: I believe it is appropriate to use TCP in scenarios
where traffic is negligible, so that we don't pollute the network with
pings just to maintain a UDP tunnel that's seeing negligible usage.

9 years agoMove try_sptps() closer to try_tx().
Etienne Dechamps [Sun, 28 Dec 2014 17:29:03 +0000 (17:29 +0000)]
Move try_sptps() closer to try_tx().

This moves related functions together. try_tx() is at the right place
since its only caller is send_packet().

This is a pure cut-and-paste change. The reason it was not done in the
previous commit is because it would have made the diff harder to review.

9 years agoAdd the try_tx() function.
Etienne Dechamps [Sun, 28 Dec 2014 17:16:27 +0000 (17:16 +0000)]
Add the try_tx() function.

Currently, the TX path (starting from send_packet()) in tinc has three
responsabilities:

 - Making sure packets can be sent (e.g. fetching SPTPS keys);
 - Making sure they can be sent optimally (e.g. fetching non-SPTPS keys
   so that UDP can be used);
 - Sending the actual packet, if feasible.

The first two are closely related; the third one, however, can be
cleanly separated from the other two - meaning, we can loosen code
coupling between sending packets and "optimizing" the way packets are
sent. This will become increasingly important as future commits will
move more tunnel establishment and maintenance code into the TX path,
so we will benefit from a cleaner separation of concerns.

This is especially relevant because of the dual nature of the TX path
(SPTPS versus non-SPTPS), which can make things really complicated when
trying to share low-level code between both.

In this commit, code related to establishing or improving tunnels is
moved away from the core TX path by introducing the "try_*()" family of
function, of which try_sptps() already existed before this commit.

This is a pure refactoring; this commit shouldn't introduce any change
in behavior.

9 years agoClarify the send_mtu_probe() function.
Etienne Dechamps [Sun, 12 Oct 2014 18:44:33 +0000 (19:44 +0100)]
Clarify the send_mtu_probe() function.

This cleans up the PMTU probing function a little bit. It moves the
low-level sending of packets to a separate function, so that the code
reads naturally instead of using a weird for loop with "special
indexes". In addition, comments are moved inside the body of the
function for additional context.

This shouldn't introduce any change of behavior, except for local
discovery which has some minor logic fixes and which now always uses
small packets (16 bytes) because there's no need for a full-length
probe just to try the local network.

9 years agoFixes for bugs in src/Makefile.am and tincctl.c introduced by cfe9285adf391ab66faeb5d...
Guus Sliepen [Wed, 31 Dec 2014 23:52:39 +0000 (00:52 +0100)]
Fixes for bugs in src/Makefile.am and tincctl.c introduced by cfe9285adf391ab66faeb5def811fe08e47a221a.

9 years agoAdd missing nolegacy/crypto.c and prf.c.
Guus Sliepen [Tue, 30 Dec 2014 10:16:08 +0000 (11:16 +0100)]
Add missing nolegacy/crypto.c and prf.c.

9 years agoAllow tinc to be compiled without OpenSSL.
Guus Sliepen [Mon, 29 Dec 2014 21:57:18 +0000 (22:57 +0100)]
Allow tinc to be compiled without OpenSSL.

The option "--disable-legacy-protocol" was added to the configure
script. The new protocol does not depend on any external crypto
libraries, so when the option is used tinc is no longer linked to
OpenSSL's libcrypto.

9 years agoReleasing 1.1pre11. release-1.1pre11
Guus Sliepen [Sat, 27 Dec 2014 08:22:31 +0000 (09:22 +0100)]
Releasing 1.1pre11.

9 years agoAdd BroadcastSubnet and DeviceStandby options to the manual and completion.
Guus Sliepen [Sat, 27 Dec 2014 08:20:46 +0000 (09:20 +0100)]
Add BroadcastSubnet and DeviceStandby options to the manual and completion.

9 years agoBetter default paths for log and PID files on Windows.
Guus Sliepen [Sat, 27 Dec 2014 08:08:34 +0000 (09:08 +0100)]
Better default paths for log and PID files on Windows.

9 years agoRemove AES-GCM support.
Guus Sliepen [Fri, 26 Dec 2014 17:22:13 +0000 (18:22 +0100)]
Remove AES-GCM support.

9 years agoLinux doesn't like .PHONY .o files.
Guus Sliepen [Fri, 26 Dec 2014 17:12:28 +0000 (18:12 +0100)]
Linux doesn't like .PHONY .o files.

In order to please every OS, make version.c .PHONY again, and add an
empty rule to make version.c.

9 years agoWe don't depend on ECDH functions from OpenSSL anymore.
Guus Sliepen [Fri, 26 Dec 2014 16:53:40 +0000 (17:53 +0100)]
We don't depend on ECDH functions from OpenSSL anymore.

9 years agoBSD make doesn't like .PHONY .c files.
Guus Sliepen [Fri, 26 Dec 2014 14:58:28 +0000 (15:58 +0100)]
BSD make doesn't like .PHONY .c files.

It then thinks there should be a rule to make the .c file, which does
not exist of course. Luckily, we can tell it that version.o is .PHONY,
and this will still cause the .o file to be regenerated and linked into
the binaries everytime make is called.

9 years agoCheck whether res_init() really lives in libresolv.
Guus Sliepen [Fri, 26 Dec 2014 14:40:09 +0000 (15:40 +0100)]
Check whether res_init() really lives in libresolv.

On some platforms (Mac OS X for example), the res_init() function requires
linking with libresolv. On others (Linux, OpenBSD for example), res_init()
lives in libc.

9 years agoUpdate THANKS file.
Guus Sliepen [Fri, 26 Dec 2014 13:59:15 +0000 (14:59 +0100)]
Update THANKS file.

9 years agoAllow running tinc without RSA keys.
Guus Sliepen [Fri, 26 Dec 2014 13:38:01 +0000 (14:38 +0100)]
Allow running tinc without RSA keys.

This allows one to run tinc with only Ed25519 keys, forcing tinc to
always use the SPTPS protocol.

9 years agoMerge remote-tracking branch 'groxxda/gui-fixes' into 1.1
Guus Sliepen [Thu, 25 Dec 2014 17:13:24 +0000 (18:13 +0100)]
Merge remote-tracking branch 'groxxda/gui-fixes' into 1.1

9 years agoUse plain old PACKET for TCP packets sent directly to a neighbor.
Etienne Dechamps [Sun, 12 Oct 2014 11:14:46 +0000 (12:14 +0100)]
Use plain old PACKET for TCP packets sent directly to a neighbor.

Currently, when sending packets over TCP where the final recipient is
a node we have a direct metaconnection to, tinc first establishes a
SPTPS handshake between the two neighbors.

It turns out this SPTPS tunnel is not actually useful, because the
packet is only being sent over one metaconnection with no intermediate
nodes, and the metaconnection itself is already secured using a separate
SPTPS handshake.

Therefore it seems simpler and more efficient to simply send these
packets directly over the metaconnection itself without any additional
layer. This commits implements this solution without any changes to the
metaprotocol, since the appropriate message already exists: it's the
good old "plaintext" PACKET message.

This change brings two significant benefits:

- Packets to neighbors can be sent immediately - there is no initial
  delay and packet loss previously caused by the SPTPS handshake;

- Performance of sending packets to neighbors over TCP is greatly
  improved since the data only goes through one round of encryption
  instead of two.

Conflicts:
src/net_packet.c

9 years agoDon't spontaneously start SPTPS with neighbors.
Etienne Dechamps [Sun, 12 Oct 2014 10:41:08 +0000 (11:41 +0100)]
Don't spontaneously start SPTPS with neighbors.

Currently, when tinc establishes a metaconnection, it automatically
starts a VPN SPTPS tunnel with the other side of the metaconnection.

It is not clear what this is trying to accomplish. Having a
metaconnection with a node does not necessarily mean we're going to send
packets to that node. This patch removes this behavior, thereby
simplifying code paths and removing unnecessary network chatter.

Naturally, this introduces a slight delay (as well as at least one
initial packet loss) between the moment a metaconnection is established
and the moment VPN packets can be exchanged between the two nodes.
However this is no different to the non-neighbor case, so it makes
things more consistent and therefore easier to reason about.

9 years agoAdd a variable offset to vpn_packet_t, drop sptps_packet_t.
Guus Sliepen [Wed, 24 Dec 2014 21:23:24 +0000 (22:23 +0100)]
Add a variable offset to vpn_packet_t, drop sptps_packet_t.

The offset value indicates where the actual payload starts, so we can
process both legacy and SPTPS UDP packets without having to do casting
tricks and/or moving memory around.

9 years agoUse void pointers for opaque data blobs in the SPTPS code.
Guus Sliepen [Wed, 24 Dec 2014 21:15:40 +0000 (22:15 +0100)]
Use void pointers for opaque data blobs in the SPTPS code.

9 years agoFix memory leaks found by Valgrind.
Guus Sliepen [Wed, 24 Dec 2014 16:31:33 +0000 (17:31 +0100)]
Fix memory leaks found by Valgrind.

9 years agoDon't use myself->name in device_disable(), it's already freed.
Guus Sliepen [Wed, 24 Dec 2014 16:06:05 +0000 (17:06 +0100)]
Don't use myself->name in device_disable(), it's already freed.

9 years agoDon't pass uninitialized bytes to ioctl().
Guus Sliepen [Wed, 24 Dec 2014 15:59:08 +0000 (16:59 +0100)]
Don't pass uninitialized bytes to ioctl().

9 years agoAvoid using OpenSSL's random number functions.
Guus Sliepen [Wed, 24 Dec 2014 15:54:12 +0000 (16:54 +0100)]
Avoid using OpenSSL's random number functions.

9 years agoFix reception of SPTPS UDP packets.
Guus Sliepen [Sun, 14 Dec 2014 12:05:30 +0000 (13:05 +0100)]
Fix reception of SPTPS UDP packets.

Some bugs were introduced in 46fa12e666badb79e480c4b2399787551f8266d0.

9 years agoFix segfault when receiving UDP packets with an unknown source address.
Guus Sliepen [Sun, 14 Dec 2014 11:42:03 +0000 (12:42 +0100)]
Fix segfault when receiving UDP packets with an unknown source address.

9 years agoChanges that should have been in commit 46fa12e666badb79e480c4b2399787551f8266d0.
Guus Sliepen [Mon, 8 Dec 2014 07:43:15 +0000 (08:43 +0100)]
Changes that should have been in commit 46fa12e666badb79e480c4b2399787551f8266d0.

9 years agoMake UDP packet handling more efficient.
Guus Sliepen [Sun, 7 Dec 2014 23:58:09 +0000 (00:58 +0100)]
Make UDP packet handling more efficient.

Limit the amount of address/ID lookups to the minimum in all cases:

1) Legacy packets, need an address lookup.
2) Indirect SPTPS packets, need an address lookup + two ID lookups.
3) Direct SPTPS packets, need an ID or an address lookup.

So we start with an address lookup. If the source is an 1.1 node, we know it's an SPTPS packet,
and then the check for direct packets is a simple check if dstid is zero. If not, do the srcid and dstid
lookup. If the source is an 1.0 node, we don't have to do anything else.

If the address is unknown, we first check whether it's from a 1.1 node by assuming it has a valid srcid
and verifying the packet. If not, use the old try_harder().

9 years agoAvoid memmove() for legacy UDP packets.
Guus Sliepen [Sun, 7 Dec 2014 23:44:38 +0000 (00:44 +0100)]
Avoid memmove() for legacy UDP packets.

9 years agoCache node IDs in a hash table for faster lookups.
Guus Sliepen [Sun, 7 Dec 2014 21:11:37 +0000 (22:11 +0100)]
Cache node IDs in a hash table for faster lookups.

9 years agoAdd an explicit hash_delete() function.
Guus Sliepen [Sun, 7 Dec 2014 21:10:16 +0000 (22:10 +0100)]
Add an explicit hash_delete() function.

9 years agoBetter log messages when we already know the peer's key during an upgrade.
Guus Sliepen [Sun, 7 Dec 2014 20:42:20 +0000 (21:42 +0100)]
Better log messages when we already know the peer's key during an upgrade.

If the peer presents a different one from the one we already know, log
an error. Otherwise, log an informational message, and terminate in the
same way as we would if we didn't already have that key.

9 years agoTry handling the case when the first side knows the ecdsa key of
Sven-Haegar Koch [Fri, 5 Dec 2014 02:06:44 +0000 (03:06 +0100)]
Try handling the case when the first side knows the ecdsa key of
the second, but the second not the key of the first.
(And both have the experimental protocol enabled)

9 years agoLog an error message with the node's name when receiving bad SPTPS packets.
Guus Sliepen [Sun, 7 Dec 2014 16:25:30 +0000 (17:25 +0100)]
Log an error message with the node's name when receiving bad SPTPS packets.

The SPTPS code doesn't know about nodes, so when it logs an error about
a bad packet, it doesn't log which node it came from. So add a log
message with the node's name and hostname in receive_udppacket().

9 years agoCheck validity of Ed25519 key during an upgrade.
Guus Sliepen [Sun, 7 Dec 2014 16:20:18 +0000 (17:20 +0100)]
Check validity of Ed25519 key during an upgrade.

9 years agoDo not disconnect when no ecdsa key is known yet.
Sven-Haegar Koch [Fri, 5 Dec 2014 01:41:55 +0000 (02:41 +0100)]
Do not disconnect when no ecdsa key is known yet.

This is the normal case when we support the experimental protocol,
but the other side is a tinc 1.0 which does not.

9 years agoFix compiler warnings.
Guus Sliepen [Wed, 3 Dec 2014 13:51:45 +0000 (14:51 +0100)]
Fix compiler warnings.

9 years agoQuery the Linux device for its MAC address.
Etienne Dechamps [Sat, 4 Oct 2014 15:33:33 +0000 (16:33 +0100)]
Query the Linux device for its MAC address.

On Linux, tinc doesn't know the MAC address of the TAP device until the
first read. This means that if no packets are sent through the
interface, tinc won't be able to figure out which MAC address to tag
incoming packets with. As a result, it is impossible to receive any
packet until at least one packet has been sent.

When IPv6 is disabled Linux does not spontanously send any packets
when the interface comes up. At first users wonder why the node is not
responding to ICMP pings, and then as soon as at least one packet is
sent through the interface, pings mysteriously start working, resulting
in user confusion.

This change fixes that problem by making sure tinc is aware of the
device's MAC address even before the first packet is sent.

10 years ago tinc-gui: Don't assign broadcast subnets to any node, fix parsing of Edges, fix...
groxxda [Tue, 14 Oct 2014 20:18:56 +0000 (22:18 +0200)]
  tinc-gui: Don't assign broadcast subnets to any node, fix parsing of Edges, fix diplay of Subnet.weight.

10 years agoMake sure to discover MTU with relays.
Etienne Dechamps [Sat, 4 Oct 2014 14:01:11 +0000 (15:01 +0100)]
Make sure to discover MTU with relays.

Currently, when tinc sends UDP SPTPS datagrams through a relay, it
doesn't automatically start discovering PMTU with the relay. This means
that unless something else triggers PMTU discovery, tinc will keep using
TCP when sending packets through the relay.

This patches fixes the issue by explicitly establishing UDP tunnels with
relays.

10 years agoDon't send MTU probes to nodes we can't reach directly.
Etienne Dechamps [Sat, 4 Oct 2014 13:25:16 +0000 (14:25 +0100)]
Don't send MTU probes to nodes we can't reach directly.

Currently, we send MTU probes to each node we receive a key for, even if
we know we will never send UDP packets to that node because of
indirection. This commit disables MTU probing between nodes that have
direct communication disabled, otherwise MTU probes end up getting sent
through relays.

With the legacy protocol this was never a problem because we would never
request the key of a node with indirection enabled; with SPTPS this was
not a problem until we introduced relaying because send_sptps_data()
would simply ignore indirections, but this is not the case anymore.

Note that the fix is implemented in a quick and dirty way, by disabling
the call to send_mtu_probe() in ans_key_h(); this is not a clean fix
because there's no code to resume sending MTU probes in case the
indirection disappears because of a graph change.

10 years agoAdd UDP datagram relay support to SPTPS.
Etienne Dechamps [Sun, 28 Sep 2014 11:38:06 +0000 (12:38 +0100)]
Add UDP datagram relay support to SPTPS.

This commit changes the layout of UDP datagrams to include a 6-byte
destination node ID at the very beginning of the datagram (i.e. before
the source node ID and the seqno). Note that this only applies to SPTPS.

Thanks to this new field, it is now possible to send SPTPS datagrams to
nodes that are not the final recipient of the packets, thereby using
these nodes as relay nodes. Previously SPTPS was unable to relay packets
using UDP, and required a fallback to TCP if the final recipient could
not be contacted directly using UDP. In that sense it fixes a regression
that SPTPS introduced with regard to the legacy protocol.

This change also updates tinc's low-level routing logic (i.e.
send_sptps_data()) to automatically use this relaying facility if at all
possible. Specifically, it will relay packets if we don't have a
confirmed UDP link to the final recipient (but we have one with the next
hop node), or if IndirectData is specified. This is similar to how the
legacy protocol forwards packets.

When sending packets directly without any relaying, the sender node uses
a special value for the destination node ID: instead of setting the
field to the ID of the recipient node, it writes a zero ID instead. This
allows the recipient node to distinguish between a relayed packet and a
direct packet, which is important when determining the UDP address of
the sending node.

On the relay side, relay nodes will happily relay packets that have a
destination ID which is non-zero *and* is different from their own,
provided that the source IP address of the packet is known. This is to
prevent abuse by random strangers, since a node can't authenticate the
packets that are being relayed through it.

This change keeps the protocol number from the previous datagram format
change (source IDs), 17.4. Compatibility is still preserved with 1.0 and
with pre-1.1 releases. Note, however, that nodes running this code won't
understand datagrams sent from nodes that only use source IDs and
vice-versa (not that we really care).

There is one caveat: in the current state, there is no way for the
original sender to know what the PMTU is beyond the first hop, and
contrary to the legacy protocol, relay nodes can't apply MSS clamping
because they can't decrypt the relayed packets. This leads to
inefficient scenarios where a reduced PMTU over some link that's part of
the relay path will result in relays falling back to TCP to send packets
to their final destinations.

Another caveat is that once a packet gets sent over TCP, it will use
TCP over the entire path, even if it is technically possible to use UDP
beyond the TCP-only link(s).

Arguably, these two caveats can be fixed by improving the
metaconnection protocol, but that's out of scope for this change. TODOs
are added instead. In any case, this is no worse than before.

In addition, this change increases SPTPS datagram overhead by another
6 bytes for the destination ID, on top of the existing 6-byte overhead
from the source ID.

10 years agoPrepend source node ID information to UDP datagrams.
Etienne Dechamps [Sat, 27 Sep 2014 17:13:33 +0000 (18:13 +0100)]
Prepend source node ID information to UDP datagrams.

This commit changes the layout of UDP datagrams to include the 6-byte ID
(i.e. node name hash) of the node that crafted the packet at the very
beginning of the datagram (i.e. before the seqno). Note that this only
applies to SPTPS.

This is implemented at the lowest layer, i.e. in
handle_incoming_vpn_data() and send_sptps_data() functions. Source ID is
added and removed there, in such a way that the upper layers are unaware
of its presence.

This is the first stepping stone towards supporting UDP relaying in
SPTPS, by providing information about the original sender in the packet
itself. Nevertheless, even without relaying this commit already provides
a few benefits such as being able to reliably determine the source node
of a packet in the presence of an unknown source IP address, without
having to painfully go through all node keys. This makes tinc's behavior
much more scalable in this regard.

This change does not break anything with regard to the protocol: It
preserves compatibility with 1.0 and even with older pre-1.1 releases
thanks to a minor protocol version change (17.4). Source ID information
won't be included in packets sent to nodes with minor version < 4.

One drawback, however, is that this change increases SPTPS datagram
overhead by 6 bytes (the size of the source ID itself).

10 years agoChange vpn_packet_t::seqno from uint32_t to uint8_t[4].
Etienne Dechamps [Sat, 27 Sep 2014 12:34:56 +0000 (13:34 +0100)]
Change vpn_packet_t::seqno from uint32_t to uint8_t[4].

This is to make sure on-wire vpn_packet_t fields are always 1-byte
aligned, otherwise padding could get in the way.

10 years agoIntroduce node IDs.
Etienne Dechamps [Sun, 21 Sep 2014 17:17:02 +0000 (18:17 +0100)]
Introduce node IDs.

This introduces a new type of identifier for nodes, which complements
node names: node IDs. Node IDs are defined as the first 6 bytes of the
SHA-256 hash of the node name. They will be used in future code in lieu
of node names as unique node identifiers in contexts where space is at
a premium (such as VPN packets).

The semantics of node IDs is that they are supposed to be unique in a
tinc graph; i.e. two different nodes that are part of the same graph
should not have the same ID, otherwise things could break. This
solution provides this guarantee based on realistic probabilities:
indeed, according to the birthday problem, with a 48-bit hash, the
probability of at least one collision is 1e-13 with 10 nodes, 1e-11
with 100 nodes, 1e-9 with 1000 nodes and 1e-7 with 10000 nodes. Things
only start getting hairy with more than 1 million nodes, as the
probability gets over 0.2%.

10 years agoInvalidate UDP information on address changes.
Etienne Dechamps [Sun, 21 Sep 2014 14:44:59 +0000 (15:44 +0100)]
Invalidate UDP information on address changes.

Currently, when tinc receives an UDP packet from an unexpected address
(i.e. an address different from the node's current address), it just
updates its internal UDP address record and carries on like nothing
happened.

This poses two problems:

 - It assumes that the PMTU for the new address is the same as the
   old address, which is risky. Packets might get dropped if the PMTU
   turns out to be smaller (or if UDP communication on the new address
   turns out to be impossible).

 - Because the source address in the UDP packet itself is not
   authenticated (i.e. it can be forged by an attacker), this
   introduces a potential vulnerability by which an attacker with
   control over one link can trick a tinc node into dumping its network
   traffic to an arbitrary IP address.

This commit fixes the issue by invalidating UDP/PMTU state for a node
when its UDP address changes. This will trigger a temporary fallback
to indirect communication until we get confirmation via PMTU discovery
that the node is indeed sitting at the other end of the new UDP address.

10 years agoFix protocol version check for type 2 MTU probe replies.
Etienne Dechamps [Sat, 27 Sep 2014 16:51:33 +0000 (17:51 +0100)]
Fix protocol version check for type 2 MTU probe replies.

Currently tinc only uses type 2 MTU probe replies if the recipient uses
protocol version 17.3. It should of course support any higher minor
protocol version as well.

10 years agotinc-gui: Use /usr/bin/env to resolve path to python
Franz Pletz [Mon, 22 Sep 2014 20:43:15 +0000 (22:43 +0200)]
tinc-gui: Use /usr/bin/env to resolve path to python

10 years agoPreemptively mirror REQ_PUBKEY messages from nodes with unknown keys.
Etienne Dechamps [Sun, 21 Sep 2014 09:38:41 +0000 (11:38 +0200)]
Preemptively mirror REQ_PUBKEY messages from nodes with unknown keys.

In this commit, if a node receives a REQ_PUBKEY message from a node it
doesn't have the key for, it will send a REQ_PUBKEY message in return
*before* sending its own key.

The rationale is to prevent delays when establishing communication
between two nodes that see each other for the first time. These delays
are caused by the first SPTPS packet being dropped on the floor, as
shown in the following typical exchange:

node1: No Ed25519 key known for node2
REQ_PUBKEY ->
<- ANS_PUBKEY
node1: Learned Ed25519 public key from node2
REQ_SPTPS_START ->
node2: No Ed25519 key known for zyklos
<- REQ_PUBKEY
ANS_PUBKEY ->
node2: Learned Ed25519 public key from node1
-- 10-second delay --
node1: No key from node2 after 10 seconds, restarting SPTPS
REQ_SPTPS_START ->
<- SPTPS ->
node1: SPTPS key exchange with node2 succesful
node2: SPTPS key exchange with node1 succesful

With this patch, the following happens instead:

node1: No Ed25519 key known for node2
REQ_PUBKEY ->
node2: Preemptively requesting Ed25519 key for node1
<- REQ_PUBKEY
<- ANS_PUBKEY
ANS_PUBKEY ->
node2: Learned Ed25519 public key from node1
node1: Learned Ed25519 public key from node2
REQ_SPTPS_START ->
<- SPTPS ->
node1: SPTPS key exchange with node2 succesful
node2: SPTPS key exchange with node1 succesful

10 years agoFix default device path selection on BSD.
Etienne Dechamps [Sun, 21 Sep 2014 10:58:23 +0000 (12:58 +0200)]
Fix default device path selection on BSD.

Currently, if DeviceType = tap but Mode = router, the default
device path is /dev/tun0, which is wrong. This commit fixes that.

10 years agoIgnore the Interface option if device rename is impossible.
Etienne Dechamps [Sun, 21 Sep 2014 10:25:49 +0000 (11:25 +0100)]
Ignore the Interface option if device rename is impossible.

There are platforms on which it is impossible to rename the TUN/TAP
device. An example is Mac OS X (tuntapx). On these platforms,
specifying the Interface option will not rename the interface, but
the specified name will still be passed to tinc-up scripts and the
like, resulting in potential confusion for the user.

10 years agoFix default TAP device on Darwin.
Etienne Dechamps [Sun, 21 Sep 2014 10:14:19 +0000 (11:14 +0100)]
Fix default TAP device on Darwin.

On Darwin (tuntapx), the first TAP device is /dev/tap0, not /dev/tun0.

10 years agoFix wrong identifier in SO_NOSIGPIPE call.
Etienne Dechamps [Sat, 6 Sep 2014 17:16:46 +0000 (18:16 +0100)]
Fix wrong identifier in SO_NOSIGPIPE call.

f134bd0c9c2213fbbb3967f3d784759cb65e2c76 broke the Mac OS X build by
introducing a reference to an identifier, c, that doesn't exist.

10 years agoDon't enable the device if the reachable count is zero.
Etienne Dechamps [Sat, 6 Sep 2014 09:43:15 +0000 (10:43 +0100)]
Don't enable the device if the reachable count is zero.

A logic bug was introduced in bd451cfe1512fa69eac35a60dbe6df17bfc39154
in which running graph() several times with zero reachable nodes had
the effect of calling device_enable() (instead of keeping the device
disabled).

This results in weird behavior when DeviceStandby is enabled, especially
on Windows where calling device_enable() several times in a row corrupts
I/O structures for the device, rendering it unusable.

10 years agoFix undefined HOST_NAME_MAX on Windows.
Etienne Dechamps [Sun, 31 Aug 2014 12:59:30 +0000 (13:59 +0100)]
Fix undefined HOST_NAME_MAX on Windows.

The Windows build was broken by commit
826ad11e419db90b66b3f76a90b54df021bb39fc which introduced a dependency
on the HOST_NAME_MAX macro, which is not defined on Windows. According
to MSDN for gethostname(), the maximum length of the returned string
is 256 bytes (including the terminating null byte), so let's use that
as a fallback.

10 years agoRemove Google from the list of copyright owners.
Etienne Dechamps [Sat, 30 Aug 2014 09:57:57 +0000 (10:57 +0100)]
Remove Google from the list of copyright owners.

Google released copyright to me for my own contributions.

10 years agotincctl: Use replace_name to properly replace and validate input hostnames
William A. Kennington III [Mon, 25 Aug 2014 05:35:25 +0000 (22:35 -0700)]
tincctl: Use replace_name to properly replace and validate input hostnames

10 years agoutils: Refactor check_id out of protocol for global access
William A. Kennington III [Mon, 25 Aug 2014 04:55:42 +0000 (21:55 -0700)]
utils: Refactor check_id out of protocol for global access

10 years agoutils: Refactor get_name's functionality into util for global access
William A. Kennington III [Mon, 25 Aug 2014 02:49:27 +0000 (19:49 -0700)]
utils: Refactor get_name's functionality into util for global access

10 years agoClarify copyright ownership for code authored by Etienne Dechamps.
Etienne Dechamps [Sun, 17 Aug 2014 19:22:44 +0000 (20:22 +0100)]
Clarify copyright ownership for code authored by Etienne Dechamps.

10 years agocommandline.test: Adding test that fetching non-existing config setting really fails.
Sven-Haegar Koch [Thu, 7 Aug 2014 20:14:20 +0000 (22:14 +0200)]
commandline.test: Adding test that fetching non-existing config setting really fails.

10 years agoFix exit code of "tinc get".
Sven-Haegar Koch [Thu, 7 Aug 2014 21:01:05 +0000 (23:01 +0200)]
Fix exit code of "tinc get".

Successfully getting an existing variable ("tinc get name") should
not result in an error exitcode (1) from the tinc command.

This changes the result of test/commandline.test from FAIL to PASS.

10 years agoHandle TAP-Win32 immediate reads correctly.
Etienne Dechamps [Sat, 19 Jul 2014 17:11:42 +0000 (18:11 +0100)]
Handle TAP-Win32 immediate reads correctly.

The handling of TAP-Win32 virtual network device reads that complete
immediately (ReadFile() returns TRUE) is incorrect - instead of
starting a new read, tinc will continue listening for the overlapped
read completion event which will never fire. As a result, tinc stops
receiving packets on the interface.

10 years agoOnly read from TAP-Win32 if the device is enabled.
Etienne Dechamps [Sat, 19 Jul 2014 15:05:23 +0000 (16:05 +0100)]
Only read from TAP-Win32 if the device is enabled.

With newer TAP-Win32 versions (such as the experimental
tap-windows6 9.21.0), tinc is unable to read from the virtual network
device:

    Error while reading from (null) {23810A13-BCA9-44CE-94C6-9AEDFBF85736}: No such file or directory

This is because these new drivers apparently don't accept reads when
the device is not in the connected state (media status).

This commit fixes the issue by making sure we start reading no sooner
than when the device is enabled, and that we stop reading when the
device is disabled. This also makes the behavior somewhat cleaner,
because it doesn't make much sense to read from a disabled device
anyway.

10 years agoAdd a non-interactive mode to tinc commands.
Etienne Dechamps [Sun, 13 Jul 2014 14:54:34 +0000 (15:54 +0100)]
Add a non-interactive mode to tinc commands.

Some tinc commands, such as "tinc generate-keys", use the terminal to
ask the user for information. This can be bypassed by making sure
there is no terminal, which is trivial on *nix but might require
jumping through some hoops on Windows depending on how the command is
invoked.

This commit adds a --batch option that ensures tinc will never ask the
user for input, even if it is attached to a terminal.

10 years agoRevert "Use git description as the tinc version."
Guus Sliepen [Sat, 12 Jul 2014 20:51:37 +0000 (22:51 +0200)]
Revert "Use git description as the tinc version."

This reverts commit e024b7a2c50e23311834e6d180e5acc72783b339. Automatic version
number generation needs a little bit more work to get it working correctly in
all cases.

10 years agoMerge branch 'keysegfault' of https://github.com/dechamps/tinc into 1.1
Guus Sliepen [Sat, 12 Jul 2014 20:25:55 +0000 (22:25 +0200)]
Merge branch 'keysegfault' of https://github.com/dechamps/tinc into 1.1

10 years agoMerge branch 'tincstart' of https://github.com/dechamps/tinc into 1.1
Guus Sliepen [Sat, 12 Jul 2014 20:22:31 +0000 (22:22 +0200)]
Merge branch 'tincstart' of https://github.com/dechamps/tinc into 1.1

10 years agoMerge branch 'ctrl' of https://github.com/dechamps/tinc into 1.1
Guus Sliepen [Sat, 12 Jul 2014 20:21:01 +0000 (22:21 +0200)]
Merge branch 'ctrl' of https://github.com/dechamps/tinc into 1.1

10 years agoMerge branch 'winwarnings' of https://github.com/dechamps/tinc into 1.1
Guus Sliepen [Sat, 12 Jul 2014 20:19:45 +0000 (22:19 +0200)]
Merge branch 'winwarnings' of https://github.com/dechamps/tinc into 1.1

10 years agoVerify seqno early in sptps_verify_datagram().
Etienne Dechamps [Mon, 30 Jun 2014 13:03:17 +0000 (14:03 +0100)]
Verify seqno early in sptps_verify_datagram().

This is a slight optimization for sptps_verify_datagram(), which might
come in handy since this function is called in a loop via try_harder().

It turns out that since sptps_verify_datagram() doesn't update any
state, it doesn't matter in which order verifications are done. However,
it does affect performance since it's much cheaper to check the seqno
than to try to decrypt the packet.

Since this function is called with the wrong node most of the time, it
makes verification vastly faster for the majority of calls because the
seqno will be wrong in most cases.

10 years agoAdd documentation about using system-assigned ports.
Etienne Dechamps [Sun, 6 Jul 2014 10:34:57 +0000 (11:34 +0100)]
Add documentation about using system-assigned ports.

There are two caveats to be aware of which are documented in this
commit:

 - Because the system will likely assign different ports when binding
   several times to different address families, it is recommended to
   only use a single address family, otherwise other nodes will only
   get one port among the several that were assigned, possibly breaking
   communication.

 - AutoConnect won't work in this scenario, because it relies on the UDP
   port being the same as the TCP port, which is not the case when using
   system-assigned ports.

10 years agoImprove subprocess behavior in tinc start command.
Etienne Dechamps [Sat, 12 Jul 2014 17:53:25 +0000 (18:53 +0100)]
Improve subprocess behavior in tinc start command.

When invoking tincd, tinc start currently uses the execvp() function,
which doesn't behave well in a console as the console displays a new
prompt before the subprocess finishes (which makes me suspect the exit
value is not handled at all). This new code uses spawnvp() instead,
which seems like a better fit.

10 years agoFix "tinc start" on Windows when the path contains spaces.
Etienne Dechamps [Sat, 12 Jul 2014 17:37:56 +0000 (18:37 +0100)]
Fix "tinc start" on Windows when the path contains spaces.

When invoking "tinc start" with spaces in the path, the following
happens:

    > "c:\Program Files (x86)\tinc\tinc.exe" start
    c:\Program: unrecognized argument 'Files'
    Try `c:\Program --help' for more information.

This is caused by inconsistent handling of command line strings between
execvp() and the spawned process' CRT, as documented on MSDN:
http://msdn.microsoft.com/library/431x4c1w.aspx

10 years agoShutdown cleanly when receiving a Windows console shutdown request.
Etienne Dechamps [Sat, 12 Jul 2014 16:47:01 +0000 (17:47 +0100)]
Shutdown cleanly when receiving a Windows console shutdown request.

This commit makes tinc exit cleanly on Windows when hitting CTRL+C at
the console or when the user logs off. This change has no effect when
running tinc as a service.

10 years agoCheck if devops is valid before closing the device.
Etienne Dechamps [Sat, 12 Jul 2014 12:56:01 +0000 (13:56 +0100)]
Check if devops is valid before closing the device.

This fixes a segfault that occurs on exit if tinc fails before the
device is initialized (for example, if it fails to read the private
key).

10 years agoFix unsafe use of strncpy() and sprintf().
Guus Sliepen [Sat, 12 Jul 2014 12:35:29 +0000 (14:35 +0200)]
Fix unsafe use of strncpy() and sprintf().

The strncpy() problem was found by cppcheck.

10 years agoFix a potential file descriptor leak.
Guus Sliepen [Sat, 12 Jul 2014 12:34:39 +0000 (14:34 +0200)]
Fix a potential file descriptor leak.

Found by cppcheck.

10 years agoResolve KEY_EVENT conflict between Windows and ncurses.
Etienne Dechamps [Sat, 12 Jul 2014 12:32:23 +0000 (13:32 +0100)]
Resolve KEY_EVENT conflict between Windows and ncurses.

This fixes the following compiler warning when building for Windows:

In file included from top.c:24:0:
/usr/local/mingw/ncurses/include/curses.h:1478:0: error: "KEY_EVENT" redefined [-Werror]
 #define KEY_EVENT 0633  /* We were interrupted by an event */
 ^
In file included from /usr/share/mingw-w64/include/windows.h:74:0,
                 from /usr/share/mingw-w64/include/winsock2.h:23,
                 from have.h:46,
                 from system.h:26,
                 from top.c:20:
/usr/share/mingw-w64/include/wincon.h:101:0: note: this is the location of the previous definition
 #define KEY_EVENT 0x1
 ^

10 years agoRemove unused device stats variables.
Etienne Dechamps [Sat, 12 Jul 2014 12:27:05 +0000 (13:27 +0100)]
Remove unused device stats variables.

This removes a bunch of variables that are never actually used anywhere.

This fixes the following compiler warning when building for Windows:

mingw/device.c:46:17: error: ‘device_total_in’ defined but not used [-Werror=unused-variable]
 static uint64_t device_total_in = 0;
                  ^

10 years agoRemove unused variable in TAP-Win32 setup_device().
Etienne Dechamps [Sat, 12 Jul 2014 11:57:11 +0000 (12:57 +0100)]
Remove unused variable in TAP-Win32 setup_device().

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;
           ^

10 years agoFix callback signature for TAP-Win32 device_handle_read().
Etienne Dechamps [Sat, 12 Jul 2014 11:54:45 +0000 (12:54 +0100)]
Fix callback signature for TAP-Win32 device_handle_read().

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);

10 years agoRemove an unnecessary pointer dereference in execute_script().
Etienne Dechamps [Sat, 12 Jul 2014 11:52:25 +0000 (12:52 +0100)]
Remove an unnecessary pointer dereference in execute_script().

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++;
          ^

10 years agoOnly declare the origpriority variable if we support priority.
Etienne Dechamps [Sat, 12 Jul 2014 11:49:59 +0000 (12:49 +0100)]
Only declare the origpriority variable if we support priority.

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;
        ^

10 years agoReserve legacy active bit in connection_status_t.
Guus Sliepen [Sat, 12 Jul 2014 12:24:16 +0000 (14:24 +0200)]
Reserve legacy active bit in connection_status_t.

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.

10 years agoRemove redundant connection_t::status.active field.
Etienne Dechamps [Sat, 12 Jul 2014 10:57:03 +0000 (11:57 +0100)]
Remove redundant connection_t::status.active field.

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.

10 years agoDon't initialize outpkt to an unused value.
Etienne Dechamps [Sat, 12 Jul 2014 10:13:04 +0000 (11:13 +0100)]
Don't initialize outpkt to an unused value.

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/

10 years agoHandle the "no local address" case in send_sptps_data().
Etienne Dechamps [Sat, 12 Jul 2014 10:06:36 +0000 (11:06 +0100)]
Handle the "no local address" case in send_sptps_data().

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
415910897122da0073a862784d148802ca390020. 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/

10 years agoFix incorrect format qualifiers.
Guus Sliepen [Thu, 10 Jul 2014 20:41:01 +0000 (22:41 +0200)]
Fix incorrect format qualifiers.

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.