Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
robocoder opened this issue Apr 3, 2012 · 20 comments
Assignees
Labels
Bug For errors / faults / flaws / inconsistencies etc. c: Security For issues that make Matomo more secure. Please report issues through HackerOne and not in Github. Major Indicates the severity or impact or benefit of an issue is much higher than normal but not critical.
Milestone

Comments

@robocoder
Copy link
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
Copy link
Member

mattab commented Apr 4, 2012

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
Copy link
Contributor Author

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
Copy link
Contributor Author

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

@mattab
Copy link
Member

mattab commented Apr 30, 2012

Well done vipsoft, excellent improvement! :)

@mattab
Copy link
Member

mattab commented Apr 30, 2012

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

@mattab
Copy link
Member

mattab commented May 7, 2012

Is there any other work appart from updating FAQ?

@mattab
Copy link
Member

mattab commented May 29, 2012

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

@robocoder
Copy link
Contributor Author

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&lt;/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&lt;/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: 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

@mattab
Copy link
Member

mattab commented Aug 10, 2012

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

@mattab
Copy link
Member

mattab commented Oct 11, 2012

I updated the spec at #3080

This is high priority for 1.9.1 Must do :)

@diosmosis
Copy link
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
Copy link
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
Copy link
Member

mattab commented Oct 22, 2012

  • 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
Copy link
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
Copy link
Member

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

@diosmosis
Copy link
Member

[7283] refs this ticket, not #1823.

@diosmosis
Copy link
Member

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

@diosmosis
Copy link
Member

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

@mattab
Copy link
Member

mattab commented Oct 24, 2012

(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
Copy link
Member

mattab commented Oct 26, 2012

(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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug For errors / faults / flaws / inconsistencies etc. c: Security For issues that make Matomo more secure. Please report issues through HackerOne and not in Github. Major Indicates the severity or impact or benefit of an issue is much higher than normal but not critical.
Projects
None yet
Development

No branches or pull requests

3 participants