comm,site: Make peer address configuration the responsibility of comm (udp)

We want to be able to introduce new kinds of comm.  These may need to
be configured differently to the current udp comm, which expects to be
given a sockaddr_in.

So move the knowledge of how to configure a peer address (ie, which
keys to look up in the site dictionary) to udp.  udp gets passed
site's dictionary during configuration setup, and extracts the
relevant information.  site can then later ask udp to (try to) find
the peer address.

As a side-effect, the resolver key now belongs to comm (ie, udp), not
to the site.  This is a backwards-incompatible change because it means
that (in the usual kind of situation) the resolver must be defined
before the udp comm in the config file.  I don't know what proportion
of existing config files need adjusting but certainly test-example
did.

We could avoid the backward-compatibility problem by having udp look
up the "resolver" key in the supplied site dictionary, and stash the
result away, but this would result in a more-convoluted configuration
setup.

Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>

diff --git a/README b/README
index 1901223e149ae4bf85ee447371dda9a95ee499b4..12a8469fdfd33e3ee792cd7e1d906918cd1283f1 100644 (file)
--- a/README
+++ b/README
@@ -201,9 +201,18 @@ Defines:
 udp: dict argument
   address (string): IP address to listen and send on
   port (integer): UDP port to listen and send on
+  resolver (resolver closure)
   buffer (buffer closure): buffer for incoming packets
   authbind (string): optional, path to authbind-helper program
 
+ udp looks up the following keys in its calling closure (usually, this
+ is a site closure) when a peer needs to be found:
+
+  address (string): optional, DNS name used to find our peer
+  port (integer): mandatory if 'address' is specified: the port used
+    to contact our peer
+
+
 ** log
 
 Defines:
@@ -274,12 +283,8 @@ site: dict argument
    first one will be used for any key setups initiated by us using the
    configured address.  Others are only used if our peer talks to
    them.
-  resolver (resolver closure)
   random (randomsrc closure)
   local-key (rsaprivkey closure)
-  address (string): optional, DNS name used to find our peer
-  port (integer): mandatory if 'address' is specified: the port used
-    to contact our peer
   key (rsapubkey closure): our peer's public key
   transform (transform closure): how to mangle packets sent between sites
   dh (dh closure)
@@ -334,6 +339,7 @@ site: dict argument
     check that there are no links both ends of which are allegedly
     mobile (which is not supported, so those links are ignored) and
     to change some of the tuning parameter defaults. [false]
+  Plus further keys depending on the first comm.
 
 Links involving mobile peers have some different tuning parameter
 default values, which are generally more aggressive about retrying key

diff --git a/secnet.h b/secnet.h
index d330f64c5323bc720b1379fd6333455b33ea4691..9d0b09e9dd60d5e5bd86dcb1b53cf438573b2655 100644 (file)
--- a/secnet.h
+++ b/secnet.h
@@ -325,12 +325,18 @@ typedef void comm_release_notify_fn(void *commst, void *nst,
                                    comm_notify_fn *fn);
 typedef bool_t comm_sendmsg_fn(void *commst, struct buffer_if *buf,
                               const struct comm_addr *dest);
-typedef bool_t comm_addr_construct_fn(void *commst, struct comm_addr *ca,
-                                     const struct sockaddr_in *sin);
-       /* Fills in ca->comm an ca->priv.  *sin must have no nonzero padding,
-          so must have been FILLZERO'd before its useful fields were filled in.
-          The resulting ca->comm has been constructed likewise, as required
-          above.  On failure, logs error, returns false on failure.*/
+typedef void *comm_peer_addr_config_fn(void *commst, dict_t*, struct cloc,
+                                      cstring_t desc);
+       /* Checks the config for the peer as specified in dict,
+        * and either returns NULL to mean the relevant parameters
+        * weren't specified, or a non-NULL void* which can be
+        * passed to comm_peer_addr_fn */
+typedef void comm_peer_addr_answer_fn(void *st, const struct comm_addr *ca);
+typedef bool_t comm_peer_addr_request_fn(void *commst, void *from_peer_config,
+                                comm_peer_addr_answer_fn *cb, void *cst);
+       /* Calls comm_peer_addr_answer_fn, either immediately or later,
+        * with a suitable comm_addr.  The caller's cb should
+        * memcpy ca.  On failure, logs error and calls cb with ca=NULL */
 typedef const char *comm_addr_to_string_fn(void *commst,
                                           const struct comm_addr *ca);
         /* Returned string is in a static buffer. */
