2011-02-16 06:22:05

by Guido Trentalancia

[permalink] [raw]
Subject: [refpolicy] [PATCH 18/34]: patch for the policykit module (labeling, start from dbus, read xdm files)

This patch adds a file context for the /var/lib/polkit-1 directory.
It then allows policykit to be started from dbus. It also adds
some other permissions needed to run policykit and a new interface
which is used to read xdm files.

diff -pruN refpolicy-git-02022011-test-apply2/policy/modules/services/policykit.fc refpolicy-git-02022011-test-apply3/policy/modules/services/policykit.fc
--- refpolicy-git-02022011-test-apply2/policy/modules/services/policykit.fc 2011-01-08 19:07:21.280747356 +0100
+++ refpolicy-git-02022011-test-apply3/policy/modules/services/policykit.fc 2011-02-07 03:31:53.547856778 +0100
@@ -11,5 +11,6 @@
/var/lib/misc/PolicyKit.reload gen_context(system_u:object_r:policykit_reload_t,s0)
/var/lib/PolicyKit(/.*)? gen_context(system_u:object_r:policykit_var_lib_t,s0)
/var/lib/PolicyKit-public(/.*)? gen_context(system_u:object_r:policykit_var_lib_t,s0)
+/var/lib/polkit-1(/.*)? gen_context(system_u:object_r:policykit_var_lib_t,s0)
/var/run/PolicyKit(/.*)? gen_context(system_u:object_r:policykit_var_run_t,s0)

diff -pruN refpolicy-git-02022011-test-apply2/policy/modules/services/policykit.te refpolicy-git-02022011-test-apply3/policy/modules/services/policykit.te
--- refpolicy-git-02022011-test-apply2/policy/modules/services/policykit.te 2011-02-07 03:31:24.763790944 +0100
+++ refpolicy-git-02022011-test-apply3/policy/modules/services/policykit.te 2011-02-07 03:31:53.550857306 +0100
@@ -35,8 +35,8 @@ files_pid_file(policykit_var_run_t)
# policykit local policy
#

