From: guido@trentalancia.net (Guido Trentalancia) Date: Wed, 08 Feb 2017 06:45:33 +0100 Subject: [refpolicy] [PATCH] bootloader: stricter permissions and more tailored file contexts In-Reply-To: <20170208021704.GB2470@meriadoc.perfinion.com> References: <1482452559.20547.19.camel@trentalancia.net> <20170205054446.GB5742@meriadoc.perfinion.com> <85ccfdef-680e-fc31-6640-18567b4609b9@ieee.org> <1486510788.7595.6.camel@trentalancia.net> <20170208021704.GB2470@meriadoc.perfinion.com> Message-ID: <68B84283-D469-46ED-A4E5-771CB4C6ABC8@trentalancia.net> To: refpolicy@oss.tresys.com List-Id: refpolicy.oss.tresys.com A very simple patch for the kernel can avoid the bootloader from "doing anything it wants", while in that context this bootloader patch that I proposed for the Reference Policy plays the dual role of aiding the detection of a tampered system and reducing the likelyhood of ending up with a completely unbootable system. All of the above is, in my opinion, completely off-topic while the only thing that matters in the context of this mailing list is that this patch leads to a correct and thus, generally speaking, safer policy. Saying "the bootloader can do whatever it wants" is like saying "it is useless to confine the bootloader", while this is not true because the bootloader can be confined to do only what the policy AND the rest of the system impose. I hope this helps. Regards, Guido On the 8th of February 2017 03:17:04 CET, Jason Zaman wrote: >On Wed, Feb 08, 2017 at 12:39:48AM +0100, Guido Trentalancia via >refpolicy wrote: >> Hello Christopher. >> >> On Tue, 07/02/2017 at 18.26 -0500, Chris PeBenito wrote: >> > On 02/07/17 18:12, Guido Trentalancia via refpolicy wrote: >> > > >> > > Hello. >> > > >> > > The problem that the patch I submitted fixes (ability to rw >kernel >> > > and initramfs files) is much worse than the problem that it >caused >> > > (inability to generate a new grub configuration file). >> > >> > I don't agree.??If grub is nonfunctional, then that's worse. >> >> The worse things that can happen is that one has to generate a >> configuration file manually from a template. >> >> On the other hand, without the initial patch, the kernel and/or >> initramfs can be hijacked by a malicious version of the bootloader, >> which is much worse ! > >We're talking about the *bootloader*. it can hijack the kernel with or >without the patch, its the thing responsible for loading it when it >boots so it can do whatever it wants. if you dont like that SELinux >cant >help you, you need secureboot or trusted / measured boot instead. > >Not being able to boot is much worse. I beleive security is important >but its also worthless if you cant do anything. > > >> > > Also, it is extremely difficult to do extensive testing with >little >> > > or no resources available... >> > > >> > > If time allows, I will look at the problem and submit a patch >which >> > > enables the creation of a new grub configuration file. Consider >> > > that this is not always needed. >> > > >> > > There is no point in reverting the patch either partially or >> > > completely. It's just a matter of a few missing permissions, as >far >> > > as I can see now. >> > >> > I would like to see it fixed in a reasonable amount of time by >> > someone,? >> > otherwise I'll have to revert it. >> >> I have carried out further testing and I only acknowledge the >following >> missing interface call: files_read_boot_files(bootloader_t). Such >> interface is only needed when using grub-mkconfig to generate a >> configuration file, which is an auxiliary and not primary function of >> the bootloader. >> >> A small patch adding such missing interface has been posted a few >> minutes ago. >> >> > > On the 5th of February 2017 06:44:46 CET, Jason Zaman >> > > ion.com> wrote: >> > > > >> > > > On Fri, Dec 23, 2016 at 01:22:39AM +0100, Guido Trentalancia >via >> > > > refpolicy wrote: >> > > > > >> > > > > Update the bootloader module so that it can manage only its >> > > > > own runtime files and not all boot_t files (which include, >> > > > > for example, the common locations for kernel images and >> > > > > initramfs archives) and so that it can execute only its own >> > > > > etc files (needed by grub2-mkconfig) and not all etc_t files >> > > > > which is more dangerous. >> > > > >> > > > This patch broke grub-mkconfig. Can you check your patches more >> > > > carefully in >> > > > the future? >> > > > >> > > > bregalad ~ # grub-mkconfig -o /boot/grub/grub.cfg >> > > > Generating grub configuration file ... >> > > > mv: cannot move '/boot/grub/grub.cfg.new' to >> > > > '/boot/grub/grub.cfg': >> > > > Permission denied >> > > > >> > > > type=AVC msg=audit(1486273313.557:26703): avc:??denied??{ >unlink >> > > > } for >> > > > pid=10757 comm="mv" name="grub.cfg" dev="md1" ino=10070 >> > > > scontext=staff_u:sysadm_r:bootloader_t:s0-s0:c0.c1023 >> > > > tcontext=system_u:object_r:bootloader_etc_t:s0 tclass=file >> > > > permissive=0 >> > > > type=SYSCALL msg=audit(1486273313.557:26703): arch=c000003e >> > > > syscall=82 >> > > > success=no exit=-13 a0=3a93725fbef a1=3a93725fc07 a2=0 a3=2 >> > > > items=4 >> > > > ppid=9489 pid=10757 auid=1000 uid=0 gid=0 euid=0 suid=0 fsuid=0 >> > > > egid=0 >> > > > sgid=0 fsgid=0 tty=pts3 ses=4 comm="mv" exe="/bin/mv" >> > > > subj=staff_u:sysadm_r:bootloader_t:s0-s0:c0.c1023 key=(null) >> > > > type=CWD msg=audit(1486273313.557:26703): cwd="/root" >> > > > type=PATH msg=audit(1486273313.557:26703): item=0 >> > > > name="/boot/grub/" >> > > > inode=10041 dev=09:01 mode=040755 ouid=0 ogid=0 rdev=00:00 >> > > > obj=system_u:object_r:bootloader_run_t:s0 nametype=PARENT >> > > > type=PATH msg=audit(1486273313.557:26703): item=1 >> > > > name="/boot/grub/" >> > > > inode=10041 dev=09:01 mode=040755 ouid=0 ogid=0 rdev=00:00 >> > > > obj=system_u:object_r:bootloader_run_t:s0 nametype=PARENT >> > > > type=PATH msg=audit(1486273313.557:26703): item=2 >> > > > name="/boot/grub/grub.cfg.new" inode=10072 dev=09:01 >mode=0100600 >> > > > ouid=0 ogid=0 rdev=00:00 >obj=staff_u:object_r:bootloader_run_t:s0 >> > > > nametype=DELETE >> > > > type=PATH msg=audit(1486273313.557:26703): item=3 >> > > > name="/boot/grub/grub.cfg" inode=10070 dev=09:01 mode=0100600 >> > > > ouid=0 >> > > > ogid=0 rdev=00:00 obj=system_u:object_r:bootloader_etc_t:s0 >> > > > nametype=DELETE >> > > > >> > > > Its broken everywhere except EFI partitions and only because >> > > > those are >> > > > just >> > > > dosfs_t everywhere so this change doesnt matter. >> > > > >> > > > -- Jason >> > > > >> > > > > >> > > > > >> > > > > Signed-off-by: Guido Trentalancia >> > > > > --- >> > > > > ?policy/modules/admin/bootloader.fc |????6 ++++++ >> > > > > ?policy/modules/admin/bootloader.te |???17 +++++++++++++---- >> > > > > ?2 files changed, 19 insertions(+), 4 deletions(-) >> > > > > >> > > > > diff -pru a/policy/modules/admin/bootloader.fc >> > > > b/policy/modules/admin/bootloader.fc >> > > > > >> > > > > --- a/policy/modules/admin/bootloader.fc 2016-08-06 >> > > > 21:26:43.273774031 +0200 >> > > > > >> > > > > +++ b/policy/modules/admin/bootloader.fc 2016-12-23 >> > > > 01:10:37.258482434 +0100 >> > > > > >> > > > > @@ -1,6 +1,12 @@ >> > > > > +/boot/grub.* -d gen_context(system_u:object_r:bo >> > > > > otloader_run_t,s0) >> > > > > +/boot/grub.*/.* gen_context(system_u:object_r:b >> > > > > ootloader_run_t,s0) >> > > > > + >> > > > > >> > > > +/boot/grub.*/grub.cfg -- gen_context(system_u:obje >> > > > ct_r:bootloader_etc_t,s0) >> > > > > >> > > > > >> > > > +/boot/grub.*/grub.conf -- gen_context(system_u:obj >> > > > ect_r:bootloader_etc_t,s0) >> > > > > >> > > > > >> > > > > >> > > > /etc/lilo\.conf.* -- gen_context(system_u:object_r: >> > > > bootloader_etc_t,s0) >> > > > > >> > > > > >> > > > /etc/yaboot\.conf.* -- gen_context(system_u:object_ >> > > > r:bootloader_etc_t,s0) >> > > > > >> > > > > >> > > > +/etc/grub.d(/.*)? -- gen_context(system_u:object_r >> > > > :bootloader_etc_t,s0) >> > > > > >> > > > > >> > > > > ?/sbin/grub -- gen_context(system_u:objec >> > > > > t_r:bootloader_exec_t,s0) >> > > > > ?/sbin/lilo.* -- gen_context(system_u:obj >> > > > > ect_r:bootloader_exec_t,s0) >> > > > > diff -pru a/policy/modules/admin/bootloader.te >> > > > b/policy/modules/admin/bootloader.te >> > > > > >> > > > > --- a/policy/modules/admin/bootloader.te 2016-08-06 >> > > > 21:26:43.274774043 +0200 >> > > > > >> > > > > +++ b/policy/modules/admin/bootloader.te 2016-12-23 >> > > > 01:17:00.900143820 +0100 >> > > > > >> > > > > @@ -22,6 +22,13 @@ application_domain(bootloader_t, bootloa >> > > > > ?role bootloader_roles types bootloader_t; >> > > > > >> > > > > ?# >> > > > > +# bootloader_run_t are image and other runtime >> > > > > +# files >> > > > > +# >> > > > > +type bootloader_run_t alias run_bootloader_t; >> > > > > +files_type(bootloader_run_t) >> > > > > + >> > > > > +# >> > > > > ?# bootloader_etc_t is the configuration file, >> > > > > ?# grub.conf, lilo.conf, etc. >> > > > > ?# >> > > > > @@ -45,7 +52,7 @@ allow bootloader_t self:capability { dac >> > > > > ?allow bootloader_t self:process { signal_perms execmem }; >> > > > > ?allow bootloader_t self:fifo_file rw_fifo_file_perms; >> > > > > >> > > > > -allow bootloader_t bootloader_etc_t:file read_file_perms; >> > > > > +allow bootloader_t bootloader_etc_t:file exec_file_perms; >> > > > > ?# uncomment the following lines if you use "lilo -p" >> > > > > ?#allow bootloader_t bootloader_etc_t:file manage_file_perms; >> > > > > ?#files_etc_filetrans(bootloader_t,bootloader_etc_t,file) >> > > > > @@ -59,6 +66,11 @@ files_tmp_filetrans(bootloader_t, bootlo >> > > > > ?# for tune2fs (cjp: ?) >> > > > > ?files_root_filetrans(bootloader_t, bootloader_tmp_t, file) >> > > > > >> > > > > +manage_dirs_pattern(bootloader_t, bootloader_run_t, >> > > > bootloader_run_t) >> > > > > >> > > > > +manage_files_pattern(bootloader_t, bootloader_run_t, >> > > > bootloader_run_t) >> > > > > >> > > > > +manage_lnk_files_pattern(bootloader_t, bootloader_run_t, >> > > > bootloader_run_t) >> > > > > >> > > > > +files_boot_filetrans(bootloader_t, bootloader_run_t, { dir >> > > > > file >> > > > lnk_file }) >> > > > > >> > > > > + >> > > > > ?kernel_getattr_core_if(bootloader_t) >> > > > > ?kernel_read_network_state(bootloader_t) >> > > > > ?kernel_read_system_state(bootloader_t) >> > > > > @@ -96,10 +108,7 @@ corecmd_exec_all_executables(bootloader_ >> > > > > ?domain_use_interactive_fds(bootloader_t) >> > > > > >> > > > > ?files_create_boot_dirs(bootloader_t) >> > > > > -files_manage_boot_files(bootloader_t) >> > > > > -files_manage_boot_symlinks(bootloader_t) >> > > > > ?files_read_etc_files(bootloader_t) >> > > > > -files_exec_etc_files(bootloader_t) >> > > > > ?files_read_usr_src_files(bootloader_t) >> > > > > ?files_read_usr_files(bootloader_t) >> > > > > ?files_read_var_files(bootloader_t) >> >> Regards, >> >> Guido >> _______________________________________________ >> refpolicy mailing list >> refpolicy at oss.tresys.com >> http://oss.tresys.com/mailman/listinfo/refpolicy