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

Reference: https://www.drupal.org/core/beta-changes
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.
CommentFileSizeAuthor
#20 2417075-20-internal-trusted-hosts.patch10.79 KBtstoeckler
#20 2417075-20-internal-trusted-hosts--tests-only.patch4.35 KBtstoeckler
#16 2417075-16-internal-trusted-hosts.patch3.62 KBtstoeckler
#12 internal-trusted-hosts-hack-do-not-test.patch882 byteststoeckler
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

sachbearbeiter’s picture

sachbearbeiter CreditAttribution: sachbearbeiter commented
Issue summary: View changes
Anonymous’s picture

Anonymous (not verified) CreditAttribution: Anonymous commented

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

sachbearbeiter’s picture

sachbearbeiter CreditAttribution: sachbearbeiter commented

What 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) -> 

Trusted Host Settings 	Not enabled
The trusted_host_patterns setting is not configured in settings.php. This can lead to security vulnerabilities. It is highly recommended that you configure this. See Protecting against HTTP HOST Header attacks for more information.

CHMOD -> change in settings.php:

$settings['trusted_host_patterns'] = array(
   '^foo-testen\.de$',
   '^.+\.foo-testen\.de$',
   '^foo-testen\.de',
   '^.+\.foo-testen\.de',
);

Now i get immediately the error message:

UnexpectedValueException: Untrusted Host "localhost" in Symfony\Component\HttpFoundation\Request->getHost() (line 1221 of core/vendor/symfony/http-foundation/Symfony/Component/HttpFoundation/Request.php).

Symfony\Component\HttpFoundation\Request->getHttpHost()
Symfony\Component\HttpFoundation\Request->getSchemeAndHttpHost()
Symfony\Component\HttpFoundation\Request->prepareRequestUri()
Symfony\Component\HttpFoundation\Request->getRequestUri()
Symfony\Component\HttpFoundation\Request->prepareBaseUrl()
Symfony\Component\HttpFoundation\Request->getBaseUrl()
Symfony\Component\HttpFoundation\Request->preparePathInfo()
Symfony\Component\HttpFoundation\Request->getPathInfo()
Drupal\Core\Routing\RouteProvider->getRouteCollectionForRequest(Object)
Symfony\Cmf\Component\Routing\NestedMatcher\NestedMatcher->matchRequest(Object)
Symfony\Cmf\Component\Routing\DynamicRouter->matchRequest(Object)
Symfony\Cmf\Component\Routing\ChainRouter->doMatch('/node/add')
Symfony\Cmf\Component\Routing\ChainRouter->match('/node/add')
Drupal\Core\Path\PathValidator->getPathAttributes('node/add', Object, )
Drupal\Core\Path\PathValidator->getUrl('node/add', )
Drupal\Core\Path\PathValidator->getUrlIfValidWithoutAccessCheck('node/add')
Drupal\link\Plugin\Field\FieldType\LinkItem->getUrl()
Drupal\shortcut\Entity\Shortcut->getUrl()
shortcut_preprocess_page(Array, 'page', Array)
Drupal\Core\Theme\ThemeManager->theme('page', Array)
Drupal\Core\Theme\ThemeManager->render('page', Array)
Drupal\Core\Render\Renderer->doRender(Array, 1)
Drupal\Core\Render\Renderer->render(Array, 1)
Drupal\Core\Render\MainContent\HtmlRenderer->renderResponse(Array, Object, Object)
Drupal\Core\EventSubscriber\MainContentViewSubscriber->onViewRenderArray(Object, 'kernel.view', Object)
Drupal\Component\EventDispatcher\ContainerAwareEventDispatcher->dispatch('kernel.view', Object)
Symfony\Component\HttpKernel\HttpKernel->handleRaw(Object, 1)
Symfony\Component\HttpKernel\HttpKernel->handle(Object, 1, 1)
Drupal\Core\StackMiddleware\KernelPreHandle->handle(Object, 1, 1)
Drupal\Core\StackMiddleware\PageCache->pass(Object, 1, 1)
Drupal\Core\StackMiddleware\PageCache->handle(Object, 1, 1)
Drupal\Core\StackMiddleware\ReverseProxyMiddleware->handle(Object, 1, 1)
Stack\StackedHttpKernel->handle(Object, 1, 1)
Drupal\Core\DrupalKernel->handle(Object)

