Simplify trust anchor handling.

Open trust anchor file for reading and writing on startup and pass it
to the frontend process. The frontend process seeks and truncates the
file apropriately when writing out new trust anchors learned via DNS
but never closes the file. On error the file is truncated to zero
length.

This is in turn handled on startup by switching to the built in trust
anchor when no trustanchor can be read from disk.

This side steps the need for an unveil'ed directory with "c" permission
and also removes the wpath and cpath pledges from the parent process.

deraadt@ pointed out that my previous design didn't make sense and I
had confused myself along the way. (It did work, but was too
complicated for no good reason).

While here validate that we actually read a trust anchor from disk by
trying to parse it and checking that it is a DNSKEY. Unfortunately
ub_ctx_add_ta() accepts just any string as a trust anchor without any
validation.

3 files changed, 83 insertions, 148 deletions

diff --git a/sbin/unwind/frontend.c b/sbin/unwind/frontend.c
index 6fb169e7717..c070dfa0cda 100644
--- a/sbin/unwind/frontend.c
+++ b/sbin/unwind/frontend.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: frontend.c,v 1.11 2019/02/07 17:20:35 florian Exp $	*/
+/*	$OpenBSD: frontend.c,v 1.12 2019/02/10 14:10:22 florian Exp $	*/
 
 /*
  * Copyright (c) 2018 Florian Obser <florian@openbsd.org>
@@ -45,6 +45,7 @@
 #include "libunbound/config.h"
 #include "libunbound/sldns/pkthdr.h"
 #include "libunbound/sldns/sbuffer.h"
+#include "libunbound/sldns/str2wire.h"
 #include "libunbound/sldns/wire2str.h"
 
 #include "uw_log.h"
@@ -98,6 +99,7 @@ struct imsgev		*iev_resolver;
 struct imsgev		*iev_captiveportal;
 struct event		 ev_route;
 int			 udp4sock = -1, udp6sock = -1, routesock = -1;
+int			 ta_fd = -1;
 
 static struct trust_anchor_head	 built_in_trust_anchors;
 static struct trust_anchor_head	 trust_anchors, new_trust_anchors;
@@ -473,20 +475,13 @@ frontend_dispatch_main(int fd, short event, void *bula)
 			parse_dhcp_lease(fd);
 			break;
 		case IMSG_TAFD:
-			if ((fd = imsg.fd) != -1)
-				parse_trust_anchor(&trust_anchors, fd);
+			if ((ta_fd = imsg.fd) != -1)
+				parse_trust_anchor(&trust_anchors, ta_fd);
 			if (!TAILQ_EMPTY(&trust_anchors))
 				send_trust_anchors(&trust_anchors);
 			else
 				send_trust_anchors(&built_in_trust_anchors);
 			break;
-		case IMSG_TAFD_W:
-			if ((fd = imsg.fd) == -1)
-				fatalx("%s: expected to receive imsg trust "
-				    "anchor fd but didn't receive any",
-				    __func__);
-			write_trust_anchors(&trust_anchors, fd);
-			break;
 		default:
 			log_debug("%s: error handling imsg %d", __func__,
 			    imsg.hdr.type);
@@ -605,8 +600,8 @@ frontend_dispatch_resolver(int fd, short event, void *bula)
 			 * always write trust anchors, the modify date on
 			 * the file is an indication when we made progress
 			 */
-			frontend_imsg_compose_main(IMSG_OPEN_TA_W, 0, NULL,
-			    0);
+			if (ta_fd != -1)
+				write_trust_anchors(&trust_anchors, ta_fd);
 			break;
 		default:
 			log_debug("%s: error handling imsg %d", __func__,
@@ -677,8 +672,6 @@ frontend_startup(void)
 
 	event_add(&ev_route, NULL);
 
-	frontend_imsg_compose_main(IMSG_OPEN_TA_RO, 0, NULL, 0);
-
 	frontend_imsg_compose_main(IMSG_STARTUP_DONE, 0, NULL, 0);
 	rtmget_default();
 }
@@ -1158,36 +1151,53 @@ merge_tas(struct trust_anchor_head *newh, struct trust_anchor_head *oldh)
 void
 parse_trust_anchor(struct trust_anchor_head *tah, int fd)
 {
-	FILE	*f;
-	char	*line = NULL, *p;
-	size_t	 linesize = 0;
-	ssize_t	 linelen;
+	size_t	 len, dname_len;
+	ssize_t	 n, sz;
+	uint8_t	 rr[LDNS_RR_BUF_SIZE];
+	char	*str, *p, buf[512], *line;
+
+	sz = 0;
+	str = NULL;
+
+	while ((n = read(fd, buf, sizeof(buf))) > 0) {
+		p = recallocarray(str, sz, sz + n, 1);
+		if (p == NULL) {
+			log_warn("%s", __func__);
+			goto out;
+		}
+		str = p;
+		memcpy(str + sz, buf, n);
+		sz += n;
+	}
 
-	if((f = fdopen(fd, "r")) == NULL) {
-		log_warn("cannot read trust anchor file");
-		close(fd);
-		return;
+	if (n == -1) {
+		log_warn("%s", __func__);
+		goto out;
 	}
 
-	while ((linelen = getline(&line, &linesize, f)) != -1) {
-		if (*line == ';')
+	/* make it a string */
+	p = recallocarray(str, sz, sz + 1, 1);
+	if (p == NULL) {
+		log_warn("%s", __func__);
+		goto out;
+	}
+	str = p;
+	sz++;
+
+	len = sizeof(rr);
+
+	while ((line = strsep(&str, "\n")) != NULL) {
+		if (sldns_str2wire_rr_buf(line, rr, &len, &dname_len,
+		    172800, NULL, 0, NULL, 0) != 0)
 			continue;
-		p = strchr(line, ';');
-		if (p == NULL)
-			p = strchr(line, '\n');
-		if (p != NULL) {
-			do {
-				p--;
-			} while(p != line && *p == ' ');
-			*(p + 1) = '\0';
-		}
-		log_debug("%s: %s", __func__, line);
-		add_new_ta(tah, line);
+		if (sldns_wirerr_get_type(rr, len, dname_len) ==
+		    LDNS_RR_TYPE_DNSKEY)
+			add_new_ta(tah, line);
 	}
-	free(line);
-	if (ferror(f))
-		log_warn("getline");
-	fclose(f);
+
+out:
+	free(str);
+	return;
 }
 
 void
@@ -1204,33 +1214,34 @@ send_trust_anchors(struct trust_anchor_head *tah)
 void
 write_trust_anchors(struct trust_anchor_head *tah, int fd)
 {
-	FILE			*f;
 	struct trust_anchor	*ta;
+	size_t			 len = 0;
+	ssize_t			 n;
+	char			*str;
 
-	if((f = fdopen(fd, "w+")) == NULL) {
-		log_warn("cannot open trust anchor file for writing");
-		goto err;
-	}
+	log_debug("%s", __func__);
 
-	TAILQ_FOREACH(ta, tah, entry)
-		if (fprintf(f, "%s\n", ta->ta) < 0)
-			goto err;
-	if (ferror(f)) {
+	if (lseek(fd, 0, SEEK_SET) == -1) {
 		log_warn("%s", __func__);
-		goto err;
+		goto out;
 	}
-	if (fclose(f) != 0) {
-		f = NULL;
-		log_warn("%s", __func__);
-		goto err;
-	}
-	frontend_imsg_compose_main(IMSG_TA_W_DONE, 0, NULL, 0);
-	return;
-err:
-	if (f == NULL)
-		close(fd);
-	else
-		fclose(f);
 
-	frontend_imsg_compose_main(IMSG_TA_W_FAILED, 0, NULL, 0);
+	TAILQ_FOREACH(ta, tah, entry) {
+		if ((n = asprintf(&str, "%s\n", ta->ta)) == -1) {
+			log_warn("%s", __func__);
+			len = 0;
+			goto out;
+		}
+		len += n;
+		if (write(fd, str, n) != n) {
+			log_warn("%s", __func__);
+			free(str);
+			len = 0;
+			goto out;
+		}
+		free(str);
+	}
+out:
+	ftruncate(fd, len);
+	fsync(fd);
 }
diff --git a/sbin/unwind/unwind.c b/sbin/unwind/unwind.c
index 2a99363fda8..56bac4f297e 100644
--- a/sbin/unwind/unwind.c
+++ b/sbin/unwind/unwind.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: unwind.c,v 1.12 2019/02/08 08:21:05 florian Exp $	*/
+/*	$OpenBSD: unwind.c,v 1.13 2019/02/10 14:10:22 florian Exp $	*/
 
 /*
  * Copyright (c) 2018 Florian Obser <florian@openbsd.org>
@@ -51,9 +51,7 @@
 #define	LEASE_DB_DIR	"/var/db/"
 #define	_PATH_LEASE_DB	"/var/db/dhclient.leases."
 
-#define	TRUST_ANCHOR_DIR	"/etc/unwind/trustanchor/"
 #define	TRUST_ANCHOR_FILE	"/etc/unwind/trustanchor/root.key"
-#define	TRUST_ANCHOR_TEMPLATE	"/etc/unwind/trustanchor/root.key.XXXXXXXXXX"
 
 __dead void	usage(void);
 __dead void	main_shutdown(void);
@@ -73,9 +71,6 @@ static int	main_imsg_send_config(struct unwind_conf *);
 int	main_reload(void);
 int	main_sendall(enum imsg_type, void *, uint16_t);
 void	open_dhcp_lease(int);
-void	open_trust_anchor(void);
-void	open_trust_anchor_w(void);
-void	wrote_trust_anchor(int);
 void	open_ports(void);
 
 struct unwind_conf	*main_conf;
@@ -84,9 +79,6 @@ struct imsgev		*iev_resolver;
 struct imsgev		*iev_captiveportal;
 char			*conffile;
 
-char			 trust_anchor_tmp_filename[sizeof(
-			     TRUST_ANCHOR_TEMPLATE)];
-
 pid_t	 frontend_pid;
 pid_t	 resolver_pid;
 pid_t	 captiveportal_pid;
@@ -139,7 +131,7 @@ main(int argc, char *argv[])
 	int			 pipe_main2resolver[2];
 	int			 pipe_main2captiveportal[2];
 	int			 frontend_routesock, rtfilter;
-	int			 control_fd;
+	int			 control_fd, ta_fd;
 	char			*csock;
 
 	conffile = CONF_FILE;
@@ -306,17 +298,20 @@ main(int argc, char *argv[])
 	    &rtfilter, sizeof(rtfilter)) < 0)
 		fatal("setsockopt(ROUTE_MSGFILTER)");
 
+	if ((ta_fd = open(TRUST_ANCHOR_FILE, O_RDWR | O_CREAT, 0644)) == -1)
+		log_warn("%s", TRUST_ANCHOR_FILE);
+
+	/* receiver handles failed open correctly */
+	main_imsg_compose_frontend_fd(IMSG_TAFD, 0, ta_fd);
+
 	main_imsg_compose_frontend_fd(IMSG_CONTROLFD, 0, control_fd);
 	main_imsg_compose_frontend_fd(IMSG_ROUTESOCK, 0, frontend_routesock);
 	main_imsg_send_config(main_conf);
 
-	if (unveil(TRUST_ANCHOR_DIR, "rwc") == -1 && errno != ENOENT)
-		err(1, "unveil");
-
 	if (unveil(LEASE_DB_DIR, "r") == -1 && errno != ENOENT)
 		err(1, "unveil");
 
-	if (pledge("stdio inet dns rpath wpath cpath sendfd", NULL) == -1)
+	if (pledge("stdio inet dns rpath sendfd", NULL) == -1)
 		fatal("pledge");
 
 	main_imsg_compose_frontend(IMSG_STARTUP, 0, NULL, 0);
@@ -466,18 +461,6 @@ main_dispatch_frontend(int fd, short event, void *bula)
 			memcpy(&rtm_index, imsg.data, sizeof(rtm_index));
 			open_dhcp_lease(rtm_index);
 			break;
-		case IMSG_OPEN_TA_RO:
-			open_trust_anchor();
-			break;
-		case IMSG_OPEN_TA_W:
-			open_trust_anchor_w();
-			break;
-		case IMSG_TA_W_DONE:
-			wrote_trust_anchor(0);
-			break;
-		case IMSG_TA_W_FAILED:
-			wrote_trust_anchor(1);
-			break;
 		case IMSG_OPEN_PORTS:
 			open_ports();
 			break;
@@ -964,57 +947,3 @@ open_ports(void)
 	if (udp6sock != -1)
 		main_imsg_compose_frontend_fd(IMSG_UDP6SOCK, 0, udp6sock);
 }
-
-void
-open_trust_anchor(void)
-{
-	int	 fd;
-
-	if ((fd = open(TRUST_ANCHOR_FILE, O_RDONLY)) == -1)
-		log_warn("%s: %s", __func__, TRUST_ANCHOR_FILE);
-
-	/* Send fd == -1, too. Receiver handles it correctly. */
-	main_imsg_compose_frontend_fd(IMSG_TAFD, 0, fd);
-}
-
-void
-open_trust_anchor_w(void)
-{
-	int fd;
-
-	if (*trust_anchor_tmp_filename != '\0') {
-		log_warnx("already writing trust anchor");
-		return;
-	}
-	strlcpy(trust_anchor_tmp_filename, TRUST_ANCHOR_TEMPLATE, sizeof(
-	    trust_anchor_tmp_filename));
-
-	if ((fd = mkstemp(trust_anchor_tmp_filename)) == -1) {
-		log_warn("%s", trust_anchor_tmp_filename);
-		*trust_anchor_tmp_filename = '\0';
-		return;
-	}
-	main_imsg_compose_frontend_fd(IMSG_TAFD_W, 0, fd);
-}
-
-void
-wrote_trust_anchor(int failure)
-{
-	if (*trust_anchor_tmp_filename == '\0') {
-		log_warnx("%s: not writing trust anchor", __func__);
-		return;
-	}
-
-	if (failure)
-		unlink(trust_anchor_tmp_filename);
-	else {
-		if (rename(trust_anchor_tmp_filename, TRUST_ANCHOR_FILE) ==
-		    -1) {
-			log_warn("%s", __func__);
-			unlink(trust_anchor_tmp_filename);
-		}
-	}
-
-	*trust_anchor_tmp_filename = '\0';
-
-}
diff --git a/sbin/unwind/unwind.h b/sbin/unwind/unwind.h
index df574ee38bc..47baff9e162 100644
--- a/sbin/unwind/unwind.h
+++ b/sbin/unwind/unwind.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: unwind.h,v 1.7 2019/02/07 17:20:35 florian Exp $	*/
+/*	$OpenBSD: unwind.h,v 1.8 2019/02/10 14:10:22 florian Exp $	*/
 
 /*
  * Copyright (c) 2018 Florian Obser <florian@openbsd.org>
@@ -97,15 +97,10 @@ enum imsg_type {
 	IMSG_OPEN_HTTP_PORT,
 	IMSG_HTTPSOCK,
 	IMSG_CAPTIVEPORTAL_STATE,
-	IMSG_OPEN_TA_RO,
-	IMSG_OPEN_TA_W,
 	IMSG_TAFD,
-	IMSG_TAFD_W,
 	IMSG_NEW_TA,
 	IMSG_NEW_TAS_ABORT,
 	IMSG_NEW_TAS_DONE,
-	IMSG_TA_W_DONE,
-	IMSG_TA_W_FAILED
 };
 
 struct unwind_forwarder {