Validate RSC filenames

Factor out POSIX portable filename check into a new valid_filename() and
rename the previous valid_filename() to valid_mft_filename().  Fixes and
supersedes imcomplete checks in the RSC code. Avoids truncation via
strndup() in case of embedded NULs.

input/ok claudio

diff --git a/usr.sbin/rpki-client/extern.h b/usr.sbin/rpki-client/extern.h
index 09bb4a1..843927b 100644 (file)
--- a/usr.sbin/rpki-client/extern.h
+++ b/usr.sbin/rpki-client/extern.h
@@ -1,4 +1,4 @@
-/*     $OpenBSD: extern.h,v 1.134 2022/05/09 17:19:32 tb Exp $ */
+/*     $OpenBSD: extern.h,v 1.135 2022/05/10 07:41:37 tb Exp $ */
 /*
  * Copyright (c) 2019 Kristaps Dzonsons <kristaps@bsd.lv>
  *
@@ -502,6 +502,7 @@ int          valid_cert(const char *, struct auth *, const struct cert *);
 int             valid_roa(const char *, struct auth *, struct roa *);
 int             valid_filehash(int, const char *, size_t);
 int             valid_hash(unsigned char *, size_t, const char *, size_t);
+int             valid_filename(const char *, size_t);
 int             valid_uri(const char *, size_t, const char *);
 int             valid_origin(const char *, const char *);
 int             valid_x509(char *, X509_STORE_CTX *, X509 *, struct auth *,

diff --git a/usr.sbin/rpki-client/mft.c b/usr.sbin/rpki-client/mft.c
index 5397916..b987593 100644 (file)
--- a/usr.sbin/rpki-client/mft.c
+++ b/usr.sbin/rpki-client/mft.c
@@ -1,4 +1,4 @@
-/*     $OpenBSD: mft.c,v 1.62 2022/05/10 07:28:43 job Exp $ */
+/*     $OpenBSD: mft.c,v 1.63 2022/05/10 07:41:37 tb Exp $ */
 /*
  * Copyright (c) 2019 Kristaps Dzonsons <kristaps@bsd.lv>
  *
@@ -129,16 +129,15 @@ rtype_from_file_extension(const char *fn)
 /*
  * Validate that a filename listed on a Manifest only contains characters
  * permitted in draft-ietf-sidrops-6486bis section 4.2.2
+ * Also ensure that there is exactly one '.'.
  */
 static int
-valid_filename(const char *fn, size_t len)
+valid_mft_filename(const char *fn, size_t len)
 {
        const unsigned char *c;
-       size_t i;
 
-       for (c = fn, i = 0; i < len; i++, c++)
-               if (!isalnum(*c) && *c != '-' && *c != '_' && *c != '.')
-                       return 0;
+       if (!valid_filename(fn, len))
+               return 0;
 
        c = memchr(fn, '.', len);
        if (c == NULL || c != memrchr(fn, '.', len))
@@ -206,7 +205,7 @@ mft_parse_filehash(struct parse *p, const ASN1_OCTET_STRING *os)
                    p->fn, ASN1_tag2str(file->type), file->type);
                goto out;
        }
-       if (!valid_filename(file->value.ia5string->data,
+       if (!valid_mft_filename(file->value.ia5string->data,
            file->value.ia5string->length)) {
                warnx("%s: RFC 6486 section 4.2.2: bad filename", p->fn);
                goto out;
@@ -484,7 +483,7 @@ mft_parse(X509 **x509, const char *fn, const unsigned char *der, size_t len)
                goto out;
        }
        if ((crlfile = strrchr(crldp, '/')) == NULL ||
-           !valid_filename(crlfile + 1, strlen(crlfile + 1)) ||
+           !valid_mft_filename(crlfile + 1, strlen(crlfile + 1)) ||
            rtype_from_file_extension(crlfile + 1) != RTYPE_CRL) {
                warnx("%s: RFC 6487 section 4.8.6: CRL: "
                    "bad CRL distribution point extension", fn);

diff --git a/usr.sbin/rpki-client/rsc.c b/usr.sbin/rpki-client/rsc.c
index 3e702d0..48b2b7a 100644 (file)
--- a/usr.sbin/rpki-client/rsc.c
+++ b/usr.sbin/rpki-client/rsc.c
@@ -1,4 +1,4 @@
-/*     $OpenBSD: rsc.c,v 1.1 2022/05/09 17:02:34 job Exp $ */
+/*     $OpenBSD: rsc.c,v 1.2 2022/05/10 07:41:37 tb Exp $ */
 /*
  * Copyright (c) 2022 Job Snijders <job@fastly.com>
  * Copyright (c) 2019 Kristaps Dzonsons <kristaps@bsd.lv>
@@ -147,6 +147,8 @@ rsc_parse_filenamehash(struct parse *p, const ASN1_OCTET_STRING *os)
        }
 
        if (elemsz == 2) {
+               ASN1_IA5STRING *filename;
+
                file = sk_ASN1_TYPE_value(seq, i++);
                if (file->type != V_ASN1_IA5STRING) {
                        warnx("%s: RSC FileNameAndHash: want ASN.1 IA5 string,"
@@ -154,25 +156,17 @@ rsc_parse_filenamehash(struct parse *p, const ASN1_OCTET_STRING *os)
                            ASN1_tag2str(file->type), file->type);
                        goto out;
                }
-               fn = strndup((const char *)file->value.ia5string->data,
-                   file->value.ia5string->length);
-               if (fn == NULL)
-                       err(1, NULL);
 
-               /*
-                * filename must confirm to portable file name character set
-                * XXX: use valid_filename() instead
-                */
-               if (strchr(fn, '/') != NULL) {
-                       warnx("%s: path components disallowed in filename: %s",
-                           p->fn, fn);
-                       goto out;
-               }
-               if (strchr(fn, '\n') != NULL) {
-                       warnx("%s: newline disallowed in filename: %s",
-                           p->fn, fn);
+               filename = file->value.ia5string;
+
+               if (!valid_filename(filename->data, filename->length)) {
+                       warnx("%s: RSC FileNameAndHash: bad filename", p->fn);
                        goto out;
                }
+
+               fn = strndup(filename->data, filename->length);
+               if (fn == NULL)
+                       err(1, NULL);
        }
 
        /* Now hash value. */

diff --git a/usr.sbin/rpki-client/validate.c b/usr.sbin/rpki-client/validate.c
index 4677093..ca03126 100644 (file)
--- a/usr.sbin/rpki-client/validate.c
+++ b/usr.sbin/rpki-client/validate.c
@@ -1,4 +1,4 @@
-/*     $OpenBSD: validate.c,v 1.31 2022/04/21 09:53:07 claudio Exp $ */
+/*     $OpenBSD: validate.c,v 1.32 2022/05/10 07:41:37 tb Exp $ */
 /*
  * Copyright (c) 2019 Kristaps Dzonsons <kristaps@bsd.lv>
  *
@@ -274,6 +274,22 @@ valid_hash(unsigned char *buf, size_t len, const char *hash, size_t hlen)
        return 1;
 }
 
+/*
+ * Validate that a filename only contains characters from the POSIX portable
+ * filename character set [A-Za-z0-9._-], see IEEE Std 1003.1-2013, 3.278.
+ */
+int
+valid_filename(const char *fn, size_t len)
+{
+       const unsigned char *c;
+       size_t i;
+
+       for (c = fn, i = 0; i < len; i++, c++)
+               if (!isalnum(*c) && *c != '-' && *c != '_' && *c != '.')
+                       return 0;
+       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