oidentd fixes

PATCH New Dominick Grift 3 participants 10 messages Aug 7, 2012 – Aug 8, 2012

Share & tools

Dominick Grift

author

2012-08-07 18:55 UTC #0 lore raw

PATCH 1/1 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]>

oident.fc +2 -1

index 5840ea8..5a99b3d 100644
@@ -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)

oident.if +0 ~~-20~~

index bb4fae5..bfdcce2 100644
@@ -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>

oident.te +3 -1

index 8845174..6e5be53 100644
@@ -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)

Guido Trentalancia

2012-08-07 19:48 UTC #1 lore raw

PATCH 1/1 oidentd fixes

Quoted 12 lines · 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 <[email protected]>
oident.fc +0 -0
index 5840ea8..5a99b3d 100644

oident.fc line 7

Quoted 8 lines

oident.fc +2 -1

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

Quoted 46 lines

oident.fc +0 -0

  /usr/sbin/oidentd		--	gen_context(system_u:object_r:oidentd_exec_t, s0)

oident.if +0 ~~-20~~

index bb4fae5..bfdcce2 100644
@@ -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>

oident.te +2 -0

index 8845174..6e5be53 100644
@@ -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)

oident.te line 66

Quoted 8 lines

oident.te +1 -1

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

Dominick Grift

author

2012-08-07 19:50 UTC #2 lore raw

PATCH 1/1 oidentd fixes

Quoted 16 lines · Guido Trentalancia

Quoted 15 lines · 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 <[email protected]>
oident.fc +1 -1
index 5840ea8..5a99b3d 100644
@@ -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)
 

Quoted 8 lines

oident.fc line 7
Quoted 5 lines
oident.fc +1 -0
  /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

Quoted 64 lines

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

Quoted 46 lines

oident.fc +0 -0

  /usr/sbin/oidentd		--	gen_context(system_u:object_r:oidentd_exec_t, s0)

oident.if +0 ~~-20~~

index bb4fae5..bfdcce2 100644
@@ -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>

oident.te +2 -0

index 8845174..6e5be53 100644
@@ -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)

oident.te line 66

Quoted 8 lines

oident.te +1 -1

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

Christopher J. PeBenito

2012-08-08 13:11 UTC #3 lore raw

PATCH 1/1 oidentd fixes

In reply to Dominick Grift

Quoted 2 lines · Dominick Grift

remove oidentd_read_user_content because interfaces aren' for internal usage

That's not actually a refpolicy rule.

Signature · 3 lines

Chris PeBenito
Tresys Technology, LLC
www.tresys.com | oss.tresys.com

Christopher J. PeBenito

2012-08-08 13:25 UTC #4 lore raw

PATCH 1/1 oidentd fixes

Quoted 5 lines · Christopher J. PeBenito

Quoted 2 lines · Dominick Grift
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()).

Signature · 3 lines

Chris PeBenito
Tresys Technology, LLC
www.tresys.com | oss.tresys.com

Dominick Grift

author

2012-08-08 13:46 UTC #5 lore raw

PATCH 1/1 oidentd fixes

Quoted 9 lines · Christopher J. PeBenito

Quoted 5 lines · Christopher J. PeBenito
Quoted 2 lines · Dominick Grift
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.

Christopher J. PeBenito

2012-08-08 13:58 UTC #6 lore raw

PATCH 1/1 oidentd fixes

Quoted 6 lines · Dominick Grift

Quoted 5 lines · Christopher J. PeBenito
Quoted 4 lines · Christopher J. PeBenito
Quoted 2 lines · Dominick Grift
remove oidentd_read_user_content because interfaces aren' for internal usage

Quoted 8 lines

Quoted 4 lines
Quoted 1 line
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.

Signature · 3 lines

Chris PeBenito
Tresys Technology, LLC
www.tresys.com | oss.tresys.com

Dominick Grift

author

2012-08-08 14:01 UTC #7 lore raw

PATCH 1/1 oidentd fixes

Quoted 9 lines · Christopher J. PeBenito

Quoted 8 lines · Dominick Grift
Quoted 7 lines · Christopher J. PeBenito
Quoted 5 lines · Christopher J. PeBenito
Quoted 2 lines · Dominick Grift
remove oidentd_read_user_content because interfaces aren' for internal usage
That's not actually a refpolicy rule.

Quoted 9 lines

Quoted 6 lines
Quoted 2 lines
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

Dominick Grift

author

2012-08-08 13:27 UTC #8 lore raw

PATCH 1/1 oidentd fixes

In reply to Christopher J. PeBenito

Quoted 6 lines · Christopher J. PeBenito

Quoted 2 lines · Dominick Grift
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.

Guido Trentalancia

2012-08-08 14:19 UTC #9 lore raw

PATCH 1/1 oidentd fixes

Quoted 7 lines · Dominick Grift

Quoted 4 lines · Christopher J. PeBenito
Quoted 2 lines · Dominick Grift
remove oidentd_read_user_content because interfaces aren' for internal usage

Quoted 7 lines

Quoted 2 lines
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