From: Alberto Leiva Popper Date: Thu, 28 Oct 2021 21:33:28 +0000 (-0500) Subject: Manifest: Improve validation of FileAndHash.file names, version 2 X-Git-Tag: 1.5.3~9 X-Git-Url: http://git.ipfire.org/gitweb/gitweb.cgi?a=commitdiff_plain;h=48d352a01a4cff49fa443240205558d35ee77a4d;p=thirdparty%2FFORT-validator.git 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 327fe528..1c83cc85 100644 --- 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 ca3c52ee..04b1cbe0 100644 --- 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 c9d59bbf..f7fe5d0b 100644 --- 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 47681d49..6604d2e6 100644 --- 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