From 1e89a63f1638e43dee79afbb18d5f733b27d830b Mon Sep 17 00:00:00 2001 From: Etienne Dechamps Date: Sun, 17 May 2015 17:09:56 +0100 Subject: [PATCH] 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. --- src/net_packet.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) 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 { -- 2.20.1