mjricka at epoch.ncsc.mil
Mon Jul 26 13:03:15 PDT 2004
On Mon, 2004-07-26 at 10:32, Havoc Pennington wrote:
> On Mon, 2004-07-26 at 08:42, Matthew Rickard wrote:
> > I see you've implemented this option in your next patch. I can test
> > this stuff out, but it seems selinux.[ch] didn't make it into your
> > patch. Can you redo the diff and send it again?
> Doh. Attached.
I've made some comments below from an initial look over your patch.
Mostly the changes look good, but you were right -- it didn't compile :)
In configure.in the header check should be:
av_permissions.h is the header with the class/permission info in it.
> if (!bus_connection_read_selinux_context (owner, &con))
it looks like you forgot to change owner to connection.
Now this one I'm not quite sure the cause. In
> bus_selinux_id_table_free_value (BusSELinuxID *sid)
It seems that sometimes the sid passed in is NULL, which causes
bus_selinux_id_unref to throw an assertion. I solved this by simply
testing for NULL before calling bus_selinux_id_unref. However, I'm
still not sure why this is ever being passed NULL, so I don't know if
this is the correct solution.
> bus_selinux_allows_send (DBusConnection *sender,
> DBusConnection *proposed_recipient)
proposed_recipient can be NULL to represent the bus driver. The
bus_selinux_allows_send was not testing this, and therefore throwing an
assertion in bus_connection_get_selinux_id(). Adding a simple check
here that uses the bus_sid on a NULL proposed_recipient corrected this.
> _dbus_assert_not_reached ("FIXME the avc_context_to_sid() error
The only error avc_context_to_sid should fail on is ENOMEM, so I think
it should be OK to just return NULL.
> _dbus_assert_not_reached ("bus_selinux_init_id doesn't properly
> indicate OOM");
it can fail on errors other than ENOMEM (like failing to get the
connection context). How should we handle this?
In bus_selinux_id_table_insert() the key/sid were being cleared before
the _dbus_verbose() displayed them. I switched the order here.
Also, some minor changes to just kill some compiler warnings. In
bus_selinux_id_ref/unref cast it back to security_id_t with
SELINUX_SID_FROM_BUS. In bus_selinux_id_table_insert I assign retval to
FALSE to start so that it won't complain that it could be used
I also noticed that rather than using security_context_t for context
types you are simply using char* instead. Is there any particular
reason for this (since we do have the SELinux headers available in
Thanks for updating the man page with the new SELinux information. I
filled in the FIXME items there for the class/permissions.
I've attached a diff that applies on top of the patch you sent to show
the few changes that I made. Hopefully we can get this ready in time to
be included in the next D-BUS release.
-------------- next part --------------
A non-text attachment was scrubbed...
Size: 4648 bytes
Desc: not available
Url : http://freedesktop.org/pipermail/dbus/attachments/20040726/bf8095ec/dbus-selinux-incremental.bin
More information about the dbus