From dc2a7de2170aa35c33c9e767532f94be7f6f32c0 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Tue, 4 Dec 2018 09:37:20 -0500 Subject: [PATCH] lib/commit: Try checksum+hardlink for untrusted local same-uid repos This mainly helps flatpak for enabling a hardlink-able local pull during deploy in the --system case. We assume the files are immutable when owned by the same uid. See https://github.com/ostreedev/ostree/issues/1723 and https://github.com/flatpak/flatpak/pull/2342 Closes: #1776 Approved by: uajain --- src/libostree/ostree-repo-commit.c | 36 ++++++++++++++---------- tests/basic-test.sh | 37 +++++++++++++++++++++++-- tests/test-basic-user-only.sh | 3 +- tests/test-basic-user.sh | 3 +- tests/test-basic.sh | 3 +- tests/test-pull-untrusted.sh | 44 +----------------------------- 6 files changed, 64 insertions(+), 62 deletions(-) diff --git a/src/libostree/ostree-repo-commit.c b/src/libostree/ostree-repo-commit.c index f8f8d07021..6cd3fcf470 100644 --- a/src/libostree/ostree-repo-commit.c +++ b/src/libostree/ostree-repo-commit.c @@ -4101,12 +4101,18 @@ import_is_bareuser_only_conversion (OstreeRepo *src_repo, && objtype == OSTREE_OBJECT_TYPE_FILE; } -/* Returns TRUE if we can potentially just call link() to copy an object. */ +/* Returns TRUE if we can potentially just call link() to copy an object; + * if untrusted the repos must be owned by the same uid. + */ static gboolean import_via_reflink_is_possible (OstreeRepo *src_repo, OstreeRepo *dest_repo, - OstreeObjectType objtype) + OstreeObjectType objtype, + gboolean trusted) { + /* Untrusted pulls require matching ownership */ + if (!trusted && (src_repo->owner_uid != dest_repo->owner_uid)) + return FALSE; /* Equal modes are always compatible, and metadata * is identical between all modes. */ @@ -4167,13 +4173,6 @@ import_one_object_direct (OstreeRepo *dest_repo, char loose_path_buf[_OSTREE_LOOSE_PATH_MAX]; _ostree_loose_path (loose_path_buf, checksum, objtype, dest_repo->mode); - if (!import_via_reflink_is_possible (src_repo, dest_repo, objtype)) - { - /* If we can't reflink, nothing to do here */ - *out_was_supported = FALSE; - return TRUE; - } - /* hardlinks require the owner to match and to be on the same device */ const gboolean can_hardlink = src_repo->owner_uid == dest_repo->owner_uid && @@ -4339,7 +4338,7 @@ _ostree_repo_import_object (OstreeRepo *self, */ const gboolean is_bareuseronly_conversion = import_is_bareuser_only_conversion (source, self, objtype); - gboolean try_direct = trusted; + gboolean try_direct = TRUE; /* If we need to do bareuseronly verification, or we're potentially doing a * bareuseronly conversion, let's verify those first so we don't complicate @@ -4379,11 +4378,20 @@ _ostree_repo_import_object (OstreeRepo *self, } } - /* We try to import via reflink/hardlink. If the remote is explicitly not trusted - * (i.e.) their checksums may be incorrect, we skip that. - */ - if (try_direct) + /* First, let's see if we can import via reflink/hardlink. */ + if (try_direct && import_via_reflink_is_possible (source, self, objtype, trusted)) { + /* For local repositories, if the untrusted flag is set, we verify the + * checksum first. This assumes then that the files are immutable - the + * above check verified that the owner uids match. + */ + if (!trusted) + { + if (!ostree_repo_fsck_object (source, objtype, checksum, + cancellable, error)) + return FALSE; + } + gboolean direct_was_supported = FALSE; if (!import_one_object_direct (self, source, checksum, objtype, &direct_was_supported, diff --git a/tests/basic-test.sh b/tests/basic-test.sh index d112533ee6..0c80a5f52e 100644 --- a/tests/basic-test.sh +++ b/tests/basic-test.sh @@ -21,7 +21,7 @@ set -euo pipefail -echo "1..$((86 + ${extra_basic_tests:-0}))" +echo "1..$((88 + ${extra_basic_tests:-0}))" CHECKOUT_U_ARG="" CHECKOUT_H_ARGS="-H" @@ -364,6 +364,39 @@ if ! skip_one_without_user_xattrs; then echo "ok pull-local --bareuseronly-files" fi +rm repo2 -rf +ostree_repo_init repo2 --mode="$mode" +$CMD_PREFIX ostree --repo=repo2 pull-local --untrusted repo test2 +target_file_object=$(ostree_file_path_to_relative_object_path repo test2 baz/saucer) +target_file_checksum=$(ostree_file_path_to_checksum repo test2 baz/saucer) +assert_files_hardlinked repo{,2}/${target_file_object} +echo "ok pull-local hardlinking, untrusted" + +if grep -q 'mode=bare' repo/config; then + # Now copy/corrupt an object in a 3rd repo, pull into 2nd (leaving the first pristine) + rm repo{2,3} -rf + ostree_repo_init repo2 --mode="$mode" + ostree_repo_init repo3 --mode="$mode" + # Pull into 3rd repo, corrupt an object + $CMD_PREFIX ostree --repo=repo3 pull-local repo test2 + cp -a --reflink=auto repo3/${target_file_object}{,.tmp} + mv repo3/${target_file_object}{.tmp,} + echo blah >> repo3/${target_file_object} + if $CMD_PREFIX ostree --repo=repo2 pull-local --untrusted repo3 2>err.txt; then + assert_not_reached "pulled --untrusted from corrupted repo" + fi + assert_file_has_content err.txt 'Corrupted.*'${target_file_checksum} + rm -f err.txt + # But this one should succeed + $CMD_PREFIX ostree --repo=repo2 pull-local repo3 + if $CMD_PREFIX ostree --repo=repo2 fsck 2>err.txt; then + fatal "repo should have pulled corrupted object" + fi + assert_file_has_content err.txt 'Corrupted.*'${target_file_checksum} +fi +echo "ok pull-local --untrusted corruption" +rm repo{2,3} -rf + # This is mostly a copy of the suid test in test-basic-user-only.sh, # but for the `pull --bareuseronly-files` case. cd ${test_tmpdir} @@ -385,7 +418,7 @@ echo "ok pull-local (bareuseronly files)" if ! skip_one_without_user_xattrs; then cd ${test_tmpdir} - ${CMD_PREFIX} ostree --repo=repo2 checkout ${CHECKOUT_U_ARG} test2 test2-checkout-from-local-clone + ${CMD_PREFIX} ostree --repo=repo checkout ${CHECKOUT_U_ARG} test2 test2-checkout-from-local-clone cd test2-checkout-from-local-clone assert_file_has_content yet/another/tree/green 'leaf' echo "ok local clone checkout" diff --git a/tests/test-basic-user-only.sh b/tests/test-basic-user-only.sh index 5f27014ecf..f65094fd5e 100755 --- a/tests/test-basic-user-only.sh +++ b/tests/test-basic-user-only.sh @@ -23,7 +23,8 @@ set -euo pipefail . $(dirname $0)/libtest.sh -setup_test_repository "bare-user-only" +mode="bare-user-only" +setup_test_repository "$mode" extra_basic_tests=5 . $(dirname $0)/basic-test.sh diff --git a/tests/test-basic-user.sh b/tests/test-basic-user.sh index 7bdb6a0ce2..e56f828aa7 100755 --- a/tests/test-basic-user.sh +++ b/tests/test-basic-user.sh @@ -25,7 +25,8 @@ set -euo pipefail skip_without_user_xattrs -setup_test_repository "bare-user" +mode="bare-user" +setup_test_repository "$mode" extra_basic_tests=6 . $(dirname $0)/basic-test.sh diff --git a/tests/test-basic.sh b/tests/test-basic.sh index c7f28c8c57..b1bc37e571 100755 --- a/tests/test-basic.sh +++ b/tests/test-basic.sh @@ -25,5 +25,6 @@ set -euo pipefail skip_without_no_selinux_or_relabel -setup_test_repository "bare" +mode="bare" +setup_test_repository "$mode" . $(dirname $0)/basic-test.sh diff --git a/tests/test-pull-untrusted.sh b/tests/test-pull-untrusted.sh index 4c97259944..ec48eb3eb4 100755 --- a/tests/test-pull-untrusted.sh +++ b/tests/test-pull-untrusted.sh @@ -25,52 +25,10 @@ set -euo pipefail . $(dirname $0)/libtest.sh -echo '1..4' +echo '1..1' setup_test_repository "bare" -cd ${test_tmpdir} -mkdir repo2 -ostree_repo_init repo2 --mode="bare" - -${CMD_PREFIX} ostree --repo=repo2 --untrusted pull-local repo - -find repo2 -type f -links +1 | while read line; do - assert_not_reached "pull-local created hardlinks" -done -echo "ok pull-local --untrusted didn't hardlink" - -# Corrupt repo -for i in ${test_tmpdir}/repo/objects/*/*.file; do - - # make sure it's not a symlink - if [ -L $i ]; then - continue - fi - - echo "corrupting $i" - echo "broke" >> $i - break; -done - -rm -rf repo2 -mkdir repo2 -ostree_repo_init repo2 --mode="bare" -if ${CMD_PREFIX} ostree --repo=repo2 pull-local repo; then - echo "ok trusted pull with corruption succeeded" -else - assert_not_reached "corrupted trusted pull unexpectedly succeeded!" -fi - -rm -rf repo2 -ostree_repo_init repo2 --mode="bare" -if ${CMD_PREFIX} ostree --repo=repo2 pull-local --untrusted repo; then - assert_not_reached "corrupted untrusted pull unexpectedly failed!" -else - echo "ok untrusted pull with corruption failed" -fi - - cd ${test_tmpdir} tar xf ${test_srcdir}/ostree-path-traverse.tar.gz rm -rf repo2