Skip to contentSkip to conversation

Various fixes to the ntp policy module

PATCH Superseded Dominick Grift 2 participants 11 messages Aug 13, 2012 – Aug 14, 2012

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore
#0 lore raw

cover Various fixes to the ntp policy module

The previous patch was bad.

Dominick Grift (6):
Introduce ntp_conf_t config file type and use that instead of
net_conf_t so that ntp_admin() does not need access to network configuration to be able to manage ntpd.
There is no need for ntp_admin() to be able to get attributes of the
ntpd process.
Allow ntp_admin() to manage ntp_drift_t content.
Allow ntp_admin() to list etc_t directories to be able to list ntp
configuration content.
Use "Role allowed access." for consistency
Use permissions sets for compatibility.

 ntp.fc |  4 ++--
 ntp.if | 11 ++++++-----
 ntp.te |  7 ++++++-
 3 files changed, 14 insertions(+), 8 deletions(-)
Signature · 1 line
1.7.11.2
#1 lore raw

PATCH 1/6 Introduce ntp_conf_t

Introduce ntp_conf_t config file type and use that instead of net_conf_t so that ntp_admin() does not need access to network configuration to be able to manage ntpd.

---
 ntp.fc | 4 ++--
 ntp.if | 4 ++--
 ntp.te | 5 +++++
 3 files changed, 9 insertions(+), 4 deletions(-)
ntp.fc +2 -2
index e79dccc..05fa48d 100644
@@ -2,11 +2,11 @@
 /etc/cron\.(daily|weekly)/ntp-simple -- gen_context(system_u:object_r:ntpd_exec_t,s0)
 /etc/cron\.(daily|weekly)/ntp-server -- gen_context(system_u:object_r:ntpd_exec_t,s0)
 
-/etc/ntpd?\.conf.*		--	gen_context(system_u:object_r:net_conf_t,s0)
+/etc/ntpd?\.conf.*		--	gen_context(system_u:object_r:ntp_conf_t,s0)
 /etc/ntp/crypto(/.*)?			gen_context(system_u:object_r:ntpd_key_t,s0)
 /etc/ntp/data(/.*)?			gen_context(system_u:object_r:ntp_drift_t,s0)
 /etc/ntp/keys			--	gen_context(system_u:object_r:ntpd_key_t,s0)
-/etc/ntp/step-tickers.*		--	gen_context(system_u:object_r:net_conf_t,s0)
+/etc/ntp/step-tickers.*		--	gen_context(system_u:object_r:ntp_conf_t,s0)
 
 /etc/rc\.d/init\.d/ntpd		--	gen_context(system_u:object_r:ntpd_initrc_exec_t,s0)
 
ntp.if +2 -2
index e80f8c0..d56b635 100644
@@ -140,7 +140,7 @@ interface(`ntp_rw_shm',`
 interface(`ntp_admin',`
 	gen_require(`
 		type ntpd_t, ntpd_tmp_t, ntpd_log_t;
-		type ntpd_key_t, ntpd_var_run_t;
+		type ntpd_key_t, ntpd_var_run_t, ntp_conf_t;
 		type ntpd_initrc_exec_t;
 	')
 
@@ -152,7 +152,7 @@ interface(`ntp_admin',`
 	role_transition $2 ntpd_initrc_exec_t system_r;
 	allow $2 system_r;
 
-	admin_pattern($1, ntpd_key_t)
+	admin_pattern($1, { ntpd_key_t ntp_conf_t })
 
 	logging_list_logs($1)
 	admin_pattern($1, ntpd_log_t)
ntp.te +5 -0
index c61adc8..08bcd96 100644
@@ -15,6 +15,9 @@ init_daemon_domain(ntpd_t, ntpd_exec_t)
 type ntpd_initrc_exec_t;
 init_script_file(ntpd_initrc_exec_t)
 
+type ntp_conf_t;
+files_config_file(ntp_conf_t)
+
 type ntpd_key_t;
 files_type(ntpd_key_t)
 
@@ -54,6 +57,8 @@ manage_files_pattern(ntpd_t, ntp_drift_t, ntp_drift_t)
 
 can_exec(ntpd_t, ntpd_exec_t)
 
+allow ntpd_t ntp_conf_t:file read_file_perms;
+
 read_files_pattern(ntpd_t, ntpd_key_t, ntpd_key_t)
 read_lnk_files_pattern(ntpd_t, ntpd_key_t, ntpd_key_t)
 
Signature · 1 line
1.7.11.2
#2 lore raw

PATCH 2/6 Remove getattr permision from ntp_admin()

In reply to Dominick Grift

There is no need for ntp_admin() to be able to get attributes of the ntpd process.

---
 ntp.if | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
ntp.if +1 -1
index d56b635..b47bc35 100644
@@ -144,7 +144,7 @@ interface(`ntp_admin',`
 		type ntpd_initrc_exec_t;
 	')
 
-	allow $1 ntpd_t:process { ptrace signal_perms getattr };
+	allow $1 ntpd_t:process { ptrace signal_perms };
 	ps_process_pattern($1, ntpd_t)
 
 	init_labeled_script_domtrans($1, ntpd_initrc_exec_t)
Signature · 1 line
1.7.11.2
#3 lore raw

PATCH 2/6 Remove getattr permision from ntp_admin()

Quoted 13 lines · Dominick Grift

There is no need for ntp_admin() to be able to get attributes of the ntpd process.

---
 ntp.if | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
ntp.if +0 -0
index d56b635..b47bc35 100644

ntp.if interface(`ntp_admin',` · line 150

Quoted 8 lines
ntp.if +1 -1
@@ -144,7 +144,7 @@ interface(`ntp_admin',`
 		type ntpd_initrc_exec_t;
 	')
 
-	allow $1 ntpd_t:process { ptrace signal_perms getattr };
+	allow $1 ntpd_t:process { ptrace signal_perms };
 	ps_process_pattern($1, ntpd_t)
 
 	init_labeled_script_domtrans($1, ntpd_initrc_exec_t)

I disagree, its reasonable to make sure its running in the right domain, which requires this permission to read /proc/pid/attr/current.

Signature · 3 lines
Chris PeBenito
Tresys Technology, LLC
www.tresys.com | oss.tresys.com
#4 lore raw

PATCH 2/6 Remove getattr permision from ntp_admin()

Quoted 17 lines · Christopher J. PeBenito
Quoted 16 lines · Dominick Grift

There is no need for ntp_admin() to be able to get attributes of the ntpd process.

---
 ntp.if | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
ntp.if +0 -0
index d56b635..b47bc35 100644
@@ -144,7 +144,7 @@ interface(`ntp_admin',`
 		type ntpd_initrc_exec_t;
 	')
 
Quoted 8 lines

ntp.if interface(`ntp_admin',` · line 150

Quoted 5 lines
ntp.if +1 -1
-	allow $1 ntpd_t:process { ptrace signal_perms getattr };
+	allow $1 ntpd_t:process { ptrace signal_perms };
 	ps_process_pattern($1, ntpd_t)
 
 	init_labeled_script_domtrans($1, ntpd_initrc_exec_t)

I disagree, its reasonable to make sure its running in the right domain, which requires this permission to read /proc/pid/attr/current.

Its already allowed with

ps_process_pattern($1, ntpd_t)

#5 lore raw

PATCH 2/6 Remove getattr permision from ntp_admin()

Quoted 24 lines · Dominick Grift
Quoted 21 lines · Christopher J. PeBenito
Quoted 20 lines · Dominick Grift

There is no need for ntp_admin() to be able to get attributes of the ntpd process.

---
 ntp.if | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
ntp.if +1 -1
index d56b635..b47bc35 100644
@@ -144,7 +144,7 @@ interface(`ntp_admin',`
 		type ntpd_initrc_exec_t;
 	')
 
-	allow $1 ntpd_t:process { ptrace signal_perms getattr };
+	allow $1 ntpd_t:process { ptrace signal_perms };
 	ps_process_pattern($1, ntpd_t)
 
Quoted 8 lines
Quoted 4 lines

ntp.if interface(`ntp_admin',` · line 150

Quoted 1 line
ntp.if +0 -0
 	init_labeled_script_domtrans($1, ntpd_initrc_exec_t)

I disagree, its reasonable to make sure its running in the right domain, which requires this permission to read /proc/pid/attr/current.

Its already allowed with

ps_process_pattern($1, ntpd_t)

My mistake. Merged.

Signature · 3 lines
Chris PeBenito
Tresys Technology, LLC
www.tresys.com | oss.tresys.com
#6 lore raw

PATCH 3/6 Allow ntp_admin() to manage ntp_drift_t content.

In reply to Dominick Grift
---
 ntp.if | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
ntp.if +2 -2
index b47bc35..07a45bf 100644
@@ -141,7 +141,7 @@ interface(`ntp_admin',`
 	gen_require(`
 		type ntpd_t, ntpd_tmp_t, ntpd_log_t;
 		type ntpd_key_t, ntpd_var_run_t, ntp_conf_t;
-		type ntpd_initrc_exec_t;
+		type ntpd_initrc_exec_t, ntp_drift_t;
 	')
 
 	allow $1 ntpd_t:process { ptrace signal_perms };
@@ -152,7 +152,7 @@ interface(`ntp_admin',`
 	role_transition $2 ntpd_initrc_exec_t system_r;
 	allow $2 system_r;
 
-	admin_pattern($1, { ntpd_key_t ntp_conf_t })
+	admin_pattern($1, { ntpd_key_t ntp_conf_t ntp_drift_t })
 
 	logging_list_logs($1)
 	admin_pattern($1, ntpd_log_t)
Signature · 1 line
1.7.11.2
#7 lore raw

PATCH 4/6 List etc_t directories

In reply to Dominick Grift

Allow ntp_admin() to list etc_t directories to be able to list ntp configuration content.

---
 ntp.if | 1 +
 1 file changed, 1 insertion(+)
ntp.if +1 -0
index 07a45bf..00752ec 100644
@@ -152,6 +152,7 @@ interface(`ntp_admin',`
 	role_transition $2 ntpd_initrc_exec_t system_r;
 	allow $2 system_r;
 
+	files_list_etc($1)
 	admin_pattern($1, { ntpd_key_t ntp_conf_t ntp_drift_t })
 
 	logging_list_logs($1)
Signature · 1 line
1.7.11.2
#8 lore raw

PATCH 5/6 Use "Role allowed access." for consistency

In reply to Dominick Grift
---
 ntp.if | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
ntp.if +1 -1
index 00752ec..e4e72b1 100644
@@ -132,7 +132,7 @@ interface(`ntp_rw_shm',`
 ## </param>
 ## <param name="role">
 ##	<summary>
-##	The role to be allowed to manage the ntp domain.
+##	Role allowed access.
 ##	</summary>
 ## </param>
 ## <rolecap/>
Signature · 1 line
1.7.11.2
#9 lore raw

PATCH 6/6 Use permissions sets for compatibility.

In reply to Dominick Grift
---
 ntp.te | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
ntp.te +1 -1
index 08bcd96..997f393 100644
@@ -62,7 +62,7 @@ allow ntpd_t ntp_conf_t:file read_file_perms;
 read_files_pattern(ntpd_t, ntpd_key_t, ntpd_key_t)
 read_lnk_files_pattern(ntpd_t, ntpd_key_t, ntpd_key_t)
 
-allow ntpd_t ntpd_log_t:dir setattr;
+allow ntpd_t ntpd_log_t:dir setattr_dir_perms;
 manage_files_pattern(ntpd_t, ntpd_log_t, ntpd_log_t)
 logging_log_filetrans(ntpd_t, ntpd_log_t, { file dir })
 
Signature · 1 line
1.7.11.2
#10 lore raw

cover Various fixes to the ntp policy module

In reply to Dominick Grift
Quoted 14 lines · Dominick Grift

The previous patch was bad.

Dominick Grift (6):
Introduce ntp_conf_t config file type and use that instead of
net_conf_t so that ntp_admin() does not need access to network configuration to be able to manage ntpd.
There is no need for ntp_admin() to be able to get attributes of the
ntpd process.
Allow ntp_admin() to manage ntp_drift_t content.
Allow ntp_admin() to list etc_t directories to be able to list ntp
configuration content.
Use "Role allowed access." for consistency
Use permissions sets for compatibility.

Quoted 5 lines
 ntp.fc |  4 ++--
 ntp.if | 11 ++++++-----
 ntp.te |  7 ++++++-
 3 files changed, 14 insertions(+), 8 deletions(-)

patches 1, 3-6 merged.

Signature · 3 lines
Chris PeBenito
Tresys Technology, LLC
www.tresys.com | oss.tresys.com