Change the way TAL files are loaded into rpki-client. Instead of passing
the filename to the parser process and have that one open the file. Do it
in the main process and pass the buffer to the parser. The benefit of this
is that TAL files are not read by the parser and therefor the unveil can
be locked to the base directory early on. In my opinion this is a good
thing.

-- 
: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      20 Oct 2019 11:18:18 -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,28 @@ 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)
 {
+       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++)
+               if (!isprint(buf[i]) && !isspace(buf[i]))
+                       errx(EXIT_FAILURE, "read: %s: invalid content", file);
+       buf[n] = '\0';
 
        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);
 }
 
 /*
@@ -1020,7 +1035,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 +1116,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 +1415,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 +1460,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 +1471,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       20 Oct 2019 10:16:51 -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;
@@ -50,25 +50,20 @@ tal_parse_stream(const char *fn, FILE *f
 
        /* Begin with the URI section. */
 
-       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';
+       while ((nl = strchr(buf, '\n')) != NULL) {
+               line = buf;
+               *nl = '\0';
+               if (*(nl - 1) == '\r')
+                       *(nl - 1) = '\0';
 
-               /* Zero-length line is end of section. */
+               /* advance buffer to next line */
+               buf = nl + 1;
 
-               if (linelen == 0)
+               /* Zero-length line is end of section. */
+               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 +72,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 +121,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 +136,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);

Reply via email to