diff --git a/src/libostree/ostree-core.c b/src/libostree/ostree-core.c index fb11f85bc6..7f1c18dddd 100644 --- a/src/libostree/ostree-core.c +++ b/src/libostree/ostree-core.c @@ -302,6 +302,44 @@ ostree_validate_collection_id (const char *collection_id, GError **error) return TRUE; } +static int +compare_xattrs (const void *a_pp, const void *b_pp) +{ + GVariant *a = *(GVariant **)a_pp; + GVariant *b = *(GVariant **)b_pp; + const char *name_a; + const char *name_b; + g_variant_get (a, "(^&ay@ay)", &name_a, NULL); + g_variant_get (b, "(^&ay@ay)", &name_b, NULL); + + return strcmp (name_a, name_b); +} + +// Sort xattrs by name +static GVariant * +canonicalize_xattrs (GVariant *xattrs) +{ + g_assert (xattrs); + g_autoptr (GPtrArray) xattr_array + = g_ptr_array_new_with_free_func ((GDestroyNotify)g_variant_unref); + + const guint n = g_variant_n_children (xattrs); + for (guint i = 0; i < n; i++) + { + GVariant *child = g_variant_get_child_value (xattrs, i); + g_ptr_array_add (xattr_array, child); + } + + g_ptr_array_sort (xattr_array, compare_xattrs); + + GVariantBuilder builder; + g_variant_builder_init (&builder, G_VARIANT_TYPE ("a(ayay)")); + for (guint i = 0; i < xattr_array->len; i++) + g_variant_builder_add_value (&builder, xattr_array->pdata[i]); + + return g_variant_ref_sink (g_variant_builder_end (&builder)); +} + /* The file header is part of the "object stream" format * that's not compressed. It's comprised of uid,gid,mode, * and possibly symlink targets from @file_info, as well @@ -324,10 +362,15 @@ _ostree_file_header_new (GFileInfo *file_info, GVariant *xattrs) g_autoptr (GVariant) tmp_xattrs = NULL; if (xattrs == NULL) tmp_xattrs = g_variant_ref_sink (g_variant_new_array (G_VARIANT_TYPE ("(ayay)"), NULL, 0)); + else + { + // We always sort the xattrs now to ensure everything is in normal/canonical form. + tmp_xattrs = canonicalize_xattrs (xattrs); + } g_autoptr (GVariant) ret = g_variant_new ("(uuuus@a(ayay))", GUINT32_TO_BE (uid), GUINT32_TO_BE (gid), - GUINT32_TO_BE (mode), 0, symlink_target, xattrs ?: tmp_xattrs); + GUINT32_TO_BE (mode), 0, symlink_target, tmp_xattrs); return variant_to_lenprefixed_buffer (g_variant_ref_sink (ret)); } @@ -2278,21 +2321,30 @@ ostree_validate_structureof_file_mode (guint32 mode, GError **error) } /* Currently ostree imposes no restrictions on xattrs on its own; - * they can e.g. be arbitrariliy sized or in number. - * However, we do validate the key is non-empty, as that is known - * to always fail. + * they can e.g. be arbitrariliy sized or in number. The xattrs + * must be sorted by name (without duplicates), and keys cannot be empty. */ gboolean _ostree_validate_structureof_xattrs (GVariant *xattrs, GError **error) { const guint n = g_variant_n_children (xattrs); + const char *previous = NULL; for (guint i = 0; i < n; i++) { - const guint8 *name; + const char *name; g_autoptr (GVariant) value = NULL; g_variant_get_child (xattrs, i, "(^&ay@ay)", &name, &value); if (!*name) return glnx_throw (error, "Invalid xattr name (empty or missing NUL) index=%d", i); + if (previous) + { + int cmp = strcmp (previous, name); + if (cmp == 0) + return glnx_throw (error, "Duplicate xattr name, index=%d", i); + else if (cmp > 0) + return glnx_throw (error, "Incorrectly sorted xattr name, index=%d", i); + } + previous = name; i++; } return TRUE; diff --git a/tests/test-basic-c.c b/tests/test-basic-c.c index 7124b5ae93..52b020fb36 100644 --- a/tests/test-basic-c.c +++ b/tests/test-basic-c.c @@ -130,12 +130,12 @@ test_raw_file_to_archive_stream (gconstpointer data) } static gboolean -hi_content_stream_new (GInputStream **out_stream, guint64 *out_length, GError **error) +basic_regfile_content_stream_new (const char *contents, GVariant *xattr, GInputStream **out_stream, + guint64 *out_length, GError **error) { - static const char hi[] = "hi"; - const size_t len = sizeof (hi) - 1; + const size_t len = strlen (contents); g_autoptr (GMemoryInputStream) hi_memstream - = (GMemoryInputStream *)g_memory_input_stream_new_from_data (hi, len, NULL); + = (GMemoryInputStream *)g_memory_input_stream_new_from_data (contents, len, NULL); g_autoptr (GFileInfo) finfo = g_file_info_new (); g_file_info_set_attribute_uint32 (finfo, "standard::type", G_FILE_TYPE_REGULAR); g_file_info_set_attribute_boolean (finfo, "standard::is-symlink", FALSE); @@ -147,6 +147,12 @@ hi_content_stream_new (GInputStream **out_stream, guint64 *out_length, GError ** out_length, NULL, error); } +static gboolean +hi_content_stream_new (GInputStream **out_stream, guint64 *out_length, GError **error) +{ + return basic_regfile_content_stream_new ("hi", NULL, out_stream, out_length, error); +} + static void test_validate_remotename (void) { @@ -174,6 +180,8 @@ test_object_writes (gconstpointer data) static const char hi_sha256[] = "2301b5923720c3edc1f0467addb5c287fd5559e3e0cd1396e7f1edb6b01be9f0"; + static const char invalid_hi_sha256[] + = "cafebabecafebabecafebabecafebabecafebabecafebabecafebabecafebabe"; /* Successful content write */ { @@ -193,12 +201,49 @@ test_object_writes (gconstpointer data) hi_content_stream_new (&hi_memstream, &len, &error); g_assert_no_error (error); g_autofree guchar *csum = NULL; - static const char invalid_hi_sha256[] - = "cafebabecafebabecafebabecafebabecafebabecafebabecafebabecafebabe"; g_assert (!ostree_repo_write_content (repo, invalid_hi_sha256, hi_memstream, len, &csum, NULL, &error)); g_assert (error); g_assert (strstr (error->message, "Corrupted file object")); + g_clear_error (&error); + } + + /* Test a basic regfile inline write, no xattrs */ + g_assert (ostree_repo_write_regfile_inline (repo, hi_sha256, 0, 0, S_IFREG | 0644, NULL, + (guint8 *)"hi", 2, NULL, &error)); + g_assert_no_error (error); + + /* Test a basic regfile inline write, erroring on checksum */ + g_assert (!ostree_repo_write_regfile_inline (repo, invalid_hi_sha256, 0, 0, S_IFREG | 0644, NULL, + (guint8 *)"hi", 2, NULL, &error)); + g_assert (error != NULL); + g_clear_error (&error); + + static const char expected_sha256_with_xattrs[] + = "72473a9e8ace75f89f1504137cfb134feb5372bc1d97e04eb5300e24ad836153"; + + /* Now test with xattrs */ + { + GVariantBuilder builder; + g_variant_builder_init (&builder, G_VARIANT_TYPE ("a(ayay)")); + g_variant_builder_add (&builder, "(^ay^ay)", "security.selinux", "foo_t"); + g_variant_builder_add (&builder, "(^ay^ay)", "user.blah", "somedata"); + g_autoptr (GVariant) xattrs = g_variant_ref_sink (g_variant_builder_end (&builder)); + ostree_repo_write_regfile_inline (repo, expected_sha256_with_xattrs, 0, 0, S_IFREG | 0644, + xattrs, (guint8 *)"hi", 2, NULL, &error); + g_assert_no_error (error); + } + + /* And now with a swapped order */ + { + GVariantBuilder builder; + g_variant_builder_init (&builder, G_VARIANT_TYPE ("a(ayay)")); + g_variant_builder_add (&builder, "(^ay^ay)", "user.blah", "somedata"); + g_variant_builder_add (&builder, "(^ay^ay)", "security.selinux", "foo_t"); + g_autoptr (GVariant) xattrs = g_variant_ref_sink (g_variant_builder_end (&builder)); + ostree_repo_write_regfile_inline (repo, expected_sha256_with_xattrs, 0, 0, S_IFREG | 0644, + xattrs, (guint8 *)"hi", 2, NULL, &error); + g_assert_no_error (error); } }