On Tue, May 10, 2022 at 08:05:00AM +0200, Theo Buehler wrote:
> This moves valid_filename() to validate.c and splits out a helper
> portable_filename() which can be used from the RSC code. While moving
> valid_filename() is not necessary, I thought it makes sense to keep the
> two functions next to each other.
>
> I could not find a short name with valid_ prefix for portable_filename()
> that I liked. valid_posix_filename()? valid_checklist_file()?
>
> Also, valid_filename() could be stricter and enforce that the unique '.'
> is followed by a 3-letter suffix.
I would use valid_filename() for what you call portable_filename() and
then introduce a mft_filename() in mft.c that does the extra check.
Or maybe rename valid_filename() to valid_mft_filename().
I don't think we need to enforce the location of the dot. That is enforced
by rtype_from_file_extension() and rtype_from_mftfile().
> Index: extern.h
> ===================================================================
> RCS file: /cvs/src/usr.sbin/rpki-client/extern.h,v
> retrieving revision 1.134
> diff -u -p -r1.134 extern.h
> --- extern.h 9 May 2022 17:19:32 -0000 1.134
> +++ extern.h 10 May 2022 06:03:19 -0000
> @@ -502,6 +502,8 @@ int valid_cert(const char *, struct au
> 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 portable_filename(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 *,
> Index: mft.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/rpki-client/mft.c,v
> retrieving revision 1.61
> diff -u -p -r1.61 mft.c
> --- mft.c 9 May 2022 17:02:34 -0000 1.61
> +++ mft.c 10 May 2022 06:03:19 -0000
> @@ -127,27 +127,6 @@ 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
> - */
> -static 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;
> -
> - c = memchr(fn, '.', len);
> - if (c == NULL || c != memrchr(fn, '.', len))
> - return 0;
> -
> - return 1;
> -}
> -
> -/*
> * Check that the file is an ASPA, CER, CRL, GBR or a ROA.
> * Returns corresponding rtype or RTYPE_INVALID on error.
> */
> Index: rsc.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/rpki-client/rsc.c,v
> retrieving revision 1.1
> diff -u -p -r1.1 rsc.c
> --- rsc.c 9 May 2022 17:02:34 -0000 1.1
> +++ rsc.c 10 May 2022 06:03:46 -0000
> @@ -147,6 +147,8 @@ rsc_parse_filenamehash(struct parse *p,
> }
>
> 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,
> 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 (!portable_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. */
> Index: validate.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/rpki-client/validate.c,v
> retrieving revision 1.31
> diff -u -p -r1.31 validate.c
> --- validate.c 21 Apr 2022 09:53:07 -0000 1.31
> +++ validate.c 10 May 2022 06:03:19 -0000
> @@ -275,6 +275,42 @@ valid_hash(unsigned char *buf, size_t le
> }
>
> /*
> + * 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
> +portable_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 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 '.'.
> + */
> +int
> +valid_filename(const char *fn, size_t len)
> +{
> + const unsigned char *c;
> +
> + if (!portable_filename(fn, len))
> + return 0;
> +
> + c = memchr(fn, '.', len);
> + if (c == NULL || c != memrchr(fn, '.', len))
> + 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
> * https:// or rsync:// as proto, if NULL is passed no protocol is enforced.
>
--
:wq Claudio
