From: Guus Sliepen <guus@tinc-vpn.org>
Date: Sat, 8 Sep 2018 18:48:14 +0000 (+0200)
Subject: Prevent oracle attacks (CVE-2018-16737, CVE-2018-16738)
X-Git-Tag: release-1.0.35~12
X-Git-Url: https://git.librecmc.org/?a=commitdiff_plain;h=d3297fbd3b8c8c8a4661f5bbf89aca5cacba8b5a;p=oweals%2Ftinc.git

Prevent oracle attacks (CVE-2018-16737, CVE-2018-16738)

The authentication protocol allows an oracle attack that could
potentially be exploited. This commit contains several mitigations:

- Connections are no longer closed immediately on error, but put in
  a "tarpit".
- The authentication protocol now requires a valid CHAL_REPLY from the
  initiator of a connection before sending a CHAL_REPLY of its own.
- Only a limited amount of connections per second are accepted.
- Null ciphers or digests are no longer allowed in METAKEYs.
- Connections that claim to have the same name as the local node are
  rejected.
---

diff --git a/src/connection.h b/src/connection.h
index 8c2d9f3..629e16b 100644
--- a/src/connection.h
+++ b/src/connection.h
@@ -42,7 +42,8 @@ typedef struct connection_status_t {
 	unsigned int decryptin: 1;      /* 1 if we have to decrypt incoming traffic */
 	unsigned int mst: 1;            /* 1 if this connection is part of a minimum spanning tree */
 	unsigned int proxy_passed: 1;   /* 1 if we are connecting via a proxy and we have finished talking with it */
-	unsigned int unused: 22;
+	unsigned int tarpit: 1;         /* 1 if the connection should be added to the tarpit */
+	unsigned int unused: 21;
 } connection_status_t;
 
 #include "edge.h"
diff --git a/src/net.c b/src/net.c
index 1fecd88..d20cbff 100644
--- a/src/net.c
+++ b/src/net.c
@@ -180,6 +180,22 @@ static int build_fdset(fd_set *readset, fd_set *writeset) {
 	return max;
 }
 
