Re: Patch to make can_network stronger and remove nscd tunable.

[Thomas Bleher <bleher@informatik.uni-muenchen.de>]

[-- Attachment #1: Type: text/plain, Size: 5955 bytes --]

* Daniel J Walsh <dwalsh@redhat.com> [2004-11-02 18:35]:
> Updated with Russell's "daemon" change and other fixes.
> 
> How does this look?

First off, it would be nice if you could split your patches into
logically independant pieces, makes it much easier to read.

I think there need to be some changes (comments below) but the
nfs_home_dirs-related stuff should be merged as soon as possible.
Currently it is broken in cvs because only some parts have been
converted from tunable to boolean.

> diff --exclude-from=exclude -N -u -r nsapolicy/domains/program/crond.te policy-1.17.37/domains/program/crond.te
> --- nsapolicy/domains/program/crond.te	2004-10-19 16:03:04.000000000 -0400
> +++ policy-1.17.37/domains/program/crond.te	2004-11-02 10:30:33.000000000 -0500
> @@ -114,6 +114,10 @@
> +allow crond_t krb5_conf_t:file { getattr read };
> +dontaudit crond_t krb5_conf_t:file { write };

If we are going to add this to more domains we should add a macro IMHO
like can_krb5_connect() or something. I do not know much about kerberos,
but I think most kerberized apps will need similar permissions which
should only be granted if kerberos is used.

> diff --exclude-from=exclude -N -u -r nsapolicy/domains/program/login.te policy-1.17.37/domains/program/login.te
> --- nsapolicy/domains/program/login.te	2004-10-19 16:03:05.000000000 -0400
> +++ policy-1.17.37/domains/program/login.te	2004-11-02 10:30:33.000000000 -0500
> @@ -21,6 +21,8 @@
>  dontaudit $1_login_t shadow_t:file { getattr read };
>  
>  general_domain_access($1_login_t);
> +can_network($1_login_t) 
> +allow $1_login_t self:{ tcp_socket udp_socket } connect;

Huh? Where does this come from? Cannot see this in the cvs policy. If
this is needed because of kerberos it should be ifdef'ed.

> -ifdef(`nfs_home_dirs', `
> +if (use_nfs_home_dirs) {
>  r_dir_file($1_login_t, nfs_t)
> -')dnl end if nfs_home_dirs
> +}

This should go into CVS ASAP, as mentioned above.

> diff --exclude-from=exclude -N -u -r nsapolicy/domains/program/unused/acct.te policy-1.17.37/domains/program/unused/acct.te
> --- nsapolicy/domains/program/unused/acct.te	2004-10-19 16:03:05.000000000 -0400
> +++ policy-1.17.37/domains/program/unused/acct.te	2004-11-02 10:30:33.000000000 -0500
> @@ -63,6 +63,7 @@
>  
>  ifdef(`logrotate.te', `
>  domain_auto_trans(logrotate_t, acct_exec_t, acct_t)
> +allow logrotate_t acct_data_t:dir { search };
>  allow logrotate_t acct_data_t:file { create_file_perms };

allow logrotate_t acct_data_t:dir search;
allow logrotate_t acct_data_t:file create_file_perms;

This makes it easier to read, IMHO.

> --- nsapolicy/domains/program/unused/ftpd.te	2004-10-27 14:32:48.000000000 -0400
> +++ policy-1.17.37/domains/program/unused/ftpd.te	2004-11-02 10:30:33.000000000 -0500
> @@ -4,6 +4,7 @@
>  #           Russell Coker <russell@coker.com.au>
>  # X-Debian-Packages: proftpd-common bsd-ftpd ftpd vsftpd
>  #
> +# Depends: inetd.te

Not true. There is a boolean ftpd_is_daemon which governs this.
Current policy needed inetd.te to compile but I think this is an error
in the policy. The following patch should fix it:

--- ftpd.te.orig	2004-11-03 00:37:16.000000000 +0100
+++ ftpd.te	2004-11-03 00:39:33.000000000 +0100
@@ -44,8 +44,6 @@
 rw_dir_create_file(ftpd_t, var_lock_t)
 allow ftpd_t ftp_port_t:tcp_socket name_bind;
 can_tcp_connect(userdomain, ftpd_t)
-# Allows it to check exec privs on daemon
-allow inetd_t ftpd_exec_t:file x_file_perms;
 }
 ifdef(`inetd.te', `
 if (!ftpd_is_daemon) {
  
>  allow ftpd_t ftp_data_port_t:tcp_socket name_bind;
> +allow ftpd_t port_t:tcp_socket { name_bind };

I confess I am not too familiar with ftp, but does it really need to
bind to arbitrary ports, seems excessive and unneeded (and is not
granted in current policy as far as I can see)

> diff --exclude-from=exclude -N -u -r nsapolicy/domains/program/unused/ping.te policy-1.17.37/domains/program/unused/ping.te
> --- nsapolicy/domains/program/unused/ping.te	2004-06-16 13:33:36.000000000 -0400
> +++ policy-1.17.37/domains/program/unused/ping.te	2004-11-02 10:30:33.000000000 -0500
> @@ -55,3 +56,5 @@
>  # it tries to access /var/run
>  dontaudit ping_t var_t:dir search;
>  
> +dontaudit ping_t devtty_t:chr_file { read write };
> +dontaudit ping_t ping_t:capability { sys_tty_config };

        dontaudit ping_t self:capability sys_tty_config;

is nicer.

> diff --exclude-from=exclude -N -u -r nsapolicy/file_contexts/program/mailman.fc policy-1.17.37/file_contexts/program/mailman.fc
> --- nsapolicy/file_contexts/program/mailman.fc	2004-10-13 22:41:58.000000000 -0400
> +++ policy-1.17.37/file_contexts/program/mailman.fc	2004-11-02 10:30:33.000000000 -0500
[...]
> +/usr/lib/mailman/bin/qrunner  	 -- system_u:object_r:mailman_queue_exec_t
> +/etc/mailman(/.*)?		   system_u:object_r:mailman_data_t
> +/var/spool/mailman(/.*)?	   system_u:object_r:mailman_data_t

Sorry, I do not know mailman at all, so please excuse my ignorance.
But does mailman really have to write to /etc/mailman, which is
presumably it's configuration data? This is not nice at all.


> diff --exclude-from=exclude -N -u -r nsapolicy/macros/program/ssh_macros.te policy-1.17.37/macros/program/ssh_macros.te
> --- nsapolicy/macros/program/ssh_macros.te	2004-10-14 23:25:20.000000000 -0400
> +++ policy-1.17.37/macros/program/ssh_macros.te	2004-11-02 10:30:33.000000000 -0500
> @@ -20,20 +20,16 @@
>  undefine(`ssh_domain')
>  ifdef(`ssh.te', `
>  define(`ssh_domain',`
> -ifdef(`single_userdomain', `
> -typealias $1_home_t alias $1_home_ssh_t;
> -typealias $1_t alias $1_ssh_t;
> -', `

Ahh, nice to see single_userdomain finally gone.

There were a few other superfluous braces, but the rest looks fine.

Thomas

-- 
http://www.cip.ifi.lmu.de/~bleher/selinux/ - my SELinux pages
GPG-Fingerprint: BC4F BB16 30D6 F253 E3EA  D09E C562 2BAE B2F4 ABE7

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]