Comment on attachment 76224
Required plumbing for reading process credentials from procfs

Review of attachment 76224:
-----------------------------------------------------------------

Thanks for your really valuable comments and your time.  I admit this PID thing is sprinkled everywhere.  I didn't want to spend too much effort without knowing your views first and this review proves that I would have gone astray.  I'll revisit this patch and address all your concerns focusing only on EXTERNAL authentication and supplementary groups.  My other goal is also try to not even touch /etc/{passwd,group} at all if not absolutely necessary.

Please see also my specific comment wrt design/security.

::: dbus/dbus-auth.c
@@ +1080,5 @@
>    else
>      {
>        if (!_dbus_credentials_add_from_user (auth->desired_identity,
> +                                            &auth->identity,
> +                                            _dbus_credentials_get_unix_pid (auth->credentials)))

I'll address only specific questions here and defer the rest to the generic comment.

If an account has access to setuid/setgid executables and is available to potentially rogue users, then the system is flawed, not D-Bus.  IMHO the vulnerability here are setuid/setgid programs that shouldn't exist at all or at least shouldn't be available to random users.  It's a bit like saying "Everyone on the system has passwordless sudo access because a sysadmin/architect had a bad day.  Oops!  Let's defend!". 

I don't understand your reasoning in one of your comments [1]:
* cp /bin/bash audiobash && chgrp audio audiobash && chmod g+s audiobash

The user must be a member of `audio' group in order to chgrp a file to that group.  Why then ever bother with these tricks if the user is a *genuine* member of the group?

You also mentioned in your comment on some other bug [2]:
/* any setuid program will do */
exec ("/bin/ping", "example.com")

Again, no setuid/setgid should be available.  In this instance `ping' program should have cap_net_admin and cap_net_raw capabilities set.  No UID change.

[1] https://bugs.freedesktop.org/show_bug.cgi?id=9328#c9
[2] https://bugs.freedesktop.org/show_bug.cgi?id=47581#c3

----
The reality is different though and no one wants to make libdbus the culprit.  So I agree with you:  UID should be received from the socket rather than from procfs.

WRT the other question where you consider different groups in /etc/group and in procfs, my opinion is that those in /etc/group should be ignored.  My view is that someone knew better and since they were privileged enough to convince kernel to set those groups on the process, libdbus in this instance should respect that.

Now again you could argue that there's a race condition here and someone was able to gain group membership the same way as for UID.  I think it's up to the system administrator/architect to provide or deny such opportunity.  For example the system I work on has all D-Bus services/clients running in their individual sandboxes where one of the features is a separate mount namespace with all filesystems mounted either ro,nosuid or noexec.

So the conclusion is that this functionality could be optional controlled in a twofold manner:
* compile/build time
  This is rather obvious since procfs is not available on all systems.  I guess __linux__
  macro would do.

* D-Bus configuration
  If someone is able to configure D-Bus to use this feature (presumably this would be a
  system administrator) then they are confident enough about their system and should be
  allowed to use this feature.

What do you think?