Limit metadata to 10 MiB

If fetching GPG-signed commits over plain HTTP, a MitM attacker can
fill up the drive of targets by simply returning an enormous stream
for the commit object.

Related to this, an attacker can also cause OSTree to perform large
memory allocations by returning enormous GVariants in the metadata.

This helps close that attack by limiting all metadata objects to 10
MiB, so the initial fetch will be truncated.

But now the attack is only slightly more difficult as the attacker
will have to return a correctly formed commit object, then return a
large stream of < 10 MiB dirmeta/dirtree objects.

https://bugzilla.gnome.org/show_bug.cgi?id=725921

Makefile-tests.am

@@ -29,6 +29,7 @@ testfiles = test-basic \
 	test-libarchive \
 	test-pull-archive-z \
 	test-pull-corruption \
+	test-pull-large-metadata \
 	test-pull-resume \
 	test-gpg-signed-commit \
 	test-admin-deploy-syslinux \

src/libostree/ostree-core.h

@@ -29,9 +29,19 @@ G_BEGIN_DECLS
 /**
  * OSTREE_MAX_METADATA_SIZE:
  * 
- * Maximum permitted size in bytes of metadata objects.
+ * Maximum permitted size in bytes of metadata objects.  This is an
+ * arbitrary number, but really, no one should be putting humongous
+ * data in metadata.
  */
-#define OSTREE_MAX_METADATA_SIZE (1 << 26)
+#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.
+ */
+#define OSTREE_MAX_METADATA_WARN_SIZE (7 * 1024 * 1024)
 
 /**
  * OSTREE_MAX_RECURSION:

src/libostree/ostree-fetcher.c

@@ -52,6 +52,8 @@ typedef struct {
   GFile *out_tmpfile;
   GOutputStream *out_stream;
 
+  guint64 max_size;
+  guint64 current_size;
   guint64 content_length;
 
   GCancellable *cancellable;
@@ -260,22 +262,21 @@ ostree_fetcher_queue_pending_uri (OstreeFetcher *self,
   ostree_fetcher_process_pending_queue (self);
 }
 
-static void
-on_splice_complete (GObject        *object,
-                    GAsyncResult   *result,
-                    gpointer        user_data) 
+static gboolean
+finish_stream (OstreeFetcherPendingURI *pending,
+               GCancellable            *cancellable,
+               GError                 **error)
 {
-  OstreeFetcherPendingURI *pending = user_data;
-  gs_unref_object GFileInfo *file_info = NULL;
+  gboolean ret = FALSE;
   goffset filesize;
-  GError *local_error = NULL;
+  gs_unref_object GFileInfo *file_info = NULL;
 
   /* Close it here since we do an async fstat(), where we don't want
    * to hit a bad fd.
    */
   if (pending->out_stream)
     {
-      if (!g_output_stream_close (pending->out_stream, pending->cancellable, &local_error))
+      if (!g_output_stream_close (pending->out_stream, pending->cancellable, error))
         goto out;
       g_hash_table_remove (pending->self->output_stream_set, pending->out_stream);
     }
@@ -283,7 +284,7 @@ on_splice_complete (GObject        *object,
   pending->state = OSTREE_FETCHER_STATE_COMPLETE;
   file_info = g_file_query_info (pending->out_tmpfile, OSTREE_GIO_FAST_QUERYINFO,
                                  G_FILE_QUERY_INFO_NOFOLLOW_SYMLINKS,
-                                 pending->cancellable, &local_error);
+                                 pending->cancellable, error);
   if (!file_info)
     goto out;
 
