Refactor outgoing connection handling.

Struct outgoing_ts and connection_ts were depending too much on each other,
causing lots of problems, especially the reuse of a connection_t. Now, whenever
a connection is closed it is immediately removed from the list of connections
and destroyed.

diff --git a/src/conf.c b/src/conf.c
index b30feed..b1529e5 100644 (file)
--- a/src/conf.c
+++ b/src/conf.c
@@ -385,14 +385,14 @@ bool read_server_config(void) {
        return x;
 }
 
-bool read_connection_config(connection_t *c) {
+bool read_host_config(splay_tree_t *config_tree, const char *name) {
        char *fname;
        bool x;
 
-       read_config_options(c->config_tree, c->name);
+       read_config_options(config_tree, name);
 
-       xasprintf(&fname, "%s" SLASH "hosts" SLASH "%s", confbase, c->name);
-       x = read_config_file(c->config_tree, fname);
+       xasprintf(&fname, "%s" SLASH "hosts" SLASH "%s", confbase, name);
+       x = read_config_file(config_tree, fname);
        free(fname);
 
        return x;

diff --git a/src/conf.h b/src/conf.h
index 743851a..0eb9760 100644 (file)
--- a/src/conf.h
+++ b/src/conf.h
@@ -60,7 +60,7 @@ extern config_t *parse_config_line(char *, const char *, int);
 extern bool read_config_file(splay_tree_t *, const char *);
 extern void read_config_options(splay_tree_t *, const char *);
 extern bool read_server_config(void);
-extern bool read_connection_config(struct connection_t *);
+extern bool read_host_config(splay_tree_t *, const char *);
 extern bool append_config_file(const char *, const char *, const char *);
 
 #endif                                                 /* __TINC_CONF_H__ */

diff --git a/src/connection.c b/src/connection.c
index 0293100..7999b6f 100644 (file)
--- a/src/connection.c
+++ b/src/connection.c
@@ -54,7 +54,10 @@ connection_t *new_connection(void) {
        return xmalloc_and_zero(sizeof(connection_t));
 }
 
-void free_connection_partially(connection_t *c) {
+void free_connection(connection_t *c) {
+       if(!c)
+               return;
+
        cipher_close(&c->incipher);
        digest_close(&c->indigest);
        cipher_close(&c->outcipher);
@@ -64,10 +67,7 @@ void free_connection_partially(connection_t *c) {
        ecdsa_free(&c->ecdsa);
        rsa_free(&c->rsa);
 
-       if(c->hischallenge) {
-               free(c->hischallenge);
-               c->hischallenge = NULL;
-       }
+       free(c->hischallenge);
 
        buffer_clear(&c->inbuf);
        buffer_clear(&c->outbuf);
@@ -81,31 +81,6 @@ void free_connection_partially(connection_t *c) {
        if(c->socket > 0)
                closesocket(c->socket);
 
-       c->socket = -1;
-
-       c->options = 0;
-       c->status.pinged = false;
-       c->status.connecting = false;
-       c->status.encryptout = false;
-       c->status.decryptin = false;
-       c->status.mst = false;
-       c->status.control = false;
-       c->status.pcap = false;
-       c->status.log = false;
-
-       c->protocol_major = 0;
-       c->protocol_minor = 0;
-       c->allow_request = 0;
-       c->tcplen = 0;
-       c->last_ping_time = 0;
-}
-
-void free_connection(connection_t *c) {
-       if(!c)
-               return;
-
-       free_connection_partially(c);
-
        free(c->name);
        free(c->hostname);
 

diff --git a/src/connection.h b/src/connection.h
index 3ed0e31..5730fbb 100644 (file)
--- a/src/connection.h
+++ b/src/connection.h
@@ -107,7 +107,6 @@ extern void init_connections(void);
 extern void exit_connections(void);
 extern connection_t *new_connection(void) __attribute__ ((__malloc__));
 extern void free_connection(connection_t *);
-extern void free_connection_partially(connection_t *);
 extern void connection_add(connection_t *);
 extern void connection_del(connection_t *);
 extern bool dump_connections(struct connection_t *);

diff --git a/src/net.c b/src/net.c
index ba8bcca..9a32d62 100644 (file)
--- a/src/net.c
+++ b/src/net.c
@@ -103,14 +103,13 @@ void purge(void) {
 
 /*
   Terminate a connection:
-  - Close the socket
-  - Remove associated edge and tell other connections about it if report = true
+  - Mark it as inactive
+  - Remove the edge representing this connection
+  - Kill it with fire
   - Check if we need to retry making an outgoing connection
-  - Deactivate the host
 */
 void terminate_connection(connection_t *c, bool report) {
-       logger(DEBUG_CONNECTIONS, LOG_NOTICE, "Closing connection with %s (%s)",
-                          c->name, c->hostname);
+       logger(DEBUG_CONNECTIONS, LOG_NOTICE, "Closing connection with %s (%s)", c->name, c->hostname);
 
        c->status.active = false;
 
@@ -141,15 +140,13 @@ void terminate_connection(connection_t *c, bool report) {
                }
        }
 
-       free_connection_partially(c);
+       outgoing_t *outgoing = c->outgoing;
+       connection_del(c);
 
        /* Check if this was our outgoing connection */
 
-       if(c->outgoing) {
-               do_outgoing_connection(c);
-       } else {
-               connection_del(c);
-        }
+       if(outgoing)
+               do_outgoing_connection(outgoing);
 }
 
 /*
@@ -175,25 +172,20 @@ static void timeout_handler(int fd, short events, void *event) {
                if(c->last_ping_time + pingtimeout <= now) {
                        if(c->status.active) {
                                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 - c->last_ping_time);
-                                       terminate_connection(c, true);
-                                       continue;
+                                       logger(DEBUG_CONNECTIONS, LOG_INFO, "%s (%s) didn't respond to PING in %ld seconds", c->name, c->hostname, (long)now - c->last_ping_time);
                                } else if(c->last_ping_time + pinginterval <= now) {
                                        send_ping(c);
+                                       continue;
+                               } else {
+                                       continue;
                                }
                        } else {
-                               if(c->status.connecting) {
+                               if(c->status.connecting)
                                        logger(DEBUG_CONNECTIONS, LOG_WARNING, "Timeout while connecting to %s (%s)", c->name, c->hostname);
-                                       c->status.connecting = false;
-                                       closesocket(c->socket);
-                                       do_outgoing_connection(c);
-                               } else {
+                               else
                                        logger(DEBUG_CONNECTIONS, LOG_WARNING, "Timeout from %s (%s) during authentication", c->name, c->hostname);
-                                       terminate_connection(c, false);
-                                       continue;
-                               }
                        }
+                       terminate_connection(c, c->status.active);
                }
        }
 
@@ -228,11 +220,8 @@ void handle_meta_connection_data(int fd, short events, void *data) {
                if(!result)
                        finish_connecting(c);
                else {
-                       logger(DEBUG_CONNECTIONS, LOG_DEBUG,
-                                          "Error while connecting to %s (%s): %s",
-                                          c->name, c->hostname, sockstrerror(result));
-                       closesocket(c->socket);
-                       do_outgoing_connection(c);
+                       logger(DEBUG_CONNECTIONS, LOG_DEBUG, "Error while connecting to %s (%s): %s", c->name, c->hostname, sockstrerror(result));
+                       terminate_connection(c, false);
                        return;
                }
        }
@@ -369,7 +358,7 @@ int reload_configuration(void) {
 
                xasprintf(&fname, "%s" SLASH "hosts" SLASH "%s", confbase, c->name);
                if(stat(fname, &s) || s.st_mtime > last_config_check) {
-                       fprintf(stderr, "ZOMG %ld > %ld\n", 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);
                }
                free(fname);
@@ -394,7 +383,7 @@ void retry(void) {
                        if(c->status.connecting)
                                close(c->socket);
                        c->outgoing->timeout = 0;
-                       do_outgoing_connection(c);
+                       terminate_connection(c, c->status.active);
                }
        }
 }

diff --git a/src/net.h b/src/net.h
index d7b868a..5987777 100644 (file)
--- a/src/net.h
+++ b/src/net.h
@@ -109,6 +109,7 @@ typedef struct listen_socket_t {
 typedef struct outgoing_t {
        char *name;
        int timeout;
+       splay_tree_t *config_tree;
        struct config_t *cfg;
        struct addrinfo *ai;
        struct addrinfo *aip;
@@ -158,7 +159,7 @@ extern char *scriptextension;
 extern void retry_outgoing(outgoing_t *);
 extern void handle_incoming_vpn_data(int, short, void *);
 extern void finish_connecting(struct connection_t *);
-extern bool do_outgoing_connection(struct connection_t *);
+extern bool do_outgoing_connection(struct outgoing_t *);
 extern void handle_new_meta_connection(int, short, void *);
 extern int setup_listen_socket(const sockaddr_t *);
 extern int setup_vpn_in_socket(const sockaddr_t *);

diff --git a/src/net_socket.c b/src/net_socket.c
index 986a766..0f0580e 100644 (file)
--- a/src/net_socket.c
+++ b/src/net_socket.c
@@ -349,63 +349,78 @@ static void do_outgoing_pipe(connection_t *c, char *command) {
 #endif
 }
 
-bool do_outgoing_connection(connection_t *c) {
+static void handle_meta_write(int sock, short events, void *data) {
+       connection_t *c = data;
+
+       ssize_t outlen = send(c->socket, c->outbuf.data + c->outbuf.offset, c->outbuf.len - c->outbuf.offset, 0);
+       if(outlen <= 0) {
+               if(!errno || errno == EPIPE) {
+                       logger(DEBUG_CONNECTIONS, LOG_NOTICE, "Connection closed by %s (%s)", c->name, c->hostname);
+               } else if(sockwouldblock(sockerrno)) {
+                       logger(DEBUG_CONNECTIONS, LOG_DEBUG, "Sending %d bytes to %s (%s) would block", c->outbuf.len - c->outbuf.offset, c->name, c->hostname);
+                       return;
+               } else {
+                       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, strerror(errno));
+               }
+
+               terminate_connection(c, c->status.active);
+               return;
+       }
+
+       buffer_read(&c->outbuf, outlen);
+       if(!c->outbuf.len && event_initialized(&c->outevent))
+               event_del(&c->outevent);
+}
+
+
+bool do_outgoing_connection(outgoing_t *outgoing) {
        char *address, *port, *space;
        struct addrinfo *proxyai = NULL;
        int result;
 
-       if(!c->outgoing) {
-               logger(DEBUG_ALWAYS, LOG_ERR, "do_outgoing_connection() for %s called without c->outgoing", c->name);
-               abort();
-       }
-
 begin:
-       if(!c->outgoing->ai) {
-               if(!c->outgoing->cfg) {
-                       logger(DEBUG_CONNECTIONS, LOG_ERR, "Could not set up a meta connection to %s",
-                                          c->name);
-                       retry_outgoing(c->outgoing);
-                       c->outgoing = NULL;
-                       connection_del(c);
+       if(!outgoing->ai) {
+               if(!outgoing->cfg) {
+                       logger(DEBUG_CONNECTIONS, LOG_ERR, "Could not set up a meta connection to %s", outgoing->name);
+                       retry_outgoing(outgoing);
                        return false;
                }
 
-               get_config_string(c->outgoing->cfg, &address);
+               get_config_string(outgoing->cfg, &address);
 
                space = strchr(address, ' ');
                if(space) {
                        port = xstrdup(space + 1);
                        *space = 0;
                } else {
-                       if(!get_config_string(lookup_config(c->config_tree, "Port"), &port))
+                       if(!get_config_string(lookup_config(outgoing->config_tree, "Port"), &port))
                                port = xstrdup("655");
                }
 
-               c->outgoing->ai = str2addrinfo(address, port, SOCK_STREAM);
+               outgoing->ai = str2addrinfo(address, port, SOCK_STREAM);
                free(address);
                free(port);
 
-               c->outgoing->aip = c->outgoing->ai;
-               c->outgoing->cfg = lookup_config_next(c->config_tree, c->outgoing->cfg);
+               outgoing->aip = outgoing->ai;
+               outgoing->cfg = lookup_config_next(outgoing->config_tree, outgoing->cfg);
        }
 
-       if(!c->outgoing->aip) {
-               if(c->outgoing->ai)
-                       freeaddrinfo(c->outgoing->ai);
-               c->outgoing->ai = NULL;
+       if(!outgoing->aip) {
+               if(outgoing->ai)
+                       freeaddrinfo(outgoing->ai);
+               outgoing->ai = NULL;
                goto begin;
        }
 
-       memcpy(&c->address, c->outgoing->aip->ai_addr, c->outgoing->aip->ai_addrlen);
-       c->outgoing->aip = c->outgoing->aip->ai_next;
+       connection_t *c = new_connection();
+       c->outgoing = outgoing;
 
-       if(c->hostname)
-               free(c->hostname);
+       memcpy(&c->address, outgoing->aip->ai_addr, outgoing->aip->ai_addrlen);
+       outgoing->aip = outgoing->aip->ai_next;
 
        c->hostname = sockaddr2hostname(&c->address);
 
-       logger(DEBUG_CONNECTIONS, LOG_INFO, "Trying to connect to %s (%s)", c->name,
-                          c->hostname);
+       logger(DEBUG_CONNECTIONS, LOG_INFO, "Trying to connect to %s (%s)", outgoing->name, c->hostname);
 
        if(!proxytype) {
                c->socket = socket(c->address.sa.sa_family, SOCK_STREAM, IPPROTO_TCP);
@@ -414,14 +429,17 @@ begin:
                do_outgoing_pipe(c, proxyhost);
        } else {
                proxyai = str2addrinfo(proxyhost, proxyport, SOCK_STREAM);
-               if(!proxyai)
+               if(!proxyai) {
+                       free_connection(c);
                        goto begin;
+               }
                logger(DEBUG_CONNECTIONS, LOG_INFO, "Using proxy at %s port %s", proxyhost, proxyport);
                c->socket = socket(proxyai->ai_family, SOCK_STREAM, IPPROTO_TCP);
        }
 
        if(c->socket == -1) {
                logger(DEBUG_CONNECTIONS, LOG_ERR, "Creating socket for %s failed: %s", c->hostname, sockstrerror(sockerrno));
+               free_connection(c);
                goto begin;
        }
 
@@ -450,92 +468,55 @@ begin:
                freeaddrinfo(proxyai);
        }
 
-       if(result == -1) {
-               if(sockinprogress(sockerrno)) {
-                       c->status.connecting = true;
-                       return true;
-               }
-
-               closesocket(c->socket);
-
-               logger(DEBUG_CONNECTIONS, LOG_ERR, "%s: %s", c->hostname, sockstrerror(sockerrno));
+       if(result == -1 && !sockinprogress(sockerrno)) {
+               logger(DEBUG_CONNECTIONS, LOG_ERR, "Could not connect to %s (%s): %s", outgoing->name, c->hostname, sockstrerror(sockerrno));
+               free_connection(c);
 
                goto begin;
        }
 
-       finish_connecting(c);
-
-       return true;
-}
+       /* Now that there is a working socket, fill in the rest and register this connection. */
 
-static void handle_meta_write(int sock, short events, void *data) {
-       connection_t *c = data;
+       c->status.connecting = true;
+       c->name = xstrdup(outgoing->name);
+       c->outcipher = myself->connection->outcipher;
+       c->outdigest = myself->connection->outdigest;
+       c->outmaclength = myself->connection->outmaclength;
+       c->outcompression = myself->connection->outcompression;
+       c->last_ping_time = time(NULL);
 
-       ssize_t outlen = send(c->socket, c->outbuf.data + c->outbuf.offset, c->outbuf.len - c->outbuf.offset, 0);
-       if(outlen <= 0) {
-               if(!errno || errno == EPIPE) {
-                       logger(DEBUG_CONNECTIONS, LOG_NOTICE, "Connection closed by %s (%s)", c->name, c->hostname);
-               } else if(sockwouldblock(sockerrno)) {
-                       logger(DEBUG_CONNECTIONS, LOG_DEBUG, "Sending %d bytes to %s (%s) would block", c->outbuf.len - c->outbuf.offset, c->name, c->hostname);
-                       return;
-               } else {
-                       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, strerror(errno));
-               }
+       connection_add(c);
 
-               terminate_connection(c, c->status.active);
-               return;
-       }
+       event_set(&c->inevent, c->socket, EV_READ | EV_PERSIST, handle_meta_connection_data, c);
+       event_set(&c->outevent, c->socket, EV_WRITE | EV_PERSIST, handle_meta_write, c);
+       event_add(&c->inevent, NULL);
 
-       buffer_read(&c->outbuf, outlen);
-       if(!c->outbuf.len && event_initialized(&c->outevent))
-               event_del(&c->outevent);
+       return true;
 }
 
 void setup_outgoing_connection(outgoing_t *outgoing) {
-       connection_t *c;
-       node_t *n;
-
        if(event_initialized(&outgoing->ev))
                event_del(&outgoing->ev);
 
-       n = lookup_node(outgoing->name);
-
-       if(n)
-               if(n->connection) {
-                       logger(DEBUG_CONNECTIONS, LOG_INFO, "Already connected to %s", outgoing->name);
+       node_t *n = lookup_node(outgoing->name);
 
-                       n->connection->outgoing = outgoing;
-                       return;
-               }
+       if(n && n->connection) {
+               logger(DEBUG_CONNECTIONS, LOG_INFO, "Already connected to %s", outgoing->name);
 
-       c = new_connection();
-       c->name = xstrdup(outgoing->name);
-       c->outcipher = myself->connection->outcipher;
-       c->outdigest = myself->connection->outdigest;
-       c->outmaclength = myself->connection->outmaclength;
-       c->outcompression = myself->connection->outcompression;
-
-       init_configuration(&c->config_tree);
-       read_connection_config(c);
+               n->connection->outgoing = outgoing;
+               return;
+       }
 
-       outgoing->cfg = lookup_config(c->config_tree, "Address");
+       init_configuration(&outgoing->config_tree);
+       read_host_config(outgoing->config_tree, outgoing->name);
+       outgoing->cfg = lookup_config(outgoing->config_tree, "Address");
 
        if(!outgoing->cfg) {
-               logger(DEBUG_ALWAYS, LOG_ERR, "No address specified for %s", c->name);
-               free_connection(c);
+               logger(DEBUG_ALWAYS, LOG_ERR, "No address specified for %s", outgoing->name);
                return;
        }
 
-       c->outgoing = outgoing;
-       c->last_ping_time = time(NULL);
-
-       connection_add(c);
-
-       if (do_outgoing_connection(c)) {
-               event_set(&c->inevent, c->socket, EV_READ | EV_PERSIST, handle_meta_connection_data, c);
-               event_set(&c->outevent, c->socket, EV_WRITE | EV_PERSIST, handle_meta_write, c);
-               event_add(&c->inevent, NULL);
-       }
+       do_outgoing_connection(outgoing);
 }
 
 /*
@@ -651,6 +632,7 @@ void try_outgoing_connections(void) {
                connection_t *c = n->data;
                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);
                }
        }

diff --git a/src/protocol_auth.c b/src/protocol_auth.c
index 88c6255..908ab24 100644 (file)
--- a/src/protocol_auth.c
+++ b/src/protocol_auth.c
@@ -212,9 +212,8 @@ bool id_h(connection_t *c, const char *request) {
        if(!c->config_tree) {
                init_configuration(&c->config_tree);
 
-               if(!read_connection_config(c)) {
-                       logger(DEBUG_ALWAYS, LOG_ERR, "Peer %s had unknown identity (%s)", c->hostname,
-                                  c->name);
+               if(!read_host_config(c->config_tree, c->name)) {
+                       logger(DEBUG_ALWAYS, LOG_ERR, "Peer %s had unknown identity (%s)", c->hostname, c->name);
                        return false;
                }