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 support for EPBF process monitor #229627

Merged
merged 1 commit into from
Aug 13, 2023

Conversation

onny
Copy link
Contributor

@onny onny commented May 3, 2023

Description of changes

Adding support for EPBF process monitor. As described here in the manual, this is the default monitor mode for OpenSnitch because it is more efficent and secure.

Nftables usage and epbf monitor mode is now default as defined in upstream configuration. The old monitor method can be configured like this:

services.opensnitch = {
  enable = true;
  settings.ProcMonitorMethod = "proc";
};

Optionally depends on upstream ability to configure the kernel module path evilsocket/opensnitch#928

Sucessfull build depends on opensnitch version update #246373

Work was done by @Slime90 which I'll add as a co-author to this PR.

Fixes #227294

Things done
  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
  • Tested, as applicable:
  • 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/)
  • 23.05 Release Notes (or backporting 22.11 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.

@Slime90
Copy link

Slime90 commented May 3, 2023

@onny looks good, I did some build tests last night, can you add a conditional for kernel version to be at least 5.10?

opensnitch-ebpf = if lib.versionAtLeast kernel.version "5.10” then callPackage ../os-specific/linux/opensnitch-ebpf { } else null;

I don’t think a path flag being added In the upstream source is a hard requirement for this PR, since the file can still be linked by environment.etc

Also the version should be version = "1.5.2-${kernel.version}"; I had renamed it during some testing and forgot to change it back.

@onny
Copy link
Contributor Author

onny commented May 4, 2023

@Slime90 Added the changes. I'm going to test it on my local setup :)

Can you tell me where you got the kernel version requirement? Do we have any older kernels here on NixOS?

@Slime90
Copy link

Slime90 commented May 4, 2023

@onny i checked the repl to see which kernel packages 22.11 currently has, and built the ebpf module backward until it failed. Everything from 5.10 and up built fine.

I’m not sure I see any new config to install the ebpf module though in the service module, I think that should be achievable, so the user doesn’t have to go do an extra step.

This should only be installed if ebpf is the chosen process monitor method in the service config

Edit: My brain is fried, is just including the src in environment.etc conditionally enough to trigger the build before the link?

@onny
Copy link
Contributor Author

onny commented May 4, 2023

If we target master after 23.05 release we might drop this kernel version condition?

@Slime90
Copy link

Slime90 commented May 4, 2023

The minimum kernel version condition is to allow maximum compatibility, and you can see in other places that this is a common practice. The system can be configured for many Linux kernels ranging from 4.x to latest, even though they are not default for the release. Users should be able to build this module for every nixos supported kernel version, in this case the build breaks below 5.10 because of missing in tree dependencies so we must set that as the minimum version

@onny onny force-pushed the opensnitch-ebpf branch 3 times, most recently from 1b6f50b to 0ee14bd Compare May 9, 2023 11:20
@onny
Copy link
Contributor Author

onny commented Aug 6, 2023

Updated PR, now only contains commits related to EPBF process monitor support

@onny onny requested review from raboof, MinerSebas, jonringer and sochotnicky and removed request for MinerSebas August 6, 2023 18:11
Copy link
Contributor

@MinerSebas MinerSebas left a comment

Choose a reason for hiding this comment

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

  1. The new system rules do not work, as no system-fw.json is present at /etc/opensnitchd/system-fw.json. Though I am unsure whether this is deemed important for the nixos use-case.
  2. The used firewall in services.opensnitch.settings.Firewall should be set according to networking.nftables.enable to either "iptables" or "nftables".

@onny onny mentioned this pull request Aug 9, 2023
12 tasks
@onny
Copy link
Contributor Author

onny commented Aug 9, 2023

  1. The new system rules do not work, as no system-fw.json is present at /etc/opensnitchd/system-fw.json. Though I am unsure whether this is deemed important for the nixos use-case.

Since the missing system-fw.json file results in errors in the daemon log, I placed the file into the desired directory. For now the file is not configurable.

2. The used firewall in `services.opensnitch.settings.Firewall` should be set according to `networking.nftables.enable` to either `"iptables"` or `"nftables"`.

I thought we already moved to nftables by default since switching to iptables-nftables-compat? #81172

@onny
Copy link
Contributor Author

onny commented Aug 11, 2023

Tests introduced with this PR ( #248011) succeed on with this PR

@onny
Copy link
Contributor Author

onny commented Aug 11, 2023

Thanks for noticing. It is not yet removed in 1.6.1 so I'll leave it as it is. I updated the PR and got a bit further, now I get this error:

[2023-08-03 14:25:21]  ERR  EBPF-DNS: Failed to attach uprobe uretprobe/gethostbyname : cannot write "r:r___nix_store_1x4ijm9r1a88qk7zcmbbfza324gx1aac_glibc_2_37_8_lib_libc_so_6_117ff0_gobpf_858 /nix/store/1x4ijm9r1a88qk7zcmbbfza324gx1aac-glibc-2.37-8/lib/libc.so.6:0x117ff0\n" to uprobe_events: write /sys/kernel/debug/tracing/uprobe_events: invalid argument
[2023-08-03 14:25:21]  WAR  EBPF-DNS: Unable to attach ebpf listener: cannot write "r:r___nix_store_1x4ijm9r1a88qk7zcmbbfza324gx1aac_glibc_2_37_8_lib_libc_so_6_117ff0_gobpf_858 /nix/store/1x4ijm9r1a88qk7zcmbbfza324gx1aac-glibc-2.37-8/lib/libc.so.6:0x117ff0\n" to uprobe_events: write /sys/kernel/debug/tracing/uprobe_events: invalid argument

Not sure how to fix this yet

Created an upstream bug report on this evilsocket/opensnitch#1013

@happysalada
Copy link
Contributor

This looks good to me, does anyone have any more objections to merging ?

@MinerSebas
Copy link
Contributor

  1. The new system rules do not work, as no system-fw.json is present at /etc/opensnitchd/system-fw.json. Though I am unsure whether this is deemed important for the nixos use-case.

Since the missing system-fw.json file results in errors in the daemon log, I placed the file into the desired directory. For now the file is not configurable.

I tested it, but I think this should reverted. Unlike the Application rules, which are each placed in separate files, the system level rules are constrained to a single File.
This means that with the readonly store symlink, you are stuck with the default settings.

Perhaps as a compromise add a copy of the file, if it doesn't exist yet at system activation?
Though that could be done in a follow up PR.

2. The used firewall in `services.opensnitch.settings.Firewall` should be set according to `networking.nftables.enable` to either `"iptables"` or `"nftables"`.

I thought we already moved to nftables by default since switching to iptables-nftables-compat? #81172

A casual glance at that PR, suggests that only the backend was switched to nftables which is used by a iptables frontend. (With the iptables-legacy package using iptables for Backend and Frontend, and the nftables using nftables for both).

Having tested with networking.nftables.enable set to true and false, opensnitch continues to work without issue.

@onny
Copy link
Contributor Author

onny commented Aug 13, 2023

@MinerSebas okay fixed it :)

@happysalada happysalada merged commit c5f4a46 into NixOS:master Aug 13, 2023
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Build EBPF module for opensnitch, at present selecting the ebpf monitor method fails.
6 participants