Factor out the URI check we do in various places into valid_uri().
authorclaudio <claudio@openbsd.org>
Fri, 5 Mar 2021 17:15:19 +0000 (17:15 +0000)
committerclaudio <claudio@openbsd.org>
Fri, 5 Mar 2021 17:15:19 +0000 (17:15 +0000)
RRDP will add a bunch more checks so this makes even more sense.
With and OK tb@

usr.sbin/rpki-client/cert.c
usr.sbin/rpki-client/extern.h
usr.sbin/rpki-client/tal.c
usr.sbin/rpki-client/validate.c

index 42b460f..43e4944 100644 (file)
@@ -1,4 +1,4 @@
-/*     $OpenBSD: cert.c,v 1.27 2021/02/18 16:23:17 claudio Exp $ */
+/*     $OpenBSD: cert.c,v 1.28 2021/03/05 17:15:19 claudio Exp $ */
 /*
  * Copyright (c) 2019 Kristaps Dzonsons <kristaps@bsd.lv>
  *
@@ -19,7 +19,6 @@
 
 #include <arpa/inet.h>
 #include <assert.h>
-#include <ctype.h>
 #include <err.h>
 #include <inttypes.h>
 #include <stdarg.h>
@@ -140,11 +139,8 @@ sbgp_addr(struct parse *p,
  * Returns zero on failure, non-zero on success.
  */
 static int
