On Mon, Jul 04, 2016 at 04:34:22PM +0100, Richard W.M. Jones wrote:
I don't exactly know where to post this, but I guess I have
everyone's
attention on this thread.
Attached are patches which work for me. They could really do with
review from someone who knows what they're doing. They also need much
more testing than I've done, but I'll be doing that myself later.
The first patch (against libselinux) sets SELinux to Permissive mode
early in boot if the /.autorelabel file is found (or autorelabel on
the command line).
The second patch (against policycoreutils in Fedora) implements the
generator itself.
This is great!
Some problems I found:
- It would be nice if systemd defined a %{_generatorsdir} RPM macro.
Yeah, we
should. If nobody beats me to it, I'll file a pull request.
- I couldn't get it to work only depending on local-fs.target.
I had
to depend on sysinit.target. With local-fs.target, /boot could not
be mounted, so there may be something broken/missing in
local-fs.target.
local-fs.target should mount /boot also. It would be much
better to have
autorelabel.service only depend on local-fs.target, since sysinit.target
pulls in quite a few daemons. Do you maybe have the logs for the failed
attempt with local-fs.target?
Some more comments inline below.
- There seems to be no upstream for selinux-autorelabel* since it
was
moved from systemd. It looks like the only upstream is Fedora's
policycoreutils itself. Maybe I missed something there.
Rich.
--
Richard Jones, Virtualization Group, Red Hat
http://people.redhat.com/~rjones
Read my programming and virtualization blog:
http://rwmj.wordpress.com
virt-p2v converts physical machines to virtual machines. Boot with a
live CD or over the network (PXE) and turn machines into KVM guests.
http://libguestfs.org/virt-v2v
From c9b8d9da73d8f530df9a8672413d1db842ff45d5 Mon Sep 17 00:00:00
2001
From: "Richard W.M. Jones" <rjones(a)redhat.com>
Date: Mon, 4 Jul 2016 11:42:14 +0100
Subject: [PATCH] libselinux: If autorelabel, force permissive mode.
Signed-off-by: Richard W.M. Jones <rjones(a)redhat.com>
---
libselinux/src/load_policy.c | 30 +++++++++++++++++++++++++-----
1 file changed, 25 insertions(+), 5 deletions(-)
diff --git a/libselinux/src/load_policy.c b/libselinux/src/load_policy.c
index 4f39fc7..337a8a9 100644
--- a/libselinux/src/load_policy.c
+++ b/libselinux/src/load_policy.c
@@ -315,7 +315,8 @@ hidden_def(selinux_mkload_policy)
*/
int selinux_init_load_policy(int *enforce)
{
- int rc = 0, orig_enforce = 0, seconfig = -2, secmdline = -1;
+ int rc = 0, orig_enforce = 0, seconfig = -2, secmdline = -1,
+ seautorelabel = -1;
If I'm reading the code correctly, seautorelabel only
gets set to 0,
so it might be nicer to do away with the -1 init value, and initialize
it to 0, and then maybe set it to 1 if autorelabel request is found.
The code will be slightly easier to read this way.
FILE *cfg;
char *buf;
@@ -332,6 +333,17 @@ int selinux_init_load_policy(int *enforce)
*/
selinux_getenforcemode(&seconfig);
+ /*
+ * If /.autorelabel exists then we should start in permissive
+ * mode because (a) the labels on the filesystem are known to
+ * be bogus and so should not be trusted to make security
+ * decisions, but more practically (b) mislabelled files may
+ * cause services & processes required for relabelling to fail.
+ */
+ if (access("/.autorelabel", F_OK) == 0) {
+ seautorelabel = 0;
+ }
+
/* Check for an override of the mode via the kernel command line. */
rc = mount("proc", "/proc", "proc", 0, 0);
cfg = fopen("/proc/cmdline", "r");
@@ -342,12 +354,18 @@ int selinux_init_load_policy(int *enforce)
fclose(cfg);
return -1;
}
- if (fgets(buf, selinux_page_size, cfg) &&
- (tmp = strstr(buf, "enforcing="))) {
- if (tmp == buf || isspace(*(tmp - 1))) {
+ if (fgets(buf, selinux_page_size, cfg)) {
+ if ((tmp = strstr(buf, "enforcing=")) &&
+ (tmp == buf || isspace(*(tmp - 1)))) {
secmdline =
atoi(tmp + sizeof("enforcing=") - 1);
}
+ else if ((tmp = strstr(buf, "autorelabel")) &&
+ (tmp == buf || isspace(*(tmp - 1))) &&
+ (tmp + sizeof("autorelabel") - 1 == '\0' ||
+ isspace(tmp + sizeof("autorelabel") - 1))) {
+ seautorelabel = 0;
+ }
}
fclose(cfg);
free(buf);
@@ -357,7 +375,9 @@ int selinux_init_load_policy(int *enforce)
* Determine the final desired mode.
* Command line argument takes precedence, then config file.
*/
- if (secmdline >= 0)
+ if (seautorelabel >= 0)
+ *enforce = seautorelabel;
+ else if (secmdline >= 0)
*enforce = secmdline;
else if (seconfig >= 0)
*enforce = seconfig;
--
2.7.4
From f1f5cf751c72b658bae08161934ffac13aee8c5f Mon Sep 17 00:00:00
2001
From: "Richard W.M. Jones" <rjones(a)redhat.com>
Date: Mon, 4 Jul 2016 13:15:08 +0100
Subject: [PATCH] Use generator approach to fix autorelabel.
See:
https://lists.fedoraproject.org/archives/list/devel@lists.fedoraproject.o...
---
policycoreutils.spec | 13 ++++++++++---
selinux-autorelabel-generator.sh | 29 +++++++++++++++++++++++++++++
selinux-autorelabel.service | 9 +++------
selinux-autorelabel.target | 7 +++++++
4 files changed, 49 insertions(+), 9 deletions(-)
create mode 100755 selinux-autorelabel-generator.sh
create mode 100644 selinux-autorelabel.target
diff --git a/policycoreutils.spec b/policycoreutils.spec
index a77c243..cb784d9 100644
--- a/policycoreutils.spec
+++ b/policycoreutils.spec
@@ -4,10 +4,12 @@
%global libselinuxver 2.5-7
%global sepolgenver 1.2.3
+%global generatorsdir %{_prefix}/lib/systemd/system-generators
+
Summary: SELinux policy core utilities
Name: policycoreutils
Version: 2.5
-Release: 11%{?dist}
+Release: 11.rwmj1%{?dist}
License: GPLv2
Group: System Environment/Base
#
https://github.com/SELinuxProject/selinux/wiki/Releases
@@ -20,6 +22,8 @@ Source4: sepolicy-icons.tgz
Source5: selinux-autorelabel
Source6: selinux-autorelabel.service
Source7: selinux-autorelabel-mark.service
+Source8: selinux-autorelabel.target
+Source9: selinux-autorelabel-generator.sh
# download
https://raw.githubusercontent.com/fedora-selinux/scripts/master/selinux/m...
# run:
# $ VERSION=2.5 ./make-fedora-selinux-patch.sh policycoreutils
@@ -123,10 +127,12 @@ rm -f
%{buildroot}%{_datadir}/system-config-selinux/system-config-selinux.deskto
#
https://bugzilla.redhat.com/show_bug.cgi?id=1328825
mkdir -m 755 -p %{buildroot}/%{_unitdir}/basic.target.wants/
+mkdir -m 755 -p %{buildroot}/%{generatorsdir}
install -m 644 -p %{SOURCE6} %{buildroot}/%{_unitdir}/
install -m 644 -p %{SOURCE7} %{buildroot}/%{_unitdir}/
+install -m 644 -p %{SOURCE8} %{buildroot}/%{_unitdir}/
+install -m 755 -p %{SOURCE9} %{buildroot}/%{generatorsdir}/
install -m 755 -p %{SOURCE5} %{buildroot}/%{_libexecdir}/selinux/
-ln -s ../selinux-autorelabel.service %{buildroot}/%{_unitdir}/basic.target.wants/
ln -s ../selinux-autorelabel-mark.service %{buildroot}/%{_unitdir}/basic.target.wants/
%find_lang %{name}
@@ -371,7 +377,8 @@ fi
%{_unitdir}/selinux-autorelabel-mark.service
%{_unitdir}/basic.target.wants/selinux-autorelabel-mark.service
%{_unitdir}/selinux-autorelabel.service
-%{_unitdir}/basic.target.wants/selinux-autorelabel.service
+%{_unitdir}/selinux-autorelabel.target
+%{generatorsdir}/selinux-autorelabel-generator.sh
%config(noreplace) %{_sysconfdir}/sestatus.conf
# selinux-policy Requires: policycoreutils, so we own this set of directories and our
files within them
%{_mandir}/man5/selinux_config.5.gz
diff --git a/selinux-autorelabel-generator.sh b/selinux-autorelabel-generator.sh
new file mode 100755
index 0000000..be60487
--- /dev/null
+++ b/selinux-autorelabel-generator.sh
@@ -0,0 +1,29 @@
+#!/bin/sh
+
+# This systemd.generator(7) detects if SELinux is running and if the
+# user requested an autorelabel, and if so sets the default target to
+# selinux-autorelabel.target, which will cause the filesystem to be
+# relabelled and then the system will reboot again and boot into the
+# real default target.
+
+PATH=/usr/sbin:$PATH
+unitdir=/usr/lib/systemd/system
+
+# If invoked with no arguments (for testing) write to /tmp.
+earlydir="/tmp"
+if [ -n "$2" ]; then
+ earlydir="$2"
+fi
+
+set_target ()
+{
+ ln -sf "$unitdir/selinux-autorelabel.target"
"$earlydir/default.target"
+}
+
+if selinuxenabled; then
+ if test -f /.autorelabel; then
+ set_target
+ elif grep -sqE "\bautorelabel\b" /proc/cmdline; then
+ set_target
+ fi
+fi
I think it's reasonable to start with a bash generator. Once things
settle down it could be useful to rewrite it in C to avoid the
dependency on grep, and a bunch of forks in early boot.
The generator should remove /.autorelabel as the first step.
Doing it as early as possible reduces the risk of a reboot loop.
I still see a potential for confusion, if "/.autorelabel" or
"autorelabel"
are found, but selinux is turned off. In that case the generator would
redirect default.target, but selinux-autorelabel.target might not
run when ConditionSecurity=selinux is not matched. I don't think
ConditionSecurity=selinux is useful: the target should only be invoked
by the generator, so it's probably better to remove the condition
to avoid the potential for mismatch between the generator and what
systemd thinks about selinux.
diff --git a/selinux-autorelabel.service
b/selinux-autorelabel.service
index a6cc332..b8461e6 100644
--- a/selinux-autorelabel.service
+++ b/selinux-autorelabel.service
@@ -1,13 +1,10 @@
[Unit]
-Description=Relabel all filesystems, if necessary
+Description=Relabel all filesystems
DefaultDependencies=no
-Requires=local-fs.target
Conflicts=shutdown.target
-After=local-fs.target
-Before=sysinit.target shutdown.target
+After=sysinit.target
+Before=shutdown.target
ConditionSecurity=selinux
-ConditionKernelCommandLine=|autorelabel
-ConditionPathExists=|/.autorelabel
[Service]
ExecStart=/usr/libexec/selinux/selinux-autorelabel
diff --git a/selinux-autorelabel.target b/selinux-autorelabel.target
new file mode 100644
index 0000000..a4f63ab
--- /dev/null
+++ b/selinux-autorelabel.target
@@ -0,0 +1,7 @@
+[Unit]
+Description=Relabel all filesystems and reboot
+DefaultDependencies=no
+Requires=sysinit.target selinux-autorelabel.service
+Conflicts=shutdown.target
+After=sysinit.target selinux-autorelabel.service
+ConditionSecurity=selinux
--
2.7.4