Created attachment 20913 [details] [review] don't process deny rules if interface doesn't match There is a flaw (or at least, problematic behavior) in the current policy logic that causes nondeterminism. This is not a security flaw per se, more of a reliability flaw because it causes denials where they were not intended. This flaw mirrors CVE-2008-0595 (http://lists.freedesktop.org/archives/dbus/2008-February/009401.html). The problem stems from rules of the form: File A: <policy context="default> <allow send_destination="com.foo.MyService"/> </policy> File B: <policy context="default"> <allow send_destination="org.foo.Bar"/> <deny send_interface="org.foo.Bar.Secure"/> </policy> This looks innocuous enough at first (as all these rules do); clearly the intent of the second rule is to lock down access to just part of org.foo.Bar. But it breaks the case of messages sent with no interface to the com.foo.MyService: **depending on include ordering** The no-interface case happens in dynamic language scripts like Python typically. Tracing the logic in policy.c, what happens when a message is sent with no interface is that we see the rule has an interface, then fall into the logic: if ((no_interface && rule->allow) || (!no_interface && strcmp (dbus_message_get_interface (message), rule->d.send.interface) != 0)) continue; So we're processing a deny rule, with no interface, and this logic does *not* match. Thus we hit the rule, and the default becomes *deny*. If we happened to process the rules from the other file first, the message will be spuriously denied.
So, we probably should have caught this when we did CVE-2008-0595. It's the exact same issue, but for deny rules instead of allow rules. Your patch seems right to me. The only reason I can think that the rule->allow test was there at all was because of a thinko. Adding it there kept the semantics in the non-allow case the same and since the non-allow case couldn't be a security issue we didn't think about it hard enough. The scenario is definitely bogus though, and the fix looks right. What about the receive? It's got the same problem right?
"Your patch seems right to me" Famous last words. Colin found me and noticed the patch is bogus. The patch even has the comment "for deny rules, if the message has no interface we want to use the rule (and thus deny)." calling continue; prematurely will sidestep some checks and reintroduce a security hole.
One idea was to ignore (well post nastygram in syslog for) lines like: <deny send_interface="org.foo.Bar.Secure"/> (i.e. require a send_destination when giving a send_interface)
Comment on attachment 20913 [details] [review] don't process deny rules if interface doesn't match As rstrode said, this patch is wrong. We need to fix every rule that does <deny send_interface="foo"/>.
Let's make this bug into a tracker for bare <deny send_interface/>. First, GDM: http://bugzilla.gnome.org/show_bug.cgi?id=564767
yum: https://bugzilla.redhat.com/show_bug.cgi?id=476718
Setroubleshoot: https://bugzilla.redhat.com/show_bug.cgi?id=476722
dnsmasq (no upstream bug tracker (sigh), patch follows): --- dnsmasq-2.45/dbus/dnsmasq.conf 2008-07-20 14:26:07.000000000 -0400 +++ dnsmasq-2.45.orig/dbus/dnsmasq.conf 2008-12-16 14:11:48.000000000 -0500 @@ -5,12 +5,10 @@ <policy user="root"> <allow own="uk.org.thekelleys.dnsmasq"/> <allow send_destination="uk.org.thekelleys.dnsmasq"/> - <allow send_interface="uk.org.thekelleys.dnsmasq"/> </policy> <policy context="default"> <deny own="uk.org.thekelleys.dnsmasq"/> <deny send_destination="uk.org.thekelleys.dnsmasq"/> - <deny send_interface="uk.org.thekelleys.dnsmasq"/> </policy> </busconfig>
ConsoleKit is in both the broken services bug and this set. http://bugs.freedesktop.org/show_bug.cgi?id=19020
NetworkManager: http://bugzilla.gnome.org/show_bug.cgi?id=563730
(In reply to comment #8) > dnsmasq (no upstream bug tracker (sigh), patch follows): Following the approach used in other clean-up changes, whole context="default" part can be dropped, as it only contains denies, right? dnsmasq also does: dbus_message_new_signal(DNSMASQ_PATH, DNSMASQ_SERVICE, "Up") so based on what the default for signals is, it may or may not need extra rule to allow this (not needed with current: http://cgit.freedesktop.org/dbus/dbus/commit/?id=920c3c02 )
I suggest we not remove any existing denies in the default context for now. This will allow the policy to remain secure even when used with dbus 1.2.4 (and upcoming temporary dbus.1.2.4.Xpermissive stream).
This just gets worse. Here's a denial I see with my new logging patch: Dec 17 16:39:40 space-ghost dbus: Rejected send message, 20 matched rules; type="method_return", sender=":1.70" (uid=0 pid=4821 comm ="/usr/libexec/nm-dispatcher.action ") interface="(unset)" member="(unset)" error name="(unset)" destination=":1.18" (uid=0 pid=2753 comm="NetworkManager --pid-file=/var/run/NetworkManager/")) I believe what's happening is that all the <deny send_interface=""/> rules are applying to this even though it's a method return, just because it has no interface! This makes all those bare deny send_interface rules even more damaging than we originally thought. My temptation here is to make <deny send_interface=""/> only apply to method_call and signal. (Or possibly only !requested_reply?) In other words, automatically turn <deny send_interface="foo"/> into: <deny send_interface="foo" send_type="method_call"/> <deny send_interface="foo" send_type="signal"/> The best thing to do of course is to fix all of those rules so that they use send_destination, but there are a *lot*.
Thinking about this, if we are really denying method returns now like that, why don't I see a ton more of them?
(In reply to comment #14) > Thinking about this, if we are really denying method returns now like that, why > don't I see a ton more of them? Likely answer here: That message is actually an unrequested reply. For requested replies, this logic kicks in: if (requested_reply && !rule->allow && !rule->d.send.requested_reply) continue;
Created attachment 21257 [details] [review] workaround for deny send_interface Proposed partial workaround for spurious denials with deny send_interface
(In reply to comment #16) > Created an attachment (id=21257) [details] > workaround for deny send_interface > > Proposed partial workaround for spurious denials with deny send_interface I can confirm this patch reduces the unrequested reply denial to 1 instead of N (where N is the number of <deny send_interface=""/> one has). It's an open question whether this patch matters - we're denying unrequested replies by default anyways. I'm dropping this one from proposal for the first 1.2.4.Xpermissive stream, but I'd like discussion of whether it makes sense to put in later.
Debian bugs related to this: http://bugs.debian.org/cgi-bin/pkgreport.cgi?users=pkg-utopia-maintainers@lists.alioth.debian.org&tag=fdo-18961
Have you arrived at a conclusion about how to address this?
Basically, messages sent with no interface on the system bus have been broken for a while, and we just have to remove these rules from config files one at a time. This issue isn't as severe as one might think at first - because they haven't been working for a long time, no one can be relying on them. Generally the best thing is for code to explicitly specify the interface anyways.
I just wonder whether we need to fix that in older distributions. Hopefully it's safe to ignore those log messages there :-)
It should be - but what denials are you getting?
The same as you got in commment #13 from nm-dispatcher.action
Comment #13 is mainly a dbus-glib bug for trying to send a reply even when none was requested (https://bugs.freedesktop.org/show_bug.cgi?id=19441), fixed in git master. You can see the analysis for that one here: http://bugzilla.gnome.org/show_bug.cgi?id=565008 I'm not actually sure offhand if this bug applies to it too; hm, it's kind of strange, I see some old denials in my logs with 20+ matched rules (implying that these <deny send_interface> rules are active), and also many that only have one. I'll investigate. Anyways the bottom line is that we need a new dbus-glib release with that bug fix which will quiet that issue.
I am puzzled by the proposed changes for apps in /etc/dbus-1/system.d Colin stated line comment #5 bare <deny send_interface/>. suggestion is to provide a matching rule for send_destination and send_interface be it an allow rule or deny rule. Then in comment#8 the patch is simply to remove the send_interface regardless of whether it is a allow or denny. This is in contradiction to comment #5 so which is it then? I am looking to see whether the avahi dbus conf file needs to be patched or not. Thanks
(In reply to comment #25) > Then in comment#8 the patch is simply to remove the send_interface regardless > of whether it is a allow or denny. This is in contradiction to comment #5 so > which is it then? Both the allow and deny send_interface rules were redundant with the send_destination ones (as is the cast for almost but not all services). > I am looking to see whether the avahi dbus conf file needs to be patched or > not. I don't think we ended up with an avahi patch.
(In reply to comment #26) > (In reply to comment #25) > > > Then in comment#8 the patch is simply to remove the send_interface regardless > > of whether it is a allow or denny. This is in contradiction to comment #5 so > > which is it then? > > Both the allow and deny send_interface rules were redundant with the > send_destination ones (as is the cast for almost but not all services). > > > I am looking to see whether the avahi dbus conf file needs to be patched or > > not. > > I don't think we ended up with an avahi patch. Just to be sure, you are saying this file, http://www.avahi.org/browser/avahi-daemon/avahi-dbus.conf.in does not need updated for dbus 1.2.8 and beyond? Thanks, -Ghee
Well it does have a <deny send_interface>, however it also has a specific member match "SetHostName", so while it should be fixed (by also adding a send_destination="org.freedesktop.Avahi", it will only affect any no-interface method). Let's avahi to the list of things that need to be fixed for this bug. I tried to file a bug at avahi.org but apparently don't have permissions.
Avahi bug filed: http://avahi.org/ticket/263
Is there anything more for us to do here?
-- GitLab Migration Automatic Message -- This bug has been migrated to freedesktop.org's GitLab instance and has been closed from further activity. You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.freedesktop.org/dbus/dbus/issues/11.
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.