From 4c6a9a9611442f958c3049a566ac4369653978e9 Mon Sep 17 00:00:00 2001 From: Kirill Isakov Date: Sun, 5 Jun 2022 17:06:05 +0600 Subject: [PATCH] Add timeouts to 'tinc join' Since server tarpits suspicious connections, `tinc join` doesn't have the best UX (if anything is broken on server's side, `tinc join` just hangs indefinitely). Since we don't want to leak information to the client, add timeouts on the client side and notify the user that something is amiss if timeout is reached. --- src/invitation.c | 22 +++++++++-- src/invitation.h | 4 ++ src/tincctl.c | 4 ++ test/integration/cmd_join.py | 73 +++++++++++++++++++++++++++++++++++- 4 files changed, 99 insertions(+), 4 deletions(-) diff --git a/src/invitation.c b/src/invitation.c index e068ae89b..46891a3cf 100644 --- a/src/invitation.c +++ b/src/invitation.c @@ -1196,6 +1196,21 @@ static bool invitation_receive(void *handle, uint8_t type, const void *msg, uint return true; } +bool wait_socket_recv(int fd) { + fd_set fds; + FD_ZERO(&fds); + FD_SET(fd, &fds); + + struct timeval tv = {.tv_sec = 5}; + + if(select(fd + 1, &fds, NULL, NULL, &tv) != 1) { + fprintf(stderr, "Timed out waiting for the server to reply.\n"); + return false; + } + + return true; +} + int cmd_join(int argc, char *argv[]) { free(data); data = NULL; @@ -1320,6 +1335,7 @@ int cmd_join(int argc, char *argv[]) { freeaddrinfo(ai); free(b64_pubkey); ecdsa_free(key); + fprintf(stderr, "Could not connect to inviter. Please make sure the URL you entered is valid.\n"); return 1; } } @@ -1383,7 +1399,7 @@ int cmd_join(int argc, char *argv[]) { } if(memcmp(hishash, hash, 18)) { - fprintf(stderr, "Peer has an invalid key!\n%s\n", line + 2); + fprintf(stderr, "Peer has an invalid key. Please make sure you're using the correct URL.\n%s\n", line + 2); ecdsa_free(key); return 1; @@ -1409,7 +1425,7 @@ int cmd_join(int argc, char *argv[]) { goto exit; } - while((len = recv(sock, line, sizeof(line), 0))) { + while(wait_socket_recv(sock) && (len = recv(sock, line, sizeof(line), 0))) { if(len < 0) { if(sockwouldblock(sockerrno)) { continue; @@ -1452,7 +1468,7 @@ int cmd_join(int argc, char *argv[]) { closesocket(sock); if(!success) { - fprintf(stderr, "Invitation cancelled.\n"); + fprintf(stderr, "Invitation cancelled. Please try again and contact the inviter for assistance if this error persists.\n"); return 1; } diff --git a/src/invitation.h b/src/invitation.h index e176c4da1..c637fa9c7 100644 --- a/src/invitation.h +++ b/src/invitation.h @@ -23,4 +23,8 @@ int cmd_invite(int argc, char *argv[]); int cmd_join(int argc, char *argv[]); +// Wait until data can be read from socket, or a timeout occurs. +// true if socket is ready, false on timeout. +bool wait_socket_recv(int fd); + #endif diff --git a/src/tincctl.c b/src/tincctl.c index 2ed286a62..3b7efd4e3 100644 --- a/src/tincctl.c +++ b/src/tincctl.c @@ -495,6 +495,10 @@ bool recvline(int fd, char *line, size_t len) { } while(!(newline = memchr(buffer, '\n', blen))) { + if(!wait_socket_recv(fd)) { + return false; + } + ssize_t nrecv = recv(fd, buffer + blen, sizeof(buffer) - blen, 0); if(nrecv == -1 && sockerrno == EINTR) { diff --git a/test/integration/cmd_join.py b/test/integration/cmd_join.py index 0db0ffe87..2a9f55814 100755 --- a/test/integration/cmd_join.py +++ b/test/integration/cmd_join.py @@ -4,11 +4,12 @@ import os import shutil +import socket from testlib import check, util from testlib.log import log from testlib.const import RUN_ACCESS_CHECKS -from testlib.proc import Tinc +from testlib.proc import Tinc, Script from testlib.test import Test FAKE_INVITE = "localhost:65535/pVOZMJGm3MqTvTu0UnhMGb2cfuqygiu79MdnERnGYdga5v8C" @@ -117,6 +118,73 @@ def test_join_errors(foo: Tinc) -> None: check.true(os.access(work_dir, mode=os.W_OK)) +def resolve(address: str) -> bool: + """Try to resolve domain and return True if successful.""" + try: + return len(socket.gethostbyname(address)) > 0 + except socket.gaierror: + return False + + +def test_broken_invite(ctx: Test) -> None: + """Test joining using a broken invitation.""" + + foo, bar = ctx.node(init="set Address 127.0.0.1"), ctx.node() + foo.start() + + for url in ( + "localhost", + "localhost/" + ("x" * 47), + "localhost/" + ("x" * 49), + "[::1/QWNVAevHNSHyMk1qarlZAQOB5swl3Ptu1yGCMSZrzKWpBUMv", + ): + _, err = bar.cmd("join", url, code=1) + check.is_in("Invalid invitation URL", err) + + # This can fail for those with braindead DNS servers that resolve + # everything to show spam search results. + # https://datatracker.ietf.org/doc/html/rfc6761#section-6.4 + if not resolve("tinc.invalid"): + log.info("test invitation with an invalid domain") + url = "tinc.invalid/QWNVAevHNSHyMk1qarlZAQOB5swl3Ptu1yGCMSZrzKWpBUMv" + _, err = bar.cmd("join", url, code=1) + check.is_in("Error looking up tinc.invalid", err) + + timeout_err = "Timed out waiting for the server" + conn_err = "Could not connect to inviter" + server_err = "Please try again" + + bad_url = f"127.0.0.1:{foo.port}/jkhjAi0LGVP0o6TN7aa_7xjqM9qTb_DUxBpk6UuLEF4ubDLX" + + log.info("test invitation created by another server before invite is created") + _, err = bar.cmd("join", bad_url, code=1, timeout=10) + check.is_in(timeout_err, err) + check.is_in(conn_err, err) + + url, _ = foo.cmd("invite", "bar") + url = url.strip() + + log.info("test invitation created by another server after invite is created") + _, err = bar.cmd("join", bad_url, code=1) + check.is_in("Peer has an invalid key", err) + + log.info("remove invitation directory") + shutil.rmtree(foo.sub("invitations")) + + log.info("test when invitation file is missing") + _, err = bar.cmd("join", url, code=1, timeout=10) + check.is_in(timeout_err, err) + check.is_in(server_err, err) + + foo.add_script(Script.TINC_DOWN) + foo.cmd("stop") + foo[Script.TINC_DOWN].wait() + + foo_log = util.read_text(foo.sub("log")) + check.is_in("we don't have an invitation key", foo_log) + check.is_in("tried to use non-existing invitation", foo_log) + + with Test("run invite success tests") as context: test_invite(context.node(init=True)) @@ -125,3 +193,6 @@ def test_join_errors(foo: Tinc) -> None: with Test("run join tests") as context: test_join_errors(context.node(init=True)) + +with Test("broken invitation") as context: + test_broken_invite(context)