From 10c92d621667dcf1f21d780ac8b9cdbcad4402d4 Mon Sep 17 00:00:00 2001 From: Alberto Leiva Popper Date: Wed, 5 Feb 2025 13:14:42 -0600 Subject: [PATCH] Increase http.max-file-size's default We got a 530 mB snapshot nowadays. Since these tend to double during key rollover, the old default of 1 gB no longer makes sense. --- docs/usage.md | 23 +++++++------ man/fort.8 | 4 +-- src/Makefile.am | 1 + src/config.c | 15 ++++----- src/config.h | 3 +- src/config/curl_offset.c | 73 ++++++++++++++++++++++++++++++++++++++++ src/config/curl_offset.h | 8 +++++ src/http/http.c | 20 +++++++++-- 8 files changed, 121 insertions(+), 26 deletions(-) create mode 100644 src/config/curl_offset.c create mode 100644 src/config/curl_offset.h diff --git a/docs/usage.md b/docs/usage.md index 319417e7..c60b8183 100644 --- a/docs/usage.md +++ b/docs/usage.md @@ -757,22 +757,23 @@ See [`--http.low-speed-limit`](#--httplow-speed-limit) for an example. ### `--http.max-file-size` -- **Type:** Integer -- **Availability:** `argv` and JSON -- **Default:** 1,000,000,000 (1 Gigabyte) -- **Range:** [0, 2000000000] (2 Gigabytes) +The maximum file size Fort will allow itself to transfer during each HTTP request. This is intended to prevent malicious RPKI repositories from stagnating Fort. + +| Type | Availability | Default | Range | +|=========|===============|=============|========| +| Integer | `argv` and JSON | 2 Gigabytes | [0, ?] | -The maximum amount of bytes files are allowed to length during HTTP transfers. Files that exceed this limit are dropped, either early (through [CURLOPT_MAXFILESIZE](https://curl.haxx.se/libcurl/c/CURLOPT_MAXFILESIZE.html)) or as they hit the limit (when the file size is not known prior to download). +`--http.max-file-size` is implemented as [CURLOPT_MAXFILESIZE_LARGE](https://curl.se/libcurl/c/CURLOPT_MAXFILESIZE_LARGE.html), although Fort ensures it also applies to ongoing transfers in cURL < 8.4.0. -This is intended to prevent malicious RPKI repositories from stagnating Fort. +The maximum allowed value is platform-specific. The data type is `curl_off_t`, which is [aggressively intended (but not guaranteed) to be a signed 64-bit integer](https://github.com/curl/curl/blob/curl-8_12_0/include/curl/system.h#L36-L44). -As of 2021-10-05, the largest legitimate file in the repositories is an RRDP snapshot that weights ~150 megabytes. (But will double in size during key rollover.) +As of 2025-02-01, the largest legitimate file in the repositories is an RRDP snapshot that weights ~530 megabytes. Its size is expected to double during key rollover. -This configuration value is _transient_. It is expected that the IETF will eventually standardize a more versatile means to prevent unbounded file transfers. In particular, because RRDP snapshots tend to grow over time, `--http.max-file-size`'s default value will likely eventually be exceeded by legitimate files. +`--http.max-file-size`'s default value is transient, as it sometimes changes between different Fort versions to account for growth of legitimate RRDP snapshots in the wild. Be aware that, if you override it, you inherit the responsibility of keeping it up-to-date. -Watch out for the following warning in the operation logs: +Assuming it refers to a legitimate file, the following warning in the operation logs is an indication that `--http.max-file-size` likely needs to be increased: - File size exceeds 50% of the configured limit + File size exceeds 40% of the configured limit ### `--http.ca-path` @@ -1027,7 +1028,7 @@ The configuration options are mostly the same as the ones from the `argv` interf "transfer-timeout": 900, "low-speed-limit": 100000, "low-speed-time": 10, - "max-file-size": 1000000000, + "max-file-size": 2000000000, "ca-path": "/usr/local/ssl/certs" }, diff --git a/man/fort.8 b/man/fort.8 index 6a8772ad..a5c104ce 100644 --- a/man/fort.8 +++ b/man/fort.8 @@ -827,7 +827,7 @@ Default: \fI10\fR .RS 4 Maximum amount of bytes files are allowed to length during HTTP transfers. .P -Default: \fI1000000000\fR (1 GB) +Default: \fI2000000000\fR (2 GB) .RE .P @@ -1161,7 +1161,7 @@ to a specific value: "transfer-timeout": 900, "low-speed-limit": 100000, "low-speed-time": 10, - "max-file-size": 1000000000, + "max-file-size": 2000000000, "ca-path": "/usr/local/ssl/certs" }, diff --git a/src/Makefile.am b/src/Makefile.am index 987acb4b..8d9c2d0e 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -50,6 +50,7 @@ fort_SOURCES += types/vrp.c types/vrp.h fort_SOURCES += cache/local_cache.c cache/local_cache.h fort_SOURCES += config/boolean.c config/boolean.h +fort_SOURCES += config/curl_offset.c config/curl_offset.h fort_SOURCES += config/file_type.h config/file_type.c fort_SOURCES += config/filename_format.h config/filename_format.c fort_SOURCES += config/log_conf.h config/log_conf.c diff --git a/src/config.c b/src/config.c index caee4691..e3e807c9 100644 --- a/src/config.c +++ b/src/config.c @@ -1,6 +1,5 @@ #include "config.h" -#include #include #include #include @@ -9,6 +8,7 @@ #include "common.h" #include "config/boolean.h" +#include "config/curl_offset.h" #include "config/incidences.h" #include "config/str.h" #include "config/uint.h" @@ -121,11 +121,8 @@ struct rpki_config { unsigned int low_speed_limit; /* CURLOPT_LOW_SPEED_TIME for our HTTP transfers. */ unsigned int low_speed_time; - /* - * CURLOPT_MAXFILESIZE, except it also works for unknown size - * files. (Though this is reactive, not preventive.) - */ - unsigned int max_file_size; + /* CURLOPT_MAXFILESIZE_LARGE for our HTTP transfers. */ + curl_off_t max_file_size; /* Directory where CA certs to verify peers are found */ char *ca_path; } http; @@ -581,7 +578,7 @@ static const struct option_field options[] = { }, { .id = 9011, .name = "http.max-file-size", - .type = >_uint, + .type = >_curl_offset, .offset = offsetof(struct rpki_config, http.max_file_size), .doc = "Fort will refuse to download files larger than this number of bytes.", .min = 0, @@ -981,7 +978,7 @@ set_default_values(void) rpki_config.http.transfer_timeout = 900; rpki_config.http.low_speed_limit = 100000; rpki_config.http.low_speed_time = 10; - rpki_config.http.max_file_size = 1000000000; + rpki_config.http.max_file_size = 2000000000; rpki_config.http.ca_path = NULL; /* Use system default */ rpki_config.log.enabled = true; @@ -1441,7 +1438,7 @@ config_get_http_low_speed_time(void) return rpki_config.http.low_speed_time; } -long +curl_off_t config_get_http_max_file_size(void) { return rpki_config.http.max_file_size; diff --git a/src/config.h b/src/config.h index 70c3c398..a40f0f15 100644 --- a/src/config.h +++ b/src/config.h @@ -2,6 +2,7 @@ #define SRC_CONFIG_H_ #include +#include #include #include #include @@ -38,7 +39,7 @@ long config_get_http_connect_timeout(void); long config_get_http_transfer_timeout(void); long config_get_http_low_speed_limit(void); long config_get_http_low_speed_time(void); -long config_get_http_max_file_size(void); +curl_off_t config_get_http_max_file_size(void); char const *config_get_http_ca_path(void); bool config_get_rsync_enabled(void); unsigned int config_get_rsync_priority(void); diff --git a/src/config/curl_offset.c b/src/config/curl_offset.c new file mode 100644 index 00000000..441463a5 --- /dev/null +++ b/src/config/curl_offset.c @@ -0,0 +1,73 @@ +#include "config/curl_offset.h" + +#include +#include +#include + +#include "log.h" + +static void +print_curloff(struct option_field const *field, void *value) +{ + pr_op_info("%s: %" CURL_FORMAT_CURL_OFF_T, + field->name, *((curl_off_t *) value)); +} + +static int +parse_argv_curloff(struct option_field const *field, char const *str, + void *result) +{ + char *tmp; + long long parsed; + int error; + + if (field->type->has_arg != required_argument || str == NULL || + strlen(str) == 0) { + return pr_op_err("Integer options ('%s' in this case) require an argument.", + field->name); + } + + errno = 0; + parsed = strtoll(str, &tmp, 10); + error = errno; + if (error || *tmp != '\0') { + if (!error) + error = -EINVAL; + pr_op_err("Value '%s' at '%s' is not an integer: %s", + str, field->name, strerror(abs(error))); + return error; + } + + if (parsed < 0) + return pr_op_err("Value of '%s' is negative.", field->name); + + *((curl_off_t *) result) = parsed; + return 0; +} + +static int +parse_json_curloff(struct option_field const *opt, json_t *json, void *result) +{ + json_int_t value; + + if (!json_is_integer(json)) + return pr_op_err("The '%s' element is not a JSON integer.", + opt->name); + + value = json_integer_value(json); + + if (value < 0) + return pr_op_err("Value of '%s' is negative.", opt->name); + + *((curl_off_t *) result) = value; + return 0; +} + +const struct global_type gt_curl_offset = { + .has_arg = required_argument, + .size = sizeof(curl_off_t), + .print = print_curloff, + .parse.argv = parse_argv_curloff, + .parse.json = parse_json_curloff, + .arg_doc = "", +}; diff --git a/src/config/curl_offset.h b/src/config/curl_offset.h new file mode 100644 index 00000000..ee2762ef --- /dev/null +++ b/src/config/curl_offset.h @@ -0,0 +1,8 @@ +#ifndef SRC_CONFIG_CURL_OFFSET_H_ +#define SRC_CONFIG_CURL_OFFSET_H_ + +#include "config/types.h" + +extern const struct global_type gt_curl_offset; + +#endif /* SRC_CONFIG_CURL_OFFSET_H_ */ diff --git a/src/http/http.c b/src/http/http.c index 2c4da138..220563a4 100644 --- a/src/http/http.c +++ b/src/http/http.c @@ -78,6 +78,20 @@ setopt_long(CURL *curl, CURLoption opt, long value) } } +static void +setopt_curlofft(CURL *curl, CURLoption opt, curl_off_t value) +{ + CURLcode result; + + result = curl_easy_setopt(curl, opt, value); + if (result != CURLE_OK) { + fprintf(stderr, + "curl_easy_setopt(%d, %" CURL_FORMAT_CURL_OFF_T + ") returned %d: %s\n", + opt, value, result, curl_easy_strerror(result)); + } +} + struct write_callback_arg { size_t total_bytes; int error; @@ -164,7 +178,7 @@ http_easy_init(struct http_handler *handler, curl_off_t ims) config_get_http_low_speed_limit()); setopt_long(result, CURLOPT_LOW_SPEED_TIME, config_get_http_low_speed_time()); - setopt_long(result, CURLOPT_MAXFILESIZE, + setopt_curlofft(result, CURLOPT_MAXFILESIZE_LARGE, config_get_http_max_file_size()); setopt_writefunction(result); @@ -230,8 +244,8 @@ validate_file_size(char const *uri, struct write_callback_arg *args) } ratio = args->total_bytes / (float) config_get_http_max_file_size(); - if (ratio > 0.5f) { - pr_op_warn("File size exceeds 50%% of the configured limit (%zu/%ld bytes).", + if (ratio > 0.4f) { + pr_op_warn("File size exceeds 40%% of the configured limit (%zu/%ld bytes).", args->total_bytes, config_get_http_max_file_size()); } -- 2.47.2