SE-DBUS

[Havoc Pennington]

Hi,

I'm looking at the patch in more detail now. Apologies I haven't had a
chance to previously.

I missed Stephen's mail on this, too much in one folder :-/

Some comments in no special order:

 - variable declarations have to be at the top of the function,
   since we don't require C99/gcc; there were several cases
   of this
 
 - dbus_connection_get_unix_fd() is a problem because not all 
   connections will necessarily have a single fd. I'd rather 
   take the approach as with dbus_connection_get_unix_user()
   where we would have some kind of higher-level accessor, 
   perhaps selinux-specific instead of unix-specific

 - this function is #ifdef WITH_SELINUX in the header, you can't
   put #ifdef WITH_SELINUX in an installed header because 
   config.h is not installed.

 - sepolicy.conf.in if it's going to be a different file format
   and type of file should not be installed to the same directory
   with same suffix as the other config files, probably

 - it would be nice to confine the Linux-specific functions and 
   headers to dbus-sysdeps.c, rather than spreading WITH_SELINUX 
   throughout the code.

 - for sepolicy.conf.in, we define a mapping from service *names* 
   to security contexts. If I understand correctly, any connecting
   process with one of the listed contexts is allowed to own the 
   service or any service starting with that string. This configuration 
   is dbus-specific. If I have this wrong please correct me.

 - for sending a message from connection A to connection B, we check 
   the actual security contexts read from those file descriptors, 
   and see if send_msg is allowed. This allow/deny is in the SELinux 
   conf files not in anything dbus-specific. please correct this if 
   confused.

 - If I have the above correct, I think we should replace sepolicy.conf
   with an extension to the regular dbus config file format, simply:
   <policy security_context="foo_t">
     <allow own="org.foo.bar"/>
   </policy>
   this avoids the whole extra config file parser. Since selinux.conf
   is totally dbus specific anyway and even in the same location as the 
   dbus conf files, I don't see why it should be a separate format.
   It can be a separate file, as <include> directives are supported.

 - An orthogonal question is whether something like this is allowed:
   <policy security_context="foo_t">
     <allow send_destination="org.foo.bar"/>
   </policy>
   i.e. can a security-context-associated policy use the fine-grained
   access controls available to user and group associated policies. 
   I don't see why not, as it could allow additional security and 
   is essentially free to implement. It could never _open_ 
   holes not permitted by the send_msg policy in SELinux as that 
   check is separate.

   The alternative is to only permit ownership rules in 
   a security-context-associated <policy>, same effect as selinux.conf
 
 - the dbus access controls can't do the equivalent of the 
   bus_avc_check_can_send_service(), which hinges on the 
   *current security contexts of two unix processes* - the dbus 
   controls rather hinge on dbus state such as services owned.
   Thus I think we should have bus_avc_check_can_send_service
   regardless of the answer to the previous question. 

   However, I wonder if a separate send_msg capability makes sense; 
   perhaps the permission check here should just hinge on whether
   process A could normally write to a pipe or socket going to 
   process B. i.e. is there a case where you want to control 
   whether A and B can talk via dbus separately from whether A and 
   B can talk at all? Or is fine-grained automatically better?
   Are pipes, sockets, and other IPC mechanisms distinguished today?

 - If there are policies keyed off security context, is it more 
   complex than "connection has security context foo_t" when deciding 
   whether to apply the policy? I get the impression it is

 - before committing, it's important that "make check-coverage" still 
   passes and ideally that there's test coverage for the new code.
   With some versions of gcc make check-coverage is wonky unfortunately 
   :-/ hopefully fixed in Rawhide, I believe it was busted in FC1.

   "decode-gcov foo.c" can be used after a make check with profiling 
   enabled to see where coverage is missing in a file.

Thanks, hope this is helpful.

Havoc