Secure errors directory #20212
Secure errors directory #20212
Conversation
|
Hi @schmengler. Thank you for your contribution
For more details, please, review the Magento Contributor Assistant documentation |
|
@magento-engcom-team give me test instance |
|
Hi @schmengler. Thank you for your request. I'm working on Magento instance for you |
|
Hi @schmengler, here is your new Magento instance. |
| @@ -198,6 +203,6 @@ gzip_types | |||
| gzip_vary on; | |||
|
|
|||
| # Banned locations (only reached if the earlier PHP entry point regexes don't match) | |||
| location ~* (\.php$|\.htaccess$|\.git) { | |||
| location ~* (\.php$|\.phtml$|\.htaccess$|\.git) { | |||
There was a problem hiding this comment.
@schmengler maybe forbid both xml and phtml in both occurrences here?
There was a problem hiding this comment.
I was tempted to, but not sure if that could block legit requests
There was a problem hiding this comment.
wouldn't that block sitemaps?
There was a problem hiding this comment.
@schmengler, ok, noticed
For Nginx: deny access to PHTML files in general and XML files within errors directory (nginx.conf.sample)
Do we need to give access for some xml in reality?
There was a problem hiding this comment.
Thanks @wigman!) That's the case.
There was a problem hiding this comment.
although they should not be, sitemaps can be named dynamically so this is not possible I guess?
so for now maybe only excluding .xml in the errors directory could do it?
There was a problem hiding this comment.
@davidverholen aren't they placed in some specific directory with name corresponding to some mask?
There was a problem hiding this comment.
you can choose a relative path and filename under the pub directory, so it's not possible to determine (at least by filename)
There was a problem hiding this comment.
maybe a solution could be, to block the errors directory completely and only explicitly allow files that need to be public? Could be difficult for custom error themes as they lie in sub directories of errors and also can be named dynamically
There was a problem hiding this comment.
I don't see a satisfying general solution here yet
|
@schmengler changes looks good to me, can you squash them into one commit maybe specifying all three points in commit message? Changes in general look atomic to me and does not require splittin into few commits. |
For apache via .htaccess and in nginx sample configuration
|
@orlangur OK, squashed |
magento-engcom-team
added
partners-contribution
|
Hi @orlangur, thank you for the review. |
ghost
added
Progress: testing in progress
and removed
Progress: ready for testing
labels
|
✔️ QA Passed Result:
|
ghost
added
Progress: extended testing
and removed
Progress: testing in progress
labels
|
Hi @schmengler, thank you for your contribution! |
Description (*)
For Apache: deny access to XML and PHTML files within errors directory (pub/errors/.htaccess)
For Nginx: deny access to PHTML files in general and XML files within errors directory (nginx.conf.sample)
Fixed Issues (if relevant)
Manual testing scenarios (*)
See issue #20209
Contribution checklist (*)