@@ -296,7 +297,7 @@ on_splice_complete (GObject        *object,
   filesize = g_file_info_get_size (file_info);
   if (filesize < pending->content_length)
     {
-      g_set_error (&local_error, G_IO_ERROR, G_IO_ERROR_FAILED, "Download incomplete");
+      g_set_error (error, G_IO_ERROR, G_IO_ERROR_FAILED, "Download incomplete");
       goto out;
     }
   else
@@ -304,12 +305,107 @@ on_splice_complete (GObject        *object,
       pending->self->total_downloaded += g_file_info_get_size (file_info);
     }
 
+  ret = TRUE;
  out:
   (void) g_input_stream_close (pending->request_body, NULL, NULL);
+  return ret;
+}
+
+static void
+on_stream_read (GObject        *object,
+                GAsyncResult   *result,
+                gpointer        user_data);
+
+static void
+on_out_splice_complete (GObject        *object,
+                        GAsyncResult   *result,
+                        gpointer        user_data) 
+{
+  OstreeFetcherPendingURI *pending = user_data;
+  gssize bytes_written;
+  GError *local_error = NULL;
+  GError **error = &local_error;
+
+  bytes_written = g_output_stream_splice_finish ((GOutputStream *)object,
+                                                 result,
+                                                 error);
+  if (bytes_written < 0)
+    goto out;
+
+  g_input_stream_read_bytes_async (pending->request_body, 8192, G_PRIORITY_DEFAULT,
+                                   pending->cancellable, on_stream_read, pending);
+
+ out:
   if (local_error)
-    g_simple_async_result_take_error (pending->result, local_error);
-  g_simple_async_result_complete (pending->result);
-  g_object_unref (pending->result);
+    {
+      g_simple_async_result_take_error (pending->result, local_error);
+      g_simple_async_result_complete (pending->result);
+    }
+}
+
+static void
+on_stream_read (GObject        *object,
+                GAsyncResult   *result,
+                gpointer        user_data) 
+{
+  OstreeFetcherPendingURI *pending = user_data;
+  gs_unref_bytes GBytes *bytes = NULL;
+  gsize bytes_read;
+  GError *local_error = NULL;
+  GError **error = &local_error;
+
+  bytes = g_input_stream_read_bytes_finish ((GInputStream*)object, result, error);
+  if (!bytes)
+    goto out;
+
+  bytes_read = g_bytes_get_size (bytes);
+  if (bytes_read == 0)
+    {
+      if (!finish_stream (pending, pending->cancellable, error))
+        goto out;
+      g_simple_async_result_complete (pending->result);
+      g_object_unref (pending->result);
+    }
+  else
+    {
+      if (pending->max_size > 0)
+        {
+          if (bytes_read > pending->max_size ||
+              (bytes_read + pending->current_size) > pending->max_size)
+            {
+              gs_free char *uristr = soup_uri_to_string (pending->uri, FALSE);
+              g_set_error (error, G_IO_ERROR, G_IO_ERROR_FAILED,
+                           "URI %s exceeded maximum size of %" G_GUINT64_FORMAT " bytes",
+                           uristr,
+                           pending->max_size);
+              goto out;
+            }
+        }
+      
+      pending->current_size += bytes_read;
+
+      /* We do this instead of _write_bytes_async() as that's not
+       * guaranteed to do a complete write.
+       */
+      {
+        gs_unref_object GInputStream *membuf =
+          g_memory_input_stream_new_from_bytes (bytes);
+        g_output_stream_splice_async (pending->out_stream, membuf,
+                                      G_OUTPUT_STREAM_SPLICE_CLOSE_SOURCE,
+                                      G_PRIORITY_DEFAULT,
+                                      pending->cancellable,
+                                      on_out_splice_complete,
+                                      pending);
+      }
+    }
+
+ out:
+  if (local_error)
+    {
+      g_simple_async_result_take_error (pending->result, local_error);
+      g_simple_async_result_complete (pending->result);
+      g_object_unref (pending->result);
+    }
 }
 
 static void
