Join GitHub today
GitHub is home to over 20 million developers working together to host and review code, manage projects, and build software together.
interfaces: add global gsettings interfaces #1193
Conversation
|
|
zyga
reviewed
May 23, 2016
| @@ -451,7 +451,7 @@ func (s *interfaceManagerSuite) TestDoSetupProfilesAddsImplicitSlots(c *C) { | ||
| // Ensure that we have slots on the OS snap. | ||
| repo := mgr.Repository() | ||
| slots := repo.Slots(snapInfo.Name()) | ||
| - c.Assert(slots, HasLen, 17) | ||
| + c.Assert(slots, HasLen, 18) |
zyga
May 23, 2016
Contributor
Can this be changed to say "more than 1" instead.
c.Assert(len(info.Slots) > 1, Equals, true)
Each new interface has to touch this
zyga
reviewed
May 23, 2016
| +# because this gives privileged access to sensitive information stored in | ||
| +# gsettings and allows adjusting settings of other applications. | ||
| + | ||
| +# dbus |
jdstrand
May 23, 2016
Contributor
This is seccomp, not apparmor so 'no'. I mentioned in other PRs that we probably do want to have a shorthand for certain things and dbus as a client is probably the most reasonable. That said, I think anything like that should be followed up in another PR.
zyga
reviewed
May 23, 2016
| + | ||
| +#include <abstractions/dconf> | ||
| +owner /{,var/}run/user/*/dconf/user w, | ||
| +owner @{HOME}/.config/dconf/user w, |
zyga
May 23, 2016
Contributor
Isn't this irrelevant since $HOME is really snap-specific home anyway?
jdstrand
May 23, 2016
Contributor
No, this is correct for 'global session gsettings' (that'll make more sense in my next response).
zyga
reviewed
May 23, 2016
| +#include <abstractions/dbus-session-strict> | ||
| + | ||
| +#include <abstractions/dconf> | ||
| +owner /{,var/}run/user/*/dconf/user w, |
zyga
May 23, 2016
Contributor
Is there a problem when writes use dconf and end up in the real /home/zyga/.config/dconf/user but reads are made from $HOME/.config/dconf/user (aka /home/zyga/snaps/$SNAP_NAME/$SNAP_REVISION/.config/dconf/user?
jdstrand
May 23, 2016
Contributor
Yes, that is absolutely a problem and not one which this PR is trying to address. It is important to remember this is for the 'global session gsettings' and as such, snaps need to use it as it is currently implemented, which is accessing /run/user//dconf/user and /home/zyga/.config/dconf/user.
The path /home/zyga/snaps/$SNAP_NAME/$SNAP_REVISION/.config/dconf/user is unknown to gsettings as implemented and it cannot be used. Also, the future plan of apparmor gsettings mediation means that apps can continue to use the well-known paths (ie, not ~/snaps/...) and dconf will mediate access to snap-specific directories in the dconf database itself rather than using the actual filesystem.
zyga
reviewed
May 23, 2016
| + connectedPlugAppArmor: gsettingsConnectedPlugAppArmor, | ||
| + connectedPlugSecComp: gsettingsConnectedPlugSecComp, | ||
| + reservedForOS: true, | ||
| + autoConnect: false, |
zyga
May 23, 2016
Contributor
It should auto connect (snaps don't work by default without it) it should be reviewed by the store.
jdstrand
May 23, 2016
Contributor
Well, this can be said of any interface that doesn't auto-connect, no? gsettings is used as a password store, contains other sensitive information and allows manipulating the user's session and other apps' settings, so I felt it prudent to not autoconnect.
jdstrand
Jun 3, 2016
Contributor
I'm going to treat this like 'home' and have it autoconnect and adjust store auto-approval accordingly.
zyga
added
the
Reviewed
label
May 23, 2016
jdstrand
changed the title from
interfaces: add global gsettings interfaces (Closes #1576308)
to
interfaces: add global gsettings interfaces (Closes LP#1576308)
May 23, 2016
niemeyer
changed the title from
interfaces: add global gsettings interfaces (Closes LP#1576308)
to
interfaces: add global gsettings interfaces
May 23, 2016
jdstrand
added some commits
May 24, 2016
|
Can you please master into this |
jdstrand
added some commits
Jun 2, 2016
zyga
reviewed
Jun 4, 2016
| @@ -454,7 +454,7 @@ func (s *interfaceManagerSuite) TestDoSetupProfilesAddsImplicitSlots(c *C) { | ||
| // NOTE: This is not an exact test as it duplicates functionality elsewhere | ||
| // and is was a pain to update each time. This is correctly handled by the | ||
| // implicit slot tests in snap/implicit_test.go | ||
| - c.Assert(len(slots) > 17, Equals, true) | ||
| + c.Assert(len(slots) > 18, Equals, true) |
jdstrand commentedMay 19, 2016
•
Edited 1 time
-
niemeyer
May 23, 2016
When connected, applications can access the global gsettings of the user's
session. This interface is restricted because it gives privileged access to
sensitive information stored in gsettings (such as passwords) and allows
adjusting settings of other applications.
In the future gsettings will gain AppArmor integration to allow safe,
application-specific access to gsettings, but for now this interface can be
used to unblock people until that work is completed.
Closes: LP#1576308