[RFC] Livecd-creator and selinux, we can play nice

[Eric Paris]

So I've spent a fair bit of time the last 2 weeks trying to get
livecd-creator and an selinux enforcing machine to play nicely together.
It doesn't look like much, but from the point of view of the livecd
creator I think the following patch is all we need.  Working with
rawhide as the host system I was able to build F8, F9 and rawhide
livecd's with an enforcing machine.

I wouldn't suggest jumping into enfocing builds just yet as there are
still some policy issues I need to work out with the selinux people but
I would like comments.  Basically its quite simple, if selinux is on the
host we create a fake /selinux which tells the install chroot lies.
I've had to make some changes to some selinux libraries to support all
this, but I think we are just about there.

I'll probably backport some of the kernel changes to F9 after they are
all tested and better settled but for now I'd like input on my livecd
changes....

---

diff -Naupr /usr/lib/python2.5/site-packages/imgcreate/creator.py
/root/imgcreate-5-28-08/creator.py
--- /usr/lib/python2.5/site-packages/imgcreate/creator.py	2008-05-06 12:16:08.000000000
-0400
+++ /root/imgcreate-5-28-08/creator.py	2008-05-28 15:48:30.000000000 -0400
@@ -23,6 +23,7 @@ import sys
 import tempfile
 import shutil
 
+import selinux
 import yum
 import rpm
 
@@ -427,7 +428,7 @@ class ImageCreator(object):
 
         self._mount_instroot(base_on)
 
-        for d in ("/dev/pts", "/etc", "/boot",
"/var/log", "/var/cache/yum"):
+        for d in ("/dev/pts", "/etc", "/boot",
"/var/log", "/var/cache/yum", "/sys", "/proc",
"/selinux"):
             makedirs(self._instroot + d)
 
         cachesrc = cachedir or (self.__builddir + "/yum-cache")
@@ -439,10 +440,6 @@ class ImageCreator(object):
                           (cachesrc, "/var/cache/yum")]:
             self.__bindmounts.append(BindChrootMount(f, self._instroot, dest))
 
-        # /selinux should only be mounted if selinux is enabled (enforcing or
permissive)
-        if kickstart.selinux_enabled(self.ks):
-            self.__bindmounts.append(BindChrootMount("/selinux",
self._instroot, None))
-
         # Create minimum /dev
         origumask = os.umask(0000)
         devices = [('null',   1, 3, 0666),
@@ -460,6 +457,37 @@ class ImageCreator(object):
         os.symlink('/proc/self/fd/2', self._instroot + "/dev/stderr")
         os.umask(origumask)
 
+        # if selinux exists on the host we need to lie to the chroot
+        if os.path.exists("/selinux/enforce"):
+            selinux_dir = self._instroot + "/selinux"
+
+            # enforce=0 tells the chroot selinux is not enforcing
+            # policyvers=99 tell the chroot to make the highest version of policy it can
+            files = [('/enforce', '0'),
+                     ('/policyvers', '99')]
+            for (file, value) in files:
+                fd = os.open(selinux_dir + file, os.O_WRONLY | os.O_TRUNC | os.O_CREAT)
+                os.write(fd, value)
+                os.close(fd)
+
+            # we steal mls from the host system for now, might be best to always set it
to 1????
+            files = ["/selinux/mls"]
+            for file in files:
+                shutil.copyfile(file, self._instroot + file)
+
+            # make /load -> /dev/null so chroot policy loads don't hurt anything
+            os.mknod(selinux_dir + "/load", 0666 | stat.S_IFCHR, os.makedev(1,
3))
+
+        # selinux is on whoo hooo
+        if kickstart.selinux_enabled(self.ks):
+            # label the fs like it is a root before the bind mounting
+            cmd = "/sbin/setfiles -F -r %s %s %s" % (self._instroot,
selinux.selinux_file_context_path(), self._instroot)
+            os.system(cmd)
+            # these dumb things don't get magically fixed, so make the user generic
+            for f in ["/proc", "/sys", "/selinux"]:
+                cmd = "chcon -u system_u %s" % (self._instroot + f)
+                os.system(cmd)
+
         self._do_bindmounts()
 
         os.symlink("../proc/mounts", self._instroot + "/etc/mtab")
@@ -853,6 +881,18 @@ class LoopImageCreator(ImageCreator):
                                (self._image, e))
 
     def _unmount_instroot(self):
+        # if the system was running selinux clean up our lies
+        if os.path.exists("/selinux/enforce"):
+            files = ['/enforce',
+                     '/policyvers',
+                     '/mls',
+                     '/load']
+            for file in files:
+                try:
+                    os.unlink(self._instroot + "/selinux" + file)
+                except OSError:
+                    pass
+
         if not self.__instloop is None:
             self.__instloop.cleanup()
 
diff -Naupr /usr/lib/python2.5/site-packages/imgcreate/kickstart.py
/root/imgcreate-5-28-08/kickstart.py
--- /usr/lib/python2.5/site-packages/imgcreate/kickstart.py	2008-05-06 12:16:08.000000000
-0400
+++ /root/imgcreate-5-28-08/kickstart.py	2008-05-19 11:22:41.000000000 -0400
@@ -372,11 +372,11 @@ class SelinuxConfig(KickstartConfig):
 
         if ksselinux.selinux == ksconstants.SELINUX_DISABLED:
             return
-        if not os.path.exists(self.path("/sbin/restorecon")):
+        if os.path.exists(self.path("/sbin/restorecon")):
+            self.call(["/sbin/restorecon", "-l", "-v",
"-r", "-F", "-e", "/proc", "-e",
"/sys", "-e", "/dev", "-e", "/selinux",
"/"])
+        else:
             return
 
-        self.call(["/sbin/restorecon", "-l", "-v",
"-r", "/"])
-
     def apply(self, ksselinux):
         if os.path.exists(self.path("/usr/sbin/lokkit")):
             args = ["/usr/sbin/lokkit", "-f", "--quiet",
"--nostart"]

[Bill Nottingham]

Eric Paris (eparis(a)redhat.com) said: 
>  So I've spent a fair bit of time the last 2 weeks trying to get
>  livecd-creator and an selinux enforcing machine to play nicely together.
>  It doesn't look like much, but from the point of view of the livecd
>  creator I think the following patch is all we need.  Working with
>  rawhide as the host system I was able to build F8, F9 and rawhide
>  livecd's with an enforcing machine.
>  
>  I wouldn't suggest jumping into enfocing builds just yet as there are
>  still some policy issues I need to work out with the selinux people but
>  I would like comments.  Basically its quite simple, if selinux is on the
>  host we create a fake /selinux which tells the install chroot lies.
>  I've had to make some changes to some selinux libraries to support all
>  this, but I think we are just about there.
>  
>  I'll probably backport some of the kernel changes to F9 after they are
>  all tested and better settled but for now I'd like input on my livecd
>  changes.... 
My concern is this is a normal occurence (needing a chroot) that you're
only patching in one place. Do we code this same logic into mock? Into
pungi? Into yum --installroot? Into the documentation for admins on
how to set up a chroot?

(Also, for general use, we need this in a RHEL 5 kernel. Fun!)

Bill

[Eric Paris]

On Wed, 2008-05-28 at 16:04 -0400, Bill Nottingham wrote:
>  Eric Paris (eparis(a)redhat.com) said: 
>  > So I've spent a fair bit of time the last 2 weeks trying to get
>  > livecd-creator and an selinux enforcing machine to play nicely together.
>  > It doesn't look like much, but from the point of view of the livecd
>  > creator I think the following patch is all we need.  Working with
>  > rawhide as the host system I was able to build F8, F9 and rawhide
>  > livecd's with an enforcing machine.
>  > 
>  > I wouldn't suggest jumping into enfocing builds just yet as there are
>  > still some policy issues I need to work out with the selinux people but
>  > I would like comments.  Basically its quite simple, if selinux is on the
>  > host we create a fake /selinux which tells the install chroot lies.
>  > I've had to make some changes to some selinux libraries to support all
>  > this, but I think we are just about there.
>  > 
>  > I'll probably backport some of the kernel changes to F9 after they are
>  > all tested and better settled but for now I'd like input on my livecd
>  > changes....
>  
>  My concern is this is a normal occurence (needing a chroot)  
Yes and no....

>  that you're
>  only patching in one place. Do we code this same logic into mock? 
Mock doesn't need any special labeling knowledge, mock just has to have
policy that doesn't get in the way....   But mock is the next thing I'm
going to start trying to find issues with. 

>   Into
>  pungi? 
don't know pungi...

>  Into yum --installroot? 
possibly....

