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/fossil-server: init #102221

Open
wants to merge 12 commits into
base: master
Choose a base branch
from
Open

nixos/fossil-server: init #102221

wants to merge 12 commits into from

Conversation

@Uthar
Copy link
Contributor

@Uthar Uthar commented Oct 31, 2020

Motivation for this change

Fix #102220

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.
Copy link
Member

@aanderse aanderse left a comment

Thanks for your contribution. I have left a few comments I hope you find helpful. Additionally, it would be nice if the systemd unit had some hardening options applied against it.

If you have any questions at all, please do not hesitate to ask. I'm happy to help.

config = mkIf cfg.enable {

networking.firewall.allowedTCPPorts = [ cfg.port ];
systemd.services.fossil = {
Copy link
Member

@aanderse aanderse Oct 31, 2020

There isn't any real need to run this as root. Maybe DynamicUser would be a good choice instead.

Copy link
Contributor Author

@Uthar Uthar Oct 31, 2020

Added the user and group options. I'm not sure if it's the best way, what do you think? The thing is that with customizable user/group, it's easy to manage permissions just by using chmod/chown and make fossil see just what it needs to see. Also I'm not sure if DynamicUser would persist between reboots which is desirable.


networking.firewall.allowedTCPPorts = [ cfg.port ];
systemd.services.fossil = {
preStart = if lib.strings.hasSuffix ".fossil" cfg.repository
Copy link
Member

@aanderse aanderse Oct 31, 2020

You should replace this directory creation with systemd.tmpfiles.rules entries instead.

Copy link
Contributor Author

@Uthar Uthar Oct 31, 2020

You made me rethink this, and IMHO the place where repository points to should be created manually. Then, one can create/remove .fossil files as needed, and manage permissions etc.

nixos/modules/services/development/fossil-server.nix Outdated Show resolved Hide resolved
nixos/modules/services/development/fossil-server.nix Outdated Show resolved Hide resolved

jsmode = mkOption {
type = types.str;
default = "";
Copy link
Member

@aanderse aanderse Oct 31, 2020

types.enum is what you're looking for here.

Copy link
Contributor Author

@Uthar Uthar Oct 31, 2020

That's much cleaner, thanks - fixed

@Uthar Uthar requested a review from aanderse Oct 31, 2020
PrivateTmp = "yes";
NoNewPrivileges = true;
ProtectSystem = "strict";
CapabilityBoundingSet = "CAP_NET_BIND_SERVICE CAP_DAC_READ_SEARCH";
RestrictNamespaces = "uts ipc pid user cgroup";
ProtectKernelTunables = "yes";
ProtectKernelModules = "yes";
ProtectControlGroups = "yes";
PrivateDevices = "yes";
RestrictSUIDSGID = true;
Copy link
Contributor Author

@Uthar Uthar Oct 31, 2020

Added some options stolen from the nginx and mysql NixOS modules

Is there something to change?

@aanderse
Copy link
Member

@aanderse aanderse commented Nov 12, 2020

Sorry I've been really busy. I'll try to get back to this at some point over the next few weeks.

@stale
Copy link

@stale stale bot commented Jun 4, 2021

I marked this as stale due to inactivity. → More info

@stale stale bot added the 2.status: stale label Jun 4, 2021
Uthar added 2 commits Jul 8, 2021
Otherwise Fossil can't write to the repository file/repositories directory
A test for clone, push and pull between server and client
@Uthar
Copy link
Contributor Author

@Uthar Uthar commented Jul 8, 2021

Updated for 21.05 and added a test - please review! @aanderse @SuperSandro2000

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.

2 participants