[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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

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")
+ # 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


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]