Nginx does not re-open log files on SIGUSR1

On 03.01.2011 16:05, Piotr K. wrote:

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.

nginx workers also write to log files.

In what cases?

in many cases, when it need write something to access/error log files.

And direct or somehow ‘via master proicess’?

direct.

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.

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.

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

On 03.01.2011 17:19, John Feuerstein wrote:

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
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.

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

Hi,

/var/log/nginx

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”

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 S. < [email protected] >

On 03.01.2011 19:03, Piotr S. wrote:

This doesn’t break anything and it could ship with nginx-0.9.4.
patch - ?


Best regards,
Gena

Hi,

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 S. < [email protected] >

On 03.01.2011 17:48, Gena M. wrote:

On 03.01.2011 16:05, Piotr K. wrote:

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.

[…]

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.

Hi,

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 :wink:

Best regards,
Piotr S. < [email protected] >

On 01/03/2011 07:06 PM, Piotr S. wrote:

Hi,

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.