diff --git a/src/net_packet.c b/src/net_packet.c index 70eac2e8..f9cec704 100644 --- a/src/net_packet.c +++ b/src/net_packet.c @@ -289,7 +289,14 @@ static bool receive_udppacket(node_t *n, vpn_packet_t *inpkt) { n->status.udppacket = false; if(!result) { - logger(DEBUG_TRAFFIC, LOG_ERR, "Got bad packet from %s (%s)", n->name, n->hostname); + /* Uh-oh. It might be that the tunnel is stuck in some corrupted state, + so let's restart SPTPS in case that helps. But don't do that too often + to prevent storms, and because that would make life a little too easy + for external attackers trying to DoS us. */ + if(n->last_req_key < now.tv_sec - 10) { + logger(DEBUG_PROTOCOL, LOG_ERR, "Failed to decode raw TCP packet from %s (%s), restarting SPTPS", n->name, n->hostname); + send_req_key(n); + } return false; } return true; @@ -465,11 +472,17 @@ bool receive_tcppacket_sptps(connection_t *c, const char *data, int len) { /* The packet is for us */ - if(!from->status.validkey) { - logger(DEBUG_PROTOCOL, LOG_ERR, "Got SPTPS packet from %s (%s) but we don't have a valid key yet", from->name, from->hostname); + if(!sptps_receive_data(&from->sptps, data, len)) { + /* Uh-oh. It might be that the tunnel is stuck in some corrupted state, + so let's restart SPTPS in case that helps. But don't do that too often + to prevent storms. */ + if(from->last_req_key < now.tv_sec - 10) { + logger(DEBUG_PROTOCOL, LOG_ERR, "Failed to decode raw TCP packet from %s (%s), restarting SPTPS", from->name, from->hostname); + send_req_key(from); + } return true; } - sptps_receive_data(&from->sptps, data, len); + send_mtu_info(myself, from, MTU); return true; } @@ -736,7 +749,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; @@ -747,9 +760,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 { diff --git a/src/netutl.c b/src/netutl.c index 701a4309..2eebb644 100644 --- a/src/netutl.c +++ b/src/netutl.c @@ -121,7 +121,10 @@ char *sockaddr2hostname(const sockaddr_t *sa) { char port[NI_MAXSERV] = "unknown"; int err; - if(sa->sa.sa_family == AF_UNKNOWN) { + if(sa->sa.sa_family == AF_UNSPEC) { + xasprintf(&str, "unspec port unspec"); + return str; + } else if(sa->sa.sa_family == AF_UNKNOWN) { xasprintf(&str, "%s port %s", sa->unknown.address, sa->unknown.port); return str; } diff --git a/src/protocol_edge.c b/src/protocol_edge.c index 21e431ae..d9475c3d 100644 --- a/src/protocol_edge.c +++ b/src/protocol_edge.c @@ -37,19 +37,26 @@ bool send_add_edge(connection_t *c, const edge_t *e) { bool x; char *address, *port; - char *local_address, *local_port; sockaddr2str(&e->address, &address, &port); - sockaddr2str(&e->local_address, &local_address, &local_port); - x = send_request(c, "%d %x %s %s %s %s %x %d %s %s", ADD_EDGE, rand(), - e->from->name, e->to->name, address, port, - e->options, e->weight, local_address, local_port); + if(e->local_address.sa.sa_family) { + char *local_address, *local_port; + sockaddr2str(&e->local_address, &local_address, &local_port); + + x = send_request(c, "%d %x %s %s %s %s %x %d %s %s", ADD_EDGE, rand(), + e->from->name, e->to->name, address, port, + e->options, e->weight, local_address, local_port); + free(local_address); + free(local_port); + } else { + x = send_request(c, "%d %x %s %s %s %s %x %d", ADD_EDGE, rand(), + e->from->name, e->to->name, address, port, + e->options, e->weight); + } free(address); free(port); - free(local_address); - free(local_port); return x; } diff --git a/src/protocol_key.c b/src/protocol_key.c index 040d3f47..eb64711d 100644 --- a/src/protocol_key.c +++ b/src/protocol_key.c @@ -36,6 +36,7 @@ static bool mykeyused = false; void send_key_changed(void) { +#ifndef DISABLE_LEGACY send_request(everyone, "%d %x %s", KEY_CHANGED, rand(), myself->name); /* Immediately send new keys to directly connected nodes to keep UDP mappings alive */ @@ -43,6 +44,7 @@ void send_key_changed(void) { for list_each(connection_t, c, connection_list) if(c->edge && c->node && c->node->status.reachable && !c->node->status.sptps) send_ans_key(c->node); +#endif /* Force key exchange for connections using SPTPS */ @@ -107,9 +109,6 @@ bool send_req_key(node_t *to) { return true; } - if(to->sptps.label) - logger(DEBUG_ALWAYS, LOG_DEBUG, "send_req_key(%s) called while sptps->label != NULL!", to->name); - char label[25 + strlen(myself->name) + strlen(to->name)]; snprintf(label, sizeof label, "tinc UDP key expansion %s %s", myself->name, to->name); sptps_stop(&to->sptps); @@ -148,11 +147,16 @@ static bool req_key_ext_h(connection_t *c, const char *request, node_t *from, no try_tx(to, true); } else { /* The packet is for us */ - if(!from->status.validkey) { - logger(DEBUG_PROTOCOL, LOG_ERR, "Got SPTPS_PACKET from %s (%s) but we don't have a valid key yet", from->name, from->hostname); + if(!sptps_receive_data(&from->sptps, buf, len)) { + /* Uh-oh. It might be that the tunnel is stuck in some corrupted state, + so let's restart SPTPS in case that helps. But don't do that too often + to prevent storms. */ + if(from->last_req_key < now.tv_sec - 10) { + logger(DEBUG_PROTOCOL, LOG_ERR, "Failed to decode TCP packet from %s (%s), restarting SPTPS", from->name, from->hostname); + send_req_key(from); + } return true; } - sptps_receive_data(&from->sptps, buf, len); send_mtu_info(myself, from, MTU); } @@ -428,9 +432,18 @@ bool ans_key_h(connection_t *c, const char *request) { if(from->status.sptps) { char buf[strlen(key)]; int len = b64decode(key, buf, strlen(key)); - - if(!len || !sptps_receive_data(&from->sptps, buf, len)) - logger(DEBUG_ALWAYS, LOG_ERR, "Error processing SPTPS data from %s (%s)", from->name, from->hostname); + if(!len || !sptps_receive_data(&from->sptps, buf, len)) { + /* Uh-oh. It might be that the tunnel is stuck in some corrupted state, + so let's restart SPTPS in case that helps. But don't do that too often + to prevent storms. + Note that simply relying on handshake timeout is not enough, because + that doesn't apply to key regeneration. */ + if(from->last_req_key < now.tv_sec - 10) { + logger(DEBUG_PROTOCOL, LOG_ERR, "Failed to decode handshake TCP packet from %s (%s), restarting SPTPS", from->name, from->hostname); + send_req_key(from); + } + return true; + } if(from->status.validkey) { if(*address && *port) { diff --git a/src/sptps.c b/src/sptps.c index 1ff0ed67..17a6ada9 100644 --- a/src/sptps.c +++ b/src/sptps.c @@ -449,6 +449,7 @@ static bool sptps_receive_data_datagram(sptps_t *s, const char *data, size_t len uint32_t seqno; memcpy(&seqno, data, 4); seqno = ntohl(seqno); + data += 4; len -= 4; if(!s->instate) { if(seqno != s->inseqno) @@ -456,37 +457,39 @@ static bool sptps_receive_data_datagram(sptps_t *s, const char *data, size_t len s->inseqno = seqno + 1; - uint8_t type = data[4]; + uint8_t type = *(data++); len--; + if(type != SPTPS_HANDSHAKE) return error(s, EIO, "Application record received before handshake finished"); - return receive_handshake(s, data + 5, len - 5); + return receive_handshake(s, data, len); } // Decrypt char buffer[len]; - size_t outlen; - - if(!chacha_poly1305_decrypt(s->incipher, seqno, data + 4, len - 4, buffer, &outlen)) + if(!chacha_poly1305_decrypt(s->incipher, seqno, data, len, buffer, &outlen)) return error(s, EIO, "Failed to decrypt and verify packet"); if(!sptps_check_seqno(s, seqno, true)) return false; // Append a NULL byte for safety. - buffer[len - 20] = 0; + buffer[outlen] = 0; - uint8_t type = buffer[0]; + data = buffer; + len = outlen; + + uint8_t type = *(data++); len--; if(type < SPTPS_HANDSHAKE) { if(!s->instate) return error(s, EIO, "Application record received before handshake finished"); - if(!s->receive_record(s->handle, type, buffer + 1, len - 21)) + if(!s->receive_record(s->handle, type, data, len)) return false; } else if(type == SPTPS_HANDSHAKE) { - if(!receive_handshake(s, buffer + 1, len - 21)) + if(!receive_handshake(s, data, len)) return false; } else { return error(s, EIO, "Invalid record type %d", type);