2012-08-07 18:55:57

by dominick.grift

[permalink] [raw]
Subject: [refpolicy] [PATCH] oidentd fixes


oident init script in debian is in /etc/init.d
~/.oidentd.conf is a single file
remove oidentd_read_user_content because interfaces aren' for internal
usage

Signed-off-by: Dominick Grift <[email protected]>
diff --git a/oident.fc b/oident.fc
index 5840ea8..5a99b3d 100644
--- a/oident.fc
+++ b/oident.fc
@@ -1,8 +1,9 @@
-HOME_DIR/\.oidentd.conf gen_context(system_u:object_r:oidentd_home_t, s0)
+HOME_DIR/\.oidentd.conf -- gen_context(system_u:object_r:oidentd_home_t, s0)

/etc/oidentd\.conf -- gen_context(system_u:object_r:oidentd_config_t, s0)
/etc/oidentd_masq\.conf -- gen_context(system_u:object_r:oidentd_config_t, s0)

/etc/rc\.d/init\.d/oidentd -- gen_context(system_u:object_r:oidentd_initrc_exec_t, s0)
+/etc/init\.d/oidentd -- gen_context(system_u:object_r:oidentd_initrc_exec_t, s0)

/usr/sbin/oidentd -- gen_context(system_u:object_r:oidentd_exec_t, s0)
diff --git a/oident.if b/oident.if
index bb4fae5..bfdcce2 100644
--- a/oident.if
+++ b/oident.if
@@ -9,26 +9,6 @@

