From: Kirill Isakov Date: Thu, 19 May 2022 07:04:20 +0000 (+0600) Subject: Improve recently seen address cache X-Git-Url: https://tinc-vpn.org/git/browse?a=commitdiff_plain;h=c6a15e27d934e90a1f3a26438dddb395bdc9de19;p=tinc Improve recently seen address cache - create cache directory on init - only remember addresses for TCP connections - update cache in more situations - add tests --- diff --git a/src/address_cache.c b/src/address_cache.c index d9996fb0..f28dbf90 100644 --- a/src/address_cache.c +++ b/src/address_cache.c @@ -25,7 +25,7 @@ #include "netutl.h" #include "xalloc.h" -static const unsigned int NOT_CACHED = -1; +static const unsigned int NOT_CACHED = UINT_MAX; // Find edges pointing to this node, and use them to build a list of unique, known addresses. static struct addrinfo *get_known_addresses(node_t *n) { @@ -90,6 +90,8 @@ void add_recent_address(address_cache_t *cache, const sockaddr_t *sa) { return; } + logger(DEBUG_CONNECTIONS, LOG_DEBUG, "Caching recent address for %s", cache->node->name); + // Shift everything, move/add the address to the first slot if(pos == NOT_CACHED) { if(cache->data.used < MAX_CACHED_ADDRESSES) { @@ -216,7 +218,7 @@ const sockaddr_t *get_recent_address(address_cache_t *cache) { exit_configuration(cache->config_tree); cache->config_tree = NULL; - return false; + return NULL; } address_cache_t *open_address_cache(node_t *node) { @@ -251,11 +253,7 @@ address_cache_t *open_address_cache(node_t *node) { return cache; } -void reset_address_cache(address_cache_t *cache, const sockaddr_t *sa) { - if(sa) { - add_recent_address(cache, sa); - } - +void reset_address_cache(address_cache_t *cache) { if(cache->config_tree) { exit_configuration(cache->config_tree); cache->config_tree = NULL; diff --git a/src/address_cache.h b/src/address_cache.h index 1f891c96..36ffa5b4 100644 --- a/src/address_cache.h +++ b/src/address_cache.h @@ -45,6 +45,6 @@ const sockaddr_t *get_recent_address(address_cache_t *cache); void close_address_cache(address_cache_t *cache); address_cache_t *open_address_cache(node_t *node) ATTR_DEALLOCATOR(close_address_cache); -void reset_address_cache(address_cache_t *cache, const sockaddr_t *sa); +void reset_address_cache(address_cache_t *cache); #endif diff --git a/src/graph.c b/src/graph.c index c100a933..5a7a16a7 100644 --- a/src/graph.c +++ b/src/graph.c @@ -55,6 +55,7 @@ #include "script.h" #include "subnet.h" #include "xalloc.h" +#include "address_cache.h" /* Implementation of Kruskal's algorithm. Running time: O(EN) @@ -226,6 +227,14 @@ static void check_reachability(void) { if(n != myself) { became_reachable_count++; + + if(n->connection && n->connection->outgoing) { + if(!n->address_cache) { + n->address_cache = open_address_cache(n); + } + + add_recent_address(n->address_cache, &n->connection->address); + } } } else { logger(DEBUG_TRAFFIC, LOG_DEBUG, "Node %s (%s) became unreachable", diff --git a/src/invitation.c b/src/invitation.c index a95a3245..08f053a9 100644 --- a/src/invitation.c +++ b/src/invitation.c @@ -384,13 +384,12 @@ int cmd_invite(int argc, char *argv[]) { } } - snprintf(filename, sizeof(filename), "%s" SLASH "invitations", confbase); - - if(mkdir(filename, 0700) && errno != EEXIST) { - fprintf(stderr, "Could not create directory %s: %s\n", filename, strerror(errno)); - return 1; + if(!makedirs(DIR_INVITATIONS)) { + return false; } + snprintf(filename, sizeof(filename), "%s" SLASH "invitations", confbase); + // Count the number of valid invitations, clean up old ones DIR *dir = opendir(filename); @@ -779,13 +778,7 @@ make_names: goto make_names; } - if(mkdir(confbase, 0777) && errno != EEXIST) { - fprintf(stderr, "Could not create directory %s: %s\n", confbase, strerror(errno)); - return false; - } - - if(mkdir(hosts_dir, 0777) && errno != EEXIST) { - fprintf(stderr, "Could not create directory %s: %s\n", hosts_dir, strerror(errno)); + if(!makedirs(DIR_HOSTS | DIR_CONFBASE | DIR_CACHE)) { return false; } @@ -1213,14 +1206,8 @@ int cmd_join(int argc, char *argv[]) { } // Make sure confbase exists and is accessible. - if(!confbase_given && mkdir(confdir, 0755) && errno != EEXIST) { - fprintf(stderr, "Could not create directory %s: %s\n", confdir, strerror(errno)); - return 1; - } - - if(mkdir(confbase, 0777) && errno != EEXIST) { - fprintf(stderr, "Could not create directory %s: %s\n", confbase, strerror(errno)); - return 1; + if(!makedirs(DIR_CONFDIR | DIR_CONFBASE)) { + return false; } if(access(confbase, R_OK | W_OK | X_OK)) { diff --git a/src/net_packet.c b/src/net_packet.c index 6fea959f..0b95a939 100644 --- a/src/net_packet.c +++ b/src/net_packet.c @@ -200,7 +200,10 @@ static void udp_probe_h(node_t *n, vpn_packet_t *packet, length_t len) { n->address_cache = open_address_cache(n); } - reset_address_cache(n->address_cache, &n->address); + if(n->connection && n->connection->edge) { + reset_address_cache(n->address_cache); + add_recent_address(n->address_cache, &n->connection->edge->address); + } } // Reset the UDP ping timer. diff --git a/src/net_setup.c b/src/net_setup.c index 3ac5a679..2cd5818b 100644 --- a/src/net_setup.c +++ b/src/net_setup.c @@ -184,8 +184,7 @@ void load_all_nodes(void) { read_host_config(&config, ent->d_name, true); if(!n) { - n = new_node(); - n->name = xstrdup(ent->d_name); + n = new_node(ent->d_name); node_add(n); } @@ -764,10 +763,9 @@ static bool setup_myself(void) { } myname = xstrdup(name); - myself = new_node(); + myself = new_node(name); myself->connection = new_connection(); - myself->name = name; - myself->connection->name = xstrdup(name); + myself->connection->name = name; read_host_config(&config_tree, name, true); if(!get_config_string(lookup_config(&config_tree, "Port"), &myport.tcp)) { diff --git a/src/net_socket.c b/src/net_socket.c index 4c76c30a..982bc800 100644 --- a/src/net_socket.c +++ b/src/net_socket.c @@ -671,22 +671,18 @@ void setup_outgoing_connection(outgoing_t *outgoing, bool verbose) { n->address_cache = open_address_cache(n); } - if(n->connection) { - logger(DEBUG_CONNECTIONS, LOG_INFO, "Already connected to %s", n->name); - - if(!n->connection->outgoing) { - n->connection->outgoing = outgoing; - return; - } else { - goto remove; - } + if(!n->connection) { + do_outgoing_connection(outgoing); + return; } - do_outgoing_connection(outgoing); - return; + logger(DEBUG_CONNECTIONS, LOG_INFO, "Already connected to %s", n->name); -remove: - list_delete(&outgoing_list, outgoing); + if(n->connection->outgoing) { + list_delete(&outgoing_list, outgoing); + } else { + n->connection->outgoing = outgoing; + } } static bool check_tarpit(const sockaddr_t *sa, int fd) { @@ -864,8 +860,7 @@ void try_outgoing_connections(void) { node_t *n = lookup_node(name); if(!n) { - n = new_node(); - n->name = xstrdup(name); + n = new_node(name); node_add(n); } diff --git a/src/node.c b/src/node.c index 8e0f162e..3e37b8eb 100644 --- a/src/node.c +++ b/src/node.c @@ -71,7 +71,7 @@ void exit_nodes(void) { splay_empty_tree(&node_tree); } -node_t *new_node(void) { +node_t *new_node(const char *name) { node_t *n = xzalloc(sizeof(*n)); if(replaywin) { @@ -84,6 +84,7 @@ node_t *new_node(void) { n->mtu = MTU; n->maxmtu = MTU; n->udp_ping_rtt = -1; + n->name = xstrdup(name); return n; } diff --git a/src/node.h b/src/node.h index fea90d1b..51322c25 100644 --- a/src/node.h +++ b/src/node.h @@ -122,7 +122,7 @@ extern splay_tree_t node_tree; extern void exit_nodes(void); extern void free_node(node_t *n); -extern node_t *new_node(void) ATTR_MALLOC ATTR_DEALLOCATOR(free_node); +extern node_t *new_node(const char *name) ATTR_MALLOC ATTR_DEALLOCATOR(free_node); extern void node_add(node_t *n); extern void node_del(node_t *n); extern node_t *lookup_node(char *name); diff --git a/src/protocol_auth.c b/src/protocol_auth.c index 7a3e6dd0..263a1317 100644 --- a/src/protocol_auth.c +++ b/src/protocol_auth.c @@ -44,6 +44,7 @@ #include "random.h" #include "compression.h" #include "proxy.h" +#include "address_cache.h" #include "ed25519/sha512.h" #include "keys.h" @@ -139,6 +140,22 @@ static bool finalize_invitation(connection_t *c, const char *data, uint16_t len) logger(DEBUG_CONNECTIONS, LOG_INFO, "Key successfully received from %s (%s)", c->name, c->hostname); + if(!c->node) { + c->node = lookup_node(c->name); + } + + if(!c->node) { + c->node = new_node(c->name); + c->node->connection = c; + node_add(c->node); + } + + if(!c->node->address_cache) { + c->node->address_cache = open_address_cache(c->node); + } + + add_recent_address(c->node->address_cache, &c->address); + // Call invitation-accepted script environment_t env; char *address, *port; @@ -936,8 +953,7 @@ bool ack_h(connection_t *c, const char *request) { n = lookup_node(c->name); if(!n) { - n = new_node(); - n->name = xstrdup(c->name); + n = new_node(c->name); node_add(n); } else { if(n->connection) { diff --git a/src/protocol_edge.c b/src/protocol_edge.c index 7569e231..d1a2d022 100644 --- a/src/protocol_edge.c +++ b/src/protocol_edge.c @@ -111,14 +111,12 @@ bool add_edge_h(connection_t *c, const char *request) { } if(!from) { - from = new_node(); - from->name = xstrdup(from_name); + from = new_node(from_name); node_add(from); } if(!to) { - to = new_node(); - to->name = xstrdup(to_name); + to = new_node(to_name); node_add(to); } diff --git a/src/protocol_misc.c b/src/protocol_misc.c index 0263d001..b68179dc 100644 --- a/src/protocol_misc.c +++ b/src/protocol_misc.c @@ -68,7 +68,8 @@ bool pong_h(connection_t *c, const char *request) { if(c->outgoing && c->outgoing->timeout) { c->outgoing->timeout = 0; - reset_address_cache(c->outgoing->node->address_cache, &c->address); + reset_address_cache(c->node->address_cache); + add_recent_address(c->node->address_cache, &c->address); } return true; diff --git a/src/protocol_subnet.c b/src/protocol_subnet.c index 78e7eebc..526cd969 100644 --- a/src/protocol_subnet.c +++ b/src/protocol_subnet.c @@ -29,7 +29,6 @@ #include "protocol.h" #include "subnet.h" #include "utils.h" -#include "xalloc.h" bool send_add_subnet(connection_t *c, const subnet_t *subnet) { char netstr[MAXNETSTR]; @@ -85,8 +84,7 @@ bool add_subnet_h(connection_t *c, const char *request) { } if(!owner) { - owner = new_node(); - owner->name = xstrdup(name); + owner = new_node(name); node_add(owner); } diff --git a/src/tincctl.c b/src/tincctl.c index b8155cbd..46073b3a 100644 --- a/src/tincctl.c +++ b/src/tincctl.c @@ -2189,6 +2189,49 @@ int check_port(const char *name) { return 0; } +static bool makedir(const char *path, mode_t mode) { + if(mkdir(path, mode) && errno != EEXIST) { + fprintf(stderr, "Could not create directory %s: %s\n", path, strerror(errno)); + return false; + } + + return true; +} + +bool makedirs(tincd_dir_t dirs) { + if(dirs & DIR_CONFBASE && !makedir(confbase, 0777)) { + return false; + } + + if(dirs & DIR_CONFDIR && !confbase_given && !makedir(confdir, 0755)) { + return false; + } + + if(dirs & DIR_HOSTS && !makedir(hosts_dir, 0777)) { + return false; + } + + char path[PATH_MAX]; + + if(dirs & DIR_INVITATIONS) { + snprintf(path, sizeof(path), "%s" SLASH "invitations", confbase); + + if(!makedir(path, 0700)) { + return false; + } + } + + if(dirs & DIR_CACHE) { + snprintf(path, sizeof(path), "%s" SLASH "%s", confbase, "cache"); + + if(!makedir(path, 0755)) { + return false; + } + } + + return true; +} + static int cmd_init(int argc, char *argv[]) { if(!access(tinc_conf, F_OK)) { fprintf(stderr, "Configuration file %s already exists!\n", tinc_conf); @@ -2234,19 +2277,8 @@ static int cmd_init(int argc, char *argv[]) { return 1; } - if(!confbase_given && mkdir(confdir, 0755) && errno != EEXIST) { - fprintf(stderr, "Could not create directory %s: %s\n", confdir, strerror(errno)); - return 1; - } - - if(mkdir(confbase, 0777) && errno != EEXIST) { - fprintf(stderr, "Could not create directory %s: %s\n", confbase, strerror(errno)); - return 1; - } - - if(mkdir(hosts_dir, 0777) && errno != EEXIST) { - fprintf(stderr, "Could not create directory %s: %s\n", hosts_dir, strerror(errno)); - return 1; + if(!makedirs(DIR_HOSTS | DIR_CONFBASE | DIR_CONFDIR | DIR_CACHE)) { + return false; } FILE *f = fopen(tinc_conf, "w"); diff --git a/src/tincctl.h b/src/tincctl.h index 94df21d8..ee5f056d 100644 --- a/src/tincctl.h +++ b/src/tincctl.h @@ -44,6 +44,14 @@ typedef struct { int type; } var_t; +typedef enum { + DIR_CACHE = 1 << 0, + DIR_CONFBASE = 1 << 1, + DIR_CONFDIR = 1 << 2, + DIR_HOSTS = 1 << 3, + DIR_INVITATIONS = 1 << 4, +} tincd_dir_t; + extern const var_t variables[]; extern size_t rstrip(char *value); @@ -52,5 +60,6 @@ extern bool connect_tincd(bool verbose); extern bool sendline(int fd, const char *format, ...) ATTR_FORMAT(printf, 2, 3); extern bool recvline(int fd, char *line, size_t len); extern int check_port(const char *name); +extern bool makedirs(tincd_dir_t dirs); #endif diff --git a/test/integration/address_cache.py b/test/integration/address_cache.py new file mode 100755 index 00000000..7af5feeb --- /dev/null +++ b/test/integration/address_cache.py @@ -0,0 +1,109 @@ +#!/usr/bin/env python3 + +"""Test recent address cache.""" + +import os +import typing as T +import shutil + +from testlib import check +from testlib.log import log +from testlib.proc import Tinc, Script +from testlib.test import Test + + +def init(ctx: Test) -> T.Tuple[Tinc, Tinc]: + """Create test node.""" + foo, bar = ctx.node(), ctx.node() + + stdin = f""" + init {foo} + set Port 0 + set Address localhost + set DeviceType dummy + set AutoConnect no + """ + foo.cmd(stdin=stdin) + + return foo, bar + + +def connect_nodes(foo: Tinc, bar: Tinc) -> None: + """Start second node and wait for connection.""" + log.info("connect nodes") + bar.cmd("start") + bar[foo.script_up].wait() + foo[bar.script_up].wait() + + +def run_tests(ctx: Test) -> None: + """Run tests.""" + foo, bar = init(ctx) + + log.info("cache directory must exist after init") + check.dir_exists(foo.sub("cache")) + + foo.add_script(Script.TINC_UP) + foo.add_script(Script.INVITATION_ACCEPTED) + foo.start() + + log.info("invite %s to %s", bar, foo) + invite, _ = foo.cmd("invite", bar.name) + invite = invite.strip() + + log.info("join %s to %s", bar, foo) + bar.cmd("join", invite) + + log.info("cache directory must exist after join") + check.dir_exists(bar.sub("cache")) + + log.info("invitee address must be cached after invitation is accepted") + foo[Script.INVITATION_ACCEPTED].wait() + check.file_exists(foo.sub(f"cache/{bar}")) + os.remove(foo.sub(f"cache/{bar}")) + + log.info("configure %s", bar) + bar.cmd("set", "DeviceType", "dummy") + bar.cmd("set", "Port", "0") + + log.info("add host-up scripts") + foo.add_script(bar.script_up) + bar.add_script(foo.script_up) + + connect_nodes(foo, bar) + + log.info("%s must cache %s's public address", bar, foo) + check.file_exists(bar.sub(f"cache/{foo}")) + + log.info("%s must not cache %s's outgoing address", foo, bar) + assert not os.path.exists(foo.sub(f"cache/{bar}")) + + log.info("stop node %s", bar) + bar.cmd("stop") + + log.info("remove %s cache directory", bar) + shutil.rmtree(bar.sub("cache")) + + connect_nodes(foo, bar) + + log.info("make sure %s cache was recreated", bar) + check.file_exists(bar.sub(f"cache/{foo}")) + + log.info("stop nodes") + bar.cmd("stop") + foo.cmd("stop") + + log.info("remove Address from all nodes") + for node in foo, bar: + node.cmd("del", "Address", code=None) + for peer in foo, bar: + node.cmd("del", f"{peer}.Address", code=None) + bar.cmd("add", "ConnectTo", foo.name) + + log.info("make sure connection works using just the cached address") + foo.cmd("start") + connect_nodes(foo, bar) + + +with Test("run address cache tests") as context: + run_tests(context) diff --git a/test/integration/cmd_dump.py b/test/integration/cmd_dump.py index 76186ee6..103ac62d 100755 --- a/test/integration/cmd_dump.py +++ b/test/integration/cmd_dump.py @@ -79,7 +79,7 @@ def dump_pending_invitation(foo: Tinc, bar: Tinc) -> None: check.equals(node, bar.name) -def run_unconnected_tests(foo: Tinc) -> None: +def run_unconnected_tests(foo: Tinc, bar: Tinc) -> None: """Run online tests with unconnected nodes.""" log.info("dump invalid type") @@ -109,11 +109,16 @@ def run_unconnected_tests(foo: Tinc) -> None: check.lines(out, 1) check.is_in("", out) - log.info("dump unconnected nodes") - for arg in (("nodes",), ("reachable", "nodes")): - out, _ = foo.cmd("dump", *arg) - check.lines(out, 1) - check.is_in(f"{foo} id ", out) + log.info("%s knows about %s", foo, bar) + out, _ = foo.cmd("dump", "nodes") + check.lines(out, 2) + check.is_in(f"{foo} id ", out) + check.is_in(f"{bar} id ", out) + + log.info("%s can only reach itself", foo) + out, _ = foo.cmd("dump", "reachable", "nodes") + check.lines(out, 1) + check.is_in(f"{foo} id ", out) def run_connected_tests(foo: Tinc, bar: Tinc) -> None: @@ -180,7 +185,7 @@ def run_tests(ctx: Test) -> None: for sub in SUBNETS_BAR: bar.cmd("add", "Subnet", sub) - run_unconnected_tests(foo) + run_unconnected_tests(foo, bar) log.info("start %s", bar) foo.add_script(bar.script_up) diff --git a/test/integration/cmd_join.py b/test/integration/cmd_join.py index 585afeed..a9bdd52d 100755 --- a/test/integration/cmd_join.py +++ b/test/integration/cmd_join.py @@ -78,18 +78,18 @@ def test_invite_errors(foo: Tinc) -> None: check.is_in("already exists", err) if os.name != "nt": + log.info("bad permissions on invitations are fixed") invites = foo.sub("invitations") os.chmod(invites, 0) - _, err = foo.cmd("invite", "foobar", code=1) - check.is_in("Could not read directory", err) - os.chmod(invites, 0o750) + out, _ = foo.cmd("invite", "foobar") + check.has_prefix(out, "localhost:") - log.info("block creating invitations directory") - shutil.rmtree(foo.sub("invitations")) + log.info("invitations directory is created with bad permissions on parent") + shutil.rmtree(invites) os.chmod(foo.work_dir, 0o500) - _, err = foo.cmd("invite", "foobar", code=1) - check.is_in("Could not create directory", err) - os.chmod(foo.work_dir, 0o750) + out, _ = foo.cmd("invite", "foobar") + check.has_prefix(out, "localhost:") + check.true(os.access(invites, os.W_OK)) log.info("fully block access to configuration directory") work_dir = foo.sub("test_no_access") @@ -122,11 +122,12 @@ def test_join_errors(foo: Tinc) -> None: check.is_in("Could not connect to", err) if os.name != "nt": - log.info("test working without access to configuration directory") + log.info("bad permissions on configuration directory are fixed") work_dir = foo.sub("wd_access_test") os.mkdir(work_dir, mode=400) _, err = foo.cmd("-c", work_dir, "join", FAKE_INVITE, code=1) - check.is_in("No permission to write", err) + check.is_in("Could not connect to", err) + check.true(os.access(work_dir, mode=os.W_OK)) with Test("run invite success tests") as context: diff --git a/test/integration/meson.build b/test/integration/meson.build index 3414ddb5..899de35e 100644 --- a/test/integration/meson.build +++ b/test/integration/meson.build @@ -1,4 +1,5 @@ tests = [ + 'address_cache.py', 'basic.py', 'cmd_dump.py', 'cmd_fsck.py', diff --git a/test/integration/testlib/check.py b/test/integration/testlib/check.py index 3b7dec17..0f3b414b 100755 --- a/test/integration/testlib/check.py +++ b/test/integration/testlib/check.py @@ -104,6 +104,12 @@ def files_eq(path0: str, path1: str) -> None: def file_exists(path: T.Union[str, Path]) -> None: - """Check that file or directory exists.""" - if not os.path.exists(path): - raise ValueError("expected path '{path}' to exist") + """Check that file exists.""" + if not os.path.isfile(path): + raise ValueError(f"expected file '{path}' to exist") + + +def dir_exists(path: T.Union[str, Path]) -> None: + """Check that directory exists.""" + if not os.path.isdir(path): + raise ValueError(f"expected directory '{path}' to exist") diff --git a/test/unit/test_graph.c b/test/unit/test_graph.c index 5d72104b..e09f2fcb 100644 --- a/test/unit/test_graph.c +++ b/test/unit/test_graph.c @@ -20,8 +20,7 @@ static void connect_nodes(node_t *from, node_t *to, int weight) { } static node_t *make_node(const char *name) { - node_t *node = new_node(); - node->name = xstrdup(name); + node_t *node = new_node(name); node->status.reachable = true; node_add(node); return node; @@ -77,8 +76,7 @@ static void test_sssp_bfs(void **state) { static int setup(void **state) { (void)state; - myself = new_node(); - myself->name = xstrdup("myself"); + myself = new_node("myself"); return 0; }