+/* Put a misbehaving connection in the tarpit */
+void tarpit(int fd) {
+	static int pits[10] = {-1, -1, -1, -1, -1, -1, -1, -1, -1, -1};
+	static int next_pit = 0;
+
+	if(pits[next_pit] != -1) {
+		closesocket(pits[next_pit]);
+	}
+
+	pits[next_pit++] = fd;
+
+	if(next_pit >= sizeof pits / sizeof pits[0]) {
+		next_pit = 0;
+	}
+}
+
 /*
   Terminate a connection:
   - Close the socket
@@ -203,7 +219,11 @@ void terminate_connection(connection_t *c, bool report) {
 	}
 
 	if(c->socket) {
-		closesocket(c->socket);
+		if(c->status.tarpit) {
+			tarpit(c->socket);
+		} else {
+			closesocket(c->socket);
+		}
 	}
 
 	if(c->edge) {
@@ -299,6 +319,7 @@ static void check_dead_connections(void) {
 					closesocket(c->socket);
 					do_outgoing_connection(c);
 				} else {
+					c->status.tarpit = true;
 					terminate_connection(c, false);
 				}
 			}
@@ -380,6 +401,7 @@ static void check_network_activity(fd_set *readset, fd_set *writeset) {
 
 		if(FD_ISSET(c->socket, readset)) {
 			if(!receive_meta(c)) {
+				c->status.tarpit = true;
 				terminate_connection(c, c->status.active);
 				continue;
 			}
diff --git a/src/net.h b/src/net.h
index a73cb04..a9becb6 100644
--- a/src/net.h
+++ b/src/net.h
@@ -150,6 +150,7 @@ extern void flush_queue(struct node_t *n);
 extern bool read_rsa_public_key(struct connection_t *c);
 extern void send_mtu_probe(struct node_t *n);
 extern void load_all_subnets(void);
+extern void tarpit(int fd);
 
 #ifndef HAVE_MINGW
 #define closesocket(s) close(s)
diff --git a/src/net_socket.c b/src/net_socket.c
index 3467804..6195c16 100644
--- a/src/net_socket.c
+++ b/src/net_socket.c
@@ -639,6 +639,9 @@ void setup_outgoing_connection(outgoing_t *outgoing) {
   new connection
 */
 bool handle_new_meta_connection(int sock) {
+	static const int max_accept_burst = 10;
+	static int last_accept_burst;
+	static int last_accept_time;
 	connection_t *c;
 	sockaddr_t sa;
 	int fd;
@@ -651,6 +654,22 @@ bool handle_new_meta_connection(int sock) {
 		return false;
 	}
 
+	if(last_accept_time == now) {
+		last_accept_burst++;
+
+		if(last_accept_burst >= max_accept_burst) {
+			if(last_accept_burst == max_accept_burst) {
+				ifdebug(CONNECTIONS) logger(LOG_WARNING, "Throttling incoming connections");
+			}
+
+			tarpit(fd);
+			return false;
+		}
+	} else {
+		last_accept_burst = 0;
+		last_accept_time = now;
+	}
+
 	sockaddrunmap(&sa);
 
 	c = new_connection();
@@ -672,7 +691,6 @@ bool handle_new_meta_connection(int sock) {
 	connection_add(c);
 
 	c->allow_request = ID;
-	send_id(c);
 
 	return true;
 }
diff --git a/src/protocol_auth.c b/src/protocol_auth.c
index 95bb751..15807c3 100644
--- a/src/protocol_auth.c
+++ b/src/protocol_auth.c
@@ -60,7 +60,7 @@ bool id_h(connection_t *c) {
 
 	/* Check if identity is a valid name */
 
-	if(!check_id(name)) {
+	if(!check_id(name) || !strcmp(name, myself->name)) {
 		logger(LOG_ERR, "Got bad %s from %s (%s): %s", "ID", c->name,
 		       c->hostname, "invalid name");
 		return false;
@@ -96,6 +96,11 @@ bool id_h(connection_t *c) {
 		}
 
 		c->allow_request = ACK;
+
+		if(!c->outgoing) {
+			send_id(c);
+		}
+
 		return send_ack(c);
 	}
 
@@ -115,6 +120,10 @@ bool id_h(connection_t *c) {
 
 	c->allow_request = METAKEY;
 
+	if(!c->outgoing) {
+		send_id(c);
+	}
+
 	return send_metakey(c);
 }
 
@@ -301,7 +310,8 @@ bool metakey_h(connection_t *c) {
 		c->inbudget = byte_budget(c->incipher);
 		c->status.decryptin = true;
 	} else {
-		c->incipher = NULL;
+		logger(LOG_ERR, "%s (%s) uses null cipher!", c->name, c->hostname);
+		return false;
 	}
 
 	c->inmaclength = maclength;
@@ -319,7 +329,8 @@ bool metakey_h(connection_t *c) {
 			return false;
 		}
 	} else {
-		c->indigest = NULL;
+		logger(LOG_ERR, "%s (%s) uses null digest!", c->name, c->hostname);
+		return false;
 	}
 
 	c->incompression = compression;
@@ -393,7 +404,11 @@ bool challenge_h(connection_t *c) {
 
 	/* Rest is done by send_chal_reply() */
 
-	return send_chal_reply(c);
+	if(c->outgoing) {
+		return send_chal_reply(c);
+	} else {
+		return true;
+	}
 }
 
 bool send_chal_reply(connection_t *c) {
@@ -495,6 +510,10 @@ bool chal_reply_h(connection_t *c) {
 
 	c->allow_request = ACK;
 
+	if(!c->outgoing) {
+		send_chal_reply(c);
+	}
+
 	return send_ack(c);
 }
 
diff --git a/src/protocol_edge.c b/src/protocol_edge.c
index be48e0d..a1cf640 100644
--- a/src/protocol_edge.c
+++ b/src/protocol_edge.c
@@ -70,7 +70,7 @@ bool add_edge_h(connection_t *c) {
 
 	/* Check if names are valid */
 
-	if(!check_id(from_name) || !check_id(to_name)) {
+	if(!check_id(from_name) || !check_id(to_name) || !strcmp(from_name, to_name)) {
 		logger(LOG_ERR, "Got bad %s from %s (%s): %s", "ADD_EDGE", c->name,
 		       c->hostname, "invalid name");
 		return false;
@@ -197,7 +197,7 @@ bool del_edge_h(connection_t *c) {
 
 	/* Check if names are valid */
 
-	if(!check_id(from_name) || !check_id(to_name)) {
+	if(!check_id(from_name) || !check_id(to_name) || !strcmp(from_name, to_name)) {
 		logger(LOG_ERR, "Got bad %s from %s (%s): %s", "DEL_EDGE", c->name,
 		       c->hostname, "invalid name");
 		return false;