/snapd

interfaces/builtin: add dbus interface #1613

Merged
merged 111 commits into from Dec 15, 2016
Commits
Jump to file or symbol
Failed to load files and symbols.
+1,191 −0
Split
View
1 interfaces/builtin/all.go
@@ -28,6 +28,7 @@ var allInterfaces = []interfaces.Interface{
&BoolFileInterface{},
&BrowserSupportInterface{},
&ContentInterface{},
+ &DbusInterface{},
&DockerInterface{},
&DockerSupportInterface{},
&FwupdInterface{},
View
1 interfaces/builtin/all_test.go
@@ -35,6 +35,7 @@ func (s *AllSuite) TestInterfaces(c *C) {
c.Check(all, Contains, &builtin.BluezInterface{})
c.Check(all, Contains, &builtin.BoolFileInterface{})
c.Check(all, Contains, &builtin.BrowserSupportInterface{})
+ c.Check(all, Contains, &builtin.DbusInterface{})
c.Check(all, Contains, &builtin.DockerInterface{})
c.Check(all, Contains, &builtin.DockerSupportInterface{})
c.Check(all, Contains, &builtin.FwupdInterface{})
View
8 interfaces/builtin/basedeclaration.go
@@ -209,6 +209,14 @@ slots:
slot-snap-type:
- core
deny-auto-connection: true
+ dbus:
+ allow-installation:
+ slot-snap-type:
+ - app
+ deny-connection:
+ slot-attributes:
+ name: .+
+ deny-auto-connection: true
@niemeyer

niemeyer Nov 22, 2016

Contributor

Why the denials here? This will prevent people from using the interface altogether until explicitly allowed.

@jdstrand

jdstrand Nov 22, 2016

Contributor

Yes. I thought we agreed that people would need to have their well-known dbus name approved in the store. This will trigger a manual review and enforce the use of a snap declaration to claim the well-known name.

@jdstrand

jdstrand Nov 22, 2016

Contributor

Also, it doesn't prevent them from using the interface-- they can install a snap that slots this interface and get the bind rules needed to run; it is just that you can't connect other snaps to this interface until a snap declaration is present.

I think this is the right approach for this interface and others in general, but it sheds a light on a developer pain point as described in https://bugs.launchpad.net/snappy/+bug/1640874. I think this is perhaps solved by a locally signed snap declaration, but that needs your input (see my comments in the bug).

@niemeyer

niemeyer Nov 23, 2016

Contributor

We'd indeed not auto-connect it, but is there a reason to disable the manual connection altogether? This would be the user manually establishing the allowance for dbus communication between two separate snaps, over a well defined API. I can't quite see why we'd reject this yet.

@jdstrand

jdstrand Nov 23, 2016 •
Edited 1 time

Contributor

We reject it because the snap has not claimed the name via the store and doesn't yet have a snap declaration. On initial upload with using this interface, the reviewer will issue a snap declaration that allows the connection, and then all is well for connections and subsequent uploads. I think this is in line with everything we've discussed regarding the base declaration and snap declarations-- slot implementations (typically) need some sort of snap declaration since they (typically) allow privileged access to the system. In this case the privilege is to claim a well-known name.

On a related note, the review tools are now considering the base declaration and prompting for manual review due to this constraint. If we remove the constraint today, the tools would let this through without prompting for manual review. If we decide that the constraint should not be there, then the tools will have to be modified to prompt for manual review based on factors outside of the base declaration for an interface review (which is totally doable, but I thought one goal of the base declaration was to be the one place the review tools would consult for how interface reviews should be performed).
dcdbas-control:
allow-installation:
slot-snap-type:
View
1 interfaces/builtin/basedeclaration_test.go
@@ -350,6 +350,7 @@ var (
"boot-config": {"gadget"},
"browser-support": {"core"},
"content": {"app", "gadget"},
+ "dbus": {"app"},
"docker-support": {"core"},
"fwupd": {"app"},
"gpio": {"core", "gadget"},
Oops, something went wrong.