@@ -341,7 +347,8 @@ struct comm_if {
     comm_request_notify_fn *request_notify;
     comm_release_notify_fn *release_notify;
     comm_sendmsg_fn *sendmsg;
-    comm_addr_construct_fn *addr_construct;
+    comm_peer_addr_config_fn *peer_addr_config;
+    comm_peer_addr_request_fn *peer_addr_request;
     comm_addr_to_string_fn *addr_to_string;
 };
 

diff --git a/site.c b/site.c
index 40819a4ab43a653d99a0df216e8d2f8faae5106a..88924009618bc6189c1c47458125cf057a9137a6 100644 (file)
--- a/site.c
+++ b/site.c
@@ -223,12 +223,10 @@ struct site {
     bool_t peer_mobile; /* Mobile client support */
     int32_t transport_peers_max;
     string_t tunname; /* localname<->remotename by default, used in logs */
-    string_t address; /* DNS name for bootstrapping, optional */
-    int remoteport; /* Port for bootstrapping, optional */
+    void *remote_addr_config; /* created and used by comm */
     struct netlink_if *netlink;
     struct comm_if **comms;
     int ncomms;
-    struct resolver_if *resolver;
     struct log_if *log;
     struct random_if *random;
     struct rsaprivkey_if *privkey;
@@ -793,28 +791,18 @@ static bool_t send_msg(struct site *st)
     }
 }
 
-static void site_resolve_callback(void *sst, struct in_addr *address)
+static void site_resolve_callback(void *sst, const struct comm_addr *address)
 {
     struct site *st=sst;
-    struct comm_addr ca_buf, *ca_use=0;
 
     if (st->state!=SITE_RESOLVE) {
        slog(st,LOG_UNEXPECTED,"site_resolve_callback called unexpectedly");
        return;
     }
-    if (address) {
-       struct sockaddr_in sin;
-       FILLZERO(sin);
-       sin.sin_family=AF_INET;
-       sin.sin_port=htons(st->remoteport);
-       sin.sin_addr=*address;
-       if (st->comms[0]->addr_construct(st->comms[0]->st, &ca_buf, &sin)) {
-           ca_use=&ca_buf;
-       }
-    } else {
-       slog(st,LOG_ERROR,"resolution of %s failed",st->address);
+    if (!address) {
+       slog(st,LOG_ERROR,"resolution of peer address failed");
     }
-    if (transport_compute_setupinit_peers(st,ca_use)) {
+    if (transport_compute_setupinit_peers(st,address)) {
        enter_new_state(st,SITE_SENTMSG1);
     } else {
        /* Can't figure out who to try to to talk to */
@@ -827,7 +815,7 @@ static bool_t initiate_key_setup(struct site *st, cstring_t reason)
 {
     if (st->state!=SITE_RUN) return False;
     slog(st,LOG_SETUP_INIT,"initiating key exchange (%s)",reason);
-    if (st->address) {
+    if (st->remote_addr_config) {
        slog(st,LOG_SETUP_INIT,"resolving peer address");
        return enter_state_resolve(st);
     } else if (transport_compute_setupinit_peers(st,0)) {
@@ -891,7 +879,7 @@ static void set_link_quality(struct site *st)
        quality=LINK_QUALITY_UP;
     else if (st->state==SITE_WAIT || st->state==SITE_STOP)
        quality=LINK_QUALITY_DOWN;
-    else if (st->address)
+    else if (st->remote_addr_config)
        quality=LINK_QUALITY_DOWN_CURRENT_ADDRESS;
     else if (transport_peers_valid(&st->peers))
        quality=LINK_QUALITY_DOWN_STALE_ADDRESS;
@@ -922,8 +910,8 @@ static bool_t enter_state_resolve(struct site *st)
     state_assert(st,st->state==SITE_RUN);
     slog(st,LOG_STATE,"entering state RESOLVE");
     st->state=SITE_RESOLVE;
-    st->resolver->request(st->resolver->st,st->address,
-                         site_resolve_callback,st);
+    st->comms[0]->peer_addr_request(st->comms[0]->st,st->remote_addr_config,
+                                   site_resolve_callback,st);
     return True;
 }
 
@@ -1352,15 +1340,12 @@ static list_t *site_apply(closure_t *self, struct cloc loc, dict_t *context,
        st->comms[i]=cl->interface;
     }
 
-    st->resolver=find_cl_if(dict,"resolver",CL_RESOLVER,True,"site",loc);
     st->log=find_cl_if(dict,"log",CL_LOG,True,"site",loc);
     st->random=find_cl_if(dict,"random",CL_RANDOMSRC,True,"site",loc);
 
+    st->remote_addr_config=st->comms[0]->peer_addr_config(
+       st->comms[0]->st,dict,loc,"site");
     st->privkey=find_cl_if(dict,"local-key",CL_RSAPRIVKEY,True,"site",loc);
-    st->address=dict_read_string(dict, "address", False, "site", loc);
-    if (st->address)
-       st->remoteport=dict_read_number(dict,"port",True,"site",loc,0);
-    else st->remoteport=0;
     st->pubkey=find_cl_if(dict,"key",CL_RSAPUBKEY,True,"site",loc);
 
     st->transform=

diff --git a/test-example/common.conf b/test-example/common.conf
index 64bfc819436c1c1cd9bd67ad476ad28f60b12f65..47b3430133c56e576c720c14cf9f0709ed507b84 100644 (file)
--- a/test-example/common.conf
+++ b/test-example/common.conf
@@ -4,8 +4,6 @@ log logfile {
 };
 system {
 };
-resolver adns {
-};
 log-events "all";
 random randomfile("/dev/urandom",no);
 transform serpent256-cbc {

diff --git a/test-example/inside.conf b/test-example/inside.conf
index a64f1252cbf1c19727fff85f8d8de737ff58b872..4d5913dfef4eb2d5fcb7bc9184bd4a5e18957207 100644 (file)
--- a/test-example/inside.conf
+++ b/test-example/inside.conf
@@ -1,3 +1,4 @@
+include test-example/top.conf
 netlink tun {
        name "netlink-tun"; # Printed in log messages from this netlink
        local-address "172.18.232.9";

diff --git a/test-example/outside.conf b/test-example/outside.conf
index db78b7b76a2c8370801cbe77eaaddcf71a9d600e..df4aba946f69601dfeff135b7e246729ad70da19 100644 (file)
--- a/test-example/outside.conf
+++ b/test-example/outside.conf
@@ -1,3 +1,4 @@
+include test-example/top.conf
 netlink tun {
        name "netlink-tun"; # Printed in log messages from this netlink
        local-address "172.18.232.1";

diff --git a/test-example/top.conf b/test-example/top.conf
new file mode 100644 (file)
index 0000000..a9ccc81
--- /dev/null
+++ b/test-example/top.conf
@@ -0,0 +1,2 @@
+resolver adns {
+};

diff --git a/udp.c b/udp.c
index a1d3b74f5b1b6d7d1963f9e38c8048622be7f844..14311353472e7a7b65783e716519203135206162 100644 (file)
--- a/udp.c
+++ b/udp.c
@@ -34,6 +34,11 @@ struct notify_list {
     struct notify_list *next;
 };
 
+struct peer_config {
+    string_t address; /* DNS name for bootstrapping, optional */
+    int remoteport; /* Port for bootstrapping, optional */
+};
+
 struct udp {
     closure_t cl;
     struct comm_if ops;
@@ -44,6 +49,7 @@ struct udp {
     string_t authbind;
     struct buffer_if *rbuf;
     struct notify_list *notify;
+    struct resolver_if *resolver;
     bool_t use_proxy;
     struct sockaddr_in proxy;
 };
@@ -72,13 +78,63 @@ static const char *addr_to_string(void *commst, const struct comm_addr *ca) {
     return sbuf;
 }
 
-static bool_t addr_construct(void *commst, struct comm_addr *ca,
-                            const struct sockaddr_in *sin) {
+struct peer_addr_cst {
+    struct udp *udp;
+    struct peer_config *cfg;
+    comm_peer_addr_answer_fn *cb;
+    void *cst;
+};
+
+static void *peer_addr_config(void *commst, dict_t *dict, struct cloc loc,
+                             cstring_t desc)
+{
+    struct peer_config *cfg;
+    cfg=safe_malloc(sizeof(*cfg),"peer addr config");
+    cfg->address=dict_read_string(dict, "address",False,desc,loc);
+    if (cfg->address)
+       cfg->remoteport=dict_read_number(dict,"port",True,desc,loc,0);
+    else cfg->remoteport=0;
+    if (cfg->address)
+       return cfg;
+    free(cfg);
+    return NULL;
+}
+
+static void peer_resolve_callback(void *sst, struct in_addr *address)
+{
+    struct peer_addr_cst *pacst=sst;
+    struct udp *st=pacst->udp;
+    struct peer_config *cfg=pacst->cfg;
+    struct comm_addr ca;
+    if (address) {
+       FILLZERO(ca);
+       ca.comm=&st->ops;
+       ca.priv.sin.sin_family=AF_INET;
+       ca.priv.sin.sin_port=htons(cfg->remoteport);
+       ca.priv.sin.sin_addr=*address;
+       pacst->cb(pacst->cst,&ca);
+    } else {
+       pacst->cb(pacst->cst,0);
+    }
+    free(pacst);
+}
+
+static bool_t peer_addr_request(void *commst, void *from_peer_config,
+                               comm_peer_addr_answer_fn *cb, void *cst)
+{
     struct udp *st=commst;
-    FILLZERO(*ca);
-    ca->comm=&st->ops;
-    ca->priv.sin=*sin;
-    return True;
+    struct peer_config *cfg=from_peer_config;
+    struct peer_addr_cst *pacst;
+    pacst=safe_malloc(sizeof(*pacst),"udp peer addr request cst");
+    pacst->udp=st;
+    pacst->cfg=cfg;
+    pacst->cb=cb;
+    pacst->cst=cst;
+    bool_t ok=st->resolver->request(st->resolver->st,cfg->address,
+                                   peer_resolve_callback,pacst);
+    if (!ok)
+       free(pacst);
+    return ok;
 }
 
 static int udp_beforepoll(void *state, struct pollfd *fds, int *nfds_io,
@@ -302,7 +358,8 @@ static list_t *udp_apply(closure_t *self, struct cloc loc, dict_t *context,
     st->ops.request_notify=request_notify;
     st->ops.release_notify=release_notify;
     st->ops.sendmsg=udp_sendmsg;
-    st->ops.addr_construct=addr_construct;
+    st->ops.peer_addr_config=peer_addr_config;
+    st->ops.peer_addr_request=peer_addr_request;
     st->ops.addr_to_string=addr_to_string;
     st->port=0;
     st->use_proxy=False;
@@ -336,6 +393,7 @@ static list_t *udp_apply(closure_t *self, struct cloc loc, dict_t *context,
        st->proxy.sin_port=htons(i->data.number);
        st->ops.min_start_pad=8;
     }
+    st->resolver=find_cl_if(d,"resolver",CL_RESOLVER,True,"comm",loc);
 
     add_hook(PHASE_GETRESOURCES,udp_phase_hook,st);