perusio / drupal-with-nginx

Running Drupal using nginx: an idiosyncratically crafted bleeding edge configuration.
854 stars 246 forks source link

Drupal Security update breaks nginx config #241

Closed tdm4 closed 6 years ago

tdm4 commented 8 years ago

Drupal links: https://www.drupal.org/node/2671940#comment-10911622 https://www.drupal.org/SA-CORE-2015-003 https://www.drupal.org/node/2599326#comment-10828976

Seems to break clean URLs somehow. The only way to fix is to add this in drupal.conf above the catchall 404 at the bottom:

    location = /index.php {
        include conf/fastcgi_drupal.conf;
        fastcgi_pass phpcgi;
    }

For some reason it tries to access index.php directly and the nginx config prevents this.

HMoen commented 8 years ago

This worked for me... Thanks!

It's a bit troubling though because of the first directive here: https://github.com/perusio/drupal-with-nginx#security-features

timwood commented 8 years ago

I used the recommendation by @omega8cc here: https://www.drupal.org/node/2599326#comment-10850594

###
### Rewrite legacy requests with /index.php to extension-free URL.
### Workaround for https://www.drupal.org/node/2599326.
###
if ( $args ~* "^q=(?<query_value>.*)" ) {
  rewrite ^/index.php$ $scheme://$host/?q=$query_value? permanent;
}
longwave commented 8 years ago

This is a duplicate of #235, but this has more useful info.

This change fixed it for me:

location = /index.php {
    include fastcgi.conf;
    fastcgi_pass phpcgi;
}

Including fastcgi_drupal.conf instead did some strange redirects for /index.php alone - probably because fastcgi_drupal tries to rewrite to /index.php?q=$uri which would end up as /index.php?q=/index.php

accuraz commented 8 years ago

@longwave I'm the reporter of #235. I'm not sure giving direct access to index.php is a secure way.

I think @iryston and @okolobaxa approaches proposed in #235 are more secure.

Best,

longwave commented 8 years ago

@accuraz Why is giving direct access to index.php a security issue?

accuraz commented 8 years ago

@longwave A part of the strategy of @perusio's config is to not giving direct access to index.php witch provides better security. By giving access to index.php you'll go right into the other direction. Go trough the NGINX documentation and you'll find out why NGINX is design to prevent direct access to index.php and why rewrites are preferred.

If you do that, in one shoot you'll get your information and far more about NGINX conception.

Best

omega8cc commented 8 years ago

I have commented on blocking access to /index.php here:

https://www.drupal.org/node/2599326#comment-10881228 https://www.drupal.org/node/2599326#comment-10888280

Feel free to disagree, but please provide reasoning and not some RTFM talk.

tdm4 commented 8 years ago

As a sysadmin of quite a few drupal sites, my fix at the top works fine, even including the fastcgi_drupal.conf. Redirects work just fine.

longwave commented 8 years ago

@tdm4 Your fix didn't actually work at all for me as-is - I have apps/drupal/fastcgi_drupal.conf instead of conf/fastcgi_drupal.conf. If I include apps/drupal/fastcgi_drupal.conf and visit plain /index.php with no querystring at all, I get a 404 generated by Drupal. Pages with the ?q= querystring work as expected.

tdm4 commented 8 years ago

@longwave I don't have QUERY_STRING set in fastcgi_drupal.conf anymore. It breaks taxonomies that have escaped web characters in them (like '&')

accuraz commented 8 years ago

@omega8cc of course its security by obscurity. The debate about security by obscurity is alway viral. It's real security or not ? I think it may help as a complement. I read NGINX documentation and it's by reading it that I find out the purpose of internal redirection. After that I realize why you went this way with the @perusio config. @longwave has asked for an answer so I recommended him to go trough NGINX documentation so maybel I'll see and realize the same way than me. @omega8cc you provide with more information with is all at you honor.

emesch commented 8 years ago

For me @omega8cc's solution causes index.php to be downloaded if the uri "domain.com/index.php" is visited. To stop that from happening, I added a return 404 as follows:

location ^~ /index.php {
    if ( $args ~* "^q=(?<query_value>.*)" ) {
        rewrite ^/index.php$ $scheme://$host/?q=$query_value? last;
    }
    return 404;
}

I like this approach as it redirects to the @drupal location which may contain special directives that I'd prefer not to repeat in another location block.

It would be great to get a solution into the repo since it doesn't look like this change to core is going to be reversed.

tdm4 commented 7 years ago

Hi,

@emesch 's solution works but I found due to SSL offloading, it's not a good idea to hardcode the $scheme and $host in there. So I rewrote it to the following:

location ^~ /index.php {
    if ( $args ~* "^q=(?<query_value>.*)" ) {
        rewrite ^/index.php$ /?q=$query_value? last;
    }
    return 404;
}
emphazer commented 7 years ago

@tdm4 i did it like that and had never problems.

location ~* ^.+\.php$ {
     if ($request_uri ~ "^/index.php$")
     {
        return 301 $scheme://$host;
     }
     try_files index.php @drupal-no-args;
}
tdm4 commented 7 years ago

@emphazer doesn't work well when you have SSL Offloading and the only protocol nginx knows about is 'http' because $scheme will always be 'http'.

Also is it better to return a 404 like direct access to index.php does or, do we fallback to the try_files?

The return 404 will give an ugly white nginx screen where as the try_files might make it fall back to something handled by drupal.

@perusio your thoughts?

tdm4 commented 7 years ago

I think the direct access to /index.php breaks Drupal 8 now too. I think setting fastcgi_param QUERY_STRING $query_string; in the fastcgi_params (and NOT using ?q=$request_uri) is the way forward as this breaks Drupal 8 in horrible ways.

And what's more, Drupal 7 may not even need the QUERY_STRING rewritten with the ?q= stuff at all. I think that is mainly a Drupal 6 thing.

tdm4 commented 7 years ago

OK I've done more testing.

Drupal 7 NEEDS direct access to index.php (it appears too break the ajax in the taxonomy autocomplete) Drupal 8 seems to not need it.

In both Drupal 7 and Drupal 8, setting fastcgi_param QUERY_STRING $query_string; makes the sites continue to function normally. Drupal 8 - if you set fastcgi_param QUERY_STRING q=$uri&$args; it horrendously breaks the site. We've seen behavior where it appends tons os /?q=/ to the end of the site and it breaks loads of things.

Using this for Drupal 7 fixes the ajax errors in this module: https://www.drupal.org/project/views_autocomplete_filters

location = /index.php {
     ## Include the FastCGI config.
    include apps/drupal/fastcgi_drupal.conf;
    fastcgi_pass phpcgi;

    ## FastCGI microcache.
    include apps/drupal/microcache_fcgi.conf;
    ## FCGI microcache for authenticated users also.
    #include apps/drupal/microcache_fcgi_auth.conf;

    ## If proxying to apache comment the two lines above and
    ## uncomment the two lines below.
    #proxy_pass http://phpapache/index.php?q=$uri;
    #proxy_set_header Connection '';

    ## Proxy microcache.
    #include apps/drupal/microcache_proxy.conf;
    ## Proxy microcache for authenticated users also.
    #include apps/drupal/microcache_proxy_auth.conf;

    ## Filefield Upload progress
    ## http://drupal.org/project/filefield_nginx_progress support
    ## through the NginxUploadProgress modules.
    track_uploads uploads 60s;
}
tdm4 commented 6 years ago

I think this project's been abandoned. No commits in nearly 2 years, issues piling up and no responses. I'm going to close this issue out as I don't think anything's going to happen with it.