Skip to content

Comments

Use temporary htaccesstest.txt for data dir security check#25014

Merged
PVince81 merged 1 commit intomasterfrom
admin-datadircheck-fix
Jun 9, 2016
Merged

Use temporary htaccesstest.txt for data dir security check#25014
PVince81 merged 1 commit intomasterfrom
admin-datadircheck-fix

Conversation

@PVince81
Copy link
Contributor

@PVince81 PVince81 commented Jun 7, 2016

Fixes #24987

Brings back the htaccesstest.txt file and uses that instead of ".ocdata" for the ajax htaccess check.
Also added cache buster for that file to avoid getting 304 when accessing the file.

I'm not too happy about the approach but it seems to work.
The reason I'm not happy:

  • stray file
  • the PR touches the existing isHtaccessWorking function which is still used at setup time, but not touching it would result in duplicate code

Please review @nickvergessen @rullzer @RealRancor @icewind1991 @danimo

@nickvergessen
Copy link
Contributor

btw is .ocdata used for anything else?

@nickvergessen
Copy link
Contributor

Btw, the warning should show up in Security & setup warnings right?
Because for me nothing displayes in there, even when I remove the .htaccess

@PVince81
Copy link
Contributor Author

PVince81 commented Jun 8, 2016

@nickvergessen hmm, in my case nothing showed up either that's why I added the t=xxx suffix to bust the cache. Can you check your network console ?

The .ocdata file is used to confirm that this folder is actually a data directory in case it gets unmounted by mistake (ex: rebooted system where the mount is missing).

@nickvergessen
Copy link
Contributor

Okay, I used a datadir outside the server root, so the check is skipped.
Works now 👍

@mmattel
Copy link
Contributor

mmattel commented Jun 8, 2016

One question: because there are now many instances out which use .ocdata.
Should there be a routine included which deletes .ocdata if present?
Keeps the datadirectory clean...

@PVince81
Copy link
Contributor Author

PVince81 commented Jun 8, 2016

@mmattel no. .ocdata is used elsewhere as explained above, we can't remove it.

Using it for data dir access was only an addition, since we knew that file would always be there.

@PVince81
Copy link
Contributor Author

PVince81 commented Jun 8, 2016

See #22310 (comment) for more context. The issue report explains it better.

@PVince81
Copy link
Contributor Author

PVince81 commented Jun 9, 2016

So, do we agree on this approach ? Note that this will require a backport to 9.0

@mmattel
Copy link
Contributor

mmattel commented Jun 9, 2016

Fine to me 👍

@PVince81 PVince81 merged commit 90c1ec1 into master Jun 9, 2016
@PVince81 PVince81 deleted the admin-datadircheck-fix branch June 9, 2016 09:58
@PVince81
Copy link
Contributor Author

PVince81 commented Jun 9, 2016

stable9: #25045

@lock
Copy link

lock bot commented Aug 5, 2019

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators Aug 5, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Security issue: Move datadir protection check back from .ocdata to htaccess.txt

3 participants