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 -- fedora-selinux-list mailing list fedora-selinux-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/fedora-selinux-list
