2016-12-23 00:22:39

by guido

[permalink] [raw]
Subject: [refpolicy] [PATCH] bootloader: stricter permissions and more tailored file contexts

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.

Signed-off-by: Guido Trentalancia <[email protected]>
---
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:bootloader_run_t,s0)
+/boot/grub.*/.* gen_context(system_u:object_r:bootloader_run_t,s0)
+
+/boot/grub.*/grub.cfg -- gen_context(system_u:object_r:bootloader_etc_t,s0)
+/boot/grub.*/grub.conf -- gen_context(system_u:object_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:object_r:bootloader_exec_t,s0)
/sbin/lilo.* -- gen_context(system_u:object_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)


2016-12-27 15:53:10

by Chris PeBenito

[permalink] [raw]
Subject: [refpolicy] [PATCH] bootloader: stricter permissions and more tailored file contexts

On 12/22/16 19:22, 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.

Merged.


> Signed-off-by: Guido Trentalancia <[email protected]>
> ---
> 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:bootloader_run_t,s0)
> +/boot/grub.*/.* gen_context(system_u:object_r:bootloader_run_t,s0)
> +
> +/boot/grub.*/grub.cfg -- gen_context(system_u:object_r:bootloader_etc_t,s0)
> +/boot/grub.*/grub.conf -- gen_context(system_u:object_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:object_r:bootloader_exec_t,s0)
> /sbin/lilo.* -- gen_context(system_u:object_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)

--
Chris PeBenito

2017-02-05 05:44:46

by Jason Zaman

[permalink] [raw]
Subject: [refpolicy] [PATCH] bootloader: stricter permissions and more tailored file contexts

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 <[email protected]>
> ---
> 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:bootloader_run_t,s0)
> +/boot/grub.*/.* gen_context(system_u:object_r:bootloader_run_t,s0)
> +
> +/boot/grub.*/grub.cfg -- gen_context(system_u:object_r:bootloader_etc_t,s0)
> +/boot/grub.*/grub.conf -- gen_context(system_u:object_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:object_r:bootloader_exec_t,s0)
> /sbin/lilo.* -- gen_context(system_u:object_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)
> _______________________________________________
> refpolicy mailing list
> refpolicy at oss.tresys.com
> http://oss.tresys.com/mailman/listinfo/refpolicy

2017-02-07 23:12:27

by guido

[permalink] [raw]
Subject: [refpolicy] [PATCH] bootloader: stricter permissions and more tailored file contexts

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).

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.
Regards,

Guido

On the 5th of February 2017 06:44:46 CET, Jason Zaman <[email protected]> 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 <[email protected]>
>> ---
>> 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:bootloader_run_t,s0)
>> +/boot/grub.*/.* gen_context(system_u:object_r:bootloader_run_t,s0)
>> +
>>
>+/boot/grub.*/grub.cfg -- gen_context(system_u:object_r:bootloader_etc_t,s0)
>>
>+/boot/grub.*/grub.conf -- gen_context(system_u:object_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:object_r:bootloader_exec_t,s0)
>> /sbin/lilo.* -- gen_context(system_u:object_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)
>> _______________________________________________
>> refpolicy mailing list
>> refpolicy at oss.tresys.com
>> http://oss.tresys.com/mailman/listinfo/refpolicy

2017-02-07 23:26:03

by Chris PeBenito

[permalink] [raw]
Subject: [refpolicy] [PATCH] bootloader: stricter permissions and more tailored file contexts

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.


> 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.


> On the 5th of February 2017 06:44:46 CET, Jason Zaman <[email protected]> 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 <[email protected]>
>>> ---
>>> 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:bootloader_run_t,s0)
>>> +/boot/grub.*/.* gen_context(system_u:object_r:bootloader_run_t,s0)
>>> +
>>>
>> +/boot/grub.*/grub.cfg -- gen_context(system_u:object_r:bootloader_etc_t,s0)
>>>
>> +/boot/grub.*/grub.conf -- gen_context(system_u:object_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:object_r:bootloader_exec_t,s0)
>>> /sbin/lilo.* -- gen_context(system_u:object_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)



--
Chris PeBenito

