Only verify OSTREE_MAX_METADATA_SIZE for HTTP fetches

There are use cases for libostree as a local content store
for content derived or delivered via other mechanisms (e.g. OCI
images, RPMs, etc.).  rpm-ostree today imports RPMs into OSTree
branches, and puts the RPM header value as commit metadata.
Some of these can be quite large because the header includes
permissions for each file.  Similarly, some OCI metadata is large.

Since there's no security issues with this, support committing
such content.

We still by default limit the size of metadata fetches, although
for good measure we make this configurable too via a new
`max-metadata-size` value.

Closes: https://github.com/ostreedev/ostree/issues/1721

Closes: #1744
Approved by: jlebon

src/libostree/ostree-core.h

@@ -32,17 +32,17 @@ 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)
 

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.
    */

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);

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: