Bug 18229 - send_requested_reply="true" allows all non-reply messages
send_requested_reply="true" allows all non-reply messages
Status: RESOLVED FIXED
Product: dbus
Classification: Unclassified
Component: core
1.2.x
Other All
: medium normal
Assigned To: Havoc Pennington
John (J5) Palmieri
: security
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2008-10-26 08:28 UTC by Joachim Breitner
Modified: 2010-01-08 00:33 UTC (History)
10 users (show)

See Also:


Attachments
Possible fix (1.28 KB, patch)
2008-10-26 08:59 UTC, Joachim Breitner
Details | Splinter Review
proposed fix (750 bytes, patch)
2008-11-10 10:04 UTC, David Zeuthen (not reading bugmail)
Details | Splinter Review
Possible system.conf change (1.11 KB, patch)
2008-11-18 01:31 UTC, Tomas Hoger
Details | Splinter Review
test case (6.91 KB, patch)
2008-12-04 11:30 UTC, Colin Walters
Details | Splinter Review
Allow-DBus.Introspection-and-DBus.Peer-by-default (2.91 KB, patch)
2008-12-08 08:00 UTC, Colin Walters
Details | Splinter Review
cleanup of system.d (18.89 KB, patch)
2008-12-08 17:14 UTC, Colin Walters
Details | Splinter Review
allow signals by default (2.45 KB, patch)
2008-12-08 18:26 UTC, Colin Walters
Details | Splinter Review
Improvement for dbus-daemon manpage wrt policy rule application ordering (822 bytes, patch)
2008-12-09 01:10 UTC, Tomas Hoger
Details | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Joachim Breitner 2008-10-26 08:28:16 UTC
Hi,

if I understand everything correctly, there is a bad security bug in dbus:

The default configuration contains the lines
    <allow send_requested_reply="true"/>
    <allow receive_requested_reply="true"/>
with the valid intention to allow all replies to be send without explicit permission. Otherwise, dbus claims to have a default-no policy.

But what happens instead is: When a message is considered for sending, it enters bus_client_policy_check_can_send in policy.c[1]. There, all rules are looked at, but only SEND rules considered (line 893) – the first of the above rules is such a rule. Now we check for various conditions that might occur in such a rule (e.g. destination and the like), but none of these exist besides send_requested_reply. But in line 909 this is only done for messages which are replies. This means that for normal messages, we continue with the code and end up in line 1028, where we set the allowed flag! If no other rule kicks in, this stays allowed until the end.

A proper fix would be to add an else statement to the if in line 909, which calls continue, I think.

I did not adjust the severity or priority, per bug submitting etiquette, but I consider this a major bug.

Thanks,
Joachim

[1] http://gitweb.freedesktop.org/?p=dbus/dbus.git;a=blob;h=caa544e7a4f041e0cc9b250dc8c814a7b06e927b;hb=14afa0564e9eea01d28d4b2fd1e6ac0bfec626e7;f=bus/policy.c#l865
Comment 1 Joachim Breitner 2008-10-26 08:59:02 UTC
Created attachment 19870 [details] [review]
Possible fix

I think this (git-format-patch created patch) would fix the problem, but of course it should be reviewed first.
Comment 2 Havoc Pennington 2008-10-26 09:38:21 UTC
Good catch. I've marked the bug security group only in case people want to do a coordinated update under embargo.

I think the patch is not quite right because <allow> and <deny> should be treated differently.

The requested_reply arg to check_can_send already incorporates whether the message is a reply. So I think perhaps simply removing the "if (dbus_message_get_reply_serial (message) != 0)" check would fix this bug, except the eavesdrop test may need to now look at whether it's a reply, so something like:

if (!requested_reply && rule->allow && rule->d.send.requested_reply)
 {
   if (dbus_message_get_reply_serial (message) != 0 && rule->d.send.eavesdrop)
    {
      /* it's a reply, but was not requested; if eavesdrop is true, allow anyway */
    }
  else 
    {
      /* skip rule, do not allow */
      continue;
    }
}

I am not 100% sure though. Clearly more test cases are needed, ideally unit tests for the full matrix of (reply vs. not reply) * (requested vs. not requested) * (allow vs. deny) etc., basically try to cover all the code paths.
Comment 3 Joachim Breitner 2008-10-26 10:57:52 UTC
Hi,

I didn’t expect you would want to embargo this, and submitted it to the public Debian bug tracker as well:
http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=503532

Sorry if that is causing trouble.

Greetings,
Joachim
Comment 4 Tomas Hoger 2008-10-30 06:09:04 UTC
Havoc, do you have any specific <deny> rule example in mind, that would be evaluated incorrectly by dbus if Joachim's patch is applied?

