diff --git a/src/libostree/ostree-core.h b/src/libostree/ostree-core.h index 08b7d451..69477a75 100644 --- a/src/libostree/ostree-core.h +++ b/src/libostree/ostree-core.h @@ -31,18 +31,18 @@ G_BEGIN_DECLS /** * OSTREE_MAX_METADATA_SIZE: - * - * Maximum permitted size in bytes of metadata objects. This is an - * arbitrary number, but really, no one should be putting humongous - * data in metadata. + * + * Default limit for maximum permitted size in bytes of metadata objects fetched + * over HTTP (including repo/config files, refs, and commit/dirtree/dirmeta + * objects). This is an arbitrary number intended to mitigate disk space + * exhaustion attacks. */ #define OSTREE_MAX_METADATA_SIZE (10 * 1024 * 1024) /** * OSTREE_MAX_METADATA_WARN_SIZE: - * - * Objects committed above this size will be allowed, but a warning - * will be emitted. + * + * This variable is no longer meaningful, it is kept only for compatibility. */ #define OSTREE_MAX_METADATA_WARN_SIZE (7 * 1024 * 1024) diff --git a/src/libostree/ostree-repo-commit.c b/src/libostree/ostree-repo-commit.c index e883cedc..9521dc6c 100644 --- a/src/libostree/ostree-repo-commit.c +++ b/src/libostree/ostree-repo-commit.c @@ -1336,18 +1336,6 @@ write_metadata_object (OstreeRepo *self, gsize len; const guint8 *bufp = g_bytes_get_data (buf, &len); - /* Do the size warning here, to avoid warning for already extant metadata */ - if (G_UNLIKELY (len > OSTREE_MAX_METADATA_WARN_SIZE)) - { - g_autofree char *metasize = g_format_size (len); - g_autofree char *warnsize = g_format_size (OSTREE_MAX_METADATA_WARN_SIZE); - g_autofree char *maxsize = g_format_size (OSTREE_MAX_METADATA_SIZE); - g_warning ("metadata object %s is %s, which is larger than the warning threshold of %s." \ - " The hard limit on metadata size is %s. Put large content in the tree itself, not in metadata.", - actual_checksum, - metasize, warnsize, maxsize); - } - /* Write the metadata to a temporary file */ g_auto(GLnxTmpfile) tmpf = { 0, }; if (!glnx_open_tmpfile_linkable_at (commit_tmp_dfd (self), ".", O_WRONLY|O_CLOEXEC, @@ -2278,25 +2266,6 @@ ostree_repo_abort_transaction (OstreeRepo *self, return TRUE; } -/* These limits were introduced since in some cases we may be processing - * malicious metadata, and we want to make disk space exhaustion attacks harder. - */ -static gboolean -metadata_size_valid (OstreeObjectType objtype, - gsize len, - GError **error) -{ - if (G_UNLIKELY (len > OSTREE_MAX_METADATA_SIZE)) - { - g_autofree char *input_bytes = g_format_size (len); - g_autofree char *max_bytes = g_format_size (OSTREE_MAX_METADATA_SIZE); - return glnx_throw (error, "Metadata object of type '%s' is %s; maximum metadata size is %s", - ostree_object_type_to_string (objtype), input_bytes, max_bytes); - } - - return TRUE; -} - /** * ostree_repo_write_metadata: * @self: Repo @@ -2349,9 +2318,6 @@ ostree_repo_write_metadata (OstreeRepo *self, normalized = g_variant_get_normal_form (object); } - if (!metadata_size_valid (objtype, g_variant_get_size (normalized), error)) - return FALSE; - /* For untrusted objects, verify their structure here */ if (expected_checksum) { @@ -2389,9 +2355,6 @@ ostree_repo_write_metadata_stream_trusted (OstreeRepo *self, GCancellable *cancellable, GError **error) { - if (length > 0 && !metadata_size_valid (objtype, length, error)) - return FALSE; - /* This is all pretty ridiculous, but we're keeping this API for backwards * compatibility, it doesn't really need to be fast. */ diff --git a/src/libostree/ostree-repo-pull.c b/src/libostree/ostree-repo-pull.c index c6b70ffb..f5c52e4a 100644 --- a/src/libostree/ostree-repo-pull.c +++ b/src/libostree/ostree-repo-pull.c @@ -150,6 +150,7 @@ typedef struct { gboolean timestamp_check; /* Verify commit timestamps */ int maxdepth; + guint64 max_metadata_size; guint64 start_time; gboolean is_mirror; @@ -2193,7 +2194,7 @@ start_fetch (OtPullData *pull_data, if (expected_max_size_p) expected_max_size = *expected_max_size_p; else if (OSTREE_OBJECT_TYPE_IS_META (objtype)) - expected_max_size = OSTREE_MAX_METADATA_SIZE; + expected_max_size = pull_data->max_metadata_size; else expected_max_size = 0; @@ -3488,6 +3489,7 @@ initiate_request (OtPullData *pull_data, * * require-static-deltas (b): Require static deltas * * override-commit-ids (as): Array of specific commit IDs to fetch for refs * * timestamp-check (b): Verify commit timestamps are newer than current (when pulling via ref); Since: 2017.11 + * * metadata-size-restriction (t): Restrict metadata objects to a maximum number of bytes; 0 to disable. Since: 2018.9 * * dry-run (b): Only print information on what will be downloaded (requires static deltas) * * override-url (s): Fetch objects from this URL if remote specifies no metalink in options * * inherit-transaction (b): Don't initiate, finish or abort a transaction, useful to do multiple pulls in one transaction. @@ -3543,6 +3545,9 @@ ostree_repo_pull_with_options (OstreeRepo *self, */ const char *the_ref_to_fetch = NULL; + /* Default */ + pull_data->max_metadata_size = OSTREE_MAX_METADATA_SIZE; + if (options) { int flags_i = OSTREE_REPO_PULL_FLAGS_NONE; @@ -3570,6 +3575,7 @@ ostree_repo_pull_with_options (OstreeRepo *self, (void) g_variant_lookup (options, "update-frequency", "u", &update_frequency); (void) g_variant_lookup (options, "localcache-repos", "^a&s", &opt_localcache_repos); (void) g_variant_lookup (options, "timestamp-check", "b", &pull_data->timestamp_check); + (void) g_variant_lookup (options, "max-metadata-size", "t", &pull_data->max_metadata_size); (void) g_variant_lookup (options, "append-user-agent", "s", &pull_data->append_user_agent); opt_n_network_retries_set = g_variant_lookup (options, "n-network-retries", "u", &pull_data->n_network_retries); diff --git a/tests/test-basic-c.c b/tests/test-basic-c.c index 8515b129..ff6d353c 100644 --- a/tests/test-basic-c.c +++ b/tests/test-basic-c.c @@ -430,6 +430,54 @@ test_devino_cache_xattrs (void) g_assert_cmpint (stats.content_objects_written, ==, 1); } +/* https://github.com/ostreedev/ostree/issues/1721 + * We should be able to commit large metadata objects now. + */ +static void +test_big_metadata (void) +{ + g_autoptr(GError) error = NULL; + gboolean ret = FALSE; + + g_autoptr(GFile) repo_path = g_file_new_for_path ("repo"); + + /* init as bare-user-only so we run everywhere */ + ret = ot_test_run_libtest ("setup_test_repository bare-user-only", &error); + g_assert_no_error (error); + g_assert (ret); + + g_autoptr(OstreeRepo) repo = ostree_repo_new (repo_path); + ret = ostree_repo_open (repo, NULL, &error); + g_assert_no_error (error); + g_assert (ret); + + g_autoptr(GFile) object_to_commit = NULL; + ret = ostree_repo_read_commit (repo, "test2", &object_to_commit, NULL, NULL, &error); + g_assert_no_error (error); + g_assert (ret); + + g_autoptr(OstreeMutableTree) mtree = ostree_mutable_tree_new (); + ret = ostree_repo_write_directory_to_mtree (repo, object_to_commit, mtree, NULL, + NULL, &error); + g_assert_no_error (error); + g_assert (ret); + + const size_t len = 20 * 1024 * 1024; + g_assert_cmpint (len, >, OSTREE_MAX_METADATA_SIZE); + g_autofree char *large_buf = g_malloc (len); + memset (large_buf, 0x42, len); + g_autoptr(GVariantBuilder) builder = + g_variant_builder_new (G_VARIANT_TYPE ("a{sv}")); + g_autofree char *commit_checksum = NULL; + g_variant_builder_add (builder, "{sv}", "large-value", + g_variant_new_fixed_array ((GVariantType*)"y", + large_buf, len, sizeof (char))); + ret = ostree_repo_write_commit (repo, NULL, NULL, NULL, g_variant_builder_end (builder), + OSTREE_REPO_FILE (object_to_commit), &commit_checksum, NULL, &error); + g_assert_no_error (error); + g_assert (ret); +} + int main (int argc, char **argv) { g_autoptr(GError) error = NULL; @@ -447,6 +495,7 @@ int main (int argc, char **argv) g_test_add_func ("/xattrs-devino-cache", test_devino_cache_xattrs); g_test_add_func ("/break-hardlink", test_break_hardlink); g_test_add_func ("/remotename", test_validate_remotename); + g_test_add_func ("/big-metadata", test_big_metadata); return g_test_run(); out: