nixos/crowdsec: init

[M0ustach3]

Crowdsec NixOS module. This resolves a part of the module request #358457 .

This takes a lot of scaffolding from the amazing work of kampka in his Nix Flake code here : https://codeberg.org/kampka/nix-flake-crowdsec . Thank you again for this !

Things done

• Built on platform(s)
  • x86_64-linux
  • aarch64-linux
  • x86_64-darwin
  • aarch64-darwin
• For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
  • sandbox = relaxed
  • sandbox = true
• Tested, as applicable:
  • NixOS test(s) (look inside nixos/tests)
  • and/or package tests
  • or, for functions and "core" functionality, tests in lib/tests or pkgs/test
  • made sure NixOS tests are linked to the relevant packages
• Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
• Tested basic functionality of all binary files (usually in ./result/bin/)
• 25.05 Release Notes (or backporting 24.11 and 25.05 Release notes)
  • (Package updates) Added a release notes entry if the change is major or breaking
  • (Module updates) Added a release notes entry if the change is significant
  • (Module addition) Added a release notes entry if adding a new NixOS module
• Fits CONTRIBUTING.md.

---

Add a 👍 reaction to pull requests you find important.

[06kellyjac]

Looks very good, thanks @M0ustach3 & of course @kampka too

Added some points for changing, some minor nits, and some general comments.

---

I'm looking forwards to testing this on some of my devices.

[06kellyjac]

Do we want an optional interval/timer for this rather than just the oneshot?

[M0ustach3]

So you mean being able to change the default "daily" calendar configuration for the systemd timer ? There's already a timer but it's for the moment only daily

[06kellyjac]

Sorry yes I missed the timer definition above. Making it configurable with daily being the default would be best 🙂

[TLATER]

I'm a bit sad we're here :( I'd like to see a module, but the flake this is based on does a lot of things very... oddly, and this copies a lot of the issues. I've pointed some of the more egregious ones out, but this is quickly becoming me just rewriting the module through a review.

I separately spent a bunch of time trying to create a PR against the flake to fix it first, only to give up and write a separate module from scratch (also not quite there yet, I was still working on getting the whitelists and stuff to properly work, and then wanted to generalize to make sure I cover more use cases). I wouldn't like to see this land in its current state, since it will be quite difficult to fix this without breaking compatibility, and I'd rather not be stuck maintaining an out-of-tree module forever.

Of course, now you've already put a lot of work into this, too, and the general community's priorities exceed my own, so my 2c probably don't ultimately count for much. But boy does this do a lot of needlessly hacky things.

[TLATER]

Can we... not do this? Any switches between users should be accomodated with systemd features, not sudo, and we certainly should not hard-code exceptions for doas and not every other sudo replacement.

IMO most of this systemd service should not be defined with the NixOS module options in general, but instead use the service file provided by upstream, only adding an ExecStartPre as required with the NixOS module override.

[M0ustach3]

I agree for the sudo hard-coding, I'm removing it from the code.

However, for the service configuration, I would suggest to not use the service configuration from upstream, as the service configuration directives are way too open for a security engine. The options defined in kampka 's service configuration are, IMHO, way better than upstream. I'm also not really a fan of "live" patching a file from upstream to change the different used paths in the service definition to comply with NixOs' locations. I am, however, going to put all the options specified in upstream in the Nix definition, as this was not the case and I thank you for pointing that out.

I'm however seeking additional feedback on this, as I'ts my first time contributing to nixpkgs, and first time writing Nix code at the same time, so I'm not at all a nix guru :)

[TLATER]

Yeah, so, what I mean is you should use the upstream service, and then use systemd's built-in overrides feature which is supported by the systemd NixOS module.

This allows just using the upstream service definition so we don't have to duplicate that code, while changing anything that needs to be changed for NixOS (or more normal distribution) purposes.

In practice this'd look like so: https://gitea.tlater.net/tlaternet/tlaternet-server/src/branch/master/modules/crowdsec/default.nix#L359

... and a systemd.packages = [ pkgs.crowdsec ] somewhere, plus whatever other service settings you want to add downstream.

[TLATER]

I see you have since realized this :)

[kampka]

It's OK by me if you want to base an upstream module on my work, but please stop @ tagging me on these discussions.
I'm not interested in collaborating on this.

Thanks :)

[M0ustach3]

I've updated the code based off the many (thanks !) replies off this PR. I apologize for the apparently useless and ugly code but as I've said, I'm not a Nix expert and I'm trying to learn and improve as much as I can.

[SuperSandro2000]

That is almost anything except the security wrappers

[M0ustach3]

Yup totally right. I tried correcting this and setting ExecPaths and NoExecPaths with the binaries used for the systemd unit, but unfortunately, I wasn't able to make it work... If you have any idea on why it tells me crowdsec.service: Failed to execute /nix/store/7s4i6c5c5bifc35p86j5jg1d0smfzm3p-crowdsec-1.6.5/bin/crowdsec: Permission denied when putting this :

ExecPaths = [
  (lib.makeBinPath [ cfg.package ])
  (lib.makeBinPath [ setupScript ])
  (lib.makeBinPath [ pkgs.runtimeShellPackage ])
  (lib.makeBinPath [ cscli ])
  "/run/wrappers/bin/"
];
NoExecPaths = [
  "/"
];

[TLATER]

I wonder if the dynamic linker needs to be in a directory that's permitted to be executed as well. You can probably figure this out in detail with strace, or by enabling systemd debug logging.

It should be noted that only 4 other services in nixpkgs even attempt this, and they all just permit the full nix store. Notably one uses builtins.storeDir, which is probably a better idea.

I do think even just that buys us something, as it's much easier to shuffle binaries that should not be executed to anywhere else on the system, even if more granular access would be cool.

This list should probably not include the wrappers, though, afaik crowdsec doesn't call out to any such? Hell, the service sets RestrictSUIDGUID.

[TLATER]

Thanks for all the work on this, this is looking significantly better.

[TLATER]

Do we need to? We could install the configuration into /etc, at which point their definitions work just fine.

Notably, they do some fancy things to permit ExecReload, which is important since the upstream manual instructs you to use systemctl reload to refresh config. Their service makes extensive use of that SIGHUP to prevent having to take the service down, we should properly support this - the current config results in some unnecessary downtime.

[acuteaangle]

@M0ustach3 Hello! Do you still intend to maintain this? I’d love to work on this when I get some free time soon, but I’m not sure yet whether it’d be best to coördinate here or open a new PR.

[M0ustach3]

Hi @acuteaangle , I have a lot going on in my life and I didn’t quite have time to make the final checks for the remaining comments on this PR… I would love to have a little help for this ! You can either take the whole package and remove me, or add yourself as a maintainer so that, when I have time,I’ll be able to take a look !
Apologies for this,
Cheers

[TornaxO7]

I can try to take over but I have no experience in go and also not that much in nix... so I'd need to have some guidance 😅 Should I create a fork and with that a new PR?

[06kellyjac]

@TornaxO7 I'm not sure there will be much go particular stuff that'll need addressing so you'll be just fine 🙂
Yeah if you can take these changes on to your own fork & we can open a new PR referencing this one. Also just mention you're still learning the nix language & people will hopefully be extra clear in reviews.

My advice would be, make a new PR off master, and m0ustach3's fork as a remote, cherry pick the commits. Either start with nixos/crowdsec: init module skipping the add maintainer one, or pick all then remove that one in a rebase. (M0ustach3 is more than welcome to also be maintainer for the module but it's best these additions are made by the person & close to the time a PR is actually merged. It'd just be a quick PR after the main one is merged.)
Or if you'd prefer copy pasting the file and making a fresh commit would work. If you're making the fresh commit from the existing code you can add Co-authored-by: M0ustach3 <37956764+M0ustach3@users.noreply.github.com> for attribution.

I think overall this module is in a good spot, just needs a bit of final tweaking & testing.

[rharish101]

I'm also not good at Nix, but I'm up for testing this in a native (systemd-nspawn) container.

[TornaxO7]

Alright, then I'll maybe start this evening/night to create a PR if I find some time today. Next weekend at the latest.

[TornaxO7]

Alright. Here we go: #426875