[PATCH] libselinux: If autorelabel, force permissive mode.

[PATCH] libselinux: If autorelabel, force permissive mode.

[Richard W.M. Jones]

The autorelabel feature has been broken in Fedora for a while.
virt-builder relies on this feature to enable SELinux in guests since
we are unable to set filesystem labels when generating the image.  So
it comes down to me to try to fix this.  There was a discussion on the
Fedora development list which explains the background and the reasons
why autorelabel is broken:

http://thread.gmane.org/gmane.linux.redhat.fedora.devel/220453

The plan to fix autorelabel (also formulated in the above thread) is
in two parts:

(1) [This patch] If the autorelabel condition is detected when loading
policy very early during boot, we set SELinux to permissive mode
(overriding the contents of /etc/selinux/config and the command line).

(2) We install a systemd "generator".  If the autorelabel condition is
detected, then the generator redirects the default target to a new,
very minimal selinux-autorelabel.target.  This will relabel the
filesystem, remove /.autorelabel and reboot.  After the reboot the
system will boot normally, with correct filesystem labels and of
course with SELinux enabled.

During relabelling (unlike currently) only a very minimal set of
services are enabled, just enough to be able to mount the filesystem.
This should ensure there is no danger from having SELinux permissive
while relabelling.

This patch is actually against the fedora-selinux.git tree, although
it probably applies upstream too.

Rich.

[PATCH] libselinux: If autorelabel, force permissive mode.

[Richard W.M. Jones]

Signed-off-by: Richard W.M. Jones <rjones@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;
 	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

Re: [PATCH] libselinux: If autorelabel, force permissive mode.

[Jason Zaman]

On Wed, Jul 06, 2016 at 10:43:21AM +0100, Richard W.M. Jones wrote:
> The autorelabel feature has been broken in Fedora for a while.
> virt-builder relies on this feature to enable SELinux in guests since
> we are unable to set filesystem labels when generating the image.  So
> it comes down to me to try to fix this.  There was a discussion on the
> Fedora development list which explains the background and the reasons
> why autorelabel is broken:
> 
> http://thread.gmane.org/gmane.linux.redhat.fedora.devel/220453
> 
> The plan to fix autorelabel (also formulated in the above thread) is
> in two parts:
> 
> (1) [This patch] If the autorelabel condition is detected when loading
> policy very early during boot, we set SELinux to permissive mode
> (overriding the contents of /etc/selinux/config and the command line).

Can't you just set enforcing=0 on the kernel commandline for the first
boot?

This patch sounds a bit dangerous. If anyone can touch /.autorelabel
later on the machine will be in permissive mode on next reboot. Also we
dont use /.autorelabel in gentoo (not sure about arch or debian) so
something like this would just make the machine always in permissive
mode since there is no service to delete the file.

There are also many times other than first install when you would want
to relabel, if /home is moved to another HDD then you might want to
relabel but the machine would be completely fine in enforcing mode.

This sounds like you want permissive for *only* the very first boot and
never again but the way of doing it leaves that open. At the very least
I'd make it check for (/.autorelabel && (security.selinux xattr is
completely missing on /)) so it will probably not happen after the first
boot, but that still makes me a little uneasy. Having too many ways to
put the machine into permissive means it's easy to miss one later.

In gentoo the default install has /etc/selinux/config set to permissive
so the user sets enforcing later once they have relabelled, that might
be another option, but kernel commandline for first boot might still be
safer.

> (2) We install a systemd "generator".  If the autorelabel condition is
> detected, then the generator redirects the default target to a new,
> very minimal selinux-autorelabel.target.  This will relabel the
> filesystem, remove /.autorelabel and reboot.  After the reboot the
> system will boot normally, with correct filesystem labels and of
> course with SELinux enabled.
> 
> During relabelling (unlike currently) only a very minimal set of
> services are enabled, just enough to be able to mount the filesystem.
> This should ensure there is no danger from having SELinux permissive
> while relabelling.

This sounds good. if the labels are wrong all the daemons that start
will be wrong anyway so rebooting is basically always required.

-- Jason

> This patch is actually against the fedora-selinux.git tree, although
> it probably applies upstream too.
> 
> Rich.
> 
> _______________________________________________
> Selinux mailing list
> Selinux@tycho.nsa.gov
> To unsubscribe, send email to Selinux-leave@tycho.nsa.gov.
> To get help, send an email containing "help" to Selinux-request@tycho.nsa.gov.

Re: [PATCH] libselinux: If autorelabel, force permissive mode.

[Richard W.M. Jones]

