[patches] SE-DBUS policy revamp [1/2]

[patches] SE-DBUS policy revamp [1/2]

[Colin Walters]

[-- Attachment #1.1: Type: text/plain, Size: 1112 bytes --]

Hi,

A while back I raised my concerns that we needed the session and system
buses to enforce different security policies:

http://marc.theaimsgroup.com/?l=selinux&m=109406674017893&w=2

Stephen first suggested having the bus compute a derived type based on
the security context of the requesting process and the bus's own
context.  Attached is a D-BUS patch to make it do that, and a rewrite of
the policy to adapt to it.  I chose to go with the more direct approach
of using the derived type for all interaction with D-BUS, instead of
only doing it on service acquisition.

I think the policy actually came out cleaner after macroizing
can_dbusd_converse.

Dan: This should only go on the FC4 branch, at least not unless we
release a newer D-BUS for FC3 later.

I tried hard to preserve correctness; this patch has been lightly tested
on my machine and appears to work.

This patch also includes a few type definitions for services, but in the
next patch I'll be going over all the D-BUS services and labeling them.

The D-BUS patch is attached first, the SELinux policy second.



[-- Attachment #1.2: dbus-cvs20041023-selinux-derived-type.patch --]
[-- Type: text/x-patch, Size: 1888 bytes --]


--- bus/selinux.c.~1.7.~	2004-10-18 09:06:27.000000000 -0400
+++ bus/selinux.c	2004-10-23 02:07:45.877958984 -0400
@@ -49,6 +49,8 @@
 /* Store an avc_entry_ref to speed AVC decisions. */
 static struct avc_entry_ref aeref;
 
+/* Store the context of the bus for computing relabels. */
+char *bus_context = NULL;
 /* Store the SID of the bus itself to use as the default. */
 static security_id_t bus_sid = SECSID_WILD;
 
@@ -213,7 +215,6 @@
 {
 #ifdef HAVE_SELINUX
   int r;
-  char *bus_context;
 
   _dbus_assert (bus_sid == SECSID_WILD);
   
@@ -275,8 +276,6 @@
       return FALSE;
     }
 
-  freecon (bus_context);
-  
   return TRUE;
 #else
   return TRUE;
@@ -433,6 +432,7 @@
                                      char              **con)
 {
   int fd;
+  char *remotecon;
 
   if (!selinux_enabled)
     return FALSE;
@@ -445,14 +445,26 @@
       return FALSE;
     }
   
-  if (getpeercon (fd, con) < 0)
+  if (getpeercon (fd, &remotecon) < 0)
     {
       _dbus_verbose ("Error getting context of socket peer: %s\n",
                      _dbus_strerror (errno));
       return FALSE;
     }
+
+  if (security_compute_relabel (remotecon, bus_context,
+				SECCLASS_DBUS,
+				con) != 0)
+    {
+      _dbus_verbose ("Error computing derived context for %s: %s", remotecon,
+		     _dbus_strerror (errno));
+      freecon (remotecon);
+      return FALSE;
+    }
+  freecon (remotecon);
+    
+  _dbus_verbose ("Successfully computed derived connection context: %s\n", *con);
   
-  _dbus_verbose ("Successfully read connection context.\n");
   return TRUE;
 }
 #endif /* HAVE_SELINUX */
@@ -808,6 +820,9 @@
 
   sidput (bus_sid);
   bus_sid = SECSID_WILD;
+  if (bus_context != NULL)
+    freecon (bus_context);
+  bus_context = NULL;
   
 #ifdef DBUS_ENABLE_VERBOSE_MODE
   bus_avc_print_stats ();

[-- Attachment #1.3: dbus-policy-revamp.patch --]
[-- Type: text/x-patch, Size: 8956 bytes --]

diff -ur -x policy.conf policy/domains/program/bluetooth.te policy.hacked/domains/program/bluetooth.te
--- policy/domains/program/bluetooth.te	2004-10-13 23:05:38.000000000 -0400
+++ policy.hacked/domains/program/bluetooth.te	2004-10-23 15:18:22.534975656 -0400
@@ -22,7 +22,7 @@
 # Use the network.
 can_network(bluetooth_t)
 can_ypbind(bluetooth_t)
-dbusd_client(system, bluetooth_t)
+dbusd_client(system, bluetooth)
 allow bluetooth_t self:socket { create setopt ioctl bind listen };
 allow bluetooth_t self:unix_dgram_socket create_socket_perms;
 allow bluetooth_t self:unix_stream_socket create_stream_socket_perms;
diff -ur -x policy.conf policy/domains/program/cups.te policy.hacked/domains/program/cups.te
--- policy/domains/program/cups.te	2004-10-13 23:05:38.000000000 -0400
+++ policy.hacked/domains/program/cups.te	2004-10-23 15:35:08.656022008 -0400
@@ -189,15 +189,10 @@
 
 allow cupsd_config_t self:unix_stream_socket create_socket_perms;
 ifdef(`dbusd.te', `
-dbusd_client(system, cupsd_t)
-dbusd_client(system, cupsd_config_t)
-allow cupsd_config_t userdomain:dbus { send_msg };
-allow userdomain cupsd_config_t:dbus { send_msg };
-allow cupsd_config_t hald_t:dbus { send_msg };
-allow hald_t cupsd_config_t:dbus { send_msg };
-allow cupsd_t userdomain:dbus { send_msg };
-allow cupsd_t hald_t:dbus { send_msg };
-allow hald_t cupsd_t:dbus { send_msg };
+dbusd_client(system, cupsd)
+dbusd_client(system, cupsd_config)
+can_dbusd_converse(system, hald, cupsd_config)
+can_dbusd_converse(system, hald, cupsd)
 ')
 
 can_exec(cupsd_config_t, { bin_t sbin_t shell_exec_t })
Only in policy.hacked/domains/program: cups.te.rpmsave
diff -ur -x policy.conf policy/domains/program/dbusd.te policy.hacked/domains/program/dbusd.te
--- policy/domains/program/dbusd.te	2004-10-13 23:05:38.000000000 -0400
+++ policy.hacked/domains/program/dbusd.te	2004-10-23 04:06:04.000000000 -0400
@@ -2,6 +2,9 @@
 #
 # Author:  Russell Coker <russell@coker.com.au>
 
+# Used for org.freedesktop.DBus
+type dbus_service_t;
+
 dbusd_domain(system)
 
 allow system_dbusd_t dbusd_var_run_t:sock_file create_file_perms;
diff -ur -x policy.conf policy/domains/program/hald.te policy.hacked/domains/program/hald.te
--- policy/domains/program/hald.te	2004-10-13 23:05:38.000000000 -0400
+++ policy.hacked/domains/program/hald.te	2004-10-23 15:38:16.103525656 -0400
@@ -12,6 +12,9 @@
 #
 daemon_domain(hald, `, fs_domain, nscd_client_domain')
 
+# Used for org.freedesktop.Hal
+type hal_service_t;
+
 can_exec_any(hald_t)
 
 allow hald_t { etc_t etc_runtime_t }:file { getattr read };
@@ -19,8 +22,8 @@
 allow hald_t self:unix_dgram_socket create_socket_perms;
 
 ifdef(`dbusd.te', `
-allow hald_t system_dbusd_t:dbus { acquire_svc };
-dbusd_client(system, hald_t)
+dbusd_client(system, hald)
+allow hald_dbusd_system_t hal_service_t:dbus { acquire_svc };
 ')
 
 allow hald_t { self proc_t }:file { getattr read };
@@ -43,8 +46,7 @@
 
 ifdef(`updfstab.te', `
 domain_auto_trans(hald_t, updfstab_exec_t, updfstab_t)
-allow updfstab_t hald_t:dbus { send_msg };
-allow hald_t updfstab_t:dbus { send_msg };
+can_dbusd_converse(system, hald, updfstab)
 ')
 ifdef(`udev.te', `
 domain_auto_trans(hald_t, udev_exec_t, udev_t)
@@ -60,7 +62,6 @@
 allow hald_t usbfs_t:dir search;
 allow hald_t usbfs_t:file { getattr read };
 allow hald_t bin_t:lnk_file read;
-dontaudit hald_t selinux_config_t:dir { search };
-allow hald_t initrc_t:dbus { send_msg };
-allow initrc_t hald_t:dbus { send_msg };
+r_dir_file(hald_t { selinux_config_t default_context_t })
+#can_dbusd_converse(system, hald, initrc)
 allow hald_t etc_runtime_t:file rw_file_perms;
diff -ur -x policy.conf policy/domains/program/hotplug.te policy.hacked/domains/program/hotplug.te
--- policy/domains/program/hotplug.te	2004-10-13 23:05:38.000000000 -0400
+++ policy.hacked/domains/program/hotplug.te	2004-10-23 15:24:44.595893568 -0400
@@ -151,7 +151,7 @@
 
 can_network(hotplug_t)
 can_ypbind(hotplug_t)
-dbusd_client(system, hotplug_t)
+dbusd_client(system, hotplug)
 
 # Allow hotplug (including /sbin/ifup-local) to start/stop services and # run sendmail -q
 domain_auto_trans(hotplug_t, initrc_exec_t, initrc_t)
diff -ur -x policy.conf policy/domains/program/initrc.te policy.hacked/domains/program/initrc.te
--- policy/domains/program/initrc.te	2004-10-13 23:05:38.000000000 -0400
+++ policy.hacked/domains/program/initrc.te	2004-10-23 04:00:34.000000000 -0400
@@ -322,7 +322,7 @@
 allow initrc_t security_t:dir { getattr search };
 allow initrc_t security_t:file { getattr read };
 ifdef(`dbusd.te', `
-allow initrc_t system_dbusd_t:dbus { send_msg acquire_svc };
+allow initrc_t system_dbusd_t:dbus { send_msg };
 ')
 
 # init script state
diff -ur -x policy.conf policy/domains/program/udev.te policy.hacked/domains/program/udev.te
--- policy/domains/program/udev.te	2004-10-13 23:05:38.000000000 -0400
+++ policy.hacked/domains/program/udev.te	2004-10-23 15:18:34.122214128 -0400
@@ -108,7 +108,7 @@
 
 allow udev_t udev_helper_exec_t:dir r_dir_perms;
 
-dbusd_client(system, udev_t)
+dbusd_client(system, udev)
 
 allow udev_t device_t:dir { relabelfrom relabelto create_dir_perms };
 allow udev_t sysctl_dev_t:dir { search };
diff -ur -x policy.conf policy/domains/program/updfstab.te policy.hacked/domains/program/updfstab.te
--- policy/domains/program/updfstab.te	2004-10-13 23:05:38.000000000 -0400
+++ policy.hacked/domains/program/updfstab.te	2004-10-23 15:18:13.324375880 -0400
@@ -28,7 +28,7 @@
 
 read_locale(updfstab_t)
 
-dbusd_client(system, updfstab_t)
+dbusd_client(system, updfstab)
 
 # not sure what the sysctl_kernel_t file is, or why it wants to write it, so
 # I will not allow it
Only in policy.hacked/file_contexts/program: cups.fc~
Only in policy.hacked/file_contexts/program: cups.fc.rpmsave
diff -ur -x policy.conf policy/macros/base_user_macros.te policy.hacked/macros/base_user_macros.te
--- policy/macros/base_user_macros.te	2004-10-13 23:05:38.000000000 -0400
+++ policy.hacked/macros/base_user_macros.te	2004-10-23 15:33:47.271394352 -0400
@@ -193,13 +193,17 @@
 
 # Grant permissions to access the system DBus
 ifdef(`dbusd.te', `
-dbusd_client(system, $1_t)
-dbusd_client($1, $1_t)
+dbusd_client(system, $1)
+dbusd_client($1, $1)
 dbusd_domain($1)
 ifdef(`hald.te', `
-allow $1_t hald_t:dbus { send_msg };
-allow hald_t $1_t:dbus { send_msg };
+can_dbusd_converse(system, hald, $1)
 ') dnl end ifdef hald.te
+ifdef(`cupsd.te',`
+# Only one-way conversation from cupsd -> user
+allow cupsd_system_dbusd_t $1_dbusd_system_t:dbus { send_msg };
+can_dbusd_converse(system, cupsd_config, $1)
+')
 ') dnl end ifdef dbus.te
 
 # allow port_t name binding for UDP because it is not very usable otherwise
diff -ur -x policy.conf policy/macros/program/dbusd_macros.te policy.hacked/macros/program/dbusd_macros.te
--- policy/macros/program/dbusd_macros.te	2004-10-13 23:05:38.000000000 -0400
+++ policy.hacked/macros/program/dbusd_macros.te	2004-10-23 17:47:09.132926496 -0400
@@ -50,26 +50,44 @@
 r_dir_file($1_dbusd_t, pam_var_console_t)
 ')
 
+allow $1_dbusd_t self:dbus { send_msg acquire_svc };
+
 ')dnl end dbusd_domain definition
 
-# dbusd_client(dbus_type, domain)
-# Example: dbusd_client_domain(system, user_t)
+# dbusd_client(dbus_type, domain_prefix)
+# Example: dbusd_client_domain(system, user)
 #
-# Grant permissions for connecting to the specified DBus type
-# from domain.
+# Define a new derived domain for connecting to dbus_type
+# from domain_prefix_t. 
 define(`dbusd_client',`')
 ifdef(`dbusd.te',`
 undefine(`dbusd_client')
 define(`dbusd_client',`
+
+# Derived type used for connection
+type $2_dbusd_$1_t;
+type_change $2_t $1_dbusd_t:dbus $2_dbusd_$1_t;
+
 # For connecting to the bus
-allow $2 $1_dbusd_t:unix_stream_socket { connectto };
+allow $2_t $1_dbusd_t:unix_stream_socket { connectto };
 ifelse(`system', `$1', `
-allow { $2 } { var_run_t system_dbusd_var_run_t }:dir search;
-allow { $2 } system_dbusd_var_run_t:sock_file { write };
+allow { $2_t } { var_run_t system_dbusd_var_run_t }:dir search;
+allow { $2_t } system_dbusd_var_run_t:sock_file { write };
 ',`
 ') dnl endif system
 # SE-DBus specific permissions
-allow $2 { $1_dbusd_t self }:dbus { send_msg };
-allow $2 $1_dbusd_t:dbus { acquire_svc };
+allow $2_dbusd_$1_t { $1_dbusd_t self }:dbus { send_msg };
+') dnl endif dbusd.te
+')
+
+# can_dbusd_converse(dbus_type, domain_prefix_a, domain_prefix_b)
+# Example: can_dbusd_converse(system, hald, updfstab)
+# Example: can_dbusd_converse(session, user, user)
+define(`can_dbusd_converse',`')
+ifdef(`dbusd.te',`
+undefine(`can_dbusd_converse')
+define(`can_dbusd_converse',`
+allow $2_dbusd_$1_t $3_dbusd_$1_t:dbus { send_msg };
+allow $3_dbusd_$1_t $2_dbusd_$1_t:dbus { send_msg };
 ') dnl endif dbusd.te
 ')

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [patches] SE-DBUS policy revamp [1/2]

[Havoc Pennington]

On Sat, 2004-10-23 at 18:09 -0400, Colin Walters wrote:
> +/* Store the context of the bus for computing relabels. */
> +char *bus_context = NULL;

This should be "static"

I can't say I understand the patch otherwise, but if you guys think it's
right go for it.

Havoc


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: [patches] SE-DBUS policy revamp [1/2]

[Colin Walters]

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

On Sat, 2004-10-23 at 18:09 -0400, Colin Walters wrote:

> I think the policy actually came out cleaner after macroizing
> can_dbusd_converse.

Should have tested the session bus in addition to the system one.
Here's an interdiff.



[-- Attachment #2: dbus-policy-revamp-2.patch --]
[-- Type: text/x-patch, Size: 1115 bytes --]

diff -u policy.hacked/macros/base_user_macros.te policy.hacked/macros/base_user_macros.te
--- policy.hacked/macros/base_user_macros.te	2004-10-23 15:33:47.271394352 -0400
+++ policy.hacked/macros/base_user_macros.te	2004-10-25 11:12:35.032343856 -0400
@@ -191,17 +191,22 @@
 can_network($1_t)
 can_ypbind($1_t)
 
-# Grant permissions to access the system DBus
 ifdef(`dbusd.te', `
+# Grant permissions to access the system bus
 dbusd_client(system, $1)
-dbusd_client($1, $1)
+# Create derived domain for session bus, allow access
 dbusd_domain($1)
+dbusd_client($1, $1, $1)
+# Allow any service acquisition on session bus
+# until we have user programs running in separate
+# domains
+allow $1_dbusd_$1_t $1_dbusd_t:dbus { acquire_svc };
 ifdef(`hald.te', `
 can_dbusd_converse(system, hald, $1)
 ') dnl end ifdef hald.te
-ifdef(`cupsd.te',`
+ifdef(`cups.te',`
 # Only one-way conversation from cupsd -> user
-allow cupsd_system_dbusd_t $1_dbusd_system_t:dbus { send_msg };
+allow cupsd_dbusd_system_t $1_dbusd_system_t:dbus { send_msg };
 can_dbusd_converse(system, cupsd_config, $1)
 ')
 ') dnl end ifdef dbus.te