From: Etienne Dechamps <etienne@edechamps.fr>
Date: Sun, 17 May 2015 16:09:56 +0000 (+0100)
Subject: Prevent SPTPS key regeneration packets from entering an UDP relay path.
X-Git-Tag: release-1.1pre12~158
X-Git-Url: https://git.tinc-vpn.org/git/browse?a=commitdiff_plain;h=1e89a63f1638e43dee79afbb18d5f733b27d830b;p=tinc

Prevent SPTPS key regeneration packets from entering an UDP relay path.

Commit 10c1f60c643607d9dafd79271c3475cddf81e903 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.
---

diff --git a/src/net_packet.c b/src/net_packet.c
index f734af29..92a3eb53 100644
--- a/src/net_packet.c
+++ b/src/net_packet.c
@@ -735,7 +735,7 @@ bool send_sptps_data(node_t *to, node_t *from, int type, const void *data, size_
 	/* Send it via TCP if it is a handshake packet, TCPOnly is in use, this is a relay packet that the other node cannot understand, or this packet is larger than the MTU. */
 
 	if(type == SPTPS_HANDSHAKE || tcponly || (!direct && !relay_supported) || (type != PKT_PROBE && (len - SPTPS_DATAGRAM_OVERHEAD) > relay->minmtu)) {
-		if((from != myself || to->status.validkey) && (to->nexthop->connection->options >> 24) >= 7) {
+		if(type != SPTPS_HANDSHAKE && (to->nexthop->connection->options >> 24) >= 7) {
 			char buf[len + sizeof to->id + sizeof from->id]; char* buf_ptr = buf;
 			memcpy(buf_ptr, &to->id, sizeof to->id); buf_ptr += sizeof to->id;
 			memcpy(buf_ptr, &from->id, sizeof from->id); buf_ptr += sizeof from->id;
@@ -746,9 +746,10 @@ bool send_sptps_data(node_t *to, node_t *from, int type, const void *data, size_
 
 		char buf[len * 4 / 3 + 5];
 		b64encode(data, buf, len);
-		/* If no valid key is known yet, send the packets using ANS_KEY requests,
-		   to ensure we get to learn the reflexive UDP address. */
-		if(from == myself && !to->status.validkey) {
+		/* If this is a handshake packet, use ANS_KEY instead of REQ_KEY, for two reasons:
+		    - We don't want intermediate nodes to switch to UDP to relay these packets;
+		    - ANS_KEY allows us to learn the reflexive UDP address. */
+		if(type == SPTPS_HANDSHAKE) {
 			to->incompression = myself->incompression;
 			return send_request(to->nexthop->connection, "%d %s %s %s -1 -1 -1 %d", ANS_KEY, from->name, to->name, buf, to->incompression);
 		} else {