nixos/geoclue2: make configurable, can whitelist applications #45306

worldofpeace · 2018-08-18T14:13:27Z

Motivation for this change

Non-functioning elementary geoclue2 agent

Eventually we'd like something similar to this but I'm unsure also, as mentioned, if elementary should be adding their agent into geoclue.conf upstream.

Things done

Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
Built on platform(s)
- NixOS
- macOS
- other Linux distributions
Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
Tested compilation of all pkgs that depend on this change using nix-shell -p nox --run "nox-review wip"
Tested execution of all binary files (usually in ./result/bin/)
Determined the impact on package closure size (by running nix path-info -S before and after)
Fits CONTRIBUTING.md.

cc @jtojnar

nixos/modules/services/desktops/geoclue2.nix

jtojnar · 2018-08-18T14:25:51Z

nixos/modules/services/desktops/geoclue2.nix

@@ -61,6 +88,106 @@ in
        wantedBy = [ "default.target" ];
      };
    };
+
+    environment.etc."geoclue/geoclue.conf".text = ''
+    # Configuration file for Geoclue


Do we need most of these options or can we leave them implicit?

I'm seeing other projects just leaving it as is aside from using a custom geolocation api key from google.

One concern with the explicit defaults is that user cannot override them with extraConfig. If they cannot be omitted, we would need to make them configurable.

Very true, maybe I'll make an option only to create more application configurations (since that's the only thing I'll need currently), and remove extraConfig. I'll deal with the rest later.

It is mainly about the things already in the text. For example, someone might want to switch to google services or other provider. Or change their leaderboard name.

Yeah I also think this config file should be as minimal as possible, because it's not really possible to change anything in it, only add to it. The default upstream file is intended to be modified, which we can't like this

Alternatively, create a config option that transforms a Nix value into the config file syntax, and provide upstream defaults via config.gnome-color-panel.allowed = mkDefault true;. Then it's possible to override everything. I'm doing pretty much the same for #41467 (which I still need to finish)

The thing is that the config file's syntax and structure is not documented. So doing that fully is unclear.

nixos/modules/services/desktops/geoclue2.nix

jtojnar · 2018-08-20T14:55:10Z

nixos/modules/services/desktops/geoclue2.nix

+
+    # Whitelist of desktop IDs (without .desktop part) of all agents we recognise,
+    # separated by a ';'.
+    whitelist=${optionalString cfg.enableDemoAgent semiColon "geoclue-demo-agent"}${semiColon "gnome-shell"}${semiColon cfg.extraWhitelistedAgents}


This should not work for many reasons. I would try something like whitelist=${let whitelist = optional cfg.enableDemoAgent "geoclue-demo-agent" ++ singleton "gnome-shell" ++ cfg.extraWhitelistedAgents; in semiColon whitelist}

nixos/modules/services/desktops/geoclue2.nix

infinisil · 2018-08-20T16:17:04Z

nixos/modules/services/desktops/geoclue2.nix

@@ -61,6 +88,106 @@ in
        wantedBy = [ "default.target" ];
      };
    };
+
+    environment.etc."geoclue/geoclue.conf".text = ''
+    # Configuration file for Geoclue


Yeah I also think this config file should be as minimal as possible, because it's not really possible to change anything in it, only add to it. The default upstream file is intended to be modified, which we can't like this

Alternatively, create a config option that transforms a Nix value into the config file syntax, and provide upstream defaults via config.gnome-color-panel.allowed = mkDefault true;. Then it's possible to override everything. I'm doing pretty much the same for #41467 (which I still need to finish)

michaelpj · 2018-08-20T16:29:51Z

So, as I understand it, the reason some gnome apps are hardcoded in is because geoclue doesn't have a "drop-in" mechanism for additional config files. So it's a real pain for installed applications to add themselves. This seems like a mistake, but not much we can do about it.

However, if we generate the file like this then we can allow modules to configure their own rules, even though upstream doesn't give us a drop-in mechanism. This seems like a win to me.

michaelpj · 2018-10-14T11:55:36Z

This is still relevant - upstream have made some progress in making "system" apps not need the agent, so once they do a release we'll want to switch to whitelisting using an approach like this.

worldofpeace · 2018-10-14T18:40:04Z