-allow policykit_t self:capability { setgid setuid };
-allow policykit_t self:process getattr;
+allow policykit_t self:capability { setgid setuid sys_ptrace };
+allow policykit_t self:process { getattr getsched signal };
allow policykit_t self:fifo_file rw_file_perms;
allow policykit_t self:unix_dgram_socket create_socket_perms;
allow policykit_t self:unix_stream_socket create_stream_socket_perms;
@@ -57,6 +57,7 @@ manage_files_pattern(policykit_t, policy
files_pid_filetrans(policykit_t, policykit_var_run_t, { file dir })

kernel_read_kernel_sysctls(policykit_t)
+kernel_read_system_state(policykit_t)

files_read_etc_files(policykit_t)
files_read_usr_files(policykit_t)
@@ -78,6 +79,14 @@ optional_policy(`
gnome_read_config(policykit_t)
')

+optional_policy(`
+ dbus_system_domain(policykit_t, policykit_exec_t)
+')
+
+optional_policy(`
+ xserver_read_xdm_files(policykit_t)
+')
+
########################################
#
# polkit_auth local policy
diff -pruN refpolicy-git-02022011-test-apply2/policy/modules/services/xserver.if refpolicy-git-02022011-test-apply3/policy/modules/services/xserver.if
--- refpolicy-git-02022011-test-apply2/policy/modules/services/xserver.if 2011-01-08 19:07:21.344757464 +0100
+++ refpolicy-git-02022011-test-apply3/policy/modules/services/xserver.if 2011-02-07 03:31:53.552857658 +0100
@@ -638,6 +638,25 @@ interface(`xserver_rw_console',`

########################################
## <summary>
+## Read xdm files.
+## </summary>
+## <param name="domain">
+## <summary>
+## Domain allowed access.
+## </summary>
+## </param>
+#
+interface(`xserver_read_xdm_files',`
+ gen_require(`
+ type xdm_t;
+ ')
+
+ allow $1 xdm_t:dir list_dir_perms;
+ read_files_pattern($1, xdm_t, xdm_t)
+')
+
+########################################
+## <summary>
## Use file descriptors for xdm.
## </summary>
## <param name="domain">


2011-02-28 13:56:18

by cpebenito

[permalink] [raw]
Subject: [refpolicy] [PATCH 18/34]: patch for the policykit module (labeling, start from dbus, read xdm files)

On 02/16/11 01:22, Guido Trentalancia wrote:
> This patch adds a file context for the /var/lib/polkit-1 directory.
> It then allows policykit to be started from dbus. It also adds
> some other permissions needed to run policykit and a new interface
> which is used to read xdm files.
>
> diff -pruN refpolicy-git-02022011-test-apply2/policy/modules/services/policykit.fc refpolicy-git-02022011-test-apply3/policy/modules/services/policykit.fc
> --- refpolicy-git-02022011-test-apply2/policy/modules/services/policykit.fc 2011-01-08 19:07:21.280747356 +0100
> +++ refpolicy-git-02022011-test-apply3/policy/modules/services/policykit.fc 2011-02-07 03:31:53.547856778 +0100
> @@ -11,5 +11,6 @@
> /var/lib/misc/PolicyKit.reload gen_context(system_u:object_r:policykit_reload_t,s0)
> /var/lib/PolicyKit(/.*)? gen_context(system_u:object_r:policykit_var_lib_t,s0)
> /var/lib/PolicyKit-public(/.*)? gen_context(system_u:object_r:policykit_var_lib_t,s0)
> +/var/lib/polkit-1(/.*)? gen_context(system_u:object_r:policykit_var_lib_t,s0)
> /var/run/PolicyKit(/.*)? gen_context(system_u:object_r:policykit_var_run_t,s0)
>
> diff -pruN refpolicy-git-02022011-test-apply2/policy/modules/services/policykit.te refpolicy-git-02022011-test-apply3/policy/modules/services/policykit.te
> --- refpolicy-git-02022011-test-apply2/policy/modules/services/policykit.te 2011-02-07 03:31:24.763790944 +0100
> +++ refpolicy-git-02022011-test-apply3/policy/modules/services/policykit.te 2011-02-07 03:31:53.550857306 +0100
> @@ -35,8 +35,8 @@ files_pid_file(policykit_var_run_t)
> # policykit local policy
> #
>
> -allow policykit_t self:capability { setgid setuid };
> -allow policykit_t self:process getattr;
> +allow policykit_t self:capability { setgid setuid sys_ptrace };

This sys_ptrace is highly questionable.

> +allow policykit_t self:process { getattr getsched signal };
> allow policykit_t self:fifo_file rw_file_perms;
> allow policykit_t self:unix_dgram_socket create_socket_perms;
> allow policykit_t self:unix_stream_socket create_stream_socket_perms;
> @@ -57,6 +57,7 @@ manage_files_pattern(policykit_t, policy
> files_pid_filetrans(policykit_t, policykit_var_run_t, { file dir })
>
> kernel_read_kernel_sysctls(policykit_t)
> +kernel_read_system_state(policykit_t)
>
> files_read_etc_files(policykit_t)
> files_read_usr_files(policykit_t)
> @@ -78,6 +79,14 @@ optional_policy(`
> gnome_read_config(policykit_t)
> ')
>
> +optional_policy(`
> + dbus_system_domain(policykit_t, policykit_exec_t)
> +')
> +
> +optional_policy(`
> + xserver_read_xdm_files(policykit_t)
> +')
> +
> ########################################
> #
> # polkit_auth local policy
> diff -pruN refpolicy-git-02022011-test-apply2/policy/modules/services/xserver.if refpolicy-git-02022011-test-apply3/policy/modules/services/xserver.if
> --- refpolicy-git-02022011-test-apply2/policy/modules/services/xserver.if 2011-01-08 19:07:21.344757464 +0100
> +++ refpolicy-git-02022011-test-apply3/policy/modules/services/xserver.if 2011-02-07 03:31:53.552857658 +0100
> @@ -638,6 +638,25 @@ interface(`xserver_rw_console',`
>
> ########################################
> ## <summary>
> +## Read xdm files.
> +## </summary>
> +## <param name="domain">
> +## <summary>
> +## Domain allowed access.
> +## </summary>
> +## </param>
> +#
> +interface(`xserver_read_xdm_files',`
> + gen_require(`
> + type xdm_t;
> + ')
> +
> + allow $1 xdm_t:dir list_dir_perms;
> + read_files_pattern($1, xdm_t, xdm_t)
> +')
> +
> +########################################
> +## <summary>
> ## Use file descriptors for xdm.
> ## </summary>
> ## <param name="domain">

--
Chris PeBenito
Tresys Technology, LLC
http://www.tresys.com | oss.tresys.com

2011-02-28 15:28:29

by Daniel Walsh

[permalink] [raw]
Subject: [refpolicy] [PATCH 18/34]: patch for the policykit module (labeling, start from dbus, read xdm files)

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 02/28/2011 08:56 AM, Christopher J. PeBenito wrote:
> On 02/16/11 01:22, Guido Trentalancia wrote:
>> This patch adds a file context for the /var/lib/polkit-1 directory.
>> It then allows policykit to be started from dbus. It also adds
>> some other permissions needed to run policykit and a new interface
>> which is used to read xdm files.
>>
>> diff -pruN refpolicy-git-02022011-test-apply2/policy/modules/services/policykit.fc refpolicy-git-02022011-test-apply3/policy/modules/services/policykit.fc
>> --- refpolicy-git-02022011-test-apply2/policy/modules/services/policykit.fc 2011-01-08 19:07:21.280747356 +0100
>> +++ refpolicy-git-02022011-test-apply3/policy/modules/services/policykit.fc 2011-02-07 03:31:53.547856778 +0100
>> @@ -11,5 +11,6 @@
>> /var/lib/misc/PolicyKit.reload gen_context(system_u:object_r:policykit_reload_t,s0)
>> /var/lib/PolicyKit(/.*)? gen_context(system_u:object_r:policykit_var_lib_t,s0)
>> /var/lib/PolicyKit-public(/.*)? gen_context(system_u:object_r:policykit_var_lib_t,s0)
>> +/var/lib/polkit-1(/.*)? gen_context(system_u:object_r:policykit_var_lib_t,s0)
>> /var/run/PolicyKit(/.*)? gen_context(system_u:object_r:policykit_var_run_t,s0)
>>
>> diff -pruN refpolicy-git-02022011-test-apply2/policy/modules/services/policykit.te refpolicy-git-02022011-test-apply3/policy/modules/services/policykit.te
>> --- refpolicy-git-02022011-test-apply2/policy/modules/services/policykit.te 2011-02-07 03:31:24.763790944 +0100
>> +++ refpolicy-git-02022011-test-apply3/policy/modules/services/policykit.te 2011-02-07 03:31:53.550857306 +0100
>> @@ -35,8 +35,8 @@ files_pid_file(policykit_var_run_t)
>> # policykit local policy
>> #
>>
>> -allow policykit_t self:capability { setgid setuid };
>> -allow policykit_t self:process getattr;
>> +allow policykit_t self:capability { setgid setuid sys_ptrace };
>
> This sys_ptrace is highly questionable.
>

We have this in Fedora. I believe policykit is examining the /proc
entry of applications and this causes the sys_ptrace. Maybe reading
/proc/PID/cmdline.


- --- snip ----
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/

iEYEARECAAYFAk1rvxwACgkQrlYvE4MpobONWQCfd5tKz7QZhJQuQvmRYtJ9peyS
yLYAoNcMMc8z3oWAcPnMR33Fw6xwlwhR
=Q84e
-----END PGP SIGNATURE-----

2011-02-28 19:07:50

by Guido Trentalancia

[permalink] [raw]
Subject: [refpolicy] [PATCH 18/34]: patch for the policykit module (labeling, start from dbus, read xdm files)

On Mon, 28/02/2011 at 08.56 -0500, Christopher J. PeBenito wrote:
> On 02/16/11 01:22, Guido Trentalancia wrote:
> > This patch adds a file context for the /var/lib/polkit-1 directory.
> > It then allows policykit to be started from dbus. It also adds
> > some other permissions needed to run policykit and a new interface
> > which is used to read xdm files.
> >
> > diff -pruN refpolicy-git-02022011-test-apply2/policy/modules/services/policykit.fc refpolicy-git-02022011-test-apply3/policy/modules/services/policykit.fc
> > --- refpolicy-git-02022011-test-apply2/policy/modules/services/policykit.fc 2011-01-08 19:07:21.280747356 +0100
> > +++ refpolicy-git-02022011-test-apply3/policy/modules/services/policykit.fc 2011-02-07 03:31:53.547856778 +0100
> > @@ -11,5 +11,6 @@
> > /var/lib/misc/PolicyKit.reload gen_context(system_u:object_r:policykit_reload_t,s0)
> > /var/lib/PolicyKit(/.*)? gen_context(system_u:object_r:policykit_var_lib_t,s0)
> > /var/lib/PolicyKit-public(/.*)? gen_context(system_u:object_r:policykit_var_lib_t,s0)
> > +/var/lib/polkit-1(/.*)? gen_context(system_u:object_r:policykit_var_lib_t,s0)
> > /var/run/PolicyKit(/.*)? gen_context(system_u:object_r:policykit_var_run_t,s0)
> >
> > diff -pruN refpolicy-git-02022011-test-apply2/policy/modules/services/policykit.te refpolicy-git-02022011-test-apply3/policy/modules/services/policykit.te
> > --- refpolicy-git-02022011-test-apply2/policy/modules/services/policykit.te 2011-02-07 03:31:24.763790944 +0100
> > +++ refpolicy-git-02022011-test-apply3/policy/modules/services/policykit.te 2011-02-07 03:31:53.550857306 +0100
> > @@ -35,8 +35,8 @@ files_pid_file(policykit_var_run_t)
> > # policykit local policy
> > #
> >
> > -allow policykit_t self:capability { setgid setuid };
> > -allow policykit_t self:process getattr;
> > +allow policykit_t self:capability { setgid setuid sys_ptrace };
>
> This sys_ptrace is highly questionable.

Could that be due to calls to the following functions:

sigemptyset()
sigaddset()
sigprocmask()

There are no calls to ptrace() and this is not due to
reading /proc/PID/cmdline.

In truth I can only check if this is critical for policykit.

> > +allow policykit_t self:process { getattr getsched signal };
> > allow policykit_t self:fifo_file rw_file_perms;
> > allow policykit_t self:unix_dgram_socket create_socket_perms;
> > allow policykit_t self:unix_stream_socket create_stream_socket_perms;
> > @@ -57,6 +57,7 @@ manage_files_pattern(policykit_t, policy
> > files_pid_filetrans(policykit_t, policykit_var_run_t, { file dir })
> >
> > kernel_read_kernel_sysctls(policykit_t)
> > +kernel_read_system_state(policykit_t)
> >
> > files_read_etc_files(policykit_t)
> > files_read_usr_files(policykit_t)
> > @@ -78,6 +79,14 @@ optional_policy(`
> > gnome_read_config(policykit_t)
> > ')
> >
> > +optional_policy(`
> > + dbus_system_domain(policykit_t, policykit_exec_t)
> > +')
> > +
> > +optional_policy(`
> > + xserver_read_xdm_files(policykit_t)
> > +')
> > +
> > ########################################
> > #
> > # polkit_auth local policy
> > diff -pruN refpolicy-git-02022011-test-apply2/policy/modules/services/xserver.if refpolicy-git-02022011-test-apply3/policy/modules/services/xserver.if
> > --- refpolicy-git-02022011-test-apply2/policy/modules/services/xserver.if 2011-01-08 19:07:21.344757464 +0100
> > +++ refpolicy-git-02022011-test-apply3/policy/modules/services/xserver.if 2011-02-07 03:31:53.552857658 +0100
> > @@ -638,6 +638,25 @@ interface(`xserver_rw_console',`
> >
> > ########################################
> > ## <summary>
> > +## Read xdm files.
> > +## </summary>
> > +## <param name="domain">
> > +## <summary>
> > +## Domain allowed access.
> > +## </summary>
> > +## </param>
> > +#
> > +interface(`xserver_read_xdm_files',`
> > + gen_require(`
> > + type xdm_t;
> > + ')
> > +
> > + allow $1 xdm_t:dir list_dir_perms;
> > + read_files_pattern($1, xdm_t, xdm_t)
> > +')
> > +
> > +########################################
> > +## <summary>
> > ## Use file descriptors for xdm.
> > ## </summary>
> > ## <param name="domain">
>

2011-03-01 19:12:00

by cpebenito

[permalink] [raw]
Subject: [refpolicy] [PATCH 18/34]: patch for the policykit module (labeling, start from dbus, read xdm files)

On 02/28/11 14:07, Guido Trentalancia wrote:
> On Mon, 28/02/2011 at 08.56 -0500, Christopher J. PeBenito wrote:
>> On 02/16/11 01:22, Guido Trentalancia wrote:
>>> This patch adds a file context for the /var/lib/polkit-1 directory.
>>> It then allows policykit to be started from dbus. It also adds
>>> some other permissions needed to run policykit and a new interface
>>> which is used to read xdm files.
>>>
>>> diff -pruN refpolicy-git-02022011-test-apply2/policy/modules/services/policykit.fc refpolicy-git-02022011-test-apply3/policy/modules/services/policykit.fc
>>> --- refpolicy-git-02022011-test-apply2/policy/modules/services/policykit.fc 2011-01-08 19:07:21.280747356 +0100
>>> +++ refpolicy-git-02022011-test-apply3/policy/modules/services/policykit.fc 2011-02-07 03:31:53.547856778 +0100
>>> @@ -11,5 +11,6 @@
>>> /var/lib/misc/PolicyKit.reload gen_context(system_u:object_r:policykit_reload_t,s0)
>>> /var/lib/PolicyKit(/.*)? gen_context(system_u:object_r:policykit_var_lib_t,s0)
>>> /var/lib/PolicyKit-public(/.*)? gen_context(system_u:object_r:policykit_var_lib_t,s0)
>>> +/var/lib/polkit-1(/.*)? gen_context(system_u:object_r:policykit_var_lib_t,s0)
>>> /var/run/PolicyKit(/.*)? gen_context(system_u:object_r:policykit_var_run_t,s0)
>>>
>>> diff -pruN refpolicy-git-02022011-test-apply2/policy/modules/services/policykit.te refpolicy-git-02022011-test-apply3/policy/modules/services/policykit.te
>>> --- refpolicy-git-02022011-test-apply2/policy/modules/services/policykit.te 2011-02-07 03:31:24.763790944 +0100
>>> +++ refpolicy-git-02022011-test-apply3/policy/modules/services/policykit.te 2011-02-07 03:31:53.550857306 +0100
>>> @@ -35,8 +35,8 @@ files_pid_file(policykit_var_run_t)
>>> # policykit local policy
>>> #
>>>
>>> -allow policykit_t self:capability { setgid setuid };
>>> -allow policykit_t self:process getattr;
>>> +allow policykit_t self:capability { setgid setuid sys_ptrace };
>>
>> This sys_ptrace is highly questionable.
>
> Could that be due to calls to the following functions:
>
> sigemptyset()
> sigaddset()
> sigprocmask()
>
> There are no calls to ptrace() and this is not due to
> reading /proc/PID/cmdline.
>
> In truth I can only check if this is critical for policykit.

dontauditing this doesn't work?

--
Chris PeBenito
Tresys Technology, LLC
http://www.tresys.com | oss.tresys.com

2011-03-01 22:47:27

by Guido Trentalancia

[permalink] [raw]
Subject: [refpolicy] [PATCH 18/34]: patch for the policykit module (labeling, start from dbus, read xdm files)

On Tue, 01/03/2011 at 14.12 -0500, Christopher J. PeBenito wrote:
> On 02/28/11 14:07, Guido Trentalancia wrote:
> > On Mon, 28/02/2011 at 08.56 -0500, Christopher J. PeBenito wrote:
> >> On 02/16/11 01:22, Guido Trentalancia wrote:
> >>> This patch adds a file context for the /var/lib/polkit-1 directory.
> >>> It then allows policykit to be started from dbus. It also adds
> >>> some other permissions needed to run policykit and a new interface
> >>> which is used to read xdm files.
> >>>
> >>> diff -pruN refpolicy-git-02022011-test-apply2/policy/modules/services/policykit.fc refpolicy-git-02022011-test-apply3/policy/modules/services/policykit.fc
> >>> --- refpolicy-git-02022011-test-apply2/policy/modules/services/policykit.fc 2011-01-08 19:07:21.280747356 +0100
> >>> +++ refpolicy-git-02022011-test-apply3/policy/modules/services/policykit.fc 2011-02-07 03:31:53.547856778 +0100
> >>> @@ -11,5 +11,6 @@
> >>> /var/lib/misc/PolicyKit.reload gen_context(system_u:object_r:policykit_reload_t,s0)
> >>> /var/lib/PolicyKit(/.*)? gen_context(system_u:object_r:policykit_var_lib_t,s0)
> >>> /var/lib/PolicyKit-public(/.*)? gen_context(system_u:object_r:policykit_var_lib_t,s0)
> >>> +/var/lib/polkit-1(/.*)? gen_context(system_u:object_r:policykit_var_lib_t,s0)
> >>> /var/run/PolicyKit(/.*)? gen_context(system_u:object_r:policykit_var_run_t,s0)
> >>>
> >>> diff -pruN refpolicy-git-02022011-test-apply2/policy/modules/services/policykit.te refpolicy-git-02022011-test-apply3/policy/modules/services/policykit.te
> >>> --- refpolicy-git-02022011-test-apply2/policy/modules/services/policykit.te 2011-02-07 03:31:24.763790944 +0100
> >>> +++ refpolicy-git-02022011-test-apply3/policy/modules/services/policykit.te 2011-02-07 03:31:53.550857306 +0100
> >>> @@ -35,8 +35,8 @@ files_pid_file(policykit_var_run_t)
> >>> # policykit local policy
> >>> #
> >>>
> >>> -allow policykit_t self:capability { setgid setuid };
> >>> -allow policykit_t self:process getattr;
> >>> +allow policykit_t self:capability { setgid setuid sys_ptrace };
> >>
> >> This sys_ptrace is highly questionable.
> >
> > Could that be due to calls to the following functions:
> >
> > sigemptyset()
> > sigaddset()
> > sigprocmask()
> >
> > There are no calls to ptrace() and this is not due to
> > reading /proc/PID/cmdline.
> >
> > In truth I can only check if this is critical for policykit.
>
> dontauditing this doesn't work?

dontaudit will just shut it up. If it is not critical we could do that,
but *what's the point of hiding stuff under the carpet* ?

Your reply "[PATCH 11/34]: patch to allow consolekit shutdown the
system" timestamped Tue, 01 Mar 2011 14:18:01 -0500 gives a practical
example of the fact that dontaudit can have side effects even for a
maintainer.

If one works everyday with the policy such side effects probably have a
minimal impact because, as soon as something goes wrong, he or she knows
that if there are no AVCs then he or she has to track down the dontaudit
rules.

But for somebody that barely knows his or her system has a security
framework and a security policy, these "side-effects" could turn into
real "blockers". For example, he or she manages to find out that the
problem is due to SELinux, then manages to find out that he/she needs to
check the audit logs, but at the end he/she doesn't find anything there
and just gets confused.

For a distribution it's different. Everything is pre-packaged and
tested. But here we are discussing the reference policy.

In the context of the reference policy it could make some sense only in
the context of well known "leaks". But this doesn't appear to be the
case...

Anyway, at the moment I cannot test, but I will let you know as soon as
possible. If you really want to go for that solution, there are good
chances that it can be done because there are good chances that this is
not critical for policykit.

If in the meanwhile you want to have a closer look at the cause, then
please fetch policykit versions 0.99 and 0.100 (latest). It's polkitd
(it has a separate directory and it's just a couple of short source
files).

Regards,

Guido

2011-03-02 13:51:38

by cpebenito

[permalink] [raw]
Subject: [refpolicy] [PATCH 18/34]: patch for the policykit module (labeling, start from dbus, read xdm files)

On 03/01/11 17:47, Guido Trentalancia wrote:
> On Tue, 01/03/2011 at 14.12 -0500, Christopher J. PeBenito wrote:
>> On 02/28/11 14:07, Guido Trentalancia wrote:
>>> On Mon, 28/02/2011 at 08.56 -0500, Christopher J. PeBenito wrote:
>>>> On 02/16/11 01:22, Guido Trentalancia wrote:
>>>>> This patch adds a file context for the /var/lib/polkit-1 directory.
>>>>> It then allows policykit to be started from dbus. It also adds
>>>>> some other permissions needed to run policykit and a new interface
>>>>> which is used to read xdm files.
>>>>>
>>>>> diff -pruN refpolicy-git-02022011-test-apply2/policy/modules/services/policykit.fc refpolicy-git-02022011-test-apply3/policy/modules/services/policykit.fc
>>>>> --- refpolicy-git-02022011-test-apply2/policy/modules/services/policykit.fc 2011-01-08 19:07:21.280747356 +0100
>>>>> +++ refpolicy-git-02022011-test-apply3/policy/modules/services/policykit.fc 2011-02-07 03:31:53.547856778 +0100
>>>>> @@ -11,5 +11,6 @@
>>>>> /var/lib/misc/PolicyKit.reload gen_context(system_u:object_r:policykit_reload_t,s0)
>>>>> /var/lib/PolicyKit(/.*)? gen_context(system_u:object_r:policykit_var_lib_t,s0)
>>>>> /var/lib/PolicyKit-public(/.*)? gen_context(system_u:object_r:policykit_var_lib_t,s0)
>>>>> +/var/lib/polkit-1(/.*)? gen_context(system_u:object_r:policykit_var_lib_t,s0)
>>>>> /var/run/PolicyKit(/.*)? gen_context(system_u:object_r:policykit_var_run_t,s0)
>>>>>
>>>>> diff -pruN refpolicy-git-02022011-test-apply2/policy/modules/services/policykit.te refpolicy-git-02022011-test-apply3/policy/modules/services/policykit.te
>>>>> --- refpolicy-git-02022011-test-apply2/policy/modules/services/policykit.te 2011-02-07 03:31:24.763790944 +0100
>>>>> +++ refpolicy-git-02022011-test-apply3/policy/modules/services/policykit.te 2011-02-07 03:31:53.550857306 +0100
>>>>> @@ -35,8 +35,8 @@ files_pid_file(policykit_var_run_t)
>>>>> # policykit local policy
>>>>> #
>>>>>
>>>>> -allow policykit_t self:capability { setgid setuid };
>>>>> -allow policykit_t self:process getattr;
>>>>> +allow policykit_t self:capability { setgid setuid sys_ptrace };
>>>>
>>>> This sys_ptrace is highly questionable.
>>>
>>> Could that be due to calls to the following functions:
>>>
>>> sigemptyset()
>>> sigaddset()
>>> sigprocmask()
>>>
>>> There are no calls to ptrace() and this is not due to
>>> reading /proc/PID/cmdline.
>>>
>>> In truth I can only check if this is critical for policykit.
>>
>> dontauditing this doesn't work?
>
> dontaudit will just shut it up. If it is not critical we could do that,
> but *what's the point of hiding stuff under the carpet* ?
>
> Your reply "[PATCH 11/34]: patch to allow consolekit shutdown the
> system" timestamped Tue, 01 Mar 2011 14:18:01 -0500 gives a practical
> example of the fact that dontaudit can have side effects even for a
> maintainer.
>
> If one works everyday with the policy such side effects probably have a
> minimal impact because, as soon as something goes wrong, he or she knows
> that if there are no AVCs then he or she has to track down the dontaudit
> rules.
>
> But for somebody that barely knows his or her system has a security
> framework and a security policy, these "side-effects" could turn into
> real "blockers". For example, he or she manages to find out that the
> problem is due to SELinux, then manages to find out that he/she needs to
> check the audit logs, but at the end he/she doesn't find anything there
> and just gets confused.
>
> For a distribution it's different. Everything is pre-packaged and
> tested. But here we are discussing the reference policy.
>
> In the context of the reference policy it could make some sense only in
> the context of well known "leaks". But this doesn't appear to be the
> case...

In general, anything that is not required should be denied. We do not
want to fill up the logs with extraneous denial messages, so we
dontaudit them. If there is a question of denials being suppressed by
dontaudits, thats why we have semodule -D.

--
Chris PeBenito
Tresys Technology, LLC
http://www.tresys.com | oss.tresys.com

2011-03-02 14:47:03

by sven.vermeulen

[permalink] [raw]
Subject: [refpolicy] [PATCH 18/34]: patch for the policykit module (labeling, start from dbus, read xdm files)

On Wed, Mar 02, 2011 at 08:51:38AM -0500, Christopher J. PeBenito wrote:
[... About when dontaudit is advantageous ...]
> In general, anything that is not required should be denied. We do not
> want to fill up the logs with extraneous denial messages, so we
> dontaudit them. If there is a question of denials being suppressed by
> dontaudits, thats why we have semodule -D.

Yup.

What we do within Gentoo Hardened during the policy development is to
embrace the dontaudits that we *think* can be used within a tunable
(gentoo_try_dontaudit). It allows us and testers to validate if the policy
is good (and doesn't fill the audit logs with unnecessary denials) but still
leave some granularity (the boolean) before really disabling all dontaudit
statements.

Eventually, when we're more confident that the dontaudit is really
opportunistic, then we'll remove it from the tunable and also suggest it for
inclusion in the reference policy.

Wkr,
Sven Vermeulen

2011-03-02 19:55:58

by Guido Trentalancia

[permalink] [raw]
Subject: [refpolicy] [PATCH 18/34]: patch for the policykit module (labeling, start from dbus, read xdm files)

On Wed, 02/03/2011 at 08.51 -0500, Christopher J. PeBenito wrote:
> On 03/01/11 17:47, Guido Trentalancia wrote:
> > On Tue, 01/03/2011 at 14.12 -0500, Christopher J. PeBenito wrote:
> >> On 02/28/11 14:07, Guido Trentalancia wrote:
> >>> On Mon, 28/02/2011 at 08.56 -0500, Christopher J. PeBenito wrote:
> >>>> On 02/16/11 01:22, Guido Trentalancia wrote:
> >>>>> This patch adds a file context for the /var/lib/polkit-1 directory.
> >>>>> It then allows policykit to be started from dbus. It also adds
> >>>>> some other permissions needed to run policykit and a new interface
> >>>>> which is used to read xdm files.
> >>>>>
> >>>>> diff -pruN refpolicy-git-02022011-test-apply2/policy/modules/services/policykit.fc refpolicy-git-02022011-test-apply3/policy/modules/services/policykit.fc
> >>>>> --- refpolicy-git-02022011-test-apply2/policy/modules/services/policykit.fc 2011-01-08 19:07:21.280747356 +0100
> >>>>> +++ refpolicy-git-02022011-test-apply3/policy/modules/services/policykit.fc 2011-02-07 03:31:53.547856778 +0100
> >>>>> @@ -11,5 +11,6 @@
> >>>>> /var/lib/misc/PolicyKit.reload gen_context(system_u:object_r:policykit_reload_t,s0)
> >>>>> /var/lib/PolicyKit(/.*)? gen_context(system_u:object_r:policykit_var_lib_t,s0)
> >>>>> /var/lib/PolicyKit-public(/.*)? gen_context(system_u:object_r:policykit_var_lib_t,s0)
> >>>>> +/var/lib/polkit-1(/.*)? gen_context(system_u:object_r:policykit_var_lib_t,s0)
> >>>>> /var/run/PolicyKit(/.*)? gen_context(system_u:object_r:policykit_var_run_t,s0)
> >>>>>
> >>>>> diff -pruN refpolicy-git-02022011-test-apply2/policy/modules/services/policykit.te refpolicy-git-02022011-test-apply3/policy/modules/services/policykit.te
> >>>>> --- refpolicy-git-02022011-test-apply2/policy/modules/services/policykit.te 2011-02-07 03:31:24.763790944 +0100
> >>>>> +++ refpolicy-git-02022011-test-apply3/policy/modules/services/policykit.te 2011-02-07 03:31:53.550857306 +0100
> >>>>> @@ -35,8 +35,8 @@ files_pid_file(policykit_var_run_t)
> >>>>> # policykit local policy
> >>>>> #
> >>>>>
> >>>>> -allow policykit_t self:capability { setgid setuid };
> >>>>> -allow policykit_t self:process getattr;
> >>>>> +allow policykit_t self:capability { setgid setuid sys_ptrace };
> >>>>
> >>>> This sys_ptrace is highly questionable.
> >>>
> >>> Could that be due to calls to the following functions:
> >>>
> >>> sigemptyset()
> >>> sigaddset()
> >>> sigprocmask()
> >>>
> >>> There are no calls to ptrace() and this is not due to
> >>> reading /proc/PID/cmdline.
> >>>
> >>> In truth I can only check if this is critical for policykit.
> >>
> >> dontauditing this doesn't work?
> >
> > dontaudit will just shut it up. If it is not critical we could do that,
> > but *what's the point of hiding stuff under the carpet* ?
> >
> > Your reply "[PATCH 11/34]: patch to allow consolekit shutdown the
> > system" timestamped Tue, 01 Mar 2011 14:18:01 -0500 gives a practical
> > example of the fact that dontaudit can have side effects even for a
> > maintainer.
> >
> > If one works everyday with the policy such side effects probably have a
> > minimal impact because, as soon as something goes wrong, he or she knows
> > that if there are no AVCs then he or she has to track down the dontaudit
> > rules.
> >
> > But for somebody that barely knows his or her system has a security
> > framework and a security policy, these "side-effects" could turn into
> > real "blockers". For example, he or she manages to find out that the
> > problem is due to SELinux, then manages to find out that he/she needs to
> > check the audit logs, but at the end he/she doesn't find anything there
> > and just gets confused.
> >
> > For a distribution it's different. Everything is pre-packaged and
> > tested. But here we are discussing the reference policy.
> >
> > In the context of the reference policy it could make some sense only in
> > the context of well known "leaks". But this doesn't appear to be the
> > case...
>
> In general, anything that is not required should be denied. We do not
> want to fill up the logs with extraneous denial messages, so we
> dontaudit them. If there is a question of denials being suppressed by
> dontaudits, thats why we have semodule -D.

I forgot to add that it's not flooding logs. It logs that only once upon
starting up.

But I will check if it is critical to normal policykit operation as soon
as possible.

Regards,

Guido

2011-03-03 13:28:27

by cpebenito

[permalink] [raw]
Subject: [refpolicy] [PATCH 18/34]: patch for the policykit module (labeling, start from dbus, read xdm files)

On 3/2/2011 2:55 PM, Guido Trentalancia wrote:
> On Wed, 02/03/2011 at 08.51 -0500, Christopher J. PeBenito wrote:
>> On 03/01/11 17:47, Guido Trentalancia wrote:
>>> On Tue, 01/03/2011 at 14.12 -0500, Christopher J. PeBenito wrote:
>>>> On 02/28/11 14:07, Guido Trentalancia wrote:
>>>>> On Mon, 28/02/2011 at 08.56 -0500, Christopher J. PeBenito wrote:
>>>>>> On 02/16/11 01:22, Guido Trentalancia wrote:
>>>>>>> This patch adds a file context for the /var/lib/polkit-1 directory.
>>>>>>> It then allows policykit to be started from dbus. It also adds
>>>>>>> some other permissions needed to run policykit and a new interface
>>>>>>> which is used to read xdm files.
>>>>>>>
>>>>>>> diff -pruN refpolicy-git-02022011-test-apply2/policy/modules/services/policykit.fc refpolicy-git-02022011-test-apply3/policy/modules/services/policykit.fc
>>>>>>> --- refpolicy-git-02022011-test-apply2/policy/modules/services/policykit.fc 2011-01-08 19:07:21.280747356 +0100
>>>>>>> +++ refpolicy-git-02022011-test-apply3/policy/modules/services/policykit.fc 2011-02-07 03:31:53.547856778 +0100
>>>>>>> @@ -11,5 +11,6 @@
>>>>>>> /var/lib/misc/PolicyKit.reload gen_context(system_u:object_r:policykit_reload_t,s0)
>>>>>>> /var/lib/PolicyKit(/.*)? gen_context(system_u:object_r:policykit_var_lib_t,s0)
>>>>>>> /var/lib/PolicyKit-public(/.*)? gen_context(system_u:object_r:policykit_var_lib_t,s0)
>>>>>>> +/var/lib/polkit-1(/.*)? gen_context(system_u:object_r:policykit_var_lib_t,s0)
>>>>>>> /var/run/PolicyKit(/.*)? gen_context(system_u:object_r:policykit_var_run_t,s0)
>>>>>>>
>>>>>>> diff -pruN refpolicy-git-02022011-test-apply2/policy/modules/services/policykit.te refpolicy-git-02022011-test-apply3/policy/modules/services/policykit.te
>>>>>>> --- refpolicy-git-02022011-test-apply2/policy/modules/services/policykit.te 2011-02-07 03:31:24.763790944 +0100
>>>>>>> +++ refpolicy-git-02022011-test-apply3/policy/modules/services/policykit.te 2011-02-07 03:31:53.550857306 +0100
>>>>>>> @@ -35,8 +35,8 @@ files_pid_file(policykit_var_run_t)
>>>>>>> # policykit local policy
>>>>>>> #
>>>>>>>
>>>>>>> -allow policykit_t self:capability { setgid setuid };
>>>>>>> -allow policykit_t self:process getattr;
>>>>>>> +allow policykit_t self:capability { setgid setuid sys_ptrace };
>>>>>>
>>>>>> This sys_ptrace is highly questionable.
>>>>>
>>>>> Could that be due to calls to the following functions:
>>>>>
>>>>> sigemptyset()
>>>>> sigaddset()
>>>>> sigprocmask()
>>>>>
>>>>> There are no calls to ptrace() and this is not due to
>>>>> reading /proc/PID/cmdline.
>>>>>
>>>>> In truth I can only check if this is critical for policykit.
>>>>
>>>> dontauditing this doesn't work?
>>>
>>> dontaudit will just shut it up. If it is not critical we could do that,
>>> but *what's the point of hiding stuff under the carpet* ?
>>>
>>> Your reply "[PATCH 11/34]: patch to allow consolekit shutdown the
>>> system" timestamped Tue, 01 Mar 2011 14:18:01 -0500 gives a practical
>>> example of the fact that dontaudit can have side effects even for a
>>> maintainer.
>>>
>>> If one works everyday with the policy such side effects probably have a
>>> minimal impact because, as soon as something goes wrong, he or she knows
>>> that if there are no AVCs then he or she has to track down the dontaudit
>>> rules.
>>>
>>> But for somebody that barely knows his or her system has a security
>>> framework and a security policy, these "side-effects" could turn into
>>> real "blockers". For example, he or she manages to find out that the
>>> problem is due to SELinux, then manages to find out that he/she needs to
>>> check the audit logs, but at the end he/she doesn't find anything there
>>> and just gets confused.
>>>
>>> For a distribution it's different. Everything is pre-packaged and
>>> tested. But here we are discussing the reference policy.
>>>
>>> In the context of the reference policy it could make some sense only in
>>> the context of well known "leaks". But this doesn't appear to be the
>>> case...
>>
>> In general, anything that is not required should be denied. We do not
>> want to fill up the logs with extraneous denial messages, so we
>> dontaudit them. If there is a question of denials being suppressed by
>> dontaudits, thats why we have semodule -D.
>
> I forgot to add that it's not flooding logs. It logs that only once upon
> starting up.

But if we didn't dontaudit things like this, the log might be flooded.

--
Chris PeBenito
Tresys Technology, LLC
http://www.tresys.com | oss.tresys.com