From d3297fbd3b8c8c8a4661f5bbf89aca5cacba8b5a Mon Sep 17 00:00:00 2001 From: Guus Sliepen Date: Sat, 8 Sep 2018 20:48:14 +0200 Subject: [PATCH] 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. --- src/connection.h | 3 ++- src/net.c | 24 +++++++++++++++++++++++- src/net.h | 1 + src/net_socket.c | 20 +++++++++++++++++++- src/protocol_auth.c | 27 +++++++++++++++++++++++---- src/protocol_edge.c | 4 ++-- 6 files changed, 70 insertions(+), 9 deletions(-) diff --git a/src/connection.h b/src/connection.h index 8c2d9f3f..629e16b9 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 1fecd88f..d20cbffb 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 a73cb047..a9becb61 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 3467804e..6195c16c 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 95bb7517..15807c33 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 be48e0d4..a1cf6409 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; -- 2.20.1