lib: unify user-mode canonical mask to 0775

This aligns all canonical permissions masks to 0775, which is relevant for bare-user-only mode. Such mask is already used for user-mode pulls and checkouts; however the commit logic/modifier was using a different and smaller mask (0755), which resulted in asymmetries across operations.
ostreedev · Aug 25, 2021 · 02f86aa · 02f86aa
1 parent 30909a2
commit 02f86aa
Show file tree

Hide file tree

Showing 6 changed files with 13 additions and 12 deletions.
diff --git a/src/libostree/ostree-core-private.h b/src/libostree/ostree-core-private.h
@@ -36,6 +36,9 @@ G_BEGIN_DECLS
 #define DEFAULT_DIRECTORY_MODE 0775
 #define DEFAULT_REGFILE_MODE 0660
 
+/* Mask to sanitize permissions into a safe canonical subset, for user-bare-only mode. */
+#define USERMODE_CANONICAL_MASK 0775
+
 /* This file contains private implementation data format definitions
  * read by multiple implementation .c files.
  */

diff --git a/src/libostree/ostree-core.c b/src/libostree/ostree-core.c
@@ -2288,7 +2288,7 @@ _ostree_validate_bareuseronly_mode (guint32 content_mode,
 {
  if (S_ISREG (content_mode))
  {
- const guint32 invalid_modebits = ((content_mode & ~S_IFMT) & ~0775);
+ const guint32 invalid_modebits = ((content_mode & ~S_IFMT) & ~USERMODE_CANONICAL_MASK);
  if (invalid_modebits > 0)
  return glnx_throw (error, "Content object %s: invalid mode 0%04o with bits 0%04o",
  checksum, content_mode, invalid_modebits);

diff --git a/src/libostree/ostree-repo-checkout.c b/src/libostree/ostree-repo-checkout.c
@@ -1097,7 +1097,7 @@ checkout_tree_at_recurse (OstreeRepo *self,
  * See also: https:/ostreedev/ostree/pull/909 i.e. 0c4b3a2b6da950fd78e63f9afec602f6188f1ab0
  */
  if (self->mode == OSTREE_REPO_MODE_BARE_USER_ONLY || options->bareuseronly_dirs)
- canonical_mode = (mode & 0775) | S_IFDIR;
+ canonical_mode = (mode & USERMODE_CANONICAL_MASK) | S_IFDIR;
  else
  canonical_mode = mode;
  if (TEMP_FAILURE_RETRY (fchmod (destination_dfd, canonical_mode)) < 0)

diff --git a/src/libostree/ostree-repo-commit.c b/src/libostree/ostree-repo-commit.c
@@ -298,7 +298,7 @@ commit_loose_regfile_object (OstreeRepo *self,
  */
  if (S_ISREG (mode))
  {
- const mode_t content_mode = (mode & (S_IFREG | 0775)) | S_IRUSR;
+ const mode_t content_mode = (mode & USERMODE_CANONICAL_MASK) | S_IFREG | S_IRUSR;
  if (!glnx_fchmod (tmpf->fd, content_mode, error))
  return FALSE;
  }
@@ -1318,7 +1318,7 @@ adopt_and_commit_regfile (OstreeRepo *self,
  if (self->mode == OSTREE_REPO_MODE_BARE_USER_ONLY)
  {
  const guint32 src_mode = g_file_info_get_attribute_uint32 (finfo, "unix::mode");
- if (fchmod (fd, src_mode & 0755) < 0)
+ if (fchmod (fd, src_mode & USERMODE_CANONICAL_MASK) < 0)
  return glnx_throw_errno_prefix (error, "fchmod");
  }
  if (renameat (dfd, name, dest_dfd, loose_path) == -1)
@@ -3323,11 +3323,11 @@ _ostree_repo_commit_modifier_apply (OstreeRepo *self,
  /* 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));
+ g_file_info_set_attribute_uint32 (modified_info, "unix::mode", (mode & USERMODE_CANONICAL_MASK) | S_IFREG);
  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));
+ g_file_info_set_attribute_uint32 (modified_info, "unix::mode", (mode & USERMODE_CANONICAL_MASK) | S_IFDIR);
  break;
  case G_FILE_TYPE_SYMBOLIC_LINK:
  break;

diff --git a/tests/basic-test.sh b/tests/basic-test.sh
@@ -789,10 +789,8 @@ $OSTREE commit ${COMMIT_ARGS} -b content-with-dir-world-writable --tree=dir=file
 $OSTREE fsck
 rm dir-co -rf
 $OSTREE checkout -U -H -M content-with-dir-world-writable dir-co
-if is_bare_user_only_repo repo; then
- assert_file_has_mode dir-co/worldwritable-dir 755
-else
- assert_file_has_mode dir-co/worldwritable-dir 775
+assert_file_has_mode dir-co/worldwritable-dir 775
+if ! is_bare_user_only_repo repo; then
  rm dir-co -rf
  $CMD_PREFIX ostree --repo=repo checkout -U -H content-with-dir-world-writable dir-co
  assert_file_has_mode dir-co/worldwritable-dir 777

diff --git a/tests/test-basic-user-only.sh b/tests/test-basic-user-only.sh
@@ -111,7 +111,7 @@ $OSTREE commit ${COMMIT_ARGS} -b perms files
 $OSTREE fsck
 rm out -rf
 $OSTREE checkout --force-copy perms out
-assert_file_has_mode out/afile 755
+assert_file_has_mode out/afile 775
 $OSTREE checkout ${CHECKOUT_H_ARGS} --union-identical perms out
-assert_file_has_mode out/afile 755
+assert_file_has_mode out/afile 775
 echo "ok automatic canonical perms for bare-user-only"