Update selinux support by glevand · Pull Request #3155 · coreos/coreos-overlay

glevand · 2018-03-29T23:00:48Z

Depends on:
~~#3100 (Update packages needed by SELinux)~~ merged
https://github.com/coreos/coreos/bootengine#143 (initrd-setup-root: Add SELinux labels to files)
coreos/portage-stable#654 (Update selinux support)

glevand · 2018-04-20T18:48:47Z

Rebased to latest.

dm0- · 2018-04-25T21:38:17Z

This looks like it needs to be rebased after its dependency was merged.

glevand · 2018-04-25T23:17:03Z

Rebased to latest, added new patches 'profiles: Set make.defaults POLICY_TYPES to mcs' and metadata refresh.

glevand · 2018-05-08T00:22:44Z

Rebased to latest.

ajeddeloh · 2018-05-08T22:07:41Z

 	KEYWORDS="~amd64 ~arm ~arm64 ~x86"
 else
-	CROS_WORKON_COMMIT="deba0732daec569545cf456f0cc514f17c7529b5"
+	CROS_WORKON_COMMIT="HEAD"


Is this meant as a workaround until coreos/bootengine#143 merges?

@ajeddeloh Yes, once coreos/bootengine#143 is merged this commit hash can be set.

ajeddeloh

Couple minor nits, otherwise LGTM. Thank's for the cleanup!

ajeddeloh · 2018-05-08T23:13:59Z

 if [[ ${PV} != 9999* ]] ; then
 	KEYWORDS="amd64 -arm arm64 ~mips x86"
 fi
+


nit: trailing newline

ajeddeloh · 2018-05-08T23:17:08Z

@@ -0,0 +1,3 @@
+process = "system_u:system_r:svirt_lxc_net_t:s0"


Where did these rules (and the ones below in the ebuiild itself) come from exactly? Since this commit is non-trivial and modifies the policy itself, can you add a commit message with more details as to what's changing and why (especially since selinux policies are not the most intuitive)

That change came from Matthew Garrett's original patch here: adb930d. I seem to have not set him as author. His commit comments says:

sec-policy/*: We need custom policy modifications

I'll try to better understand the changes and update the commit comments.

@ajeddeloh The added lxc_contexts file is needed to support rkt. You can find some info here: https://coreos.com/rkt/docs/latest/selinux.html. Also, the lines in the ebuild that add to the policy are to support rkt. I've added comments to the sec-policy/selinux-base files that mention this. On another note, I think Container Linux's SELinux support for rkt needs to be reviewed and verified it is working correctly. See: rkt/rkt#3927

ajeddeloh · 2018-05-08T23:19:01Z

@@ -0,0 +1,31 @@
+# Copyright 2014 CoreOS, Inc.


2018? Or is this being moved from somewhere I'm not seeing?

dm0-

Oops, I wrote a few comments a while ago and forgot to click the "submit" button.

dm0- · 2018-05-04T18:07:59Z

+	# image in the SDK build_image script.
+	[[ "${CBUILD}" == "${CHOST}" ]] || return
+
+	selinux-policy-2_pkg_postinst


This eclass should probably inherit selinux-policy-2 so ebuilds don't need to know about both.

I would have liked to do that, but that caused problems. This new coreos-sec-policy.eclass has common things intended for use by all the sec-policy packages. selinux-policy-2.eclass is intended for use by the policy add-on packages like selinux-virt and selinux-unconfined, and not for the base policy package. Errors occur if selinux-policy-2 is inherit in selinux-base.

One way to avoid having to inherit both is to have a conditional on ${PN} inside coreos-sec-policy.eclass to inherit selinux-policy-2. Would you prefer that?

It seems the above is no longer true. It may have been a condition of older packages. Things seem to work OK with a simple inherit selinux-policy-2 in coreos-sec-policy.eclass.

I just did a fresh sdk build and it turns out that the problem is still there. selinux-policy-2.eclass has a DEPEND on selinux-base-policy, and so creates a circular dependency if either selinux-base or selinux-base-policy inherit selinux-policy-2.eclass. I added a conditional to coreos-sec-policy.eclass to fix this.

dm0- · 2018-05-04T18:09:03Z

@@ -0,0 +1,31 @@
+# Copyright 2014 CoreOS, Inc.


A new file should have the current year.

dm0- · 2018-05-04T18:13:07Z

 src_install() {
 	emake DESTDIR="${D}" \
-		LIBSEPOLA="/usr/$(get_libdir)/libsepol.a" \
+		LIBSEPOLA="${ROOT:-/}usr/$(get_libdir)" \


Is this expected to be changing a file to a directory?

Seems like that's a typo, but LIBSEPOLA is just used as a makefile target dependency so it was working. I'll fix it.

dm0- · 2018-05-04T18:16:51Z

 MODS="unconfined"

-inherit selinux-policy-2
+inherit selinux-policy-2 coreos-sec-policy


The selinux-policy-2 eclass can be dropped if coreos-sec-policy inherits it, since it depends on it.

dm0- · 2018-05-04T18:17:34Z

 MODS="virt"

-inherit selinux-policy-2
+inherit selinux-policy-2 coreos-sec-policy


Again only inherit coreos-sec-policy should be needed.

dm0- · 2018-05-04T18:19:19Z

 # bind-tools' configure script breaks when cross-compiling with seccomp enabled
 net-dns/bind-tools -seccomp
+
+# Enable SELinux for amd64 targets


The amd64 targets is no longer correct since this profile is for all boards.

We have a few SELinux entries in the generic/package.use file, each saying Enable SELinux for XXX. I'll clean that up and have a single section for SELinux with the packages in alphabetical order.

dm0- · 2018-05-04T18:20:14Z

 else
 	SRC_URI="https://raw.githubusercontent.com/wiki/SELinuxProject/selinux/files/releases/${MY_RELEASEDATE}/${MY_P}.tar.gz"
-	KEYWORDS="amd64 ~arm ~arm64 ~mips x86"
+	KEYWORDS="amd64 ~arm arm64 ~mips x86"


If this is the only customization required now, maybe it should be moved back to portage-stable and accepted in the arm64 profile.

Yes, I missed that. It goes together with sys-libs/libsepol.

glevand · 2018-05-23T16:35:22Z

Rebased to latest. Addressed all comments.

From: David Michael <david.michael@coreos.com> [rebased to latest SELinux ebuilds] Signed-off-by: Geoff Levand <geoff@infradead.org>

Container Linux only uses the mcs policy type. Signed-off-by: Geoff Levand <geoff@infradead.org>