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.

Sign up for GitHub

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

Jump to bottom

nixos/nzbget: cfg.configFile should be owned by cfg.user/cfg.group #58916

Closed
wants to merge 2 commits into from
Closed

nixos/nzbget: cfg.configFile should be owned by cfg.user/cfg.group #58916

wants to merge 2 commits into from

Conversation

cmacrae
Copy link
Contributor

@cmacrae cmacrae commented Apr 3, 2019 •
edited

Motivation for this change

These changes ensure, via the preStart script, that the configFile and dataDir
are owned by the user & group that nzbget has been configured to run with.
Without this, when the config template is copied over to facilitate
initial startup, it's done so with root permissions. This causes the
service to fail during the startup script (grep is used to inspect the
file for some values). The dataDir is similar circumstances; without these
changes, it's created with root permissions, which means the service cannot
read or write data there, if it's running as a different user.

Example of failure scenario

Mar 31 21:50:25 slim systemd[1]: Starting NZBGet Daemon...
Mar 31 21:50:25 slim 2f3n6zr1r5q4xplvnd6zizpknlhcha14-unit-script-nzbget-pre-start[2615]: /var/lib/nzbget/nzbget.conf not found. Copying default config /nix/store/q8q5af4cxypc75dhp223jl89amfhbn00-nzbget-20.0/s>
Mar 31 21:50:25 slim 2f3n6zr1r5q4xplvnd6zizpknlhcha14-unit-script-nzbget-pre-start[2615]: Setting temporary $MAINDIR variable in default config required in order to allow nzbget to complete initial start
Mar 31 21:50:25 slim 2f3n6zr1r5q4xplvnd6zizpknlhcha14-unit-script-nzbget-pre-start[2615]: Remember to change this to a proper value once NZBGet startup has been completed
Mar 31 21:50:25 slim 2xgi9aacik9f2q5wyyqa1vswd6izy9vg-unit-script-nzbget-start[2618]: grep: /var/lib/nzbget/nzbget.conf: Permission denied
Mar 31 21:50:25 slim 2xgi9aacik9f2q5wyyqa1vswd6izy9vg-unit-script-nzbget-start[2618]: In /var/lib/nzbget/nzbget.conf, valid ConfigTemplate not found; falling back to ConfigTemplate=/nix/store/q8q5af4cxypc75dhp>
Mar 31 21:50:25 slim 2xgi9aacik9f2q5wyyqa1vswd6izy9vg-unit-script-nzbget-start[2618]: grep: /var/lib/nzbget/nzbget.conf: Permission denied
Mar 31 21:50:25 slim 2xgi9aacik9f2q5wyyqa1vswd6izy9vg-unit-script-nzbget-start[2618]: In /var/lib/nzbget/nzbget.conf, valid WebDir not found; falling back to WebDir=/nix/store/q8q5af4cxypc75dhp223jl89amfhbn00->
Mar 31 21:50:25 slim 2xgi9aacik9f2q5wyyqa1vswd6izy9vg-unit-script-nzbget-start[2618]: nzbget.conf(0): Could not open file /var/lib/nzbget/nzbget.conf
Mar 31 21:50:25 slim systemd[1]: nzbget.service: Control process exited, code=exited status=1
Mar 31 21:50:25 slim systemd[1]: nzbget.service: Failed with result 'exit-code'.
Mar 31 21:50:25 slim systemd[1]: Failed to start NZBGet Daemon.

Things done

Ensure the configFile and dataDir are owned by the user & group the service has been configured to run with.

  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
  • 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 nix-review --run "nix-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)
  • Assured whether relevant documentation is up to date
  • Fits CONTRIBUTING.md.

nixos/nzbget: cfg.configFile should be owned by cfg.user/cfg.group
9b0d701 
These changes ensure, via the preStart script, that the configFile is
owned by the user & group that nzbget has been configured to run with.
Without this, when the config template is copied over to facilitate
initial startup, it's done so with root permissions. This causes the
service to fail during the startup script (grep is used to inspect the
file for some values).
@cmacrae cmacrae requested a review from infinisil as a code owner April 3, 2019 09:34
@cmacrae
Copy link
Contributor Author

cmacrae commented Apr 3, 2019

Hmm, further tests also reveal that the dataDir needs to be owned by the user/group as well.
I'd worked around this by setting that user's home to match.

I'll look at implementing a fix as part of this PR, as they're both closely coupled.

nixos/nzbget: cfg.dataDir should be owned by cfg.user/cfg.group
7649f2a 
These changes ensure, via the preStart script, that the dataDir is
present and owned by the user & group that nzbget has been configured
to run with.

Without this, the service fails during the startup script and data is
unable to be written to the directory, if the directory isn't owned by
the user under which the service is running.
@cmacrae
Copy link
Contributor Author

cmacrae commented Apr 3, 2019

UPDATE Also added fix for dataDir ownership. I'll update the PR description.
Let me know if these should be squashed into 1 commit, and I'll happily do so.

@cmacrae
Copy link
Contributor Author

cmacrae commented Apr 3, 2019

Aaaand I've just realised my branch is named incorrectly... whoops!
Going to close this and re-open from a new branch to ensure there's no confusion!

@cmacrae cmacrae closed this Apr 3, 2019
@cmacrae cmacrae deleted the nixos-unstable/sabnzbd_user_enhancement branch April 3, 2019 11:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@infinisil infinisil Awaiting requested review from infinisil

Assignees
No one assigned
Labels
6.topic: nixos 8.has: module (update) 10.rebuild-darwin: 0 10.rebuild-linux: 1-10
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@cmacrae @GrahamcOfBorg