From 3441a48c5835e9470a77032ec150cde87aabe242 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Wed, 16 Sep 2020 13:23:04 +0000 Subject: [PATCH] checkout: Ensure copies of unreadable usermode checkouts are readable The extreme special case of "zero mode" files like `/etc/shadow` comes up again. What we want is for "user mode" checkouts to override it to make the file readable; otherwise when operating as non-root without `CAP_DAC_OVERRIDE` it becomes very difficult to work with. Previously, we were hardlinking these files, but then it intersects with *another* special case around zero sized files, which is *also* true for `/etc/shadow`. Trying to avoid hardlinking there unveiled this bug - when we go to do a copy checkout, we need to override the mode. --- src/libostree/ostree-repo-checkout.c | 20 ++++++++++++++++---- tests/test-basic-user.sh | 2 ++ 2 files changed, 18 insertions(+), 4 deletions(-) diff --git a/src/libostree/ostree-repo-checkout.c b/src/libostree/ostree-repo-checkout.c index dc36370f..10535a35 100644 --- a/src/libostree/ostree-repo-checkout.c +++ b/src/libostree/ostree-repo-checkout.c @@ -613,9 +613,12 @@ checkout_one_file_at (OstreeRepo *repo, } const gboolean is_symlink = (g_file_info_get_file_type (source_info) == G_FILE_TYPE_SYMBOLIC_LINK); + const guint32 source_mode = g_file_info_get_attribute_uint32 (source_info, "unix::mode"); + const gboolean is_unreadable = (!is_symlink && (source_mode & S_IRUSR) == 0); const gboolean is_whiteout = (!is_symlink && options->process_whiteouts && g_str_has_prefix (destination_name, WHITEOUT_PREFIX)); const gboolean is_reg_zerosized = (!is_symlink && g_file_info_get_size (source_info) == 0); + const gboolean override_user_unreadable = (options->mode == OSTREE_REPO_CHECKOUT_MODE_USER && is_unreadable); /* First, see if it's a Docker whiteout, * https://github.com/docker/docker/blob/1a714e76a2cb9008cd19609059e9988ff1660b78/pkg/archive/whiteouts.go @@ -634,7 +637,7 @@ checkout_one_file_at (OstreeRepo *repo, need_copy = FALSE; } - else if (options->force_copy_zerosized && is_reg_zerosized) + else if ((options->force_copy_zerosized && is_reg_zerosized) || override_user_unreadable) { need_copy = TRUE; } @@ -735,7 +738,7 @@ checkout_one_file_at (OstreeRepo *repo, if (can_cache && !is_whiteout && !is_symlink - && !is_reg_zerosized + && !(is_reg_zerosized || override_user_unreadable) && need_copy && repo->mode == OSTREE_REPO_MODE_ARCHIVE && options->mode == OSTREE_REPO_CHECKOUT_MODE_USER) @@ -799,12 +802,21 @@ checkout_one_file_at (OstreeRepo *repo, * succeeded at hardlinking above. */ if (options->no_copy_fallback) - g_assert (is_bare_user_symlink || is_reg_zerosized); + g_assert (is_bare_user_symlink || is_reg_zerosized || override_user_unreadable); if (!ostree_repo_load_file (repo, checksum, &input, NULL, &xattrs, cancellable, error)) return FALSE; - if (!create_file_copy_from_input_at (repo, options, state, checksum, source_info, xattrs, input, + GFileInfo *copy_source_info = source_info; + g_autoptr(GFileInfo) modified_info = NULL; + if (override_user_unreadable) + { + modified_info = g_file_info_dup (source_info); + g_file_info_set_attribute_uint32 (modified_info, "unix::mode", (source_mode | S_IRUSR)); + copy_source_info = modified_info; + } + + if (!create_file_copy_from_input_at (repo, options, state, checksum, copy_source_info, xattrs, input, destination_dfd, destination_name, cancellable, error)) return glnx_prefix_error (error, "Copy checkout of %s to %s", checksum, destination_name); diff --git a/tests/test-basic-user.sh b/tests/test-basic-user.sh index e56f828a..fa17beee 100755 --- a/tests/test-basic-user.sh +++ b/tests/test-basic-user.sh @@ -75,6 +75,8 @@ $OSTREE fsck rm test2-checkout -rf $OSTREE checkout -U -H test2-unreadable test2-checkout assert_file_has_mode test2-checkout/unreadable 400 +# Should not be hardlinked +assert_streq $(stat -c "%h" test2-checkout/unreadable) 1 echo "ok bare-user handled unreadable file" cd ${test_tmpdir}