Bug 100317

Summary: should be able to add new header fields that can be trusted
Product: dbus Reporter: Simon McVittie <simon.mcvittie>
Component: coreAssignee: D-Bus Maintainers <dbus>
Status: NEW --- QA Contact: D-Bus Maintainers <dbus>
Severity: normal    
Priority: medium CC: desrt, dh.herrmann
Version: git master   
Hardware: Other   
OS: All   
Whiteboard:
i915 platform: i915 features:

Description Simon McVittie 2017-03-22 14:43:20 UTC
The D-Bus Specification says:

    A header must contain the required header fields for its message
    type, and zero or more of any optional header fields. Future 
    versions of this protocol specification may add new fields.
    **Implementations must ignore fields they do not understand.**
    Implementations must not invent their own header fields; only changes
    to this specification may introduce new header fields.

In practice the reference dbus-daemon happily passes through messages with arbitrary message header fields that it does not understand. The only restrictions are:

* the message header fields that *are* understood must have their documented types
* the message header fields that *are* understood can only appear once each
* the sender field is overwritten by the dbus-daemon

This unfortunately makes them non-useful as a way to add fields that are validated/guaranteed by the dbus-daemon, because a recipient can't tell the difference between "a new dbus-daemon set this field, it is definitely true" and "the sender set this field, an old dbus-daemon merely didn't understand it well enough to remove it".

For example, "uid of sender" and "LSM label of sender" as message headers would be good feature requests - if we could trust those headers, they would remove the need to call GetConnectionCredentials() in many cases.

Discussing this with Allison Lortie at the GTK Hackfest, we thought dbus-daemon should reinterpret "implementations must ignore fields they do not understand" to mean it will now filter out any header field that it does not understand; then we can add a negotiation step in which a peer asks the dbus-daemon to confirm that it actually does filter out the others. If the dbus-daemon doesn't, the high-level APIs used by services (libdbus, GDBus) would behave as though new/future header fields weren't there.

Unfortunately, GDBus already has g_dbus_message_get_header(), which can look up arbitrary fields by numeric ID.

It seems like this is potentially too many "sharp edges" - at best it's an API in which it's easy to be insecure by mistake - so we might have to close this WONTFIX :-(
Comment 1 Philip Withnall 2017-03-24 15:29:48 UTC
(In reply to Simon McVittie from comment #0)
> Unfortunately, GDBus already has g_dbus_message_get_header(), which can look
> up arbitrary fields by numeric ID.

Does that matter? Surely what GDBus would do is to rewrite the GDBusMessage on receiving it, to omit the new/future header fields (unless it had negotiated with the dbus-daemon to ensure they’re safe).
Comment 2 Simon McVittie 2017-03-24 16:30:47 UTC
(In reply to Philip Withnall from comment #1)
> Does that matter? Surely what GDBus would do

Current GDBus doesn't do whatever future thing you would like it to do.
Comment 3 Philip Withnall 2017-03-24 18:01:14 UTC
(In reply to Simon McVittie from comment #2)
> (In reply to Philip Withnall from comment #1)
> > Does that matter? Surely what GDBus would do
> 
> Current GDBus doesn't do whatever future thing you would like it to do.

Could we perhaps rely on this only in the situation where we’re using containers, and hence there’s a gating point for relying on a new-enough GDBus? i.e. “We can trust the header fields on this system because we are in/interacting-with a container, and the container technology mandates a new-enough GDBus and dbus-daemon to make this secure”.
Comment 4 Simon McVittie 2017-03-24 20:23:33 UTC
(In reply to Philip Withnall from comment #3)
> Could we perhaps rely on this only in the situation where we’re using
> containers, and hence there’s a gating point for relying on a new-enough
> GDBus?

We can do that for container instance metadata, sure, and that's why this doesn't block Bug #100344. But we can't use that argument for the other things I'd like to put in header fields: the uid and the LSM label.
Comment 5 Philip Withnall 2017-03-29 12:42:43 UTC
(In reply to Simon McVittie from comment #4)
> (In reply to Philip Withnall from comment #3)
> > Could we perhaps rely on this only in the situation where we’re using
> > containers, and hence there’s a gating point for relying on a new-enough
> > GDBus?
> 
> We can do that for container instance metadata, sure, and that's why this
> doesn't block Bug #100344. But we can't use that argument for the other
> things I'd like to put in header fields: the uid and the LSM label.

Surely we could for the case where one of the peers involved in a message is in a container (in the sense of using the API from bug #100344), since then both peers know dbus-daemon is new enough to filter out the UID/LSM label otherwise?

This basically shifts the trust decision from ensuring that the UID/LSM label are unforgeable, to ensuring that it’s not possible to make a D-Bus peer look like it’s using the container API (bug #100344) when it’s not.
Comment 6 David Herrmann 2017-05-11 09:15:25 UTC
How about bumping the version number? We use protocol version '2' and then force dbus-daemon to filter header fields.

gdbus can now easily see whether header fields are trusted or not, by just looking at the version number. Additionally, whoever *sends* a message, simply uses version '2' if the calling application did *NOT* add custom fields. If it added custom fields, it uses version '1'.

This still requires a negotiation at AUTH time (or Hello) to figure out which versions a client supports. But it would provide perfect backwards compatibility. The only issue is that nobody can rely on augmented header fields, if their clients send custom header fields.

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct.