From 46f3eba7755089ff68fdc137b0754cae2fa523eb Mon Sep 17 00:00:00 2001 From: Guus Sliepen Date: Sun, 9 Sep 2018 18:19:15 +0200 Subject: [PATCH] Prevent oracle attacks in the legacy protocol (CVE-2018-16737, CVE-2018-16738) The legacy 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. - Reduce the amount of connections per second 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. Just to be on the safe side: - The new protocol now requires a valid SIG from the initiator of a connection before sending a SIG of its own. --- src/connection.c | 8 ++- src/connection.h | 4 +- src/net.c | 21 +++++++ src/net.h | 1 + src/net_packet.c | 5 +- src/net_setup.c | 2 +- src/net_socket.c | 15 +---- src/protocol.c | 10 ++- src/protocol_auth.c | 79 ++++++++++++++++++------ src/protocol_edge.c | 4 +- src/sptps.c | 10 ++- src/tincctl.c | 4 +- test/Makefile.am | 6 ++ test/security.test | 98 ++++++++++++++++++++++++++++++ test/splice.c | 144 ++++++++++++++++++++++++++++++++++++++++++++ 15 files changed, 367 insertions(+), 44 deletions(-) create mode 100755 test/security.test create mode 100644 test/splice.c diff --git a/src/connection.c b/src/connection.c index 92a9f48e..1c638a4d 100644 --- a/src/connection.c +++ b/src/connection.c @@ -27,6 +27,7 @@ #include "control_common.h" #include "list.h" #include "logger.h" +#include "net.h" #include "rsa.h" #include "subnet.h" #include "utils.h" @@ -68,6 +69,7 @@ void free_connection(connection_t *c) { ecdsa_free(c->ecdsa); free(c->hischallenge); + free(c->mychallenge); buffer_clear(&c->inbuf); buffer_clear(&c->outbuf); @@ -75,7 +77,11 @@ void free_connection(connection_t *c) { io_del(&c->io); if(c->socket > 0) { - closesocket(c->socket); + if(c->status.tarpit) { + tarpit(c->socket); + } else { + closesocket(c->socket); + } } free(c->name); diff --git a/src/connection.h b/src/connection.h index 48c839b5..206417b7 100644 --- a/src/connection.h +++ b/src/connection.h @@ -49,7 +49,8 @@ typedef struct connection_status_t { unsigned int log: 1; /* 1 if this is a control connection requesting log dump */ unsigned int invitation: 1; /* 1 if this is an invitation */ unsigned int invitation_used: 1; /* 1 if the invitation has been consumed */ - unsigned int unused: 18; + unsigned int tarpit: 1; /* 1 if the connection should be added to the tarpit */ + unsigned int unused: 17; } connection_status_t; #include "ecdsa.h" @@ -94,6 +95,7 @@ typedef struct connection_t { int outcompression; char *hischallenge; /* The challenge we sent to him */ + char *mychallenge; /* The challenge we received */ struct buffer_t inbuf; struct buffer_t outbuf; diff --git a/src/net.c b/src/net.c index 5d84741d..e9aed34c 100644 --- a/src/net.c +++ b/src/net.c @@ -92,6 +92,22 @@ void purge(void) { } } +/* 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: - Mark it as inactive @@ -218,6 +234,7 @@ static void timeout_handler(void *data) { logger(DEBUG_CONNECTIONS, LOG_WARNING, "Timeout while connecting to %s (%s)", c->name, c->hostname); } else { logger(DEBUG_CONNECTIONS, LOG_WARNING, "Timeout from %s (%s) during authentication", c->name, c->hostname); + c->status.tarpit = true; } terminate_connection(c, c->edge); @@ -285,6 +302,10 @@ static void periodic_handler(void *data) { void handle_meta_connection_data(connection_t *c) { if(!receive_meta(c)) { + if(!c->status.control) { + c->status.tarpit = true; + } + terminate_connection(c, c->edge); return; } diff --git a/src/net.h b/src/net.h index d69eabda..aaf29b6f 100644 --- a/src/net.h +++ b/src/net.h @@ -214,6 +214,7 @@ extern void retry(void); extern int reload_configuration(void); extern void load_all_nodes(void); extern void try_tx(struct node_t *n, bool mtu); +extern void tarpit(int fd); #ifndef HAVE_MINGW #define closesocket(s) close(s) diff --git a/src/net_packet.c b/src/net_packet.c index 5d704815..a516b4a9 100644 --- a/src/net_packet.c +++ b/src/net_packet.c @@ -461,10 +461,11 @@ static bool receive_udppacket(node_t *n, vpn_packet_t *inpkt) { inpkt = outpkt; - if (origlen > MTU / 64 + 20) + if(origlen > MTU / 64 + 20) { origlen -= MTU / 64 + 20; - else + } else { origlen = 0; + } } if(inpkt->len > n->maxrecentlen) { diff --git a/src/net_setup.c b/src/net_setup.c index 73d46c99..d8b3116b 100644 --- a/src/net_setup.c +++ b/src/net_setup.c @@ -687,7 +687,7 @@ bool setup_myself_reloadable(void) { keylifetime = 3600; } - if (!get_config_bool(lookup_config(config_tree, "AutoConnect"), &autoconnect)) { + if(!get_config_bool(lookup_config(config_tree, "AutoConnect"), &autoconnect)) { autoconnect = true; } diff --git a/src/net_socket.c b/src/net_socket.c index 2da6253e..15d32db2 100644 --- a/src/net_socket.c +++ b/src/net_socket.c @@ -41,7 +41,7 @@ int maxtimeout = 900; int seconds_till_retry = 5; int udp_rcvbuf = 1024 * 1024; int udp_sndbuf = 1024 * 1024; -int max_connection_burst = 100; +int max_connection_burst = 10; int fwmark; listen_socket_t listen_socket[MAXSOCKETS]; @@ -672,12 +672,6 @@ void handle_new_meta_connection(void *data, int flags) { // Check if we get many connections from the same host static sockaddr_t prev_sa; - static int tarpit = -1; - - if(tarpit >= 0) { - closesocket(tarpit); - tarpit = -1; - } if(!sockaddrcmp_noport(&sa, &prev_sa)) { static int samehost_burst; @@ -693,7 +687,7 @@ void handle_new_meta_connection(void *data, int flags) { samehost_burst++; if(samehost_burst > max_connection_burst) { - tarpit = fd; + tarpit(fd); return; } } @@ -716,7 +710,7 @@ void handle_new_meta_connection(void *data, int flags) { if(connection_burst >= max_connection_burst) { connection_burst = max_connection_burst; - tarpit = fd; + tarpit(fd); return; } @@ -745,7 +739,6 @@ void handle_new_meta_connection(void *data, int flags) { connection_add(c); c->allow_request = ID; - send_id(c); } #ifndef HAVE_MINGW @@ -782,8 +775,6 @@ void handle_new_unix_connection(void *data, int flags) { connection_add(c); c->allow_request = ID; - - send_id(c); } #endif diff --git a/src/protocol.c b/src/protocol.c index 7cb19448..c7dd8fb4 100644 --- a/src/protocol.c +++ b/src/protocol.c @@ -82,7 +82,8 @@ bool send_request(connection_t *c, const char *format, ...) { return false; } - logger(DEBUG_META, LOG_DEBUG, "Sending %s to %s (%s): %s", request_name[atoi(request)], c->name, c->hostname, request); + int id = atoi(request); + logger(DEBUG_META, LOG_DEBUG, "Sending %s to %s (%s): %s", request_name[id], c->name, c->hostname, request); request[len++] = '\n'; @@ -90,7 +91,12 @@ bool send_request(connection_t *c, const char *format, ...) { broadcast_meta(NULL, request, len); return true; } else { - return send_meta(c, request, len); + if(id) { + return send_meta(c, request, len); + } else { + send_meta_raw(c, request, len); + return true; + } } } diff --git a/src/protocol_auth.c b/src/protocol_auth.c index 1e00f09f..9d61ab8f 100644 --- a/src/protocol_auth.c +++ b/src/protocol_auth.c @@ -300,7 +300,7 @@ static bool receive_invitation_sptps(void *handle, uint8_t type, const void *dat buf[len] = 0; - if(!*buf || !*name || strcasecmp(buf, "Name") || !check_id(name)) { + if(!*buf || !*name || strcasecmp(buf, "Name") || !check_id(name) || !strcmp(name, myself->name)) { logger(DEBUG_ALWAYS, LOG_ERR, "Invalid invitation file %s\n", cookie); fclose(f); return false; @@ -346,6 +346,10 @@ bool id_h(connection_t *c, const char *request) { free(c->name); c->name = xstrdup(""); + if(!c->outgoing) { + send_id(c); + } + return send_request(c, "%d %d %d", ACK, TINC_CTL_VERSION_CURRENT, getpid()); } @@ -369,6 +373,10 @@ bool id_h(connection_t *c, const char *request) { return false; } + if(!c->outgoing) { + send_id(c); + } + if(!send_request(c, "%d %s", ACK, mykey)) { return false; } @@ -382,7 +390,7 @@ bool id_h(connection_t *c, const char *request) { /* Check if identity is a valid name */ - if(!check_id(name)) { + if(!check_id(name) || !strcmp(name, myself->name)) { logger(DEBUG_ALWAYS, LOG_ERR, "Got bad %s from %s (%s): %s", "ID", c->name, c->hostname, "invalid name"); return false; @@ -418,6 +426,11 @@ bool id_h(connection_t *c, const char *request) { } c->allow_request = ACK; + + if(!c->outgoing) { + send_id(c); + } + return send_ack(c); } @@ -454,6 +467,10 @@ bool id_h(connection_t *c, const char *request) { c->allow_request = METAKEY; + if(!c->outgoing) { + send_id(c); + } + if(c->protocol_minor >= 2) { c->allow_request = ACK; char label[25 + strlen(myself->name) + strlen(c->name)]; @@ -618,7 +635,8 @@ bool metakey_h(connection_t *c, const char *request) { return false; } } else { - c->incipher = NULL; + logger(DEBUG_ALWAYS, LOG_ERR, "Possible intruder %s (%s): %s", c->name, c->hostname, "null cipher"); + return false; } c->inbudget = cipher_budget(c->incipher); @@ -629,7 +647,8 @@ bool metakey_h(connection_t *c, const char *request) { return false; } } else { - c->indigest = NULL; + logger(DEBUG_ALWAYS, LOG_ERR, "Possible intruder %s (%s): %s", c->name, c->hostname, "null digest"); + return false; } c->status.decryptin = true; @@ -647,9 +666,7 @@ bool send_challenge(connection_t *c) { const size_t len = rsa_size(c->rsa); char buffer[len * 2 + 1]; - if(!c->hischallenge) { - c->hischallenge = xrealloc(c->hischallenge, len); - } + c->hischallenge = xrealloc(c->hischallenge, len); /* Copy random data to the buffer */ @@ -676,41 +693,59 @@ bool challenge_h(connection_t *c, const char *request) { char buffer[MAX_STRING_SIZE]; const size_t len = rsa_size(myself->connection->rsa); - size_t digestlen = digest_length(c->indigest); - char digest[digestlen]; if(sscanf(request, "%*d " MAX_STRING, buffer) != 1) { logger(DEBUG_ALWAYS, LOG_ERR, "Got bad %s from %s (%s)", "CHALLENGE", c->name, c->hostname); return false; } - /* Convert the challenge from hexadecimal back to binary */ - - int inlen = hex2bin(buffer, buffer, sizeof(buffer)); - /* Check if the length of the challenge is all right */ - if(inlen != len) { + if(strlen(buffer) != (size_t)len * 2) { logger(DEBUG_ALWAYS, LOG_ERR, "Possible intruder %s (%s): %s", c->name, c->hostname, "wrong challenge length"); return false; } + c->mychallenge = xrealloc(c->mychallenge, len); + + /* Convert the challenge from hexadecimal back to binary */ + + hex2bin(buffer, c->mychallenge, len); + + /* The rest is done by send_chal_reply() */ + + c->allow_request = CHAL_REPLY; + + if(c->outgoing) { + return send_chal_reply(c); + } else { + return true; + } + +#endif +} + +bool send_chal_reply(connection_t *c) { + const size_t len = rsa_size(myself->connection->rsa); + size_t digestlen = digest_length(c->indigest); + char digest[digestlen * 2 + 1]; + /* Calculate the hash from the challenge we received */ - if(!digest_create(c->indigest, buffer, len, digest)) { + if(!digest_create(c->indigest, c->mychallenge, len, digest)) { return false; } + free(c->mychallenge); + c->mychallenge = NULL; + /* Convert the hash to a hexadecimal formatted string */ - bin2hex(digest, buffer, digestlen); + bin2hex(digest, digest, digestlen); /* Send the reply */ - c->allow_request = CHAL_REPLY; - - return send_request(c, "%d %s", CHAL_REPLY, buffer); -#endif + return send_request(c, "%d %s", CHAL_REPLY, digest); } bool chal_reply_h(connection_t *c, const char *request) { @@ -752,6 +787,10 @@ bool chal_reply_h(connection_t *c, const char *request) { c->hischallenge = NULL; c->allow_request = ACK; + if(!c->outgoing) { + send_chal_reply(c); + } + return send_ack(c); #endif } diff --git a/src/protocol_edge.c b/src/protocol_edge.c index 18b6befe..9fd301f2 100644 --- a/src/protocol_edge.c +++ b/src/protocol_edge.c @@ -85,7 +85,7 @@ bool add_edge_h(connection_t *c, const char *request) { /* 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(DEBUG_ALWAYS, LOG_ERR, "Got bad %s from %s (%s): %s", "ADD_EDGE", c->name, c->hostname, "invalid name"); return false; @@ -237,7 +237,7 @@ bool del_edge_h(connection_t *c, const char *request) { /* 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(DEBUG_ALWAYS, LOG_ERR, "Got bad %s from %s (%s): %s", "DEL_EDGE", c->name, c->hostname, "invalid name"); return false; diff --git a/src/sptps.c b/src/sptps.c index 93a7ad3f..82e913eb 100644 --- a/src/sptps.c +++ b/src/sptps.c @@ -287,7 +287,11 @@ static bool receive_kex(sptps_t *s, const char *data, uint16_t len) { memcpy(s->hiskex, data, len); - return send_sig(s); + if(s->initiator) { + return send_sig(s); + } else { + return true; + } } // Receive a SIGnature record, verify it, if it passed, compute the shared secret and calculate the session keys. @@ -327,6 +331,10 @@ static bool receive_sig(sptps_t *s, const char *data, uint16_t len) { return false; } + if(!s->initiator && !send_sig(s)) { + return false; + } + free(s->mykex); free(s->hiskex); diff --git a/src/tincctl.c b/src/tincctl.c index 12e5ead9..7244779e 100644 --- a/src/tincctl.c +++ b/src/tincctl.c @@ -902,6 +902,8 @@ bool connect_tincd(bool verbose) { setsockopt(fd, SOL_SOCKET, SO_NOSIGPIPE, (void *)&one, sizeof(one)); #endif + sendline(fd, "%d ^%s %d", ID, controlcookie, TINC_CTL_VERSION_CURRENT); + char data[4096]; int version; @@ -915,8 +917,6 @@ bool connect_tincd(bool verbose) { return false; } - sendline(fd, "%d ^%s %d", ID, controlcookie, TINC_CTL_VERSION_CURRENT); - if(!recvline(fd, line, sizeof(line)) || sscanf(line, "%d %d %d", &code, &version, &pid) != 3 || code != 4 || version != TINC_CTL_VERSION_CURRENT) { if(verbose) { fprintf(stderr, "Could not fully establish control socket connection\n"); diff --git a/test/Makefile.am b/test/Makefile.am index 29b2c21d..9c2f0011 100644 --- a/test/Makefile.am +++ b/test/Makefile.am @@ -8,6 +8,7 @@ TESTS = \ invite-tinc-up.test \ ns-ping.test \ scripts.test \ + security.test \ sptps-basic.test \ variables.test @@ -17,6 +18,11 @@ EXTRA_DIST = testlib.sh AM_CFLAGS = -iquote. +check_PROGRAMS = \ + splice + +splice_SOURCES = splice.c + clean-local: -for pid in *.test.?/pid; do ../src/tinc --pidfile="$$pid" stop; done -killall ../src/sptps_test diff --git a/test/security.test b/test/security.test new file mode 100755 index 00000000..91d29e29 --- /dev/null +++ b/test/security.test @@ -0,0 +1,98 @@ +#!/bin/sh + +. "${0%/*}/testlib.sh" + +# Skip this test if tools are missing + +which socket >/dev/null || exit 77 +which timeout >/dev/null || exit 77 + +# Initialize two nodes + +$tinc $c1 < + + This program is free software; you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation; either version 2 of the License, or + (at your option) any later version. + + This program is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License along + with this program; if not, write to the Free Software Foundation, Inc., + 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. +*/ + +#include +#include +#include +#include +#include +#include +#include + +#ifdef HAVE_MINGW +extern const char *winerror(int); +#define strerror(x) ((x)>0?strerror(x):winerror(GetLastError())) +#define sockerrno WSAGetLastError() +#define sockstrerror(x) winerror(x) +#else +#define sockerrno errno +#define sockstrerror(x) strerror(x) +#endif + +int main(int argc, char *argv[]) { + if(argc < 7) { + fprintf(stderr, "Usage: %s name1 host1 port1 name2 host2 port2 [protocol]\n", argv[0]); + return 1; + } + + const char *protocol; + + if(argc >= 8) { + protocol = argv[7]; + } else { + protocol = "17.7"; + } + +#ifdef HAVE_MINGW + static struct WSAData wsa_state; + + if(WSAStartup(MAKEWORD(2, 2), &wsa_state)) { + return 1; + } + +#endif + int sock[2]; + char buf[1024]; + + struct addrinfo *ai, hint; + memset(&hint, 0, sizeof(hint)); + + hint.ai_family = AF_UNSPEC; + hint.ai_socktype = SOCK_STREAM; + hint.ai_protocol = IPPROTO_TCP; + hint.ai_flags = 0; + + for (int i = 0; i < 2; i++) { + if(getaddrinfo(argv[2 + 3 * i], argv[3 + 3 * i], &hint, &ai) || !ai) { + fprintf(stderr, "getaddrinfo() failed: %s\n", sockstrerror(sockerrno)); + return 1; + } + + sock[i] = socket(ai->ai_family, ai->ai_socktype, ai->ai_protocol); + + if(sock[i] == -1) { + fprintf(stderr, "Could not create socket: %s\n", sockstrerror(sockerrno)); + return 1; + } + + if(connect(sock[i], ai->ai_addr, ai->ai_addrlen)) { + fprintf(stderr, "Could not connect to %s: %s\n", argv[i + 3 * i], sockstrerror(sockerrno)); + return 1; + } + + fprintf(stderr, "Connected to %s\n", argv[1 + 3 * i]); + + /* Pretend to be the other one */ + int len = snprintf(buf, sizeof buf, "0 %s %s\n", argv[4 - 3 * i], protocol); + if (send(sock[i], buf, len, 0) != len) { + fprintf(stderr, "Error sending data to %s: %s\n", argv[1 + 3 * i], sockstrerror(sockerrno)); + return 1; + } + + /* Ignore the response */ + do { + if (recv(sock[i], buf, 1, 0) != 1) { + fprintf(stderr, "Error reading data from %s: %s\n", argv[1 + 3 * i], sockstrerror(sockerrno)); + return 1; + } + } while(*buf != '\n'); + } + + fprintf(stderr, "Splicing...\n"); + + int nfds = (sock[0] > sock[1] ? sock[0] : sock[1]) + 1; + + while(true) { + fd_set fds; + FD_ZERO(&fds); + FD_SET(sock[0], &fds); + FD_SET(sock[1], &fds); + + if(select(nfds, &fds, NULL, NULL, NULL) <= 0) { + return 1; + } + + for(int i = 0; i < 2; i++ ) { + if(FD_ISSET(sock[i], &fds)) { + ssize_t len = recv(sock[i], buf, sizeof buf, 0); + + if(len < 0) { + fprintf(stderr, "Error while reading from %s: %s\n", argv[1 + i * 3], sockstrerror(sockerrno)); + return 1; + } + + if(len == 0) { + fprintf(stderr, "Connection closed by %s\n", argv[1 + i * 3]); + return 0; + } + + if(send(sock[i ^ 1], buf, len, 0) != len) { + fprintf(stderr, "Error while writing to %s: %s\n", argv[4 - i * 3], sockstrerror(sockerrno)); + return 1; + } + } + } + } + + return 0; +} -- 2.20.1