@michaelpj Cool, I'll finish cleaning this up in that case.

Edit: That was quick 🤣

worldofpeace · 2018-12-01T00:53:38Z

So I'm ok with this, though it's rather unfortunate that I wrote this without awareness of lib.generators.toINI.
That would have made this much simpler.
If any maintainer feels like making that change feel free to push it.

@michaelpj

Note that in recent versions they've dropped the agent requirement for "system" apps, so we can potentially drop that whole annoyance and just go back to modifying the config file.

I think that means there's no usecase for enableDemoAgent or extraWhitelistedAgents, and the hacks for redshift can be dropped and replaced with

services.geoclue2.appConfig."redshift" = {
  isAllowed = true;
  isSystem = true;
};

I think...

michaelpj · 2018-12-01T11:16:38Z

Yes, I agree. I'm happy to make that change once this is in, or you can do it.

All options within geoclue.conf[1] have been made configurable. Additonally, we can now specify whether or not GeoClue should ask the agent to authorize an application like so: ``` services.geoclue2.appConfig."redshift" = { isAllowed = true; isSystem = true; }; ``` [1]: https://gitlab.freedesktop.org/geoclue/geoclue/blob/master/data/geoclue.conf.in

worldofpeace · 2018-12-02T04:07:11Z

I've dropped extraWhitelistedAgents because people should just upstream that config to geoclue IIRC.

I feel like the removal of enableDemoAgent should be in a different pr, coinciding with the redshift change once this is in.

@jtojnar Is this ok, or do you still have reservations about it?

michaelpj · 2018-12-04T19:52:54Z

I had a brief look at what they actually did to implement "system apps don't need authorization" and I'm not sure that it corresponds to the system config option at all. I'll investigate separately.

michaelpj · 2018-12-04T19:54:38Z

Things I would like to see from this:

Use toINI
Move whichever bits of configuration we can to the right modules. Just because upstream doesn't have a drop-in mechanism and so has to hardcode things doesn't mean that we have to.

worldofpeace · 2018-12-04T20:20:35Z

Move whichever bits of configuration we can to the right modules.

I can do that for gnome, however I can't for epiphany, and for firefox I'm not sure.

michaelpj · 2018-12-04T20:27:27Z

Sure, for things that don't have a module there's nothing we can do at the moment.

michaelpj · 2018-12-04T20:28:05Z

Also I guess the whitelisting of the elementary agent could come from that module?

leotaku · 2019-04-30T20:03:11Z

Is there anything we can do to push this forward?

@michaelpj are you still interested in rewriting/seeing this rewritten to use toINI?

michaelpj · 2019-04-30T22:29:19Z

I still think this would be useful indeed! I kind of gave up on geoclue, but it would be nice if it worked better.

worldofpeace · 2019-05-16T22:46:58Z

Completed in #60681 ✨

GrahamcOfBorg added 6.topic: nixos 8.has: module (update) labels Aug 18, 2018

jtojnar reviewed Aug 18, 2018

View reviewed changes

nixos/modules/services/desktops/geoclue2.nix Outdated Show resolved Hide resolved

jtojnar reviewed Aug 18, 2018

View reviewed changes

GrahamcOfBorg added 10.rebuild-darwin: 0 10.rebuild-linux: 0 labels Aug 18, 2018

jtojnar reviewed Aug 18, 2018

View reviewed changes

nixos/modules/services/desktops/geoclue2.nix Outdated Show resolved Hide resolved

worldofpeace force-pushed the geoclue2-whitelisted-agents branch 4 times, most recently from 27899e3 to 0814aac Compare August 20, 2018 09:44

jtojnar reviewed Aug 20, 2018

View reviewed changes

worldofpeace force-pushed the geoclue2-whitelisted-agents branch from 0814aac to b464d6a Compare August 20, 2018 15:30

michaelpj reviewed Aug 20, 2018

View reviewed changes

nixos/modules/services/desktops/geoclue2.nix Outdated Show resolved Hide resolved

michaelpj reviewed Aug 20, 2018

View reviewed changes

nixos/modules/services/desktops/geoclue2.nix Outdated Show resolved Hide resolved

infinisil requested changes Aug 20, 2018

View reviewed changes

michaelpj mentioned this pull request Oct 14, 2018

