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 sandbox-netdata branch 2 times, most recently from 44ffdc8 to 93bff48 Compare May 15, 2020 19:19
@Izorkin
Copy link
Contributor Author

Izorkin commented May 15, 2020

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

@Izorkin
Copy link
Contributor Author

Izorkin commented May 22, 2020

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

@Mic92
Copy link
Member

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

@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 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 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
Member

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 commented May 31, 2020

Created PR.

@Mic92
Copy link
Member

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
Member

Mic92 commented Aug 2, 2020

@Mic92
Copy link
Member

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
Copy link
Contributor Author

Izorkin commented Aug 9, 2020

Updated.

@Izorkin Izorkin requested a review from Mic92 August 9, 2020 07:20
@Mic92 Mic92 merged commit e0e107b into NixOS:master Aug 9, 2020
@Izorkin Izorkin deleted the sandbox-netdata branch August 9, 2020 12:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants