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