Skip to content

Commit

Permalink
lib/commit: Try checksum+hardlink for untrusted local same-uid repos
Browse files Browse the repository at this point in the history
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 #1723
and flatpak/flatpak#2342

Closes: #1776
Approved by: uajain
  • Loading branch information
cgwalters authored and rh-atomic-bot committed Dec 4, 2018
1 parent c515c69 commit dc2a7de
Show file tree
Hide file tree
Showing 6 changed files with 64 additions and 62 deletions.
36 changes: 22 additions & 14 deletions src/libostree/ostree-repo-commit.c
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*/
Expand Down Expand Up @@ -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 &&
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand Down
37 changes: 35 additions & 2 deletions tests/basic-test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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}
Expand All @@ -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"
Expand Down
3 changes: 2 additions & 1 deletion tests/test-basic-user-only.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
3 changes: 2 additions & 1 deletion tests/test-basic-user.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 2 additions & 1 deletion tests/test-basic.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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
44 changes: 1 addition & 43 deletions tests/test-pull-untrusted.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit dc2a7de

Please sign in to comment.