Opened 2 years ago

Closed 18 months ago

Last modified 18 months ago

#3080 closed Bug (fixed)

Record known "Hostnames" for improved security + Warn when hostname has changed after migration

Reported by: vipsoft Owned by: capedfuzz
Priority: major Milestone: 1.9.1 - Piwik 1.9.1
Component: Security Keywords:
Cc: Sensitive: no

Description

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

Change History (25)

comment:1 Changed 2 years ago by matt (mattab)

  • Milestone changed from 1.x - Piwik 1.x to Feature requests

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?

comment:2 Changed 2 years ago by vipsoft (robocoder)

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.

comment:3 Changed 2 years ago by vipsoft (robocoder)

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

comment:4 Changed 2 years ago by matt (mattab)

Well done vipsoft, excellent improvement! :)

comment:5 Changed 2 years ago by matt (mattab)

  • Milestone changed from Feature requests to 1.7.x - Piwik 1.7.2

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

comment:6 Changed 2 years ago by matt (mattab)

  • Milestone changed from 1.7.x - Piwik 1.7.x to 1.7.2 - Piwik 1.7.2

Is there any other work appart from updating FAQ?

comment:7 Changed 23 months ago by matt (mattab)

(In [6370]) Refs #3080

  • Enabling adding localhost / 127.0.0.1 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)

comment:8 Changed 23 months ago by matt (mattab)

  • Summary changed from "Host" configuration to Record known "Hostnames" for improved security + Warn when hostname has changed after migration

comment:9 Changed 23 months ago by matt (mattab)

  • Milestone changed from 1.8 to 1.8.x - Piwik 1.8.x

comment:10 Changed 23 months ago by vipsoft (robocoder)

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@… Hostname Message at this URL URL: 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" 127.0.0.1, 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 http://127.0.0.1/piwik-path/some-static-resource

  • 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

Last edited 18 months ago by matt (previous) (diff)

comment:11 Changed 21 months ago by matt (mattab)

  • Priority changed from normal to major

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

comment:12 Changed 20 months ago by matt (mattab)

  • Component changed from Core to Security

comment:13 Changed 18 months ago by matt (mattab)

  • Milestone changed from 1.9.x - Piwik 1.9.x to 1.9 -- Piwik 1.9

I updated the spec at http://dev.piwik.org/trac/ticket/3080#comment:10

This is high priority for 1.9.1 Must do :)

comment:14 Changed 18 months ago by matt (mattab)

  • Milestone changed from 1.9.2 - Piwik 1.9.2 to 1.9.1 - Piwik 1.9.1

comment:15 Changed 18 months ago by matt (mattab)

  • Owner set to capedfuzz

comment:16 Changed 18 months ago by capedfuzz (diosmosis)

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

comment:17 Changed 18 months ago by capedfuzz (diosmosis)

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?

comment:18 Changed 18 months ago by matt (mattab)

  • 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 :-)
Last edited 18 months ago by matt (previous) (diff)

comment:19 Changed 18 months ago by capedfuzz (diosmosis)

  • Resolution set to fixed
  • Status changed from new to closed

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

comment:20 Changed 18 months ago by capedfuzz (diosmosis)

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

comment:21 Changed 18 months ago by capedfuzz (diosmosis)

[7283] refs this ticket, not #1823.

comment:22 Changed 18 months ago by capedfuzz (diosmosis)

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

comment:23 Changed 18 months ago by capedfuzz (diosmosis)

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

comment:24 Changed 18 months ago by matt (mattab)

(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

comment:25 Changed 18 months ago by matt (mattab)

(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

Note: See TracTickets for help on using tickets.