On Sun, Jul 19, 2015 at 01:08:46PM +0200, Gregor Best wrote:
> Hello,
>
> the following is a patch that adds an option called `update_unbound' to
> dhclient.conf. With this option enabled, dhclient will call
>
> unbound-control forwards <ns1> <ns2> <ns3>
>
> instead of rewriting /etc/resolv.conf.
>
> My usage scenario is that I'm running unbound on my laptop as a local
> resolver. /etc/resolv.conf is configured to only use 127.0.0.1 as the
> nameserver.
>
Oh god, yes please. I always wanted to write this diff myself ;)
More comments in the diff below.
> I use this because I have a few VPN connections that have custom DNS
> namespaces, which I manage with Unbound's forward zones. Since
> sometimes I am in networks that have a split horizon DNS (e.g.
> University, hackerspace, etc...), I can't use a statically configured
> fallback forward zone for all my requests. Manually calling using
> unbound-control to change the forward DNS became too annoying, so I
> wrote this patch.
>
> Does the patch make sense in OpenBSD or should I keep it as a local
> change?
>
> --
> Gregor Best
> --
>
> Index: clparse.c
> ===================================================================
> RCS file: /mnt/media/cvs/src/sbin/dhclient/clparse.c,v
> retrieving revision 1.92
> diff -u -p -r1.92 clparse.c
> --- clparse.c 18 May 2015 17:51:21 -0000 1.92
> +++ clparse.c 9 Jul 2015 15:00:05 -0000
> @@ -75,6 +75,7 @@ read_client_conf(void)
> config->backoff_cutoff = 15;
> config->initial_interval = 3;
> config->bootp_policy = ACCEPT;
> + config->update_unbound = 0;
> config->requested_options
> [config->requested_option_count++] = DHO_SUBNET_MASK;
> config->requested_options
> @@ -269,6 +270,10 @@ parse_client_statement(FILE *cfile)
> case TOK_NEXT_SERVER:
> if (parse_ip_addr(cfile, &config->next_server))
> parse_semi(cfile);
> + break;
> + case TOK_UPDATE_UNBOUND:
> + config->update_unbound = 1;
> + parse_semi(cfile);
> break;
> default:
> parse_warn("expecting a statement.");
> Index: conflex.c
> ===================================================================
> RCS file: /mnt/media/cvs/src/sbin/dhclient/conflex.c,v
> retrieving revision 1.32
> diff -u -p -r1.32 conflex.c
> --- conflex.c 18 May 2015 17:51:21 -0000 1.32
> +++ conflex.c 9 Jul 2015 14:54:56 -0000
> @@ -341,7 +341,8 @@ static const struct keywords {
> { "send", TOK_SEND },
> { "server-name", TOK_SERVER_NAME },
> { "supersede", TOK_SUPERSEDE },
> - { "timeout", TOK_TIMEOUT }
> + { "timeout", TOK_TIMEOUT },
> + { "update_unbound", TOK_UPDATE_UNBOUND }
Use update-unbound (minus, not underscrore) for the keyword; also
update the man page.
> };
>
> int kw_cmp(const void *k, const void *e);
> Index: dhclient.c
> ===================================================================
> RCS file: /mnt/media/cvs/src/sbin/dhclient/dhclient.c,v
> retrieving revision 1.361
> diff -u -p -r1.361 dhclient.c
> --- dhclient.c 18 May 2015 14:59:42 -0000 1.361
> +++ dhclient.c 9 Jul 2015 15:52:41 -0000
> @@ -97,6 +97,7 @@ int res_hnok(const char *dn);
>
> void fork_privchld(int, int);
> void get_ifname(char *);
> +void update_unbound_forwards(struct option_data *);
> char *resolv_conf_contents(struct option_data *,
> struct option_data *);
> void write_resolv_conf(u_int8_t *, size_t);
> @@ -903,8 +904,12 @@ bind_lease(void)
> goto newlease;
> }
>
> - client->new->resolv_conf = resolv_conf_contents(
> - &options[DHO_DOMAIN_NAME], &options[DHO_DOMAIN_NAME_SERVERS]);
> + if (config->update_unbound) {
> + update_unbound_forwards(&options[DHO_DOMAIN_NAME_SERVERS]);
> + } else {
> + client->new->resolv_conf = resolv_conf_contents(
> + &options[DHO_DOMAIN_NAME],
> &options[DHO_DOMAIN_NAME_SERVERS]);
> + }
>
> /* Replace the old active lease with the new one. */
> client->active = client->new;
> @@ -2042,6 +2047,88 @@ get_ifname(char *arg)
> close(s);
> } else if (strlcpy(ifi->name, arg, IFNAMSIZ) >= IFNAMSIZ)
> error("Interface name too long");
> +}
> +
> +/*
> + * Update unbound forwarder list
> + */
> +void
> +update_unbound_forwards(struct option_data *nameservers)
> +{
> + char *ns;
> + int rslt;
> +
> + if (!nameservers->len) {
> + return;
> + }
> +
> + ns = pretty_print_option(DHO_DOMAIN_NAME_SERVERS, nameservers, 0);
> +
> + rslt = imsg_compose(unpriv_ibuf, IMSG_UPDATE_UNBOUND_FORWARDS,
> + 0, 0, -1, ns, strlen(ns) + 1);
> +
> + if (rslt == -1) {
> + warning("update_unbound_forwards: imsg_compose: %s",
> + strerror(errno));
> + }
> +
> + flush_unpriv_ibuf("update_unbound_forwards");
> +}
> +
> +void
> +priv_update_unbound_forwards(struct imsg *imsg)
> +{
> + char *args[MAXNS + 3]; /* `unbound-control', `forward', final NULL */
> + char *ns, *p;
> + int i, rslt;
> + size_t sz;
> + pid_t child;
> +
> + if (imsg->hdr.len < IMSG_HEADER_SIZE) {
> + warning("short IMSG_UPDATE_UNBOUND_FORWARDS");
> + return;
> + }
> +
> + ns = imsg->data;
> + sz = imsg->hdr.len - IMSG_HEADER_SIZE;
> + ns[sz] = '\0'; /* Make sure we're terminated properly */
> +
> + /* Construct unbound-control arguments */
> + memset(args, 0, sizeof(args));
> + args[0] = "unbound-control";
> + args[1] = "forward";
> + for (i = 0; i < MAXNS; i++) {
> + p = strsep(&ns, " ");
> + if (p == NULL)
> + break;
> + if (*p == '\0')
> + continue;
I think you should rejiggle this loop. I don't think you will have the
case where you have two seperators in a row because the sender in
dhclient sanatizes this. However, if you do end up with two seperators
suddenly you have a NULL arg in the middle and you'll lose the rest of
the args.
> + rslt = asprintf(&args[i + 2], "%s", p);
> + if (rslt == -1)
> + error("no memory for nameserver");
> + }
> +
> + switch ((child = fork())) {
> + case -1:
> + error("cannot fork");
> + break;
> + case 0:
> + fclose(stdout); /* Prevent noise from unbound-control */
> + execvp("unbound-control", args);
> + error("execve failed: %s", strerror(errno));
> + break;
> + default:
> + if (waitpid(child, NULL, 0) < 0) {
> + error("waitpid: %s", strerror(errno));
> + }
> + break;
> + }
> +
> + for (i = 0; i < MAXNS; i++) {
> + if (!args[i + 2])
> + continue;
If you rejiggle the loop above this can be a break.
> + free(args[i + 2]);
> + }
> }
>
> /*
> Index: dhclient.conf.5
> ===================================================================
> RCS file: /mnt/media/cvs/src/sbin/dhclient/dhclient.conf.5,v
> retrieving revision 1.31
> diff -u -p -r1.31 dhclient.conf.5
> --- dhclient.conf.5 11 Nov 2013 15:39:20 -0000 1.31
> +++ dhclient.conf.5 9 Jul 2015 16:24:32 -0000
> @@ -406,6 +406,16 @@ specified name.
> Interfaces for which there is no interface declaration will use the
> parameters declared outside of any interface declaration,
> or the default settings.
> +.It Ic update_unbound ;
> +The
> +.Ic update_unbound
> +statement causes dhclient to update the unbound forward DNS list with
> received
> +name servers instead of updating /etc/resolv.conf.
> +This is essentially the same as calling
> +.Pp
> +.D1 unbound-control forward <ns1> <ns2> <ns3>
> +.Pp
> +after acquiring a lease that contains name server information.
> .El
> .Sh EXAMPLES
> The following configuration file is used on a laptop
> Index: dhcpd.h
> ===================================================================
> RCS file: /mnt/media/cvs/src/sbin/dhclient/dhcpd.h,v
> retrieving revision 1.150
> diff -u -p -r1.150 dhcpd.h
> --- dhcpd.h 18 May 2015 14:59:42 -0000 1.150
> +++ dhcpd.h 9 Jul 2015 14:51:11 -0000
> @@ -146,6 +146,7 @@ struct client_config {
> time_t backoff_cutoff;
> enum { IGNORE, ACCEPT, PREFER }
> bootp_policy;
> + int update_unbound: 1;
I see that dhclient does the :1 in other structs. It's not
particularly sensible here, just use an int.
> TAILQ_HEAD(, reject_elem) reject_list;
> char *resolv_tail;
> char *filename;
> Index: dhctoken.h
> ===================================================================
> RCS file: /mnt/media/cvs/src/sbin/dhclient/dhctoken.h,v
> retrieving revision 1.9
> diff -u -p -r1.9 dhctoken.h
> --- dhctoken.h 5 Dec 2013 22:31:35 -0000 1.9
> +++ dhctoken.h 9 Jul 2015 14:55:15 -0000
> @@ -78,6 +78,7 @@
> #define TOK_REJECT 292
> #define TOK_LINK_TIMEOUT 294
> #define TOK_IGNORE 295
> +#define TOK_UPDATE_UNBOUND 296
>
> #define is_identifier(x) ((x) >= TOK_FIRST_TOKEN && \
> (x) != TOK_STRING && \
> Index: privsep.c
> ===================================================================
> RCS file: /mnt/media/cvs/src/sbin/dhclient/privsep.c,v
> retrieving revision 1.39
> diff -u -p -r1.39 privsep.c
> --- privsep.c 7 Feb 2015 10:08:06 -0000 1.39
> +++ privsep.c 9 Jul 2015 15:20:34 -0000
> @@ -93,6 +93,9 @@ dispatch_imsg(struct imsgbuf *ibuf)
> case IMSG_WRITE_OPTION_DB:
> priv_write_option_db(&imsg);
> break;
> + case IMSG_UPDATE_UNBOUND_FORWARDS:
> + priv_update_unbound_forwards(&imsg);
> + break;
>
> default:
> warning("received unknown message, code %u",
> Index: privsep.h
> ===================================================================
> RCS file: /mnt/media/cvs/src/sbin/dhclient/privsep.h,v
> retrieving revision 1.28
> diff -u -p -r1.28 privsep.h
> --- privsep.h 10 Feb 2015 04:20:26 -0000 1.28
> +++ privsep.h 9 Jul 2015 15:21:18 -0000
> @@ -19,6 +19,7 @@
> #include <arpa/inet.h>
>
> #include <imsg.h>
> +#include <resolv.h>
>
> enum imsg_code {
> IMSG_NONE,
> @@ -29,7 +30,8 @@ enum imsg_code {
> IMSG_SET_INTERFACE_MTU,
> IMSG_HUP,
> IMSG_WRITE_RESOLV_CONF,
> - IMSG_WRITE_OPTION_DB
> + IMSG_WRITE_OPTION_DB,
> + IMSG_UPDATE_UNBOUND_FORWARDS
> };
>
> struct imsg_delete_address {
> @@ -72,3 +74,4 @@ void priv_set_interface_mtu(struct imsg_
> void priv_write_resolv_conf(struct imsg *);
> void priv_write_option_db(struct imsg *);
> void priv_write_file(char *, int, mode_t, uid_t, gid_t, u_int8_t *, size_t);
> +void priv_update_unbound_forwards(struct imsg *);
>
--
I'm not entirely sure you are real.
