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/netadata: enable simple sandboxing #87867

Merged
merged 1 commit into from Aug 9, 2020
Merged

Conversation

@Izorkin
Copy link
Contributor

@Izorkin Izorkin commented May 15, 2020

Motivation for this change

Running netdata service in simple sandbox mode.

сс @Mic92

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/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.
@Izorkin Izorkin force-pushed the Izorkin:sandbox-netdata branch 2 times, most recently from 44ffdc8 to 93bff48 May 15, 2020
@Izorkin
Copy link
Contributor Author

@Izorkin Izorkin commented May 15, 2020

Changed ProtectHome to "read-only";. Mounted folders in /home were not viewed.

@Izorkin Izorkin force-pushed the Izorkin:sandbox-netdata branch from 93bff48 to c247fc7 May 22, 2020
@Izorkin
Copy link
Contributor Author

@Izorkin Izorkin commented May 22, 2020

Added "CAP_FOWNER" capabilities, need to correct work freeipmi plugun.

@Mic92
Copy link
Contributor

@Mic92 Mic92 commented May 22, 2020

@joelhans Is this something upstream would be interested in? If those sandboxing options would be bundled with netdata itself every systemd-based distributions would adopt it.

@joelhans
Copy link

@joelhans joelhans commented May 22, 2020

@Mic92 This definitely could be of interest! I'll send this on to our packaging team so they can investigate what the work you've done here. Hopefully you'll hear from them soon, or from me if they have an update I can pass along. Thanks for the ping.

@Ferroin
Copy link

@Ferroin Ferroin commented May 22, 2020

@Mic92 I can't speak for our whole packaging team, but I suspect the answer is likely to be yes, we would be interested in this. I'll make a point to bring it up with the rest of the team during our daily sync and hopefully have a more conclusive answer for you some time next week.

@Ferroin
Copy link

@Ferroin Ferroin commented May 26, 2020

@Mic92 Based on discussion with the rest of the packaging and SRE team, we are potentially interested having this sandboxing upstream, but don't currently have the time to add it ourselves. If you (or one of the other NixOS contributors) want to open a PR to add it (ideally with info about minimum required version of systemd and other such things), we'll be happy to review it and will probably merge it unless we determine that it would be too much effort to maintain on our end.

@Mic92
Copy link
Contributor

@Mic92 Mic92 commented May 26, 2020

@Izorkin Would you be interested in upstreaming this? They have merchandise: netdata/netdata#7133

@Izorkin
Copy link
Contributor Author

@Izorkin Izorkin commented May 31, 2020

Created PR.

@Mic92
Copy link
Contributor

@Mic92 Mic92 commented Jun 10, 2020

I would like to get at least one review round on netdata/netdata#9234 before merging this.

@Mic92
Copy link
Contributor

@Mic92 Mic92 commented Aug 2, 2020

@Mic92
Copy link
Contributor

@Mic92 Mic92 commented Aug 2, 2020

In the next release we should just switch to the upstream systemd file to receive sandbox fixes in future.

@Izorkin Izorkin force-pushed the Izorkin:sandbox-netdata branch from c247fc7 to 2f6a18a Aug 9, 2020
@Izorkin
Copy link
Contributor Author

@Izorkin Izorkin commented Aug 9, 2020

Updated.

@Izorkin Izorkin requested a review from Mic92 Aug 9, 2020
@Mic92 Mic92 merged commit e0e107b into NixOS:master Aug 9, 2020
15 checks passed
15 checks passed
tests tests
Details
tests
Details
Evaluation Performance Report Evaluator Performance Report
Details
grahamcofborg-eval ^.^!
Details
grahamcofborg-eval-check-meta config.nix: checkMeta = true
Details
grahamcofborg-eval-darwin nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="2f6a18a"; rev="2f6a18af5a76894c172298e7f1457fb932f7f1b7"; } ./pkgs/t
Details
grahamcofborg-eval-lib-tests nix-build --arg pkgs import ./. {} ./lib/tests/release.nix
Details
grahamcofborg-eval-nixos nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="2f6a18a"; rev="2f6a18af5a76894c172298e7f1457fb932f7f1b7"; } ./nixos/
Details
grahamcofborg-eval-nixos-manual nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="2f6a18a"; rev="2f6a18af5a76894c172298e7f1457fb932f7f1b7"; } ./nixos/
Details
grahamcofborg-eval-nixos-options nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="2f6a18a"; rev="2f6a18af5a76894c172298e7f1457fb932f7f1b7"; } ./nixos/
Details
grahamcofborg-eval-nixpkgs-manual nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="2f6a18a"; rev="2f6a18af5a76894c172298e7f1457fb932f7f1b7"; } ./pkgs/t
Details
grahamcofborg-eval-nixpkgs-tarball nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="2f6a18a"; rev="2f6a18af5a76894c172298e7f1457fb932f7f1b7"; } ./pkgs/t
Details
grahamcofborg-eval-nixpkgs-unstable-jobset nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="2f6a18a"; rev="2f6a18af5a76894c172298e7f1457fb932f7f1b7"; } ./pkgs/t
Details
grahamcofborg-eval-package-list nix-env -qa --json --file .
Details
grahamcofborg-eval-package-list-no-aliases nix-env -qa --json --file . --arg config { allowAliases = false; }
Details
@Izorkin Izorkin deleted the Izorkin:sandbox-netdata branch Aug 9, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants
You can’t perform that action at this time.