New SE-DBUS patch

[Matthew Rickard]

Thanks for the feedback.  I put together a new patch addressing these
issues (let me know if I missed anything).  The new patch is attached
(sorry -- not posted on http this time, let me know if someone needs it
to be).

Additional comments are in-line below.

On Fri, 2004-06-25 at 12:19, Havoc Pennington wrote:
> - the #ifdef (and also the SELinux header file inclusion) should
>   be confined to dbus-sysdeps.c basically, or at most to security.[hc];
security.[hc] are now selinux.[hc].  I've confined SELinux specific
headers and #ifdef to selinux.c.  Without SELinux support access these
routines should always allow access.  Also the case of SELinux support
but a non-SELinux kernel running should be handled properly now.

Let me know if this is anything like what you are looking for :)

> - I wouldn't worry about get_unix_fd() multiple fds until we have a 
>   DBusTransport with multiple, at that time we can add a new 
>   function
Left that alone for now.

> 
> - "FIXME Actually check errors like we should" should be fixed 
>   probably
Fixed.

> - e->d.policy.security_context = dbus_new0 (DBusString, 1);
>   is a bit different from how D-BUS is normally done, usually 
>   either DBusString is directly in the struct (rather than DBusString*) 
>   or char* is used (char* is used in DBusPolicy right now)
This now uses a DBusString directly.

> - I'd either put all the selinux system API access in security.[hc]
>   or sysdeps, not in both
It's in selinux.[hc] now.  It could be put in sysdeps, but I think it
makes it easier to manage having the SELinux stuff separate.

> - It looks like some functions may be exported from security.[hc] 
>   that are only used in security.[hc]? Ideally the only exports
>   are the check() routines and an init() routine
Should be fixed now.

> - debugging routines such as print_service_sid_table should 
>   be #ifdef DBUS_ENABLE_VERBOSE_MODE
Fixed.

> - I'd default enable_selinux to auto rather than no
Fixed.  configure.in should now auto detect SELinux support at build
time with AC_CHECK_LIB.

> - the other config.h defines are HAVE_ rather than WITH_
>   (is WITH_SELINUX significant to the system headers
>   or something?)
Changed it to HAVE_SELINUX.  WITH_SELINUX is not significant.

> - DBUS_BUS_CFLAGS="$XML_CFLAGS -DWITH_SELINUX"
>   shouldn't be required if you define WITH_SELINUX in config.h
Removed it.

> - the _dbus_warn() in dbus_connection_get_unix_fd() should 
>   instead be e.g. "_dbus_return_val_if_fail (connection != NULL, FALSE)"
Fixed this.  For future reference, in what cases should
_dbus_return_val_if_fail be used?

> 
> - dbus_bool_t result = FALSE; should be two lines, one declaration
>   and one assignment
Fixed.


More comments are welcome.

Matt

-------------- next part --------------
SE-DBUS Changelog
Matthew Rickard <mjricka at epoch.ncsc.mil>

7/1/04 - 0.5
-New patch release.  security.[ch] are now known as selinux.[ch].
WITH_SELINUX is now changed to HAVE_SELINUX.
-configure.in will now autodetect SELinux support at build time.
-Cleaned up to confine #ifdef HAVE_SELINUX to selinux.c.
-Various other cleanups.  Thanks to Havoc Pennington for all the
suggestions on how to fix this patch up.
-Added additional support for the case of SELinux support being built
in, but running a non-SELinux kernel.

6/24/04 - 0.4
-New patch release.  Note that sepolicy.conf is distributed separately
from the patch since it will be distributed with the SELinux policy 
and not D-BUS.
-Changed parsing to take selinux.conf file in XML format consistent
with the other D-BUS configs.  Don't do longest match on service
names.
-Service hash table is stored as part of the BusPolicy.  BusRegistry
holds a reference to this table.  This was done to keep the mappings
global and not per client.  However, it is kind of ugly and there is
probably a better way to do it.
-Fixed up some header problems.  Also fixed a few places not to assume
C99/gcc.
-Various SID handling fixes and cleanups.

2/23/04 - 0.3
-Changed code to store a SID for a service rather than a context.
This saves us from having to transition between context<->sid so much.
-Changed parsing code to store SIDs rather than contexts in the hash
table for the same reason as above.
-Changed bus_connection_avc_has_perm to take an optional SID now
instead of a context so it works appropriately with the above changes.
-Fixed a bug in the log callback struct.  Had an uninitialized pointer
there that could cause segfaults.
-Broke off part of the parsing function into bus_hash_service_sid() to do
the hashing separately.  Parse function still needs more cleanup.
-Various other cleanups.

2/19/04 - 0.2
-avc_context_to_sid() increments sid ref count.  We don't need to
increment it with sidget.  This caused ref leaks.
-bus_avc_store_service_context() shouldn't freecon -- we
aren't done with it until the service is released (it will be
freed by bus_service_unref() at that point).

2/19/04 - 0.1
-Initial Release

-------------- next part --------------
A non-text attachment was scrubbed...
Name: se-dbus-0.5.diff
Type: text/x-patch
Size: 42609 bytes
Desc: not available
Url : http://freedesktop.org/pipermail/dbus/attachments/20040701/75cf87b9/se-dbus-0.5-0001.bin