Canonicalize bare-user-only perms with 0755 mask

For the flatpak use case where bare-user-only was introduced, we actually
don't want to support s{u,g} id files in particular.

Actually, I can't think of a reason to have anything outside of the
`0755 i.e. (u=rwx,g=rx,o=rx)` mask, so that's what we do here.

This will have the effect of treating existing `bare-user-only` repositories as
corrupted if they have files outside of that mask, but I think we should do this
now; most of the flatpak users will still be on `bare-user`, and we haven't
changed the semantics of that mode yet.

Note that in this patch we will also *reject* file content that doesn't
match this.  This is somewhat asymmetric, since we aren't similarly rejecting
e.g. directory metadata.  But, this will close off the biggest source
of the problem for flatpak (setuid binaries).

See: https://github.com/ostreedev/ostree/pull/908
See: https://github.com/flatpak/flatpak/pull/837

Closes: #909
Approved by: alexlarsson
This commit is contained in:
Colin Walters 2017-06-06 13:34:27 -04:00 committed by Atomic Bot
parent 25696b3fb0
commit 0c4b3a2b6d
2 changed files with 37 additions and 9 deletions

View File

@ -281,7 +281,7 @@ commit_loose_object_trusted (OstreeRepo *self,
}
if (objtype == OSTREE_OBJECT_TYPE_FILE &&
(self->mode == OSTREE_REPO_MODE_BARE_USER || self->mode == OSTREE_REPO_MODE_BARE_USER_ONLY))
self->mode == OSTREE_REPO_MODE_BARE_USER)
{
if (!object_is_symlink)
{
@ -294,10 +294,21 @@ commit_loose_object_trusted (OstreeRepo *self,
return glnx_throw_errno (error);
}
if (self->mode == OSTREE_REPO_MODE_BARE_USER &&
!write_file_metadata_to_xattr (fd, uid, gid, mode, xattrs, error))
if (!write_file_metadata_to_xattr (fd, uid, gid, mode, xattrs, error))
return FALSE;
}
else if (objtype == OSTREE_OBJECT_TYPE_FILE &&
self->mode == OSTREE_REPO_MODE_BARE_USER_ONLY
&& !object_is_symlink)
{
guint32 invalid_modebits = (mode & ~S_IFMT) & ~0755;
if (invalid_modebits > 0)
return glnx_throw (error, "Invalid mode 0%04o with bits 0%04o in bare-user-only repository",
mode, invalid_modebits);
if (fchmod (fd, mode) < 0)
return glnx_throw_errno_prefix (error, "fchmod");
}
if (objtype == OSTREE_OBJECT_TYPE_FILE && _ostree_repo_mode_is_bare (self->mode))
{
@ -2293,11 +2304,23 @@ _ostree_repo_commit_modifier_apply (OstreeRepo *self,
if ((modifier->flags & OSTREE_REPO_COMMIT_MODIFIER_FLAGS_CANONICAL_PERMISSIONS) != 0)
{
if (g_file_info_get_file_type (file_info) == G_FILE_TYPE_REGULAR)
guint mode = g_file_info_get_attribute_uint32 (modified_info, "unix::mode");
switch (g_file_info_get_file_type (file_info))
{
guint current_mode = g_file_info_get_attribute_uint32 (modified_info, "unix::mode");
g_file_info_set_attribute_uint32 (modified_info, "unix::mode", current_mode | 0744);
case G_FILE_TYPE_REGULAR:
/* In particular, we want to squash the s{ug}id bits, but this also
* catches the sticky bit for example.
*/
g_file_info_set_attribute_uint32 (modified_info, "unix::mode", mode & (S_IFREG | 0755));
break;
case G_FILE_TYPE_DIRECTORY:
/* Like the above but for directories */
g_file_info_set_attribute_uint32 (modified_info, "unix::mode", mode & (S_IFDIR | 0755));
break;
case G_FILE_TYPE_SYMBOLIC_LINK:
break;
default:
g_assert_not_reached ();
}
g_file_info_set_attribute_uint32 (modified_info, "unix::uid", 0);
g_file_info_set_attribute_uint32 (modified_info, "unix::gid", 0);

View File

@ -312,8 +312,13 @@ cd ${test_tmpdir}/checkout-test2-4
$OSTREE commit ${COMMIT_ARGS} -b test2-override -s "with statoverride" --statoverride=../test-statoverride.txt
cd ${test_tmpdir}
$OSTREE checkout test2-override checkout-test2-override
test -g checkout-test2-override/a/nested/2
test -u checkout-test2-override/a/nested/3
if ! is_bare_user_only_repo repo; then
test -g checkout-test2-override/a/nested/2
test -u checkout-test2-override/a/nested/3
else
test '!' -g checkout-test2-override/a/nested/2
test '!' -u checkout-test2-override/a/nested/3
fi
echo "ok commit statoverride"
cd ${test_tmpdir}