########################################
## <summary>
-## Allow the specified domain to read
-## Oidentd personal configuration files.
-## </summary>
-## <param name="domain">
-## <summary>
-## Domain allowed access.
-## </summary>
-## </param>
-#
-interface(`oident_read_user_content', `
- gen_require(`
- type oidentd_home_t;
- ')
-
- allow $1 oidentd_home_t:file read_file_perms;
- userdom_search_user_home_dirs($1)
-')
-
-########################################
-## <summary>
## Allow the specified domain to create, read, write, and delete
## Oidentd personal configuration files.
## </summary>
diff --git a/oident.te b/oident.te
index 8845174..6e5be53 100644
--- a/oident.te
+++ b/oident.te
@@ -34,6 +34,8 @@

allow oidentd_t oidentd_config_t:file read_file_perms;

+allow oidentd_t oidentd_home_t:file read_file_perms;
+
corenet_all_recvfrom_unlabeled(oidentd_t)
corenet_all_recvfrom_netlabel(oidentd_t)
corenet_tcp_sendrecv_generic_if(oidentd_t)
@@ -58,7 +60,7 @@

sysnet_read_config(oidentd_t)

-oident_read_user_content(oidentd_t)
+userdom_search_user_home_dirs(oidentd_t)

optional_policy(`
nis_use_ypbind(oidentd_t)


2012-08-07 19:48:08

by Guido Trentalancia

[permalink] [raw]
Subject: [refpolicy] [PATCH] oidentd fixes

On 07/08/2012 20:55, Dominick Grift wrote:
>
> oident init script in debian is in /etc/init.d
> ~/.oidentd.conf is a single file
> remove oidentd_read_user_content because interfaces aren' for internal
> usage
>
> Signed-off-by: Dominick Grift <[email protected]>
> diff --git a/oident.fc b/oident.fc
> index 5840ea8..5a99b3d 100644
> --- a/oident.fc
> +++ b/oident.fc
> @@ -1,8 +1,9 @@
> -HOME_DIR/\.oidentd.conf gen_context(system_u:object_r:oidentd_home_t, s0)
> +HOME_DIR/\.oidentd.conf -- gen_context(system_u:object_r:oidentd_home_t, s0)
>
> /etc/oidentd\.conf -- gen_context(system_u:object_r:oidentd_config_t, s0)
> /etc/oidentd_masq\.conf -- gen_context(system_u:object_r:oidentd_config_t, s0)
>
> /etc/rc\.d/init\.d/oidentd -- gen_context(system_u:object_r:oidentd_initrc_exec_t, s0)
> +/etc/init\.d/oidentd -- gen_context(system_u:object_r:oidentd_initrc_exec_t, s0)

Why don't you use ifdef distro here ? And perhaps you might want to
align it further right, as it would be easier to read.

Also, does anybody know whether a complete and always updated list of
files for each distribution is available ? That would make the job of
creating file context specifications much easier...

> /usr/sbin/oidentd -- gen_context(system_u:object_r:oidentd_exec_t, s0)
> diff --git a/oident.if b/oident.if
> index bb4fae5..bfdcce2 100644
> --- a/oident.if
> +++ b/oident.if
> @@ -9,26 +9,6 @@
>
> ########################################
> ## <summary>
> -## Allow the specified domain to read
> -## Oidentd personal configuration files.
> -## </summary>
> -## <param name="domain">
> -## <summary>
> -## Domain allowed access.
> -## </summary>
> -## </param>
> -#
> -interface(`oident_read_user_content', `
> - gen_require(`
> - type oidentd_home_t;
> - ')
> -
> - allow $1 oidentd_home_t:file read_file_perms;
> - userdom_search_user_home_dirs($1)
> -')
> -
> -########################################
> -## <summary>
> ## Allow the specified domain to create, read, write, and delete
> ## Oidentd personal configuration files.
> ## </summary>
> diff --git a/oident.te b/oident.te
> index 8845174..6e5be53 100644
> --- a/oident.te
> +++ b/oident.te
> @@ -34,6 +34,8 @@
>
> allow oidentd_t oidentd_config_t:file read_file_perms;
>
> +allow oidentd_t oidentd_home_t:file read_file_perms;
> +
> corenet_all_recvfrom_unlabeled(oidentd_t)
> corenet_all_recvfrom_netlabel(oidentd_t)
> corenet_tcp_sendrecv_generic_if(oidentd_t)
> @@ -58,7 +60,7 @@
>
> sysnet_read_config(oidentd_t)
>
> -oident_read_user_content(oidentd_t)
> +userdom_search_user_home_dirs(oidentd_t)
>
> optional_policy(`
> nis_use_ypbind(oidentd_t)

Seems alright, although I am not using this.

Regards,

Guido

2012-08-07 19:50:07

by dominick.grift

[permalink] [raw]
Subject: [refpolicy] [PATCH] oidentd fixes



On Tue, 2012-08-07 at 21:48 +0200, Guido Trentalancia wrote:
> On 07/08/2012 20:55, Dominick Grift wrote:
> >
> > oident init script in debian is in /etc/init.d
> > ~/.oidentd.conf is a single file
> > remove oidentd_read_user_content because interfaces aren' for internal
> > usage
> >
> > Signed-off-by: Dominick Grift <[email protected]>
> > diff --git a/oident.fc b/oident.fc
> > index 5840ea8..5a99b3d 100644
> > --- a/oident.fc
> > +++ b/oident.fc
> > @@ -1,8 +1,9 @@
> > -HOME_DIR/\.oidentd.conf gen_context(system_u:object_r:oidentd_home_t, s0)
> > +HOME_DIR/\.oidentd.conf -- gen_context(system_u:object_r:oidentd_home_t, s0)
> >
> > /etc/oidentd\.conf -- gen_context(system_u:object_r:oidentd_config_t, s0)
> > /etc/oidentd_masq\.conf -- gen_context(system_u:object_r:oidentd_config_t, s0)
> >
> > /etc/rc\.d/init\.d/oidentd -- gen_context(system_u:object_r:oidentd_initrc_exec_t, s0)
> > +/etc/init\.d/oidentd -- gen_context(system_u:object_r:oidentd_initrc_exec_t, s0)
>
> Why don't you use ifdef distro here ? And perhaps you might want to
> align it further right, as it would be easier to read.

Because that's overengineering. If there is no file there then theres no
harm if there is it gets labeled properly

> Also, does anybody know whether a complete and always updated list of
> files for each distribution is available ? That would make the job of
> creating file context specifications much easier...
>
> > /usr/sbin/oidentd -- gen_context(system_u:object_r:oidentd_exec_t, s0)
> > diff --git a/oident.if b/oident.if
> > index bb4fae5..bfdcce2 100644
> > --- a/oident.if
> > +++ b/oident.if
> > @@ -9,26 +9,6 @@
> >
> > ########################################
> > ## <summary>
> > -## Allow the specified domain to read
> > -## Oidentd personal configuration files.
> > -## </summary>
> > -## <param name="domain">
> > -## <summary>
> > -## Domain allowed access.
> > -## </summary>
> > -## </param>
> > -#
> > -interface(`oident_read_user_content', `
> > - gen_require(`
> > - type oidentd_home_t;
> > - ')
> > -
> > - allow $1 oidentd_home_t:file read_file_perms;
> > - userdom_search_user_home_dirs($1)
> > -')
> > -
> > -########################################
> > -## <summary>
> > ## Allow the specified domain to create, read, write, and delete
> > ## Oidentd personal configuration files.
> > ## </summary>
> > diff --git a/oident.te b/oident.te
> > index 8845174..6e5be53 100644
> > --- a/oident.te
> > +++ b/oident.te
> > @@ -34,6 +34,8 @@
> >
> > allow oidentd_t oidentd_config_t:file read_file_perms;
> >
> > +allow oidentd_t oidentd_home_t:file read_file_perms;
> > +
> > corenet_all_recvfrom_unlabeled(oidentd_t)
> > corenet_all_recvfrom_netlabel(oidentd_t)
> > corenet_tcp_sendrecv_generic_if(oidentd_t)
> > @@ -58,7 +60,7 @@
> >
> > sysnet_read_config(oidentd_t)
> >
> > -oident_read_user_content(oidentd_t)
> > +userdom_search_user_home_dirs(oidentd_t)
> >
> > optional_policy(`
> > nis_use_ypbind(oidentd_t)
>
> Seems alright, although I am not using this.
>
> Regards,
>
> Guido

2012-08-08 13:11:35

by cpebenito

[permalink] [raw]
Subject: [refpolicy] [PATCH] oidentd fixes

On 08/07/12 14:55, Dominick Grift wrote:
> remove oidentd_read_user_content because interfaces aren' for internal
> usage

That's not actually a refpolicy rule.

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

2012-08-08 13:25:01

by cpebenito

[permalink] [raw]
Subject: [refpolicy] [PATCH] oidentd fixes

On 08/08/12 09:11, Christopher J. PeBenito wrote:
> On 08/07/12 14:55, Dominick Grift wrote:
>> remove oidentd_read_user_content because interfaces aren' for internal
>> usage
>
> That's not actually a refpolicy rule.

To complete the thought, its fine to use an interface internally. However, its preferred that you not create an interface only to use it internally unless its a complicated concept you're trying to abstract (e.g. portage_compile_domain()).

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

2012-08-08 13:27:24

by dominick.grift

[permalink] [raw]
Subject: [refpolicy] [PATCH] oidentd fixes



On Wed, 2012-08-08 at 09:11 -0400, Christopher J. PeBenito wrote:
> On 08/07/12 14:55, Dominick Grift wrote:
> > remove oidentd_read_user_content because interfaces aren' for internal
> > usage
>
> That's not actually a refpolicy rule.
>

Am not going to spend time in the archives to find where this was
touched or argue but you pointed this out once and it actually makes
good sense. So whether it is a rule or not i will continue with this
guideline.

2012-08-08 13:46:10

by dominick.grift

[permalink] [raw]
Subject: [refpolicy] [PATCH] oidentd fixes



On Wed, 2012-08-08 at 09:25 -0400, Christopher J. PeBenito wrote:
> On 08/08/12 09:11, Christopher J. PeBenito wrote:
> > On 08/07/12 14:55, Dominick Grift wrote:
> >> remove oidentd_read_user_content because interfaces aren' for internal
> >> usage
> >
> > That's not actually a refpolicy rule.
>
> To complete the thought, its fine to use an interface internally. However, its preferred that you not create an interface only to use it internally unless its a complicated concept you're trying to abstract (e.g. portage_compile_domain()).
>

And that is actually the case here. The oidentd_read_user_content() is
trivial and is only used internally.

2012-08-08 13:58:35

by cpebenito

[permalink] [raw]
Subject: [refpolicy] [PATCH] oidentd fixes

On 08/08/12 09:46, Dominick Grift wrote:
> On Wed, 2012-08-08 at 09:25 -0400, Christopher J. PeBenito wrote:
>> On 08/08/12 09:11, Christopher J. PeBenito wrote:
>>> On 08/07/12 14:55, Dominick Grift wrote:
>>>> remove oidentd_read_user_content because interfaces aren' for internal
>>>> usage
>>>
>>> That's not actually a refpolicy rule.
>>
>> To complete the thought, its fine to use an interface internally. However, its preferred that you not create an interface only to use it internally unless its a complicated concept you're trying to abstract (e.g. portage_compile_domain()).
>>
>
> And that is actually the case here. The oidentd_read_user_content() is
> trivial and is only used internally.
>

True, but technically the interface shouldn't be removed otherwise it breaks the API (the interface has been at least 1 release). So you can make the change in the te file and do the usual deprecation notifications in the interface. Then we can drop the interface some time in the future. It may not be used in refpolicy, but theres the chance that it might be used in someone's custom policy.

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

2012-08-08 14:01:45

by dominick.grift

[permalink] [raw]
Subject: [refpolicy] [PATCH] oidentd fixes



On Wed, 2012-08-08 at 09:58 -0400, Christopher J. PeBenito wrote:
> On 08/08/12 09:46, Dominick Grift wrote:
> > On Wed, 2012-08-08 at 09:25 -0400, Christopher J. PeBenito wrote:
> >> On 08/08/12 09:11, Christopher J. PeBenito wrote:
> >>> On 08/07/12 14:55, Dominick Grift wrote:
> >>>> remove oidentd_read_user_content because interfaces aren' for internal
> >>>> usage
> >>>
> >>> That's not actually a refpolicy rule.
> >>
> >> To complete the thought, its fine to use an interface internally. However, its preferred that you not create an interface only to use it internally unless its a complicated concept you're trying to abstract (e.g. portage_compile_domain()).
> >>
> >
> > And that is actually the case here. The oidentd_read_user_content() is
> > trivial and is only used internally.
> >
>
> True, but technically the interface shouldn't be removed otherwise it breaks the API (the interface has been at least 1 release). So you can make the change in the te file and do the usual deprecation notifications in the interface. Then we can drop the interface some time in the future. It may not be used in refpolicy, but theres the chance that it might be used in someone's custom policy.
>

You got me there

2012-08-08 14:19:26

by Guido Trentalancia

[permalink] [raw]
Subject: [refpolicy] [PATCH] oidentd fixes

On 08/08/2012 15:27, Dominick Grift wrote:
>
>
> On Wed, 2012-08-08 at 09:11 -0400, Christopher J. PeBenito wrote:
>> On 08/07/12 14:55, Dominick Grift wrote:
>>> remove oidentd_read_user_content because interfaces aren' for internal
>>> usage
>>
>> That's not actually a refpolicy rule.
>>
>
> Am not going to spend time in the archives to find where this was
> touched or argue but you pointed this out once and it actually makes
> good sense. So whether it is a rule or not i will continue with this
> guideline.

May I have a word about it ?

A rule which strictly forbids using interfaces internally would not make
any sense.

Interfaces are the abstraction of functions (or procedures) in classical
procedural (imperative) programming.

Therefore, interfaces should always be preferred, when they improve
readability and more importantly maintanability and (internal) modularity.

The above view also matches quite well with the advice that Christopher
has given you, because if a very elementary interface is created for
internal use (a good example is perhaps the initial mistake I've made
while creating the mcelog module), then its cost overweights its
benefits (both in terms of programming time/complexity and in terms of
readability, because it would be split in different files).

So, in my opinion, this means there isn't a general and rigid rule that
can be stated, but only good sense and a little of experience might help.

Regards,

Guido