@@ -320,7 +416,6 @@ on_request_sent (GObject        *object,
   OstreeFetcherPendingURI *pending = user_data;
   GError *local_error = NULL;
   gs_unref_object SoupMessage *msg = NULL;
-  GOutputStreamSpliceFlags flags = G_OUTPUT_STREAM_SPLICE_CLOSE_SOURCE;
 
   pending->state = OSTREE_FETCHER_STATE_COMPLETE;
   pending->request_body = soup_request_send_finish ((SoupRequest*) object,
@@ -371,8 +466,8 @@ on_request_sent (GObject        *object,
       if (!pending->out_stream)
         goto out;
       g_hash_table_add (pending->self->output_stream_set, g_object_ref (pending->out_stream));
-      g_output_stream_splice_async (pending->out_stream, pending->request_body, flags, G_PRIORITY_DEFAULT,
-                                    pending->cancellable, on_splice_complete, pending);
+      g_input_stream_read_bytes_async (pending->request_body, 8192, G_PRIORITY_DEFAULT,
+                                       pending->cancellable, on_stream_read, pending);
       
     }
   else
@@ -394,6 +489,7 @@ static OstreeFetcherPendingURI *
 ostree_fetcher_request_uri_internal (OstreeFetcher         *self,
                                      SoupURI               *uri,
                                      gboolean               is_stream,
+                                     guint64                max_size,
                                      GCancellable          *cancellable,
                                      GAsyncReadyCallback    callback,
                                      gpointer               user_data,
@@ -406,6 +502,7 @@ ostree_fetcher_request_uri_internal (OstreeFetcher         *self,
   pending->refcount = 1;
   pending->self = g_object_ref (self);
   pending->uri = soup_uri_copy (uri);
+  pending->max_size = max_size;
   pending->is_stream = is_stream;
   if (!is_stream)
     {
@@ -433,6 +530,7 @@ ostree_fetcher_request_uri_internal (OstreeFetcher         *self,
 void
 ostree_fetcher_request_uri_with_partial_async (OstreeFetcher         *self,
                                                SoupURI               *uri,
+                                               guint64                max_size,
                                                GCancellable          *cancellable,
                                                GAsyncReadyCallback    callback,
                                                gpointer               user_data)
@@ -443,7 +541,7 @@ ostree_fetcher_request_uri_with_partial_async (OstreeFetcher         *self,
 
   self->total_requests++;
 
-  pending = ostree_fetcher_request_uri_internal (self, uri, FALSE, cancellable,
+  pending = ostree_fetcher_request_uri_internal (self, uri, FALSE, max_size, cancellable,
                                                  callback, user_data,
                                                  ostree_fetcher_request_uri_with_partial_async);
 
@@ -496,6 +594,7 @@ ostree_fetcher_request_uri_with_partial_finish (OstreeFetcher         *self,
 void
 ostree_fetcher_stream_uri_async (OstreeFetcher         *self,
                                  SoupURI               *uri,
+                                 guint64                max_size,
                                  GCancellable          *cancellable,
                                  GAsyncReadyCallback    callback,
                                  gpointer               user_data)
@@ -504,7 +603,7 @@ ostree_fetcher_stream_uri_async (OstreeFetcher         *self,
 
   self->total_requests++;
 
-  pending = ostree_fetcher_request_uri_internal (self, uri, TRUE, cancellable,
+  pending = ostree_fetcher_request_uri_internal (self, uri, TRUE, max_size, cancellable,
                                                  callback, user_data,
                                                  ostree_fetcher_stream_uri_async);
 

src/libostree/ostree-fetcher.h

@@ -65,6 +65,7 @@ guint ostree_fetcher_get_n_requests (OstreeFetcher       *self);
 
 void ostree_fetcher_request_uri_with_partial_async (OstreeFetcher         *self,
                                                     SoupURI               *uri,
+                                                    guint64                max_size,
                                                     GCancellable          *cancellable,
                                                     GAsyncReadyCallback    callback,
                                                     gpointer               user_data);
@@ -74,10 +75,11 @@ GFile *ostree_fetcher_request_uri_with_partial_finish (OstreeFetcher *self,
                                                        GError       **error);
 
 void ostree_fetcher_stream_uri_async (OstreeFetcher         *self,
-                                       SoupURI               *uri,
-                                       GCancellable          *cancellable,
-                                       GAsyncReadyCallback    callback,
-                                       gpointer               user_data);
+                                      SoupURI               *uri,
+                                      guint64                max_size,
+                                      GCancellable          *cancellable,
+                                      GAsyncReadyCallback    callback,
+                                      gpointer               user_data);
 
 GInputStream *ostree_fetcher_stream_uri_finish (OstreeFetcher         *self,
                                                 GAsyncResult          *result,

src/libostree/ostree-repo-commit.c

@@ -551,6 +551,20 @@ write_object (OstreeRepo         *self,
                                         cancellable, error))
         goto out;
 
+      if (OSTREE_OBJECT_TYPE_IS_META (objtype))
+        {
+          if (G_UNLIKELY (file_object_length > OSTREE_MAX_METADATA_WARN_SIZE))
+            {
+              gs_free char *metasize = g_format_size (file_object_length);
+              gs_free char *warnsize = g_format_size (OSTREE_MAX_METADATA_WARN_SIZE);
+              gs_free 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);
+            }
+        }
+
       g_clear_pointer (&temp_filename, g_free);
       g_clear_object (&temp_file);
     }
@@ -1049,16 +1063,35 @@ ostree_repo_write_metadata (OstreeRepo         *self,
                             GCancellable       *cancellable,
                             GError            **error)
 {
+  gboolean ret = FALSE;
   gs_unref_object GInputStream *input = NULL;
   gs_unref_variant GVariant *normalized = NULL;
 
   normalized = g_variant_get_normal_form (object);
+
+  if (G_UNLIKELY (g_variant_get_size (normalized) > OSTREE_MAX_METADATA_SIZE))
+    {
+      gs_free char *input_bytes = g_format_size (g_variant_get_size (normalized));
+      gs_free char *max_bytes = g_format_size (OSTREE_MAX_METADATA_SIZE);
+      g_set_error (error, G_IO_ERROR, G_IO_ERROR_FAILED,
+                   "Metadata object of type '%s' is %s; maximum metadata size is %s",
+                   ostree_object_type_to_string (objtype),
+                   input_bytes,
+                   max_bytes);
+      goto out;
+    }
+
   input = ot_variant_read (normalized);
 
-  return write_object (self, objtype, expected_checksum,
-                       input, g_variant_get_size (normalized),
-                       out_csum,
-                       cancellable, error);
+  if (!write_object (self, objtype, expected_checksum,
+                     input, g_variant_get_size (normalized),
+                     out_csum,
+                     cancellable, error))
+    goto out;
+
+  ret = TRUE;
+ out:
+  return ret;
 }
 
 /**

src/libostree/ostree-repo-pull.c

@@ -287,7 +287,9 @@ fetch_uri_contents_membuf_sync (OtPullData    *pull_data,
   fetch_data.pull_data = pull_data;
 
   pull_data->fetching_sync_uri = uri;
-  ostree_fetcher_stream_uri_async (pull_data->fetcher, uri, cancellable,
+  ostree_fetcher_stream_uri_async (pull_data->fetcher, uri,
+                                   OSTREE_MAX_METADATA_SIZE,
+                                   cancellable,
                                    fetch_uri_sync_on_complete, &fetch_data);
 
   run_mainloop_monitor_fetcher (pull_data);
@@ -883,7 +885,9 @@ enqueue_one_object_request (OtPullData        *pull_data,
   fetch_data->pull_data = pull_data;
   fetch_data->object = ostree_object_name_serialize (checksum, objtype);
   fetch_data->is_detached_meta = is_detached_meta;
-  ostree_fetcher_request_uri_with_partial_async (pull_data->fetcher, obj_uri, pull_data->cancellable,
+  ostree_fetcher_request_uri_with_partial_async (pull_data->fetcher, obj_uri,
+                                                 is_meta ? OSTREE_MAX_METADATA_SIZE : 0,
+                                                 pull_data->cancellable,
                                                  is_meta ? meta_fetch_on_complete : content_fetch_on_complete, fetch_data);
   soup_uri_free (obj_uri);
 }

tests/test-pull-corruption.sh

@@ -23,7 +23,7 @@ set -e
 
 setup_fake_remote_repo1 "archive-z2"
 
-echo '1..1'
+echo '1..2'
 
 repopath=${test_tmpdir}/ostree-srv/gnomerepo
 cp -a ${repopath} ${repopath}.orig

tests/test-pull-large-metadata.sh

@@ -0,0 +1,41 @@
+#!/bin/bash
+#
+# Copyright (C) 2011 Colin Walters <walters@verbum.org>
+#
+# This library is free software; you can redistribute it and/or
+# modify it under the terms of the GNU Lesser General Public
+# License as published by the Free Software Foundation; either
+# version 2 of the License, or (at your option) any later version.
+#
+# This library is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+# Lesser General Public License for more details.
+#
+# You should have received a copy of the GNU Lesser General Public
+# License along with this library; if not, write to the
+# Free Software Foundation, Inc., 59 Temple Place - Suite 330,
+# Boston, MA 02111-1307, USA.
+
+set -e
+
+. $(dirname $0)/libtest.sh
+
+setup_fake_remote_repo1 "archive-z2"
+
+echo '1..1'
+
+# Overwrite the commit object with 20 M of 
+cd ${test_tmpdir}
+rev=$(cd ostree-srv && ostree --repo=gnomerepo rev-parse main)
+dd if=/dev/zero bs=1M count=20 of=ostree-srv/gnomerepo/objects/$(echo $rev | cut -b 1-2)/$(echo $rev | cut -b 3-).commit
+
+cd ${test_tmpdir}
+mkdir repo
+${CMD_PREFIX} ostree --repo=repo init
+${CMD_PREFIX} ostree --repo=repo remote add --set=gpg-verify=false origin $(cat httpd-address)/ostree/gnomerepo
+
+if ostree --repo=repo pull origin main 2>pulllog.txt 1>&2; then
+    assert_not_reached "pull unexpectedly succeeded!"
+fi
+assert_file_has_content pulllog.txt "exceeded maximum"