Discussion:
Nginx does not re-open log files on SIGUSR1.
Piotr Karbowski
2011-01-03 10:47:59 UTC
Permalink
Hi there.

I think this is bug, I saw that after logrotate (which on the end
running SIGUSR1 on master process) my access log is empty. However if I
do SIGHUP it starts working. Looks like Nginx does not re-open logfiles
after USR1.

I was able reproduce it multiple times, nginx version 0.8.53, I am not
sure if it was working in older versions.

-- Piotr.
Piotr Sikora
2011-01-03 11:38:35 UTC
Permalink
Hi Piotr,
Post by Piotr Karbowski
I think this is bug, I saw that after logrotate (which on the end
running SIGUSR1 on master process) my access log is empty. However if I
do SIGHUP it starts working. Looks like Nginx does not re-open logfiles
after USR1.
I was able reproduce it multiple times, nginx version 0.8.53, I am not
sure if it was working in older versions.
Are you sure that logrotate sends SIGUSR1?
Did you try sending this signal yourself?
How did you verify that logs did not re-open?

Everything works fine with nginx/0.8.53 on my system (OpenBSD):

$ nginx -V 2>&1 | head -1
nginx version: nginx/0.8.53

$ sudo rm -rf /var/log/nginx/*
$ sudo nginx

$ curl localhost >/dev/null 2>&1
$ ls -l /var/log/nginx/
total 8
-rw-r--r-- 1 root wheel 167 Jan 3 11:30 access.log
-rw-r--r-- 1 root wheel 775 Jan 3 11:30 error.log

$ sudo pkill -USR1 nginx

$ curl localhost >/dev/null 2>&1
$ ls -l /var/log/nginx/
total 8
-rw-r--r-- 1 _nginx wheel 334 Jan 3 11:31 access.log
-rw-r--r-- 1 _nginx wheel 1593 Jan 3 11:31 error.log

Best regards,
Piotr Sikora < piotr.sikora-***@public.gmane.org >
Gena Makhomed
2011-01-03 12:01:53 UTC
Permalink
Post by Piotr Sikora
Post by Piotr Karbowski
I think this is bug, I saw that after logrotate (which on the end
running SIGUSR1 on master process) my access log is empty. However if
I do SIGHUP it starts working. Looks like Nginx does not re-open
logfiles after USR1.
[...]
Post by Piotr Sikora
$ sudo pkill -USR1 nginx
pkill send USR1 signal to all nginx processes,
not only master, but also all worker processes
in random order - it may be first workers, and last master.

if worker processes does not have write permissions
to /var/log/nginx directory - worker processes can`t
create new log files, and can't write to log files.

USR1 signal must be sent only to nginx master process
and only as postrotate script:

# cat /etc/logrotate.d/nginx

/var/log/nginx/*log {
daily
rotate 9
missingok
notifempty
compress
sharedscripts
postrotate
[ ! -f /var/run/nginx.pid ] || kill -USR1 `cat /var/run/nginx.pid`
endscript
}

# documentation: http://sysoev.ru/nginx/docs/control.html#logs
--
Best regards,
Gena
Piotr Sikora
2011-01-03 12:18:51 UTC
Permalink
Hi,
Post by Gena Makhomed
pkill send USR1 signal to all nginx processes,
not only master, but also all worker processes
in random order - it may be first workers, and last master.
$ curl localhost >/dev/null 2>&1
$ ls -l /var/log/nginx/
total 8
-rw-r--r-- 1 root wheel 167 Jan 3 12:09 access.log
-rw-r--r-- 1 root wheel 1202 Jan 3 12:12 error.log

$ sudo pkill -USR1 -f ^nginx:.*master.*process

$ curl localhost >/dev/null 2>&1
$ ls -l /var/log/nginx/
total 12
-rw-r--r-- 1 _nginx wheel 334 Jan 3 12:13 access.log
-rw-r--r-- 1 _nginx wheel 2738 Jan 3 12:13 error.log

Happy? Result is exactly the same...
Post by Gena Makhomed
if worker processes does not have write permissions
to /var/log/nginx directory - worker processes can`t
create new log files, and can't write to log files.
Worker process doesn't create log files, master process does.

Other than complaining about non-existing issue, you didn't answer any of my
questions...

Best regards,
Piotr Sikora < piotr.sikora-***@public.gmane.org >
Gena Makhomed
2011-01-03 12:59:23 UTC
Permalink
Post by Piotr Sikora
Post by Gena Makhomed
pkill send USR1 signal to all nginx processes,
not only master, but also all worker processes
in random order - it may be first workers, and last master.
if worker processes does not have write permissions
to /var/log/nginx directory - worker processes can`t
create new log files, and can't write to log files.
Worker process doesn't create log files, master process does.
if worker processes have write permissions to /var/log/nginx
- it can create log files, see nginx sources for reference:
ngx_process_cycle.c file, ngx_worker_process_cycle function.
Post by Piotr Sikora
you didn't answer any of my questions...
you didn't ask any questions.

P.S.

nginx 0.8.53 log files rotation work fine on my machines
with logrotate config mentioned in my previous message.
--
Best regards,
Gena
Piotr Sikora
2011-01-03 13:10:31 UTC
Permalink
Hi,
Post by Gena Makhomed
you didn't ask any questions.
I did... But I've realized just now that you're not the OP ;)

Best regards,
Piotr Sikora < piotr.sikora-***@public.gmane.org >
John Feuerstein
2011-01-03 13:56:27 UTC
Permalink
Is there a reason why every script or suggestion I see floating around
the net still does it "the hard way" (and I wonder, even on _this_
mailing list):

$ kill -USR1 `cat /var/run/nginx.pid`
$ pkill -USR1 -f ^nginx:.*master.*process

instead of simply using nginx's built-in way to do this:

$ nginx -s reopen

After all, this is supported since 0.7.53.

It should also work fine if using multiple nginx instances, just pass
the path to the configuration file of the specific instance with -c.

See also http://wiki.nginx.org/CommandLine or `nginx -h`:

-s signal : send signal to a master process: stop, quit, reopen, reload
Piotr Sikora
2011-01-03 14:14:56 UTC
Permalink
Hi John,
Post by John Feuerstein
Is there a reason why every script or suggestion I see floating around
the net still does it "the hard way" (and I wonder, even on _this_
$ kill -USR1 `cat /var/run/nginx.pid`
$ pkill -USR1 -f ^nginx:.*master.*process
Yes, there is: consistency.

Pretty much every daemon supports "the standard way" for control (graceful
restarts, termination, reloads, etc), so people that use other daemons
prefer to use the method that is consistent among all of them instead of
using daemon-specific one.

Best regards,
Piotr Sikora < piotr.sikora-***@public.gmane.org >
John Feuerstein
2011-01-03 15:19:44 UTC
Permalink
Hi Piotr!
Post by Piotr Sikora
Pretty much every daemon supports "the standard way" for control
(graceful restarts, termination, reloads, etc), so people that use other
daemons prefer to use the method that is consistent among all of them
instead of using daemon-specific one.
I don't argue that the most common set of signals and their similar
handlers in various daemons (HUP=reload, TERM=clean shutdown,
QUIT=graceful shutdown...) make life a lot easier for most administrators.

But I'm not convinced that this is the case here. As soon as USR* and
whatever other custom signals are handled in a way that is special to
the specific daemon, there's not really a point to "consistency" any
more. It _is_ special already.

And IMHO this is exactly the time where a control CLI can shine,
especially since the available signals are limited and the possible
control knobs are not.

Apache has apachectl[1] or httpd's "-k" option for quite some time now.
It is used by many distributions in init scripts and log rotation
scripts instead of hardcoding signals and paths to pid files everywhere.

Nginx has the "-s" option. For me, this appears to be cleaner and
simpler than to remember or always look up what custom signal X does to
this daemon. For the most basic set of control signals I agree with you,
but it still has the downside of having to find the pid first (and
hardcoding parameters of the daemon configuration into external scripts).

[1] http://httpd.apache.org/docs/2.2/programs/apachectl.html
Piotr Sikora
2011-01-03 15:34:07 UTC
Permalink
Hi,
Post by John Feuerstein
I don't argue that the most common set of signals and their similar
handlers in various daemons (HUP=reload, TERM=clean shutdown,
QUIT=graceful shutdown...) make life a lot easier for most administrators.
But I'm not convinced that this is the case here. As soon as USR* and
whatever other custom signals are handled in a way that is special to
the specific daemon, there's not really a point to "consistency" any
more. It _is_ special already.
USR1 is pretty much "standard" for log files re-open.
Post by John Feuerstein
For the most basic set of control signals I agree with you,
but it still has the downside of having to find the pid first (and
hardcoding parameters of the daemon configuration into external scripts).
With "pkill" you don't have to ;)

Anyway, I'm not saying that sending signals is better than "nginx -s", I'm
just saying it's more consistent and that's why I'm using it.

Best regards,
Piotr Sikora < piotr.sikora-***@public.gmane.org >
Gena Makhomed
2011-01-03 16:20:36 UTC
Permalink
Post by John Feuerstein
Apache has apachectl[1] or httpd's "-k" option for quite some time now.
It is used by many distributions in init scripts and log rotation
scripts instead of hardcoding signals and paths to pid files everywhere.
apachectl is ulgy hack, which *must* be used becouse apache has

-D name : define a name for use in <IfDefine name> directives
-d directory : specify an alternate initial ServerRoot
-f file : specify an alternate ServerConfigFile
-C "directive" : process directive before reading config files
-c "directive" : process directive after reading config files
-e level : show startup errors of level (see LogLevel)
-E file : log startup errors to file

options, which can dramatically change behavior of apache instance.

and apachectl is just shell script, which only read /etc/sysconfig/httpd
options before execution -k start|restart|graceful|graceful-stop|stop

and this is the main reason why httpd -k can't be used for this -
because it didn't read options from /etc/sysconfig/httpd config file.
Post by John Feuerstein
Nginx has the "-s" option. For me, this appears to be cleaner and
simpler than to remember or always look up what custom signal X does to
this daemon. For the most basic set of control signals I agree with you,
but it still has the downside of having to find the pid first (and
hardcoding parameters of the daemon configuration into external scripts).
just use

service nginx start|stop|reload|force-reload|restart|status|configtest

for control over nginx via command line.

and nginx -s now can't be used for control over nginx instances in UNIX
by same reason as httpd -k can't be used for control over httpd instances.

adding nginxctl is useless, because "service nginx ..." already exists

and

kill -USR1 `cat /var/run/nginx.pid`

works significantly faster then

nginx -s reopen
--
Best regards,
Gena
Gena Makhomed
2011-01-03 16:02:02 UTC
Permalink
Post by John Feuerstein
Is there a reason why every script or suggestion I see floating around
the net still does it "the hard way" (and I wonder, even on _this_
$ kill -USR1 `cat /var/run/nginx.pid`
$ pkill -USR1 -f ^nginx:.*master.*process
$ nginx -s reopen
kill -USR1 `cat /var/run/nginx.pid`

works faster then nginx -s reopen

and also

kill -USR1 `cat /var/run/nginx.pid`

has explicitly defined syntax and behavior.
Post by John Feuerstein
It should also work fine if using multiple nginx instances, just pass
the path to the configuration file of the specific instance with -c.
in many cases nginx config can be very big, with thousands of includes.

nginx -s reopen feature was added for windows version of nginx, AFAIK.
--
Best regards,
Gena
Piotr Karbowski
2011-01-03 12:49:18 UTC
Permalink
Hi Piotr,
Post by Piotr Sikora
Are you sure that logrotate sends SIGUSR1?
Did you try sending this signal yourself?
How did you verify that logs did not re-open?
Yes, I checked twice logrotate's nginx rules and I tried send USR1 by
myself. To check, after rotate I did wc -l on my_vhost.access_log and it
was empty, I was hitting it with wget but until I did restart or reload,
log was empty.

I was able to 'fix' it, which is more like workaround than a real fix,
by adding permissions for nginx user to /var/log/nginx.

Before I had 700 root:root on /var/log/nginx because I am a little
paranoid and I saw no real reason to add workers there since master
process, running as root, is writting there.

After changing owner to nginx, nginx is able re-open logs after SIGUSR1.

Looks like rotated empty logs have root:root 600 perms, maybe it is the
problem? But again, I think master write there, not workers.

-- Piotr.
Piotr Sikora
2011-01-03 12:54:02 UTC
Permalink
Hi,
Post by Piotr Karbowski
Before I had 700 root:root on /var/log/nginx because I am a little
paranoid and I saw no real reason to add workers there since master
process, running as root, is writting there.
You need at least 711, otherwise workers won't be able to open files in that
directory.

Best regards,
Piotr Sikora < piotr.sikora-***@public.gmane.org >
Gena Makhomed
2011-01-03 13:16:00 UTC
Permalink
Post by Piotr Karbowski
I was able to 'fix' it, which is more like workaround than a real fix,
by adding permissions for nginx user to /var/log/nginx.
Before I had 700 root:root on /var/log/nginx because I am a little
paranoid and I saw no real reason to add workers there since master
process, running as root, is writting there.
After changing owner to nginx, nginx is able re-open logs after SIGUSR1.
master process running as root open/write files in /var/log/nginx
- if nginx user have write permissions to this directory,
700 nginx:nginx - such setup is vulnerable by symlink attack

better approach set permissions 750 root:nginx /var/log/nginx

or 750 root:www-logs /var/log/nginx and add user nginx to group www-logs
Post by Piotr Karbowski
Looks like rotated empty logs have root:root 600 perms, maybe it is the
problem?
show your logrotate config for nginx log files.
Post by Piotr Karbowski
But again, I think master write there, not workers.
nginx workers also write to log files.
--
Best regards,
Gena
Piotr Karbowski
2011-01-03 14:05:30 UTC
Permalink
Post by Gena Makhomed
master process running as root open/write files in /var/log/nginx
- if nginx user have write permissions to this directory,
700 nginx:nginx - such setup is vulnerable by symlink attack
better approach set permissions 750 root:nginx /var/log/nginx
or 750 root:www-logs /var/log/nginx and add user nginx to group www-logs
Now when you mention it, if nginx worker have read perms there (as you
suggested above), then if user symlink to any log, he will be able fetch
it via nginx which is security hole.
Post by Gena Makhomed
nginx workers also write to log files.
In what cases? And direct or somehow 'via master proicess'?
Post by Gena Makhomed
You need at least 711, otherwise workers won't be able to open
files in that directory.
So nginx' workers need exec permission on logdir? Exec on dir will allow
only chdir there, why worker have to chdir there?

The only problem is that after SIGUSR1 nginx worker *need* access to
logs (shouldn't), where on restart/reload nginx can handle it without
access to logs by workers, which as I said above, is [in my opinion]
security hole.

-- Piotr
Piotr Sikora
2011-01-03 14:09:32 UTC
Permalink
Hi,
Post by Piotr Karbowski
So nginx' workers need exec permission on logdir?
Yes.
Post by Piotr Karbowski
Exec on dir will allow only chdir there, why worker have to chdir there?
They don't, man chmod.

Best regards,
Piotr Sikora < piotr.sikora-***@public.gmane.org >
Piotr Karbowski
2011-01-03 14:16:46 UTC
Permalink
Hi,
Post by Piotr Karbowski
So nginx' workers need exec permission on logdir?
Yes.
Any reason to? Nginx works for me flawless on each box with 700
root:root on /var/log/nginx, the only problem I found is SIGUSR1,
Whatever you agree with me or not, nginx shoudn't need perms on its logs
dir, because it will allow users to use symlink to fetch logs.

-- Piotr.
Piotr Sikora
2011-01-03 14:25:42 UTC
Permalink
Hi,
Post by Piotr Karbowski
Any reason to?
Yes, user requires "+x" permission to the directory in order to be able to
open any file(s) inside it. Google/Bing/whatever for "unix permissions",
this is as simple as it gets.
Post by Piotr Karbowski
Nginx works for me flawless on each box with 700 root:root on
/var/log/nginx, the only problem I found is SIGUSR1, Whatever you agree
with me or not, nginx shoudn't need perms on its logs dir, because it will
allow users to use symlink to fetch logs.
This is because:
- on start and reload - master process opens log files before fork() and
worker processes only inherit them,
- on reopen - all processes need to open logs, so workers also need
permission to open log files.

Best regards,
Piotr Sikora < piotr.sikora-***@public.gmane.org >
Piotr Karbowski
2011-01-03 14:43:04 UTC
Permalink
Post by Piotr Sikora
Hi,
Post by Piotr Karbowski
Any reason to?
Yes, user requires "+x" permission to the directory in order to be able
to open any file(s) inside it. Google/Bing/whatever for "unix
permissions", this is as simple as it gets.
This is what I mean by 'exec will allow only chdir there'. With X you
can access dir content and depends on files rights, you can read them
etc. Mental shortcut.
Post by Piotr Sikora
Post by Piotr Karbowski
Nginx works for me flawless on each box with 700 root:root on
/var/log/nginx, the only problem I found is SIGUSR1, Whatever you
agree with me or not, nginx shoudn't need perms on its logs dir,
because it will allow users to use symlink to fetch logs.
- on start and reload - master process opens log files before fork() and
worker processes only inherit them,
- on reopen - all processes need to open logs, so workers also need
permission to open log files.
Well ok, I understand [now] why it is needed (perms that is). However
security issue still remains which in my opinion should be addressed as
bug and fixed, can you agree with me?

-- Piotr.
Piotr Sikora
2011-01-03 15:47:08 UTC
Permalink
Hi,
This is what I mean by 'exec will allow only chdir there'. With X you can
access dir content and depends on files rights, you can read them etc.
Mental shortcut.
OK, but it has nothing to do with chdir ;)
Well ok, I understand [now] why it is needed (perms that is). However
security issue still remains which in my opinion should be addressed as
bug and fixed, can you agree with me?
I would imagine that there are far more important files than nginx logs that
your workers have access to ;)

Best regards,
Piotr Sikora < piotr.sikora-***@public.gmane.org >
Gena Makhomed
2011-01-03 15:48:48 UTC
Permalink
Post by Piotr Karbowski
Post by Gena Makhomed
master process running as root open/write files in /var/log/nginx
- if nginx user have write permissions to this directory,
700 nginx:nginx - such setup is vulnerable by symlink attack
better approach set permissions 750 root:nginx /var/log/nginx
or 750 root:www-logs /var/log/nginx and add user nginx to group www-logs
Now when you mention it, if nginx worker have read perms there (as you
suggested above), then if user symlink to any log, he will be able fetch
it via nginx which is security hole.
nginx was primary developed on FreeBSD, and FreeBSD mount has option
-o nosymfollow - Do not follow symlinks on the mounted file system.

so, user vhosts on FreeBSD can be located on file system mounted
with -o nosymfollow option, and there is no security hole in this case.

but if nginx log files will have permissons nginx:root 244
in this case nginx worker processes can only write(append)
to log files, and can't read anything from it, even via symlink -
it this case 403 Forbidden will be returned to access via symlink.

but nginx source need to be patched for 244 logfile permissions,
and this is can't be done via logrotate create 0244 nginx root
directive, because nginx master process after USR1 signal
explicitly reset logfiles permissioons to S_IRUSR|S_IWUSR

other approach - set permissions to 0700 root:root
and always use slow reload configuration operation
instead of fast log files reopening via USR1 signal.
Post by Piotr Karbowski
Post by Gena Makhomed
nginx workers also write to log files.
In what cases?
in many cases, when it need write something to access/error log files.
Post by Piotr Karbowski
And direct or somehow 'via master proicess'?
direct.
Post by Piotr Karbowski
Post by Gena Makhomed
You need at least 711, otherwise workers won't be able to open
files in that directory.
So nginx' workers need exec permission on logdir?
yes.
Post by Piotr Karbowski
Exec on dir will allow only chdir there, why worker have to chdir there?
allow chdir and "access files in that directory by name".
nginx workers open log files only for append, and never for read.
Post by Piotr Karbowski
The only problem is that after SIGUSR1 nginx worker *need* access to
logs (shouldn't), where on restart/reload nginx can handle it without
access to logs by workers, which as I said above, is [in my opinion]
security hole.
Igor say in russian mail list:

http://nginx.org/pipermail/nginx-ru/2010-December/038658.html

in my translation:

"open log files by the master process and transmit descriptors
to workers, not changing the permissions, - it's for nginx 2.0"
--
Best regards,
Gena
Piotr Sikora
2011-01-03 17:03:53 UTC
Permalink
Hi,
Post by Gena Makhomed
http://nginx.org/pipermail/nginx-ru/2010-December/038658.html
"open log files by the master process and transmit descriptors
to workers, not changing the permissions, - it's for nginx 2.0"
Yes, this is probably optimal solution.

However, I wonder why Igor scheduled it for nginx-2.0?
This doesn't break anything and it could ship with nginx-0.9.4.

Best regards,
Piotr Sikora < piotr.sikora-***@public.gmane.org >
Gena Makhomed
2011-01-03 17:43:56 UTC
Permalink
Post by Piotr Sikora
Post by Gena Makhomed
http://nginx.org/pipermail/nginx-ru/2010-December/038658.html
"open log files by the master process and transmit descriptors
to workers, not changing the permissions, - it's for nginx 2.0"
Yes, this is probably optimal solution.
However, I wonder why Igor scheduled it for nginx-2.0?
This doesn't break anything and it could ship with nginx-0.9.4.
patch - ?
--
Best regards,
Gena
Piotr Sikora
2011-01-03 18:06:11 UTC
Permalink
Hi,
Post by Gena Makhomed
patch - ?
No, because creating one is pointless. This mailing list is full of patches
(mostly from Maxim) that solve real, recurring issues and that are obviously
correct, but for some reason they are not imported to the nginx and Igor
doesn't provide any feedback why.

Best regards,
Piotr Sikora < piotr.sikora-***@public.gmane.org >
Piotr Karbowski
2011-01-03 20:37:32 UTC
Permalink
Post by Piotr Sikora
Hi,
Post by Gena Makhomed
patch - ?
No, because creating one is pointless. This mailing list is full of
patches (mostly from Maxim) that solve real, recurring issues and that
are obviously correct, but for some reason they are not imported to the
nginx and Igor doesn't provide any feedback why.
In that case - is there any fork of nginx with patches what you are
talking about?

-- Piotr.
Piotr Sikora
2011-01-04 09:19:20 UTC
Permalink
Hi,
Post by Piotr Karbowski
In that case - is there any fork of nginx with patches what you are
talking about?
I would imagine that Maxim might have one, but I don't think it's publicly
available. There are also two forks that I'm aware of, that include at least
some of the patches from the mailing list, but again, neither is publicly
available (at least not yet).

So for now, your best option is to check archives for mails starting with
"[PATCH" and apply them yourself ;)

Best regards,
Piotr Sikora < piotr.sikora-***@public.gmane.org >
Gena Makhomed
2011-01-03 17:42:43 UTC
Permalink
Post by Gena Makhomed
Post by Piotr Karbowski
Post by Gena Makhomed
master process running as root open/write files in /var/log/nginx
- if nginx user have write permissions to this directory,
700 nginx:nginx - such setup is vulnerable by symlink attack
better approach set permissions 750 root:nginx /var/log/nginx
or 750 root:www-logs /var/log/nginx and add user nginx to group www-logs
Now when you mention it, if nginx worker have read perms there (as you
suggested above), then if user symlink to any log, he will be able fetch
it via nginx which is security hole.
[...]
Post by Gena Makhomed
if nginx log files will have permissons nginx:root 244
in this case nginx worker processes can only write(append)
to log files, and can't read anything from it, even via symlink -
it this case 403 Forbidden will be returned to access via symlink.
but nginx source need to be patched for 244 logfile permissions,
and this is can't be done via logrotate create 0244 nginx root
directive, because nginx master process after USR1 signal
explicitly reset logfiles permissioons to S_IRUSR|S_IWUSR
patches:
tested with 0.8.53, 0.8.54 and 0.9.3.

nginx-logfiles-permissions-1.patch:
reset log files owner (nginx) permissions to S_IWUSR only.

nginx-logfiles-permissions-2.patch:
first change log file permissions, thereafter change owner.
--
Best regards,
Gena
Loading...