interfaces: allow plugging DBus clients to introspect the slot service #3266

Merged
merged 13 commits into from May 4, 2017

Conversation

Projects
None yet
3 participants
Contributor

jdstrand commented May 3, 2017

A number of DBus services are missing DBus inspection rules allowing clients to introspect the service. This came up specifically in the case of the 'shutdown' interface when a connected snap was using 'pydbus'. In the course of my investigation I discovered that tools like dbus-send, gdbus and qdbus (unsurprisingly) did not require introspection rules to call methods and neither did the python-dbus. In the case of pydbus, the act of obtaining the proxy object (ie, something you do before calling any methods) was enough to cause the library to perform DBus introspection on the service.

I went through all the interfaces that have dbus policy on the plugs side and found that 11 interfaces already allowed it, 11 did not allow it but should (if you are counting files changed, unity8_pim_common counts as 2 because it affect unity8-calendar and unity8-contacts), 1 could not allow it (avahi-observe) and 8 had dbus rules but for one reason or another did not require changing.

Interestingly, fwupd, ofono, and avahi-observe all use the same non-service-specific object path of '/'. Because this object is not service-specific, care needed to be taken to not allow introspecting services beyond what the interface should allow:

  • fwupd limits the introspection to label=###SLOT_SECURITY_TAGS### (###SLOT_SECURITY_TAGS### is never 'unconfined' in this interface) and adds a comment to remind future policy developers to be careful
  • ofono adjusts ofonoConnectedPlugAppArmor to limit the introspection to label=###SLOT_SECURITY_TAGS### and adds a comment to ofonoConnectedPlugAppArmorClassic to remind future policy developers to be careful
  • avahi-observe as an implicit-only slot does not allow introspection (since it would allow introspecting all unconfined services that use an object path of '/', not just avahi-observe) and adds a comment to remind future policy developers to be careful

jdstrand added some commits May 2, 2017

zyga approved these changes May 3, 2017

+1

Thank you for explaining this.
FYI: in case anyone wonders: https://dbus.freedesktop.org/doc/dbus-specification.html#introspection-format shows that the introspection data can leak what is on the bus (objects paths/interfaces)

Contributor

morphis commented May 4, 2017

LGTM, though a spread test case for one of these changed interfaces would be quite nice to verify introspection is now really working.

@zyga zyga merged commit f220b63 into snapcore:master May 4, 2017

7 checks passed

artful-amd64 autopkgtest finished (success)
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
xenial-amd64 autopkgtest finished (success)
Details
xenial-i386 autopkgtest finished (success)
Details
xenial-ppc64el autopkgtest finished (success)
Details
yakkety-amd64 autopkgtest finished (success)
Details
zesty-amd64 autopkgtest finished (success)
Details
Contributor

jdstrand commented May 4, 2017

Thanks for the reviews and merge!

@jdstrand jdstrand deleted the jdstrand:allow-instrospection branch May 4, 2017

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