Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

nixos/opensnitch: Add module for opensnitch #132319

Merged
merged 1 commit into from
Sep 19, 2021
Merged

Conversation

onny
Copy link
Contributor

@onny onny commented Aug 1, 2021

Motivation for this change

Opensnitch is one of the few Linux application firewalls. It is well maintained and we already have a package for it in Nixpkgs. As requested in this issue, this PR provides a module for OpenSnitch.

This PR is a continuation of an earlier one. It just enables the systemd service without any further configuration, which is not needed for using the daemon.

It depends on an other PR for including systemd unit files in the opensnitch package.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • 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 nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • 21.11 Release Notes (or backporting 21.05 Relase 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.

@github-actions github-actions bot added 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: module (update) This PR changes an existing module in `nixos/` labels Aug 1, 2021
@ofborg ofborg bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 1-10 labels Aug 1, 2021
Copy link
Member

@aanderse aanderse left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great module. Would merge as is. I have left some comments... they are all nitpicks, though. Hopefully nothing too annoying. If you want a hand with anything, or strongly disagree, please do mention.

Would it be possible in a future PR to read in the NixOS firewall rules and apply them here as well?

nixos/modules/services/security/opensnitch.nix Outdated Show resolved Hide resolved
nixos/modules/services/security/opensnitch.nix Outdated Show resolved Hide resolved
nixos/modules/services/security/opensnitch.nix Outdated Show resolved Hide resolved
nixos/modules/services/security/opensnitch.nix Outdated Show resolved Hide resolved
a package's store path. Intended for process rules
which should survive NixOS updates.
'';
default = with pkgs; [ nix ];
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe the same comment about default above applies here as well...

nixos/modules/services/security/opensnitch.nix Outdated Show resolved Hide resolved
nixos/modules/services/security/opensnitch.nix Outdated Show resolved Hide resolved
nixos/modules/services/security/opensnitch.nix Outdated Show resolved Hide resolved
nixos/modules/services/security/opensnitch.nix Outdated Show resolved Hide resolved
nixos/modules/services/security/opensnitch.nix Outdated Show resolved Hide resolved
@github-actions github-actions bot added 8.has: changelog 8.has: documentation This PR adds or changes documentation labels Aug 4, 2021
@ofborg ofborg bot added the 2.status: merge conflict This PR has merge conflicts with the target branch label Aug 4, 2021
@ofborg ofborg bot removed the 2.status: merge conflict This PR has merge conflicts with the target branch label Aug 5, 2021
@onny onny changed the title Opensnitch: Add module for opensnitchd and opensnitch-ui [DRAFT] nixos/opensnitch: Add module for opensnitch Sep 14, 2021
@onny
Copy link
Contributor Author

onny commented Sep 14, 2021

I reworked this PR and module which now simply enables the opensnitch systemd service. Depends on including systemd unit files in opensnitch package.

@onny onny changed the title [DRAFT] nixos/opensnitch: Add module for opensnitch nixos/opensnitch: Add module for opensnitch Sep 14, 2021
@onny onny requested review from tilpner and aanderse September 14, 2021 16:52
Copy link
Member

@aanderse aanderse left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@onny sorry for the delay. All tested, ready to merge? LGTM 👍

@onny
Copy link
Contributor Author

onny commented Sep 17, 2021

It works fine but it depends on this PR #137679 :)

@onny onny requested a review from lxea September 17, 2021 09:02
@onny
Copy link
Contributor Author

onny commented Sep 18, 2021

#137679 got merged and this PR is ready to go ;)

@aanderse aanderse merged commit 5594495 into NixOS:master Sep 19, 2021
@aanderse
Copy link
Member

@onny awesome stuff. What are the plans from here? Once the ui is in place what happens if a sysadmin has set rules with the NixOS firewall declaratively? If a user modifies these NixOS rules with opensnitch the rules will come back as soon as the firewall is reloaded.

Thoughts?

@asdf8dfafjk
Copy link
Contributor

Stuff like this is when you kinda wish github added all the stars/bouquets one has received :)

@asdf8dfafjk
Copy link
Contributor

For anyone only following the "latest" post- rules are in /var/lib/opensnitch/rules

Also, for those who didn't know, like me, opensnitch bundles adblocking too (https://github.com/evilsocket/opensnitch/blob/be32ddc574f70157e214f9018ea42e4b24148202/utils/legacy/make_ads_rules.py), although looks like this has some hardcoded paths too.

A bit of suggestion for future work "to whomsoever it might concern :)"

Ideally there would be "nixy" way of specifying rules so that every update doesn't need me to recreate all the rules (would get boring soon). A starting point for that work could be https://github.com/evilsocket/opensnitch/blob/8d3540f7f95b40afdd44255ab0ab22ce23cbd333/daemon/rule/rule.go. So I envisage an array of options with the fields same as the go file that would be serialized to a json inside /var/lib/opensnitch/rules.

@onny
Copy link
Contributor Author

onny commented Sep 20, 2021

@onny awesome stuff. What are the plans from here? Once the ui is in place what happens if a sysadmin has set rules with the NixOS firewall declaratively? If a user modifies these NixOS rules with opensnitch the rules will come back as soon as the firewall is reloaded.

Thoughts?

I'm planning to add the UI part as a service/module to home-manager. I'm only using it to ask for all apps as default without any predefined rules. So this is not my use case and maybe someone else could add further config options?

@onny onny deleted the opensnitch branch February 4, 2022 11:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: changelog 8.has: documentation This PR adds or changes documentation 8.has: module (update) This PR changes an existing module in `nixos/` 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 1-10
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants