[RFC] Livecd-creator and selinux, we can play nice
Jeremy Katz
katzj at redhat.com
Thu May 29 12:51:12 UTC 2008
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
More information about the selinux
mailing list