Improve recently seen address cache
authorKirill Isakov <bootctl@gmail.com>
Thu, 19 May 2022 07:04:20 +0000 (13:04 +0600)
committerKirill Isakov <bootctl@gmail.com>
Sun, 29 May 2022 08:43:37 +0000 (14:43 +0600)
- create cache directory on init
- only remember addresses for TCP connections
- update cache in more situations
- add tests

21 files changed:
src/address_cache.c
src/address_cache.h
src/graph.c
src/invitation.c
src/net_packet.c
src/net_setup.c
src/net_socket.c
src/node.c
src/node.h
src/protocol_auth.c
src/protocol_edge.c
src/protocol_misc.c
src/protocol_subnet.c
src/tincctl.c
src/tincctl.h
test/integration/address_cache.py [new file with mode: 0755]
test/integration/cmd_dump.py
test/integration/cmd_join.py
test/integration/meson.build
test/integration/testlib/check.py
test/unit/test_graph.c

index d9996fb..f28dbf9 100644 (file)
@@ -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;
index 1f891c9..36ffa5b 100644 (file)
@@ -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
index c100a93..5a7a16a 100644 (file)
@@ -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",
index a95a324..08f053a 100644 (file)
@@ -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)) {
index 6fea959..0b95a93 100644 (file)
@@ -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.
index 3ac5a67..2cd5818 100644 (file)
@@ -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)) {
index 4c76c30..982bc80 100644 (file)
@@ -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);
                        }
 
index 8e0f162..3e37b8e 100644 (file)
@@ -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;
 }
index fea90d1..51322c2 100644 (file)
@@ -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);
index 7a3e6dd..263a131 100644 (file)
@@ -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) {
index 7569e23..d1a2d02 100644 (file)
@@ -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);
        }
 
index 0263d00..b68179d 100644 (file)
@@ -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;
index 78e7eeb..526cd96 100644 (file)
@@ -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);
        }
 
index b8155cb..46073b3 100644 (file)
@@ -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");
index 94df21d..ee5f056 100644 (file)
@@ -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 (executable)
index 0000000..7af5fee
--- /dev/null
@@ -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)
index 76186ee..103ac62 100755 (executable)
@@ -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("<control>", 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)
index 585afee..a9bdd52 100755 (executable)
@@ -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:
index 3414ddb..899de35 100644 (file)
@@ -1,4 +1,5 @@
 tests = [
+  'address_cache.py',
   'basic.py',
   'cmd_dump.py',
   'cmd_fsck.py',
index 3b7dec1..0f3b414 100755 (executable)
@@ -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")
index 5d72104..e09f2fc 100644 (file)
@@ -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;
 }