Improved nginx config #452

Merged
merged 1 commit into from Jan 8, 2013

3 participants

@tenebrousedge

try_files is preferred over if()
the =404 prevents some attacks
fastcgi_params and port 9000 are more common defaults

@markstory
CakePHP member

This looks good to me but @lorenzo knows nginx better than I do.

@lorenzo lorenzo commented on the diff Dec 30, 2012
en/installation/advanced-installation.rst
try_files $uri $uri/ /index.php?$uri&$args;
}
location ~ \.php$ {
- include /etc/nginx/fcgi.conf;
- fastcgi_pass 127.0.0.1:10005;
+ try_files $uri =404;
@lorenzo
CakePHP member
lorenzo added a line comment Dec 30, 2012

Why add a try_files here?

@lorenzo
CakePHP member
lorenzo added a line comment Dec 30, 2012

I get it now, CakePHP installations are not vulnerable to that kind of attack, though. But adding it should not hurt, unless running PHP on a separate server.

Anyone has objections on adding it despite it possibly breaks something?

@AD7six
CakePHP member
AD7six added a line comment Dec 31, 2012

I think this adds some confusion and possibly will cause requests with an extension to not reach the application

@tenebrousedge
tenebrousedge added a line comment Jan 1, 2013

It does not cause any issues with extensions as far as I have been able to determine. Also, it seems unlikely that this would be a security issue unless you're doing really dumb things with uploaded file directories. So, it will probably not help that much and it will probably not hurt that much. If there's a consensus against including it I'll fix the commit.

@AD7six
CakePHP member
AD7six added a line comment Jan 1, 2013

Let's try that again, with a bit less new-year-cheer floating around, I misread it the first time.

So: request for a php file, if the request uri doesn't exist throw a 404. Thus preventing /foo.gif/doesnt-exist.php executing the gif file as php.

I think it's fine to add that but it needs explaining - it's going to e.g. break some wordpress plugins if they are on the same server.

incidentally, the location / block has an odd (pre-existing) try_files statement, should be:

location / {
    try_files $uri $uri/ /index.php;
}

can you make that change also please?

@tenebrousedge
tenebrousedge added a line comment Jan 1, 2013

If you don't include $args then query strings will not be properly parsed (CakeRequest::$query will always be empty), although the $uri seems to be unnecessary. It does work as written however.

On the =404 thing, after checking my error logs, it seems that Cake makes no especial provisions for where PHP code can be executed from, but there may be restrictions on what file extensions fastcgi will process.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@markstory
CakePHP member

Any resolution on this?

@lorenzo
CakePHP member

I'm going to merge this and then remove the parts that are not relevant to a cake install

@lorenzo lorenzo merged commit eb05b5f into cakephp:master Jan 8, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment