Created attachment 121381 [details] [review]
check MLS permissions when ListNames() is called

(In reply to David King from comment #0)
> It's desirable to block services with different MLS permissions from knowing
> about other, specifically when calling ListNames().

I think we should work out what the use-case here is, and how it compares with the SEE permission in kdbus. Are you in contact with any of the kdbus people within Red Hat?

I would like to avoid having two subtly incompatible definitions of processes seeing each other.

Comment on attachment 121381 [details] [review]
check MLS permissions when ListNames() is called

Review of attachment 121381 [details] [review]:
-----------------------------------------------------------------

::: bus/driver.c
@@ +452,5 @@
> +              _dbus_warn_check_failed ("service lookup failed: %s", requester);
> +              ++i;
> +              continue;
> +            }
> +          requester_conn = bus_service_get_primary_owners_connection (service);

Er, isn't this just @connection? You shouldn't need to look up the message's destination's owner.

@@ +454,5 @@
> +              continue;
> +            }
> +          requester_conn = bus_service_get_primary_owners_connection (service);
> +          _dbus_string_init_const (&str, services[i]);
> +          service = bus_registry_lookup (registry, &str);

I'm a bit surprised this is needed, too.

@@ +475,5 @@
> +                }
> +
> +              /* Skip any services which are disallowed by SELinux policy. */
> +              ++i;
> +              continue;

This leaks @error, and will cause a (fatal-by-default?) warning if visibility is denied for more than one name.

::: bus/selinux.c
@@ +758,5 @@
> + * @param destination the name being requested.
> + * @returns whether the name should be visible by the source of the request
> + */
> +dbus_bool_t
> +bus_selinux_allows_name (DBusConnection     *source,

Perhaps bus_selinux_allows_see() or bus_selinux_allows_visibility()?

If process A cannot see process B, then I would expect process A to be unable to receive messages from process B, or NameOwnerChanged messages about process B, either.

The SELinux code should probably also forbid BecomeMonitor() and eavesdropping if MLS is used, or restrict them to the most-trusted trust domain (if it has one).

I'd prefer @source and @destination to be called something like @observer and @observed, or @observer and @seen - it isn't obvious what "source" or "destination" should mean in this context.

@@ +775,5 @@
> +
> +  if (!selinux_mls_enabled)
> +    return TRUE;
> +
> +  err = selinux_getpolicytype (&policy_type);

Can this be cached? Doing a malloc/free per name in the ListNames() reply seems a little excessive.

It seems bad that we're looking for a magic string. What if SELinux later adds a new policy type named "compartmentalised" or "paranoid" or something, that should also be subject to visibility restructions? (I realise this is not necessarily something that can be fixed within dbus-daemon, though.)

"You are not authorized to access bug #1118399.

Most likely the bug has been restricted for internal development processes and we cannot grant access."

I would very much prefer to have some sort of public rationale for this feature in the commit message - I happen to have a reasonably good idea of what MLS means (I'm assuming it's multilevel security, and not major league soccer or any of the other possible expansions suggested by Wikipedia), but the same is unlikely to be true for all D-Bus maintainers and contributors.

(In reply to Simon McVittie from comment #4)
> I would very much prefer to have some sort of public rationale for this
> feature in the commit message - I happen to have a reasonably good idea of
> what MLS means (I'm assuming it's multilevel security, and not major league
> soccer or any of the other possible expansions suggested by Wikipedia), but
> the same is unlikely to be true for all D-Bus maintainers and contributors.

Well, I had no idea. I was assuming it's a different acronym for what I had known as LSM (Linux Security Module).

Either way, the idea by itself makes sense. Like in grsec, don't even list the connections that the caller wouldn't be able to send a message to or receive from anyway.

The devil is in the details though. With just plain system.conf/system.d rules, it would take a lot of processing for that.

(In reply to Thiago Macieira from comment #5)
> The devil is in the details though. With just plain system.conf/system.d
> rules, it would take a lot of processing for that.

Right; I think it's clear that system.conf/system.d are generally not a great answer to any given requirement.

I'd like to have a clear picture of what we are and aren't aiming to achieve here. The general idea of making process A unable to see process B (if that is indeed what is going on here) is reasonable, but to achieve that, we'd need to filter a lot more than just ListNames.

-- GitLab Migration Automatic Message --

This bug has been migrated to freedesktop.org's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.freedesktop.org/dbus/dbus/issues/142.