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

nginx.service specs differ from apt-get install #71

Open
fabianegli opened this issue Feb 27, 2019 · 10 comments
Open

nginx.service specs differ from apt-get install #71

fabianegli opened this issue Feb 27, 2019 · 10 comments

Comments

@fabianegli
Copy link

While preparing to compile nginx from source with this build script, I realized that there are some differences in the nginx.service file produced by it and the one I have from an installation with apt-get.

Since I am not quite familiar with these kind of files and the instruction they convey, I thought I ask. What is the rationale behind these differences?

From apt-get:

# Stop dance for nginx
# =======================
#
# ExecStop sends SIGSTOP (graceful stop) to the nginx process.
# If, after 5s (--retry QUIT/5) nginx is still running, systemd takes control
# and sends SIGTERM (fast shutdown) to the main process.
# After another 5s (TimeoutStopSec=5), and if nginx is alive, systemd sends
# SIGKILL to all the remaining processes in the process group (KillMode=mixed).
#
# nginx signals reference doc:
# http://nginx.org/en/docs/control.html
#
[Unit]
Description=A high performance web server and a reverse proxy server
Documentation=man:nginx(8)
After=network.target

[Service]
Type=forking
PIDFile=/run/nginx.pid
ExecStartPre=/usr/sbin/nginx -t -q -g 'daemon on; master_process on;'
ExecStart=/usr/sbin/nginx -g 'daemon on; master_process on;'
ExecReload=/usr/sbin/nginx -g 'daemon on; master_process on;' -s reload
ExecStop=-/sbin/start-stop-daemon --quiet --stop --retry QUIT/5 --pidfile /run/nginx.pid
TimeoutStopSec=5
KillMode=mixed

[Install]
WantedBy=multi-user.target

From nginx-build:

[Unit]
Description=The NGINX HTTP and reverse proxy server
After=syslog.target network.target remote-fs.target nss-lookup.target

[Service]
Type=forking
PIDFile=/var/run/nginx.pid
ExecStartPre=/usr/sbin/nginx -t
ExecStart=/usr/sbin/nginx
ExecReload=/bin/kill -s HUP $MAINPID
ExecStop=/bin/kill -s QUIT $MAINPID
PrivateTmp=true

[Install]
WantedBy=multi-user.target

I guess that the -g part is created/modified by systemctl, but am not sure at all. I am especially wary about the ExecReload and ExecStop, since they seem to be radically different.

@MatthewVance
Copy link
Owner

MatthewVance commented Mar 1, 2019 via email

@MatthewVance
Copy link
Owner

MatthewVance commented Mar 1, 2019 via email

@fabianegli
Copy link
Author

Thank you for this response and the additional resources on the systemd. An interesting read, indeed!

However, my immediate question remains: Why do you use the kill command instead of the nginx -s reload in the ExecReload directive?

The use of kill in the systemd for ExecReload causes an unexpected behavior for nginx users. See: https://stackoverflow.com/a/13527535/6018688 (reload only if config checks out) and https://serverfault.com/a/378585/511536 (graceful hot replacement of nginx instance).

@fabianegli
Copy link
Author

fabianegli commented Mar 1, 2019

I was mistaken and my worries were of no substance. Sending signals to nginx seems to be one of the ways to trigger a reconfiguration: http://nginx.org/en/docs/control.html#reconfiguration (sig HUP for reload and sig QUIT for a graceful shutdown). I am sorry for the confusion I caused. This issue may be closed.

@MatthewVance
Copy link
Owner

MatthewVance commented Mar 1, 2019 via email

@fabianegli
Copy link
Author

I think it would be nice to update the reload part in the next iteration of this build script.

The use of the reload flag instead of the HUP signal is more descriptive and my guess is, that more people understand the reload syntax than the HUP (Just because it is used in all tutorials I came across and from the documentation page I had to click through two pages explaining reload and the other named signals before arriving at the command line description linked above that introduces HUP.) - even if it results in the same behavior.

@MatthewVance
Copy link
Owner

Good point.

I'm tempted to revise it further to be closer to this Caddy service file. However, I'm concerned the extra security hardening would make it less general purpose.

@fabianegli
Copy link
Author

There are many options how to include that additional security information in the script. As you already mentioned, changing some of these parameters probably restrict some use cases out there. I see multiple ways to add this:

  1. Add all the security possible: for (quite) sure not user friendly.
  2. Add a link to the resource: probably most will ignore it.
  3. Add an additional complete version of the service file that adds all the security and allow to switch between them.
  4. Add all the security but comment it out by default and tell users to uncomment what they need (explicitly mention the comment character - it is not one that I see a lot in other places...).

I believe the last is the most streamlined and I favor it over the others. I think 2. in general is not a bad idea for people interested in further reading.

@MatthewVance
Copy link
Owner

Thanks for this. These are all valid options. I wanted to briefly write here to let you know I have not forgotten about this—I just have not made time to properly research and implement.

I am leaning towards number four. Number three is appealing also, but I am hesitant about it due to the its complexity.

@jellemdekker
Copy link
Contributor

I am leaning towards number four. Number three is appealing also, but I am hesitant about it due to the its complexity.

This would be my reasoning too. Normally, I lean more towards making something secure by default and telling users when and how to lower that in case of problems, but in the Caddy service file you linked to I already noticed two options that can affect a lot of people negatively because they depend on the hardware, OS or user's preference: line 34 and 41.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants