[ostree] Limit metadata to 10 MiB
- From: Colin Walters <walters src gnome org>
- To: commits-list gnome org
- Cc:
- Subject: [ostree] Limit metadata to 10 MiB
- Date: Tue, 27 May 2014 18:15:39 +0000 (UTC)
commit 47610b45c2ac91d7d9749bc0a1ea5b3150a09a70
Author: Colin Walters <walters verbum org>
Date: Fri Apr 25 15:14:42 2014 -0400
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 | 1 +
src/libostree/ostree-core.h | 14 +++-
src/libostree/ostree-fetcher.c | 135 +++++++++++++++++++++++++++++++-----
src/libostree/ostree-fetcher.h | 10 ++-
src/libostree/ostree-repo-commit.c | 41 ++++++++++-
src/libostree/ostree-repo-pull.c | 8 ++-
tests/test-pull-corruption.sh | 2 +-
tests/test-pull-large-metadata.sh | 41 +++++++++++
8 files changed, 221 insertions(+), 31 deletions(-)
---
diff --git a/Makefile-tests.am b/Makefile-tests.am
index 13c5d60..5c86841 100644
--- a/Makefile-tests.am
+++ b/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 \
diff --git a/src/libostree/ostree-core.h b/src/libostree/ostree-core.h
index ced4ff4..a867dbe 100644
--- a/src/libostree/ostree-core.h
+++ b/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:
diff --git a/src/libostree/ostree-fetcher.c b/src/libostree/ostree-fetcher.c
index 7314098..92dd0a0 100644
--- a/src/libostree/ostree-fetcher.c
+++ b/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);
diff --git a/src/libostree/ostree-fetcher.h b/src/libostree/ostree-fetcher.h
index 928d2a3..55b5e1d 100644
--- a/src/libostree/ostree-fetcher.h
+++ b/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,
diff --git a/src/libostree/ostree-repo-commit.c b/src/libostree/ostree-repo-commit.c
index c3bc836..b955370 100644
--- a/src/libostree/ostree-repo-commit.c
+++ b/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;
}
/**
diff --git a/src/libostree/ostree-repo-pull.c b/src/libostree/ostree-repo-pull.c
index e3e5a77..9097d01 100644
--- a/src/libostree/ostree-repo-pull.c
+++ b/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);
}
diff --git a/tests/test-pull-corruption.sh b/tests/test-pull-corruption.sh
index 9a3cbf5..7e4055c 100755
--- a/tests/test-pull-corruption.sh
+++ b/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
diff --git a/tests/test-pull-large-metadata.sh b/tests/test-pull-large-metadata.sh
new file mode 100644
index 0000000..9b97b82
--- /dev/null
+++ b/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"
[
Date Prev][
Date Next] [
Thread Prev][
Thread Next]
[
Thread Index]
[
Date Index]
[
Author Index]
