Merge branch 'keysegfault' of https://github.com/dechamps/tinc into 1.1

diff --git a/doc/tinc.conf.5.in b/doc/tinc.conf.5.in
index 771a25f..9d9bf76 100644 (file)
--- a/doc/tinc.conf.5.in
+++ b/doc/tinc.conf.5.in
@@ -121,6 +121,8 @@ will automatically set up meta connections to other nodes,
 without requiring
 .Va ConnectTo
 variables.
+.Pp
+Note: it is not possible to connect to nodes using zero (system-assigned) ports in this way.
 .It Va BindToAddress Li = Ar address Op Ar port
 This is the same as
 .Va ListenAddress ,
@@ -340,6 +342,14 @@ To only listen on a specific port but not on a specific address, use
 .Li *
 for the
 .Ar address .
+.Pp
+If
+.Ar port
+is set to zero, it will be randomly assigned by the system. This is useful to randomize source ports of UDP packets, which can improve UDP hole punching reliability. In this case it is recommended to set
+.Va AddressFamily
+as well, otherwise
+.Nm tinc
+will assign different ports to different address families but other nodes can only know of one.
 .It Va LocalDiscovery Li = yes | no Pq yes
 When enabled,
 .Nm tinc
@@ -549,6 +559,14 @@ The port number on which this tinc daemon is listening for incoming connections,
 which is used if no port number is specified in an
 .Va Address
 statement.
+.Pp
+If this is set to zero, the port will be randomly assigned by the system. This is useful to randomize source ports of UDP packets, which can improve UDP hole punching reliability. When setting
+.Va Port
+to zero it is recommended to set
+.Va AddressFamily
+as well, otherwise
+.Nm tinc
+will assign different ports to different address families but other nodes can only know of one.
 .It Va PublicKey Li = Ar key Bq obsolete
 The public RSA key of this tinc daemon.
 It will be used to cryptographically verify it's identity and to set up a secure connection.

diff --git a/src/connection.h b/src/connection.h
index b5d3d18..b74b582 100644 (file)
--- a/src/connection.h
+++ b/src/connection.h
@@ -36,7 +36,7 @@
 
 typedef struct connection_status_t {
                unsigned int pinged:1;                  /* sent ping */
-               unsigned int active:1;                  /* 1 if active.. */
+               unsigned int unused_active:1;
                unsigned int connecting:1;              /* 1 if we are waiting for a non-blocking connect() to finish */
                unsigned int unused_termreq:1;          /* the termination of this connection was requested */
                unsigned int remove_unused:1;           /* Set to 1 if you want this connection removed */
@@ -49,7 +49,7 @@ 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:19;
+               unsigned int unused:18;
 } connection_status_t;
 
 #include "ecdsa.h"

