Problem/Motivation
Since #2221699: HTTP_HOST header cannot be trusted Drupal uses Symfony's trusted host verification. The trusted hosts are stored statically, so they are automatically shared between all request objects, even "fake" ones created with Request::create()
. When generating a link to a URI with the internal
scheme, the actual host of the main request is not passed into the "fake" request created for URL generation, so when host verification happens the host that gets checked falls back to Symfony's default localhost
. If localhost
is not among the trusted hosts, the verification fails and throws an exception rendering the site completely unusable.
This can be triggered by adding a shortcut and in fact is triggered with the default shortcuts in the Standard profile, making this problem particularly nasty.
It can also be triggered by adding a link such as /admin/config
to a menu and then visiting the list of menu links in the admin area.
Proposed resolution
Create a request factory that always uses the host from the initial request; this will prevent the Symfony default 'localhost' from ever being used in a Request object. The factory can then be attached to the Request class using the builtin Request::setFactory(), which will eliminate any API changes.
Remaining tasks
None.
User interface changes
None.
API changes
None.
Original report by sachbearbeiter
I get here the following error message:
admin/config/user-interface/shortcut/manage/default/add-link-inline?token=u4Io3c4iD8cw4lG-cYEXbwjdw9s1SR_bzV09fDNL3Oc&link=admin/config/development/performance&name=Performance&destination=admin/config/development/performance
UnexpectedValueException: Untrusted Host "localhost" in Symfony\Component\HttpFoundation\Request->getHost() (line 1221 of core/vendor/symfony/http-foundation/Symfony/Component/HttpFoundation/Request.php).
Settings.php i configured like that:
$settings['trusted_host_patterns'] = array(
'^foo\.com$',
'^.+\.foo\.com$',
);
Beta phase evaluation
Issue category | Bug; see test-only patch in #20 |
---|---|
Issue priority | Critical because it occurs when the security hardening provided by the trusted hosts is enabled. |
Disruption | None; no API change. |
Comment | File | Size | Author |
---|---|---|---|
#20 | 2417075-20-internal-trusted-hosts.patch | 10.79 KB | tstoeckler |
#20 | 2417075-20-internal-trusted-hosts--tests-only.patch | 4.35 KB | tstoeckler |
Comments
Comment #1
sachbearbeiter CreditAttribution: sachbearbeiter commentedComment #2
Anonymous (not verified) CreditAttribution: Anonymous commentedI'm confused by this description. The regex in trusted_host_patterns doesn't include 'localhost' and thus the exception is correct.
What are you trying to do? Can you resolve this by adding 'localhost' to the trusted hosts?
Comment #3
sachbearbeiter CreditAttribution: sachbearbeiter commentedWhat are you trying to do? Can you resolve this by adding 'localhost' to the trusted hosts?
I'm trying to reproduce:
D8b6 fresh install on server (not meant as localhost) ->
CHMOD -> change in settings.php:
Now i get immediately the error message:
Ok i add localhost to the settings:
Everything is running fine ... but normally i would place the localhost stuff only into settings.local.php right?
Comment #4
tstoeckler@sachbearbeiter Seems like your problem has been fixed?
Comment #5
David4514 CreditAttribution: David4514 commentedI'm not sure that this is fixed.
I use Desktop Ubuntu as a development server. This server runs as a virtual machine under Windows. I use Apache2 VirtualHosting to support my drupal 8 development site. I do not use "localhost" in the browser to access the website. In fact if I do use localhost, I get the Apache2 Ubuntu Default Page instead of my website as I would expect.
When I first encountered this error, I was browsing from within my Unbuntu Desktop to the local server. Even though I was not using "localhost", I was willing to accept that a local browser could show up as localhost. I will not claim to be a linux/apache expert.
I then tried accessing the website from an external host. I removed "^localhost$" from the list of trusted_host_patterns in settings.php, expecting that the url (like www.example.com) would be used. However, it still failed with the UnexpectedValueException: Untrusted Host "localhost".
If I remove my entry for my url from the trusted_host_patterns, I do get the message:
So it always fails unless "^localhost$" is included in the trusted_host_patterns. This is true for both local and external browsers. It appears that I need both the localhost and my actual url name in the trusted_host_patterns.
I'm leaving this as a "support request" just in case I've made an noob error.
Comment #6
David4514 CreditAttribution: David4514 commentedI've been chasing this down for hours. It appears for me, the error occurs starting from the Shortcut.php module with line 91:
I don't think Shortcut.php is to blame. This eventually calls Drupal\Core\Url::fromUri(). The two shortcuts that are active have the following paths associated with them:
It is the "internal:" that seems to cause the problem. Through a very lengthy chain of events, the path is matched to routes, a bare bones request object is created, and an eventual call to Symfony\Component\HttpFoundation\Request\getHost() where the request is verified against the trusted_host_patterns. The problem is that this bare bones request object is taking a default of "localhost" in the headers. If "localhost" is not in your trusted_host_patterns list, the UnexpectedValueException is thrown.
If I log out so that the shortcuts are not there, then I do not need "^localhost$" in my trusted_host_patterns array. Or if I override the data in the shortcut_field_data table with "http:/add/node" and "http:/admin/content" and do a drush cr, everthing works fine without "^localhost$" in the trusted_host_patterns array.
What I cannot tell is if localhost was really correct or not. However, I believe this to be a bug which needs to be fixed.
Comment #7
David4514 CreditAttribution: David4514 commentedAdditional Info
The request object that has the "localhost" name is created in \Symfony\Cmf\Component\Routing\ChainRouter in the doMatch function at line 182.
The $url contains "/node/add". The $requestForMatching object contains ['headers']['headers']['host']['0'] = 'localhost' which is the eventual source of the problem.
Comment #8
GaëlGIt looks like I get the same problem. I try to access the website with an url like
http://www.mysite.org
.It works if I do not set the trusted hosts setting in settings.php.
If I only set
^www\.mysite\.org$
in trusted hosts patterns, I get a 500 error and the same message in the watchdog: "UnexpectedValueException: Untrusted Host "localhost"". But as an anonymous user I get no error.If I only set
^localhost$
, I get a 400 error and the text "The provided host name is not valid for this server." is displayed.Comment #9
tstoecklerI can reproduce this now. This is at least major. Trying to find out some details now.
Also needs an issue summary update. Bottom line though:
$settings['trusted_host_patterns']
(which is a security best-practice) is incompatible with Shortcut module (at least).Comment #10
tstoecklerRaising to critical.
The detective work in #6 and #7 is spot on, thanks for that @David4514!
Updating the issue summary accordingly.
I put everything I could think of in the proposed resolutions, feel free to amend.
This is a critical bug so skipping the beta evaluation.
Comment #11
tstoecklerComment #12
tstoecklerFor anyone suffering from this and not willing to sacrifice the trusted hosts feature here's a super hacky quick-fix patch to "fix" the problem.
It is in no way meant to be committed to core.
Comment #13
dawehnerI kinda prefer number 6:
But note: you can actually provide a request factory, by calling
Request::setFactory()
Comment #14
tstoecklerWow, I didn't know it would be that easy. Thanks! Sounds like that's the way to go then.
Comment #15
dawehnerSymfony seems to solve that problem directly in the places which do the subrequest: https://github.com/symfony/symfony/blob/85d5413bebe48333269436252bc7ed13...
Comment #16
tstoecklerThis seems to work.
Didn't write any tests yet.
Comment #17
David4514 CreditAttribution: David4514 commented@tstoeckler - I'm impresssed! Thanks. I could not have done what you have. I cannot say that I understand every detail, but at least the patch in #16 seems to work for me.
Comment #18
dawehnerI think this is the most pragmatic approach for now. Fixing the various levels in the routers would be more problematic but certainly worth a try.
Does someone know why symfony people just have way less problems than we have sometimes? There has to be an underlying reason ;)
Comment #19
tstoecklerWriting some tests.
Comment #20
tstoecklerHere we go.
I messed up locally and couldn't be bothered to retroactively create an interdiff. The only change besides all the added test hunks is that I added @param and @return docs to TrustedHostsRequestFactory::createRequest().
Comment #21
tstoecklerComment #23
dawehnerBoth the tests and the actual code are looking great!
Comment #24
David4514 CreditAttribution: David4514 commentedThe patch in #20 (2417075-20-internal-trusted-hosts.patch) looks good and seems to be working for me.
Comment #25
David4514 CreditAttribution: David4514 commentedCorrected a couple of typos in the Issue Summary (math to match)
Comment #26
tstoeckler@David4514: How dare you! This was step 1 of my 300 step plan to get a MATH request type accepted for HTTP3.
(< / kidding > thanks!)
Comment #27
mpdonadioI'll try to do both later today after I thaw out and can type.
Comment #28
tstoecklerYes, an issue summary update is necessary. Forgot about that. But do critical bugs really need a beta evaluation? Seems rather pointless to me...
Comment #29
dawehnerI think as well, its more important that that issue summary is good.
Comment #30
tstoecklerOh, and of course forgot the most important thing: @mpdonadio Thanks for offering to update the IS!!!
Comment #31
mpdonadioI think having this is place may have prevented some of the weirdness we saw when developing #2221699: HTTP_HOST header cannot be trusted, but I don't think anything from that needs to come out with this going in.
No API change, so no CR is needed.
Comment #32
webchickI'll admit this goes over my head a bit, but it's well-documented and seems to have agreement from the right folks.
Committed and pushed to 8.0.x. Thanks!
Comment #34
mikey_p CreditAttribution: mikey_p commentedJust came by to say the same thing that @mpdonadio mentioned in #31. This is very similar to the issues we saw in the original trusted host patch, and probably would have solved some of the problems there as well.
Comment #35
alexpottThis issue has introduced a regression. Since it is critical I'm not going to revert - I've opened #2448223: Cannot run all PHPUnit tests using PHPUnit to track progress on fixing this.