On Wed, Jul 06, 2016 at 07:29:42PM +0800, Jason Zaman wrote:
> On Wed, Jul 06, 2016 at 10:43:21AM +0100, Richard W.M. Jones wrote:
> > The autorelabel feature has been broken in Fedora for a while.
> > virt-builder relies on this feature to enable SELinux in guests since
> > we are unable to set filesystem labels when generating the image.  So
> > it comes down to me to try to fix this.  There was a discussion on the
> > Fedora development list which explains the background and the reasons
> > why autorelabel is broken:
> > 
> > http://thread.gmane.org/gmane.linux.redhat.fedora.devel/220453
> > 
> > The plan to fix autorelabel (also formulated in the above thread) is
> > in two parts:
> > 
> > (1) [This patch] If the autorelabel condition is detected when loading
> > policy very early during boot, we set SELinux to permissive mode
> > (overriding the contents of /etc/selinux/config and the command line).
> 
> Can't you just set enforcing=0 on the kernel commandline for the first
> boot?

For virt-builder, no.  We create automated images that must
boot themselves without manual intervention.

However even ignoring my use case, I don't think it's a good idea
anyway.  If /.autorelabel is set, then the labels on the filesystem
are known to be wrong.  You shouldn't be making enforcement decisions
based on labels which you know are wrong.

> This patch sounds a bit dangerous. If anyone can touch /.autorelabel
> later on the machine will be in permissive mode on next reboot.

If someone can create files in / then I suspect you've got other
problems.  In any case it will then immediately jump to the generator
and relabel in a (very) minimal environment and reboot.  After this
second boot it'll be back to enforcing (or whatever is requested by
/etc/selinux/config).

> Also we dont use /.autorelabel in gentoo (not sure about arch or
> debian) so something like this would just make the machine always in
> permissive mode since there is no service to delete the file.

As I noted, I made this patch against the downstream fedora-selinux
git.  That repository says to send patches here.

> There are also many times other than first install when you would want
> to relabel, if /home is moved to another HDD then you might want to
> relabel but the machine would be completely fine in enforcing mode.
> 
> This sounds like you want permissive for *only* the very first boot and
> never again but the way of doing it leaves that open. At the very least
> I'd make it check for (/.autorelabel && (security.selinux xattr is
> completely missing on /))

The way autorelabel is documented to work, users can set it any time,
not just on first boot.  The labels can be wrong, not unset.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
libguestfs lets you edit virtual machines.  Supports shell scripting,
bindings from many languages.  http://libguestfs.org

Re: [PATCH] libselinux: If autorelabel, force permissive mode.

[Sven Vermeulen]

On Wed, Jul 6, 2016 at 2:12 PM, Richard W.M. Jones <rjones@redhat.com> wrote:
>> > The autorelabel feature has been broken in Fedora for a while.
>> > virt-builder relies on this feature to enable SELinux in guests since
>> > we are unable to set filesystem labels when generating the image.  So
>> > it comes down to me to try to fix this.  There was a discussion on the
>> > Fedora development list which explains the background and the reasons
>> > why autorelabel is broken:
>> >
>> > http://thread.gmane.org/gmane.linux.redhat.fedora.devel/220453
>> >
>> > The plan to fix autorelabel (also formulated in the above thread) is
>> > in two parts:
>> >
>> > (1) [This patch] If the autorelabel condition is detected when loading
>> > policy very early during boot, we set SELinux to permissive mode
>> > (overriding the contents of /etc/selinux/config and the command line).
>>
>> Can't you just set enforcing=0 on the kernel commandline for the first
>> boot?
>
> For virt-builder, no.  We create automated images that must
> boot themselves without manual intervention.
>
> However even ignoring my use case, I don't think it's a good idea
> anyway.  If /.autorelabel is set, then the labels on the filesystem
> are known to be wrong.  You shouldn't be making enforcement decisions
> based on labels which you know are wrong.

I agree with the sentiment, but not with its execution. You can't
guarantee that there is a systemd generator or other process in place
that will pick this situation up, run the setfiles (or equivalent)
service, remove the touch file and reboot. If you could, then that
would be awesome.

Let's say there is a platform that does not support /.autorelabel (not
sure how SEAndroid works, but on Gentoo we don't look at that file).
This patch would place the system in permissive mode, but just
continue regardless.

>> This patch sounds a bit dangerous. If anyone can touch /.autorelabel
>> later on the machine will be in permissive mode on next reboot.
>
> If someone can create files in / then I suspect you've got other
> problems.  In any case it will then immediately jump to the generator
> and relabel in a (very) minimal environment and reboot.  After this
> second boot it'll be back to enforcing (or whatever is requested by
> /etc/selinux/config).

SELinux is quite capable of allowing people to create files in / while
still not being able to do much else on the system. And the generator
step is only on a set of platforms, not all of those supporting
SELinux.

Wouldn't it be possible to reverse the logic? Introduce a
"checkautorelabel" kernel/boot option, and if that option is set, then
activate this logic? That way, other platforms beyond Fedora can
happily continue regardless, and you can by default enable the
checkautorelabel boot option.

Wkr,
  Sven Vermeulen

Re: [PATCH] libselinux: If autorelabel, force permissive mode.

[William Roberts]

[-- Attachment #1: Type: text/plain, Size: 3612 bytes --]

On Jul 7, 2016 08:40, "Sven Vermeulen" <sven.j.vermeulen@gmail.com> wrote:
>
> On Wed, Jul 6, 2016 at 2:12 PM, Richard W.M. Jones <rjones@redhat.com>
wrote:
> >> > The autorelabel feature has been broken in Fedora for a while.
> >> > virt-builder relies on this feature to enable SELinux in guests since
> >> > we are unable to set filesystem labels when generating the image.  So
> >> > it comes down to me to try to fix this.  There was a discussion on
the
> >> > Fedora development list which explains the background and the reasons
> >> > why autorelabel is broken:
> >> >
> >> > http://thread.gmane.org/gmane.linux.redhat.fedora.devel/220453
> >> >
> >> > The plan to fix autorelabel (also formulated in the above thread) is
> >> > in two parts:
> >> >
> >> > (1) [This patch] If the autorelabel condition is detected when
loading
> >> > policy very early during boot, we set SELinux to permissive mode
> >> > (overriding the contents of /etc/selinux/config and the command
line).
> >>
> >> Can't you just set enforcing=0 on the kernel commandline for the first
> >> boot?
> >
> > For virt-builder, no.  We create automated images that must
> > boot themselves without manual intervention.
> >
> > However even ignoring my use case, I don't think it's a good idea
> > anyway.  If /.autorelabel is set, then the labels on the filesystem
> > are known to be wrong.  You shouldn't be making enforcement decisions
> > based on labels which you know are wrong.
>
> I agree with the sentiment, but not with its execution. You can't
> guarantee that there is a systemd generator or other process in place
> that will pick this situation up, run the setfiles (or equivalent)
> service, remove the touch file and reboot. If you could, then that
> would be awesome.
>
> Let's say there is a platform that does not support /.autorelabel (not
> sure how SEAndroid works, but on Gentoo we don't look at that file).

Android uses a readonly ramdisk so even if it was remounted and written to,
the file would be gone on reboot. However, some newer Android stuff is
moving to an ext4 root filesystem. I'm a bit leary of this change as well,
although in Android if you write to the rootfs we've likely already lost,
best not to be adding functionality that could be used against us.

> This patch would place the system in permissive mode, but just
> continue regardless.
>
> >> This patch sounds a bit dangerous. If anyone can touch /.autorelabel
> >> later on the machine will be in permissive mode on next reboot.
> >
> > If someone can create files in / then I suspect you've got other
> > problems.  In any case it will then immediately jump to the generator
> > and relabel in a (very) minimal environment and reboot.  After this
> > second boot it'll be back to enforcing (or whatever is requested by
> > /etc/selinux/config).
>
> SELinux is quite capable of allowing people to create files in / while
> still not being able to do much else on the system. And the generator
> step is only on a set of platforms, not all of those supporting
> SELinux.
>
> Wouldn't it be possible to reverse the logic? Introduce a
> "checkautorelabel" kernel/boot option, and if that option is set, then
> activate this logic? That way, other platforms beyond Fedora can
> happily continue regardless, and you can by default enable the
> checkautorelabel boot option.
>
> Wkr,
>   Sven Vermeulen
> _______________________________________________
> Selinux mailing list
> Selinux@tycho.nsa.gov
> To unsubscribe, send email to Selinux-leave@tycho.nsa.gov.
> To get help, send an email containing "help" to
Selinux-request@tycho.nsa.gov.

[-- Attachment #2: Type: text/html, Size: 4819 bytes --]

Re: [PATCH] libselinux: If autorelabel, force permissive mode.

[Jason Zaman]

On Thu, Jul 07, 2016 at 08:43:45AM -0400, William Roberts wrote:
> On Jul 7, 2016 08:40, "Sven Vermeulen" <sven.j.vermeulen@gmail.com> wrote:
> >
> > On Wed, Jul 6, 2016 at 2:12 PM, Richard W.M. Jones <rjones@redhat.com>
> wrote:
> > >> > The autorelabel feature has been broken in Fedora for a while.
> > >> > virt-builder relies on this feature to enable SELinux in guests since
> > >> > we are unable to set filesystem labels when generating the image.  So
> > >> > it comes down to me to try to fix this.  There was a discussion on
> the
> > >> > Fedora development list which explains the background and the reasons
> > >> > why autorelabel is broken:
> > >> >
> > >> > http://thread.gmane.org/gmane.linux.redhat.fedora.devel/220453
> > >> >
> > >> > The plan to fix autorelabel (also formulated in the above thread) is
> > >> > in two parts:
> > >> >
> > >> > (1) [This patch] If the autorelabel condition is detected when
> loading
> > >> > policy very early during boot, we set SELinux to permissive mode
> > >> > (overriding the contents of /etc/selinux/config and the command
> line).
> > >>
> > >> Can't you just set enforcing=0 on the kernel commandline for the first
> > >> boot?
> > >
> > > For virt-builder, no.  We create automated images that must
> > > boot themselves without manual intervention.
> > >
> > > However even ignoring my use case, I don't think it's a good idea
> > > anyway.  If /.autorelabel is set, then the labels on the filesystem
> > > are known to be wrong.  You shouldn't be making enforcement decisions
> > > based on labels which you know are wrong.
> >
> > I agree with the sentiment, but not with its execution. You can't
> > guarantee that there is a systemd generator or other process in place
> > that will pick this situation up, run the setfiles (or equivalent)
> > service, remove the touch file and reboot. If you could, then that
> > would be awesome.
> >
> > Let's say there is a platform that does not support /.autorelabel (not
> > sure how SEAndroid works, but on Gentoo we don't look at that file).
> 
> Android uses a readonly ramdisk so even if it was remounted and written to,
> the file would be gone on reboot. However, some newer Android stuff is
> moving to an ext4 root filesystem. I'm a bit leary of this change as well,
> although in Android if you write to the rootfs we've likely already lost,
> best not to be adding functionality that could be used against us.

Alternatively, if someone edits the boot image to have .autorelabel it
will always come back. I definitely agree with not adding functionality
that can be used against us. Currently a user has to check
/etc/selinux/config and the kernel commandline, adding more ways means
more things the user can miss.

Doesn't Android set the labels on the /system disk image during build?
Maybe virt-builder can copy that? This would also speed up initial
deployment of new images.

What steps are required during a default install in RHEL? does an
install from a livecd without virt-builder also have this relabelling
problem?

One way I can think of is have a transition from kernel_t (or whatever
the context would be on a completely unlabelled system) to a domain with
perms to relabel everything. Since the labels would be missing pid1
would have to runcon -t autorelabel_t ... but it seems the safer road
than making absolutely everything permissive.

Alternatively, is there a reason /etc/selinux/config shouldn't be set to
permissive by default in the image? What do we gain with this extra way?
If the user is going to autorelabel after install, they are already
probably setting permissive in the config before they reboot too.

-- Jason

> > This patch would place the system in permissive mode, but just
> > continue regardless.
> >
> > >> This patch sounds a bit dangerous. If anyone can touch /.autorelabel
> > >> later on the machine will be in permissive mode on next reboot.
> > >
> > > If someone can create files in / then I suspect you've got other
> > > problems.  In any case it will then immediately jump to the generator
> > > and relabel in a (very) minimal environment and reboot.  After this
> > > second boot it'll be back to enforcing (or whatever is requested by
> > > /etc/selinux/config).
> >
> > SELinux is quite capable of allowing people to create files in / while
> > still not being able to do much else on the system. And the generator
> > step is only on a set of platforms, not all of those supporting
> > SELinux.
> >
> > Wouldn't it be possible to reverse the logic? Introduce a
> > "checkautorelabel" kernel/boot option, and if that option is set, then
> > activate this logic? That way, other platforms beyond Fedora can
> > happily continue regardless, and you can by default enable the
> > checkautorelabel boot option.
> >
> > Wkr,
> >   Sven Vermeulen
> > _______________________________________________
> > Selinux mailing list
> > Selinux@tycho.nsa.gov
> > To unsubscribe, send email to Selinux-leave@tycho.nsa.gov.
> > To get help, send an email containing "help" to
> Selinux-request@tycho.nsa.gov.

> _______________________________________________
> Selinux mailing list
> Selinux@tycho.nsa.gov
> To unsubscribe, send email to Selinux-leave@tycho.nsa.gov.
> To get help, send an email containing "help" to Selinux-request@tycho.nsa.gov.

Re: [PATCH] libselinux: If autorelabel, force permissive mode.

[William Roberts]

[-- Attachment #1: Type: text/plain, Size: 5667 bytes --]

On Jul 7, 2016 09:50, "Jason Zaman" <jason@perfinion.com> wrote:
>
> On Thu, Jul 07, 2016 at 08:43:45AM -0400, William Roberts wrote:
> > On Jul 7, 2016 08:40, "Sven Vermeulen" <sven.j.vermeulen@gmail.com>
wrote:
> > >
> > > On Wed, Jul 6, 2016 at 2:12 PM, Richard W.M. Jones <rjones@redhat.com>
> > wrote:
> > > >> > The autorelabel feature has been broken in Fedora for a while.
> > > >> > virt-builder relies on this feature to enable SELinux in guests
since
> > > >> > we are unable to set filesystem labels when generating the
image.  So
> > > >> > it comes down to me to try to fix this.  There was a discussion
on
> > the
> > > >> > Fedora development list which explains the background and the
reasons
> > > >> > why autorelabel is broken:
> > > >> >
> > > >> > http://thread.gmane.org/gmane.linux.redhat.fedora.devel/220453
> > > >> >
> > > >> > The plan to fix autorelabel (also formulated in the above
thread) is
> > > >> > in two parts:
> > > >> >
> > > >> > (1) [This patch] If the autorelabel condition is detected when
> > loading
> > > >> > policy very early during boot, we set SELinux to permissive mode
> > > >> > (overriding the contents of /etc/selinux/config and the command
> > line).
> > > >>
> > > >> Can't you just set enforcing=0 on the kernel commandline for the
first
> > > >> boot?
> > > >
> > > > For virt-builder, no.  We create automated images that must
> > > > boot themselves without manual intervention.
> > > >
> > > > However even ignoring my use case, I don't think it's a good idea
> > > > anyway.  If /.autorelabel is set, then the labels on the filesystem
> > > > are known to be wrong.  You shouldn't be making enforcement
decisions
> > > > based on labels which you know are wrong.
> > >
> > > I agree with the sentiment, but not with its execution. You can't
> > > guarantee that there is a systemd generator or other process in place
> > > that will pick this situation up, run the setfiles (or equivalent)
> > > service, remove the touch file and reboot. If you could, then that
> > > would be awesome.
> > >
> > > Let's say there is a platform that does not support /.autorelabel (not
> > > sure how SEAndroid works, but on Gentoo we don't look at that file).
> >
> > Android uses a readonly ramdisk so even if it was remounted and written
to,
> > the file would be gone on reboot. However, some newer Android stuff is
> > moving to an ext4 root filesystem. I'm a bit leary of this change as
well,
> > although in Android if you write to the rootfs we've likely already
lost,
> > best not to be adding functionality that could be used against us.
>
> Alternatively, if someone edits the boot image to have .autorelabel it
> will always come back. I definitely agree with not adding functionality

True, I didn't even think about that case.

> that can be used against us. Currently a user has to check
> /etc/selinux/config and the kernel commandline, adding more ways means
> more things the user can miss.
>
> Doesn't Android set the labels on the /system disk image during build?

Yes that is correct.

> Maybe virt-builder can copy that? This would also speed up initial
> deployment of new images.
>
> What steps are required during a default install in RHEL? does an
> install from a livecd without virt-builder also have this relabelling
> problem?
>
> One way I can think of is have a transition from kernel_t (or whatever
> the context would be on a completely unlabelled system) to a domain with
> perms to relabel everything. Since the labels would be missing pid1
> would have to runcon -t autorelabel_t ... but it seems the safer road
> than making absolutely everything permissive.
>
> Alternatively, is there a reason /etc/selinux/config shouldn't be set to
> permissive by default in the image? What do we gain with this extra way?
> If the user is going to autorelabel after install, they are already
> probably setting permissive in the config before they reboot too.
>
> -- Jason
>
> > > This patch would place the system in permissive mode, but just
> > > continue regardless.
> > >
> > > >> This patch sounds a bit dangerous. If anyone can touch
/.autorelabel
> > > >> later on the machine will be in permissive mode on next reboot.
> > > >
> > > > If someone can create files in / then I suspect you've got other
> > > > problems.  In any case it will then immediately jump to the
generator
> > > > and relabel in a (very) minimal environment and reboot.  After this
> > > > second boot it'll be back to enforcing (or whatever is requested by
> > > > /etc/selinux/config).
> > >
> > > SELinux is quite capable of allowing people to create files in / while
> > > still not being able to do much else on the system. And the generator
> > > step is only on a set of platforms, not all of those supporting
> > > SELinux.
> > >
> > > Wouldn't it be possible to reverse the logic? Introduce a
> > > "checkautorelabel" kernel/boot option, and if that option is set, then
> > > activate this logic? That way, other platforms beyond Fedora can
> > > happily continue regardless, and you can by default enable the
> > > checkautorelabel boot option.
> > >
> > > Wkr,
> > >   Sven Vermeulen
> > > _______________________________________________
> > > Selinux mailing list
> > > Selinux@tycho.nsa.gov
> > > To unsubscribe, send email to Selinux-leave@tycho.nsa.gov.
> > > To get help, send an email containing "help" to
> > Selinux-request@tycho.nsa.gov.
>
> > _______________________________________________
> > Selinux mailing list
> > Selinux@tycho.nsa.gov
> > To unsubscribe, send email to Selinux-leave@tycho.nsa.gov.
> > To get help, send an email containing "help" to
Selinux-request@tycho.nsa.gov.
>

[-- Attachment #2: Type: text/html, Size: 7971 bytes --]

Re: [PATCH] libselinux: If autorelabel, force permissive mode.

[Richard W.M. Jones]

On Thu, Jul 07, 2016 at 09:50:17PM +0800, Jason Zaman wrote:
> Doesn't Android set the labels on the /system disk image during build?
> Maybe virt-builder can copy that? This would also speed up initial
> deployment of new images.

Well this is the real problem.  Because the guest policy is a binary
blob, and because the binary blobs are not (necessarily) compatible
across kernel versions, we cannot just load the policy blob of the
guest into our kernel, so we cannot label guests properly.  Sure be
nice if policy wasn't stored in this way.

> What steps are required during a default install in RHEL? does an
> install from a livecd without virt-builder also have this relabelling
> problem?

During a live CD install the live CD runs its own kernel.  That's just
not the way virt-builder works.  Also virt-builder customizes a
template, it doesn't build a whole VM from scratch (because that would
be orders of magnitude slower), so we start with whatever labels are
in the base template.

