document the assumption that makes our use of credentials-passing secure
Submitted by Simon McVittie
Assigned to Simon McVittie
Here is a potential attack by a malicious message sender:
- connect to the D-Bus socket
- race with dbus-daemon, and win:
- queue up the entire handshake, plus a malicious message, in the outgoing socket buffer
- make the D-Bus connection not close-on-exec
- exec() a setuid process, or fd-pass the D-Bus connection to a more privileged process
- dbus-daemon checks the sender's credentials (let's say "uid 1000"), and thinks it has the credentials of the setuid process (e.g. "uid 0")
- now dbus-daemon reads the malicious message, determines that uid 0 may send it, and forwards it to the recipient, with no indication that uid 1000 was actually responsible
We are not vulnerable to this, but it is not instantly obvious why not.
Having done some research in a bunch of OS kernels, the reason we're not vulnerable is that in practice, SO_PEERCRED and analogous APIs like getpeereid() are based on the kernel stashing the credentials as part of the socket descriptor early in its life cycle, either at the time that it is created with socket() (*BSD) or at the time it connect()s or listen()s (Linux).
Linux LSMs also have a hook to do the same with their LSM-specific identity info; as of 3.17-rc3 this is implemented for SELinux and Smack, but not AppArmor. Ubuntu's patched kernel adds the same thing for AppArmor.
If you think about it further, it becomes clear that a kernel would have a hard time implementing "check what the credentials of the other end of this socket are right now", because in the presence of fd-passing, "the other end" is non-unique. So I'm reasonably confident that this isn't suddenly going to change.
However, I think we should document this assumption so that unsuitable APIs are not added as new code paths for _dbus_read_credentials_socket() in future.
In particular, any API of the form "get the pid, then get that process's $SPECIAL_CREDENTIAL (from /proc or otherwise), then decide based on that" cannot be relied upon for security, because (1) exec() exists, and so do setuid binaries, and (2) even if exec() didn't exist, pids get recycled.