Sat, 06 Mar 2004 16:20:42 -0500
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
- 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
- If I have the above correct, I think we should replace sepolicy.conf
with an extension to the regular dbus config file format, simply:
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:
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.