[refpolicy] [PATCH] oidentd fixes

[refpolicy] [PATCH] oidentd fixes

[Dominick Grift]

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 <dominick.grift@gmail.com>
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)

[refpolicy] [PATCH] oidentd fixes

[Guido Trentalancia]

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 <dominick.grift@gmail.com>
> 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

[refpolicy] [PATCH] oidentd fixes

[Dominick Grift]

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 <dominick.grift@gmail.com>
> > 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

[refpolicy] [PATCH] oidentd fixes

[Christopher J. PeBenito]

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
www.tresys.com | oss.tresys.com

[refpolicy] [PATCH] oidentd fixes

[Christopher J. PeBenito]

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
www.tresys.com | oss.tresys.com

[refpolicy] [PATCH] oidentd fixes

[Dominick Grift]

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.

[refpolicy] [PATCH] oidentd fixes

[Dominick Grift]

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.

[refpolicy] [PATCH] oidentd fixes

[Christopher J. PeBenito]

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
www.tresys.com | oss.tresys.com

[refpolicy] [PATCH] oidentd fixes

[Dominick Grift]

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 

[refpolicy] [PATCH] oidentd fixes

[Guido Trentalancia]

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