From mboxrd@z Thu Jan 1 00:00:00 1970 From: jason@perfinion.com (Jason Zaman) Date: Wed, 8 Feb 2017 10:17:04 +0800 Subject: [refpolicy] [PATCH] bootloader: stricter permissions and more tailored file contexts In-Reply-To: <1486510788.7595.6.camel@trentalancia.net> 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> Message-ID: <20170208021704.GB2470@meriadoc.perfinion.com> To: refpolicy@oss.tresys.com List-Id: refpolicy.oss.tresys.com 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