Handle the TAL files in the master process and pass them as buffer to the

parser process. This way the parser never needs to read outside of the
cache directory which makes the unveil simpler. Additionally rsync_uri_parse
no longer needs to know about .tal files so there is now no chance to sneak
in a .tal file later on.
OK deraadt@

4 files changed, 104 insertions, 119 deletions

diff --git a/usr.sbin/rpki-client/extern.h b/usr.sbin/rpki-client/extern.h
index 95bfdf43b0d..666b9955abd 100644
--- a/usr.sbin/rpki-client/extern.h
+++ b/usr.sbin/rpki-client/extern.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: extern.h,v 1.9 2019/10/16 17:43:29 claudio Exp $ */
+/*	$OpenBSD: extern.h,v 1.10 2019/10/31 08:36:43 claudio Exp $ */
 /*
  * Copyright (c) 2019 Kristaps Dzonsons <kristaps@bsd.lv>
  *
@@ -229,7 +229,7 @@ extern int verbose;
 
 void		 tal_buffer(char **, size_t *, size_t *, const struct tal *);
 void		 tal_free(struct tal *);
-struct tal	*tal_parse(const char *);
+struct tal	*tal_parse(const char *, char *);
 struct tal	*tal_read(int);
 
 void		 cert_buffer(char **, size_t *, size_t *, const struct cert *);
diff --git a/usr.sbin/rpki-client/main.c b/usr.sbin/rpki-client/main.c
index 528a476ba6a..95b30b6b861 100644
--- a/usr.sbin/rpki-client/main.c
+++ b/usr.sbin/rpki-client/main.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: main.c,v 1.20 2019/10/16 21:43:41 jmc Exp $ */
+/*	$OpenBSD: main.c,v 1.21 2019/10/31 08:36:43 claudio Exp $ */
 /*
  * Copyright (c) 2019 Kristaps Dzonsons <kristaps@bsd.lv>
  *
@@ -22,6 +22,7 @@
 #include <sys/wait.h>
 
 #include <assert.h>
+#include <ctype.h>
 #include <err.h>
 #include <dirent.h>
 #include <fcntl.h>
@@ -462,14 +463,64 @@ queue_add_from_mft_set(int fd, struct entityq *q, const struct mft *mft,
 static void
 queue_add_tal(int fd, struct entityq *q, const char *file, size_t *eid)
 {
-	char		*nfile;
+	char		*nfile, *nbuf, *line = NULL, *buf = NULL;
+	FILE		*in;
+	ssize_t		 n, i;
+	size_t		 sz = 0, bsz = 0;
+	int		 optcomment = 1;
+
+	if ((in = fopen(file, "r")) == NULL)
+		err(EXIT_FAILURE, "fopen: %s", file);
+
+	while ((n = getline(&line, &sz, in)) != -1) {
+		/* replace CRLF with just LF */
+		if (n > 1 && line[n - 1] == '\n' && line[n - 2] == '\r') {
+			line[n - 2] = '\n';
+			line[n - 1] = '\0';
+			n--;
+		}
+		if (optcomment) {
+			/* if this is comment, just eat the line */
+			if (line[0] == '#')
+				continue;
+			optcomment = 0;
+			/*
+			 * Empty line is end of section and needs
+			 * to be eaten as well.
+			 */
+			if (line[0] == '\n')
+				continue;
+		}
+
+		/* make sure every line is valid ascii */
+		for (i = 0; i < n; i++)
+			if (!isprint(line[i]) && !isspace(line[i]))
+				errx(EXIT_FAILURE, "getline: %s: "
+				    "invalid content", file);
+
+		/* concat line to buf */
+		if ((nbuf = realloc(buf, bsz + n + 1)) == NULL)
+			err(EXIT_FAILURE, NULL);
+		buf = nbuf;
+		bsz += n + 1;
+		strlcat(buf, line, bsz);
+		/* limit the buffer size */
+		if (bsz > 4096)
+			errx(EXIT_FAILURE, "%s: file too big", file);
+	}
+
+	free(line);
+	if (ferror(in))
+		err(EXIT_FAILURE, "getline: %s", file);
+	fclose(in);
 
 	if ((nfile = strdup(file)) == NULL)
 		err(EXIT_FAILURE, "strdup");
 
 	/* Not in a repository, so directly add to queue. */
-
-	entityq_add(fd, q, nfile, RTYPE_TAL, NULL, NULL, NULL, 0, NULL, eid);
+	entityq_add(fd, q, nfile, RTYPE_TAL, NULL, NULL, NULL, 0, buf, eid);
+	/* entityq_add makes a copy of buf */
+	free(buf);
 }
 
 /*
@@ -1020,7 +1071,6 @@ proc_parser(int fd, int force, int norev)
 	X509_STORE	*store;
 	X509_STORE_CTX	*ctx;
 	struct auth	*auths = NULL;
-	int		 first_tals = 1;
 
 	ERR_load_crypto_strings();
 	OpenSSL_add_all_ciphers();
@@ -1102,31 +1152,12 @@ proc_parser(int fd, int force, int norev)
 		entp = TAILQ_FIRST(&q);
 		assert(entp != NULL);
 
-		/*
-		 * Extra security.
-		 * Our TAL files may be anywhere, but the repository
-		 * resources may only be in BASE_DIR.
-		 * When we've finished processing TAL files, make sure
-		 * that we can only see what's under that.
-		 */
-
-		if (entp->type != RTYPE_TAL && first_tals) {
-			if (unveil(BASE_DIR, "r") == -1)
-				err(EXIT_FAILURE, "%s: unveil", BASE_DIR);
-			if (unveil(NULL, NULL) == -1)
-				err(EXIT_FAILURE, "unveil");
-			first_tals = 0;
-		} else if (entp->type != RTYPE_TAL) {
-			assert(!first_tals);
-		} else if (entp->type == RTYPE_TAL)
-			assert(first_tals);
-
 		entity_buffer_resp(&b, &bsz, &bmax, entp);
 
 		switch (entp->type) {
 		case RTYPE_TAL:
 			assert(!entp->has_dgst);
-			if ((tal = tal_parse(entp->uri)) == NULL)
+			if ((tal = tal_parse(entp->uri, entp->descr)) == NULL)
 				goto out;
 			tal_buffer(&b, &bsz, &bmax, tal);
 			tal_free(tal);
@@ -1420,7 +1451,12 @@ main(int argc, char *argv[])
 
 	if (procpid == 0) {
 		close(fd[1]);
-		if (pledge("stdio rpath unveil", NULL) == -1)
+		/* Only allow access to BASE_DIR. */
+		if (unveil(BASE_DIR, "r") == -1)
+			err(EXIT_FAILURE, "%s: unveil", BASE_DIR);
+		if (unveil(NULL, NULL) == -1)
+			err(EXIT_FAILURE, "unveil");
+		if (pledge("stdio rpath", NULL) == -1)
 			err(EXIT_FAILURE, "pledge");
 		proc_parser(fd[0], force, norev);
 		/* NOTREACHED */
@@ -1460,13 +1496,7 @@ main(int argc, char *argv[])
 
 	assert(rsync != proc);
 
-	/*
-	 * The main process drives the top-down scan to leaf ROAs using
-	 * data downloaded by the rsync process and parsed by the
-	 * parsing process.
-	 */
-
-	if (pledge("stdio", NULL) == -1)
+	if (pledge("stdio rpath", NULL) == -1)
 		err(EXIT_FAILURE, "pledge");
 
 	/*
@@ -1478,6 +1508,15 @@ main(int argc, char *argv[])
 	for (i = 0; i < talsz; i++)
 		queue_add_tal(proc, &q, tals[i], &eid);
 
+	/*
+	 * The main process drives the top-down scan to leaf ROAs using
+	 * data downloaded by the rsync process and parsed by the
+	 * parsing process.
+	 */
+
+	if (pledge("stdio", NULL) == -1)
+		err(EXIT_FAILURE, "pledge");
+
 	pfd[0].fd = rsync;
 	pfd[1].fd = proc;
 	pfd[0].events = pfd[1].events = POLLIN;
diff --git a/usr.sbin/rpki-client/rsync.c b/usr.sbin/rpki-client/rsync.c
index a503390577d..0e54680a8ff 100644
--- a/usr.sbin/rpki-client/rsync.c
+++ b/usr.sbin/rpki-client/rsync.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: rsync.c,v 1.6 2019/06/19 16:30:37 deraadt Exp $ */
+/*	$OpenBSD: rsync.c,v 1.7 2019/10/31 08:36:43 claudio Exp $ */
 /*
  * Copyright (c) 2019 Kristaps Dzonsons <kristaps@bsd.lv>
  *
@@ -129,8 +129,6 @@ rsync_uri_parse(const char **hostp, size_t *hostsz,
 			*rtypep = RTYPE_CER;
 		else if (strcasecmp(path + sz - 4, ".crl") == 0)
 			*rtypep = RTYPE_CRL;
-		else if (strcasecmp(path + sz - 4, ".tal") == 0)
-			*rtypep = RTYPE_TAL;
 	}
 
 	return 1;
diff --git a/usr.sbin/rpki-client/tal.c b/usr.sbin/rpki-client/tal.c
index d8d86f7dae9..f6f0742e7a7 100644
--- a/usr.sbin/rpki-client/tal.c
+++ b/usr.sbin/rpki-client/tal.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: tal.c,v 1.7 2019/10/08 10:04:36 claudio Exp $ */
+/*	$OpenBSD: tal.c,v 1.8 2019/10/31 08:36:43 claudio Exp $ */
 /*
  * Copyright (c) 2019 Kristaps Dzonsons <kristaps@bsd.lv>
  *
@@ -29,18 +29,18 @@
 #include "extern.h"
 
 /*
- * Inner function for parsing RFC 7730 from a file stream.
+ * Inner function for parsing RFC 7730 from a buffer.
  * Returns a valid pointer on success, NULL otherwise.
  * The pointer must be freed with tal_free().
  */
 static struct tal *
-tal_parse_stream(const char *fn, FILE *f)
+tal_parse_buffer(const char *fn, char *buf)
 {
-	char		*line = NULL;
+	char		*nl, *line;
 	unsigned char	*b64 = NULL;
-	size_t		 sz, b64sz = 0, linesize = 0, lineno = 0;
-	ssize_t		 linelen, ssz;
-	int		 rc = 0;
+	size_t		 sz;
+	ssize_t		 linelen;
+	int		 rc = 0, b64sz;
 	struct tal	*tal = NULL;
 	enum rtype	 rp;
 	EVP_PKEY	*pkey = NULL;
@@ -48,27 +48,19 @@ tal_parse_stream(const char *fn, FILE *f)
 	if ((tal = calloc(1, sizeof(struct tal))) == NULL)
 		err(EXIT_FAILURE, NULL);
 
-	/* Begin with the URI section. */
+	/* Begin with the URI section, comment section already removed. */
+	while ((nl = strchr(buf, '\n')) != NULL) {
+		line = buf;
+		*nl = '\0';
 
-	while ((linelen = getline(&line, &linesize, f)) != -1) {
-		lineno++;
-		assert(linelen);
-		if (line[linelen - 1] != '\n') {
-			warnx("%s: RFC 7730 section 2.1: "
-			    "failed to parse URL", fn);
-			goto out;
-		}
-		line[--linelen] = '\0';
-		if (linelen && line[linelen - 1] == '\r')
-			line[--linelen] = '\0';
+		/* advance buffer to next line */
+		buf = nl + 1;
 
 		/* Zero-length line is end of section. */
-
-		if (linelen == 0)
+		if (*line == '\0')
 			break;
 
 		/* Append to list of URIs. */
-
 		tal->uri = reallocarray(tal->uri,
 			tal->urisz + 1, sizeof(char *));
 		if (tal->uri == NULL)
@@ -77,85 +69,48 @@ tal_parse_stream(const char *fn, FILE *f)
 		tal->uri[tal->urisz] = strdup(line);
 		if (tal->uri[tal->urisz] == NULL)
 			err(EXIT_FAILURE, NULL);
-
 		tal->urisz++;
 
 		/* Make sure we're a proper rsync URI. */
-
 		if (!rsync_uri_parse(NULL, NULL,
 		    NULL, NULL, NULL, NULL, &rp, line)) {
 			warnx("%s: RFC 7730 section 2.1: "
 			    "failed to parse URL: %s", fn, line);
 			goto out;
-		} else if (rp != RTYPE_CER) {
+		}
+		if (rp != RTYPE_CER) {
 			warnx("%s: RFC 7730 section 2.1: "
 			    "not a certificate URL: %s", fn, line);
 			goto out;
 		}
-	}
 
-	if (ferror(f))
-		err(EXIT_FAILURE, "%s: getline", fn);
+	}
 
 	if (tal->urisz == 0) {
 		warnx("%s: no URIs in manifest part", fn);
 		goto out;
-	} else if (tal->urisz > 1) {
+	} else if (tal->urisz > 1)
 		warnx("%s: multiple URIs: using the first", fn);
-		goto out;
-	}
-
-	/* Now the BASE64-encoded public key. */
-
-	while ((linelen = getline(&line, &linesize, f)) != -1) {
-		lineno++;
-		assert(linelen);
-		if (line[linelen - 1] != '\n') {
-			warnx("%s: RFC 7730 section 2.1: "
-			    "failed to parse public key", fn);
-			goto out;
-		}
-		line[--linelen] = '\0';
-		if (linelen && line[linelen - 1] == '\r')
-			line[--linelen] = '\0';
-
-		/* Zero-length line can be ignored... ? */
-
-		if (linelen == 0)
-			continue;
-
-		/* Do our base64 decoding in-band. */
-
-		sz = ((linelen + 2) / 3) * 4 + 1;
-		if ((b64 = realloc(b64, b64sz + sz)) == NULL)
-			err(EXIT_FAILURE, NULL);
-		if ((ssz = b64_pton(line, b64 + b64sz, sz)) < 0)
-			errx(EXIT_FAILURE, "b64_pton");
-
-		/*
-		 * This might be different from our allocation size, but
-		 * that doesn't matter: the slop here is minimal.
-		 */
 
-		b64sz += ssz;
-	}
-
-	if (ferror(f))
-		err(EXIT_FAILURE, "%s: getline", fn);
-
-	if (b64sz == 0) {
+	sz = strlen(buf);
+	if (sz == 0) {
 		warnx("%s: RFC 7730 section 2.1: subjectPublicKeyInfo: "
 		    "zero-length public key", fn);
 		goto out;
 	}
 
+	/* Now the BASE64-encoded public key. */
+	sz = ((sz + 2) / 3) * 4 + 1;
+	if ((b64 = malloc(sz)) == NULL)
+		err(EXIT_FAILURE, NULL);
+	if ((b64sz = b64_pton(buf, b64, sz)) < 0)
+		errx(EXIT_FAILURE, "b64_pton");
+
 	tal->pkey = b64;
 	tal->pkeysz = b64sz;
 
 	/* Make sure it's a valid public key. */
-
 	pkey = d2i_PUBKEY(NULL, (const unsigned char **)&b64, b64sz);
-	b64 = NULL;
 	if (pkey == NULL) {
 		cryptowarnx("%s: RFC 7730 section 2.1: subjectPublicKeyInfo: "
 		    "failed public key parse", fn);
@@ -163,8 +118,6 @@ tal_parse_stream(const char *fn, FILE *f)
 	}
 	rc = 1;
 out:
-	free(line);
-	free(b64);
 	if (rc == 0) {
 		tal_free(tal);
 		tal = NULL;
@@ -180,18 +133,13 @@ out:
  * memory, bad syntax, etc.
  */
 struct tal *
-tal_parse(const char *fn)
+tal_parse(const char *fn, char *buf)
 {
-	FILE		*f;
 	struct tal	*p;
 	char		*d;
 	size_t		 dlen;
 
-	if ((f = fopen(fn, "r")) == NULL)
-		err(EXIT_FAILURE, "%s: open", fn);
-
-	p = tal_parse_stream(fn, f);
-	fclose(f);
+	p = tal_parse_buffer(fn, buf);
 
 	/* extract the TAL basename (without .tal suffix) */
 	d = basename(fn);