From 203dfefc80f463dd05384bf5ace416e3efbdb840 Mon Sep 17 00:00:00 2001 From: tb Date: Tue, 10 May 2022 07:41:37 +0000 Subject: [PATCH] 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 --- usr.sbin/rpki-client/extern.h | 3 ++- usr.sbin/rpki-client/mft.c | 15 +++++++-------- usr.sbin/rpki-client/rsc.c | 28 +++++++++++----------------- usr.sbin/rpki-client/validate.c | 18 +++++++++++++++++- 4 files changed, 37 insertions(+), 27 deletions(-) diff --git a/usr.sbin/rpki-client/extern.h b/usr.sbin/rpki-client/extern.h index 09bb4a14deb..843927bce40 100644 --- 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 * @@ -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 5397916f5e0..b9875936b5b 100644 --- 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 * @@ -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 3e702d02233..48b2b7af980 100644 --- 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 * Copyright (c) 2019 Kristaps Dzonsons @@ -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 4677093f1b7..ca0312692d9 100644 --- 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 * @@ -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 -- 2.20.1