Bug 100317 - should be able to add new header fields that can be trusted
Summary: should be able to add new header fields that can be trusted
Status: NEW
Alias: None
Product: dbus
Classification: Unclassified
Component: core (show other bugs)
Version: git master
Hardware: Other All
: medium normal
Assignee: D-Bus Maintainers
QA Contact: D-Bus Maintainers
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-03-22 14:43 UTC by Simon McVittie
Modified: 2017-03-29 12:42 UTC (History)
1 user (show)

See Also:
i915 platform:
i915 features:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
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.


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