> One way I can think of is have a transition from kernel_t (or whatever
> the context would be on a completely unlabelled system) to a domain with
> perms to relabel everything. Since the labels would be missing pid1
> would have to runcon -t autorelabel_t ... but it seems the safer road
> than making absolutely everything permissive.

Note that /.autorelabel can be used at any time.  The system is not
necessarily unlabelled, it is wrongly labelled.

> Alternatively, is there a reason /etc/selinux/config shouldn't be set to
> permissive by default in the image? What do we gain with this extra way?
> If the user is going to autorelabel after install, they are already
> probably setting permissive in the config before they reboot too.

As documented, the user can simply touch /.autorelabel and reboot to
autorelabel the system.  They don't have to edit /etc/selinux/config
(and if they did, what would set it back to enforcing, and how would
that thing know what to set it back to?)

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

Re: [PATCH] libselinux: If autorelabel, force permissive mode.

[Russell Coker]

On Fri, 8 Jul 2016 06:56:14 AM Richard W.M. Jones wrote:
> On Thu, Jul 07, 2016 at 09:50:17PM +0800, Jason Zaman wrote:
> > Doesn't Android set the labels on the /system disk image during build?
> > Maybe virt-builder can copy that? This would also speed up initial
> > deployment of new images.
> 
> Well this is the real problem.  Because the guest policy is a binary
> blob, and because the binary blobs are not (necessarily) compatible
> across kernel versions, we cannot just load the policy blob of the
> guest into our kernel, so we cannot label guests properly.  Sure be
> nice if policy wasn't stored in this way.

While virt-builder is one case that needs special attention the important fact 
is that autorelabel has never worked as well as it should have.  There has 
never been a guarantee that an autorelabel operation would complete 
successfully in the face of the wrong combination of mislabeled files.  It 
might be a good idea to copy the Android build process to virt-builder for 
other reasons but even so the design Richard proposes is worth having 
regardless.

Also the way that the reboot was managed was never that great, especially on a 
systemd system.  I can't recall how much of that is my responsibility.

I'd like to get this in Debian.

-- 
My Main Blog         http://etbe.coker.com.au/
My Documents Blog    http://doc.coker.com.au/

Re: [PATCH] libselinux: If autorelabel, force permissive mode.

[Stephen Smalley]

On 07/07/2016 04:56 PM, Richard W.M. Jones wrote:
> On Thu, Jul 07, 2016 at 09:50:17PM +0800, Jason Zaman wrote:
>> Doesn't Android set the labels on the /system disk image during build?
>> Maybe virt-builder can copy that? This would also speed up initial
>> deployment of new images.
> 
> Well this is the real problem.  Because the guest policy is a binary
> blob, and because the binary blobs are not (necessarily) compatible
> across kernel versions, we cannot just load the policy blob of the
> guest into our kernel, so we cannot label guests properly.  Sure be
> nice if policy wasn't stored in this way.

Just to clarify, it is not necessary to load the guest policy into the
host kernel in order to set labels on the guest filesystem.  SELinux
long ago introduced support for setting foreign/unknown labels on files
by processes with the appropriate permissions, and that mechanism was
used by livecd creator IIRC - it was also intended for use by rpm for
labeling files before the corresponding policy module was installed but
they never took advantage of it.

The other approach would be to follow what we did in Android, i.e.
extend the filesystem generation tools to look up the appropriate
context and set the xattr when generating the image files.  Similar
support was also recently added to the OpenEmbedded tools for labeling
those images.

Re: [PATCH] libselinux: If autorelabel, force permissive mode.

[Richard W.M. Jones]

On Tue, Jul 12, 2016 at 01:22:55PM -0400, Stephen Smalley wrote:
> On 07/07/2016 04:56 PM, Richard W.M. Jones wrote:
> > On Thu, Jul 07, 2016 at 09:50:17PM +0800, Jason Zaman wrote:
> >> Doesn't Android set the labels on the /system disk image during build?
> >> Maybe virt-builder can copy that? This would also speed up initial
> >> deployment of new images.
> > 
> > Well this is the real problem.  Because the guest policy is a binary
> > blob, and because the binary blobs are not (necessarily) compatible
> > across kernel versions, we cannot just load the policy blob of the
> > guest into our kernel, so we cannot label guests properly.  Sure be
> > nice if policy wasn't stored in this way.
> 
> Just to clarify, it is not necessary to load the guest policy into the
> host kernel in order to set labels on the guest filesystem.  SELinux
> long ago introduced support for setting foreign/unknown labels on files
> by processes with the appropriate permissions, and that mechanism was
> used by livecd creator IIRC - it was also intended for use by rpm for
> labeling files before the corresponding policy module was installed but
> they never took advantage of it.

