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;
}

Comments

D34dMan’s picture

FileSize
1.05 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 84,024 pass(es), 2 fail(s), and 0 exception(s). View

The patch supplied with the issue was not enough or didn't work. Attaching a rule that works for me.

D34dMan’s picture

FileSize
1.05 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 84,030 pass(es). View

Sorry ignore the above patch, it was created against D7, this one has modification particular to D8

The last submitted patch, 1: issue-2408321-support-rfc5785-D34dMan-01.patch, failed testing.

mfb’s picture

FileSize
1.15 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 84,148 pass(es). View

my 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.

D34dMan’s picture

@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.

mfb’s picture

mfb’s picture

FileSize
1.12 KB

Rerolled.

walterebert’s picture

This 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.

walterebert’s picture

C_Logemann’s picture

Status: Needs review » Reviewed & tested by the community
Issue tags: -needs backport to D6 +Quick fix

The #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.

C_Logemann’s picture

Here is a D7-backport patch which needs review to prepare the next step. Because it's against D7 the automatic test should fail.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 12: support_rfc5785_D7_backport-2408321-12.patch, failed testing.

C_Logemann’s picture

Version: 8.0.x-dev » 7.x-dev
Status: Needs work » Needs review
C_Logemann’s picture

Version: 7.x-dev » 8.0.x-dev
Status: Needs review » Reviewed & tested by the community

I'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".

Wim Leers’s picture

Issue tags: +security

This affects security in that it updates the regexp of protected files/directories in .htaccess.

Daniel Kulbe’s picture

My 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)$

mfb’s picture

Version: 8.0.x-dev » 8.1.x-dev
FileSize
1.17 KB

Needed yet another reroll. This patch currently applies on both 8.1.x and 8.0.x branches.

klausi’s picture

Issue summary: View changes

Nginx users can allow the .well-known directory like this (above the general line to block hidden directories and other stuff):

# Allow Let's Encrypt RFC 5785 ACME protocol
location ~* ^/.well-known/ {
  allow all;
}
mfb’s picture

@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?

serg2’s picture

walterebert’s picture

Just to be clear, Let's Encrypt is not the only practical use case. For example, CalDAV and CardDAV also depend on .well-known.

serg2’s picture

Issue summary: View changes

thanks @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.

jcnventura’s picture

Issue tags: +SprintWeekend2016

We looked at this in the Frankfurt sprint.

catch’s picture

Status: Reviewed & tested by the community » Needs review
Issue tags: +Needs change record

This 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.

serg2’s picture

Issue tags: -Needs change record

I created a draft change record at: https://www.drupal.org/node/2661732 .

catch’s picture

Status: Needs review » Reviewed & tested by the community

Change record looks fine.

John Morahan’s picture

This 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.

catch’s picture

Status: Reviewed & tested by the community » Needs review
mfb’s picture

@John Morahan thanks for catching that! New patch restores the .* in FilesMatch

sanduhrs’s picture

Status: Needs review » Reviewed & tested by the community

Applied the patch 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 granted

This looks good, change to RTBC.

  • catch committed 9a283ad on 8.1.x
    Issue #2408321 by mfb, D34dMan, C_Logemann, serg2, walterebert, sanduhrs...
catch’s picture

Version: 8.1.x-dev » 7.x-dev
Status: Reviewed & tested by the community » Patch (to be ported)

Committed/pushed to 8.1.x, thanks!

Moving to 7.x for backport.

Barryvdh’s picture

Drupal 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?

<FilesMatch "\.(engine|inc|info|install|make|module|profile|test|po|sh|.*sql|theme|tpl(\.php)?|xtmpl)(~|\.sw[op]|\.bak|\.orig|\.save)?$|^(\.(?!well-known).*|Entries.*|Repository|Root|Tag|Template)$|^#.*#$|\.php(~|\.sw[op]|\.bak|\.orig\.save)$">
  Order allow,deny
</FilesMatch>

  RewriteRule "(^|/)\.(?!well-known)" - [F]
xumepadismal’s picture

Status: Patch (to be ported) » Needs review
FileSize
1.16 KB

Yep, pretty simple port

sanduhrs’s picture

Status: Needs review » Reviewed & tested by the community

Applied 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.

DamienMcKenna’s picture

Should there be tests for this?

sanduhrs’s picture

This is a patch for D7, the D8 patch has already been committed.
So, I'd say the testing is a followup?

sanduhrs’s picture

MrPaulDriver’s picture

Be good to see this committed for D7

Fabianx’s picture

RTBC + 1 for D7, marking for commit soon

David_Rothstein’s picture

Is 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).

Fabianx’s picture

Version: 7.x-dev » 8.1.x-dev
Status: Reviewed & tested by the community » Needs work
Issue tags: -Quick fix

