On Sun, Oct 20, 2019 at 12:46:44PM -0600, Theo de Raadt wrote:
> There has been an update to
>
> https://www.ietf.org/rfcdiff?url2=draft-ietf-sidrops-https-tal-05
>
> Which permits comments in the tal files. I proposed this at nanog yvr
> as something which might help the arin tal hangup.
>
> queue_add_tal(int fd, struct entityq *q, const char *file, size_t *eid)
> {
> + static unsigned char buf[4096];
> char *nfile;
> + ssize_t n, i;
> + int tfd;
> +
> + if ((tfd = open(file, O_RDONLY)) == -1)
> + err(EXIT_FAILURE, "open: %s", file);
> + n = read(tfd, buf, sizeof(buf));
> + if (n == -1)
> + err(EXIT_FAILURE, "read: %s", file);
> + if (n == sizeof(buf))
> + errx(EXIT_FAILURE, "read: %s: file too big", file);
> + for (i = 0; i < n; i++)
>
> A commented file might not fit in your buffer. I suggest you minimally
> parse the file, discarding the comment lines. The remaining tal contents
> will be small enough to pass to the other process.
I would have preferred to not parse anything in the parent but skipping
comments makes sense since it keeps the passed buffer small.
Here is an updated diff.
--
:wq Claudio
Index: extern.h
===================================================================
RCS file: /cvs/src/usr.sbin/rpki-client/extern.h,v
retrieving revision 1.9
diff -u -p -r1.9 extern.h
--- extern.h 16 Oct 2019 17:43:29 -0000 1.9
+++ extern.h 20 Oct 2019 10:17:11 -0000
@@ -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 *);
Index: main.c
===================================================================
RCS file: /cvs/src/usr.sbin/rpki-client/main.c,v
retrieving revision 1.20
diff -u -p -r1.20 main.c
--- main.c 16 Oct 2019 21:43:41 -0000 1.20
+++ main.c 21 Oct 2019 12:10:15 -0000
@@ -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 en
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");
/*
@@ -1477,6 +1507,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;
Index: rsync.c
===================================================================
RCS file: /cvs/src/usr.sbin/rpki-client/rsync.c,v
retrieving revision 1.6
diff -u -p -r1.6 rsync.c
--- rsync.c 19 Jun 2019 16:30:37 -0000 1.6
+++ rsync.c 20 Oct 2019 16:34:19 -0000
@@ -129,8 +129,6 @@ rsync_uri_parse(const char **hostp, size
*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;
Index: tal.c
===================================================================
RCS file: /cvs/src/usr.sbin/rpki-client/tal.c,v
retrieving revision 1.7
diff -u -p -r1.7 tal.c
--- tal.c 8 Oct 2019 10:04:36 -0000 1.7
+++ tal.c 21 Oct 2019 10:09:27 -0000
@@ -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);