Additionally, is this actually a problem with dbus itself, or rather issue of default policy in system.conf?  Reading the dbus-daemon manpage [1], it contains following:

  [1] http://dbus.freedesktop.org/doc/dbus-daemon.1.html

  The [send|receive]_requested_reply attribute works similarly to the
  eavesdrop attribute. It controls whether the <deny> or <allow> matches
  a reply that is expected (corresponds to a previous method call message).
  **This attribute only makes sense for reply messages (errors and method
  returns), and is ignored for other message types.**

If I read it correctly, it basically says: if message type is not error or method return, attribute (not the whole rule) is ignored.  So according to that sematics definition, for method call message the rule:

  <allow send_requested_reply="true"/>

should be identical to:

  <allow/>

So the code seems to do exactly what is specified.

Isn't it the default configuration to be blamed for this problem?  It says:

  <allow send_requested_reply="true"/>

and expects this rule only to be applied to reply messages.  This probably needs to be changed to:

  <allow send_requested_reply="true" send_type="method_return"/>
  <allow send_requested_reply="true" send_type="error"/>

I do understand that Joachim's patch changes semantics for non-reply messages from "attribute is ignored" to "whole rule is ignored".

Additionally, your suggestion does not seem to do the right thing either.  Consider call message and rule:

  <allow send_destination="org.designfu.SampleService"/>

If I'm reading the man page correctly, send_requested_reply="true" is implied by default.  In that case,

  if (!requested_reply && rule->allow && rule->d.send.requested_reply)

should be true, and

  if (dbus_message_get_reply_serial (message) != 0 && rule->d.send.eavesdrop)

should be false and the rule is skipped.

I'm likely to miss something important, so feel free to correct me in whatever I got wrong.  Thanks!
Comment 5 Havoc Pennington 2008-10-30 09:11:56 UTC
Tomas, I think your analysis is correct. So maybe just change the config file.
Comment 6 Tomas Hoger 2008-11-04 07:53:01 UTC
Havoc, thanks for reviewing my comment!

In comment #2 you stated you may want some coordinated release for this fix along with vendors.  Do you have any specific proposed date?  The issue is technically public already via Debian bug report (they can't make it private).  We do have a pending update to do for bug #17803, so it would be a good opportunity to fix both issues at the same time.  Thanks!
Comment 7 Havoc Pennington 2008-11-04 09:33:04 UTC
If there's coordination, I wouldn't be the person doing it ;-)
(I'd ask johnp or walters or jbressers at Red Hat)
Comment 8 Tomas Hoger 2008-11-06 08:08:35 UTC
(In reply to comment #7)
> If there's coordination, I wouldn't be the person doing it ;-)
> (I'd ask johnp or walters or jbressers at Red Hat)

Ok, so I will take care of it.  Is there any date until which you as an upstream would like to keep this private?
Comment 9 Havoc Pennington 2008-11-06 12:49:45 UTC
No preferred date from me (apparently it isn't truly private anyway)
Comment 10 Tomas Hoger 2008-11-07 06:32:33 UTC
CVE-2008-4311 will be used to identify this flaw.
Comment 11 David Zeuthen (not reading bugmail) 2008-11-10 10:04:19 UTC
Created attachment 20193 [details] [review]
proposed fix

Here's a patch for fixing this via modifying the configuration file. Does this fix the issue? Thanks.
Comment 12 Havoc Pennington 2008-11-11 05:42:50 UTC
Patch looks good
Comment 13 Tomas Hoger 2008-11-11 07:23:05 UTC
(In reply to comment #11)
> Here's a patch for fixing this via modifying the configuration file. Does this
> fix the issue? Thanks.

This is wrong, your need to use receive_type for rules with receive_requested_reply attribute, otherwise dbus daemon won't even start.  Additionally, we will need to spend more time chasing down various apps that rely on the currently used over-permissive policy.  When I restarted dbus with this change applied, it killed my kitten^W X session and prevented X login.  Ouch!
Comment 14 Tomas Hoger 2008-11-11 07:35:01 UTC
Looks like console-kit-daemon is one to blame, it spew some error messages, one obviously dbus-related:

ConsoleKit open: Unable to open session: A security policy in place prevents this recipient from receiving this message from this sender, see message bus configuration file (rejected message had interface "org.freedesktop.ConsoleKit.Manager" member "OpenSessionWithParameters" error name "(unset)" destination "org.freedesktop.ConsoleKit" reply serial 0 requested_reply=0)

and segfaults.
Comment 15 David Zeuthen (not reading bugmail) 2008-11-11 09:58:08 UTC
Added the ConsoleKit author to the Cc as per comment 14.
Comment 16 Tomas Hoger 2008-11-12 03:59:32 UTC
ConsoleKit will likely need something like this at minimum:

@@ -7,6 +7,11 @@
   <policy user="root">
     <allow own="org.freedesktop.ConsoleKit"/>
 
+    <!-- Allow receiving of messages to ConsoleKit interfaces -->
+    <allow receive_interface="org.freedesktop.ConsoleKit.Manager"/>
+    <allow receive_interface="org.freedesktop.ConsoleKit.Seat"/>
+    <allow receive_interface="org.freedesktop.ConsoleKit.Session"/>
+
     <!-- Allow all methods on interfaces -->
     <allow send_interface="org.freedesktop.ConsoleKit.Manager"/>
     <allow send_interface="org.freedesktop.ConsoleKit.Seat"/>


But this problem is likely to bite almost any service using system service bus.  Looking at the existing configs, even those apps that punch proper holes on the sender side fails to punch holes on receiver side.  What is the main reasoning for having to approve each message twice?

So if we want to tighten rule with receive_requested_reply by adding proper message types, almost all service-specific configuration files need to be fixed.

Alternatively, we can do a first half-step towards the full fix and leave that rule as unchanged for now and only turn to stricter rules on sender side (i.e. send_requested_reply rule).

Ideas?
Comment 17 Tomas Hoger 2008-11-12 06:20:34 UTC
Another question is how to deal with messages to some interfaces (such as org.freedesktop.DBus.Introspectable) on the receiver side.  There does not seem to be a good way to create receiver rules that would allow receiving of messages to certain interface on one service and deny it on other, as there's not destination-based check for receive rules.
Comment 18 Tomas Hoger 2008-11-16 01:54:19 UTC
Does anyone have any other ideas wrt handling of receive rules?
Comment 19 Tomas Hoger 2008-11-18 01:31:13 UTC
Created attachment 20397 [details] [review]
Possible system.conf change

What about this change for system.conf?  It restricts send_requested_reply rule, so it should deal with all unintended method calls.  It also tries to set correct expectations wrt future of the receive_requested_reply rule.

This should resolve the problem without need for immediate fix of all applications using system bus.  Creating allow receive_interface= rules for all interfaces implemented by any application using the bus would result in a similarly secure policy as one with current receive_requested_reply rule.  Not being able to check destination in receive rules still seems quite limiting to me.
Comment 20 David Zeuthen (not reading bugmail) 2008-12-02 08:29:14 UTC
(In reply to comment #19)
> Created an attachment (id=20397) [details]
> Possible system.conf change
> 
> What about this change for system.conf?  It restricts send_requested_reply
> rule, so it should deal with all unintended method calls.  It also tries to set
> correct expectations wrt future of the receive_requested_reply rule.
> 
> This should resolve the problem without need for immediate fix of all
> applications using system bus.  Creating allow receive_interface= rules for all
> interfaces implemented by any application using the bus would result in a
> similarly secure policy as one with current receive_requested_reply rule.  Not
> being able to check destination in receive rules still seems quite limiting to
> me.
> 

Colin, Havoc: Are you guys fine with committing Tomas' patch upstream? If not, what kind of work is needed to get this issue resolved in upstream D-Bus?

(I'm asking because I'm wary of putting out an D-Bus errata for Fedora/RHEL without a clear indication that the fix is blessed upstream.)
Comment 21 Colin Walters 2008-12-02 09:11:39 UTC
(In reply to comment #20)

> Colin, Havoc: Are you guys fine with committing Tomas' patch upstream? If not,
> what kind of work is needed to get this issue resolved in upstream D-Bus?

hp said in comment #5 that the config change is probably right; I haven't had a chance to study this in detail but from reading this report through again it makes sense to me as well.

We should get this fix in.

Tomas - the reason I believe that there's no interface based check is that it's legal for messages to be sent to any interface.
Comment 22 Colin Walters 2008-12-02 09:25:51 UTC
I vaguely recall a discussion about this that I think ended up at the conclusion that the unit of security granularity should be the service; i.e. not interfaces or methods.

In other words the config file for 3rd party services should just be:

<user="root">
  <allow service="org.freedesktop.ConsoleKit"/>
</user>

This would fit in with the trend of using PolicyKit for more fine grained authorization too.
Comment 23 Tomas Hoger 2008-12-02 09:52:10 UTC
(In reply to comment #21)
> Tomas - the reason I believe that there's no interface based check is that
> it's legal for messages to be sent to any interface.

Sorry, I don't understand this.  Proposed patch fixes default rule that is supposed to allow all requested replies.  Why should it be bound to any specific interface?

(In reply to comment #22)
> I vaguely recall a discussion about this that I think ended up at the
> conclusion that the unit of security granularity should be the service; i.e.
> not interfaces or methods.

Doesn't this make my note (end of comment #19) about not being able to use destination in receive rules even more valid?  Removing possibility to use interface and method attributes will only leave sender, message type and error name checks for use in the receive rules (I believe path falls to the same category as interfaces and methods).  So receive rules will have to be wide open to actually work (as they are now).

> In other words the config file for 3rd party services should just be:
> 
> <user="root">
>   <allow service="org.freedesktop.ConsoleKit"/>
> </user>

Is this supposed to be send or receive rule?  Or some new policy concept, that does not distinguish the two any more?

Anyway, if this is expected to be full config for some 3rd party service, dbus will have to get the default configuration right, so that requests from non-root users are denied.
Comment 24 Colin Walters 2008-12-03 13:26:54 UTC
Should we try to embargo this through vendor-sec, or just go ahead and commit the patch to git under the assumption that since it's already in the Debian BTS it's public?
Comment 25 Colin Walters 2008-12-03 13:40:55 UTC
(In reply to comment #23)
> (In reply to comment #21)
> > Tomas - the reason I believe that there's no interface based check is that
> > it's legal for messages to be sent to any interface.
> 
> Sorry, I don't understand this.  Proposed patch fixes default rule that is
> supposed to allow all requested replies.  Why should it be bound to any
> specific interface?

I wasn't suggesting the system.conf change should be bound to a specific interface, just that we shouldn't rely on the destination interface in general (there is a note about this in the policy docs).

> (In reply to comment #22)

> Doesn't this make my note (end of comment #19) about not being able to use
> destination in receive rules even more valid?  Removing possibility to use
> interface and method attributes will only leave sender, message type and error
> name checks for use in the receive rules (I believe path falls to the same
> category as interfaces and methods).  So receive rules will have to be wide
> open to actually work (as they are now).

Right...hm.  How useful are the receive rules in general?  I guess you'd need them if you wanted to keep a signal private between services.

> > In other words the config file for 3rd party services should just be:
> > 
> > <user="root">
> >   <allow service="org.freedesktop.ConsoleKit"/>
> > </user>
> 
> Is this supposed to be send or receive rule?  Or some new policy concept, that
> does not distinguish the two any more?
>
> Anyway, if this is expected to be full config for some 3rd party service, dbus
> will have to get the default configuration right, so that requests from
> non-root users are denied.

I was thinking this policy would allow root to own the service name, and any uid to send&receive messages from it, under the assumption that the service is using PolicyKit for fine grained permissions, or that it makes sense for any uid to be able to access it.

Comment 26 Tomas Hoger 2008-12-04 00:14:50 UTC
(In reply to comment #24)
> Should we try to embargo this through vendor-sec, or just go ahead and commit
> the patch to git under the assumption that since it's already in the Debian BTS
> it's public?

v-s is already aware of this issue, we already passed the previously proposed embargo date and there was not request for extension.  Given the Debian bug, I don't think there's any need for further embargo.
Comment 27 Tomas Hoger 2008-12-04 00:25:05 UTC
(In reply to comment #25)
> > Sorry, I don't understand this.  Proposed patch fixes default rule that is
> > supposed to allow all requested replies.  Why should it be bound to any
> > specific interface?
> 
> I wasn't suggesting the system.conf change should be bound to a specific
> interface, just that we shouldn't rely on the destination interface in general
> (there is a note about this in the policy docs).

Do you refer to this: "Be careful with send_interface/receive_interface, because the interface field in messages is optional." ?  We don't need this in such a generic rule.

> Right...hm.  How useful are the receive rules in general?  I guess you'd need
> them if you wanted to keep a signal private between services.

I guess send/receive rules split was made as a design decision for signal handling.  For method_calls and method_returns, using single rule making point to point check would be more natural.  Having to deal with both type of rules for these types of messages is bit clumsy.

> I was thinking this policy would allow root to own the service name, and any
> uid to send&receive messages from it, under the assumption that the service is
> using PolicyKit for fine grained permissions, or that it makes sense for any
> uid to be able to access it.

Ah, ok.  Though this does not sound like a change that can easily fit as a fix into existing systems (some of them even pre-dating PolicyKit).
Comment 28 Colin Walters 2008-12-04 07:35:15 UTC
Writing a test case for this now.  Will work on an upstream release today after that.
Comment 29 Tomas Hoger 2008-12-04 07:47:14 UTC
(In reply to comment #28)
> Writing a test case for this now.

Have you checked the one in our BZ?
Comment 30 Colin Walters 2008-12-04 11:30:24 UTC
Created attachment 20817 [details] [review]
test case

This patch adds a test case, but doesn't modify the system policy yet.  I chose to use Tomas' suggested policy for the test case policy, and I get the expected denial.
Comment 31 Colin Walters 2008-12-04 11:55:26 UTC
Ok, now I've double checked with this patch that the rules:

    <allow send_requested_reply="true"/>
    <allow send_requested_reply="true"/>
    <allow receive_requested_reply="true"/>

Allow the message through (incorrectly), whereas:

    <allow send_requested_reply="true" send_type="method_return"/>
    <allow send_requested_reply="true" send_type="error"/>
    <allow receive_requested_reply="true"/>

Disallow the message.

A quick once over of this patch would be appreciated; once that's done I'll merge it and then change the system.conf.in to match it per Tomas' patch.
Comment 32 Tomas Hoger 2008-12-05 00:33:02 UTC
(In reply to comment #31)

> A quick once over of this patch would be appreciated; once that's done I'll
> merge it and then change the system.conf.in to match it per Tomas' patch.

Is there any other patch to look at besides the one in comment #30?  Test test config has differences to my proposed patch, so some thoughts on them:

- It also restricts receive_requested_reply rule.  While this change really turns default config to what it claims to be, it also breaks existing stuff badly and has questionable benefits.

- I guess there should be some note used there, so that the config is at honest at what it claims, and make users aware it may get better fix in the future.  Feel free to re-word my note as needed.

Thank you!
Comment 33 Colin Walters 2008-12-05 09:21:31 UTC
Proposed text of mail announcement:

Subject: [CVE-2008-4311] DBus 1.2.6

A new security release of DBus is now available:

http://dbus.freedesktop.org/dbus/releases/dbus-1.2.6.tar.gz

This release contains a (partial, see below) fix for: https://bugs.freedesktop.org/show_bug.cgi?id=18229

== Summary ==

Because of a mistake in the default configuration for the system bus (system.conf), the default policy for both sent and received messages was effectively *allow*, and not deny as stated. 

This release fixes the send side permission, but does not change the receive.  See below for more details.

== Available workarounds ==

It is possible to add explicit <deny> rules to existing policy files, and in fact many have these already.

== Mitigating factors ==

There are three important mitigating factors.  

* First, in an examination of a Fedora 10 system, many services (though not all, see below) contained explicit <deny> rules.  These deny rules did (and continue to) operate as expected.

* Second, an increasing trend has been for core system services to use PolicyKit, or otherwise do security checks on the service side.  Any system which relies on PolicyKit is unaffected by this flaw.

* Third, the SELinux DBus support is not affected by this flaw.

Now, as mentioned above this fix is partial.  DBus has two kinds of policy permissions, send and receive.  Generally speaking, the send side permission is much more important.  However, DBus has supported receive side permissions for a few reasons, among those are:

* Ensuring signals containing sensitive data aren't visible by unexpected processes
* A way for processes to "second-pass" filter messages before they reach their C code; however, what one can do in the policy language is relatively limited, and something like PolicyKit (or just manual service-side permission checks) remain a better way to do this.

For compatibility reasons, this release only fixes the send-side permission check, and not receive.  A greater number of services will need to be updated for a future tightening of the receive permission.  

We are as yet unsure when (and in fact, if) the receive permission will be tightened in the DBus 1.2 stable branch.  We will gather information about any affected programs and make a final determination at in the near future.

For now, it is suggested to use explicit <deny> rules for receive side.

Comment 34 Colin Walters 2008-12-05 09:24:33 UTC
Further note will send after:

== Affected Services ==

An examination of a Fedora 10 desktop installation discovered only one affected program, the APT backend for PackageKit.

A fix is available here:

[link here]

Does anyone know of any other affected programs?
Comment 35 Colin Walters 2008-12-05 10:55:19 UTC
A few updates; credits, stronger suggestion to always use <deny>.

Subject: [CVE-2008-4311] DBus 1.2.6

A new security release of DBus is now available:

http://dbus.freedesktop.org/dbus/releases/dbus-1.2.6.tar.gz

This release contains a (partial, see below) fix for:
https://bugs.freedesktop.org/show_bug.cgi?id=18229

== Summary ==

Joachim Breitner discovered a mistake in the default configuration for the system bus (system.conf) which made the default policy for both sent and received messages was effectively *allow*, and not deny as stated. 

This release fixes the send side permission, but does not change the receive. 
See below for more details.

== Available workarounds ==

It is possible to add explicit <deny> rules to existing policy files, and in
fact many have these already.

== Mitigating factors ==

There are three important mitigating factors.  

* First, in an examination of a Fedora 10 system, many services contained explicit <deny> rules.  These deny rules did (and
continue to) operate as expected.

* Second, an increasing trend has been for core system services to use
PolicyKit, or otherwise do security checks on the service side.  Any system
which relies on PolicyKit is unaffected by this flaw.

* Third, the SELinux DBus support is not affected by this flaw.

Now, as mentioned above this fix is partial.  DBus has two kinds of policy
permissions, send and receive.  Generally speaking, the send side permission is
much more important.  However, DBus has supported receive side permissions for
a few reasons, among those are:

* Ensuring signals containing sensitive data aren't visible by unexpected
processes.  Suggested fix: Do not put sensitive data in DBus signals; use targeted method calls.

* A way for processes to "second-pass" filter messages before they reach their
C code.  Suggested fix: Something like PolicyKit (or just manual service-side permission checks) remain a better way to do this.

For compatibility reasons, this release only fixes the send-side permission
check, and not receive.  A greater number of services will need to be updated
for a future tightening of the receive permission.  

We are as yet unsure when (and in fact, if) the receive permission will be
tightened in the DBus 1.2 stable branch.  We will gather information about any
affected programs and make a final determination at in the near future.

== Conclusion Summary ==

* Add explicit <deny> rules under the default policy if this is applicable to your service (i.e. not using PolicyKit or similar)
* Do not put sensitive information in signals

== Thanks ==

Thanks to Joachim Breitner for the initial report and proposed patch, Tomas Hoger for the current fix, and others for their assistance with this issue.
Comment 36 Tomas Hoger 2008-12-05 11:09:26 UTC
(In reply to comment #34)
> Does anyone know of any other affected programs?

bluetooth.conf in bluez-utils only permit some messages from at_console users,  with no restriction for context=default, so looks affected.
Comment 37 Colin Walters 2008-12-05 11:26:13 UTC
(In reply to comment #36)
> (In reply to comment #34)
> > Does anyone know of any other affected programs?
> 
> bluetooth.conf in bluez-utils only permit some messages from at_console users, 
> with no restriction for context=default, so looks affected.

Ok, I'll try to touch base with them now and see if they can get an update ready.
Comment 38 Tomas Hoger 2008-12-05 11:46:58 UTC
(In reply to comment #37)
> > bluetooth.conf in bluez-utils only permit some messages from at_console
> > users, with no restriction for context=default, so looks affected.
> 
> Ok, I'll try to touch base with them now and see if they can get an update
> ready.

Update bluez?  Why?  Once dbus default policy is right, there will be no need for explicit denies in bluez.  Or have I managed to misunderstand what you referred to as affected programs?
Comment 39 Colin Walters 2008-12-05 11:57:42 UTC
Release done, lifting security tag.  I'll keep this bug open as a data gathering point for what to do about the receive aspect.
Comment 40 Colin Walters 2008-12-05 12:12:05 UTC
(In reply to comment #38)
> (In reply to comment #37)
> > > bluetooth.conf in bluez-utils only permit some messages from at_console
> > > users, with no restriction for context=default, so looks affected.
> > 
> > Ok, I'll try to touch base with them now and see if they can get an update
> > ready.
> 
> Update bluez?  Why?  Once dbus default policy is right, there will be no need
> for explicit denies in bluez.

Right, it's not strictly speaking necessary, it just seems prudent to add the <deny> rules.

Even besides that it seems important to touch base with those projects without the <deny>, simply because they were affected and should be aware of the impact.
Comment 41 Colin Walters 2008-12-07 13:10:53 UTC
Note - PackageKit broke with this change:

https://bugs.freedesktop.org/show_bug.cgi?id=18931
Comment 42 Sjoerd Simons 2008-12-07 16:14:09 UTC
avahi-daemon also breaks. It has several dbus interface from which it sends signals which are denied with this fix. Most of those interface also have methods but those should work correctly as it has <allow send_destination="org.freedesktop.Avahi"/>. The easiest way to test for this is by running avahi-discover.

I'll try to investigate the minimal fix tomorrow (if time allows, feel free to beat me to it). 
Comment 43 Tomas Hoger 2008-12-08 05:37:04 UTC
PolicyKit policy bug:
  https://bugs.freedesktop.org/show_bug.cgi?id=18948
Comment 44 Tomas Hoger 2008-12-08 05:52:04 UTC
(In reply to comment #42)
> avahi-daemon also breaks. It has several dbus interface from which it sends
> signals which are denied with this fix.

What is the interface used in the signals?  org.freedesktop.Avahi?  Probably adding:

  <allow send_interface="org.freedesktop.Avahi"/>

to the <policy group="avahi"> section should be the easiest fix (well, it may be better to bound it to the owner of the org.freedesktop.Avahi name as a sender, but that can't be done in the send rules).  On the receiver side, following already existing context="default" rule should be sufficient:

  <allow receive_sender="org.freedesktop.Avahi"/>

As for the fix proposed here:
  http://lists.freedesktop.org/archives/dbus/2008-December/010706.html
  <allow send_requested_reply="true" send_type="signal"/>

This rule will only be considered for messages of type signal.  send_requested_reply attribute will be ignored, so the rule will allow sending any signal, by any service connected to the bus (not only avahi-daemon).

Not quite sure what to look for in the avahi-discover output...
Comment 45 Colin Walters 2008-12-08 08:00:19 UTC
Created attachment 20899 [details] [review]
Allow-DBus.Introspection-and-DBus.Peer-by-default

This patch allows Introspection and Peer methods by default.
Comment 46 David Zeuthen (not reading bugmail) 2008-12-08 08:58:56 UTC
(In reply to comment #45)
> Created an attachment (id=20899) [details]
> Allow-DBus.Introspection-and-DBus.Peer-by-default
> 
> This patch allows Introspection and Peer methods by default.
> 

Hmm. I honestly think this is confusing. If we require that system services need to punch holes *anyway*, why not let them punch holes for Peer and Introspectable too? 

I think doing this automatically will just make it even harder to grasp how to use <allow> and <deny> correctly; it's already way too overcomplicated and we've seen both the implementation and users get it wrong.
Comment 47 Colin Walters 2008-12-08 09:26:25 UTC
(In reply to comment #46)
> (In reply to comment #45)
> > Created an attachment (id=20899) [details] [details]
> > Allow-DBus.Introspection-and-DBus.Peer-by-default
> > 
> > This patch allows Introspection and Peer methods by default.
> > 
> 
> Hmm. I honestly think this is confusing. If we require that system services
> need to punch holes *anyway*, why not let them punch holes for Peer and
> Introspectable too? 
> 
> I think doing this automatically will just make it even harder to grasp how to
> use <allow> and <deny> correctly; it's already way too overcomplicated and
> we've seen both the implementation and users get it wrong.

I think the bottom line is to use send_destination liberally.  I'll post about this on the list.
Comment 48 Colin Walters 2008-12-08 14:04:06 UTC
There is one major issue left to address: signals.  Basically, we should add a global 

<allow send_type="signal"/>


Comment 49 Colin Walters 2008-12-08 17:14:36 UTC
Created attachment 20924 [details] [review]
cleanup of system.d

Here is a first pass through the system.d directory.  The major themes are:

* Changing things to use send_destination.  This fixes the introspection handling.
* Removing rules of the form <deny send_interface="org.foo.Bar"/>.  These are actively harmful and break introspection - see bug 18961.  I also took the opportunity to remove rules that should be redundant now.
* Adding missing <context="default"><allow send_destination="com.foo.Baz"/> rules.
Comment 50 Colin Walters 2008-12-08 18:26:45 UTC
Created attachment 20928 [details] [review]
allow signals by default

In order to move forward with the system.d cleanup, I propose we allow all signals by default.  There is not great value in restricting the namespaces of sent signals.
Comment 51 Tomas Hoger 2008-12-09 01:10:47 UTC
Created attachment 20942 [details] [review]
Improvement for dbus-daemon manpage wrt policy rule application ordering

Colin, can you please consider this minor dbus-daemon man page improvement for next update?  It adds mention of at_console <policy> attribute.
Comment 52 Tomas Hoger 2008-12-09 01:44:04 UTC
(In reply to comment #50)
> In order to move forward with the system.d cleanup, I propose we allow all
> signals by default.  There is not great value in restricting the namespaces of
> sent signals.

Should receiving of all signals be allowed by default as well?  Are there any known use-cases of private signals?
Comment 53 Tomas Hoger 2008-12-09 02:52:09 UTC
(In reply to comment #49)
> Here is a first pass through the system.d directory.

Some thoughts...

--- system.d/avahi-dbus.conf

There are few receive_sender rules there as well that should only be needed for signals, so their fate actually depends on what the default for signals will be.


--- system.d/ConsoleKit.conf

Did you leave user="root" rules unchanged intentionally?  They look like a good candidate for allow send_destination.  Why not remove all those context="default" deny rules and only keep explicit allows.

The similar comments apply to gdm.conf.


--- system.d/dnsmasq.conf

Rule <allow send_interface="uk.org.thekelleys.dnsmasq"/> should only be needed for signal sending, so is candidate for removal if signals are permitted by default.


--- system.d/newprinternotification.conf

- <allow send_interface="com.redhat.NewPrinterNotification"/>

Again, may be needed for signals.


--- system.d/org.fedoraproject.Config.Services.conf

I'd prefer to see allow send_destination rule removed from user="root" section.  All context="default" rules apply to root as well, so having the rule only there should be sufficient.


--- system.d/org.gnome.ClockApplet.Mechanism.conf

This seems incorrect to me.  Probably something like this is needed:

@@ -8,12 +8,12 @@
   <!-- Only root can own the service -->
   <policy user="root">
     <allow own="org.gnome.ClockApplet.Mechanism"/>
-    <allow send_interface="org.gnome.ClockApplet.Mechanism.SetTimezoneInterface"/>
   </policy>
 
   <!-- Allow anyone to invoke methods on the interfaces -->
   <policy context="default">
-    <allow send_interface="org.gnome.ClockApplet.Mechanism.SetTimezoneInterface"/>
+    <allow send_destination="org.gnome.ClockApplet.Mechanism"
+           send_interface="org.gnome.ClockApplet.Mechanism.SetTimezoneInterface"/>
   </policy>
 
 </busconfig>

Not sure if binding to a particular interface is really needed.


--- system.d/org.gnome.*

Allow rules in user="root" section can be omitted when service should be accessible to all users and there's context="default" allow rule.


--- system.d/setroubleshootd.conf

It may requires some of the removed rules, or allow receive_sender rule for signal handling.

Similar probably applies to yum-updatesd.conf as well.
Comment 54 Colin Walters 2008-12-09 05:49:34 UTC
(In reply to comment #52)
> (In reply to comment #50)
> > In order to move forward with the system.d cleanup, I propose we allow all
> > signals by default.  There is not great value in restricting the namespaces of
> > sent signals.
> 
> Should receiving of all signals be allowed by default as well?  Are there any
> known use-cases of private signals?
> 

I believe receiving signals will be covered by the receive_requested_reply opening all receive side.
Comment 55 Colin Walters 2008-12-09 06:09:49 UTC
(In reply to comment #51)
> Created an attachment (id=20942) [details]
> Improvement for dbus-daemon manpage wrt policy rule application ordering
> 
> Colin, can you please consider this minor dbus-daemon man page improvement for
> next update?  It adds mention of at_console <policy> attribute.

Will do, I also have an update to the man page lying around.
Comment 56 Tomas Hoger 2008-12-09 06:48:23 UTC
(In reply to comment #54)
> > Should receiving of all signals be allowed by default as well?  Are there any
> > known use-cases of private signals?
> 
> I believe receiving signals will be covered by the receive_requested_reply
> opening all receive side.

Oh, well, that's correct.  Looks like I was thinking too long-term, assuming complete fix.
Comment 57 Colin Walters 2008-12-09 06:54:23 UTC
Thanks for the review of the system.d changes.  Let's actually break this list up into 3 separate parts:

1) Services broken by the new security policy (e.g. the original PolicyKit.conf could never have worked in a default-deny world)
2) Services which use <deny send_interface/> without also specifying send_destination.  These break no-interface requests from scripting languages.  
3) Cleanup

Suggest we focus on these in order.  I've created tracking bugs:

Broken: https://bugs.freedesktop.org/show_bug.cgi?id=18980
Deny send_interface: https://bugs.freedesktop.org/show_bug.cgi?id=18961
Cleanup: (We'll do this later)

Comment 58 Dan Ziemba 2008-12-09 12:45:00 UTC
(In reply to comment #34)
> 
> Does anyone know of any other affected programs?
> 

I am a Fedora 10 user, and I have been following this bug.  My system is running the updated dbus, and I have noticed two other affected applications:  system-config-services and system-config-samba.  It looks as though s-c-services has already been addressed in the system.d patch, but I think there is something wrong in the org.fedoraproject.Config.Samba.conf file for s-c-samba.  The file is identical to the org.fedoraproject.Config.Services.conf file except for the word Services, so I assume the same fix could be used on it?
Comment 59 Dan Ziemba 2008-12-09 15:40:46 UTC
(In reply to comment #58)
> (In reply to comment #34)
> > 
> > Does anyone know of any other affected programs?
> > 
> 
> I am a Fedora 10 user, and I have been following this bug.  My system is
> running the updated dbus, and I have noticed two other affected applications: 
> system-config-services and system-config-samba.  It looks as though
> s-c-services has already been addressed in the system.d patch, but I think
> there is something wrong in the org.fedoraproject.Config.Samba.conf file for
> s-c-samba.  The file is identical to the org.fedoraproject.Config.Services.conf
> file except for the word Services, so I assume the same fix could be used on
> it?
> 

Nevermind, I see that new versions of each have been build on Koji.  I have tested them both and the seem to work.
Comment 60 Joachim Breitner 2010-01-07 11:12:02 UTC
(In reply to comment #39)
> Release done, lifting security tag.  I'll keep this bug open as a data
> gathering point for what to do about the receive aspect.

I’m just going through my bugs here, and noticed this one. Is there still stuff to do or can this bug be closed?
Comment 61 Richard Hughes 2010-01-08 00:33:03 UTC
Yes. I think we can close this now. Thanks.