IME you cannot set any label unless SELinux is enabled in the
appliance kernel, but even assuming this is really possible, how do
you know what label should you set?  Really we just want to do
"restorecon -R /" but that has proven to be impossible.

> The other approach would be to follow what we did in Android, i.e.
> extend the filesystem generation tools to look up the appropriate
> context and set the xattr when generating the image files.  Similar
> support was also recently added to the OpenEmbedded tools for labeling
> those images.

Look up how?

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-df lists disk usage of guests without needing to install any
software inside the virtual machine.  Supports Linux and Windows.
http://people.redhat.com/~rjones/virt-df/

Re: [PATCH] libselinux: If autorelabel, force permissive mode.

[Stephen Smalley]

On 07/12/2016 02:01 PM, Richard W.M. Jones wrote:
> On Tue, Jul 12, 2016 at 01:22:55PM -0400, Stephen Smalley wrote:
>> On 07/07/2016 04:56 PM, Richard W.M. Jones wrote:
>>> On Thu, Jul 07, 2016 at 09:50:17PM +0800, Jason Zaman wrote:
>>>> Doesn't Android set the labels on the /system disk image during build?
>>>> Maybe virt-builder can copy that? This would also speed up initial
>>>> deployment of new images.
>>>
>>> Well this is the real problem.  Because the guest policy is a binary
>>> blob, and because the binary blobs are not (necessarily) compatible
>>> across kernel versions, we cannot just load the policy blob of the
>>> guest into our kernel, so we cannot label guests properly.  Sure be
>>> nice if policy wasn't stored in this way.
>>
>> Just to clarify, it is not necessary to load the guest policy into the
>> host kernel in order to set labels on the guest filesystem.  SELinux
>> long ago introduced support for setting foreign/unknown labels on files
>> by processes with the appropriate permissions, and that mechanism was
>> used by livecd creator IIRC - it was also intended for use by rpm for
>> labeling files before the corresponding policy module was installed but
>> they never took advantage of it.
> 
> IME you cannot set any label unless SELinux is enabled in the
> appliance kernel, but even assuming this is really possible, how do
> you know what label should you set?  Really we just want to do
> "restorecon -R /" but that has proven to be impossible.

Hmm...the kernel certainly supports setting labels as long as the
filesystem xattr support is enabled, and setfiles used to work even if
SELinux is disabled, but admittedly we don't test on SELinux-disabled
very often.

For SELinux-enabled, something like:
runcon -t setfiles_mac_t -- chroot /mnt /sbin/setfiles -v -F -e /proc -e
/sys -e /dev -e /selinux
/etc/selinux/targeted/contexts/files/file_contexts /
has been reported to work in the past.  The process needs CAP_MAC_ADMIN
in its effective capability set and it needs to be in a domain that is
allowed mac_admin by policy (hence the runcon -t setfiles_mac_t above).

IIRC, they had to add a local policy module to allow setfiles_mac_t to
be entered via chroot rather than via setfiles.

> 
>> The other approach would be to follow what we did in Android, i.e.
>> extend the filesystem generation tools to look up the appropriate
>> context and set the xattr when generating the image files.  Similar
>> support was also recently added to the OpenEmbedded tools for labeling
>> those images.
> 
> Look up how?

In the Android case, we instrumented the ext4fs tools to use the
libselinux selabel_lookup interface for looking up the right context to
assign to a given file; see ext4_utils in
https://android.googlesource.com/platform/system/extras.  In the
OpenEmbedded case, I think they run setfiles on the directory using
pseudo to store the attributes (so the host OS never sees them), then
have their filesystem generation tools fetch the xattrs again using
pseudo and set them in the image during image generation.

Re: [PATCH] libselinux: If autorelabel, force permissive mode.

[Richard W.M. Jones]

