commit: filter out selinux label before commit

The new --selinux-policy added in [0] exposed a subtle issue in the way
we handle labeling during commit. The CI system in rpm-ostree hit this
when trying to make use of it[1].

Basically, because of the way we use a GVariant to represent xattrs, if
a file to be committed already has an SELinux label, the xattr object
ends up with *two* label entries. This of course throws off fsck later
on, since the checksum will have gone over both entries, even though the
on-disk file will only have a single label (in which the second entry
wins).

I confirmed that the `fsck` added in the installed test fails without
the rest of this patch.

[0] https://github.com/ostreedev/ostree/pull/1114
[1] https://github.com/projectatomic/rpm-ostree/pull/953

Closes: #1121
Approved by: cgwalters

diff --git a/src/libostree/ostree-repo-commit.c b/src/libostree/ostree-repo-commit.c
index dd63bf1a2d005b8e67129df73a1ca5354b84b203..078b289d38dcdf4cf7aec0c920729a429e58f273 100644 (file)
--- a/src/libostree/ostree-repo-commit.c
+++ b/src/libostree/ostree-repo-commit.c
@@ -34,6 +34,7 @@
 #include "ostree.h"
 #include "ostree-core-private.h"
 #include "ostree-repo-private.h"
+#include "ostree-sepolicy-private.h"
 #include "ostree-repo-file-enumerator.h"
 #include "ostree-checksum-input-stream.h"
 #include "ostree-varint.h"
@@ -2484,6 +2485,16 @@ get_modified_xattrs (OstreeRepo                       *self,
         {
           g_autoptr(GVariantBuilder) builder = NULL;
 
+          if (ret_xattrs)
+            {
+              /* drop out any existing SELinux policy from the set, so we don't end up
+               * counting it twice in the checksum */
+              g_autoptr(GVariant) new_ret_xattrs = NULL;
+              new_ret_xattrs = _ostree_filter_selinux_xattr (ret_xattrs);
+              g_variant_unref (ret_xattrs);
+              ret_xattrs = g_steal_pointer (&new_ret_xattrs);
+            }
+
           /* ret_xattrs may be NULL */
           builder = ot_util_variant_builder_from_variant (ret_xattrs,
                                                           G_VARIANT_TYPE ("a(ayay)"));

diff --git a/tests/installed/itest-label-selinux.sh b/tests/installed/itest-label-selinux.sh
index 1a2b4aaf8126d9db0626b5d7227950dfb0affdaf..1218a8bc8a70d46c03376c3e1bf486bf8c3542e2 100755 (executable)
--- a/tests/installed/itest-label-selinux.sh
+++ b/tests/installed/itest-label-selinux.sh
@@ -24,6 +24,7 @@ ostree --repo=/ostree/repo commit -b testbranch --link-checkout-speedup \
        --selinux-policy co --tree=dir=co
 ostree --repo=/ostree/repo ls -X testbranch /usr/bin/foo-a-generic-binary > ls.txt
 assert_file_has_content ls.txt ${oldcon}
+ostree --repo=/ostree/repo fsck
 
 ostree --repo=/ostree/repo refs --delete testbranch
 rm co -rf