I 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.

David_Rothstein’s picture

Adding an issue credit for the person who discovered that problem.

  • catch committed 9a283ad on 8.3.x
    Issue #2408321 by mfb, D34dMan, C_Logemann, serg2, walterebert, sanduhrs...

  • catch committed 9a283ad on 8.3.x
    Issue #2408321 by mfb, D34dMan, C_Logemann, serg2, walterebert, sanduhrs...
sammuell’s picture

What'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?

klausi’s picture

7.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.

Version: 8.1.x-dev » 8.2.x-dev

Drupal 8.1.9 was released on September 7 and is the final bugfix release for the Drupal 8.1.x series. Drupal 8.1.x will not receive any further development aside from security fixes. Drupal 8.2.0-rc1 is now available and sites should prepare to upgrade to 8.2.0.

Bug reports should be targeted against the 8.2.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

xmacinfo’s picture

I 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?

Fabianx’s picture

The 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.

Status: Needs review » Needs work

The last submitted patch, 53: d7-support_rfc_5785_by-2408321-53.patch, failed testing.

Fabianx’s picture

Hm, it feels to me that the OR is missing a (), too.

e.g. could someone do some manual regex testing on this?

xmacinfo’s picture

I 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. :-)

D34dMan’s picture

I can test this if somebody could point me at a documentation of what is allowed and what is not allowed.

sanduhrs’s picture

Defining Well-Known Uniform Resource Identifiers (URIs)
https://tools.ietf.org/html/rfc5785

grossmann’s picture

I 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.

D34dMan’s picture

oh 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

Leo Pitt’s picture

I've been using this patch on D7 sites for a little while now, no problems. Can we please get it committed to 7.x?

jabrister’s picture

I used the patch on #53 on my D7 site and now I get this error message:


The challenge url is not working with an HTTP status of 0. Please check your .htaccess as it may need modifications, see Support RFC 5785 by whitelisting the .well-known directory.

Before the patch I was getting this error:

The challenge url is not working with an HTTP status of 403. Please check your .htaccess as it may need modifications, see Support RFC 5785 by whitelisting the .well-known directory

John Morahan’s picture

To 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:

A well-known URI is a URI [RFC3986] whose path component begins with
the characters "/.well-known/", and whose scheme is "HTTP", "HTTPS",
or another scheme that has explicitly been specified to use well-
known URIs.

sammuell’s picture

Why 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.

  • catch committed 9a283ad on 8.4.x
    Issue #2408321 by mfb, D34dMan, C_Logemann, serg2, walterebert, sanduhrs...

  • catch committed 9a283ad on 8.4.x
    Issue #2408321 by mfb, D34dMan, C_Logemann, serg2, walterebert, sanduhrs...
jay.lee.bio’s picture

For 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.

sanduhrs’s picture

Version: 8.2.x-dev » 7.x-dev
Status: Needs work » Reviewed & tested by the community

As per #56

John Morahan’s picture

Version: 7.x-dev » 8.4.x-dev
Status: Reviewed & tested by the community » Needs review

@sanduhrs your updated patch hasn't been committed to D8 yet ;-)

John Morahan’s picture

Status: Needs review » Reviewed & tested by the community

Finally got around to testing this.

# check() { curl -I "http://localhost$1" 2>/dev/null | head -n 1; }
# check /
HTTP/1.1 200 OK
# check /.well-known
HTTP/1.1 403 Forbidden
# check /.well-known-not
HTTP/1.1 403 Forbidden
# mkdir -p .well-known/acme-challenge
# echo test > .well-known/acme-challenge/test
# check /.well-known/acme-challenge/test
HTTP/1.1 200 OK
# mkdir -p .well-known-not/acme-challenge
# echo test > .well-known-not/acme-challenge/test
# check /.well-known-not/acme-challenge/test
HTTP/1.1 403 Forbidden

Seems fine to me.

  • catch committed b52ac5e on 8.4.x
    Issue #2408321 by mfb, sanduhrs, D34dMan, C_Logemann, xumepadismal, John...

  • catch committed df8954c on 8.3.x
    Issue #2408321 by mfb, sanduhrs, D34dMan, C_Logemann, xumepadismal, John...
catch’s picture

Version: 8.4.x-dev » 8.3.x-dev
Status: Reviewed & tested by the community » Fixed

Committed/pushed to 8.4.x and cherry-picked to 8.3.x. Thanks!

Please open the 7.x backport.

sammuell’s picture

The 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.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.

Wim Leers’s picture

Published the CR at https://www.drupal.org/node/2661732. It probably fell under the radar because it was not associated with any project…

David_Rothstein’s picture

The 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.

David_Rothstein’s picture

Did 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.