Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

core: Always sort incoming xattrs #3346

Merged
merged 1 commit into from
Dec 3, 2024
Merged

core: Always sort incoming xattrs #3346

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
72 changes: 62 additions & 10 deletions src/libostree/ostree-core.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)) \
Expand Down Expand Up @@ -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);
cgwalters marked this conversation as resolved.
Show resolved Hide resolved
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)
cgwalters marked this conversation as resolved.
Show resolved Hide resolved
{
// 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
Expand All @@ -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));
}

Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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);
cgwalters marked this conversation as resolved.
Show resolved Hide resolved
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;
Expand Down
76 changes: 70 additions & 6 deletions tests/test-basic-c.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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)
{
Expand Down Expand Up @@ -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 */
{
Expand All @@ -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);

cgwalters marked this conversation as resolved.
Show resolved Hide resolved
/* 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);
}
}

Expand Down
Loading