On Tue, Jul 12, 2016 at 02:25:41PM -0400, Stephen Smalley wrote:
> On 07/12/2016 02:01 PM, Richard W.M. Jones wrote:
> > On Tue, Jul 12, 2016 at 01:22:55PM -0400, Stephen Smalley wrote:
> >> On 07/07/2016 04:56 PM, Richard W.M. Jones wrote:
> >>> On Thu, Jul 07, 2016 at 09:50:17PM +0800, Jason Zaman wrote:
> >>>> Doesn't Android set the labels on the /system disk image during build?
> >>>> Maybe virt-builder can copy that? This would also speed up initial
> >>>> deployment of new images.
> >>>
> >>> Well this is the real problem.  Because the guest policy is a binary
> >>> blob, and because the binary blobs are not (necessarily) compatible
> >>> across kernel versions, we cannot just load the policy blob of the
> >>> guest into our kernel, so we cannot label guests properly.  Sure be
> >>> nice if policy wasn't stored in this way.
> >>
> >> Just to clarify, it is not necessary to load the guest policy into the
> >> host kernel in order to set labels on the guest filesystem.  SELinux
> >> long ago introduced support for setting foreign/unknown labels on files
> >> by processes with the appropriate permissions, and that mechanism was
> >> used by livecd creator IIRC - it was also intended for use by rpm for
> >> labeling files before the corresponding policy module was installed but
> >> they never took advantage of it.
> > 
> > IME you cannot set any label unless SELinux is enabled in the
> > appliance kernel, but even assuming this is really possible, how do
> > you know what label should you set?  Really we just want to do
> > "restorecon -R /" but that has proven to be impossible.
> 
> Hmm...the kernel certainly supports setting labels as long as the
> filesystem xattr support is enabled, and setfiles used to work even if
> SELinux is disabled, but admittedly we don't test on SELinux-disabled
> very often.
> 
> For SELinux-enabled, something like:
> runcon -t setfiles_mac_t -- chroot /mnt /sbin/setfiles -v -F -e /proc -e
> /sys -e /dev -e /selinux
> /etc/selinux/targeted/contexts/files/file_contexts /
> has been reported to work in the past.  The process needs CAP_MAC_ADMIN
> in its effective capability set and it needs to be in a domain that is
> allowed mac_admin by policy (hence the runcon -t setfiles_mac_t above).

Thanks - can confirm this works even with SELinux disabled in the
appliance kernel.  I think this is the approach we will take, and it
also means we don't need /.autorelabel to be fixed now.

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

Re: [PATCH] libselinux: If autorelabel, force permissive mode.

[Stephen Smalley]

On 07/13/2016 03:31 PM, Richard W.M. Jones wrote:
> On Tue, Jul 12, 2016 at 02:25:41PM -0400, Stephen Smalley wrote:
>> On 07/12/2016 02:01 PM, Richard W.M. Jones wrote:
>>> On Tue, Jul 12, 2016 at 01:22:55PM -0400, Stephen Smalley wrote:
>>>> On 07/07/2016 04:56 PM, Richard W.M. Jones wrote:
>>>>> On Thu, Jul 07, 2016 at 09:50:17PM +0800, Jason Zaman wrote:
>>>>>> Doesn't Android set the labels on the /system disk image during build?
>>>>>> Maybe virt-builder can copy that? This would also speed up initial
>>>>>> deployment of new images.
>>>>>
>>>>> Well this is the real problem.  Because the guest policy is a binary
>>>>> blob, and because the binary blobs are not (necessarily) compatible
>>>>> across kernel versions, we cannot just load the policy blob of the
>>>>> guest into our kernel, so we cannot label guests properly.  Sure be
>>>>> nice if policy wasn't stored in this way.
>>>>
>>>> Just to clarify, it is not necessary to load the guest policy into the
>>>> host kernel in order to set labels on the guest filesystem.  SELinux
>>>> long ago introduced support for setting foreign/unknown labels on files
>>>> by processes with the appropriate permissions, and that mechanism was
>>>> used by livecd creator IIRC - it was also intended for use by rpm for
>>>> labeling files before the corresponding policy module was installed but
>>>> they never took advantage of it.
>>>
>>> IME you cannot set any label unless SELinux is enabled in the
>>> appliance kernel, but even assuming this is really possible, how do
>>> you know what label should you set?  Really we just want to do
>>> "restorecon -R /" but that has proven to be impossible.
>>
>> Hmm...the kernel certainly supports setting labels as long as the
>> filesystem xattr support is enabled, and setfiles used to work even if
>> SELinux is disabled, but admittedly we don't test on SELinux-disabled
>> very often.
>>
>> For SELinux-enabled, something like:
>> runcon -t setfiles_mac_t -- chroot /mnt /sbin/setfiles -v -F -e /proc -e
>> /sys -e /dev -e /selinux
>> /etc/selinux/targeted/contexts/files/file_contexts /
>> has been reported to work in the past.  The process needs CAP_MAC_ADMIN
>> in its effective capability set and it needs to be in a domain that is
>> allowed mac_admin by policy (hence the runcon -t setfiles_mac_t above).
> 
> Thanks - can confirm this works even with SELinux disabled in the
> appliance kernel.  I think this is the approach we will take, and it
> also means we don't need /.autorelabel to be fixed now.

Hmm...I think we still want to fix autorelabel regardless, just
hopefully either without the need to switch to permissive mode at all or
a tighter binding between switching to permissive and triggering the
relabeling and reboot sequence.

In Android, there is a notion of a recursive restorecon on upgrades that
is automatically triggered when the file_contexts configuration changes.
 That works by saving a hash of the file_contexts configuration(s) in an
xattr on the top-level directory of the mount and checking whether it
has changed.  But in that case everything happens from the init process
(based on init.rc configuration files), and SELinux is always enabled
and enforcing in Android, so we don't have to deal with scenarios where
the user has temporarily disabled SELinux and then re-enables it, and
init has all the permissions it needs.  The support for performing the
recursive restorecon however is now available in libselinux, so it could
be triggered directly by systemd.