nixos/freeradius : init - Added freeradius service #34587

netixx · 2018-02-04T10:36:24Z

Resubmit because of branch mixup

Inspired from the dhcpd service implementation
Only 2 configurations options at the moment:
enabled and path to configfile

folder networking has been chosen because radius is mainly used for network device and network authentication (e.g. WPA2 Entreprise)

Motivation for this change

Freeradius package is already supported by nix, but no service exists for nixos. This patch adds basic support to start the freeradius service.

Things done

I tested the service configuration in my personal nix installation successfully.

Tested using sandboxing (nix.useSandbox on NixOS, or option build-use-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/)
Fits CONTRIBUTING.md.

dotlambda · 2018-02-04T10:46:16Z

nixos/modules/services/networking/freeradius.nix

+
+  freeradiusConfig = {
+
+    enable = mkOption {


Use mkEnableOption

dotlambda · 2018-02-04T10:48:31Z

nixos/modules/services/networking/freeradius.nix

+
+    configDir = mkOption {
+      type = types.nullOr types.path;
+      default = null;


Why is this allowed to be null? This could default to /var/lib/freeradius.

default value is set to /var/lib/freeradius. Without this parameter, the service will failed, so it makes sense to force default value. Default value for the radiusd command is /etc/raddb, should we use this ? I don't know about nixos policy on this.

Probably yes. I believe freeradius never writes to this directory, does it? If it doesn't, /etc makes sense.
It's always good to find inspiration in other distros: https://git.archlinux.org/svntogit/community.git/tree/trunk/freeradius.service?h=packages/freeradius
Depending on whether freeradius needs write access to the filesystem, you should consider ProtectSystem=strict and ReadWritePaths: https://www.freedesktop.org/software/systemd/man/systemd.exec.html

Btw, please amend/squash the commit(s) instead of adding new ones.

Done.

I used ProtectSystem=full because I had difficulties with ReadWritePaths= :

the pre-start would fail freeradius.service: Failed at step NAMESPACE spawning /nix/store/z0ppd98vmw50pc2fj6ycc71ayxvzyqsr-unit-script/bin/freeradius-pre-start: No such file or directory

I added ReadOnlyPaths="/nix/store" but it didn't solve the issue

also, radius may need to write files (to /var/log/radius and maybe others), so it didn't seem wise (at least until we generate the freeradius configuration from nixos expressions).

dotlambda · 2018-02-04T10:49:53Z

nixos/modules/services/networking/freeradius.nix

+      after = ["network-online.target"];
+      requires = ["network-online.target"];
+      preStart = ''
+        ${cfg.configDir}/certs/bootstrap


Is the user supposed to provide that script?

This is some kind of typo that I forgot to remove.

netixx · 2018-02-04T17:20:39Z

I pushed fixes according to your very good suggestions. I only have a doubt on the default value for the config directory parameter (see comment).

Also, I changed the requires dependency to a wants dependency, following discussions is this PR

andir · 2018-02-05T14:07:30Z

Are you able to construct a NixOS test that verifies that the module produces a working freeradius? Something with static username/password in the configuration would probably be sufficient.

netixx · 2018-02-06T20:01:42Z

What would the test consist on ? Doing an end to end test with a radius client ?

andir · 2018-02-06T20:04:23Z

Yes, nothing super fancy just ensuring the service gets up and answers on simple requests.

…

On Feb 6, 2018 9:01 PM, "netixx" ***@***.***> wrote: What would the test consist on ? Doing an end to end test with a radius client ? — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#34587 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAm_dJrujY8QPPriVjydtm2zgHvQTASEks5tSK-pgaJpZM4R4hZ0> .

netixx · 2018-02-07T07:32:22Z

In this case, we could do the following:
a. generate radius config (at least clients.conf and users files) with predefined keys
b. start the freeradius service
c. use the included radclient like so echo "User-Name=xxx,Password=xxx" | sudo radclient -s <your-server-ip> auth <shared secret> (example taken from here) and verify output/return code

However,

The configuration for freeradius consists on multiples files and folders (currently, I have 284 files in my config, which I constructed from the vanilla freeradius config).
I don't know which are strictly required.

What I suggest regarding tests is that we wait until we provide nixos configuration options (at least defining clients.conf and users) to generate a full working config (providing the other required files statically). With the complete configuration, we can start the server and perform tests.

fpletz · 2018-02-09T18:36:16Z

nixos/modules/services/networking/freeradius.nix

+
+  cfg = config.services.freeradius;
+
+    freeradiusService = cfg: optionalAttrs cfg.enable {


The optionalAttrs is not needed because you already guard the config below with mkIf. Also, returning the systemd service attrset is nicer than returning { freeradius = { ... }; } because the resulting service name can be chosen more elegantly.

Also, the indenten is slightly off, Should be on the same level as cfg.

Done.

I moved the freeradius name to the config section (not sure if that's where it should go). And I tested it in my setup.

@dotlambda

Inspired from the dhcpd service implementation Only 2 configurations options at the moment: - enabled - path to config directory (defaults to /etc/raddb) Implementation was also inspired from ArchLinux systemd file and corrected with @dotlambda and @fpletz help.

joachifm · 2018-02-17T09:19:53Z

Thank you

The freeradius service was merged with NixOS#34587 but the module was not added to module-list. This commit fixes that and enables the use of services.freeradius in nixos configuration.

GrahamcOfBorg added 6.topic: nixos 8.has: module (update) 10.rebuild-darwin: 0 10.rebuild-linux: 0 labels Feb 4, 2018

dotlambda reviewed Feb 4, 2018

View reviewed changes

netixx mentioned this pull request Feb 4, 2018

nixos/freeradius : init - Added freeradius service #34330

Closed

8 tasks

netixx force-pushed the add-freeradius-service branch from 7b0d554 to 4308652 Compare February 5, 2018 08:18

fpletz reviewed Feb 9, 2018

View reviewed changes

netixx force-pushed the add-freeradius-service branch from 4308652 to 2a2e885 Compare February 10, 2018 08:59

joachifm merged commit 71a32c3 into NixOS:master Feb 17, 2018

netixx deleted the add-freeradius-service branch February 17, 2018 10:03

netixx mentioned this pull request Jun 22, 2018

freeradius : added service to module-list #42394

Merged

9 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

nixos/freeradius : init - Added freeradius service #34587

nixos/freeradius : init - Added freeradius service #34587

netixx commented Feb 4, 2018

dotlambda Feb 4, 2018

netixx Feb 4, 2018

dotlambda Feb 4, 2018

netixx Feb 4, 2018

dotlambda Feb 4, 2018

netixx Feb 5, 2018

dotlambda Feb 4, 2018

netixx Feb 4, 2018

netixx commented Feb 4, 2018 •

edited

andir commented Feb 5, 2018

netixx commented Feb 6, 2018

andir commented Feb 6, 2018 via email

netixx commented Feb 7, 2018

fpletz Feb 9, 2018

fpletz Feb 9, 2018

netixx Feb 10, 2018

joachifm commented Feb 17, 2018


		cfg = config.services.freeradius;

		freeradiusService = cfg: optionalAttrs cfg.enable {

nixos/freeradius : init - Added freeradius service #34587

nixos/freeradius : init - Added freeradius service #34587

Conversation

netixx commented Feb 4, 2018

Motivation for this change

Things done

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

netixx commented Feb 4, 2018 • edited

andir commented Feb 5, 2018

netixx commented Feb 6, 2018

andir commented Feb 6, 2018 via email

netixx commented Feb 7, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

joachifm commented Feb 17, 2018

netixx commented Feb 4, 2018 •

edited