[PATCH] selinux: Only attempt to load policy exactly once, in the real root

* [PATCH] selinux: Only attempt to load policy exactly once, in the real root
@ 2014-02-20 15:42 Colin Walters
  2014-02-20 18:06 ` Stephen Smalley
  0 siblings, 1 reply; 15+ messages in thread
From: Colin Walters @ 2014-02-20 15:42 UTC (permalink / raw)
  To: SELinux-NSA; +Cc: systemd Mailing List


[-- Attachment #1.1: Type: text/plain, Size: 1325 bytes --]

Currently on at least Fedora, SELinux policy does not come in
the initramfs. systemd will attempt to load *both* in the
initramfs and in the real root.

Now, the selinux_init_load_policy() API has a regular error return
value, as well as an "enforcing" boolean. To determine enforcing
state, it looks for /etc/selinux/config as well as the presence
of "enforcing=" on the kernel command line.

Ordinarily, neither of those exist in the initramfs, so it will return
"unknown" for enforcing, and systemd will simply ignore the failure to
load policy.

Then later after we switch to the real root, we have the config file,
and all will work properly.

Except...this all blows up if someone explicitly specifies enforcing=1
on the kernel command line. Then systemd will fail to load the
nonexistent policy in the initramfs and freeze.

What this patch does is quite simple - we add an internal API that
says where we expect to find policy, and attempt to load it exactly
from there. Right now since I'm not aware of anyone who does
policy-in-initramfs, this function is hardcoded to return false.

Lots-of-very-painful-debugging-by: Colin Walters <walters@verbum.org>
---
 src/core/main.c | 6 ++++--
 src/core/selinux-setup.c | 10 ++++++++++
 src/core/selinux-setup.h | 2 ++
 3 files changed, 16 insertions(+), 2 deletions(-)




[-- Attachment #1.2: Type: text/html, Size: 1716 bytes --]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-selinux-Only-attempt-to-load-policy-exactly-once-in-.patch --]
[-- Type: text/x-patch, Size: 3275 bytes --]

>From b109b6e0c1ea7509f08a09d66072c86bea279a06 Mon Sep 17 00:00:00 2001
From: Colin Walters <walters@verbum.org>
Date: Thu, 20 Feb 2014 10:15:10 -0500
Subject: [PATCH] selinux: Only attempt to load policy exactly once, in the
 real root

Currently on at least Fedora, SELinux policy does not come in
the initramfs.  systemd will attempt to load *both* in the
initramfs and in the real root.

Now, the selinux_init_load_policy() API has a regular error return
value, as well as an "enforcing" boolean.  To determine enforcing
state, it looks for /etc/selinux/config as well as the presence
of "enforcing=" on the kernel command line.

Ordinarily, neither of those exist in the initramfs, so it will return
"unknown" for enforcing, and systemd will simply ignore the failure to
load policy.

Then later after we switch to the real root, we have the config file,
and all will work properly.

Except...this all blows up if someone explicitly specifies enforcing=1
on the kernel command line.  Then systemd will fail to load the
nonexistent policy in the initramfs and freeze.

What this patch does is quite simple - we add an internal API that
says where we expect to find policy, and attempt to load it exactly
from there.  Right now since I'm not aware of anyone who does
policy-in-initramfs, this function is hardcoded to return false.

Lots-of-very-painful-debugging-by: Colin Walters <walters@verbum.org>
---
 src/core/main.c          |  6 ++++--
 src/core/selinux-setup.c | 10 ++++++++++
 src/core/selinux-setup.h |  2 ++
 3 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/src/core/main.c b/src/core/main.c
index 58c3a9e..8a13b54 100644
--- a/src/core/main.c
+++ b/src/core/main.c
@@ -1296,8 +1296,10 @@ int main(int argc, char *argv[]) {
 
                 if (!skip_setup) {
                         mount_setup_early();
-                        if (selinux_setup(&loaded_policy) < 0)
-                                goto finish;
+                        if (in_initrd() == selinux_load_policy_in_initramfs()) {
+                                if (selinux_setup(&loaded_policy) < 0)
+                                        goto finish;
+                        }
                         if (ima_setup() < 0)
                                 goto finish;
                         if (smack_setup() < 0)
diff --git a/src/core/selinux-setup.c b/src/core/selinux-setup.c
index 7a32ed5..f689149 100644
--- a/src/core/selinux-setup.c
+++ b/src/core/selinux-setup.c
@@ -37,6 +37,16 @@
 #include "util.h"
 #include "log.h"
 
+/**
+ * At present, I'm not aware of a SELinux user who loads the policy
+ * from the initramfs.  If you are such a user, you can flip this
+ * define with a patch.  Alternatively in the future, this information
+ * could come from the kernel command line.
+ */
+bool selinux_load_policy_in_initramfs(void) {
+        return false;
+}
+
 #ifdef HAVE_SELINUX
 static int null_log(int type, const char *fmt, ...) {
         return 0;
diff --git a/src/core/selinux-setup.h b/src/core/selinux-setup.h
index 39e2bc2..d3f3bbc 100644
--- a/src/core/selinux-setup.h
+++ b/src/core/selinux-setup.h
@@ -23,4 +23,6 @@
 
 #include <stdbool.h>
 
+bool selinux_load_policy_in_initramfs(void);
+
 int selinux_setup(bool *loaded_policy);
-- 
1.8.3.1


^ permalink raw reply related	[flat|nested] 15+ messages in thread