2012-08-10 17:43:58

by dominick.grift

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


Use ntp_conf_t instead of net_conf_t
Permission getattr on process is not needed
Use "Role allowed access" in ntp_admin() XML
Allow ntp_admin to manage ntp drift files
Use setattr_dir_perms for compatibility

Signed-off-by: Dominick Grift <[email protected]>
diff --git a/ntp.fc b/ntp.fc
index e79dccc..05fa48d 100644
--- a/ntp.fc
+++ b/ntp.fc
@@ -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)

diff --git a/ntp.if b/ntp.if
index e80f8c0..16e8a93 100644
--- a/ntp.if
+++ b/ntp.if
@@ -132,7 +132,7 @@
## </param>
## <param name="role">
## <summary>
-## The role to be allowed to manage the ntp domain.
+## Role allowed access.
## </summary>
## </param>
## <rolecap/>
@@ -140,11 +140,11 @@
interface(`ntp_admin',`
gen_require(`
type ntpd_t, ntpd_tmp_t, ntpd_log_t;
- type ntpd_key_t, ntpd_var_run_t;
- type ntpd_initrc_exec_t;
+ type ntpd_key_t, ntpd_var_run_t, ntp_drift_t;
+ type ntpd_initrc_exec_t, ntp_conf_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)
@@ -152,7 +152,8 @@
role_transition $2 ntpd_initrc_exec_t system_r;
allow $2 system_r;

- admin_pattern($1, ntpd_key_t)
+ files_list_etc($1)
+ admin_pattern($1, { ntpd_key_t ntp_conf_t ntp_drift_t })

logging_list_logs($1)
admin_pattern($1, ntpd_log_t)
diff --git a/ntp.te b/ntp.te
index c61adc8..2f50998 100644
--- a/ntp.te
+++ b/ntp.te
@@ -15,6 +15,9 @@
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,10 +57,12 @@

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)

-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 })



2012-08-13 11:00:51

by dominick.grift

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

Ignore this one. It has a bug. I am having issues testing my patches
without Eclipse-Slide. I got used to slide and i miss using it.

On Fri, 2012-08-10 at 19:43 +0200, Dominick Grift wrote:
> Use ntp_conf_t instead of net_conf_t
> Permission getattr on process is not needed
> Use "Role allowed access" in ntp_admin() XML
> Allow ntp_admin to manage ntp drift files
> Use setattr_dir_perms for compatibility
>
> Signed-off-by: Dominick Grift <[email protected]>
> diff --git a/ntp.fc b/ntp.fc
> index e79dccc..05fa48d 100644
> --- a/ntp.fc
> +++ b/ntp.fc
> @@ -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)
>
> diff --git a/ntp.if b/ntp.if
> index e80f8c0..16e8a93 100644
> --- a/ntp.if
> +++ b/ntp.if
> @@ -132,7 +132,7 @@
> ## </param>
> ## <param name="role">
> ## <summary>
> -## The role to be allowed to manage the ntp domain.
> +## Role allowed access.
> ## </summary>
> ## </param>
> ## <rolecap/>
> @@ -140,11 +140,11 @@
> interface(`ntp_admin',`
> gen_require(`
> type ntpd_t, ntpd_tmp_t, ntpd_log_t;
> - type ntpd_key_t, ntpd_var_run_t;
> - type ntpd_initrc_exec_t;
> + type ntpd_key_t, ntpd_var_run_t, ntp_drift_t;
> + type ntpd_initrc_exec_t, ntp_conf_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)
> @@ -152,7 +152,8 @@
> role_transition $2 ntpd_initrc_exec_t system_r;
> allow $2 system_r;
>
> - admin_pattern($1, ntpd_key_t)
> + files_list_etc($1)
> + admin_pattern($1, { ntpd_key_t ntp_conf_t ntp_drift_t })
>
> logging_list_logs($1)
> admin_pattern($1, ntpd_log_t)
> diff --git a/ntp.te b/ntp.te
> index c61adc8..2f50998 100644
> --- a/ntp.te
> +++ b/ntp.te
> @@ -15,6 +15,9 @@
> 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,10 +57,12 @@
>
> 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)
>
> -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 })
>

2012-08-15 08:09:56

by Guido Trentalancia

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

On 10/08/2012 19:43, Dominick Grift wrote:
>
> Use ntp_conf_t instead of net_conf_t
> Permission getattr on process is not needed
> Use "Role allowed access" in ntp_admin() XML
> Allow ntp_admin to manage ntp drift files
> Use setattr_dir_perms for compatibility
>
> Signed-off-by: Dominick Grift <[email protected]>
> diff --git a/ntp.fc b/ntp.fc
> index e79dccc..05fa48d 100644
> --- a/ntp.fc
> +++ b/ntp.fc
> @@ -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)

I would suggest ntpd_conf_t instead of ntp_conf_t, so that ntp_conf_t
can be left available for clients.

> /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)

Same as above for the name. Perhaps, ntp_drift_t should also be modified
accordingly, so that ntpd_* types are allocated and used by the daemon
and ntp_* types are allocated and used by the clients (unless they
concide, e.g. log file or something similar, in which case ntp_* can be
used).

> /etc/rc\.d/init\.d/ntpd -- gen_context(system_u:object_r:ntpd_initrc_exec_t,s0)
>
> diff --git a/ntp.if b/ntp.if
> index e80f8c0..16e8a93 100644
> --- a/ntp.if
> +++ b/ntp.if
> @@ -132,7 +132,7 @@
> ## </param>
> ## <param name="role">
> ## <summary>
> -## The role to be allowed to manage the ntp domain.
> +## Role allowed access.
> ## </summary>
> ## </param>
> ## <rolecap/>
> @@ -140,11 +140,11 @@
> interface(`ntp_admin',`
> gen_require(`
> type ntpd_t, ntpd_tmp_t, ntpd_log_t;
> - type ntpd_key_t, ntpd_var_run_t;
> - type ntpd_initrc_exec_t;
> + type ntpd_key_t, ntpd_var_run_t, ntp_drift_t;
> + type ntpd_initrc_exec_t, ntp_conf_t;

Same as above for the name.

> ')
>
> - 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)
> @@ -152,7 +152,8 @@
> role_transition $2 ntpd_initrc_exec_t system_r;
> allow $2 system_r;
>
> - admin_pattern($1, ntpd_key_t)
> + files_list_etc($1)
> + admin_pattern($1, { ntpd_key_t ntp_conf_t ntp_drift_t })

Same as above for the name.

> logging_list_logs($1)
> admin_pattern($1, ntpd_log_t)
> diff --git a/ntp.te b/ntp.te
> index c61adc8..2f50998 100644
> --- a/ntp.te
> +++ b/ntp.te
> @@ -15,6 +15,9 @@
> type ntpd_initrc_exec_t;
> init_script_file(ntpd_initrc_exec_t)
>
> +type ntp_conf_t
> +files_config_file(ntp_conf_t)
> +

Same as above for the name.

> type ntpd_key_t;
> files_type(ntpd_key_t)
>
> @@ -54,10 +57,12 @@
>
> can_exec(ntpd_t, ntpd_exec_t)
>
> +allow ntpd_t ntp_conf_t:file read_file_perms;
> +

Same as above for the name.

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

Since setattr_dir_perms is equal to { setattr }, it might be better to
leave as it is because it should be easier to read (unless the pattern
is widely used elsewhere).

> manage_files_pattern(ntpd_t, ntpd_log_t, ntpd_log_t)
> logging_log_filetrans(ntpd_t, ntpd_log_t, { file dir })

2012-08-15 10:02:58

by dominick.grift

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



On Wed, 2012-08-15 at 10:09 +0200, Guido Trentalancia wrote:

> >
> > -allow ntpd_t ntpd_log_t:dir setattr;
> > +allow ntpd_t ntpd_log_t:dir setattr_dir_perms;
>
> Since setattr_dir_perms is equal to { setattr }, it might be better to
> leave as it is because it should be easier to read (unless the pattern
> is widely used elsewhere).
>

I do not agree. Using permission sets wherever possible provides a
compatibility layer. It gives us a single point of failure

Ive learned this when the open avperm was introduced a while ago.

If you use permission sets consistently then life will be easier.

Besides that, the setattr / getattr permission sets a are there so might
as well use it.

2012-08-15 10:11:35

by dominick.grift

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



On Wed, 2012-08-15 at 10:09 +0200, Guido Trentalancia wrote:
> On 10/08/2012 19:43, Dominick Grift wrote:
> >
> > Use ntp_conf_t instead of net_conf_t
> > Permission getattr on process is not needed
> > Use "Role allowed access" in ntp_admin() XML
> > Allow ntp_admin to manage ntp drift files
> > Use setattr_dir_perms for compatibility
> >
> > Signed-off-by: Dominick Grift <[email protected]>
> > diff --git a/ntp.fc b/ntp.fc
> > index e79dccc..05fa48d 100644
> > --- a/ntp.fc
> > +++ b/ntp.fc
> > @@ -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)
>
> I would suggest ntpd_conf_t instead of ntp_conf_t, so that ntp_conf_t
> can be left available for clients.

I considered that but decided to go with ntp instead of ntpd. One reason
is because maintainer suggested i use that type. Another because drift
file is also ntp prefixed.

The file itself is also called ntp.conf rather than ntpd.conf.

Anyways its committed now so we will just have to make due. Can always
change it later if needed i guess.