2012-08-29 19:28:41

by sven.vermeulen

[permalink] [raw]
Subject: [refpolicy] [PATCH v1 0/5] Small set of updates

This patchset contains a few smaller updates on the SELinux policies.

Sven Vermeulen (5):
Puppet uses mount output for verification
Allow syslogd to create /var/lib/syslog and
/var/lib/misc/syslog-ng.persist
Gentoo's openrc does not require initrc_exec_t for runscripts anymore
Allow init scripts to read courier configuration
Udev's tables (run data) is stored in directories

policy/modules/system/init.fc | 4 ----
policy/modules/system/init.te | 4 ++++
policy/modules/system/logging.fc | 4 ++++
policy/modules/system/logging.te | 2 ++
policy/modules/system/mount.te | 4 ++++
policy/modules/system/udev.te | 7 +++++--
6 files changed, 19 insertions(+), 6 deletions(-)

--
1.7.8.6


2012-08-29 19:28:42

by sven.vermeulen

[permalink] [raw]
Subject: [refpolicy] [PATCH v1 1/5] Puppet uses mount output for verification

Puppet calls mount to obtain the list of mounted file systems, redirecting its
output to a temporary file (labeled puppet_tmp_t). This allows the mount domain
to write to this resource.

Signed-off-by: Sven Vermeulen <[email protected]>
---
policy/modules/system/mount.te | 4 ++++
1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/policy/modules/system/mount.te b/policy/modules/system/mount.te
index 63931f6..4175ff7 100644
--- a/policy/modules/system/mount.te
+++ b/policy/modules/system/mount.te
@@ -193,6 +193,10 @@ optional_policy(`
')
')

+optional_policy(`
+ puppet_rw_tmp(mount_t)
+')
+
# for kernel package installation
optional_policy(`
rpm_rw_pipes(mount_t)
--
1.7.8.6

2012-08-29 19:28:43

by sven.vermeulen

[permalink] [raw]
Subject: [refpolicy] [PATCH v1 2/5] Allow syslogd to create /var/lib/syslog and /var/lib/misc/syslog-ng.persist

If the /var/lib/syslog directory does not exist, then syslog-ng (running in
syslogd_t) will attempt to create the directory.

Allow the syslogd_t domain to create the directory, and use an automatic file
transition towards syslogd_var_lib_t.

Also, the syslog-ng daemon uses a persistence file in
/var/lib/misc/syslog-ng.persist (and .persist- if it suspects a collision). As
/var/lib/misc is still a generic var_lib_t, we have the syslogd_t daemon write
its files as syslogd_var_lib_t therein.

Signed-off-by: Sven Vermeulen <[email protected]>
---
policy/modules/system/logging.fc | 4 ++++
policy/modules/system/logging.te | 2 ++
2 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/policy/modules/system/logging.fc b/policy/modules/system/logging.fc
index 02f4c97..aeaabe1 100644
--- a/policy/modules/system/logging.fc
+++ b/policy/modules/system/logging.fc
@@ -32,6 +32,10 @@ ifdef(`distro_suse', `
/var/lib/stunnel/dev/log -s gen_context(system_u:object_r:devlog_t,s0)
')

+ifdef(`distro_gentoo', `
+/var/lib/misc/syslog-ng.persist-? -- gen_context(system_u:object_r:syslogd_var_lib_t,s0)
+')
+
/var/axfrdns/log/main(/.*)? gen_context(system_u:object_r:var_log_t,s0)
/var/dnscache/log/main(/.*)? gen_context(system_u:object_r:var_log_t,s0)
/var/cfengine/outputs(/.*)? gen_context(system_u:object_r:var_log_t,s0)
diff --git a/policy/modules/system/logging.te b/policy/modules/system/logging.te
index 0034021..9cebc41 100644
--- a/policy/modules/system/logging.te
+++ b/policy/modules/system/logging.te
@@ -439,6 +439,8 @@ files_read_etc_runtime_files(syslogd_t)
# /initrd is not umounted before minilog starts
files_dontaudit_search_isid_type_dirs(syslogd_t)
files_read_kernel_symbol_table(syslogd_t)
+files_rw_var_lib_dirs(syslogd_t)
+files_var_lib_filetrans(syslogd_t, syslogd_var_lib_t, { file dir })

fs_getattr_all_fs(syslogd_t)
fs_search_auto_mountpoints(syslogd_t)
--
1.7.8.6

2012-08-29 19:28:44

by sven.vermeulen

[permalink] [raw]
Subject: [refpolicy] [PATCH v1 3/5] Gentoo's openrc does not require initrc_exec_t for runscripts anymore

The Gentoo-specific runscripts in /sbin should not be marked as initrc_exec_t
anymore (just bin_t).

Signed-off-by: Sven Vermeulen <[email protected]>
---
policy/modules/system/init.fc | 4 ----
1 files changed, 0 insertions(+), 4 deletions(-)

diff --git a/policy/modules/system/init.fc b/policy/modules/system/init.fc
index 03e27db..9a4d3a7 100644
--- a/policy/modules/system/init.fc
+++ b/policy/modules/system/init.fc
@@ -35,10 +35,6 @@ ifdef(`distro_gentoo', `

ifdef(`distro_gentoo', `
/sbin/rc -- gen_context(system_u:object_r:rc_exec_t,s0)
-/sbin/runscript -- gen_context(system_u:object_r:initrc_exec_t,s0)
-/sbin/runscript\.sh -- gen_context(system_u:object_r:initrc_exec_t,s0)
-/sbin/runsvcscript\.sh -- gen_context(system_u:object_r:initrc_exec_t,s0)
-/sbin/svcinit -- gen_context(system_u:object_r:initrc_exec_t,s0)
')

#
--
1.7.8.6

2012-08-29 19:28:45

by sven.vermeulen

[permalink] [raw]
Subject: [refpolicy] [PATCH v1 4/5] Allow init scripts to read courier configuration

The courier-imap and courier-pop3 daemons are started by sourcing their
configuration files, and then invoking the daemons using the proper options. If
this is done through a specialized script, then init only needs to call this
script (where a proper transition occurs) but if the init script itself does
this, it needs to be able to read the configuration files.

Signed-off-by: Sven Vermeulen <[email protected]>
---
policy/modules/system/init.te | 4 ++++
1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/policy/modules/system/init.te b/policy/modules/system/init.te
index 0a7eda5..32dd043 100644
--- a/policy/modules/system/init.te
+++ b/policy/modules/system/init.te
@@ -613,6 +613,10 @@ optional_policy(`
')

optional_policy(`
+ courier_read_config(initrc_t)
+')
+
+optional_policy(`
cpucontrol_stub(initrc_t)
dev_getattr_cpu_dev(initrc_t)
')
--
1.7.8.6

2012-08-29 19:28:46

by sven.vermeulen

[permalink] [raw]
Subject: [refpolicy] [PATCH v1 5/5] Udev's tables (run data) is stored in directories

When udev creates a directory to store its runtime data in, it will attempt to
relabel the directory too. So allow udev SELinux policy to do so, as well as
manage the udev_tbl_t files and directories.

Signed-off-by: Sven Vermeulen <[email protected]>
---
policy/modules/system/udev.te | 7 +++++--
1 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/policy/modules/system/udev.te b/policy/modules/system/udev.te
index 00fcf27..261055d 100644
--- a/policy/modules/system/udev.te
+++ b/policy/modules/system/udev.te
@@ -63,8 +63,11 @@ can_exec(udev_t, udev_helper_exec_t)
# read udev config
allow udev_t udev_etc_t:file read_file_perms;

-# create udev database in /dev/.udevdb
-allow udev_t udev_tbl_t:file manage_file_perms;
+allow udev_t udev_tbl_t:dir relabelto;
+manage_dirs_pattern(udev_t, udev_tbl_t, udev_tbl_t)
+manage_files_pattern(udev_t, udev_tbl_t, udev_tbl_t)
+manage_lnk_files_pattern(udev_t, udev_tbl_t, udev_tbl_t)
+
dev_filetrans(udev_t, udev_tbl_t, file)

list_dirs_pattern(udev_t, udev_rules_t, udev_rules_t)
--
1.7.8.6

2012-08-29 19:37:55

by dominick.grift

[permalink] [raw]
Subject: [refpolicy] [PATCH v1 5/5] Udev's tables (run data) is stored in directories



On Wed, 2012-08-29 at 21:28 +0200, Sven Vermeulen wrote:
> When udev creates a directory to store its runtime data in, it will attempt to
> relabel the directory too. So allow udev SELinux policy to do so, as well as
> manage the udev_tbl_t files and directories.
>
> Signed-off-by: Sven Vermeulen <[email protected]>
> ---
> policy/modules/system/udev.te | 7 +++++--
> 1 files changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/policy/modules/system/udev.te b/policy/modules/system/udev.te
> index 00fcf27..261055d 100644
> --- a/policy/modules/system/udev.te
> +++ b/policy/modules/system/udev.te
> @@ -63,8 +63,11 @@ can_exec(udev_t, udev_helper_exec_t)
> # read udev config
> allow udev_t udev_etc_t:file read_file_perms;
>
> -# create udev database in /dev/.udevdb
> -allow udev_t udev_tbl_t:file manage_file_perms;
> +allow udev_t udev_tbl_t:dir relabelto;
> +manage_dirs_pattern(udev_t, udev_tbl_t, udev_tbl_t)
> +manage_files_pattern(udev_t, udev_tbl_t, udev_tbl_t)
> +manage_lnk_files_pattern(udev_t, udev_tbl_t, udev_tbl_t)
> +
> dev_filetrans(udev_t, udev_tbl_t, file)

This doesnt make sense to me.

First we had:

allow udev_t udev_tbl_t:file manage_file_perms;
dev_filetrans(udev_t, udev_tbl_t, file)

with these specs:

/dev/\.udev(/.*)? -- gen_context(system_u:object_r:udev_tbl_t,s0)
/dev/\.udevdb -- gen_context(system_u:object_r:udev_tbl_t,s0)
/dev/udev\.tbl -- gen_context(system_u:object_r:udev_tbl_t,s0)

What does this tell me?

Well there shouldnt be any dirs and symlinks with type udev_tbl_t.

Only files. dirs and lnk_files should be device_t.

>
> list_dirs_pattern(udev_t, udev_rules_t, udev_rules_t)

2012-08-29 19:41:27

by dominick.grift

[permalink] [raw]
Subject: [refpolicy] [PATCH v1 2/5] Allow syslogd to create /var/lib/syslog and /var/lib/misc/syslog-ng.persist



On Wed, 2012-08-29 at 21:28 +0200, Sven Vermeulen wrote:
> If the /var/lib/syslog directory does not exist, then syslog-ng (running in
> syslogd_t) will attempt to create the directory.
>
> Allow the syslogd_t domain to create the directory, and use an automatic file
> transition towards syslogd_var_lib_t.
>
> Also, the syslog-ng daemon uses a persistence file in
> /var/lib/misc/syslog-ng.persist (and .persist- if it suspects a collision). As
> /var/lib/misc is still a generic var_lib_t, we have the syslogd_t daemon write
> its files as syslogd_var_lib_t therein.
>
> Signed-off-by: Sven Vermeulen <[email protected]>
> ---
> policy/modules/system/logging.fc | 4 ++++
> policy/modules/system/logging.te | 2 ++
> 2 files changed, 6 insertions(+), 0 deletions(-)
>
> diff --git a/policy/modules/system/logging.fc b/policy/modules/system/logging.fc
> index 02f4c97..aeaabe1 100644
> --- a/policy/modules/system/logging.fc
> +++ b/policy/modules/system/logging.fc
> @@ -32,6 +32,10 @@ ifdef(`distro_suse', `
> /var/lib/stunnel/dev/log -s gen_context(system_u:object_r:devlog_t,s0)
> ')
>
> +ifdef(`distro_gentoo', `
> +/var/lib/misc/syslog-ng.persist-? -- gen_context(system_u:object_r:syslogd_var_lib_t,s0)
> +')

no need for ifdef.

> /var/axfrdns/log/main(/.*)? gen_context(system_u:object_r:var_log_t,s0)
> /var/dnscache/log/main(/.*)? gen_context(system_u:object_r:var_log_t,s0)
> /var/cfengine/outputs(/.*)? gen_context(system_u:object_r:var_log_t,s0)
> diff --git a/policy/modules/system/logging.te b/policy/modules/system/logging.te
> index 0034021..9cebc41 100644
> --- a/policy/modules/system/logging.te
> +++ b/policy/modules/system/logging.te
> @@ -439,6 +439,8 @@ files_read_etc_runtime_files(syslogd_t)
> # /initrd is not umounted before minilog starts
> files_dontaudit_search_isid_type_dirs(syslogd_t)
> files_read_kernel_symbol_table(syslogd_t)
> +files_rw_var_lib_dirs(syslogd_t)

above is redundant allowed below with files_var_lib_filetrans()

> +files_var_lib_filetrans(syslogd_t, syslogd_var_lib_t, { file dir })

only file transition on dirs. be conservative.

> fs_getattr_all_fs(syslogd_t)
> fs_search_auto_mountpoints(syslogd_t)

2012-08-29 19:55:28

by sven.vermeulen

[permalink] [raw]
Subject: [refpolicy] [PATCH v1 5/5] Udev's tables (run data) is stored in directories

On Wed, Aug 29, 2012 at 09:37:55PM +0200, Dominick Grift wrote:
> > -# create udev database in /dev/.udevdb
> > -allow udev_t udev_tbl_t:file manage_file_perms;
> > +allow udev_t udev_tbl_t:dir relabelto;
> > +manage_dirs_pattern(udev_t, udev_tbl_t, udev_tbl_t)
> > +manage_files_pattern(udev_t, udev_tbl_t, udev_tbl_t)
> > +manage_lnk_files_pattern(udev_t, udev_tbl_t, udev_tbl_t)
> > +
> > dev_filetrans(udev_t, udev_tbl_t, file)
>
> This doesnt make sense to me.
>
> First we had:
>
> allow udev_t udev_tbl_t:file manage_file_perms;
> dev_filetrans(udev_t, udev_tbl_t, file)
>
> with these specs:
>
> /dev/\.udev(/.*)? -- gen_context(system_u:object_r:udev_tbl_t,s0)
> /dev/\.udevdb -- gen_context(system_u:object_r:udev_tbl_t,s0)
> /dev/udev\.tbl -- gen_context(system_u:object_r:udev_tbl_t,s0)
>
> What does this tell me?
>
> Well there shouldnt be any dirs and symlinks with type udev_tbl_t.
>
> Only files. dirs and lnk_files should be device_t.

Well, the udev code (looking at udev-182 here) has the code for relabeling
in it. For instance, when copy_dev_dir is called, it has

#v+
udev_selinux_setfscreateconat(udev, dirfd(dir_to), dent->d_name, S_IFDIR|0755);
mkdirat(dirfd(dir_to), dent->d_name, 0755);
udev_selinux_resetfscreatecon(udev);
#v-

I believe this is the source, but I'm no master in this. I mainly based
myself on the denials and errors I got. If I put in an "auditallow" to show
this, this is the result:

#v+
testsys ~ # grep grant /var/log/avc.log
Aug 29 21:51:23 testsys kernel: [ 3.339771] type=1400 audit(1346269880.338:6): avc: granted { create } for pid=1162
comm="systemd-udevd" name="data" scontext=system_u:system_r:udev_t tcontext=system_u:object_r:udev_tbl_t tclass=dir
#v-

This, btw, is for the /var/run/udev/data location, and the file contexts for
udev does hold this as a udev_tbl_t currently:

#v+
testsys ~ # grep udev_tbl_t udev.fc
/dev/\.udev(/.*)? -- gen_context(system_u:object_r:udev_tbl_t,s0)
/dev/\.udevdb -- gen_context(system_u:object_r:udev_tbl_t,s0)
/dev/udev\.tbl -- gen_context(system_u:object_r:udev_tbl_t,s0)
/var/run/udev(/.*)? gen_context(system_u:object_r:udev_tbl_t,s0)
#v-

(last line)

Wkr,
Sven Vermeulen

2012-08-29 19:59:25

by sven.vermeulen

[permalink] [raw]
Subject: [refpolicy] [PATCH v1 2/5] Allow syslogd to create /var/lib/syslog and /var/lib/misc/syslog-ng.persist

On Wed, Aug 29, 2012 at 09:41:27PM +0200, Dominick Grift wrote:
> > +ifdef(`distro_gentoo', `
> > +/var/lib/misc/syslog-ng.persist-? -- gen_context(system_u:object_r:syslogd_var_lib_t,s0)
> > +')
>
> no need for ifdef.

ACK.

> > +files_rw_var_lib_dirs(syslogd_t)
>
> above is redundant allowed below with files_var_lib_filetrans()

ACK.

> > +files_var_lib_filetrans(syslogd_t, syslogd_var_lib_t, { file dir })
>
> only file transition on dirs. be conservative.

The file class transition is for the /var/lib/misc/syslog-ng.persist file.
Since /var/lib/misc is var_lib_t, we need a file transition here for
syslog-ng.persist to become syslogd_var_lib_t.

Perhaps it is safer to make named file transitions (perhaps even for both),
like so:

#v+
files_var_lib_filetrans(syslogd_t, syslogd_var_lib_t, file, "syslog-ng.persist")
files_var_lib_filetrans(syslogd_t, syslogd_var_lib_t, file, "syslog-ng.persist-")
files_var_lib_filetrans(syslogd_t, syslogd_var_lib_t, dir, "syslog")
#v-

On the other hand, I don't see syslog doing any other writes that would need
me limiting this. Although more precise, I don't know if it is better to
riddle the policy with named file transitions if they aren't needed.

Thanks for the feedback on the others, greatly appreciated!

Wkr,
Sven Vermeulen

2012-08-29 20:04:01

by dominick.grift

[permalink] [raw]
Subject: [refpolicy] [PATCH v1 5/5] Udev's tables (run data) is stored in directories



On Wed, 2012-08-29 at 21:55 +0200, Sven Vermeulen wrote:
> On Wed, Aug 29, 2012 at 09:37:55PM +0200, Dominick Grift wrote:
> > > -# create udev database in /dev/.udevdb
> > > -allow udev_t udev_tbl_t:file manage_file_perms;
> > > +allow udev_t udev_tbl_t:dir relabelto;
> > > +manage_dirs_pattern(udev_t, udev_tbl_t, udev_tbl_t)
> > > +manage_files_pattern(udev_t, udev_tbl_t, udev_tbl_t)
> > > +manage_lnk_files_pattern(udev_t, udev_tbl_t, udev_tbl_t)
> > > +
> > > dev_filetrans(udev_t, udev_tbl_t, file)
> >
> > This doesnt make sense to me.
> >
> > First we had:
> >
> > allow udev_t udev_tbl_t:file manage_file_perms;
> > dev_filetrans(udev_t, udev_tbl_t, file)
> >
> > with these specs:
> >
> > /dev/\.udev(/.*)? -- gen_context(system_u:object_r:udev_tbl_t,s0)
> > /dev/\.udevdb -- gen_context(system_u:object_r:udev_tbl_t,s0)
> > /dev/udev\.tbl -- gen_context(system_u:object_r:udev_tbl_t,s0)
> >
> > What does this tell me?
> >
> > Well there shouldnt be any dirs and symlinks with type udev_tbl_t.
> >
> > Only files. dirs and lnk_files should be device_t.
>
> Well, the udev code (looking at udev-182 here) has the code for relabeling
> in it. For instance, when copy_dev_dir is called, it has
>
> #v+
> udev_selinux_setfscreateconat(udev, dirfd(dir_to), dent->d_name, S_IFDIR|0755);
> mkdirat(dirfd(dir_to), dent->d_name, 0755);
> udev_selinux_resetfscreatecon(udev);
> #v-
>
> I believe this is the source, but I'm no master in this. I mainly based
> myself on the denials and errors I got. If I put in an "auditallow" to show
> this, this is the result:
>
> #v+
> testsys ~ # grep grant /var/log/avc.log
> Aug 29 21:51:23 testsys kernel: [ 3.339771] type=1400 audit(1346269880.338:6): avc: granted { create } for pid=1162
> comm="systemd-udevd" name="data" scontext=system_u:system_r:udev_t tcontext=system_u:object_r:udev_tbl_t tclass=dir
> #v-
>
> This, btw, is for the /var/run/udev/data location, and the file contexts for
> udev does hold this as a udev_tbl_t currently:
>
> #v+
> testsys ~ # grep udev_tbl_t udev.fc
> /dev/\.udev(/.*)? -- gen_context(system_u:object_r:udev_tbl_t,s0)
> /dev/\.udevdb -- gen_context(system_u:object_r:udev_tbl_t,s0)
> /dev/udev\.tbl -- gen_context(system_u:object_r:udev_tbl_t,s0)
> /var/run/udev(/.*)? gen_context(system_u:object_r:udev_tbl_t,s0)
> #v-
>
> (last line)
>

Thats just udev trying to be smart. It should stick to its own business.

dirs and symlinks in /dev should be type device_t as evidence by the
file context specs and current policy.

/var/run/udev is a different story

> Wkr,
> Sven Vermeulen
> _______________________________________________
> refpolicy mailing list
> refpolicy at oss.tresys.com
> http://oss.tresys.com/mailman/listinfo/refpolicy

2012-08-29 20:10:10

by dominick.grift

[permalink] [raw]
Subject: [refpolicy] [PATCH v1 2/5] Allow syslogd to create /var/lib/syslog and /var/lib/misc/syslog-ng.persist



On Wed, 2012-08-29 at 21:59 +0200, Sven Vermeulen wrote:
> On Wed, Aug 29, 2012 at 09:41:27PM +0200, Dominick Grift wrote:
> > > +ifdef(`distro_gentoo', `
> > > +/var/lib/misc/syslog-ng.persist-? -- gen_context(system_u:object_r:syslogd_var_lib_t,s0)
> > > +')
> >
> > no need for ifdef.
>
> ACK.
>
> > > +files_rw_var_lib_dirs(syslogd_t)
> >
> > above is redundant allowed below with files_var_lib_filetrans()
>
> ACK.
>
> > > +files_var_lib_filetrans(syslogd_t, syslogd_var_lib_t, { file dir })
> >
> > only file transition on dirs. be conservative.
>
> The file class transition is for the /var/lib/misc/syslog-ng.persist file.
> Since /var/lib/misc is var_lib_t, we need a file transition here for
> syslog-ng.persist to become syslogd_var_lib_t.
>
> Perhaps it is safer to make named file transitions (perhaps even for both),
> like so:
>
> #v+
> files_var_lib_filetrans(syslogd_t, syslogd_var_lib_t, file, "syslog-ng.persist")
> files_var_lib_filetrans(syslogd_t, syslogd_var_lib_t, file, "syslog-ng.persist-")
> files_var_lib_filetrans(syslogd_t, syslogd_var_lib_t, dir, "syslog")
> #v-
>
> On the other hand, I don't see syslog doing any other writes that would need
> me limiting this. Although more precise, I don't know if it is better to
> riddle the policy with named file transitions if they aren't needed.
>
> Thanks for the feedback on the others, greatly appreciated!
>

No need for named file transitions. if it create both files and dirs in
var_lib_t directories then you indeed need a filetrans for both files
and dirs.

I assumed that it only created dirs in var_lib_t dirs

> Wkr,
> Sven Vermeulen
> _______________________________________________
> refpolicy mailing list
> refpolicy at oss.tresys.com
> http://oss.tresys.com/mailman/listinfo/refpolicy

2012-08-29 20:20:37

by dominick.grift

[permalink] [raw]
Subject: [refpolicy] [PATCH v1 5/5] Udev's tables (run data) is stored in directories



On Wed, 2012-08-29 at 22:04 +0200, Dominick Grift wrote:
>
> On Wed, 2012-08-29 at 21:55 +0200, Sven Vermeulen wrote:
> > On Wed, Aug 29, 2012 at 09:37:55PM +0200, Dominick Grift wrote:
> > > > -# create udev database in /dev/.udevdb
> > > > -allow udev_t udev_tbl_t:file manage_file_perms;
> > > > +allow udev_t udev_tbl_t:dir relabelto;
> > > > +manage_dirs_pattern(udev_t, udev_tbl_t, udev_tbl_t)
> > > > +manage_files_pattern(udev_t, udev_tbl_t, udev_tbl_t)
> > > > +manage_lnk_files_pattern(udev_t, udev_tbl_t, udev_tbl_t)
> > > > +
> > > > dev_filetrans(udev_t, udev_tbl_t, file)
> > >
> > > This doesnt make sense to me.
> > >
> > > First we had:
> > >
> > > allow udev_t udev_tbl_t:file manage_file_perms;
> > > dev_filetrans(udev_t, udev_tbl_t, file)
> > >
> > > with these specs:
> > >
> > > /dev/\.udev(/.*)? -- gen_context(system_u:object_r:udev_tbl_t,s0)
> > > /dev/\.udevdb -- gen_context(system_u:object_r:udev_tbl_t,s0)
> > > /dev/udev\.tbl -- gen_context(system_u:object_r:udev_tbl_t,s0)
> > >
> > > What does this tell me?
> > >
> > > Well there shouldnt be any dirs and symlinks with type udev_tbl_t.
> > >
> > > Only files. dirs and lnk_files should be device_t.
> >
> > Well, the udev code (looking at udev-182 here) has the code for relabeling
> > in it. For instance, when copy_dev_dir is called, it has
> >
> > #v+
> > udev_selinux_setfscreateconat(udev, dirfd(dir_to), dent->d_name, S_IFDIR|0755);
> > mkdirat(dirfd(dir_to), dent->d_name, 0755);
> > udev_selinux_resetfscreatecon(udev);
> > #v-
> >
> > I believe this is the source, but I'm no master in this. I mainly based
> > myself on the denials and errors I got. If I put in an "auditallow" to show
> > this, this is the result:
> >
> > #v+
> > testsys ~ # grep grant /var/log/avc.log
> > Aug 29 21:51:23 testsys kernel: [ 3.339771] type=1400 audit(1346269880.338:6): avc: granted { create } for pid=1162
> > comm="systemd-udevd" name="data" scontext=system_u:system_r:udev_t tcontext=system_u:object_r:udev_tbl_t tclass=dir
> > #v-
> >
> > This, btw, is for the /var/run/udev/data location, and the file contexts for
> > udev does hold this as a udev_tbl_t currently:
> >
> > #v+
> > testsys ~ # grep udev_tbl_t udev.fc
> > /dev/\.udev(/.*)? -- gen_context(system_u:object_r:udev_tbl_t,s0)
> > /dev/\.udevdb -- gen_context(system_u:object_r:udev_tbl_t,s0)
> > /dev/udev\.tbl -- gen_context(system_u:object_r:udev_tbl_t,s0)
> > /var/run/udev(/.*)? gen_context(system_u:object_r:udev_tbl_t,s0)
> > #v-
> >
> > (last line)
> >
>
> Thats just udev trying to be smart. It should stick to its own business.
>
> dirs and symlinks in /dev should be type device_t as evidence by the
> file context specs and current policy.
>
> /var/run/udev is a different story
>

Oops sorry didnt take time to think again.

Weird. so theres a file context spec :

/var/run/udev(/.*)? gen_context(system_u:object_r:udev_tbl_t,s0)

but no rules to support it?

Strange indeed.

> > Wkr,
> > Sven Vermeulen
> > _______________________________________________
> > refpolicy mailing list
> > refpolicy at oss.tresys.com
> > http://oss.tresys.com/mailman/listinfo/refpolicy
>
>

2012-08-29 20:31:06

by dominick.grift

[permalink] [raw]
Subject: [refpolicy] [PATCH v1 5/5] Udev's tables (run data) is stored in directories



On Wed, 2012-08-29 at 22:20 +0200, Dominick Grift wrote:
>
> On Wed, 2012-08-29 at 22:04 +0200, Dominick Grift wrote:
> >
> > On Wed, 2012-08-29 at 21:55 +0200, Sven Vermeulen wrote:
> > > On Wed, Aug 29, 2012 at 09:37:55PM +0200, Dominick Grift wrote:
> > > > > -# create udev database in /dev/.udevdb
> > > > > -allow udev_t udev_tbl_t:file manage_file_perms;
> > > > > +allow udev_t udev_tbl_t:dir relabelto;
> > > > > +manage_dirs_pattern(udev_t, udev_tbl_t, udev_tbl_t)
> > > > > +manage_files_pattern(udev_t, udev_tbl_t, udev_tbl_t)
> > > > > +manage_lnk_files_pattern(udev_t, udev_tbl_t, udev_tbl_t)
> > > > > +
> > > > > dev_filetrans(udev_t, udev_tbl_t, file)
> > > >
> > > > This doesnt make sense to me.
> > > >
> > > > First we had:
> > > >
> > > > allow udev_t udev_tbl_t:file manage_file_perms;
> > > > dev_filetrans(udev_t, udev_tbl_t, file)
> > > >
> > > > with these specs:
> > > >
> > > > /dev/\.udev(/.*)? -- gen_context(system_u:object_r:udev_tbl_t,s0)
> > > > /dev/\.udevdb -- gen_context(system_u:object_r:udev_tbl_t,s0)
> > > > /dev/udev\.tbl -- gen_context(system_u:object_r:udev_tbl_t,s0)
> > > >
> > > > What does this tell me?
> > > >
> > > > Well there shouldnt be any dirs and symlinks with type udev_tbl_t.
> > > >
> > > > Only files. dirs and lnk_files should be device_t.
> > >
> > > Well, the udev code (looking at udev-182 here) has the code for relabeling
> > > in it. For instance, when copy_dev_dir is called, it has
> > >
> > > #v+
> > > udev_selinux_setfscreateconat(udev, dirfd(dir_to), dent->d_name, S_IFDIR|0755);
> > > mkdirat(dirfd(dir_to), dent->d_name, 0755);
> > > udev_selinux_resetfscreatecon(udev);
> > > #v-
> > >
> > > I believe this is the source, but I'm no master in this. I mainly based
> > > myself on the denials and errors I got. If I put in an "auditallow" to show
> > > this, this is the result:
> > >
> > > #v+
> > > testsys ~ # grep grant /var/log/avc.log
> > > Aug 29 21:51:23 testsys kernel: [ 3.339771] type=1400 audit(1346269880.338:6): avc: granted { create } for pid=1162
> > > comm="systemd-udevd" name="data" scontext=system_u:system_r:udev_t tcontext=system_u:object_r:udev_tbl_t tclass=dir
> > > #v-
> > >
> > > This, btw, is for the /var/run/udev/data location, and the file contexts for
> > > udev does hold this as a udev_tbl_t currently:
> > >
> > > #v+
> > > testsys ~ # grep udev_tbl_t udev.fc
> > > /dev/\.udev(/.*)? -- gen_context(system_u:object_r:udev_tbl_t,s0)
> > > /dev/\.udevdb -- gen_context(system_u:object_r:udev_tbl_t,s0)
> > > /dev/udev\.tbl -- gen_context(system_u:object_r:udev_tbl_t,s0)
> > > /var/run/udev(/.*)? gen_context(system_u:object_r:udev_tbl_t,s0)
> > > #v-
> > >
> > > (last line)
> > >
> >
> > Thats just udev trying to be smart. It should stick to its own business.
> >
> > dirs and symlinks in /dev should be type device_t as evidence by the
> > file context specs and current policy.
> >
> > /var/run/udev is a different story
> >
>
> Oops sorry didnt take time to think again.
>
> Weird. so theres a file context spec :
>
> /var/run/udev(/.*)? gen_context(system_u:object_r:udev_tbl_t,s0)
>
> but no rules to support it?
>
> Strange indeed.

/var/run/udev(/.*)? should be type udev_var_run_t i believe

# matchpathcon /var/run/udev/
/var/run/udev system_u:object_r:udev_var_run_t:s0
# ls -alZ /var/run/udev/
drwxr-xr-x. root root system_u:object_r:udev_var_run_t:s0 .
drwxr-xr-x. root root system_u:object_r:var_run_t:s0 ..
srw-------. root root system_u:object_r:udev_var_run_t:s0 control
drwxr-xr-x. root root system_u:object_r:udev_var_run_t:s0 data
drwxr-xr-x. root root system_u:object_r:udev_var_run_t:s0 links
-rw-r--r--. root root system_u:object_r:udev_var_run_t:s0 queue.bin
drwxr-xr-x. root root system_u:object_r:udev_var_run_t:s0 rules.d
drwxr-xr-x. root root system_u:object_r:udev_var_run_t:s0 tags
drwxr-xr-x. root root system_u:object_r:udev_var_run_t:s0 watch

Either that or Fedora has it wrong as well


> > > Wkr,
> > > Sven Vermeulen
> > > _______________________________________________
> > > refpolicy mailing list
> > > refpolicy at oss.tresys.com
> > > http://oss.tresys.com/mailman/listinfo/refpolicy
> >
> >
>
>

2012-09-02 12:06:20

by Guido Trentalancia

[permalink] [raw]
Subject: [refpolicy] [PATCH v1 5/5] Udev's tables (run data) is stored in directories

On Wed, 2012-08-29 at 22:31 +0200, Dominick Grift wrote:
>
> On Wed, 2012-08-29 at 22:20 +0200, Dominick Grift wrote:
> >
> > On Wed, 2012-08-29 at 22:04 +0200, Dominick Grift wrote:
> > >
> > > On Wed, 2012-08-29 at 21:55 +0200, Sven Vermeulen wrote:
> > > > On Wed, Aug 29, 2012 at 09:37:55PM +0200, Dominick Grift wrote:
> > > > > > -# create udev database in /dev/.udevdb
> > > > > > -allow udev_t udev_tbl_t:file manage_file_perms;
> > > > > > +allow udev_t udev_tbl_t:dir relabelto;
> > > > > > +manage_dirs_pattern(udev_t, udev_tbl_t, udev_tbl_t)
> > > > > > +manage_files_pattern(udev_t, udev_tbl_t, udev_tbl_t)
> > > > > > +manage_lnk_files_pattern(udev_t, udev_tbl_t, udev_tbl_t)
> > > > > > +
> > > > > > dev_filetrans(udev_t, udev_tbl_t, file)
> > > > >
> > > > > This doesnt make sense to me.

[cut]

> > > > Well, the udev code (looking at udev-182 here) has the code for relabeling
> > > > in it. For instance, when copy_dev_dir is called, it has
> > > >
> > > > #v+
> > > > udev_selinux_setfscreateconat(udev, dirfd(dir_to), dent->d_name, S_IFDIR|0755);
> > > > mkdirat(dirfd(dir_to), dent->d_name, 0755);
> > > > udev_selinux_resetfscreatecon(udev);
> > > > #v-
> > > >
> > > > I believe this is the source, but I'm no master in this. I mainly based
> > > > myself on the denials and errors I got. If I put in an "auditallow" to show
> > > > this, this is the result:

At a quick look, the code in most recent versions (the one merged in
systemd) apparently has been extended even further in that direction by
adding a shared label.c source file.

As for such labelling issue, since it is usually always preferable that
each one sticks to its own job, perhaps it can be dontaudited and then
the relevant "dynamic" path added to restorecond.conf ?

The strange thing is that I am running latest udev without any of these
modifications (thus including the relabel permission)...

Regards,

Guido

2012-09-02 19:51:43

by dominick.grift

[permalink] [raw]
Subject: [refpolicy] [PATCH v1 5/5] Udev's tables (run data) is stored in directories



On Sun, 2012-09-02 at 14:06 +0200, Guido Trentalancia wrote:
> On Wed, 2012-08-29 at 22:31 +0200, Dominick Grift wrote:
> >
> > On Wed, 2012-08-29 at 22:20 +0200, Dominick Grift wrote:
> > >
> > > On Wed, 2012-08-29 at 22:04 +0200, Dominick Grift wrote:
> > > >
> > > > On Wed, 2012-08-29 at 21:55 +0200, Sven Vermeulen wrote:
> > > > > On Wed, Aug 29, 2012 at 09:37:55PM +0200, Dominick Grift wrote:
> > > > > > > -# create udev database in /dev/.udevdb
> > > > > > > -allow udev_t udev_tbl_t:file manage_file_perms;
> > > > > > > +allow udev_t udev_tbl_t:dir relabelto;
> > > > > > > +manage_dirs_pattern(udev_t, udev_tbl_t, udev_tbl_t)
> > > > > > > +manage_files_pattern(udev_t, udev_tbl_t, udev_tbl_t)
> > > > > > > +manage_lnk_files_pattern(udev_t, udev_tbl_t, udev_tbl_t)
> > > > > > > +
> > > > > > > dev_filetrans(udev_t, udev_tbl_t, file)
> > > > > >
> > > > > > This doesnt make sense to me.
>
> [cut]
>
> > > > > Well, the udev code (looking at udev-182 here) has the code for relabeling
> > > > > in it. For instance, when copy_dev_dir is called, it has
> > > > >
> > > > > #v+
> > > > > udev_selinux_setfscreateconat(udev, dirfd(dir_to), dent->d_name, S_IFDIR|0755);
> > > > > mkdirat(dirfd(dir_to), dent->d_name, 0755);
> > > > > udev_selinux_resetfscreatecon(udev);
> > > > > #v-
> > > > >
> > > > > I believe this is the source, but I'm no master in this. I mainly based
> > > > > myself on the denials and errors I got. If I put in an "auditallow" to show
> > > > > this, this is the result:
>
> At a quick look, the code in most recent versions (the one merged in
> systemd) apparently has been extended even further in that direction by
> adding a shared label.c source file.
>
> As for such labelling issue, since it is usually always preferable that
> each one sticks to its own job, perhaps it can be dontaudited and then
> the relevant "dynamic" path added to restorecond.conf ?
>
> The strange thing is that I am running latest udev without any of these
> modifications (thus including the relabel permission)...

I am currently rewriting udev policy as part of my project to write a
systemd policy and i havent noticed this either yet, although it is
hinting at it (it needs setfscreate capability and so does systemd by
the way)

I am not letting these daemons run my security though.

> Regards,
>
> Guido
>
>
>

2012-09-02 19:59:58

by dominick.grift

[permalink] [raw]
Subject: [refpolicy] [PATCH v1 5/5] Udev's tables (run data) is stored in directories



On Sun, 2012-09-02 at 21:51 +0200, Dominick Grift wrote:
>
> On Sun, 2012-09-02 at 14:06 +0200, Guido Trentalancia wrote:
> > On Wed, 2012-08-29 at 22:31 +0200, Dominick Grift wrote:
> > >
> > > On Wed, 2012-08-29 at 22:20 +0200, Dominick Grift wrote:
> > > >
> > > > On Wed, 2012-08-29 at 22:04 +0200, Dominick Grift wrote:
> > > > >
> > > > > On Wed, 2012-08-29 at 21:55 +0200, Sven Vermeulen wrote:
> > > > > > On Wed, Aug 29, 2012 at 09:37:55PM +0200, Dominick Grift wrote:
> > > > > > > > -# create udev database in /dev/.udevdb
> > > > > > > > -allow udev_t udev_tbl_t:file manage_file_perms;
> > > > > > > > +allow udev_t udev_tbl_t:dir relabelto;
> > > > > > > > +manage_dirs_pattern(udev_t, udev_tbl_t, udev_tbl_t)
> > > > > > > > +manage_files_pattern(udev_t, udev_tbl_t, udev_tbl_t)
> > > > > > > > +manage_lnk_files_pattern(udev_t, udev_tbl_t, udev_tbl_t)
> > > > > > > > +
> > > > > > > > dev_filetrans(udev_t, udev_tbl_t, file)
> > > > > > >
> > > > > > > This doesnt make sense to me.
> >
> > [cut]
> >
> > > > > > Well, the udev code (looking at udev-182 here) has the code for relabeling
> > > > > > in it. For instance, when copy_dev_dir is called, it has
> > > > > >
> > > > > > #v+
> > > > > > udev_selinux_setfscreateconat(udev, dirfd(dir_to), dent->d_name, S_IFDIR|0755);
> > > > > > mkdirat(dirfd(dir_to), dent->d_name, 0755);
> > > > > > udev_selinux_resetfscreatecon(udev);
> > > > > > #v-
> > > > > >
> > > > > > I believe this is the source, but I'm no master in this. I mainly based
> > > > > > myself on the denials and errors I got. If I put in an "auditallow" to show
> > > > > > this, this is the result:
> >
> > At a quick look, the code in most recent versions (the one merged in
> > systemd) apparently has been extended even further in that direction by
> > adding a shared label.c source file.
> >
> > As for such labelling issue, since it is usually always preferable that
> > each one sticks to its own job, perhaps it can be dontaudited and then
> > the relevant "dynamic" path added to restorecond.conf ?
> >
> > The strange thing is that I am running latest udev without any of these
> > modifications (thus including the relabel permission)...
>
> I am currently rewriting udev policy as part of my project to write a
> systemd policy and i havent noticed this either yet, although it is
> hinting at it (it needs setfscreate capability and so does systemd by
> the way)
>
> I am not letting these daemons run my security though.

That plus i do not like hard-coding types. it is degrading selinux

> > Regards,
> >
> > Guido
> >
> >
> >
>
>

2012-09-03 09:26:22

by Guido Trentalancia

[permalink] [raw]
Subject: [refpolicy] [PATCH v1 5/5] Udev's tables (run data) is stored in directories

On 02/09/2012 21:51, Dominick Grift wrote:
>
>
> On Sun, 2012-09-02 at 14:06 +0200, Guido Trentalancia wrote:
>> On Wed, 2012-08-29 at 22:31 +0200, Dominick Grift wrote:
>>>
>>> On Wed, 2012-08-29 at 22:20 +0200, Dominick Grift wrote:
>>>>
>>>> On Wed, 2012-08-29 at 22:04 +0200, Dominick Grift wrote:
>>>>>
>>>>> On Wed, 2012-08-29 at 21:55 +0200, Sven Vermeulen wrote:
>>>>>> On Wed, Aug 29, 2012 at 09:37:55PM +0200, Dominick Grift wrote:
>>>>>>>> -# create udev database in /dev/.udevdb
>>>>>>>> -allow udev_t udev_tbl_t:file manage_file_perms;
>>>>>>>> +allow udev_t udev_tbl_t:dir relabelto;
>>>>>>>> +manage_dirs_pattern(udev_t, udev_tbl_t, udev_tbl_t)
>>>>>>>> +manage_files_pattern(udev_t, udev_tbl_t, udev_tbl_t)
>>>>>>>> +manage_lnk_files_pattern(udev_t, udev_tbl_t, udev_tbl_t)
>>>>>>>> +
>>>>>>>> dev_filetrans(udev_t, udev_tbl_t, file)
>>>>>>>
>>>>>>> This doesnt make sense to me.
>>
>> [cut]
>>
>>>>>> Well, the udev code (looking at udev-182 here) has the code for relabeling
>>>>>> in it. For instance, when copy_dev_dir is called, it has
>>>>>>
>>>>>> #v+
>>>>>> udev_selinux_setfscreateconat(udev, dirfd(dir_to), dent->d_name, S_IFDIR|0755);
>>>>>> mkdirat(dirfd(dir_to), dent->d_name, 0755);
>>>>>> udev_selinux_resetfscreatecon(udev);
>>>>>> #v-
>>>>>>
>>>>>> I believe this is the source, but I'm no master in this. I mainly based
>>>>>> myself on the denials and errors I got. If I put in an "auditallow" to show
>>>>>> this, this is the result:
>>
>> At a quick look, the code in most recent versions (the one merged in
>> systemd) apparently has been extended even further in that direction by
>> adding a shared label.c source file.
>>
>> As for such labelling issue, since it is usually always preferable that
>> each one sticks to its own job, perhaps it can be dontaudited and then
>> the relevant "dynamic" path added to restorecond.conf ?
>>
>> The strange thing is that I am running latest udev without any of these
>> modifications (thus including the relabel permission)...

I am not 100% sure yet about the above because I still need to complete
one more test, nevertheless I am a bit sceptic about it.

Relabelling is somewhat critical when done externally by other entities.

Also, as already pointed out, the opportunity that each one should stick
to its own job if possible is a very good idea.

We have restorecond for setting the context of newly created files in
paths that contain such "dynamic" files. There is no proved reason at
the moment to think that restorecond (if started at the right time) is
not up the job.

> I am currently rewriting udev policy as part of my project to write a
> systemd policy and i havent noticed this either yet, although it is
> hinting at it (it needs setfscreate capability and so does systemd by
> the way)
>
> I am not letting these daemons run my security though.

Yes, let's see how it goes. Perhaps we can come up with something better...

Regards,

Guido

2012-09-04 10:18:58

by mgrepl

[permalink] [raw]
Subject: [refpolicy] [PATCH v1 5/5] Udev's tables (run data) is stored in directories

On 09/02/2012 09:51 PM, Dominick Grift wrote:
>
> On Sun, 2012-09-02 at 14:06 +0200, Guido Trentalancia wrote:
>> On Wed, 2012-08-29 at 22:31 +0200, Dominick Grift wrote:
>>> On Wed, 2012-08-29 at 22:20 +0200, Dominick Grift wrote:
>>>> On Wed, 2012-08-29 at 22:04 +0200, Dominick Grift wrote:
>>>>> On Wed, 2012-08-29 at 21:55 +0200, Sven Vermeulen wrote:
>>>>>> On Wed, Aug 29, 2012 at 09:37:55PM +0200, Dominick Grift wrote:
>>>>>>>> -# create udev database in /dev/.udevdb
>>>>>>>> -allow udev_t udev_tbl_t:file manage_file_perms;
>>>>>>>> +allow udev_t udev_tbl_t:dir relabelto;
>>>>>>>> +manage_dirs_pattern(udev_t, udev_tbl_t, udev_tbl_t)
>>>>>>>> +manage_files_pattern(udev_t, udev_tbl_t, udev_tbl_t)
>>>>>>>> +manage_lnk_files_pattern(udev_t, udev_tbl_t, udev_tbl_t)
>>>>>>>> +
>>>>>>>> dev_filetrans(udev_t, udev_tbl_t, file)
>>>>>>> This doesnt make sense to me.
>> [cut]
>>
>>>>>> Well, the udev code (looking at udev-182 here) has the code for relabeling
>>>>>> in it. For instance, when copy_dev_dir is called, it has
>>>>>>
>>>>>> #v+
>>>>>> udev_selinux_setfscreateconat(udev, dirfd(dir_to), dent->d_name, S_IFDIR|0755);
>>>>>> mkdirat(dirfd(dir_to), dent->d_name, 0755);
>>>>>> udev_selinux_resetfscreatecon(udev);
>>>>>> #v-
>>>>>>
>>>>>> I believe this is the source, but I'm no master in this. I mainly based
>>>>>> myself on the denials and errors I got. If I put in an "auditallow" to show
>>>>>> this, this is the result:
>> At a quick look, the code in most recent versions (the one merged in
>> systemd) apparently has been extended even further in that direction by
>> adding a shared label.c source file.
>>
>> As for such labelling issue, since it is usually always preferable that
>> each one sticks to its own job, perhaps it can be dontaudited and then
>> the relevant "dynamic" path added to restorecond.conf ?
>>
>> The strange thing is that I am running latest udev without any of these
>> modifications (thus including the relabel permission)...
> I am currently rewriting udev policy as part of my project to write a
> systemd policy
Is it based on Fedora systemd policy?
> and i havent noticed this either yet, although it is
> hinting at it (it needs setfscreate capability and so does systemd by
> the way)
>
> I am not letting these daemons run my security though.
>
>> Regards,
>>
>> Guido
>>
>>
>>
>
> _______________________________________________
> refpolicy mailing list
> refpolicy at oss.tresys.com
> http://oss.tresys.com/mailman/listinfo/refpolicy

2012-09-04 12:27:02

by Guido Trentalancia

[permalink] [raw]
Subject: [refpolicy] [PATCH v1 5/5] Udev's tables (run data) is stored in directories

On Sun, 2012-09-02 at 21:59 +0200, Dominick Grift wrote:
>
> On Sun, 2012-09-02 at 21:51 +0200, Dominick Grift wrote:
> >
> > On Sun, 2012-09-02 at 14:06 +0200, Guido Trentalancia wrote:
> > > On Wed, 2012-08-29 at 22:31 +0200, Dominick Grift wrote:
> > > >
> > > > On Wed, 2012-08-29 at 22:20 +0200, Dominick Grift wrote:
> > > > >
> > > > > On Wed, 2012-08-29 at 22:04 +0200, Dominick Grift wrote:
> > > > > >
> > > > > > On Wed, 2012-08-29 at 21:55 +0200, Sven Vermeulen wrote:
> > > > > > > On Wed, Aug 29, 2012 at 09:37:55PM +0200, Dominick Grift wrote:
> > > > > > > > > -# create udev database in /dev/.udevdb
> > > > > > > > > -allow udev_t udev_tbl_t:file manage_file_perms;
> > > > > > > > > +allow udev_t udev_tbl_t:dir relabelto;
> > > > > > > > > +manage_dirs_pattern(udev_t, udev_tbl_t, udev_tbl_t)
> > > > > > > > > +manage_files_pattern(udev_t, udev_tbl_t, udev_tbl_t)
> > > > > > > > > +manage_lnk_files_pattern(udev_t, udev_tbl_t, udev_tbl_t)
> > > > > > > > > +
> > > > > > > > > dev_filetrans(udev_t, udev_tbl_t, file)
> > > > > > > >
> > > > > > > > This doesnt make sense to me.
> > >
> > > [cut]
> > >
> > > > > > > Well, the udev code (looking at udev-182 here) has the code for relabeling
> > > > > > > in it. For instance, when copy_dev_dir is called, it has
> > > > > > >
> > > > > > > #v+
> > > > > > > udev_selinux_setfscreateconat(udev, dirfd(dir_to), dent->d_name, S_IFDIR|0755);
> > > > > > > mkdirat(dirfd(dir_to), dent->d_name, 0755);
> > > > > > > udev_selinux_resetfscreatecon(udev);
> > > > > > > #v-
> > > > > > >
> > > > > > > I believe this is the source, but I'm no master in this. I mainly based
> > > > > > > myself on the denials and errors I got. If I put in an "auditallow" to show
> > > > > > > this, this is the result:
> > >
> > > At a quick look, the code in most recent versions (the one merged in
> > > systemd) apparently has been extended even further in that direction by
> > > adding a shared label.c source file.
> > >
> > > As for such labelling issue, since it is usually always preferable that
> > > each one sticks to its own job, perhaps it can be dontaudited and then
> > > the relevant "dynamic" path added to restorecond.conf ?
> > >
> > > The strange thing is that I am running latest udev without any of these
> > > modifications (thus including the relabel permission)...
> >
> > I am currently rewriting udev policy as part of my project to write a
> > systemd policy and i havent noticed this either yet, although it is
> > hinting at it (it needs setfscreate capability and so does systemd by
> > the way)
> >
> > I am not letting these daemons run my security though.

As already said, relabelling is critical, it is SELinux-domain by
definition and therefore it should not normally be required/allowed. It
makes me wonder whether the daemon is getting slightly over-engineered ?

>From raw testing, it seems that it produces a few warnings:

udev: Failed to initialize SELinux context: No such file or directory
udev: failed to create queue file: No such file or directory

Have you tried to test it without the new permissions ?

> That plus i do not like hard-coding types. it is degrading selinux

What do you mean exactly for hard-coding types ?

Degradation just for the sake of keeping things running is bad as in
allowing permissions that are not normally required for carrying out a
given task.

At the end, it has just more or less rebased from /var/run and /dev
to /run/udev.

Regards,

Guido

2012-09-04 14:51:09

by Guido Trentalancia

[permalink] [raw]
Subject: [refpolicy] [PATCH v1 5/5] Udev's tables (run data) is stored in directories

On 04/09/2012 14:27, Guido Trentalancia wrote:
> On Sun, 2012-09-02 at 21:59 +0200, Dominick Grift wrote:
>>
>> On Sun, 2012-09-02 at 21:51 +0200, Dominick Grift wrote:
>>>
>>> On Sun, 2012-09-02 at 14:06 +0200, Guido Trentalancia wrote:
>>>> On Wed, 2012-08-29 at 22:31 +0200, Dominick Grift wrote:
>>>>>
>>>>> On Wed, 2012-08-29 at 22:20 +0200, Dominick Grift wrote:
>>>>>>
>>>>>> On Wed, 2012-08-29 at 22:04 +0200, Dominick Grift wrote:
>>>>>>>
>>>>>>> On Wed, 2012-08-29 at 21:55 +0200, Sven Vermeulen wrote:
>>>>>>>> On Wed, Aug 29, 2012 at 09:37:55PM +0200, Dominick Grift wrote:
>>>>>>>>>> -# create udev database in /dev/.udevdb
>>>>>>>>>> -allow udev_t udev_tbl_t:file manage_file_perms;
>>>>>>>>>> +allow udev_t udev_tbl_t:dir relabelto;
>>>>>>>>>> +manage_dirs_pattern(udev_t, udev_tbl_t, udev_tbl_t)
>>>>>>>>>> +manage_files_pattern(udev_t, udev_tbl_t, udev_tbl_t)
>>>>>>>>>> +manage_lnk_files_pattern(udev_t, udev_tbl_t, udev_tbl_t)
>>>>>>>>>> +
>>>>>>>>>> dev_filetrans(udev_t, udev_tbl_t, file)
>>>>>>>>>
>>>>>>>>> This doesnt make sense to me.
>>>>
>>>> [cut]
>>>>
>>>>>>>> Well, the udev code (looking at udev-182 here) has the code for relabeling
>>>>>>>> in it. For instance, when copy_dev_dir is called, it has
>>>>>>>>
>>>>>>>> #v+
>>>>>>>> udev_selinux_setfscreateconat(udev, dirfd(dir_to), dent->d_name, S_IFDIR|0755);
>>>>>>>> mkdirat(dirfd(dir_to), dent->d_name, 0755);
>>>>>>>> udev_selinux_resetfscreatecon(udev);
>>>>>>>> #v-
>>>>>>>>
>>>>>>>> I believe this is the source, but I'm no master in this. I mainly based
>>>>>>>> myself on the denials and errors I got. If I put in an "auditallow" to show
>>>>>>>> this, this is the result:
>>>>
>>>> At a quick look, the code in most recent versions (the one merged in
>>>> systemd) apparently has been extended even further in that direction by
>>>> adding a shared label.c source file.
>>>>
>>>> As for such labelling issue, since it is usually always preferable that
>>>> each one sticks to its own job, perhaps it can be dontaudited and then
>>>> the relevant "dynamic" path added to restorecond.conf ?
>>>>
>>>> The strange thing is that I am running latest udev without any of these
>>>> modifications (thus including the relabel permission)...
>>>
>>> I am currently rewriting udev policy as part of my project to write a
>>> systemd policy and i havent noticed this either yet, although it is
>>> hinting at it (it needs setfscreate capability and so does systemd by
>>> the way)
>>>
>>> I am not letting these daemons run my security though.
>
> As already said, relabelling is critical, it is SELinux-domain by
> definition and therefore it should not normally be required/allowed. It
> makes me wonder whether the daemon is getting slightly over-engineered ?

Init is supposed to be a simple thing in functionality which switches
the runlevels and eventually shuts down the system...

Is this going to be a third replacement of old good init ? Perhaps
aiming at centralizing everything in one daemon ?

Everything is digital there will always going to be one single point of
failure anyway, from a single line of C code for example, down to an
assembly instruction and ultimately to the value of one single bit...

The system I am using for testing is still using upstart as init and it
is now a bit unstable after all these changes...

This centralization is diverting from modularity, while the point of
failure will always ultimately be one single bit.

Hopefully it will become stable again soon...

Regards,

Guido

2012-09-09 16:51:33

by Guido Trentalancia

[permalink] [raw]
Subject: [refpolicy] [PATCH v1 5/5] Udev's tables (run data) is stored in directories

On 04/09/2012 14:27, Guido Trentalancia wrote:
> On Sun, 2012-09-02 at 21:59 +0200, Dominick Grift wrote:
>>
>> On Sun, 2012-09-02 at 21:51 +0200, Dominick Grift wrote:
>>>
>>> On Sun, 2012-09-02 at 14:06 +0200, Guido Trentalancia wrote:
>>>> On Wed, 2012-08-29 at 22:31 +0200, Dominick Grift wrote:
>>>>>
>>>>> On Wed, 2012-08-29 at 22:20 +0200, Dominick Grift wrote:
>>>>>>
>>>>>> On Wed, 2012-08-29 at 22:04 +0200, Dominick Grift wrote:
>>>>>>>
>>>>>>> On Wed, 2012-08-29 at 21:55 +0200, Sven Vermeulen wrote:
>>>>>>>> On Wed, Aug 29, 2012 at 09:37:55PM +0200, Dominick Grift wrote:
>>>>>>>>>> -# create udev database in /dev/.udevdb
>>>>>>>>>> -allow udev_t udev_tbl_t:file manage_file_perms;
>>>>>>>>>> +allow udev_t udev_tbl_t:dir relabelto;
>>>>>>>>>> +manage_dirs_pattern(udev_t, udev_tbl_t, udev_tbl_t)
>>>>>>>>>> +manage_files_pattern(udev_t, udev_tbl_t, udev_tbl_t)
>>>>>>>>>> +manage_lnk_files_pattern(udev_t, udev_tbl_t, udev_tbl_t)
>>>>>>>>>> +
>>>>>>>>>> dev_filetrans(udev_t, udev_tbl_t, file)
>>>>>>>>>
>>>>>>>>> This doesnt make sense to me.
>>>>
>>>> [cut]
>>>>
>>>>>>>> Well, the udev code (looking at udev-182 here) has the code for relabeling
>>>>>>>> in it. For instance, when copy_dev_dir is called, it has
>>>>>>>>
>>>>>>>> #v+
>>>>>>>> udev_selinux_setfscreateconat(udev, dirfd(dir_to), dent->d_name, S_IFDIR|0755);
>>>>>>>> mkdirat(dirfd(dir_to), dent->d_name, 0755);
>>>>>>>> udev_selinux_resetfscreatecon(udev);
>>>>>>>> #v-
>>>>>>>>
>>>>>>>> I believe this is the source, but I'm no master in this. I mainly based
>>>>>>>> myself on the denials and errors I got. If I put in an "auditallow" to show
>>>>>>>> this, this is the result:
>>>>
>>>> At a quick look, the code in most recent versions (the one merged in
>>>> systemd) apparently has been extended even further in that direction by
>>>> adding a shared label.c source file.
>>>>
>>>> As for such labelling issue, since it is usually always preferable that
>>>> each one sticks to its own job, perhaps it can be dontaudited and then
>>>> the relevant "dynamic" path added to restorecond.conf ?
>>>>
>>>> The strange thing is that I am running latest udev without any of these
>>>> modifications (thus including the relabel permission)...
>>>
>>> I am currently rewriting udev policy as part of my project to write a
>>> systemd policy and i havent noticed this either yet, although it is
>>> hinting at it (it needs setfscreate capability and so does systemd by
>>> the way)
>>>
>>> I am not letting these daemons run my security though.

Yes, I am personally starting to drop this in favour of reverting back
to sysvinit and perhaps even drop udev, because it seems to be just
evolving towards overengineering with no tangible benefit at all...

> As already said, relabelling is critical, it is SELinux-domain by
> definition and therefore it should not normally be required/allowed. It
> makes me wonder whether the daemon is getting slightly over-engineered ?
>
>>From raw testing, it seems that it produces a few warnings:
>
> udev: Failed to initialize SELinux context: No such file or directory

Apart from the warning, that is due to a printout message recently added
in the source code, it can essentially proceed without relabelling
/run/udev as that is SELinux domain and almost no application or daemon
should be relabelling filesystems (with the exception of a few
applications such as the installers from the various distributions, if
the relative module is loaded in modular policy).

As already said, it should be possible to tackle such problem by adding
/run to restorecond.conf and making sure that restorecond is started
early enough during boot and/or statically relabel runtime udev files at
boot-time (restorecon calls from rc startup scripts or similar methods).

> udev: failed to create queue file: No such file or directory

The latter is not related to SELinux but to local misconfiguration.

> Have you tried to test it without the new permissions ?
>
>> That plus i do not like hard-coding types. it is degrading selinux
>
> What do you mean exactly for hard-coding types ?
>
> Degradation just for the sake of keeping things running is bad as in
> allowing permissions that are not normally required for carrying out a
> given task.
>
> At the end, it has just more or less rebased from /var/run and /dev
> to /run/udev.
>
> Regards,
>
> Guido