-sbgp_sia_resource_notify(struct parse *p,
-       const unsigned char *d, size_t dsz)
+sbgp_sia_resource_notify(struct parse *p, const char *d, size_t dsz)
 {
-       size_t i;
-
        if (p->res->notify != NULL) {
                warnx("%s: RFC 6487 section 4.8.8: SIA: "
                    "Notify location already specified", p->fn);
@@ -152,21 +148,12 @@ sbgp_sia_resource_notify(struct parse *p,
        }
 
        /* Make sure it's a https:// address. */
-       if (dsz <= 8 || strncasecmp(d, "https://", 8)) {
-               warnx("%s: RFC 8182 section 3.2: not using https schema",
-                   p->fn);
-               return 0;
-       }
-       /* make sure only US-ASCII chars are in the URL */
-       for (i = 0; i < dsz; i++) {
-               if (isalnum(d[i]) || ispunct(d[i]))
-                       continue;
-               warnx("%s: invalid URI", p->fn);
+       if (!valid_uri(d, dsz, "https://")) {
+               warnx("%s: RFC 8182 section 3.2: bad Notify URI", p->fn);
                return 0;
        }
 
-
-       if ((p->res->notify = strndup((const char *)d, dsz)) == NULL)
+       if ((p->res->notify = strndup(d, dsz)) == NULL)
                err(1, NULL);
 
        return 1;
@@ -177,11 +164,8 @@ sbgp_sia_resource_notify(struct parse *p,
  * Returns zero on failure, non-zero on success.
  */
 static int
-sbgp_sia_resource_mft(struct parse *p,
-       const unsigned char *d, size_t dsz)
+sbgp_sia_resource_mft(struct parse *p, const char *d, size_t dsz)
 {
-       size_t i;
-
        if (p->res->mft != NULL) {
                warnx("%s: RFC 6487 section 4.8.8: SIA: "
                    "MFT location already specified", p->fn);
@@ -189,26 +173,18 @@ sbgp_sia_resource_mft(struct parse *p,
        }
 
        /* Make sure it's an MFT rsync address. */
-       if (dsz <= 8 || strncasecmp(d, "rsync://", 8)) {
-               warnx("%s: RFC 6487 section 4.8.8: not using rsync schema",
-                   p->fn);
+       if (!valid_uri(d, dsz, "rsync://")) {
+               warnx("%s: RFC 6487 section 4.8.8: bad MFT location", p->fn);
                return 0;
        }
 
-       if (strcasecmp(d + dsz - 4, ".mft") != 0) {
+       if (dsz < 4 || strcasecmp(d + dsz - 4, ".mft") != 0) {
                warnx("%s: RFC 6487 section 4.8.8: SIA: "
-                   "invalid rsync URI suffix", p->fn);
-               return 0;
-       }
-       /* make sure only US-ASCII chars are in the URL */
-       for (i = 0; i < dsz; i++) {
-               if (isalnum(d[i]) || ispunct(d[i]))
-                       continue;
-               warnx("%s: invalid URI", p->fn);
+                   "not an MFT file", p->fn);
                return 0;
        }
 
-       if ((p->res->mft = strndup((const char *)d, dsz)) == NULL)
+       if ((p->res->mft = strndup(d, dsz)) == NULL)
                err(1, NULL);
 
        return 1;
@@ -219,11 +195,8 @@ sbgp_sia_resource_mft(struct parse *p,
  * Returns zero on failure, non-zero on success.
  */
 static int
-sbgp_sia_resource_carepo(struct parse *p,
-       const unsigned char *d, size_t dsz)
+sbgp_sia_resource_carepo(struct parse *p, const char *d, size_t dsz)
 {
-       size_t i;
-
        if (p->res->repo != NULL) {
                warnx("%s: RFC 6487 section 4.8.8: SIA: "
                    "CA repository already specified", p->fn);
@@ -231,21 +204,13 @@ sbgp_sia_resource_carepo(struct parse *p,
        }
 
        /* Make sure it's an rsync:// address. */
-       if (dsz <= 8 || strncasecmp(d, "rsync://", 8)) {
-               warnx("%s: RFC 6487 section 4.8.8: not using rsync schema",
+       if (!valid_uri(d, dsz, "rsync://")) {
+               warnx("%s: RFC 6487 section 4.8.8: bad CA repository URI",
                    p->fn);
                return 0;
        }
 
-       /* make sure only US-ASCII chars are in the URL */
-       for (i = 0; i < dsz; i++) {
-               if (isalnum(d[i]) || ispunct(d[i]))
-                       continue;
-               warnx("%s: invalid URI", p->fn);
-               return 0;
-       }
-
-       if ((p->res->repo = strndup((const char *)d, dsz)) == NULL)
+       if ((p->res->repo = strndup(d, dsz)) == NULL)
                err(1, NULL);
 
        return 1;
@@ -1175,7 +1140,6 @@ out:
 struct cert *
 cert_parse(X509 **xp, const char *fn)
 {
-
        return cert_parse_inner(xp, fn, 0);
 }
 
index 4a77b43..002c62c 100644 (file)
@@ -1,4 +1,4 @@
-/*     $OpenBSD: extern.h,v 1.52 2021/03/05 16:00:00 claudio Exp $ */
+/*     $OpenBSD: extern.h,v 1.53 2021/03/05 17:15:19 claudio Exp $ */
 /*
  * Copyright (c) 2019 Kristaps Dzonsons <kristaps@bsd.lv>
  *
@@ -355,6 +355,7 @@ int          valid_cert(const char *, struct auth_tree *,
                    const struct cert *);
 int             valid_roa(const char *, struct auth_tree *, struct roa *);
 int             valid_filehash(const char *, const char *, size_t);
+int             valid_uri(const char *, size_t, const char *);
 
 /* Working with CMS files. */
 
index 463a77f..94b191a 100644 (file)
@@ -1,4 +1,4 @@
-/*     $OpenBSD: tal.c,v 1.27 2021/02/19 10:23:50 claudio Exp $ */
+/*     $OpenBSD: tal.c,v 1.28 2021/03/05 17:15:19 claudio Exp $ */
 /*
  * Copyright (c) 2019 Kristaps Dzonsons <kristaps@bsd.lv>
  *
@@ -82,7 +82,6 @@ tal_parse_buffer(const char *fn, char *buf)
        char            *nl, *line, *f, *file = NULL;
        unsigned char   *der;
        size_t           sz, dersz;
-       ssize_t          i;
        int              rc = 0;
        struct tal      *tal = NULL;
        EVP_PKEY        *pkey = NULL;
@@ -103,9 +102,7 @@ tal_parse_buffer(const char *fn, char *buf)
                        break;
 
                /* make sure only US-ASCII chars are in the URL */
-               for (i = 0; i < nl - line; i++) {
-                       if (isalnum(line[i]) || ispunct(line[i]))
-                               continue;
+               if (!valid_uri(line, nl - line, NULL)) {
                        warnx("%s: invalid URI", fn);
                        goto out;
                }
index 39779a3..3470884 100644 (file)
@@ -1,4 +1,4 @@
-/*     $OpenBSD: validate.c,v 1.12 2021/03/05 16:00:00 claudio Exp $ */
+/*     $OpenBSD: validate.c,v 1.13 2021/03/05 17:15:19 claudio Exp $ */
 /*
  * Copyright (c) 2019 Kristaps Dzonsons <kristaps@bsd.lv>
  *
@@ -19,6 +19,7 @@
 
 #include <arpa/inet.h>
 #include <assert.h>
+#include <ctype.h>
 #include <err.h>
 #include <fcntl.h>
 #include <inttypes.h>
@@ -272,3 +273,32 @@ valid_filehash(const char *fn, const char *hash, size_t hlen)
 
        return 1;
 }
+
+/*
+ * Validate a URI to make sure it is pure ASCII and does not point backwards
+ * or doing some other silly tricks. To enforce the protocol pass either
+ * https:// or rsync:// as proto, if NULL is passed no protocol is enforced.
+ * Returns 1 if valid, 0 otherwise.
+ */
+int
+valid_uri(const char *uri, size_t usz, const char *proto)
+{
+       size_t s;
+
+       for (s = 0; s < usz; s++)
+               if (!isalnum((unsigned char)uri[s]) &&
+                   !ispunct((unsigned char)uri[s]))
+                       return 0;
+
+       if (proto != NULL) {
+               s = strlen(proto);
+               if (strncasecmp(uri, proto, s) != 0)
+                       return 0;
+       }
+
+       /* do not allow files or directories to start with a '.' */
+       if (strstr(uri, "/.") != NULL)
+               return 0;
+
+       return 1;
+}