diff --git a/src/control.c b/src/control.c
index dc8890a..98eae80 100644 (file)
--- a/src/control.c
+++ b/src/control.c
@@ -106,7 +106,7 @@ bool control_h(connection_t *c, const char *request) {
                        for list_each(connection_t, other, connection_list) {
                                if(strcmp(other->name, name))
                                        continue;
-                               terminate_connection(other, other->status.active);
+                               terminate_connection(other, other->edge);
                                found = true;
                        }
 

diff --git a/src/device.h b/src/device.h
index ad91a0e..8046a25 100644 (file)
--- a/src/device.h
+++ b/src/device.h
@@ -27,11 +27,6 @@ extern int device_fd;
 extern char *device;
 extern char *iface;
 
-extern uint64_t device_in_packets;
-extern uint64_t device_in_bytes;
-extern uint64_t device_out_packets;
-extern uint64_t device_out_bytes;
-
 typedef struct devops_t {
        bool (*setup)(void);
        void (*close)(void);

diff --git a/src/invitation.c b/src/invitation.c
index a6996cc..28f9f8c 100644 (file)
--- a/src/invitation.c
+++ b/src/invitation.c
@@ -613,6 +613,7 @@ make_names:
        FILE *fh = fopen(filename, "w");
        if(!fh) {
                fprintf(stderr, "Could not create file %s: %s\n", filename, strerror(errno));
+               fclose(f);
                return false;
        }
 

diff --git a/src/meta.c b/src/meta.c
index 25cca5f..fbd3e26 100644 (file)
--- a/src/meta.c
+++ b/src/meta.c
@@ -76,7 +76,7 @@ bool send_meta(connection_t *c, const char *buffer, int length) {
 
 void broadcast_meta(connection_t *from, const char *buffer, int length) {
        for list_each(connection_t, c, connection_list)
-               if(c != from && c->status.active)
+               if(c != from && c->edge)
                        send_meta(c, buffer, length);
 }
 

diff --git a/src/mingw/device.c b/src/mingw/device.c
index 33b13da..b6dffbc 100644 (file)
--- a/src/mingw/device.c
+++ b/src/mingw/device.c
@@ -43,9 +43,6 @@ char *device = NULL;
 char *iface = NULL;
 static char *device_info = NULL;
 
-static uint64_t device_total_in = 0;
-static uint64_t device_total_out = 0;
-
 extern char *myport;
 
 static void device_issue_read() {
@@ -60,7 +57,7 @@ static void device_issue_read() {
        }
 }
 
-static void device_handle_read(void *data) {
+static void device_handle_read(void *data, int flags) {
        ResetEvent(device_read_overlapped.hEvent);
 
        DWORD len;
@@ -89,7 +86,6 @@ static bool setup_device(void) {
        bool found = false;
 
        int err;
-       HANDLE thread;
 
        get_config_string(lookup_config(config_tree, "Device"), &device);
        get_config_string(lookup_config(config_tree, "Interface"), &iface);
@@ -235,8 +231,6 @@ static bool write_packet(vpn_packet_t *packet) {
                return false;
        }
 
-       device_total_out += packet->len;
-
        return true;
 }
 

diff --git a/src/net.c b/src/net.c
index 8fe3c51..91b9305 100644 (file)
--- a/src/net.c
+++ b/src/net.c
@@ -97,8 +97,6 @@ void purge(void) {
 void terminate_connection(connection_t *c, bool report) {
        logger(DEBUG_CONNECTIONS, LOG_NOTICE, "Closing connection with %s (%s)", c->name, c->hostname);
 
-       c->status.active = false;
-
        if(c->node && c->node->connection == c)
                c->node->connection = NULL;
 
@@ -155,7 +153,7 @@ static void timeout_handler(void *data) {
                        continue;
 
                if(c->last_ping_time + pingtimeout <= now.tv_sec) {
-                       if(c->status.active) {
+                       if(c->edge) {
                                if(c->status.pinged) {
                                        logger(DEBUG_CONNECTIONS, LOG_INFO, "%s (%s) didn't respond to PING in %ld seconds", c->name, c->hostname, (long)now.tv_sec - c->last_ping_time);
                                } else if(c->last_ping_time + pinginterval <= now.tv_sec) {
@@ -170,7 +168,7 @@ static void timeout_handler(void *data) {
                                else
                                        logger(DEBUG_CONNECTIONS, LOG_WARNING, "Timeout from %s (%s) during authentication", c->name, c->hostname);
                        }
-                       terminate_connection(c, c->status.active);
+                       terminate_connection(c, c->edge);
                }
        }
 
@@ -204,7 +202,7 @@ static void periodic_handler(void *data) {
                /* Count number of active connections */
                int nc = 0;
                for list_each(connection_t, c, connection_list) {
-                       if(c->status.active && !c->status.control)
+                       if(c->edge)
                                nc++;
                }
 
@@ -251,7 +249,7 @@ static void periodic_handler(void *data) {
                        int i = 0;
 
                        for list_each(connection_t, c, connection_list) {
-                               if(!c->status.active || c->status.control)
+                               if(!c->edge)
                                        continue;
 
                                if(i++ != r)
@@ -263,7 +261,7 @@ static void periodic_handler(void *data) {
                                logger(DEBUG_CONNECTIONS, LOG_INFO, "Autodisconnecting from %s", c->name);
                                list_delete(outgoing_list, c->outgoing);
                                c->outgoing = NULL;
-                               terminate_connection(c, c->status.active);
+                               terminate_connection(c, c->edge);
                                break;
                        }
                }
@@ -293,7 +291,7 @@ static void periodic_handler(void *data) {
 
 void handle_meta_connection_data(connection_t *c) {
        if (!receive_meta(c)) {
-               terminate_connection(c, c->status.active);
+               terminate_connection(c, c->edge);
                return;
        }
 }
@@ -418,7 +416,7 @@ int reload_configuration(void) {
                struct stat s;
                if(stat(fname, &s) || s.st_mtime > last_config_check) {
                        logger(DEBUG_CONNECTIONS, LOG_INFO, "Host config file of %s has been changed", c->name);
-                       terminate_connection(c, c->status.active);
+                       terminate_connection(c, c->edge);
                }
                free(fname);
        }

diff --git a/src/net_packet.c b/src/net_packet.c
index 6b3183d..4ec70fa 100644 (file)
--- a/src/net_packet.c
+++ b/src/net_packet.c
@@ -360,7 +360,6 @@ static void receive_udppacket(node_t *n, vpn_packet_t *inpkt) {
        vpn_packet_t pkt1, pkt2;
        vpn_packet_t *pkt[] = { &pkt1, &pkt2, &pkt1, &pkt2 };
        int nextpkt = 0;
-       vpn_packet_t *outpkt = pkt[0];
        size_t outlen;
 
        if(n->status.sptps) {
@@ -402,7 +401,7 @@ static void receive_udppacket(node_t *n, vpn_packet_t *inpkt) {
        /* Decrypt the packet */
 
        if(cipher_active(n->incipher)) {
-               outpkt = pkt[nextpkt++];
+               vpn_packet_t *outpkt = pkt[nextpkt++];
                outlen = MAXSIZE;
 
                if(!cipher_decrypt(n->incipher, &inpkt->seqno, inpkt->len, &outpkt->seqno, &outlen, true)) {
@@ -459,7 +458,7 @@ static void receive_udppacket(node_t *n, vpn_packet_t *inpkt) {
        length_t origlen = inpkt->len;
 
        if(n->incompression) {
-               outpkt = pkt[nextpkt++];
+               vpn_packet_t *outpkt = pkt[nextpkt++];
 
                if((outpkt->len = uncompress_packet(outpkt->data, inpkt->data, inpkt->len, n->incompression)) < 0) {
                        logger(DEBUG_TRAFFIC, LOG_ERR, "Error while uncompressing packet from %s (%s)",
@@ -629,8 +628,8 @@ static void send_udppacket(node_t *n, vpn_packet_t *origpkt) {
        size_t outlen;
 #if defined(SOL_IP) && defined(IP_TOS)
        static int priority = 0;
-#endif
        int origpriority = origpkt->priority;
+#endif
 
        if(!n->status.reachable) {
                logger(DEBUG_TRAFFIC, LOG_INFO, "Trying to send UDP packet to unreachable node %s (%s)", n->name, n->hostname);
@@ -769,12 +768,12 @@ bool send_sptps_data(void *handle, uint8_t type, const char *data, size_t len) {
 
        /* Otherwise, send the packet via UDP */
 
-       const sockaddr_t *sa;
+       const sockaddr_t *sa = NULL;
        int sock;
 
        if(to->status.send_locally)
                choose_local_address(to, &sa, &sock);
-       else
+       if(!sa)
                choose_udp_address(to, &sa, &sock);
 
        if(sendto(listen_socket[sock].udp.fd, data, len, 0, &sa->sa, SALEN(sa->sa)) < 0 && !sockwouldblock(sockerrno)) {
@@ -937,7 +936,7 @@ void broadcast_packet(const node_t *from, vpn_packet_t *packet) {
                // usually distributes the sending of broadcast packets over all nodes.
                case BMODE_MST:
                        for list_each(connection_t, c, connection_list)
-                               if(c->status.active && c->status.mst && c != from->nexthop->connection)
+                               if(c->edge && c->status.mst && c != from->nexthop->connection)
                                        send_packet(c->node, packet);
                        break;
 

diff --git a/src/net_socket.c b/src/net_socket.c
index b2297af..1bf9d16 100644 (file)
--- a/src/net_socket.c
+++ b/src/net_socket.c
@@ -388,7 +388,7 @@ static void handle_meta_write(connection_t *c) {
                        logger(DEBUG_CONNECTIONS, LOG_ERR, "Could not send %d bytes of data to %s (%s): %s", c->outbuf.len - c->outbuf.offset, c->name, c->hostname, sockstrerror(sockerrno));
                }
 
-               terminate_connection(c, c->status.active);
+               terminate_connection(c, c->edge);
                return;
        }
 
@@ -820,7 +820,7 @@ void try_outgoing_connections(void) {
                if(c->outgoing && c->outgoing->timeout == -1) {
                        c->outgoing = NULL;
                        logger(DEBUG_CONNECTIONS, LOG_INFO, "No more outgoing connection to %s", c->name);
-                       terminate_connection(c, c->status.active);
+                       terminate_connection(c, c->edge);
                }
        }
 

diff --git a/src/process.c b/src/process.c
index 9f99a94..6135efb 100644 (file)
--- a/src/process.c
+++ b/src/process.c
@@ -109,7 +109,7 @@ static bool install_service(void) {
        return true;
 }
 
-static io_t stop_io;
+io_t stop_io;
 
 DWORD WINAPI controlhandler(DWORD request, DWORD type, LPVOID boe, LPVOID bah) {
        switch(request) {
@@ -135,17 +135,9 @@ DWORD WINAPI controlhandler(DWORD request, DWORD type, LPVOID boe, LPVOID bah) {
        return NO_ERROR;
 }
 
-static void stop_handler(void *data, int flags) {
-       event_exit();
-}
-
 VOID WINAPI run_service(DWORD argc, LPTSTR* argv) {
        extern int main2(int argc, char **argv);
 
-       io_add_event(&stop_io, stop_handler, NULL, WSACreateEvent());
-       if (stop_io.event == FALSE)
-               abort();
-
        status.dwServiceType = SERVICE_WIN32;
        status.dwControlsAccepted = SERVICE_ACCEPT_STOP | SERVICE_ACCEPT_SHUTDOWN;
        status.dwWin32ExitCode = 0;
@@ -172,9 +164,6 @@ VOID WINAPI run_service(DWORD argc, LPTSTR* argv) {
                SetServiceStatus(statushandle, &status);
        }
 
-       if (WSACloseEvent(stop_io.event) == FALSE)
-               abort();
-       io_del(&stop_io);
        return;
 }
 

diff --git a/src/process.h b/src/process.h
index 4cdf711..ce2daed 100644 (file)
--- a/src/process.h
+++ b/src/process.h
@@ -29,6 +29,7 @@ extern bool detach(void);
 extern bool kill_other(int);
 
 #ifdef HAVE_MINGW
+extern io_t stop_io;
 extern bool init_service(void);
 #endif
 

diff --git a/src/protocol_auth.c b/src/protocol_auth.c
index 778c607..ac486ea 100644 (file)
--- a/src/protocol_auth.c
+++ b/src/protocol_auth.c
@@ -805,7 +805,6 @@ bool ack_h(connection_t *c, const char *request) {
        /* Activate this connection */
 
        c->allow_request = ALL;
-       c->status.active = true;
 
        logger(DEBUG_CONNECTIONS, LOG_NOTICE, "Connection with %s (%s) activated", c->name,
                           c->hostname);

diff --git a/src/protocol_key.c b/src/protocol_key.c
index e838f61..fcb748f 100644 (file)
--- a/src/protocol_key.c
+++ b/src/protocol_key.c
@@ -41,7 +41,7 @@ void send_key_changed(void) {
        /* Immediately send new keys to directly connected nodes to keep UDP mappings alive */
 
        for list_each(connection_t, c, connection_list)
-               if(c->status.active && c->node && c->node->status.reachable && !c->node->status.sptps)
+               if(c->edge && c->node && c->node->status.reachable && !c->node->status.sptps)
                        send_ans_key(c->node);
 
        /* Force key exchange for connections using SPTPS */

diff --git a/src/route.c b/src/route.c
index 8c65c4c..17f1c71 100644 (file)
--- a/src/route.c
+++ b/src/route.c
@@ -203,7 +203,7 @@ static void age_subnets(void *data) {
                        }
 
                        for list_each(connection_t, c, connection_list)
-                               if(c->status.active)
+                               if(c->edge)
                                        send_del_subnet(c, s);
 
                        subnet_del(myself, s);
@@ -238,7 +238,7 @@ static void learn_mac(mac_t *address) {
                /* And tell all other tinc daemons it's our MAC */
 
                for list_each(connection_t, c, connection_list)
-                       if(c->status.active)
+                       if(c->edge)
                                send_add_subnet(c, subnet);
 
                timeout_add(&age_subnets_timeout, age_subnets, NULL, &(struct timeval){10, rand() % 100000});

diff --git a/src/script.c b/src/script.c
index 9a43d53..6389cb4 100644 (file)
--- a/src/script.c
+++ b/src/script.c
@@ -49,7 +49,7 @@ bool execute_script(const char *name, char **envp) {
                        if(q) {
                                memcpy(ext, p, q - p);
                                ext[q - p] = 0;
-                               *q++;
+                               q++;
                        } else {
                                strcpy(ext, p);
                        }

diff --git a/src/sptps.c b/src/sptps.c
index 3fbd854..e9ce94a 100644 (file)
--- a/src/sptps.c
+++ b/src/sptps.c
@@ -431,13 +431,12 @@ bool sptps_verify_datagram(sptps_t *s, const char *data, size_t len) {
        uint32_t seqno;
        memcpy(&seqno, data, 4);
        seqno = ntohl(seqno);
+       if (!sptps_check_seqno(s, seqno, false))
+               return false;
 
        char buffer[len];
        size_t outlen;
-       if(!chacha_poly1305_decrypt(s->incipher, seqno, data + 4, len - 4, buffer, &outlen))
-               return false;
-
-       return sptps_check_seqno(s, seqno, false);
+       return chacha_poly1305_decrypt(s->incipher, seqno, data + 4, len - 4, buffer, &outlen);
 }
 
 // Receive incoming data, datagram version.

diff --git a/src/subnet_parse.c b/src/subnet_parse.c
index 1d54c13..c919b59 100644 (file)
--- a/src/subnet_parse.c
+++ b/src/subnet_parse.c
@@ -186,6 +186,7 @@ int subnet_compare(const subnet_t *a, const subnet_t *b) {
 bool str2net(subnet_t *subnet, const char *subnetstr) {
        char str[1024];
        strncpy(str, subnetstr, sizeof(str));
+       str[sizeof str - 1] = 0;
        int consumed;
 
        int weight = DEFAULT_WEIGHT;
@@ -255,7 +256,7 @@ bool str2net(subnet_t *subnet, const char *subnetstr) {
                for (int i = 0; i < 4; i++)
                        if (x[i] > 255)
                                return false;
-               sprintf(last_colon, ":%02x%02x:%02x%02x", x[0], x[1], x[2], x[3]);
+               snprintf(last_colon, sizeof str - (last_colon - str), ":%02x%02x:%02x%02x", x[0], x[1], x[2], x[3]);
        }
 
        char* double_colon = strstr(str, "::");

diff --git a/src/tincctl.c b/src/tincctl.c
index 6227750..f4379b7 100644 (file)
--- a/src/tincctl.c
+++ b/src/tincctl.c
@@ -810,16 +810,31 @@ static int cmd_start(int argc, char *argv[]) {
        int nargc = 0;
        char **nargv = xzalloc((optind + argc) * sizeof *nargv);
 
-       nargv[nargc++] = c;
+       char *arg0 = c;
+#ifdef HAVE_MINGW
+       /*
+          Windows has no real concept of an "argv array". A command line is just one string.
+          The CRT of the new process will decode the command line string to generate argv before calling main(), and (by convention)
+          it uses quotes to handle spaces in arguments.
+          Therefore we need to quote all arguments that might contain spaces. No, execvp() won't do that for us (see MSDN).
+          If we don't do that, then execvp() will run fine but any spaces in the filename contained in arg0 will bleed
+          into the next arguments when the spawned process' CRT parses its command line, resulting in chaos.
+       */
+       xasprintf(&arg0, "\"%s\"", arg0);
+#endif
+       nargv[nargc++] = arg0;
        for(int i = 1; i < optind; i++)
                nargv[nargc++] = orig_argv[i];
        for(int i = 1; i < argc; i++)
                nargv[nargc++] = argv[i];
 
 #ifdef HAVE_MINGW
-       execvp(c, nargv);
-       fprintf(stderr, "Error starting %s: %s\n", c, strerror(errno));
-       return 1;
+       int status = spawnvp(_P_WAIT, c, nargv);
+       if (status == -1) {
+               fprintf(stderr, "Error starting %s: %s\n", c, strerror(errno));
+               return 1;
+       }
+       return status;
 #else
        pid_t pid = fork();
        if(pid == -1) {

diff --git a/src/tincd.c b/src/tincd.c
index 9f83a3c..748fe13 100644 (file)
--- a/src/tincd.c
+++ b/src/tincd.c
@@ -49,6 +49,7 @@
 #include "control.h"
 #include "crypto.h"
 #include "device.h"
+#include "event.h"
 #include "logger.h"
 #include "names.h"
 #include "net.h"
@@ -303,6 +304,17 @@ static bool drop_privs(void) {
 
 #ifdef HAVE_MINGW
 # define setpriority(level) !SetPriorityClass(GetCurrentProcess(), (level))
+
+static void stop_handler(void *data, int flags) {
+       event_exit();
+}
+
+static BOOL WINAPI console_ctrl_handler(DWORD type) {
+       logger(DEBUG_ALWAYS, LOG_NOTICE, "Got console shutdown request");
+       if (WSASetEvent(stop_io.event) == FALSE)
+               abort();
+       return TRUE;
+}
 #else
 # define NORMAL_PRIORITY_CLASS 0
 # define BELOW_NORMAL_PRIORITY_CLASS 10
@@ -371,10 +383,21 @@ int main(int argc, char **argv) {
 #endif
 
 #ifdef HAVE_MINGW
-       if(!do_detach || !init_service())
-               return main2(argc, argv);
-       else
-               return 1;
+       io_add_event(&stop_io, stop_handler, NULL, WSACreateEvent());
+       if (stop_io.event == FALSE)
+               abort();
+
+       int result;
+       if(!do_detach || !init_service()) {
+               SetConsoleCtrlHandler(console_ctrl_handler, TRUE);
+               result = main2(argc, argv);
+       } else
+               result = 1;
+
+       if (WSACloseEvent(stop_io.event) == FALSE)
+               abort();
+       io_del(&stop_io);
+       return result;
 }
 
 int main2(int argc, char **argv) {

diff --git a/src/top.c b/src/top.c
index 2824261..40b8047 100644 (file)
--- a/src/top.c
+++ b/src/top.c
@@ -21,6 +21,7 @@
 
 #ifdef HAVE_CURSES
 
+#undef KEY_EVENT  /* There are conflicting declarations for KEY_EVENT in Windows wincon.h and curses.h. */
 #include <curses.h>
 
 #include "control_common.h"