| Message ID | 1400947886-18103-1-git-send-email-zeeshanak@gnome.org |
|---|---|
| State | Changes Requested |
| Headers | show |
On Sat, 2014-05-24 at 17:11 +0100, Zeeshan Ali (Khattak) wrote: > It doesn't make sense to deny all non-root users access to all D-Bus API. > Lets at least give everyone the ability to receive signals, read > properties and introspect. Introspect seems fine, but signals and properties might contain private data like wifi keys? johannes
On Sun, May 25, 2014 at 7:46 AM, Johannes Berg <johannes@sipsolutions.net> wrote: > On Sat, 2014-05-24 at 17:11 +0100, Zeeshan Ali (Khattak) wrote: >> It doesn't make sense to deny all non-root users access to all D-Bus API. >> Lets at least give everyone the ability to receive signals, read >> properties and introspect. > > Introspect seems fine, but signals and properties might contain private > data like wifi keys? Thats a fair point. I see that the following interfaces don't have any private data so I'll provide a v2 of this patch with only granting access to them: fi.w1.wpa_supplicant1 fi.w1.wpa_supplicant1.Interface fi.w1.wpa_supplicant1.BSS
On Sun, 2014-05-25 at 11:10 +0100, Zeeshan Ali (Khattak) wrote: > > Introspect seems fine, but signals and properties might contain private > > data like wifi keys? > > Thats a fair point. I see that the following interfaces don't have any > private data so I'll provide a v2 of this patch with only granting > access to them: > > fi.w1.wpa_supplicant1 > fi.w1.wpa_supplicant1.Interface I don't think that's true for .Interface - I believe you can retrieve blobs which may contain private keys. It would also stand to reason that to avoid breaking this in the future something should be done in the *code* to mark things as safe for anyone. Does DBus provide any mechanism where you could tag properties in some way and enable permissions based on that, or would it require another interface? johannes
On Sun, May 25, 2014 at 11:29 AM, Johannes Berg <johannes@sipsolutions.net> wrote: > On Sun, 2014-05-25 at 11:10 +0100, Zeeshan Ali (Khattak) wrote: > >> > Introspect seems fine, but signals and properties might contain private >> > data like wifi keys? >> >> Thats a fair point. I see that the following interfaces don't have any >> private data so I'll provide a v2 of this patch with only granting >> access to them: >> >> fi.w1.wpa_supplicant1 >> fi.w1.wpa_supplicant1.Interface > > I don't think that's true for .Interface - I believe you can retrieve > blobs which may contain private keys. Which properties or signals are those exactly? > It would also stand to reason that to avoid breaking this in the future > something should be done in the *code* to mark things as safe for > anyone. Yeah. you can easily deny access from code if/when you need to. We do this sort of thing in Geoclue, where each client gets its own client object and we reject any access to it by anyone other than the related client. We also don't broadcast signals from these objects but send it only to related client. > Does DBus provide any mechanism where you could tag properties > in some way and enable permissions based on that, or would it require > another interface? Unfortunately not and that is why I think you want to not restrict access to properties through D-Bus policy but rather from code.
On Sun, May 25, 2014 at 11:10 AM, Zeeshan Ali (Khattak) <zeeshanak@gnome.org> wrote: > On Sun, May 25, 2014 at 7:46 AM, Johannes Berg > <johannes@sipsolutions.net> wrote: >> On Sat, 2014-05-24 at 17:11 +0100, Zeeshan Ali (Khattak) wrote: >>> It doesn't make sense to deny all non-root users access to all D-Bus API. >>> Lets at least give everyone the ability to receive signals, read >>> properties and introspect. >> >> Introspect seems fine, but signals and properties might contain private >> data like wifi keys? > > Thats a fair point. I see that the following interfaces don't have any > private data so I'll provide a v2 of this patch with only granting > access to them: > > fi.w1.wpa_supplicant1 > fi.w1.wpa_supplicant1.Interface > fi.w1.wpa_supplicant1.BSS Oh, actually that is not possible for properties as the interface for them is org.freedesktop.DBus.Properties. I'm afraid you'll have to do all properties access control from the code instead. I can still provide a patch that only gives access to signals on the above objects and I can even make it more specific if we want that?
On Sun, 2014-05-25 at 12:04 +0100, Zeeshan Ali (Khattak) wrote: > I'm afraid you'll have to do all properties access control from the > code instead. I can still provide a patch that only gives access to > signals on the above objects and I can even make it more specific if > we want that? Well, the signals have the same problem really - there's a "new blob" signal I believe which probably has all the blob properties. johannes
On Mon, May 26, 2014 at 9:28 AM, Johannes Berg <johannes@sipsolutions.net> wrote: > On Sun, 2014-05-25 at 12:04 +0100, Zeeshan Ali (Khattak) wrote: > >> I'm afraid you'll have to do all properties access control from the >> code instead. I can still provide a patch that only gives access to >> signals on the above objects and I can even make it more specific if >> we want that? > > Well, the signals have the same problem really - there's a "new blob" > signal I believe which probably has all the blob properties. As I said: 1. You can define a refined policy for signals so thats fixable. 2. For properties, you can deny access from code. BTW, can we please not talk in terms of facts and keep 'I believe' and 'probably' out of this discussion?
On Mon, 2014-05-26 at 10:56 +0100, Zeeshan Ali (Khattak) wrote: > On Mon, May 26, 2014 at 9:28 AM, Johannes Berg > <johannes@sipsolutions.net> wrote: > > On Sun, 2014-05-25 at 12:04 +0100, Zeeshan Ali (Khattak) wrote: > > > >> I'm afraid you'll have to do all properties access control from the > >> code instead. I can still provide a patch that only gives access to > >> signals on the above objects and I can even make it more specific if > >> we want that? > > > > Well, the signals have the same problem really - there's a "new blob" > > signal I believe which probably has all the blob properties. > > As I said: > > 1. You can define a refined policy for signals so thats fixable. > 2. For properties, you can deny access from code. Ok. > BTW, can we please not talk in terms of facts and keep 'I believe' and > 'probably' out of this discussion? Not really, that'd mean I have to reread the whole code for every email :) johannes
On Mon, 2014-05-26 at 10:56 +0100, Zeeshan Ali (Khattak) wrote: > On Mon, May 26, 2014 at 9:28 AM, Johannes Berg > <johannes@sipsolutions.net> wrote: > > On Sun, 2014-05-25 at 12:04 +0100, Zeeshan Ali (Khattak) wrote: > > > >> I'm afraid you'll have to do all properties access control from the > >> code instead. I can still provide a patch that only gives access to > >> signals on the above objects and I can even make it more specific if > >> we want that? > > > > Well, the signals have the same problem really - there's a "new blob" > > signal I believe which probably has all the blob properties. > > As I said: > > 1. You can define a refined policy for signals so thats fixable. > 2. For properties, you can deny access from code. That seems like the right way forward... I guess we have to audit the signals and properties to make sure what the permissions should be. But we really should do that anyway :) I can try to look at that this week, but can't guarantee when exactly I'd get to it. Dan
diff --git a/wpa_supplicant/dbus/dbus-wpa_supplicant.conf b/wpa_supplicant/dbus/dbus-wpa_supplicant.conf index c091234..06c9515 100644 --- a/wpa_supplicant/dbus/dbus-wpa_supplicant.conf +++ b/wpa_supplicant/dbus/dbus-wpa_supplicant.conf @@ -23,5 +23,22 @@ <deny send_destination="fi.w1.wpa_supplicant1"/> <deny send_interface="fi.w1.wpa_supplicant1"/> <deny receive_sender="fi.w1.wpa_supplicant1" receive_type="signal"/> + + <!-- Allow receiving signals --> + <allow receive_sender="fi.w1.wpa_supplicant1" + receive_type="signal"/> + + <!-- Allow reading properties --> + <allow send_destination="fi.w1.wpa_supplicant1" + send_interface="org.freedesktop.DBus.Properties" + send_member="Get"/> + + <allow send_destination="fi.w1.wpa_supplicant1" + send_interface="org.freedesktop.DBus.Properties" + send_member="GetAll"/> + + <!-- Allow full access to introspection --> + <allow send_destination="fi.w1.wpa_supplicant1" + send_interface="org.freedesktop.DBus.Introspectable"/> </policy> </busconfig>
It doesn't make sense to deny all non-root users access to all D-Bus API. Lets at least give everyone the ability to receive signals, read properties and introspect. Signed-off-by: Zeeshan Ali (Khattak) <zeeshanak@gnome.org> --- wpa_supplicant/dbus/dbus-wpa_supplicant.conf | 17 +++++++++++++++++ 1 file changed, 17 insertions(+)