From: guido@trentalancia.net (Guido Trentalancia) Date: Wed, 08 Feb 2017 00:39:48 +0100 Subject: [refpolicy] [PATCH] bootloader: stricter permissions and more tailored file contexts In-Reply-To: <85ccfdef-680e-fc31-6640-18567b4609b9@ieee.org> References: <1482452559.20547.19.camel@trentalancia.net> <20170205054446.GB5742@meriadoc.perfinion.com> <85ccfdef-680e-fc31-6640-18567b4609b9@ieee.org> Message-ID: <1486510788.7595.6.camel@trentalancia.net> To: refpolicy@oss.tresys.com List-Id: refpolicy.oss.tresys.com 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 ! > > 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