]> git.ipfire.org Git - thirdparty/FORT-validator.git/commitdiff
Stop rejecting RPPs if unrecognizable absent files are fileListed
authorAlberto Leiva Popper <ydahhrk@gmail.com>
Wed, 22 Jan 2025 22:38:37 +0000 (16:38 -0600)
committerAlberto Leiva Popper <ydahhrk@gmail.com>
Wed, 22 Jan 2025 23:26:24 +0000 (17:26 -0600)
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.

src/crypto/hash.c
src/object/manifest.c
src/rpp.c
src/rpp.h

index 003df53b6f405a2a7ce6ed60827b2d4b20e66a86..ff5020b7acf8f4c205d1586ebd301373ea5a6025 100644 (file)
@@ -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,
index b5d4a94e303c58efd6ac41b0bb88d00fff360f9b..8e404aa78f38cd48fd98dbf6057b7af021cc6e98 100644 (file)
@@ -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;
index 5854efaffe11fc4648d5b9a513cc73a27937feff..d62dcae054cfb48c63931cbb8db4902c7556ce8d 100644 (file)
--- 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. */
index f328b911a5fed0c2d36f5efa336eb5779ad28476..874068f575a563ce2315f5258c958d4410cf1cd1 100644 (file)
--- 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) **);