2017-02-07 23:32:32

by guido

[permalink] [raw]
Subject: [refpolicy] [PATCH] bootloader: add permissions to read boot files in order to generate a configuration file

Allow the bootloader to read boot files in order to generate
a configuration file.

Signed-off-by: Guido Trentalancia <[email protected]>
---
policy/modules/admin/bootloader.te | 1 +
1 file changed, 1 insertion(+)

diff -pru refpolicy-git-08022017-orig/policy/modules/admin/bootloader.te refpolicy-git-08022017/policy/modules/admin/bootloader.te
--- refpolicy-git-08022017-orig/policy/modules/admin/bootloader.te 2016-12-29 22:48:16.446818415 +0100
+++ refpolicy-git-08022017/policy/modules/admin/bootloader.te 2017-02-08 00:14:22.923674773 +0100
@@ -108,6 +108,7 @@ corecmd_exec_all_executables(bootloader_
domain_use_interactive_fds(bootloader_t)

files_create_boot_dirs(bootloader_t)
+files_read_boot_files(bootloader_t)
files_read_etc_files(bootloader_t)
files_read_usr_src_files(bootloader_t)
files_read_usr_files(bootloader_t)

2017-02-07 23:39:48

by guido

[permalink] [raw]
Subject: [refpolicy] [PATCH] bootloader: stricter permissions and more tailored file contexts

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 <jason@perfin
> > 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 <[email protected]>
> > > > ---
> > > > ?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

2017-02-08 02:13:35

by Jason Zaman

[permalink] [raw]
Subject: [refpolicy] [PATCH] bootloader: add permissions to read boot files in order to generate a configuration file

On Wed, Feb 08, 2017 at 12:32:32AM +0100, Guido Trentalancia via refpolicy wrote:
> Allow the bootloader to read boot files in order to generate
> a configuration file.
>
> Signed-off-by: Guido Trentalancia <[email protected]>

NACK. this wont work. Just use the patch I posted
http://oss.tresys.com/pipermail/refpolicy/2017-February/009011.html
[PATCH v2] bootloader: grub needs to manage grub.cfg and read kernels


> ---
> policy/modules/admin/bootloader.te | 1 +
> 1 file changed, 1 insertion(+)
>
> diff -pru refpolicy-git-08022017-orig/policy/modules/admin/bootloader.te refpolicy-git-08022017/policy/modules/admin/bootloader.te
> --- refpolicy-git-08022017-orig/policy/modules/admin/bootloader.te 2016-12-29 22:48:16.446818415 +0100
> +++ refpolicy-git-08022017/policy/modules/admin/bootloader.te 2017-02-08 00:14:22.923674773 +0100
> @@ -108,6 +108,7 @@ corecmd_exec_all_executables(bootloader_
> domain_use_interactive_fds(bootloader_t)
>
> files_create_boot_dirs(bootloader_t)
> +files_read_boot_files(bootloader_t)
> files_read_etc_files(bootloader_t)
> files_read_usr_src_files(bootloader_t)
> files_read_usr_files(bootloader_t)
> _______________________________________________
> refpolicy mailing list
> refpolicy at oss.tresys.com
> http://oss.tresys.com/mailman/listinfo/refpolicy

2017-02-08 02:17:04

by Jason Zaman

[permalink] [raw]
Subject: [refpolicy] [PATCH] bootloader: stricter permissions and more tailored file contexts

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 <jason@perfin
> > > 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 <[email protected]>
> > > > > ---
> > > > > ?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

2017-02-08 05:45:33

by guido

[permalink] [raw]
Subject: [refpolicy] [PATCH] bootloader: stricter permissions and more tailored file contexts

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 <[email protected]> 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
><jason@perfin
>> > > 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 <[email protected]>
>> > > > > ---
>> > > > > ?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

2017-02-11 19:51:54

by Chris PeBenito

[permalink] [raw]
Subject: [refpolicy] [PATCH] bootloader: add permissions to read boot files in order to generate a configuration file

On 02/07/17 21:13, Jason Zaman via refpolicy wrote:
> On Wed, Feb 08, 2017 at 12:32:32AM +0100, Guido Trentalancia via refpolicy wrote:
>> Allow the bootloader to read boot files in order to generate
>> a configuration file.
>>
>> Signed-off-by: Guido Trentalancia <[email protected]>
>
> NACK. this wont work. Just use the patch I posted
> http://oss.tresys.com/pipermail/refpolicy/2017-February/009011.html
> [PATCH v2] bootloader: grub needs to manage grub.cfg and read kernels

I've decided to revert this patch. A nonfunctional system app like this
is not acceptable. I am still open to a change along these lines,
though arguably because bootloader has raw disk access, it doesn't
matter much if it can overwrite the kernel via normal file access.


>> ---
>> policy/modules/admin/bootloader.te | 1 +
>> 1 file changed, 1 insertion(+)
>>
>> diff -pru refpolicy-git-08022017-orig/policy/modules/admin/bootloader.te refpolicy-git-08022017/policy/modules/admin/bootloader.te
>> --- refpolicy-git-08022017-orig/policy/modules/admin/bootloader.te 2016-12-29 22:48:16.446818415 +0100
>> +++ refpolicy-git-08022017/policy/modules/admin/bootloader.te 2017-02-08 00:14:22.923674773 +0100
>> @@ -108,6 +108,7 @@ corecmd_exec_all_executables(bootloader_
>> domain_use_interactive_fds(bootloader_t)
>>
>> files_create_boot_dirs(bootloader_t)
>> +files_read_boot_files(bootloader_t)
>> files_read_etc_files(bootloader_t)
>> files_read_usr_src_files(bootloader_t)
>> files_read_usr_files(bootloader_t)
>> _______________________________________________
>> refpolicy mailing list
>> refpolicy at oss.tresys.com
>> http://oss.tresys.com/mailman/listinfo/refpolicy
> _______________________________________________
> refpolicy mailing list
> refpolicy at oss.tresys.com
> http://oss.tresys.com/mailman/listinfo/refpolicy
>


--
Chris PeBenito

2017-02-11 20:18:05

by guido

[permalink] [raw]
Subject: [refpolicy] [PATCH] bootloader: add permissions to read boot files in order to generate a configuration file

Hello.

On Sat, 11/02/2017 at 14.51 -0500, Chris PeBenito wrote:
> On 02/07/17 21:13, Jason Zaman via refpolicy wrote:
> >
> > On Wed, Feb 08, 2017 at 12:32:32AM +0100, Guido Trentalancia via
> > refpolicy wrote:
> > >
> > > Allow the bootloader to read boot files in order to generate
> > > a configuration file.
> > >
> > > Signed-off-by: Guido Trentalancia <[email protected]>
> >
> > NACK. this wont work. Just use the patch I posted
> > http://oss.tresys.com/pipermail/refpolicy/2017-February/009011.html
> > [PATCH v2] bootloader: grub needs to manage grub.cfg and read
> > kernels
>
> I've decided to revert this patch.??A nonfunctional system app like
> this?
> is not acceptable.??I am still open to a change along these lines,?
> though arguably because bootloader has raw disk access, it doesn't?
> matter much if it can overwrite the kernel via normal file access.

The core grub functionality has never stopped working.

The only thing that was not fully functional is an auxiliary
application shipped with grub (grub-mkconfig) that can be optionally
used to create the grub configuration file. It basically scans the
/boot directory for kernel images and creates an entry for each kernel
image it found.

With the additional very simple patch that has been posted, the above
mentioned optional functionality works again.

> > > ---
> > > ?policy/modules/admin/bootloader.te |????1 +
> > > ?1 file changed, 1 insertion(+)
> > >
> > > diff -pru refpolicy-git-08022017-
> > > orig/policy/modules/admin/bootloader.te refpolicy-git-
> > > 08022017/policy/modules/admin/bootloader.te
> > > --- refpolicy-git-08022017-
> > > orig/policy/modules/admin/bootloader.te 2016-12-29
> > > 22:48:16.446818415 +0100
> > > +++ refpolicy-git-08022017/policy/modules/admin/bootloader.te
> > > 2017-02-08 00:14:22.923674773 +0100
> > > @@ -108,6 +108,7 @@ corecmd_exec_all_executables(bootloader_
> > > ?domain_use_interactive_fds(bootloader_t)
> > >
> > > ?files_create_boot_dirs(bootloader_t)
> > > +files_read_boot_files(bootloader_t)
> > > ?files_read_etc_files(bootloader_t)
> > > ?files_read_usr_src_files(bootloader_t)
> > > ?files_read_usr_files(bootloader_t)
> > >?

Regards,

Guido

2017-02-11 20:23:51

by Dac Override

[permalink] [raw]
Subject: [refpolicy] [PATCH] bootloader: add permissions to read boot files in order to generate a configuration file

On 02/11/2017 09:18 PM, Guido Trentalancia via refpolicy wrote:
> Hello.
>
> On Sat, 11/02/2017 at 14.51 -0500, Chris PeBenito wrote:
>> On 02/07/17 21:13, Jason Zaman via refpolicy wrote:
>>>
>>> On Wed, Feb 08, 2017 at 12:32:32AM +0100, Guido Trentalancia via
>>> refpolicy wrote:
>>>>
>>>> Allow the bootloader to read boot files in order to generate
>>>> a configuration file.
>>>>
>>>> Signed-off-by: Guido Trentalancia <[email protected]>
>>>
>>> NACK. this wont work. Just use the patch I posted
>>> http://oss.tresys.com/pipermail/refpolicy/2017-February/009011.html
>>> [PATCH v2] bootloader: grub needs to manage grub.cfg and read
>>> kernels
>>
>> I've decided to revert this patch. A nonfunctional system app like
>> this
>> is not acceptable. I am still open to a change along these lines,
>> though arguably because bootloader has raw disk access, it doesn't
>> matter much if it can overwrite the kernel via normal file access.
>
> The core grub functionality has never stopped working.
>
> The only thing that was not fully functional is an auxiliary
> application shipped with grub (grub-mkconfig) that can be optionally
> used to create the grub configuration file. It basically scans the
> /boot directory for kernel images and creates an entry for each kernel
> image it found.
>
> With the additional very simple patch that has been posted, the above
> mentioned optional functionality works again.

Except when ones uses efi i suppose. since /boot/efi is dosfs_t


>
>>>> ---
>>>> policy/modules/admin/bootloader.te | 1 +
>>>> 1 file changed, 1 insertion(+)
>>>>
>>>> diff -pru refpolicy-git-08022017-
>>>> orig/policy/modules/admin/bootloader.te refpolicy-git-
>>>> 08022017/policy/modules/admin/bootloader.te
>>>> --- refpolicy-git-08022017-
>>>> orig/policy/modules/admin/bootloader.te 2016-12-29
>>>> 22:48:16.446818415 +0100
>>>> +++ refpolicy-git-08022017/policy/modules/admin/bootloader.te
>>>> 2017-02-08 00:14:22.923674773 +0100
>>>> @@ -108,6 +108,7 @@ corecmd_exec_all_executables(bootloader_
>>>> domain_use_interactive_fds(bootloader_t)
>>>>
>>>> files_create_boot_dirs(bootloader_t)
>>>> +files_read_boot_files(bootloader_t)
>>>> files_read_etc_files(bootloader_t)
>>>> files_read_usr_src_files(bootloader_t)
>>>> files_read_usr_files(bootloader_t)
>>>>
>
> Regards,
>
> Guido
> _______________________________________________
> refpolicy mailing list
> refpolicy at oss.tresys.com
> http://oss.tresys.com/mailman/listinfo/refpolicy
>


--
Key fingerprint = 5F4D 3CDB D3F8 3652 FBD8 02D5 3B6C 5F1D 2C7B 6B02
https://sks-keyservers.net/pks/lookup?op=get&search=0x3B6C5F1D2C7B6B02
Dominick Grift

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 648 bytes
Desc: OpenPGP digital signature
Url : http://oss.tresys.com/pipermail/refpolicy/attachments/20170211/9a5006f1/attachment-0001.bin

2017-02-11 20:31:29

by Dac Override

[permalink] [raw]
Subject: [refpolicy] [PATCH] bootloader: add permissions to read boot files in order to generate a configuration file

On 02/11/2017 09:23 PM, Dominick Grift wrote:
> On 02/11/2017 09:18 PM, Guido Trentalancia via refpolicy wrote:
>> Hello.
>>
>> On Sat, 11/02/2017 at 14.51 -0500, Chris PeBenito wrote:
>>> On 02/07/17 21:13, Jason Zaman via refpolicy wrote:
>>>>
>>>> On Wed, Feb 08, 2017 at 12:32:32AM +0100, Guido Trentalancia via
>>>> refpolicy wrote:
>>>>>
>>>>> Allow the bootloader to read boot files in order to generate
>>>>> a configuration file.
>>>>>
>>>>> Signed-off-by: Guido Trentalancia <[email protected]>
>>>>
>>>> NACK. this wont work. Just use the patch I posted
>>>> http://oss.tresys.com/pipermail/refpolicy/2017-February/009011.html
>>>> [PATCH v2] bootloader: grub needs to manage grub.cfg and read
>>>> kernels
>>>
>>> I've decided to revert this patch. A nonfunctional system app like
>>> this
>>> is not acceptable. I am still open to a change along these lines,
>>> though arguably because bootloader has raw disk access, it doesn't
>>> matter much if it can overwrite the kernel via normal file access.
>>
>> The core grub functionality has never stopped working.
>>
>> The only thing that was not fully functional is an auxiliary
>> application shipped with grub (grub-mkconfig) that can be optionally
>> used to create the grub configuration file. It basically scans the
>> /boot directory for kernel images and creates an entry for each kernel
>> image it found.
>>
>> With the additional very simple patch that has been posted, the above
>> mentioned optional functionality works again.
>
> Except when ones uses efi i suppose. since /boot/efi is dosfs_t

anyhow, if bootloader_t (i wonder why it needs that) has raw disk access
then i would suggest we create a separate domain for the grub2-.* utils
since i dont believe they need that

>
>
>>
>>>>> ---
>>>>> policy/modules/admin/bootloader.te | 1 +
>>>>> 1 file changed, 1 insertion(+)
>>>>>
>>>>> diff -pru refpolicy-git-08022017-
>>>>> orig/policy/modules/admin/bootloader.te refpolicy-git-
>>>>> 08022017/policy/modules/admin/bootloader.te
>>>>> --- refpolicy-git-08022017-
>>>>> orig/policy/modules/admin/bootloader.te 2016-12-29
>>>>> 22:48:16.446818415 +0100
>>>>> +++ refpolicy-git-08022017/policy/modules/admin/bootloader.te
>>>>> 2017-02-08 00:14:22.923674773 +0100
>>>>> @@ -108,6 +108,7 @@ corecmd_exec_all_executables(bootloader_
>>>>> domain_use_interactive_fds(bootloader_t)
>>>>>
>>>>> files_create_boot_dirs(bootloader_t)
>>>>> +files_read_boot_files(bootloader_t)
>>>>> files_read_etc_files(bootloader_t)
>>>>> files_read_usr_src_files(bootloader_t)
>>>>> files_read_usr_files(bootloader_t)
>>>>>
>>
>> Regards,
>>
>> Guido
>> _______________________________________________
>> refpolicy mailing list
>> refpolicy at oss.tresys.com
>> http://oss.tresys.com/mailman/listinfo/refpolicy
>>
>
>


--
Key fingerprint = 5F4D 3CDB D3F8 3652 FBD8 02D5 3B6C 5F1D 2C7B 6B02
https://sks-keyservers.net/pks/lookup?op=get&search=0x3B6C5F1D2C7B6B02
Dominick Grift

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 648 bytes
Desc: OpenPGP digital signature
Url : http://oss.tresys.com/pipermail/refpolicy/attachments/20170211/672496db/attachment.bin

2017-02-11 20:47:31

by Dac Override

[permalink] [raw]
Subject: [refpolicy] [PATCH] bootloader: add permissions to read boot files in order to generate a configuration file

On 02/11/2017 09:31 PM, Dominick Grift wrote:
> On 02/11/2017 09:23 PM, Dominick Grift wrote:
>> On 02/11/2017 09:18 PM, Guido Trentalancia via refpolicy wrote:
>>> Hello.
>>>
>>> On Sat, 11/02/2017 at 14.51 -0500, Chris PeBenito wrote:
>>>> On 02/07/17 21:13, Jason Zaman via refpolicy wrote:
>>>>>
>>>>> On Wed, Feb 08, 2017 at 12:32:32AM +0100, Guido Trentalancia via
>>>>> refpolicy wrote:
>>>>>>
>>>>>> Allow the bootloader to read boot files in order to generate
>>>>>> a configuration file.
>>>>>>
>>>>>> Signed-off-by: Guido Trentalancia <[email protected]>
>>>>>
>>>>> NACK. this wont work. Just use the patch I posted
>>>>> http://oss.tresys.com/pipermail/refpolicy/2017-February/009011.html
>>>>> [PATCH v2] bootloader: grub needs to manage grub.cfg and read
>>>>> kernels
>>>>
>>>> I've decided to revert this patch. A nonfunctional system app like
>>>> this
>>>> is not acceptable. I am still open to a change along these lines,
>>>> though arguably because bootloader has raw disk access, it doesn't
>>>> matter much if it can overwrite the kernel via normal file access.
>>>
>>> The core grub functionality has never stopped working.
>>>
>>> The only thing that was not fully functional is an auxiliary
>>> application shipped with grub (grub-mkconfig) that can be optionally
>>> used to create the grub configuration file. It basically scans the
>>> /boot directory for kernel images and creates an entry for each kernel
>>> image it found.
>>>
>>> With the additional very simple patch that has been posted, the above
>>> mentioned optional functionality works again.
>>
>> Except when ones uses efi i suppose. since /boot/efi is dosfs_t
>
> anyhow, if bootloader_t (i wonder why it needs that) has raw disk access
> then i would suggest we create a separate domain for the grub2-.* utils
> since i dont believe they need that

I suppose it might be grub2?-install that needs raw disk access to
install to boot sector

>
>>
>>
>>>
>>>>>> ---
>>>>>> policy/modules/admin/bootloader.te | 1 +
>>>>>> 1 file changed, 1 insertion(+)
>>>>>>
>>>>>> diff -pru refpolicy-git-08022017-
>>>>>> orig/policy/modules/admin/bootloader.te refpolicy-git-
>>>>>> 08022017/policy/modules/admin/bootloader.te
>>>>>> --- refpolicy-git-08022017-
>>>>>> orig/policy/modules/admin/bootloader.te 2016-12-29
>>>>>> 22:48:16.446818415 +0100
>>>>>> +++ refpolicy-git-08022017/policy/modules/admin/bootloader.te
>>>>>> 2017-02-08 00:14:22.923674773 +0100
>>>>>> @@ -108,6 +108,7 @@ corecmd_exec_all_executables(bootloader_
>>>>>> domain_use_interactive_fds(bootloader_t)
>>>>>>
>>>>>> files_create_boot_dirs(bootloader_t)
>>>>>> +files_read_boot_files(bootloader_t)
>>>>>> files_read_etc_files(bootloader_t)
>>>>>> files_read_usr_src_files(bootloader_t)
>>>>>> files_read_usr_files(bootloader_t)
>>>>>>
>>>
>>> Regards,
>>>
>>> Guido
>>> _______________________________________________
>>> refpolicy mailing list
>>> refpolicy at oss.tresys.com
>>> http://oss.tresys.com/mailman/listinfo/refpolicy
>>>
>>
>>
>
>


--
Key fingerprint = 5F4D 3CDB D3F8 3652 FBD8 02D5 3B6C 5F1D 2C7B 6B02
https://sks-keyservers.net/pks/lookup?op=get&search=0x3B6C5F1D2C7B6B02
Dominick Grift

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 648 bytes
Desc: OpenPGP digital signature
Url : http://oss.tresys.com/pipermail/refpolicy/attachments/20170211/cc1a8dde/attachment.bin

2017-02-11 20:51:08

by guido

[permalink] [raw]
Subject: [refpolicy] [PATCH] bootloader: add permissions to read boot files in order to generate a configuration file

It's grub2-bios-setup that needs raw storage write access...

On the 11th of February 2017 21:47:31 CET, Dominick Grift via refpolicy <[email protected]> wrote:
>On 02/11/2017 09:31 PM, Dominick Grift wrote:
>> On 02/11/2017 09:23 PM, Dominick Grift wrote:
>>> On 02/11/2017 09:18 PM, Guido Trentalancia via refpolicy wrote:
>>>> Hello.
>>>>
>>>> On Sat, 11/02/2017 at 14.51 -0500, Chris PeBenito wrote:
>>>>> On 02/07/17 21:13, Jason Zaman via refpolicy wrote:
>>>>>>
>>>>>> On Wed, Feb 08, 2017 at 12:32:32AM +0100, Guido Trentalancia via
>>>>>> refpolicy wrote:
>>>>>>>
>>>>>>> Allow the bootloader to read boot files in order to generate
>>>>>>> a configuration file.
>>>>>>>
>>>>>>> Signed-off-by: Guido Trentalancia <[email protected]>
>>>>>>
>>>>>> NACK. this wont work. Just use the patch I posted
>>>>>>
>http://oss.tresys.com/pipermail/refpolicy/2017-February/009011.html
>>>>>> [PATCH v2] bootloader: grub needs to manage grub.cfg and read
>>>>>> kernels
>>>>>
>>>>> I've decided to revert this patch. A nonfunctional system app
>like
>>>>> this
>>>>> is not acceptable. I am still open to a change along these lines,
>
>>>>> though arguably because bootloader has raw disk access, it doesn't
>
>>>>> matter much if it can overwrite the kernel via normal file access.
>>>>
>>>> The core grub functionality has never stopped working.
>>>>
>>>> The only thing that was not fully functional is an auxiliary
>>>> application shipped with grub (grub-mkconfig) that can be
>optionally
>>>> used to create the grub configuration file. It basically scans the
>>>> /boot directory for kernel images and creates an entry for each
>kernel
>>>> image it found.
>>>>
>>>> With the additional very simple patch that has been posted, the
>above
>>>> mentioned optional functionality works again.
>>>
>>> Except when ones uses efi i suppose. since /boot/efi is dosfs_t
>>
>> anyhow, if bootloader_t (i wonder why it needs that) has raw disk
>access
>> then i would suggest we create a separate domain for the grub2-.*
>utils
>> since i dont believe they need that
>
>I suppose it might be grub2?-install that needs raw disk access to
>install to boot sector
>
>>
>>>
>>>
>>>>
>>>>>>> ---
>>>>>>> policy/modules/admin/bootloader.te | 1 +
>>>>>>> 1 file changed, 1 insertion(+)
>>>>>>>
>>>>>>> diff -pru refpolicy-git-08022017-
>>>>>>> orig/policy/modules/admin/bootloader.te refpolicy-git-
>>>>>>> 08022017/policy/modules/admin/bootloader.te
>>>>>>> --- refpolicy-git-08022017-
>>>>>>> orig/policy/modules/admin/bootloader.te 2016-12-29
>>>>>>> 22:48:16.446818415 +0100
>>>>>>> +++ refpolicy-git-08022017/policy/modules/admin/bootloader.te
>>>>>>> 2017-02-08 00:14:22.923674773 +0100
>>>>>>> @@ -108,6 +108,7 @@ corecmd_exec_all_executables(bootloader_
>>>>>>> domain_use_interactive_fds(bootloader_t)
>>>>>>>
>>>>>>> files_create_boot_dirs(bootloader_t)
>>>>>>> +files_read_boot_files(bootloader_t)
>>>>>>> files_read_etc_files(bootloader_t)
>>>>>>> files_read_usr_src_files(bootloader_t)
>>>>>>> files_read_usr_files(bootloader_t)
>>>>>>>
>>>>
>>>> Regards,
>>>>
>>>> Guido
>>>> _______________________________________________
>>>> refpolicy mailing list
>>>> refpolicy at oss.tresys.com
>>>> http://oss.tresys.com/mailman/listinfo/refpolicy
>>>>
>>>
>>>
>>
>>