Manifest: Improve validation of FileAndHash.file names, version 2

Now following the rules defined in 6486bis, section 4.2.2

diff --git a/src/object/manifest.c b/src/object/manifest.c
index 327fe52886e4c7a000cd308bf6f54ae84d63dbec..1c83cc8556923fcd83db3b51273b4254485f9a88 100644 (file)
--- a/src/object/manifest.c
+++ b/src/object/manifest.c
@@ -167,6 +167,8 @@ build_rpp(struct Manifest *mft, struct rpki_uri *mft_uri, bool rrdp_workspace,
 
                error = uri_create_mft(&uri, mft_uri, &fah->file,
                    rrdp_workspace);
+               if (error == ESKIP)
+                       continue;
                /*
                 * Not handling ENOTRSYNC is fine because the manifest URL
                 * should have been RSYNC. Something went wrong if an RSYNC URL

diff --git a/src/types/uri.c b/src/types/uri.c
index ca3c52ee2035458e856c3b61d1f4473fc56c2287..04b1cbe0857d233a5c2bfa4bf7bba18bd11a6543 100644 (file)
--- a/src/types/uri.c
+++ b/src/types/uri.c
@@ -134,39 +134,40 @@ str2global(char const *str, size_t str_len, struct rpki_uri *uri)
        return 0;
 }
 
+static bool
+is_valid_mft_file_chara(uint8_t chara)
+{
+       return ('a' <= chara && chara <= 'z')
+           || ('A' <= chara && chara <= 'Z')
+           || ('0' <= chara && chara <= '9')
+           || (chara == '-')
+           || (chara == '_');
+}
+
+/* RFC 6486bis, section 4.2.2 */
 static int
-validate_mft_ia5(IA5String_t *ia5)
+validate_mft_file(IA5String_t *ia5)
 {
+       size_t dot;
        size_t i;
-       int error;
 
-       if (ia5->size == 0)
-               return pr_val_err("Manifest references a file with an empty string as name.");
-       if (ia5->size == 1 && ia5->buf[0] == '.')
-               return pr_val_err("Manifest references a file named '.'");
-       if (ia5->size == 2 && ia5->buf[0] == '.' && ia5->buf[1] == '.')
-               return pr_val_err("Manifest references a file named '..'");
+       if (ia5->size < 5)
+               return pr_val_err("File name is too short (%zu < 5).", ia5->size);
+       dot = ia5->size - 4;
+       if (ia5->buf[dot] != '.')
+               return pr_val_err("File name seems to lack a three-letter extension.");
 
        for (i = 0; i < ia5->size; i++) {
-               /*
-                * RFC 6486: "Each FileAndHash is an ordered pair consisting of
-                * the name of the file in the repository publication point
-                * (directory)"
-                *
-                * I'm assuming "file name" implies "slash not allowed."
-                *
-                * This is particularly relevant because this RPP should never
-                * be allowed to write in other RPP paths, or worse yet, outside
-                * of the cache directory.
-                */
-               if (ia5->buf[i] == '/')
-                       return pr_val_err("Manifest references a file whose name has at least one slash character.");
-
-               error = validate_url_character(ia5->buf[i]);
-               if (error)
-                       return error;
+               if (i != dot && !is_valid_mft_file_chara(ia5->buf[i])) {
+                       return pr_val_err("File name contains illegal character #%u",
+                           ia5->buf[i]);
+               }
        }
 
+       /*
+        * Actual extension doesn't matter; if there's no handler,
+        * we'll naturally ignore the file.
+        */
        return 0;
 }
 
@@ -193,7 +194,7 @@ ia5str2global(struct rpki_uri *uri, char const *mft, IA5String_t *ia5)
         * `(char *) ia5->buf` is fair, but `strlen(ia5->buf)` is not.
         */
 
-       error = validate_mft_ia5(ia5);
+       error = validate_mft_file(ia5);
        if (error)
                return error;
 

diff --git a/src/types/uri.h b/src/types/uri.h
index c9d59bbfe2f7d4c4e732be0cf9b01b3e0061276e..f7fe5d0be4a1676cbdb7cb0c177b959c581b43ec 100644 (file)
--- a/src/types/uri.h
+++ b/src/types/uri.h
@@ -11,6 +11,8 @@
 /* Work with a local workspace (eg. map rsync RRPD uri's) */
 #define URI_USE_RRDP_WORKSPACE  0x10
 
+#define ESKIP 85830
+
 struct rpki_uri;
 
 /* Maps RSYNC URIs of RRDP to a local workspace */

diff --git a/test/types/uri_test.c b/test/types/uri_test.c
index 47681d492d1601bd3d4fc9b61b233634c96ac4a1..6604d2e6b08b32750767660d827b94ea01612b44 100644 (file)
--- a/test/types/uri_test.c
+++ b/test/types/uri_test.c
@@ -7,45 +7,55 @@
 #include "impersonator.c"
 #include "types/uri.c"
 
+#define BUFFER_LEN 128
+static uint8_t buffer[BUFFER_LEN];
+
 static int
-test_validate(char const *src)
+__test_validate(char const *src, size_t len)
 {
-       uint8_t buffer[32];
        IA5String_t dst;
        unsigned int i;
 
-       dst.size = strlen(src);
-
-       memcpy(buffer, src, dst.size);
-       for (i = dst.size; i < 31; i++)
+       memcpy(buffer, src, len);
+       for (i = len; i < BUFFER_LEN - 1; i++)
                buffer[i] = '_';
-       buffer[31] = 0;
+       buffer[BUFFER_LEN - 1] = 0;
 
        dst.buf = buffer;
+       dst.size = len;
 
-       return validate_mft_ia5(&dst);
+       return validate_mft_file(&dst);
 }
 
+#define test_validate(str) __test_validate(str, sizeof(str) - 1)
+
 START_TEST(check_validate_current_directory)
 {
-       ck_assert_int_eq(0, test_validate("file"));
-
        ck_assert_int_eq(-EINVAL, test_validate(""));
-
        ck_assert_int_eq(-EINVAL, test_validate("."));
-       ck_assert_int_eq(0, test_validate(".file"));
-       ck_assert_int_eq(0, test_validate("fi.le"));
-       ck_assert_int_eq(0, test_validate("file."));
-
        ck_assert_int_eq(-EINVAL, test_validate(".."));
-       ck_assert_int_eq(0, test_validate("..file"));
-       ck_assert_int_eq(0, test_validate("fi..le"));
-       ck_assert_int_eq(0, test_validate("file.."));
-
-       ck_assert_int_eq(-EINVAL, test_validate("/"));
-       ck_assert_int_eq(-EINVAL, test_validate("/file"));
-       ck_assert_int_eq(-EINVAL, test_validate("fi/le"));
-       ck_assert_int_eq(-EINVAL, test_validate("file/"));
+
+       ck_assert_int_eq(-EINVAL, test_validate("filename"));
+       ck_assert_int_eq(-EINVAL, test_validate("filename."));
+       ck_assert_int_eq(-EINVAL, test_validate("filename.a"));
+       ck_assert_int_eq(-EINVAL, test_validate("filename.ab"));
+       ck_assert_int_eq(0, test_validate("filename.abc"));
+       ck_assert_int_eq(-EINVAL, test_validate("file.abcd"));
+
+       ck_assert_int_eq(0, test_validate("file-name.ABC"));
+       ck_assert_int_eq(0, test_validate("file_name.123"));
+       ck_assert_int_eq(0, test_validate("file0name.aB2"));
+       ck_assert_int_eq(0, test_validate("file9name.---"));
+       ck_assert_int_eq(0, test_validate("FileName.A3_"));
+       ck_assert_int_eq(-EINVAL, test_validate("file.name.abc"));
+       ck_assert_int_eq(-EINVAL, test_validate("file/name.abc"));
+       ck_assert_int_eq(-EINVAL, test_validate("file\0name.abc"));
+       ck_assert_int_eq(-EINVAL, test_validate("filename.abc\0filename.abc"));
+       ck_assert_int_eq(-EINVAL, test_validate("filenameabc\0filename.abc"));
+       ck_assert_int_eq(0, test_validate("-.---"));
+
+       ck_assert_int_eq(0, test_validate("abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ01234567890-_.-_-"));
+       ck_assert_int_eq(0, test_validate("vixxBTS_TVXQ-2pmGOT7.cer"));
 }
 END_TEST