@robocoder opened this Issue on April 3rd 2012 Contributor

Piwik config should contain a list of valid hosts (where the Piwik server resides) to either validate $_SERVER['HTTP_HOST'], or use in place of, when generating absolute URLs.

@mattab commented on April 4th 2012 Member

maybe we should do like Wordpress and require users to specify the piwik URL and never rely on HTTP_HOST ?

It's quite less user friendly to do so, but maybe useful?

@robocoder commented on April 13th 2012 Contributor

Sure, we can make it configureable.

We can also set it initially using the URL at the time of installation, and/or the first website's URL.

@robocoder commented on April 13th 2012 Contributor

(In [6207]) refs #3080 - backend implementation of trusted_hosts validation; need front-end UI for runtime configuration

@mattab commented on April 30th 2012 Member

Well done vipsoft, excellent improvement! :)

@mattab commented on April 30th 2012 Member

Is there any other work appart from updating FAQ, to do before closing the ticket?

@mattab commented on May 7th 2012 Member

Is there any other work appart from updating FAQ?

@mattab commented on May 29th 2012 Member

(In [6370]) Refs #3080

  • Enabling adding localhost / to trusted hosts list otherwise logging in was failing in my test Install
  • Disable the feature until we have the UI to change the URL or display some kind of warning "The host you are using is different from the hostname known for this Piwik server. Do you wish to add the hostname %S to the list of trusted hostname?" Then the super user could accept. Otherwise this will cause problems with users migrating to different URLs which will then break logging (as the "Local URL" test will fail after piwik url changes)
@robocoder commented on May 29th 2012 Contributor

Specification for fixing this issue nicely:

  • Anytime in Piwik reports or admin

    After the installation when using Piwik, when the Host is different from recorded PiwiK URL, display a yellow warning, that warns users about possible Host hijack, and link to edit the hostname (to make migration still easy for users).

    • Proposal message in warning box, to recommend not to access Piwik from a hostname you do not trust as it could have serious security implications + text to fix the setting if legitimate host change:

    You are now accessing Piwik from http://injected-host/path/piwik, but Piwik has been configured to run this address: <a>http://valid-host/path/piwik</a>.
    (if user is super user) Piwik may be misconfigured (for example, if Piwik was recently migrated to a new server or URL). You can either Use $injected-host as the valid Piwik hostname, or go to $valid-host to access Piwik safely.
    (if not super user) <a>Click to http://valid-host/path/piwik</a> to access Piwik safely and remove this warning. You may also contact your Piwik administrator and notify them about this warning (<a href="mailto:superuser@host?subject=Piwik Hostname Message at this URL URL: click">http://$injected-host/path/piwik">click here to email</a>).

    • for super users, yellow message directly links to the setting page, replacing the current default host with injected-host, and a message is written "You are about to update Piwik URL to: http://$new-host/ - Please click Save to validate.", ie. we do ask for confirmation after clicking the link.
  • New simple Admin UI

    Allows Super user only to view & change valid Piwik hostname.

    • When storing or checking the hostname, make sure that the port is recorded as well as the host, so we alert if port only changes. Record port along with hostname only if it was set in the URL.
    • The hosts are recorded in the config file trusted_hosts[], not in the DB
    • Proposal : we could "whitelist", localhost, test
    • It is NOT safe to to whitelist hostnames without extension eg "mydev" or "server-test-001".
    • The config file currently allows to specify several hostnames:
; List of trusted hosts (eg domain or subdomain names) when generating absolute URLs.
; Examples:
;trusted_hosts[] = example.com
;trusted_hosts[] = stats.example.com
  • TODO: create a FAQ explaining how to setup more than 1 host (by editing config file).
  • Link the Super User, with a <class="form-description"> to the FAQ, to let users know easily how to add more than 1 host (which will be useful in some legit cases I'm sure)
  • When config file has more than 1 hostname, in the UI we could display "The following hosts are also valid: $otherhost, $otherhost2.
  • Installation:

    add an installation test that curl's to

    • if Piwik is found, then enable this feature because Apache is arguably misconfigured; i.e., missing ServerName configuration
    • if Piwik is not found, assume Host: can be trusted as Apache is routing on the Host: header to the appropriate virtual host
  • Important security: as well as displaying notice when header is injected in UI, we should ensure that Password reset emails should only be sent with trusted hosts

See also #3220

@mattab commented on August 10th 2012 Member

Increasing priority since it has security implications and will improve general safety.

@mattab commented on October 11th 2012 Member

I updated the spec at #3080

This is high priority for 1.9.1 Must do :)