>  Into the documentation for admins on
>  how to set up a chroot? 
where is this documentation?   usually, no, but if the chroot is
supposed to be drastically different than the host policy maybe...

>  (Also, for general use, we need this in a RHEL 5 kernel. Fun!)
Yeah, I've heard that, and its likely to not be possible....   Once we
all agree on how to make it work going forward we can talk about RHEL5.

[Seth Vidal]

On Wed, 2008-05-28 at 16:11 -0400, Eric Paris wrote:
>  > My concern is this is a normal occurence (needing a chroot) 
>  
>  Yes and no....
>   
sure looks like we'd need to make sure:

yum, mock and rpm all know how to set this up given how it would impact
chroot creation.

We may want to drop this back to the lowest level chroot creation.

-sv

[Jeremy Katz]

seth vidal wrote:
>  On Wed, 2008-05-28 at 16:11 -0400, Eric Paris wrote:
> >> My concern is this is a normal occurence (needing a chroot) 
> > Yes and no....
>  
>  sure looks like we'd need to make sure:
>  
>  yum, mock and rpm all know how to set this up given how it would impact
>  chroot creation. 
If we do so, then we should also make sure that all do things 
consistently wrt /dev for creating a chroot as well.  And /proc and 
/sys.  The reality is that the different applications do have a somewhat 
different idea of what they need/want out of their chroots and do things 
(or don't) accordingly.

>  We may want to drop this back to the lowest level chroot creation.
Which isn't to say that we might not decide down the road to push it 
down the stack, but I don't know that livecd-creator is a bad place in 
the short to medium term as Eric continues looking at SELinux and chroot 
interactions (Right Eric? :-)

Jeremy

[Jeremy Katz]

Eric Paris wrote:
>  So I've spent a fair bit of time the last 2 weeks trying to get
>  livecd-creator and an selinux enforcing machine to play nicely together.
>  It doesn't look like much, but from the point of view of the livecd
>  creator I think the following patch is all we need.  Working with
>  rawhide as the host system I was able to build F8, F9 and rawhide
>  livecd's with an enforcing machine.
>  
>  I wouldn't suggest jumping into enfocing builds just yet as there are
>  still some policy issues I need to work out with the selinux people but
>  I would like comments.  Basically its quite simple, if selinux is on the
>  host we create a fake /selinux which tells the install chroot lies.
>  I've had to make some changes to some selinux libraries to support all
>  this, but I think we are just about there. 
Very cool and definitely long needed.  Thanks for taking the time to 
really dive into this.  And this is far simpler than the approach I had 
started looking at once upon a time (... which involved fuse)

A few comments on the patch

>  diff -Naupr /usr/lib/python2.5/site-packages/imgcreate/creator.py
> /root/imgcreate-5-28-08/creator.py
>  --- /usr/lib/python2.5/site-packages/imgcreate/creator.py	2008-05-06 12:16:08.000000000
> -0400
>  +++ /root/imgcreate-5-28-08/creator.py	2008-05-28 15:48:30.000000000 -0400
>  @@ -460,6 +457,37 @@ class ImageCreator(object):
>           os.symlink('/proc/self/fd/2', self._instroot + "/dev/stderr")
>           os.umask(origumask)
>   
>  +        # if selinux exists on the host we need to lie to the chroot
>  +        if os.path.exists("/selinux/enforce"):
>  +            selinux_dir = self._instroot + "/selinux"
>  +
>  +            # enforce=0 tells the chroot selinux is not enforcing
>  +            # policyvers=99 tell the chroot to make the highest version of policy it
> can
>  +            files = [('/enforce', '0'),
>  +                     ('/policyvers', '99')] 
Does the kernel guarantee that 99 will be the highest version of policy? 
  Not that it likely matters much.  Also, having this as a tuple rather 
than a list makes it marginally faster since we're never going to modify it

>  +            for (file, value) in files:
>  +                fd = os.open(selinux_dir + file, os.O_WRONLY | os.O_TRUNC | os.O_CREAT)
>  +                os.write(fd, value)
>  +                os.close(fd)
>  +
>  +            # we steal mls from the host system for now, might be best to always set it
> to 1????
>  +            files = ["/selinux/mls"]
>  +            for file in files:
>  +                shutil.copyfile(file, self._instroot + file)
>  +
>  +            # make /load -> /dev/null so chroot policy loads don't hurt anything
>  +            os.mknod(selinux_dir + "/load", 0666 | stat.S_IFCHR, os.makedev(1,
> 3)) 
This being the big win :)

>  +        # selinux is on whoo hooo
>  +        if kickstart.selinux_enabled(self.ks):
>  +            # label the fs like it is a root before the bind mounting
>  +            cmd = "/sbin/setfiles -F -r %s %s %s" % (self._instroot,
> selinux.selinux_file_context_path(), self._instroot)
>  +            os.system(cmd)
>  +            # these dumb things don't get magically fixed, so make the user generic
>  +            for f in ["/proc", "/sys", "/selinux"]:
>  +                cmd = "chcon -u system_u %s" % (self._instroot + f)
>  +                os.system(cmd) 
os.system is generally not preferred -- using the subprocess module is a 
lot safer.

Also, overall it might be nice to encapsulate the /selinux creation here 
into its own __create_selinuxfs() method that gets called.  /me makes a 
note to do that to the /dev creation too.

>  @@ -853,6 +881,18 @@ class LoopImageCreator(ImageCreator):
>                                  (self._image, e))
>   
>       def _unmount_instroot(self):
>  +        # if the system was running selinux clean up our lies
>  +        if os.path.exists("/selinux/enforce"):
>  +            files = ['/enforce',
>  +                     '/policyvers',
>  +                     '/mls',
>  +                     '/load'] 
Again a tuple versus a list

>  +            for file in files:
>  +                try:
>  +                    os.unlink(self._instroot + "/selinux" + file)
>  +                except OSError:
>  +                    pass 
And again having it in a method is probably the nice thing to do.  And I 
know I said to stick it into _unmount_instroot, but seeing where you've 
put the mount side, it probably makes more sense in unmount() instead

Jeremy

[Daniel J Walsh]

Bill Nottingham wrote:
>  Eric Paris (eparis(a)redhat.com) said: 
> > So I've spent a fair bit of time the last 2 weeks trying to get
> > livecd-creator and an selinux enforcing machine to play nicely together.
> > It doesn't look like much, but from the point of view of the livecd
> > creator I think the following patch is all we need.  Working with
> > rawhide as the host system I was able to build F8, F9 and rawhide
> > livecd's with an enforcing machine.
> >
> > I wouldn't suggest jumping into enfocing builds just yet as there are
> > still some policy issues I need to work out with the selinux people but
> > I would like comments.  Basically its quite simple, if selinux is on the
> > host we create a fake /selinux which tells the install chroot lies.
> > I've had to make some changes to some selinux libraries to support all
> > this, but I think we are just about there.
> >
> > I'll probably backport some of the kernel changes to F9 after they are
> > all tested and better settled but for now I'd like input on my livecd
> > changes....
>  
>  My concern is this is a normal occurence (needing a chroot) that you're
>  only patching in one place. Do we code this same logic into mock? Into
>  pungi? Into yum --installroot? Into the documentation for admins on
>  how to set up a chroot?
>  
>  (Also, for general use, we need this in a RHEL 5 kernel. Fun!)
>  
>  Bill
>  
>  --
>  fedora-selinux-list mailing list
>  fedora-selinux-list(a)redhat.com
>  https://www.redhat.com/mailman/listinfo/fedora-selinux-list 
Well I think we need to
do a couple of these to figure out the common
requirements.

I envision mock to be quite different then livecd.  I think we need to
full the mock chroot to think SELinux is disabled and to do no labeling
in the chroot.  This would allow us to confine the mock process to be
able to write to the chroot and label the chroot mock_rw_t.  We could
then use SELinux to prevent mock environments from breaking out of the
chroot, and stop mock environments from doing evil network things within
the chroot.

In livecd we need to be able to put down labels that the host machine
does not understand.

[Bill Nottingham]

Daniel J Walsh (dwalsh(a)redhat.com) said: 
>  Well I think we need to do a couple of these to figure out the
> common
>  requirements.
>  
>  I envision mock to be quite different then livecd.  I think we need to
>  full the mock chroot to think SELinux is disabled and to do no labeling
>  in the chroot.  This would allow us to confine the mock process to be
>  able to write to the chroot and label the chroot mock_rw_t.  We could
>  then use SELinux to prevent mock environments from breaking out of the
>  chroot, and stop mock environments from doing evil network things within
>  the chroot.
>  
>  In livecd we need to be able to put down labels that the host machine
>  does not understand. 
The problem is that mock can be used to do non-build things. (For example,
creating the anaconda install images.)

Bill