Drupal should support RFC 5785, which establishes a .well-known URI location: https://tools.ietf.org/html/rfc5785
These URIs are registered with IANA: https://www.iana.org/assignments/well-known-uris/well-known-uris.xhtml
This patch whitelists the .well-known directory in Drupal's .htaccess directive which blocks access to all hidden directories.
Nginx users can allow the .well-known directory like this (above the general line to block hidden directories and other stuff):
# Allow "Well-Known URIs" as per RFC 5785
location ~* ^/.well-known/ {
allow all;
}
Comment | File | Size | Author |
---|---|---|---|
#53 | support_rfc_5785_by-2408321-53.patch | 548 bytes | sanduhrs |
Comments
Comment #1
D34dMan CreditAttribution: D34dMan commentedThe patch supplied with the issue was not enough or didn't work. Attaching a rule that works for me.
Comment #2
D34dMan CreditAttribution: D34dMan commentedSorry ignore the above patch, it was created against D7, this one has modification particular to D8
Comment #4
mfbmy initial patch only worked if you created the .well-known directory in your doc root. Here's a patch which whitelists .well-known in the FilesMatch but disallows other .dot files, so it's not necessary to create the .well-known directory.
Comment #5
D34dMan CreditAttribution: D34dMan commented@mfb, maybe it could also be noted that its not necessary to create the ".well-known" directory. It could be handled by drupal through a hook_menu declaration.
Comment #6
mfbComment #8
mfbRerolled.
Comment #9
walterebert CreditAttribution: walterebert commentedThis patch works as expected.
Let's Encrypt (https://letsencrypt.org/) uses .well-known for its verification process. Without this patch Drupal users cannot generate free SSL/TLS certificates.
Comment #10
walterebert CreditAttribution: walterebert commentedComment #11
C-LogemannThe #4 patch is applying without errors and working fine. So I set the Status to "RTBC".
Already existing Drupal projects with individual settings in their .htaccess file needs a manual change with these settings.
And because this can easily done manually and D6 is near EOL we don't not need a D6 backport and I remove this tag.
But I add the tag "Quick fix" because It's just a minor change in the default .htaccess file.
Comment #12
C-LogemannHere is a D7-backport patch which needs review to prepare the next step. Because it's against D7 the automatic test should fail.
Comment #14
C-LogemannComment #15
C-LogemannI'm currently failing with starting a retest on D7 branch.
For now I reset the issue back to D8 and RTBC for patch "issue-2408321-support-rfc5785-04.patch".
Comment #16
Wim LeersThis affects security in that it updates the regexp of protected files/directories in
.htaccess
.Comment #17
Daniel KulbeMy suggestion:
D7:
\.(engine|inc|info|install|make|module|profile|test|po|sh|.*sql|theme|tpl(\.php)?|xtmpl)(~|\.sw[op]|\.bak|\.orig|\.save)?$|^(?!\.well-known\/acme-challenge)(\..*|Entries.*|Repository|Root|Tag|Template)$|^#.*#$|\.php(~|\.sw[op]|\.bak|\.orig\.save)$
D8:
\.(engine|inc|install|make|module|profile|po|sh|.*sql|theme|twig|tpl(\.php)?|xtmpl|yml)(~|\.sw[op]|\.bak|\.orig|\.save)?$|^(?!\.well-known\/acme-challenge)(\..*|Entries.*|Repository|Root|Tag|Template)$|^#.*#$|\.php(~|\.sw[op]|\.bak|\.orig|\.save)$
Comment #18
mfbNeeded yet another reroll. This patch currently applies on both 8.1.x and 8.0.x branches.
Comment #19
klausiNginx users can allow the .well-known directory like this (above the general line to block hidden directories and other stuff):
Comment #20
mfb@klausi looks like we should submit a pull request for that here: https://github.com/nginxinc/nginx-wiki/edit/master/source/start/topics/r... - is there any other place where nginx drupal docs are hosted?
Comment #21
serg2 CreditAttribution: serg2 commentedSubmitted a pull request @ https://github.com/nginxinc/nginx-wiki/edit/master/source/start/topics/recipes/drupal.rst which was accepted earlier.
Comment #22
walterebert CreditAttribution: walterebert commentedJust to be clear, Let's Encrypt is not the only practical use case. For example, CalDAV and CardDAV also depend on .well-known.
Comment #23
serg2 CreditAttribution: serg2 commentedthanks @walterebert, sent a pull request to Nginx Wiki which removes reference to Let's Encrypt, now only references "Well-Known URIs".
Updated Nginx code in IS too.
Comment #24
jcnventura CreditAttribution: jcnventura commentedWe looked at this in the Frankfurt sprint.
Comment #25
catchThis could use a change record for 8.1.x - sites that have already got a custom .htaccess could easily miss the change, and a change notice at least lets people checking those do the extra step. CNR for that.
Comment #26
serg2 CreditAttribution: serg2 commentedI created a draft change record at: https://www.drupal.org/node/2661732 .
Comment #27
catchChange record looks fine.
Comment #28
John Morahan CreditAttribution: John Morahan as a volunteer commentedThis changes
^(\..*|........)$
to^(\.(?!well-known)|........)$
- why remove the.*
? Seems that would remove protection for *all* dotfiles (note^.(?!anything)$
is equivalent to^.$
). Granted, that makes no difference as long as mod_rewrite is enabled, thanks to the other rule, but still.Comment #29
catchComment #30
mfb@John Morahan thanks for catching that! New patch restores the .* in FilesMatch
Comment #31
sanduhrsApplied the patch and tested the following scenarios:
mkdir .well-known
GET .well-known
: Access forbiddenmkdir .not-well-known
GET .not-well-known
: Access forbiddenecho 'test' > .well-known/test.txt
GET .well-known/test.txt
: Access grantedecho 'test' > .not-well-known/test.txt
GET .not-well-known/test.txt
: Access forbiddenrm -Rf .well-known
rm -Rf .not-well-known
Created a module with a .well-known route: Access granted
GET .well-known
: Access grantedCreated a module with a .not-well-known route: Access denied
GET .not-well-known
: Access deniedCreated a module with a .well-known/test.txt route: Access granted
GET .well-known/test.txt
: Access grantedCreated a module with a .not-well-known/test.txt route: Access denied
GET .not-well-known/test.txt
: Access grantedThis looks good, change to RTBC.
Comment #33
catchCommitted/pushed to 8.1.x, thanks!
Moving to 7.x for backport.
Comment #34
barryvdh CreditAttribution: barryvdh commentedDrupal 7 support would be very welcome. I'm not really sure how to submit a PR on D.org, but it should be pretty simple change, right?
Comment #35
xumepadismal CreditAttribution: xumepadismal as a volunteer commentedYep, pretty simple port
Comment #36
sanduhrsApplied the patch to D7 and tested the following scenarios:
mkdir .well-known
GET .well-known: Access forbidden
mkdir .not-well-known
GET .not-well-known: Access forbidden
echo 'test' > .well-known/test.txt
GET .well-known/test.txt: Access granted
echo 'test' > .not-well-known/test.txt
GET .not-well-known/test.txt: Access forbidden
rm -Rf .well-known
rm -Rf .not-well-known
Created a module with a .well-known route: Access granted
GET .well-known: Access granted
Created a module with a .not-well-known route: Access denied
GET .not-well-known: Access denied
Created a module with a .well-known/test.txt route: Access granted
GET .well-known/test.txt: Access granted
Created a module with a .not-well-known/test.txt route: Access denied
GET .not-well-known/test.txt: Access denied
From my point of view this is RTBC.
Comment #37
DamienMcKennaShould there be tests for this?
Comment #38
sanduhrsThis is a patch for D7, the D8 patch has already been committed.
So, I'd say the testing is a followup?
Comment #39
sanduhrsFollowup at #2699701: Testing RFC 5785 Support
Comment #40
MrPaulDriver CreditAttribution: MrPaulDriver commentedBe good to see this committed for D7
Comment #41
Fabianx CreditAttribution: Fabianx as a volunteer and at Tag1 Consulting commentedRTBC + 1 for D7, marking for commit soon
Comment #42
David_Rothstein CreditAttribution: David_Rothstein as a volunteer commentedIs it necessary to address https://www.drupal.org/node/2661732#comment-11000005 here first (and for Drupal 8 too)? That looks a bit iffy... although maybe not a big deal in practice.
By the way, that change notice is still in Draft state... I didn't publish it, due to the above - and also because it seems incomplete (it only refers to one half of the changes from this issue, which I don't think works on its own... probably better to point people to an actual diff if we're going to show people suggested changes to make).
Comment #43
Fabianx CreditAttribution: Fabianx as a volunteer and at Tag1 Consulting commentedI think we should fix the bug first in D8. Having the wrong things white-listed is bad.
Thanks David for careful review of the rules.
Comment #45
David_Rothstein CreditAttribution: David_Rothstein as a volunteer commentedAdding an issue credit for the person who discovered that problem.
Comment #48
sammuell CreditAttribution: sammuell commentedWhat's the procedure to get this committed to 7.x soon? It has been committed to the 8.x branches and there is already a patch for D7, so which conditions have not been met?
Comment #49
klausi7.x is on hold for now because we have to fix it in 8.x first.
The next step is to create a patch for 8.x addressing the feedback from above.
Comment #51
xmacinfoI applied the changes committed to 8.3.x on a Drupal 7 .htaccess file.
Which concerns do we have that prevents a commit for Drupal 7?
Comment #52
Fabianx CreditAttribution: Fabianx as a volunteer and at Tag1 Consulting commentedThe regex was wrong, so we have to fix D8 first.
If someone can do a patch, we can get this committed to 8.2.x pretty fast - I think AND then also to 7.x.
Comment #53
sanduhrsAttached patches for D8 and D7.
Comment #55
Fabianx CreditAttribution: Fabianx as a volunteer and at Tag1 Consulting commentedHm, it feels to me that the OR is missing a (), too.
e.g. could someone do some manual regex testing on this?
Comment #56
xmacinfoI am using Drupal 7 and the changes in https://www.drupal.org/node/2408321#comment-11614247
It works fine on my site. The .well-known is publicly readable. And, contrary to the first version I tried earlier, when I redirect my site to only HTTPS, now I get the proper redirection; the previous version redirect http to a 403 error, caused by a [F] (or forbidden) in the error log.
Any other tests that I could do?
Not sure I can do the regex tests, though. :-)
Comment #57
D34dMan CreditAttribution: D34dMan as a volunteer and commentedI can test this if somebody could point me at a documentation of what is allowed and what is not allowed.
Comment #58
sanduhrsDefining Well-Known Uniform Resource Identifiers (URIs)
https://tools.ietf.org/html/rfc5785
Comment #59
grossmann CreditAttribution: grossmann commentedI tested this on D7 as I needed it for Let'sEncrypt certificates. Patch seems to work for D7. D8 not tested.
But for D7 I would say RTBC.
Comment #60
D34dMan CreditAttribution: D34dMan as a volunteer and commentedoh wait, not just well-known uris @sanduhrs, but since same regex is used to protect other files and folders, i would like to test regression too. So what is Drupal's policy in general for what files should be shown and what not
Comment #61
Leo Pitt CreditAttribution: Leo Pitt commentedI've been using this patch on D7 sites for a little while now, no problems. Can we please get it committed to 7.x?
Comment #62
jabrister CreditAttribution: jabrister as a volunteer commentedI used the patch on #53 on my D7 site and now I get this error message:
Before the patch I was getting this error:
Comment #63
John Morahan CreditAttribution: John Morahan as a volunteer commentedTo answer #55 (by a visual review, I haven't tested it) it looks right to me that there is no () on the OR. The first branch is just
/\.
which should catch any dotfile in a subdirectory. The second branch is all of^\.(?!well-known/)
i.e. the well-known exception applies only after^\.
which is not in a subdirectory. That is correct:Comment #64
sammuell CreditAttribution: sammuell commentedWhy exactly is this issue still open? It has been committed to 8.2 and 8.3 after all.
I created a backport issue against 7.x to get it committed there.
Comment #67
jay.lee.bio CreditAttribution: jay.lee.bio commentedFor those of you using Virtualmin, don't forget that you also need to edit the ssl.conf file on your server for everything to work properly.
Comment #68
sanduhrsAs per #56
Comment #69
John Morahan CreditAttribution: John Morahan as a volunteer commented@sanduhrs your updated patch hasn't been committed to D8 yet ;-)
Comment #70
John Morahan CreditAttribution: John Morahan as a volunteer commentedFinally got around to testing this.
Seems fine to me.
Comment #73
catchCommitted/pushed to 8.4.x and cherry-picked to 8.3.x. Thanks!
Please open the 7.x backport.
Comment #74
sammuell CreditAttribution: sammuell commentedThe backport issue is already open: https://www.drupal.org/node/2847325
It just needs another set of eyes to change the status and a commit.
Comment #76
Wim LeersPublished the CR at https://www.drupal.org/node/2661732. It probably fell under the radar because it was not associated with any project…
Comment #77
David_Rothstein CreditAttribution: David_Rothstein as a volunteer commentedThe change notice was never published because it was incorrect - see https://www.drupal.org/node/2408321#comment-11361857
I will try to fix it now.
Comment #78
David_Rothstein CreditAttribution: David_Rothstein as a volunteer commentedDid my best to fix it via https://www.drupal.org/node/2661732/revisions/view/10527162/10527685
If someone wants to copy this and create a corresponding change record for the Drupal 7 version, feel free. But we already mentioned it in the 7.55 release notes so I didn't think it was that important.