From 776213c4a0fa88cf817b011ea0850dc4d5015b15 Mon Sep 17 00:00:00 2001
From: Colin Walters <walters@verbum.org>
Date: Tue, 24 Sep 2024 00:31:28 +0000
Subject: [PATCH] store: Use policy from merged tree with derived layers

I was initially worried this was going to slow things
down but actually AFAICS because we are already setting
a policy we end up rechecking the labels for all of
the base image files anyways, even in the optimal
path.

So what's really happening here is we could likely
speed up the non-derived case. But let's leave that
as a TODO as we have much more important things.

Signed-off-by: Colin Walters <walters@verbum.org>
---
 .github/workflows/rust.yml      | 18 ++++++++++++++++++
 ci/priv-test-cockpit-selinux.sh | 32 ++++++++++++++++++++++++++++++++
 lib/src/container/store.rs      | 15 ++++++++++++++-
 3 files changed, 64 insertions(+), 1 deletion(-)
 create mode 100755 ci/priv-test-cockpit-selinux.sh

diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml
index 6c7a988b..c4dbd46c 100644
--- a/.github/workflows/rust.yml
+++ b/.github/workflows/rust.yml
@@ -146,6 +146,24 @@ jobs:
         run: install ostree-ext-cli /usr/bin && rm -v ostree-ext-cli
       - name: Integration tests
         run: ./ci/priv-integration.sh
+  privtest-cockpit:
+    name: "Privileged testing (cockpit)"
+    needs: build
+    runs-on: ubuntu-latest
+    container:
+      image: quay.io/fedora/fedora-bootc:41
+      options: "--privileged --pid=host -v /var/tmp:/var/tmp -v /run/dbus:/run/dbus -v /run/systemd:/run/systemd -v /:/run/host"
+    steps:
+      - name: Checkout repository
+        uses: actions/checkout@v4
+      - name: Download
+        uses: actions/download-artifact@v4.1.7
+        with:
+          name: ostree-ext-cli
+      - name: Install
+        run: install ostree-ext-cli /usr/bin && rm -v ostree-ext-cli
+      - name: Integration tests
+        run: ./ci/priv-test-cockpit-selinux.sh
   container-build:
     name: "Container build"
     needs: build
diff --git a/ci/priv-test-cockpit-selinux.sh b/ci/priv-test-cockpit-selinux.sh
new file mode 100755
index 00000000..2d71038d
--- /dev/null
+++ b/ci/priv-test-cockpit-selinux.sh
@@ -0,0 +1,32 @@
+#!/bin/bash
+# Assumes that the current environment is a privileged container
+# with the host mounted at /run/host.  We can basically write
+# whatever we want, however we can't actually *reboot* the host.
+set -euo pipefail
+
+sysroot=/run/host
+stateroot=test-cockpit
+repo=$sysroot/ostree/repo
+image=registry.gitlab.com/fedora/bootc/tests/container-fixtures/cockpit
+imgref=ostree-unverified-registry:${image}
+
+cd $(mktemp -d -p /var/tmp)
+
+set -x
+
+if test '!' -e "${sysroot}/ostree"; then
+    ostree admin init-fs --epoch=1 "${sysroot}"
+    ostree config --repo $repo set sysroot.bootloader none
+fi
+ostree admin stateroot-init "${stateroot}" --sysroot "${sysroot}"
+ostree-ext-cli container image deploy --sysroot "${sysroot}" \
+    --stateroot "${stateroot}" --imgref "${imgref}"
+ref=$(ostree refs --repo $repo ostree/container/image | head -1)
+commit=$(ostree rev-parse --repo $repo ostree/container/image/$ref)
+ostree ls --repo $repo -X ${commit} /usr/lib/systemd/system|grep -i cockpit >out.txt
+if ! grep -q :cockpit_unit_file_t:s0 out.txt; then
+    echo "failed to find cockpit_unit_file_t" 1>&2
+    exit 1
+fi
+
+echo ok "derived selinux"
diff --git a/lib/src/container/store.rs b/lib/src/container/store.rs
index 6dc15713..68549165 100644
--- a/lib/src/container/store.rs
+++ b/lib/src/container/store.rs
@@ -851,6 +851,7 @@ impl ImageImporter {
 
         let mut layer_commits = Vec::new();
         let mut layer_filtered_content: MetaFilteredData = HashMap::new();
+        let have_derived_layers = !import.layers.is_empty();
         for layer in import.layers {
             if let Some(c) = layer.commit {
                 tracing::debug!("Reusing fetched commit {}", c);
@@ -980,7 +981,19 @@ impl ImageImporter {
                 let modifier =
                     ostree::RepoCommitModifier::new(ostree::RepoCommitModifierFlags::CONSUME, None);
                 modifier.set_devino_cache(&devino);
-                modifier.set_sepolicy_from_commit(repo, &base_commit, cancellable)?;
+                // If we have derived layers, then we need to handle the case where
+                // the derived layers include custom policy. Just relabel everything
+                // in this case.
+                if have_derived_layers {
+                    let rootpath = td.open_dir(rootpath)?;
+                    let sepolicy = ostree::SePolicy::new_at(rootpath.as_raw_fd(), cancellable)?;
+                    tracing::debug!("labeling from merged tree");
+                    modifier.set_sepolicy(Some(&sepolicy));
+                } else {
+                    tracing::debug!("labeling from base tree");
+                    // TODO: We can likely drop this; we know all labels should be pre-computed.
+                    modifier.set_sepolicy_from_commit(repo, &base_commit, cancellable)?;
+                }
 
                 let mt = ostree::MutableTree::new();
                 repo.write_dfd_to_mtree(