ex-container: Make /etc/shadow 0400 on import, not post-checkout

Switching to the `_CONSUME` flag revealed an "oh god how did I write that"
bug in the previous patch in https://github.com/projectatomic/rpm-ostree/pull/1046
AKA commit: 334f0b89be

The way that actually fixed the bug before was because we were using
hardlink checkouts, and we were operating outside an `rofiles-fuse`
context, we simply directly changed the on-disk object mode.

But with the `_CONSUME` flag we started deleting the files as we write,
meaning that stopped working.

I *initially* wrote a patch to do the same split "prepare/processing/commit"
flow that treecompose and package layering do, but that can't really fix this
bug - we need to do it on import.

So do the chmod on import and drop the postprocessing bits.

Closes: #1067
Approved by: jlebon

src/app/rpmostree-container-builtins.c

@@ -224,9 +224,6 @@ download_rpms_and_assemble_commit (ROContainerContext *rocctx,
                                           &ret_commit, cancellable, error))
     return FALSE;
 
-  if (!rpmostree_rootfs_postprocess_container (tmpdir.fd, cancellable, error))
-    return FALSE;
-
   *out_commit = g_steal_pointer (&ret_commit);
   return TRUE;
 }

src/libpriv/rpmostree-postprocess.c

@@ -1641,33 +1641,6 @@ rpmostree_prepare_rootfs_for_commit (int            src_rootfs_dfd,
   return TRUE;
 }
 
-/* Run through a standard set of postprocessing for "container"
- * flows as used by `ex container`.  Currently:
- *
- *  - Make /usr/etc/{g,}shadow user readable
- *    See https://github.com/projectatomic/rpm-ostree/issues/1045
- */
-gboolean
-rpmostree_rootfs_postprocess_container (int           rootfs_fd,
-                                        GCancellable *cancellable,
-                                        GError       **error)
-{
-  const char *shadow_paths[] = { "usr/etc/shadow", "usr/etc/gshadow" };
-  for (guint i = 0; i < G_N_ELEMENTS (shadow_paths); i++)
-    {
-      struct stat stbuf;
-      const char *path = shadow_paths[i];
-      if (!glnx_fstatat_allow_noent (rootfs_fd, path, &stbuf, AT_SYMLINK_NOFOLLOW, error))
-        return FALSE;
-      /* Silently ignore if it's not there, or isn't a regular file for some reason */
-      if (errno == ENOENT || !S_ISREG (stbuf.st_mode))
-        continue;
-      if (fchmodat (rootfs_fd, path, stbuf.st_mode | S_IRUSR, 0) < 0)
-        return glnx_throw_errno_prefix (error, "fchmodat");
-    }
-  return TRUE;
-}
-
 struct CommitThreadData {
   volatile gint done;
   off_t n_bytes;

src/libpriv/rpmostree-postprocess.h

@@ -52,11 +52,6 @@ rpmostree_rootfs_postprocess_common (int           rootfs_fd,
                                      GCancellable *cancellable,
                                      GError       **error);
 
-gboolean
-rpmostree_rootfs_postprocess_container (int           rootfs_fd,
-                                        GCancellable *cancellable,
-                                        GError       **error);
-
 gboolean
 rpmostree_prepare_rootfs_get_sepolicy (int            dfd,
                                        OstreeSePolicy **out_sepolicy,

src/libpriv/rpmostree-unpacker.c

@@ -529,7 +529,7 @@ typedef struct
  * https://bugzilla.redhat.com/show_bug.cgi?id=517575
  */
 static void
-workaround_fedora_rpm_permissions (GFileInfo *file_info)
+ensure_directories_user_writable (GFileInfo *file_info)
 {
   if (g_file_info_get_file_type (file_info) == G_FILE_TYPE_DIRECTORY)
     {
@@ -652,7 +652,7 @@ compose_filter_cb (OstreeRepo         *repo,
         }
     }
 
-  workaround_fedora_rpm_permissions (file_info);
+  ensure_directories_user_writable (file_info);
 
   return OSTREE_REPO_COMMIT_FILTER_ALLOW;
 }
@@ -663,7 +663,21 @@ unprivileged_filter_cb (OstreeRepo         *repo,
                         GFileInfo          *file_info,
                         gpointer            user_data)
 {
-  workaround_fedora_rpm_permissions (file_info);
+  /* First, the common directory workaround */
+  ensure_directories_user_writable (file_info);
+
+  /* For unprivileged unpacks, ensure that all files are at least user-readable.
+   * this is (AFAIK) just limited to /usr/etc/{,g}shadow.
+   * See also: https://github.com/projectatomic/rpm-ostree/pull/1046
+   * AKA commit 334f0b89be271cbe2b9973ebc7eab50f955517e8
+   */
+  if (g_file_info_get_file_type (file_info) == G_FILE_TYPE_REGULAR)
+    {
+      guint32 mode = g_file_info_get_attribute_uint32 (file_info, "unix::mode");
+      mode |= S_IRUSR;
+      g_file_info_set_attribute_uint32 (file_info, "unix::mode", mode);
+    }
+
   return OSTREE_REPO_COMMIT_FILTER_ALLOW;
 }
 

tests/ex-container-tests/test-bash.sh

@@ -14,5 +14,6 @@ repos=fedora;
 EOF
 
 rpm-ostree ex container assemble bash.conf
+ostree --repo=repo fsck -q
 ostree --repo=repo ls bash /usr/etc/shadow > shadowls.txt
 assert_file_has_content shadowls.txt '^-00400 .*/usr/etc/shadow'