redshift doesn't work with geoclue location provider #45994

Closed

worldofpeace force-pushed the geoclue2-whitelisted-agents branch from ac1435c to e909ed3 Compare October 14, 2018 23:12

worldofpeace force-pushed the geoclue2-whitelisted-agents branch from e909ed3 to 9ba805d Compare November 30, 2018 22:46

GrahamcOfBorg added 10.rebuild-linux: 1-10 and removed 10.rebuild-linux: 0 labels Nov 30, 2018

worldofpeace mentioned this pull request Nov 30, 2018

geoclue2: 2.4.12 -> 2.5.1 #48438

Merged

9 tasks

worldofpeace force-pushed the geoclue2-whitelisted-agents branch from 9ba805d to 84a126f Compare December 2, 2018 03:56

worldofpeace changed the title ~~nixos/geoclue2: add ability to whitelist agents and append to config~~ nixos/geoclue2: make configurable, can whitelist applications Dec 2, 2018

nixos/geoclue2: modularize declared configurations

807f3f3

leotaku mentioned this pull request May 1, 2019

nixos/geoclue2: restructure module made by worldofpeace #60681

Merged

10 tasks

worldofpeace closed this May 16, 2019

worldofpeace deleted the geoclue2-whitelisted-agents branch May 16, 2019 22:47

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

nixos/geoclue2: make configurable, can whitelist applications #45306

nixos/geoclue2: make configurable, can whitelist applications #45306

worldofpeace commented Aug 18, 2018

jtojnar Aug 18, 2018

worldofpeace Aug 18, 2018

jtojnar Aug 18, 2018 •

edited

worldofpeace Aug 18, 2018 •

edited

jtojnar Aug 18, 2018

infinisil Aug 20, 2018

worldofpeace Aug 24, 2018

jtojnar Aug 20, 2018

infinisil Aug 20, 2018

michaelpj commented Aug 20, 2018

michaelpj commented Oct 14, 2018

worldofpeace commented Oct 14, 2018 •

edited

worldofpeace commented Dec 1, 2018

michaelpj commented Dec 1, 2018

worldofpeace commented Dec 2, 2018

michaelpj commented Dec 4, 2018 •

edited

michaelpj commented Dec 4, 2018

worldofpeace commented Dec 4, 2018

michaelpj commented Dec 4, 2018

michaelpj commented Dec 4, 2018

leotaku commented Apr 30, 2019

michaelpj commented Apr 30, 2019

worldofpeace commented May 16, 2019

nixos/geoclue2: make configurable, can whitelist applications #45306

nixos/geoclue2: make configurable, can whitelist applications #45306

Conversation

worldofpeace commented Aug 18, 2018

Motivation for this change

Things done

jtojnar Aug 18, 2018

Choose a reason for hiding this comment

worldofpeace Aug 18, 2018

Choose a reason for hiding this comment

jtojnar Aug 18, 2018 • edited

Choose a reason for hiding this comment

worldofpeace Aug 18, 2018 • edited

Choose a reason for hiding this comment

jtojnar Aug 18, 2018

Choose a reason for hiding this comment

infinisil Aug 20, 2018

Choose a reason for hiding this comment

worldofpeace Aug 24, 2018

Choose a reason for hiding this comment

jtojnar Aug 20, 2018

Choose a reason for hiding this comment

infinisil Aug 20, 2018

Choose a reason for hiding this comment

michaelpj commented Aug 20, 2018

michaelpj commented Oct 14, 2018

worldofpeace commented Oct 14, 2018 • edited

worldofpeace commented Dec 1, 2018

michaelpj commented Dec 1, 2018

worldofpeace commented Dec 2, 2018

michaelpj commented Dec 4, 2018 • edited

michaelpj commented Dec 4, 2018

worldofpeace commented Dec 4, 2018

michaelpj commented Dec 4, 2018

michaelpj commented Dec 4, 2018

leotaku commented Apr 30, 2019

michaelpj commented Apr 30, 2019

worldofpeace commented May 16, 2019

jtojnar Aug 18, 2018 •

edited

worldofpeace Aug 18, 2018 •

edited

worldofpeace commented Oct 14, 2018 •

edited

michaelpj commented Dec 4, 2018 •

edited