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.

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

lib/checkout: use canonical permissions in bare-user-only mode #2415

Merged
merged 2 commits into from
Aug 19, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
8 changes: 8 additions & 0 deletions src/libostree/ostree-core.c
Original file line number Diff line number Diff line change
Expand Up @@ -984,13 +984,21 @@ ostree_checksum_file_at (int dfd,

g_autoptr(GFileInfo) file_info = _ostree_stbuf_to_gfileinfo (stbuf);

const gboolean canonicalize_perms =
((flags & OSTREE_CHECKSUM_FLAGS_CANONICAL_PERMISSIONS) != 0);

g_autoptr(GInputStream) in = NULL;
if (S_ISREG (stbuf->st_mode))
{
glnx_autofd int fd = -1;
if (!glnx_openat_rdonly (dfd, path, FALSE, &fd, error))
return FALSE;
in = g_unix_input_stream_new (glnx_steal_fd (&fd), TRUE);
if (canonicalize_perms)
{
g_file_info_set_attribute_uint32 (file_info, "unix::uid", 0);
g_file_info_set_attribute_uint32 (file_info, "unix::gid", 0);
}
}
else if (S_ISLNK (stbuf->st_mode))
{
Expand Down
9 changes: 9 additions & 0 deletions src/libostree/ostree-core.h
Original file line number Diff line number Diff line change
Expand Up @@ -460,12 +460,21 @@ gboolean ostree_break_hardlink (int dfd,

/**
* OstreeChecksumFlags:
* @OSTREE_CHECKSUM_FLAGS_NONE: Default checksumming without tweaks.
* (Since: 2017.13.)
* @OSTREE_CHECKSUM_FLAGS_IGNORE_XATTRS: Ignore xattrs when checksumming.
* (Since: 2017.13.)
* @OSTREE_CHECKSUM_FLAGS_CANONICAL_PERMISSIONS: Use canonical uid/gid/mode
* values, for bare-user-only mode. (Since: 2021.4.)
*
* Flags influencing checksumming logic.
*
* Since: 2017.13
*/
typedef enum {
OSTREE_CHECKSUM_FLAGS_NONE = 0,
OSTREE_CHECKSUM_FLAGS_IGNORE_XATTRS = (1 << 0),
OSTREE_CHECKSUM_FLAGS_CANONICAL_PERMISSIONS = (1 << 1),
} OstreeChecksumFlags;

_OSTREE_PUBLIC
Expand Down
8 changes: 7 additions & 1 deletion src/libostree/ostree-repo-checkout.c
Original file line number Diff line number Diff line change
Expand Up @@ -375,6 +375,9 @@ create_file_copy_from_input_at (OstreeRepo *repo,
if (repo->disable_xattrs)
flags |= OSTREE_CHECKSUM_FLAGS_IGNORE_XATTRS;

if (repo->mode == OSTREE_REPO_MODE_BARE_USER_ONLY)
flags |= OSTREE_CHECKSUM_FLAGS_CANONICAL_PERMISSIONS;

g_autofree char *actual_checksum = NULL;
if (!ostree_checksum_file_at (destination_dfd, destination_name,
&dest_stbuf, OSTREE_OBJECT_TYPE_FILE,
Expand Down Expand Up @@ -526,7 +529,10 @@ checkout_file_hardlink (OstreeRepo *self,
* */
OstreeChecksumFlags flags = 0;
if (self->disable_xattrs)
flags |= OSTREE_CHECKSUM_FLAGS_IGNORE_XATTRS;
flags |= OSTREE_CHECKSUM_FLAGS_IGNORE_XATTRS;

if (self->mode == OSTREE_REPO_MODE_BARE_USER_ONLY)
flags |= OSTREE_CHECKSUM_FLAGS_CANONICAL_PERMISSIONS;

g_autofree char *actual_checksum = NULL;
if (!ostree_checksum_file_at (destination_dfd, destination_name,
Expand Down
8 changes: 4 additions & 4 deletions tests/basic-test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ if is_bare_user_only_repo repo; then
# In bare-user-only repos we can only represent files with uid/gid 0, no
# xattrs and canonical permissions, so we need to commit them as such, or
# we end up with repos that don't pass fsck
COMMIT_ARGS="--canonical-permissions"
COMMIT_ARGS="--canonical-permissions --no-xattrs"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess this is the bit we should be able to get rid of once we fix the commit side of this.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it is the same workaround we suggested at #2410 (comment).

DIFF_ARGS="--owner-uid=0 --owner-gid=0 --no-xattrs"
# Also, since we can't check out uid=0 files we need to check out in user mode
CHECKOUT_U_ARG="-U"
Expand Down Expand Up @@ -706,7 +706,7 @@ for x in $(seq 3); do
# But they share the GPL
echo 'this is the GPL' > pkg${x}/usr/share/licenses/COPYING
ln -s COPYING pkg${x}/usr/share/licenses/LICENSE
$OSTREE commit -b union-identical-pkg${x} --tree=dir=pkg${x}
$OSTREE commit ${COMMIT_ARGS} -b union-identical-pkg${x} --tree=dir=pkg${x}
done
rm union-identical-test -rf
for x in $(seq 3); do
Expand Down Expand Up @@ -756,7 +756,7 @@ cd ${test_tmpdir}
rm files -rf && mkdir files
touch files/anemptyfile
touch files/anotheremptyfile
$CMD_PREFIX ostree --repo=repo commit --consume -b tree-with-empty-files --tree=dir=files
$CMD_PREFIX ostree --repo=repo commit ${COMMIT_ARGS} --consume -b tree-with-empty-files --tree=dir=files
$CMD_PREFIX ostree --repo=repo checkout ${CHECKOUT_H_ARGS} tree-with-empty-files tree-with-empty-files
if files_are_hardlinked tree-with-empty-files/an{,other}emptyfile; then
fatal "--force-copy-zerosized failed"
Expand All @@ -773,7 +773,7 @@ echo "ok checkout zero sized files are not hardlinked"
$CMD_PREFIX ostree --repo=repo checkout ${CHECKOUT_H_ARGS} --union-identical -z tree-with-empty-files tree-with-empty-files
$CMD_PREFIX ostree --repo=repo checkout ${CHECKOUT_H_ARGS} --union-identical -z tree-with-empty-files tree-with-empty-files
echo notempty > tree-with-empty-files/anemptyfile.new && mv tree-with-empty-files/anemptyfile{.new,}
$CMD_PREFIX ostree --repo=repo commit --consume -b tree-with-conflicting-empty-files --tree=dir=tree-with-empty-files
$CMD_PREFIX ostree --repo=repo commit ${COMMIT_ARGS} --consume -b tree-with-conflicting-empty-files --tree=dir=tree-with-empty-files
# Reset back to base
rm tree-with-empty-files -rf
$CMD_PREFIX ostree --repo=repo checkout ${CHECKOUT_H_ARGS} --union-identical -z tree-with-empty-files tree-with-empty-files
Expand Down
15 changes: 14 additions & 1 deletion tests/test-basic-user-only.sh
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ set -euo pipefail

mode="bare-user-only"
setup_test_repository "$mode"
extra_basic_tests=5
extra_basic_tests=6
. $(dirname $0)/basic-test.sh

$CMD_PREFIX ostree --version > version.yaml
Expand All @@ -41,6 +41,7 @@ ostree_repo_init repo init --mode=bare-user-only
cd ${test_tmpdir}
rm repo-input -rf
ostree_repo_init repo-input init --mode=archive

cd ${test_tmpdir}
cat > statoverride.txt <<EOF
2048 /some-setuid
Expand Down Expand Up @@ -99,3 +100,15 @@ if ! skip_one_without_user_xattrs; then
$OSTREE fsck -q
echo "ok hardlink pull from bare-user"
fi

cd ${test_tmpdir}
rm repo -rf
ostree_repo_init repo init --mode=bare-user-only
rm files -rf && mkdir files
echo afile > files/afile
$OSTREE commit ${COMMIT_ARGS} -b perms files
rm out -rf
$OSTREE checkout --force-copy perms out
$OSTREE checkout ${CHECKOUT_H_ARGS} --union-identical perms out
$OSTREE fsck
echo "ok checkout checksum with canonical perms"