From: Guus Sliepen Date: Sun, 17 May 2015 19:07:45 +0000 (+0200) Subject: Merge remote-tracking branches 'dechamps/sptpsrestart' and 'dechamps/keychanged'... X-Git-Tag: release-1.1pre12~154 X-Git-Url: https://www.tinc-vpn.org/git/browse?a=commitdiff_plain;h=5c32bd1578d59e005f634621d17ca96af32bb630;hp=2cb216d83d825fcca2fa2b66c756b253f8f0828b;p=tinc Merge remote-tracking branches 'dechamps/sptpsrestart' and 'dechamps/keychanged' into 1.1 --- diff --git a/src/net_packet.c b/src/net_packet.c index 92a3eb53..8313a54f 100644 --- a/src/net_packet.c +++ b/src/net_packet.c @@ -288,7 +288,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; @@ -464,11 +471,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; } diff --git a/src/protocol_key.c b/src/protocol_key.c index 6721aa44..b409a357 100644 --- a/src/protocol_key.c +++ b/src/protocol_key.c @@ -109,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); @@ -150,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); } @@ -430,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 e5946ed6..d6e6d417 100644 --- a/src/sptps.c +++ b/src/sptps.c @@ -447,6 +447,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) @@ -454,38 +455,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; + + data = buffer; + len = outlen; - uint8_t type = buffer[0]; + 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);