interfaces/desktop: allow access to Mutter IdleMonitor idle time

[alexmurray]

Whilst other methods on the Mutter IdleMonitor look potentially problematic,
GetIdletime should be relatively safe to expose to all desktop applications - in
particular this is used by firefox nowadays as seen in
https://bugs.launchpad.net/ubuntu/+source/firefox/+bug/2037655

Signed-off-by: Alex Murray alex.murray@canonical.com

---

Thanks for helping us make a better snapd!
Have you signed the license agreement and read the contribution guide?

[codecov-commenter]

Codecov Report

Merging #13304 (9228ace) into master (6aa9e0e) will decrease coverage by 0.01%.
The diff coverage is 100.00%.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

@@            Coverage Diff             @@
##           master   #13304      +/-   ##
==========================================
- Coverage   78.81%   78.80%   -0.01%     
==========================================
  Files        1019     1019              
  Lines      126593   126603      +10     
==========================================
+ Hits        99768    99772       +4     
- Misses      20589    20594       +5     
- Partials     6236     6237       +1     

Flag	Coverage Δ
unittests	78.80% <100.00%> (-0.01%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files	Coverage Δ
interfaces/builtin/desktop.go	92.10% <100.00%> (+0.75%)	⬆️

... and 5 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

[jhenstridge]

This seems relatively safe.

Looking at the Firefox source, it looks like it's only using the one method, so there's presumably not going to be a follow up denial once this is allowed:

https://searchfox.org/mozilla-central/source/widget/gtk/nsUserIdleServiceGTK.cpp#160-162

Longer term, it'd be good to see if apps like this could instead use the Idle portal:

https://github.com/flatpak/xdg-desktop-portal/blob/main/data/org.freedesktop.impl.portal.Inhibit.xml

It's not clear whether it directly maps to what Firefox wants though: the CreateMonitor() method can tell you when the screensaver activates, but doesn't give the exact idle time.

[jhenstridge]

This seems like something that'd make sense to include in desktopConnectedPlugAppArmor rather than desktopConnectedPlugAppArmorClassic. We'd need to use something other than label=unconfined when it's not an implicit slot.

We can always handle this later if you'd prefer though.

[alexmurray]

Yes, that is a good point. So ideally we want this in desktopConnectedPlugAppArmor but then to handle the label=unconfined case properly - the only snap which provides this DBus name currently is the ubuntu-desktop-session snap - in which case the label would be snap.ubuntu-desktop-session right? - I'll see what I can come up with to handle that properly in this PR.

[alexmurray]

Or we just drop the peer=(label=...) restriction on this rule entirely / switch it to peer=(name=org.gnome.Mutter.IdleMonitor), instead

[alexmurray]

I went with the option to change this to peer=(name=org.gnome.Mutter.IdleMonitor), instead - let me know what you think.

[jhenstridge]

This is closer to what I was thinking of (although probably using implicitSystemConnectedSlot(slot) rather than release.OnClassic):

snapd/interfaces/builtin/network_manager.go

Lines 522 to 532 in 6d1a3ce

	old := "###SLOT_SECURITY_TAGS###"
	var new string
	if release.OnClassic {
	// If we're running on classic NetworkManager will be part
	// of the OS snap and will run unconfined.
	new = "unconfined"
	} else {
	new = slotAppLabelExpr(slot)
	}
	snippet := strings.Replace(networkManagerConnectedPlugAppArmor, old, new, -1)
	spec.AddSnippet(snippet)

It's a little more involved than your current patch, since we haven't yet got the boilerplate for this in the interface. That's why I said I'd understand if you left it as something to deal with in future.

[alexmurray]

Yeah I had assumed you meant something like this - I also prefer this approach to be honest as it ends up being more strictly correct than my current proposal. Will try and rework it along these lines. Thanks @jhenstridge.

[alexmurray]

Hopefully b56a612 is what you had in mind.

[pedronis]

lgtm

[MiguelPires]

@alexmurray I hope you don't mind, I merged master in your branch to pull in some spread test fixes so this can merge

[alexmurray]

Of course not, thanks @MiguelPires