-
-
Notifications
You must be signed in to change notification settings - Fork 5.2k
Improved nginx config to not expose other php files #6008
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
Conversation
I'm a bit -1 on denying access to all other files. Many people just copy/past this code example without even reading it. Having some custom php file that doesn't work anymore will result in questions. I think we should explicitely match config.php and app_dev.php. |
the web directory. All other files will be denied. You **must** also make | ||
sure that if you *do* deploy ``app_dev.php`` or ``config.php`` that these | ||
files are secured and not available to any outside user (the IP address | ||
checking code at the top of each file does this by default). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this last sentence no longer makes sense, if you do deploy app_dev.php or config.php, a 404 is returned so you don't have to secure it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Still the user should be aware of it... If he removes the config above he will get the issue again.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, perhaps we should just talk about the end result to avoid.
- Move this out of a
tip
block - just make it an explanation - Turn the following into a
caution
block.
.. caution::
After you deploy to production, make sure that you **cannot** access the ``app_dev.php``
or ``config.php`` scripts (i.e. ``http://example.com/app_dev.php`` and ``http://example.com/config.php``).
If you *can* access these, be sure to remove the ``DEV`` section from the above configuration.
The thing is matching app_* and config does not make sense for dev environments and could cause confusion and troubles. Especially if people copy and paste the chances are higher that they remove the dev clauses than removing the files. If they manage to do that they should be safe and can enable other files if they really need it. |
Can we get that merged or shall I close the PR? |
Is there no way to let nginx execute other PHP files too instead of returning them as plain text? |
@xabbuh yes, that would mean changing the current However, for security reasons, you don't want to run the config.php and other scripts in the SE. |
We could still have the |
Yeah, I think that's the best option. @peterrehm can you maybe update your PR? |
Don't you think that the current approach is better as it forces you to define the additional files? If we would allow more files the |
I am still a bit mixed on this. I agree with you on the Security perspective, but I am afraid that this config will add confusion for people who have to ship there own PHP files. |
On the other hand thinking of this again how common is it that you have to serve your own PHP files in a Symfony application? |
Rather uncommon. And I think security is more important. |
👍 |
also make sure that if you *do* deploy ``app_dev.php`` or ``config.php`` | ||
that these files are secured and not available to any outside user (the | ||
IP address checking code at the top of each file does this by default). | ||
the web directory. All other files will be denied. You **must** also make |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All other files ending in .php
will be denied.
Updated according to @weaverryan's comments. |
Thanks Peter! |
This should improve the security as all non defined php files will return
a 404 error code instead of providing them as text which is a more sane
solution.