interfaces: bluez: fix dbus rules #1037

Closed
wants to merge 12 commits into
from

Conversation

Projects
None yet
6 participants
Contributor

ssweeny commented Apr 18, 2016

This fixes a few issues with the bluez interface, specifically making the dbus policy permanent, fixing the label to match what we see in snappy, and allowing obexd to claim its dbus name.

Simon Fels and others added some commits Apr 17, 2016

interfaces: bluez: create dbus policy permantenly
If we don't define the dbus policy snippet for the permanent slot bluez
wouldn't start automatically as long as not plug is connected. We want
the bluez service to run even when no connection exists.

Signed-off-by: Simon Fels <simon.fels@canonical.com>
interfaces: bluez: Fix the plug dbus peer label
The AppArmor peer label for the plug side was incorrect. This change
fixes a denial on the bluetoothctl client trying to connect
interfaces: bluez: Allow obexd to claim org.bluez.obexd
Add a missing dbus rule to allow obexd to claim its dbus interface
Contributor

zyga commented Apr 19, 2016

Looks good

Contributor

ssweeny commented Apr 19, 2016

Looks like the autopkgtest is hitting some issues downloading snaps. I don't think this is something my branch could have caused, but you're the expert @zyga 😄

Contributor

morphis commented Apr 19, 2016

Tested this with the snap from https://code.launchpad.net/~ssweeny/+snap/bluez/+build/601 in KVM instance having a BT dongle connect. Works fine, see https://paste.ubuntu.com/15931839/ for details.

interfaces/builtin/bluez.go
@@ -91,7 +96,7 @@ var bluezConnectedPlugAppArmor = []byte(`
# Allow all access to bluez service
dbus (receive, send)
bus=system
- peer=(label=bluez5_bluez_*),
+ peer=(label=snap.bluez.*),
@niemeyer

niemeyer Apr 20, 2016

Contributor

This is hard-coding an interface to be used by a specifically named snap, which doesn't seem to make sense for an interface in a platform.

@zyga can you please work with him to ensure we have the proper content in this template? I thought we had already discussed this last week?

@morphis

morphis Apr 21, 2016

Contributor

From what I got the agreement was that we go with this until we have a proper mechanism in place (which @zyga and @jdstrand are working on) to replace the peer label with the correct label from the connected plug.

@ssweeny

ssweeny Apr 21, 2016

Contributor

This was my understanding as well. With this fix we can have a working bluez snap in the store today (as opposed to the broken one that's in there now) while we wait on the proper fix.

@jdstrand

jdstrand Apr 21, 2016

Contributor

I agree with Gustavo but in the interest of time if people want to fix bluez (and network-manager) quickly, I would be ok with this but please add a comment:

FIXME: adjust to use 'snap..' from the connecting slot

Contributor

niemeyer commented Apr 20, 2016

That doesn't seem to reflect the conversation we had last week about this problem.

ssweeny added some commits Apr 21, 2016

Add a FIXME comment to the plug-side AppArmor rule
We want to replace the hardcoded peer label with one generated from the
connected plug.
Contributor

zyga commented Apr 25, 2016

I'm working on improving interface details now

zyga added some commits Apr 25, 2016

interfaces/builtin: add plug/slot for bluez tests
Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
interfaces/builtin: don't hard-code slot name in connected plug apparmor
This patch replaces the hard-coded peer label (snap.bluez.*) for connected
plugs to the security tag glob for the snap that has the slot. In practice
the content should be exactly the same but now it will not be hard-coded.

Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
Contributor

morphis commented Apr 25, 2016

@jdstrand @zyga @niemeyer So what is the decision? Do you want automatic peer label be fixed before we merge this? I would like to get this merged now and fix those bits afterwards so that we get a working snap back in the store.

Contributor

ssweeny commented Apr 25, 2016

@morphis @zyga sent me a patch to stop hardcoding the snap name. I think he's waiting on some input from @jdstrand before giving me the go-ahead to pull it in.

ssweeny added some commits Apr 25, 2016

Contributor

morphis commented Apr 26, 2016

@ssweeny Thanks. Once we have this here I will do the same for the networkmanager interface

Collaborator

mvo5 commented Apr 26, 2016

retest this please

Contributor

jdstrand commented Apr 26, 2016

I think we are deadlocked. I gave @zyga a patch that starts the non-hardcoding, but it required snapd changes to implement correctly which @zyga is working on. But then people said they are waiting on me-- can someone say specifically what this is still waiting on?

Contributor

jdstrand commented Apr 26, 2016

I'm told that the PR in question was zyga/snapd@16228ad which I didn't see in my email. I commented on that just now.

Contributor

zyga commented Apr 26, 2016

I'm closing this pull request. I've improved on this branch in #1078

@zyga zyga closed this Apr 26, 2016

@ssweeny ssweeny deleted the ssweeny:bluez-fix-rules branch Sep 6, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment