diff --git a/src/libostree/ostree-core.c b/src/libostree/ostree-core.c index fb11f85bc6..e67c23f81b 100644 --- a/src/libostree/ostree-core.c +++ b/src/libostree/ostree-core.c @@ -43,6 +43,7 @@ G_STATIC_ASSERT (OSTREE_REPO_MODE_BARE_USER_ONLY == 3); G_STATIC_ASSERT (OSTREE_REPO_MODE_BARE_SPLIT_XATTRS == 4); static GBytes *variant_to_lenprefixed_buffer (GVariant *variant); +static GVariant *canonicalize_xattrs (GVariant *xattrs); #define ALIGN_VALUE(this, boundary) \ ((((unsigned long)(this)) + (((unsigned long)(boundary)) - 1)) \ @@ -302,6 +303,47 @@ 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) +{ + // We always need to provide data, so NULL is canonicalized to the empty array + if (xattrs == NULL) + return g_variant_ref_sink (g_variant_new_array (G_VARIANT_TYPE ("(ayay)"), NULL, 0)); + + 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 @@ -321,13 +363,12 @@ _ostree_file_header_new (GFileInfo *file_info, GVariant *xattrs) else symlink_target = ""; - 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)); + // We always sort the xattrs now to ensure everything is in normal/canonical form. + g_autoptr (GVariant) 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)); } @@ -1111,11 +1152,13 @@ ostree_create_directory_metadata (GFileInfo *dir_info, GVariant *xattrs) { GVariant *ret_metadata = NULL; + // We always sort the xattrs now to ensure everything is in normal/canonical form. + g_autoptr (GVariant) tmp_xattrs = canonicalize_xattrs (xattrs); + ret_metadata = g_variant_new ( "(uuu@a(ayay))", GUINT32_TO_BE (g_file_info_get_attribute_uint32 (dir_info, "unix::uid")), GUINT32_TO_BE (g_file_info_get_attribute_uint32 (dir_info, "unix::gid")), - GUINT32_TO_BE (g_file_info_get_attribute_uint32 (dir_info, "unix::mode")), - xattrs ? xattrs : g_variant_new_array (G_VARIANT_TYPE ("(ayay)"), NULL, 0)); + GUINT32_TO_BE (g_file_info_get_attribute_uint32 (dir_info, "unix::mode")), tmp_xattrs); g_variant_ref_sink (ret_metadata); return ret_metadata; @@ -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..a27dbc8fe8 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,68 @@ 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"; + + 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) inorder_xattrs = g_variant_ref_sink (g_variant_builder_end (&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) unsorted_xattrs = g_variant_ref_sink (g_variant_builder_end (&builder)); + + /* Now test with xattrs */ + g_assert (ostree_repo_write_regfile_inline (repo, expected_sha256_with_xattrs, 0, 0, + S_IFREG | 0644, inorder_xattrs, (guint8 *)"hi", 2, + NULL, &error)); + g_assert_no_error (error); + + /* And now with a swapped order */ + g_assert (ostree_repo_write_regfile_inline (repo, expected_sha256_with_xattrs, 0, 0, + S_IFREG | 0644, unsorted_xattrs, (guint8 *)"hi", 2, + NULL, &error)); + g_assert_no_error (error); + + /* Tests of directory metadata */ + static const char expected_dirmeta_sha256[] + = "f773ab98198d8e46f77be6ffff5fc1920888c0af5794426c3b1461131d509f34"; + g_autoptr (GFileInfo) fi = g_file_info_new (); + g_file_info_set_attribute_uint32 (fi, "unix::uid", 0); + g_file_info_set_attribute_uint32 (fi, "unix::gid", 0); + g_file_info_set_attribute_uint32 (fi, "unix::mode", (0755 | S_IFDIR)); + { + g_autoptr (GVariant) dirmeta = ostree_create_directory_metadata (fi, inorder_xattrs); + ostree_repo_write_metadata (repo, OSTREE_OBJECT_TYPE_DIR_META, expected_dirmeta_sha256, dirmeta, + NULL, NULL, &error); + g_assert_no_error (error); + } + /* And now with unsorted xattrs */ + { + g_autoptr (GVariant) dirmeta = ostree_create_directory_metadata (fi, unsorted_xattrs); + ostree_repo_write_metadata (repo, OSTREE_OBJECT_TYPE_DIR_META, expected_dirmeta_sha256, dirmeta, + NULL, NULL, &error); + g_assert_no_error (error); } }