==27135== Use of uninitialised value of size 8
==27135== at 0x57BE17B: BN_num_bits_word (in /usr/lib/libcrypto.so.1.0.0)
==27135== by 0x57BE205: BN_num_bits (in /usr/lib/libcrypto.so.1.0.0)
==27135== by 0x57BADF7: BN_div (in /usr/lib/libcrypto.so.1.0.0)
==27135== by 0x57C48FC: BN_mod_inverse (in /usr/lib/libcrypto.so.1.0.0)
==27135== by 0x57C3647: BN_BLINDING_create_param (in /usr/lib/libcrypto.so.1.0.0)
==27135== by 0x5812D44: RSA_setup_blinding (in /usr/lib/libcrypto.so.1.0.0)
==27135== by 0x58095CB: rsa_get_blinding (in /usr/lib/libcrypto.so.1.0.0)
==27135== by 0x580A64F: RSA_eay_private_decrypt (in /usr/lib/libcrypto.so.1.0.0)
==27135== by 0x4E5D9BC: rsa_private_decrypt (rsa.c:97)
==27135== by 0x4E51E1B: metakey_h (protocol_auth.c:524)
==27135== by 0x4E505FD: receive_request (protocol.c:136)
==27135== by 0x4E46002: receive_meta (meta.c:290)
==27135== Uninitialised value was created by a heap allocation
==27135== at 0x4C29F90: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==27135== by 0x575DCD7: CRYPTO_malloc (in /usr/lib/libcrypto.so.1.0.0)
==27135== by 0x57C24E1: BN_rand (in /usr/lib/libcrypto.so.1.0.0)
==27135== by 0x57C216F: bn_rand_range (in /usr/lib/libcrypto.so.1.0.0)
==27135== by 0x57C3630: BN_BLINDING_create_param (in /usr/lib/libcrypto.so.1.0.0)
==27135== by 0x5812D44: RSA_setup_blinding (in /usr/lib/libcrypto.so.1.0.0)
==27135== by 0x58095CB: rsa_get_blinding (in /usr/lib/libcrypto.so.1.0.0)
==27135== by 0x580A64F: RSA_eay_private_decrypt (in /usr/lib/libcrypto.so.1.0.0)
==27135== by 0x4E5D9BC: rsa_private_decrypt (rsa.c:97)
==27135== by 0x4E51E1B: metakey_h (protocol_auth.c:524)
==27135== by 0x4E505FD: receive_request (protocol.c:136)
==27135== by 0x4E46002: receive_meta (meta.c:290)
It is not unusual for tinc to receive SPTPS packets to be relayed to
nodes that just became unreachable, due to state propagation delays in
the metagraph.
Unfortunately, the current code doesn't handle that situation correctly,
and still tries to relay the packet to the unreachable node. This
typically ends up segfaulting.
This commit fixes the issue by checking for reachability before relaying
the packet.
timeout_handler() calls try_tx(c->node) when c->edge exists.
Unfortunately, the existence of c->edge is not enough to conclude that
the node is reachable.
In fact, during connection establishment, there is a short period of
time where we create an edge for the node at the other end of the
metaconnection, but we don't have one from the other side yet.
Unfortunately, if timeout_handler() runs during that short time
window, it will call try_tx() on an unreachable node, which makes
things explode because that function is not prepared to handle that
case.
A typical symptom of this race condition is a hard SEGFAULT while trying
to send packets using metaconnections that don't exist, due to
n->nexthop containing garbage.
This patch fixes the issue by making try_tx() check for reachability,
and then making all code paths use try_tx() instead of the more
specialized methods so that they go through the check.
This regression was introduced in
eb7a0db18e.
try_tx_sptps() gives up on UDP communication if the recipient doesn't
support relaying. This is too restrictive - we only need the other node
to support relaying if we actually want to relay through them. If the
packet is sent directly, it's fine to send it to an old pre-node-IDs
tinc-1.1 node.
Currently, tinc tries to parse node IDs for all SPTPS packets, including
ones sent from older, pre-node-IDs tinc-1.1 nodes, and therefore doesn't
recognize packets from these nodes. This commit fixes that.
It also makes code slightly clearer by reducing the amount of fiddling
around packet offset/length.
A condition in try_harder() is always evaluating to false when talking
to a SPTPS node because n->status.validkey_in is always false in that
case. Fix the condition so that the SPTPS status is correctly checked.
This prevented recent tinc-1.1 nodes from talking to older, pre-node-ID
tinc-1.1 nodes.
The regression was introduced in
6056f1c13b.
There are a number of ways a SPTPS tunnel can get into a corrupt state.
For example, during key regeneration, the KEX and SIG messages from
other nodes might arrive out of order, which confuses the hell out of
the SPTPS code. Another possible scenario is not noticing another node
crashed and restarted because there was no point in time where the node
was seen completely disconnected from *all* nodes; this could result in
using the wrong (old) key. There are probably other scenarios which have
not even been considered yet. Distributed systems are hard.
When SPTPS got confused by a packet, it used to crash the entire
process; fortunately that was fixed by commit
2e7f68ad2b. However, the error handling
(or lack thereof) leaves a lot to be desired. Currently, when SPTPS
encounters an error when receiving a packet, it just shrugs it off and
continues as if nothing happened. The problem is, sometimes getting
receive errors mean the tunnel is completely stuck and will not recover
on its own. In that case, the node will become unreachable - possibly
indefinitely.
The goal of this commit is to improve SPTPS error handling by taking
proactive action when an incoming packet triggers a failure, which is
often an indicator that the tunnel is stuck in some way. When that
happens, we simply restart SPTPS entirely, which should make the tunnel
recover quickly.
To prevent "storms" where two buggy nodes flood each other with invalid
packets and therefore spend all their time negotiating new tunnels, we
limit the frequency at which tunnel restarts happen to ten seconds.
It is likely this commit will solve the "Invalid KEX record length
during key regeneration" issue that has been seen in the wild. It is
difficult to be sure though because we do not have a full understanding
of all the possible conditions that can trigger this problem.
Commit 10c1f60c64 introduced a mechanism
by which a packet received by REQ_KEY could continue its journey over
UDP. This was based on the assumption that REQ_KEY messages would never
be used for handshake packets (which should never be sent over UDP,
because SPTPS currently doesn't handle lost handshake packets very
well).
Unfortunately, there is one case where handshake packets are sent using
REQ_KEY: when regenerating the SPTPS key for a pre-established channel.
With the current code, such packets risk getting relayed over UDP.
When processing a REQ_KEY message, it is impossible for the receiving
end to distinguish between a data SPTPS packet and a handshake packet,
because this information is stored in the type field which is encrypted
with the end-to-end key.
This commit fixes the issue by making tinc use ANS_KEY for all SPTPS
handshake messages. This works because ANS_KEY messages are never
forwarded using the SPTPS relay mechanisms, therefore they are
guaranteed to stick to TCP.
Currently, SPTPS packets are transported over TCP metaconnections using
extended REQ_KEY requests, in order for the packets to pass through
tinc-1.0 nodes unaltered. Unfortunately, this method presents two
significant downsides:
- An already encrypted SPTPS packet is decrypted and then encrypted
again every time it passes through a node, since it is transported
over the SPTPS channels of the metaconnections. This
double-encryption is unnecessary and wastes CPU cycles.
- More importantly, the only way to transport binary data over
standard metaconnection messages such as REQ_KEY is to encode it
in base64, which has a 33% encoding overhead. This wastes 25% of the
network bandwidth.
This commit introduces a new protocol message, SPTPS_PACKET, which can
be used to transport SPTPS packets over a TCP metaconnection in an
efficient way. The new message is appropriately protected through a
minor protocol version increment, and extended REQ_KEY messages are
still used with nodes that do not support the new message, as well as
for the intial handshake packets, for which efficiency is not a concern.
The way SPTPS_PACKET works is very similar to how the traditional PACKET
message works: after the SPTPS_PACKET message, the raw binary packet is
sent directly over the metaconnection. There is one important
difference, however: in the case of SPTPS_PACKET, the packet is sent
directly over the TCP stream completely bypassing the SPTPS channel of
the metaconnection itself for maximum efficiency. This is secure because
the SPTPS packet that is being sent is already encrypted with an
end-to-end key.
REQ_SPTPS implies the message has an ANS_ counterpart (like REQ_KEY,
ANS_KEY), but it doesn't. Therefore dropping the REQ_ seems more
appropriate, and we add a _PACKET suffix to reduce the likelihood of
naming conflicts.
net_packet doesn't actually use send_sptps_data(); it only uses
send_sptps_data_priv(). In addition, the only user of send_sptps_data()
is protocol_key. Therefore it makes sense to expose
send_sptps_data_priv() directly, and move send_sptps_data() (which is
basically just boilerplate) as a local function in protocol_key.
Currently, when relaying SPTPS UDP packets, the code uses the direct
sender as the originator, instead of preserving the original source ID.
This wouldn't cause any issues in most cases because the originator and
the sender are the same in simple one-hop relay chains, but this will
break as soon as there is more than one relay.
Unfortunately, glibc assumes that /etc/resolv.conf is a static file that
never changes. Even on servers, /etc/resolv.conf might be a dynamically
generated file, and we never know when it changes. So just call
res_init() every time, so glibc uses up-to-date nameserver information.
Conflicts:
src/have.h
src/net.c
src/net_setup.c
Average RTT can be used to update edge weight and propagate it to the network.
tinc dump edges has been also extended to give the current RTT.
New edge weight will change only if the config has EdgeUpdateInterval set to other value than 0.
- Ignore local configuration for editors
- Extended manpage with informations about EdgeUpdateInterval
- Added clone_edge and fixed potential segfault when b->from not defined
- Compute avg_rtt based on the time values we got back in PONG
- Add avg_rtt on dump edge
- Send current time on PING and return it on PONG
- Changed last_ping_time to struct timeval
- Extended edge_t with avg_rtt
As a rule, it seems reasonable to make sure that tinc operates correctly
on at least 1G links, since these are pretty common. However, I have
observed replay window issues when operating at speeds of 600 Mbit/s and
above, especially when the receiving end is a Windows system (not sure
why). This commit increases the default so that this won't occur on
fresh setups.
Ironically, commit 0f8e2cc78c introduced
a regression on its own, since it accidently removed a return statement
that prevented try_tx_sptps() from sending UDP/MTU probes to nodes that
are past static relays.
In this commit, nodes use MTU_INFO messages to provide MTU information.
The issue this code is meant to address is the non-trivial problem of
finding the proper MTU when UDP SPTPS relays are involved. Currently,
tinc has no idea what the MTU looks like beyond the first relay, and
will arbitrarily use the first relay's MTU as the limit. This will fail
miserably if the MTU decreases after the first relay, forcing relays to
fall back to TCP. More generally, one should keep in mind that relay
paths can be arbitrarily complex, resulting in packets taking "epic
journeys" through the graph, switching back and forth between UDP (with
variable MTUs) and TCP multiple times along the path.
A solution that was considered consists in sending standard MTU probes
through the relays. This is inefficient (if there are 3 nodes on one
side of relay and 3 nodes on the other side, we end up with 3*3=9 MTU
discoveries taking place at the same time, while technically only
3+3=6 are needed) and would involve eyebrow-raising behaviors such as
probes being sent over TCP.
This commit implements an alternative solution, which consists in
the packet receiver sending MTU_INFO messages to the packet sender.
The message contains an MTU value which is set to maximum when the
message is originally sent. The message gets altered as it travels
through the metagraph, such that when the message arrives to the
destination, the MTU value contained in the message can be used to
send packets while making sure no relays will be forced to fall back to
TCP to deliver them.
The operating principles behind such a protocol message are similar to
how the UDP_INFO message works, but there is a key difference that
prevents us from simply reusing the same message: the UDP_INFO message
only cares about relay-to-relay links (i.e. it is sent between static
relays and the information it contains only makes sense between two
adjacent static relays), while the MTU_INFO cares about the end-to-end
MTU, including the entire relay path. Therefore, UDP_INFO messages stop
when they encounter static relays, while MTU_INFO messages don't stop
until they get to the original packet sender.
Note that, technically, the MTU that is obtained through this mechanism
can be slightly pessimistic, because it can be lowered by an
intermediate node that is not being used as a relay. Since nodes have no
way of knowing whether they'll be used as dynamic relays or not (and
have no say in the matter), this is not a trivial problem. That said,
this is highly unlikely to result in noticeable issues in realistic
scenarios.
In this commit, nodes use UDP_INFO messages to provide UDP address
information. The basic principle is that the node that receives packets
sends UDP_INFO messages to the node that's sending the packets. The
message originally contains no address information, and is (hopefully)
updated with relevant address information as it gets relayed through the
metagraph - specifically, each intermediate node will update the message
with its best guess as to what the address is while forwarding it.
When a node receives an UDP_INFO message, and it doesn't have a
confirmed UDP tunnel with the originator node, it will update its
records with the new address for that node, so that it always has the
best possible guess as to how to reach that node. This applies to the
destination node of course, but also to any intermediate nodes, because
there's no reason they should pass on the free intel, and because it
results in nice behavior in the presence of relay chains (multiple nodes
in a path all trying to reach the same destination).
If, on the other hand, the node does have a confirmed UDP tunnel, it
will ignore the address information contained in the message.
In all cases, if the node that receives the message is not the
destination node specified in the message, it will forward the message
but not before overriding the address information with the one from its
own records. If the node has a confirmed UDP tunnel, that means the
message is updated with the address of the confirmed tunnel; if not,
the message simply reflects the records of the intermediate node, which
just happen to be the contents of the UDP_INFO message it just got, so
it's simply forwarded with no modification.
This is similar to the way ANS_KEY messages are currently
overloaded to provide UDP address information, with two differences:
- UDP_INFO messages are sent way more often than ANS_KEY messages,
thereby keeping the address information fresh. Previously, if the UDP
situation were to change after the ANS_KEY message was sent, the
sender would virtually never get the updated information.
- Once a node puts address information in an ANS_KEY message, it is
never changed again as the message travels through the metagraph; in
contrast, UDP_INFO messages behave the opposite way, as they get
rewritten every time they travel through a node with a confirmed UDP
tunnel. The latter behavior seems more appropriate because UDP tunnel
information becomes more relevant as it moves closer to the
destination node. The ANS_KEY behavior is not satisfactory in some
cases such as multi-layered graphs where the first hop is located
before a NAT.
Ultimately, the rationale behind this whole process is to improve UDP
hole punching capabilities when port translation is in effect, and more
generally, to make tinc more reliable in (very) hostile network
conditions (such as multi-layered NAT).
Refactoring commit 81578484dc seems to
have introduced a regression as it moved discovery code away from
send_sptps_data_priv() and within send_packet(). The issue is,
send_packet() is not called when the node is simply relaying an UDP
SPTPS packet: indeed, send_sptps_data_priv() is called directly from
handle_incoming_vpn_data() in that case.
As a result, try_tx_sptps() is not called in the relaying case, which in
practice means that a relay doesn't initiate UDP/MTU discovery with the
next relay (unless some other activity compels it to do so). This can
result in packets getting sent over TCP instead of UDP from the relay.
Refactoring commit 0e65326047 broke UDP
SPTPS relaying by accidently removing try_tx_sptps() logic related to
establishing connectivity to so-called "dynamic" relays (i.e. relays
that are not specified by IndirectData configuration statements, but
are used on-the-fly to circumvent loss of direct UDP connectivity).
Specifically, the TX path was not trying to establish a tunnel to
dynamic relays (nexthop) anymore. This meant that MTU was not being
discovered with dynamic relays, which basically meant that all packets
being sent to dynamic relays went over TCP, thereby defeating the whole
purpose of SPTPS UDP relaying.
Note that this bug could easily go unnoticed if a tunnel was established
with the dynamic tunnel for some other reason (i.e. exchanging actual
data packets with the relay node).
When no UDP communication has been done yet, tinc establishes a guess
for the UDP address+port of each node. However, when there are multiple nodes
behind a NAT, tinc will guess the exact same address+port combination
for them, because it doesn't know about the NAT mappings yet. So when
receiving a packet, don't trust that guess unless we have confirmed UDP
communication.
This ensures try_harder() is called in such cases. However, this
function was actually very inefficient, trying to verify packets
multiple times for nodes with multiple edges. Only call try_mac() at
most once per node.
If we receive any traffic from another node, we periodically send back a
gratuitous type 2 probe reply with the maximum received packet length.
On the other node, this causes the udp and perhaps mtu probe timers to
be reset, so it does not need to send a probe request. Gratuitous probe
replies from another node also count as received traffic for this
purpose, so for nodes that also have a meta-connection, UDP keepalive
packets in principle can now solely be type 2 replies. This reduces the
amount of probe traffic even more.
To work, gratuitous replies should be sent slightly more often than
udp_discovery_keepalive_interval, so probe requests won't be triggered.
This also means that the timer resolution must be smaller than the
difference between the two, and at the moment it's kind of a hack.
When we have fixed the PMTU, n->mtuprobes == -1. When we send MTU probes
when mtuprobes == -1, decrease mtuprobes, and reset it back to -1 in
mtu_probe_h(). If mtuprobes < -1, send MTU probes every second, until
mtuprobes <= -4, in which case we will restart MTU discovery.
This is not working at all anymore. Just remove it, and we'll do another
attempt at RTT, bandwidth and packet loss estimation after the new
probing code stabilizes.
We are trying to decouple UDP probing from MTU probing, so only send
very small packets during UDP probing. This significantly reduces the
amount of traffic sent (54 to 67 bytes per probe instead of 1500 bytes).
This means the MTU probing code takes over sending PMTU sized probes,
but this commit does not take care of detecting PMTU decreases.
