From: Alberto Leiva Popper Date: Wed, 22 Jan 2025 22:38:37 +0000 (-0600) Subject: Stop rejecting RPPs if unrecognizable absent files are fileListed X-Git-Tag: 1.6.6~3 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=7f3094d8d50c32df208ed81e54a1da78e33167d9;p=thirdparty%2FFORT-validator.git Stop rejecting RPPs if unrecognizable absent files are fileListed RFC 9286: > The RP MUST acquire all of the files enumerated in the manifest > (fileList) from the publication point. If there are files listed in > the manifest that cannot be retrieved from the publication point, > the RP MUST treat this as a failed fetch. This was clashing with Fort's default rsync filters because they were preventing unknown extensions from being downloaded: > rsync (...) --include=*.cer --include=*.crl --include=*.gbr \ > --include=*.mft --include=*.roa --exclude=* (...) Which will be a problem whenever the IETF defines new legal repository extensions, such as .asa. Therefore, ignore unknown manifest fileList extensions. This technically violates RFC 9286, but it's necessary evil given that we can't trust repositories to always only serve proper RPKI content. Fixes #155. --- diff --git a/src/crypto/hash.c b/src/crypto/hash.c index 003df53b..ff5020b7 100644 --- a/src/crypto/hash.c +++ b/src/crypto/hash.c @@ -113,22 +113,20 @@ hash_validate_mft_file(struct rpki_uri *uri, BIT_STRING_t const *expected) if (expected->bits_unused != 0) return pr_val_err("Hash string has unused bits."); - do { - error = hash_file(uri, actual, &actual_len); - if (!error) - break; - - if (error == EACCES || error == ENOENT) { - if (incidence(INID_MFT_FILE_NOT_FOUND, - "File '%s' listed at manifest doesn't exist.", - uri_val_get_printable(uri))) - return -EINVAL; - - return error; - } - /* Any other error (crypto, enomem, file read) */ + error = hash_file(uri, actual, &actual_len); + switch (error) { + case 0: + break; + case EACCES: + case ENOENT: + if (incidence(INID_MFT_FILE_NOT_FOUND, + "File '%s' listed at manifest doesn't exist.", + uri_val_get_printable(uri))) + return -EINVAL; + return error; + default: return ENSURE_NEGATIVE(error); - } while (0); + } if (!hash_matches(expected->buf, expected->size, actual, actual_len)) { return incidence(INID_MFT_FILE_HASH_NOT_MATCH, diff --git a/src/object/manifest.c b/src/object/manifest.c index b5d4a94e..8e404aa7 100644 --- a/src/object/manifest.c +++ b/src/object/manifest.c @@ -201,6 +201,18 @@ shuffle_file_list(struct Manifest *mft) } } +/* + * Contract: + * - Length = 4 (includes dot) + * - Not NULL-terminated! + * - Can return NULL + */ +static char * +get_extension(IA5String_t *file) +{ + return (file->size < 4) ? NULL : (((char *)file->buf) + file->size - 4); +} + static int build_rpp(struct Manifest *mft, struct rpki_uri *notif, struct rpki_uri *mft_uri, struct rpp **pp) @@ -208,6 +220,8 @@ build_rpp(struct Manifest *mft, struct rpki_uri *notif, char const *tal; unsigned int i; struct FileAndHash *fah; + char *ext; + int (*rpp_add)(struct rpp *pp, struct rpki_uri *uri); struct rpki_uri *uri; int error; @@ -222,6 +236,30 @@ build_rpp(struct Manifest *mft, struct rpki_uri *notif, for (i = 0; i < mft->fileList.list.count; i++) { fah = mft->fileList.list.array[i]; + /* + * rsync filters unknown files. We don't want absent unknown + * files to induce RPP rejection, so we'll skip them. + * This contradicts rfc9286#6.4, but it's necessary evil because + * we can't trust the repositories to not accidentally serve + * garbage. + * + * This includes .mft; They're presently not supposed to be + * listed. + */ + ext = get_extension(&fah->file); + if (ext == NULL) + continue; + else if (strncmp(ext, ".cer", 4) == 0) + rpp_add = rpp_add_cer; + else if (strncmp(ext, ".roa", 4) == 0) + rpp_add = rpp_add_roa; + else if (strncmp(ext, ".crl", 4) == 0) + rpp_add = rpp_add_crl; + else if (strncmp(ext, ".gbr", 4) == 0) + rpp_add = rpp_add_gbr; + else + continue; + error = uri_create_mft(&uri, tal, notif, mft_uri, &fah->file); /* * Not handling ENOTRSYNC is fine because the manifest URL @@ -251,17 +289,7 @@ build_rpp(struct Manifest *mft, struct rpki_uri *notif, continue; } - if (uri_has_extension(uri, ".cer")) - rpp_add_cert(*pp, uri); - else if (uri_has_extension(uri, ".roa")) - rpp_add_roa(*pp, uri); - else if (uri_has_extension(uri, ".crl")) - error = rpp_add_crl(*pp, uri); - else if (uri_has_extension(uri, ".gbr")) - rpp_add_ghostbusters(*pp, uri); - else - uri_refput(uri); /* ignore it. */ - + error = rpp_add(*pp, uri); if (error) { uri_refput(uri); goto fail; diff --git a/src/rpp.c b/src/rpp.c index 5854efaf..d62dcae0 100644 --- a/src/rpp.c +++ b/src/rpp.c @@ -84,24 +84,27 @@ rpp_refput(struct rpp *pp) } /** Steals ownership of @uri. */ -void -rpp_add_cert(struct rpp *pp, struct rpki_uri *uri) +int +rpp_add_cer(struct rpp *pp, struct rpki_uri *uri) { uris_add(&pp->certs, uri); + return 0; } /** Steals ownership of @uri. */ -void +int rpp_add_roa(struct rpp *pp, struct rpki_uri *uri) { uris_add(&pp->roas, uri); + return 0; } /** Steals ownership of @uri. */ -void -rpp_add_ghostbusters(struct rpp *pp, struct rpki_uri *uri) +int +rpp_add_gbr(struct rpp *pp, struct rpki_uri *uri) { uris_add(&pp->ghostbusters, uri); + return 0; } /** Steals ownership of @uri. */ diff --git a/src/rpp.h b/src/rpp.h index f328b911..874068f5 100644 --- a/src/rpp.h +++ b/src/rpp.h @@ -12,10 +12,10 @@ struct rpp *rpp_create(void); void rpp_refget(struct rpp *pp); void rpp_refput(struct rpp *pp); -void rpp_add_cert(struct rpp *, struct rpki_uri *); +int rpp_add_cer(struct rpp *, struct rpki_uri *); int rpp_add_crl(struct rpp *, struct rpki_uri *); -void rpp_add_roa(struct rpp *, struct rpki_uri *); -void rpp_add_ghostbusters(struct rpp *, struct rpki_uri *); +int rpp_add_roa(struct rpp *, struct rpki_uri *); +int rpp_add_gbr(struct rpp *, struct rpki_uri *); struct rpki_uri *rpp_get_crl(struct rpp const *); int rpp_crl(struct rpp *, STACK_OF(X509_CRL) **);