Ok i add localhost to the settings:

$settings['trusted_host_patterns'] = array(
   '^foo-testen\.de$',
   '^.+\.foo-testen\.de$',
   '^foo-testen\.de',
   '^.+\.foo-testen\.de',
   '^localhost$',
   '^localhost\.*',
   '\.local$',
);

Everything is running fine ... but normally i would place the localhost stuff only into settings.local.php right?

tstoeckler’s picture

tstoeckler
he/him
Primary language German
Location Essen, Germany
CreditAttribution: tstoeckler commented
Category: Bug report » Support request
Status: Active » Fixed

@sachbearbeiter Seems like your problem has been fixed?

David4514’s picture

David4514 CreditAttribution: David4514 commented
Status: Fixed » Active

I'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:

The provided host name is not valid for this server.

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.

David4514’s picture

David4514 CreditAttribution: David4514 commented
Category: Support request » Bug report

I've been chasing this down for hours. It appears for me, the error occurs starting from the Shortcut.php module with line 91:

  return $this->link->first()->getUrl();

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:

  1. internal:/add/node
  2. internal:/admin/content

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.

David4514’s picture

David4514 CreditAttribution: David4514 commented

Additional Info

The request object that has the "localhost" name is created in \Symfony\Cmf\Component\Routing\ChainRouter in the doMatch function at line 182.

  $requestForMatching = Request::create($url);

The $url contains "/node/add". The $requestForMatching object contains ['headers']['headers']['host']['0'] = 'localhost' which is the eventual source of the problem.

GaëlG’s picture

GaëlG
he/him
Primary language French
Location Lille, France
CreditAttribution: GaëlG commented

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

tstoeckler’s picture

tstoeckler
he/him
Primary language German
Location Essen, Germany
CreditAttribution: tstoeckler commented
Priority: Normal » Major
Issue tags: +Needs issue summary update

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

tstoeckler’s picture

tstoeckler
he/him
Primary language German
Location Essen, Germany
CreditAttribution: tstoeckler commented
Priority: Major » Critical
Issue summary: View changes

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

tstoeckler’s picture

tstoeckler
he/him
Primary language German
Location Essen, Germany
CreditAttribution: tstoeckler commented
Title: UnexpectedValueException: Untrusted Host "localhost" » Trusted host verification is incompatible with URIs with the "internal" scheme
Version: 8.0.0-beta6 » 8.0.x-dev
Component: shortcut.module » routing system
tstoeckler’s picture

tstoeckler
he/him
Primary language German
Location Essen, Germany
CreditAttribution: tstoeckler commented
FileSize
internal-trusted-hosts-hack-do-not-test.patch882 bytes

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

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented

I kinda prefer number 6:

Provide a "request factory" so that people do not use Request::create() In that request factory we could properly pass in the $server array. Would not prevent people from using Request::create(), though

But note: you can actually provide a request factory, by calling Request::setFactory()

tstoeckler’s picture

tstoeckler
he/him
Primary language German
Location Essen, Germany
CreditAttribution: tstoeckler commented
Issue tags: -Needs issue summary update +Needs tests

Wow, I didn't know it would be that easy. Thanks! Sounds like that's the way to go then.

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented

Symfony seems to solve that problem directly in the places which do the subrequest: https://github.com/symfony/symfony/blob/85d5413bebe48333269436252bc7ed13...

tstoeckler’s picture

tstoeckler
he/him
Primary language German
Location Essen, Germany
CreditAttribution: tstoeckler commented
Status: Active » Needs review
FileSize
2417075-16-internal-trusted-hosts.patch3.62 KB

This seems to work.

Didn't write any tests yet.

David4514’s picture

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.

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented

I 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 ;)

tstoeckler’s picture

tstoeckler
he/him
Primary language German
Location Essen, Germany
CreditAttribution: tstoeckler commented
Assigned: Unassigned » tstoeckler

Writing some tests.

tstoeckler’s picture

tstoeckler
he/him
Primary language German
Location Essen, Germany
CreditAttribution: tstoeckler commented
Issue tags: -Needs tests
FileSize
2417075-20-internal-trusted-hosts--tests-only.patch4.35 KB
2417075-20-internal-trusted-hosts.patch10.79 KB

Here 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().

tstoeckler’s picture

tstoeckler
he/him
Primary language German
Location Essen, Germany
CreditAttribution: tstoeckler commented
Assigned: tstoeckler » Unassigned

The last submitted patch, 20: 2417075-20-internal-trusted-hosts--tests-only.patch, failed testing.

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented
Status: Needs review » Reviewed & tested by the community

Both the tests and the actual code are looking great!

David4514’s picture

David4514 CreditAttribution: David4514 commented

The patch in #20 (2417075-20-internal-trusted-hosts.patch) looks good and seems to be working for me.

David4514’s picture

David4514 CreditAttribution: David4514 commented
Issue summary: View changes

Corrected a couple of typos in the Issue Summary (math to match)

  1. ChainRouter::doMath() --> ChainRouter::doMatch()
  2. $router->mathRequest($request) --> $router->matchRequest($request)
tstoeckler’s picture

tstoeckler
he/him
Primary language German
Location Essen, Germany
CreditAttribution: tstoeckler commented

@David4514: How dare you! This was step 1 of my 300 step plan to get a MATH request type accepted for HTTP3.

(< / kidding > thanks!)

mpdonadio’s picture

mpdonadio
he/him
Primary language English
Location Philadelphia/PA/USA (UTC-5)
CreditAttribution: mpdonadio commented
Issue summary: View changes
Issue tags: +Needs issue summary update, +Needs beta evaluation
Related issues: +#2221699: HTTP_HOST header cannot be trusted

I'll try to do both later today after I thaw out and can type.

tstoeckler’s picture

tstoeckler
he/him
Primary language German
Location Essen, Germany
CreditAttribution: tstoeckler commented

Yes, an issue summary update is necessary. Forgot about that. But do critical bugs really need a beta evaluation? Seems rather pointless to me...

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented

Yes, an issue summary update is necessary. Forgot about that. But do critical bugs really need a beta evaluation? Seems rather pointless to me...

I think as well, its more important that that issue summary is good.

tstoeckler’s picture

tstoeckler
he/him
Primary language German
Location Essen, Germany
CreditAttribution: tstoeckler commented

Oh, and of course forgot the most important thing: @mpdonadio Thanks for offering to update the IS!!!

mpdonadio’s picture

mpdonadio
he/him
Primary language English
Location Philadelphia/PA/USA (UTC-5)
CreditAttribution: mpdonadio commented
Issue summary: View changes
Issue tags: -Needs issue summary update, -Needs beta evaluation

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

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
CreditAttribution: webchick commented
Status: Reviewed & tested by the community » Fixed

I'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!

  • webchick committed 0a84d31 on 8.0.x 
    Issue #2417075 by tstoeckler, David4514, dawehner, sachbearbeiter,...
mikey_p’s picture

mikey_p CreditAttribution: mikey_p commented

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

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott commented

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

Status: Fixed » Closed (fixed)

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

lojose85 queued 16: 2417075-16-internal-trusted-hosts.patch for re-testing.

The last submitted patch, 16: 2417075-16-internal-trusted-hosts.patch, failed testing.