@diosmosis commented on October 22nd 2012 Member

(In [7279]) Refs #3080, added trusted host admin UI, display warning in login, normal & admin screens if hostname is not trusted, and make sure password reset is not possible if hostname is not trusted.

@diosmosis commented on October 22nd 2012 Member

My last commit does everything necessary for this ticket, only thing left is the FAQ entry and Learn more link. However, I added a description to the Trusted Hosts admin section, so maybe it's not needed anymore?

@mattab commented on October 22nd 2012 Member
  • Piwik shouldn't be allowed to run with a empty trusted_host, or this security won't be used! So whenhever Piwik is accessed with empty trusted_host AND the hostname is set AND the user is super user, then we should write the current host as trusted_host.
    • it means that, until the super user logs in in the browser, users will not benefit from this improvement. This is OK. But Super User viewing any page with the trusted_host not yet set, should set it. (since we assume super user will not access via fake host the first time post update)
    • when I updated, the UI shows no INPUT field and no trusted host. By default, the current host after update should be trusted and added to the config file automatically.
  • signature of getCurrentHost was changed but it's still called with old signature in Mail.php and Nonce.php
  • in all translation rename "Trusted Host" to "Trusted Piwik Host" I think this clarifies the idea.
  • Encountered injected hostname -> rename for non geeks, ie "Piwik was accessed from a non trusted Hostname: $hostfake " and remove the feeling of attack ? because most of the time it will not be an attack, so the UI should not be scary at any moment. Feel free to review other strings if you find other changes.
  • "Hostname" in UI renamed to "Piwik is installed at this Hostname" and display in a line rather than table.
  • The default view, for 99% of users, will have only one hostname. So, the default view should be something very simple such as:
    • "Piwik is installed at this URL: [ INPUT FIELD SHOWING DEFAULT PIWIK HOST URL ] "
    • remove intro text and move to a FAQ, because it's not good to talk about attacks or threats in the UI. The General Settings page should be simple to understand and to the point... so basically we simple tell users "Piwik is installed here [INPUT]" and then, once an attack or Server hostname change occurs, we can show the "Add button". OK?
  • UX: You can either use XX as the valid Piwik hostname --> Should be more clear and instructional maybe "Click here, then Add a new Hostname 'XXXX' if you trust it" or similar. All UIs should be clear on the next step to fix the issue :-)
@diosmosis commented on October 23rd 2012 Member

(In [7280]) Fixes #3080, add config option to disable trusted_hosts check, tweak many translations, modify UI to display one input w/ a label if only one trusted host is set (or if there's an injected host), set trusted host to Host if no stored trusted hosts and user is superuser, and don't use regex to check host.

@diosmosis commented on October 23rd 2012 Member

(In [7282]) Refs #3080, UI tweaks.

@diosmosis commented on October 23rd 2012 Member

[7283] refs this ticket, not #1823.

@diosmosis commented on October 24th 2012 Member

(In [7291]) Refs #3080, get tests to pass and use previous regex code (w/ escaping) instead of forloop.

@diosmosis commented on October 24th 2012 Member

(In [7295]) Refs #3080, fix regression in install process.

@mattab commented on October 24th 2012 Member

(In [7301]) Refs #3080

  • Make sure a Port change results in warning message
  • Disabling trusted host chek should disable the Nonce check (otherwise login will still fail)
  • Displayed message in header.tpl rather than index,tpl since other "top menus" don't display 'index.tpl'
  • Login will never work when trusted host warning is displayed (because of Nonce check) so let's be clear and suggest a clear fix for this issue
@mattab commented on October 26th 2012 Member

(In [7314]) Fixes #3478 We cannot apparently set the value of a config file section directly, it fails for some php versions which don't understand the &__get() magic function.. Refs #3080
Refactoring the two setters into one helper function

Also fixing notice in graph code

This Issue was closed on October 26th 2012
Powered by GitHub Issue Mirror