Summary: | spec requires "no-interface" method calls to work, but they can't be secure on the system bus | ||
---|---|---|---|
Product: | dbus | Reporter: | Simon McVittie <smcv> |
Component: | core | Assignee: | Simon McVittie <smcv> |
Status: | RESOLVED FIXED | QA Contact: | |
Severity: | normal | ||
Priority: | medium | CC: | chengwei.yang.cn, desrt, hp, thiago, walters, zeuthen |
Version: | unspecified | Keywords: | patch |
Hardware: | Other | ||
OS: | All | ||
Whiteboard: | review? | ||
i915 platform: | i915 features: | ||
Attachments: |
Do not require messages without INTERFACE to be dispatched
spec: explicitly mention filtering messages with no INTERFACE |
Description
Simon McVittie
2013-08-27 10:19:59 UTC
This is probably a carry-over from DCOP, where interfaces were possible but never used. I'm fine with reducing the no-interface case to optional behaviour instead of required. Please don't make implementations have to return an error in case of ambiguity -- it might require a linear search to find instead of stopping on the first match. (In reply to comment #1) > Please don't make implementations have to return an error in > case of ambiguity -- it might require a linear search to find instead of > stopping on the first match. That's a fair point. OK, so on receiving a method call with no interface, implementations MAY either treat it as a call to the same method on an implementation-chosen interface, or return an error. By the way, some implementations have taken upon themselves (or as a side-effect) to apply the same rules for properties: calling Get or Set with an empty interface name causes the receiver to search for the property name in all available interfaces. I remember that QtDBus did not support this when first released, but the code is there today (on the receiver side) because someone reported a bug. (In reply to comment #0) > I think we should consider *encouraging* implementations to return an error > in this ambiguous case. I think that's a good idea. > The spec also says > > However, if a method name is unique implementations must not > require an interface field > > but that doesn't seem very well thought-out, and I'm tempted to > delete it from the spec. I concur. Seems this is another difference between system and session bus, plus to the one "servicehelper <service>" requires the .service file match its owned bus name while the dbus spec doesn't say that. See #bug 66608. (In reply to comment #3) > By the way, some implementations have taken upon themselves (or as a > side-effect) to apply the same rules for properties The spec does note this: An empty string may be provided for the interface name; in this case, if there are multiple properties on an object with the same name, the results are undefined (picking one by according to an arbitrary deterministic rule, or returning an error, are the reasonable possibilities). Created attachment 85910 [details] [review] Do not require messages without INTERFACE to be dispatched reviously, if we have interfaces: interface com.example.foo: method Ambiguous() interface com.example.bar: method Ambiguous() method Unambiguous() implementations were required to deliver a message with no INTERFACE and METHOD=Unambiguous to "bar". A message with no INTERFACE and METHOD=Ambiguous could either be delivered to "foo", delivered to "bar" or treated as an error. Relax this to allow an error for the unambiguous case, too, and strongly recommend specifying the interface (which is best-practice). Created attachment 85911 [details] [review] spec: explicitly mention filtering messages with no INTERFACE This is an important security measure. Without it, the system bus would not deliver its intended security properties. The actual implementation has always behaved like this, I think. + In the absence of an <literal>INTERFACE</literal> field, if two + or more interfaces on the same object have a method with the same + name, it is undefined which of those methods will be invoked. + Implementations may choose to either return an error, or deliver the + message as though it had an arbitrary one of those interfaces. This sounds like if the method exists only on one interface then we still must deliver it and may only return an error as one of our possible options in the case that there is ambiguity. I'd much prefer if we changed the language to something like "Client implementations are strongly encouraged to reject all incoming messages without interface set and to never send such messages. The message bug will pass these messages on only for reasons of backward compatibility." *bus ;) (In reply to comment #9) > This sounds like if the method exists only on one interface then we still > must deliver it and may only return an error as one of our possible options > in the case that there is ambiguity. That was my intention, but if you want to change the spec further (so it's undefined in all cases), I wouldn't be against that. > I'd much prefer if we changed the language to something like "Client > implementations are strongly encouraged to reject all incoming messages > without interface set and to never send such messages. The message bug will > pass these messages on only for reasons of backward compatibility." It seems a bit strange to be going straight from "MUST call the method in the unambiguous case, and MAY call a method in the ambiguous case" to "SHOULD NOT call the method, even in the unambiguous case". I'm generally against specification changes that make every previously-correct implementation violate a SHOULD. (The service-side of GDBus is not currently a correct implementation according to the spec. I'd be inclined to say that GDBus is fine, and it's the spec that's wrong, hence this bug.) How about this, which makes it explicitly undefined, like properties? + <literal>INTERFACE</literal> field giving the interface the method is a part of. + Including the <literal>INTERFACE</literal> in all method call + messages is strongly recommended. + </para> + <para> + In the absence of an <literal>INTERFACE</literal> field, the + result is undefined. Implementations may choose to either return + an error, or deliver the message to a method of that name + on any interface (if there is one). I'd also be fine with appending "Returning an error is recommended in this situation" if there's consensus, but I'm not going to try to force that if people consider it to be equally valid. In practical terms, I don't think existing libraries that handle the unambiguous case, or even the ambiguous case, as a non-error (QtDBus, dbus-python, etc.) should change it now; but for new implementations, I agree with desrt. Comment on attachment 85910 [details] [review] Do not require messages without INTERFACE to be dispatched Review of attachment 85910 [details] [review]: ----------------------------------------------------------------- looks good! Comment on attachment 85911 [details] [review] spec: explicitly mention filtering messages with no INTERFACE Review of attachment 85911 [details] [review]: ----------------------------------------------------------------- Looks good. Yes, this is the current way how dbus-daemon works. I'm going to merge these, since they make the spec reflect reality, and are halfway between what the spec currently says and what desrt wants it to say. desrt, if you want further divergence from the current spec, please reply to Comment #11 or propose different wording. If any maintainers object to these patches going in, please complain before 1.7.10. :-) Fixed in git for 1.7.10. desrt (or other interested maintainers), if you think that the result should be undefined even when unambiguous, please propose a patch, or state that you agree with my text below "How about this" in Comment #11. If you think that, additionally, returning an error for any no-INTERFACE call should be recommended, please propose further patches that you'd be happy with. |
Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.