From ce059e36fdb3d3049c278e8b2f36b03c93778996 Mon Sep 17 00:00:00 2001 From: Guus Sliepen Date: Sun, 7 Oct 2012 21:02:40 +0200 Subject: [PATCH] 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. --- src/conf.c | 8 +-- src/conf.h | 2 +- src/connection.c | 35 ++------- src/connection.h | 1 - src/net.c | 49 +++++-------- src/net.h | 3 +- src/net_socket.c | 168 ++++++++++++++++++++------------------------ src/protocol_auth.c | 5 +- 8 files changed, 108 insertions(+), 163 deletions(-) diff --git a/src/conf.c b/src/conf.c index b30feedd..b1529e57 100644 --- 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 743851a7..0eb97602 100644 --- 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 0293100e..7999b6fb 100644 --- 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 3ed0e317..5730fbb8 100644 --- 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 ba8bcca7..9a32d628 100644 --- 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 d7b868a5..59877771 100644 --- 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 986a7668..0f0580ec 100644 --- 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 88c62554..908ab247 100644 --- 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; } -- 2.20.1