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

Download Piwik upgrade packages via HTTPS #6441

Closed
defuse opened this issue Oct 14, 2014 · 50 comments
Closed

Download Piwik upgrade packages via HTTPS #6441

defuse opened this issue Oct 14, 2014 · 50 comments
Assignees
Labels
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. Task Indicates an issue is neither a feature nor a bug and it's purely a "technical" change.
Milestone

Comments

@defuse
Copy link

defuse commented Oct 14, 2014

The one-click updater downloads the latest .zip version over from an http:// URL, which is not secure. An man-in-the-middle can inject their own code, which will be executed, as the file downloads, to compromise Piwik and the server it is running on. This can be fixed as follows:

  • At minimum, use the https:// URL, and make sure the certificate is correctly verified by the download code (builds.piwik.org already has a valid SSL cert installed, so this should be a really easy fix).
  • Better, sign each release with GnuPG, and have the updater verify the signatures using an embedded public key.
@mattab
Copy link
Member

mattab commented Oct 17, 2014

Thanks for suggestion - code signing is covered in #1757

@mattab mattab added Task Indicates an issue is neither a feature nor a bug and it's purely a "technical" change. c: Security For issues that make Matomo more secure. Please report issues through HackerOne and not in Github. labels Oct 17, 2014
@mattab mattab added this to the Mid term milestone Oct 17, 2014
@mattab mattab changed the title One Click Updater Retrieves Code in an Insecure Manner Download Piwik upgrade packages via HTTPS Oct 17, 2014
@mattab
Copy link
Member

mattab commented Nov 14, 2014

Now that our releases are signed in #1757 it would be great to also make progress on trying use SSL to download the builds (fallback http). moving into Short term.

@mattab mattab modified the milestones: Short term, Mid term Nov 14, 2014
@mattab mattab added the Major Indicates the severity or impact or benefit of an issue is much higher than normal but not critical. label Nov 17, 2014
@tsteur
Copy link
Member

tsteur commented Feb 4, 2015

@mattab can we work on this during 2.12.0? Probably too short to get it into 2.11.0?

@mattab mattab modified the milestones: Piwik 2.12.0, Short term Feb 9, 2015
@mattab
Copy link
Member

mattab commented Feb 9, 2015

+1, moved in 2.12.0

@taoeffect
Copy link

So, from skimming #1757, unless I missed something, it sounds like updates are not being signed & verified either.

Thanks for adding the Major tag to this.

@taoeffect
Copy link

This is a critical issue. @defuse pointed out that there are two things that can (and should) be done. One of them is simple, the other more difficult.

Unless there's some difficulty that hasn't been mentioned with adding HTTPS, that should be done immediately for the next update. If you can't get that in for the next update, please explain why.

@mattab
Copy link
Member

mattab commented Feb 27, 2015

We know this, it's assigned to 2.12.0 - please double check things before double commenting

@taoeffect
Copy link

We know this, it's assigned to 2.12.0 - please double check things before double commenting

What do you mean by "know this"? Which part do you know, that you need to immediately add HTTPS?

If you know that, then how have you allowed at least 8 releases since this issue was first opened and still have not added HTTPS?

@taoeffect
Copy link

I am not understanding the difficulty. What beyond the following do you need to do?

  1. Get an HTTPS certificate (you don't even need to pay for one). A basic first step would be to get one for free from StartSSL or LetsEncrypt. If you need any help with that please let me know, I'd be happy to assist you. (Edit: just noticed that @defuse says you already have one?)
  2. Add the letter "s" after the letters "http".

@mattab
Copy link
Member

mattab commented Feb 27, 2015

if it's so easy, why don't you issue a pull request? this would be great!
PS: make sure to test it, don't submit it as fast as you write comments here :-)

@taoeffect
Copy link

if it's so easy, why don't you issue a pull request? this would be great!

Because in the amount of time that it takes me to clone your repo, do a search/replace, and send you a PR, you could've already fixed this and fixed it better than I can since you know your codebase and I don't.

It would help at least to know what the difficulty is (if one exists).

@mattab
Copy link
Member

mattab commented Feb 27, 2015

it's obvious that you do not have respect for the hard work that is professional software engineering. Please comment only once per issue in the future if you can manage. We do our best with limited resources and 200+ important issues.

@taoeffect
Copy link

it's obvious that you do not have respect for the hard work that is professional software engineering.

This seems more like unexplained apathy and negligence, not "professional software engineering."

What I don't have respect for is when a project claims to "care a lot about security" and yet proceeds to ignore a warning that they are exposing all their users to dangerous malware, and for some reason can't be bothered to add an "s" after "http" to fix the problem. Or explain what the difficulty is.

@mattab
Copy link
Member

mattab commented Feb 27, 2015

Or explain what the difficulty is.

I don't think giving orders in this way on the internet is a good idea. You failed to get what you wanted, so please be patient instead :-)

@defuse
Copy link
Author

defuse commented Feb 27, 2015

This is an urgent issue, and I definitely would have expected it to be fixed before the end of 2014. It's very important to have some sort of policy in place for prioritizing security issues, so please make sure there is not a systematic problem with addressing them. That means: You'll have to pull people away from other things to address tickets like this.

It's not necessarily as simple as adding an s, since you need to confirm the SSL certificate is actually being verified with whichever API you're using, but that's a simple test, and you need to ensure there is policy in place for keeping the SSL on builds.piwik.org up to date.

However, I see refusing to prioritize this as negligent, and the responsible thing to do at this point is to post it to Full Disclosure so that people are at least aware of the problem and can work around it themselves.

@defuse
Copy link
Author

defuse commented Feb 27, 2015

Sometimes it just isn't possible to prioritize a security issue. Other things can be more important. If that's the case, an explanation, which is all @taoeffect asked for, is reasonable.

@bglxx
Copy link

bglxx commented Feb 27, 2015

I think the code changes to HTTPS are the minor part. There must be compatibility to older versions using HTTP - needs additional time.

And they need a couple of hours to configure and test the webserver. Changing to HTTPS is much more than the "s" in the URL. We didnt have a better security if it is not done with care.

We have changed our website to https: approx 1 hour for html/php, but we need more than half a day day for configuring, testing and so on. Security changes needs to be done correcty or it is nothing more than cosmetics.

@mattab
Copy link
Member

mattab commented Feb 27, 2015

However, I see refusing to prioritize this as negligent, and the responsible thing to do at this point is to post it to Full Disclosure so that people are at least aware of the problem and can work around it themselves.

Guys please don't need to comment on this issue it does not help - it was already assigned to the next version milestone 18 days ago!

FYI: The SSL certificate is already in place for few years and will stay here of course. We will need support 3 http libraries (curl, socket, fopen) which we'll need to test for, as well as implement and test the fallback to http, as well as write automated tests. We also would need to report in the System check whether the downloads are done over https or http to inform users. it will be done in the next release as you can see the milestone set (and our version milestones have a approx monthly release cycle).

@taoeffect
Copy link

Thanks for providing more info on what's involved in fixing this issue, @mattab.

Regarding:

as well as implement and test the fallback to http

Doing that would be to not fix the issue. If all an attacker has to do is block HTTPS to be able to MITM, it's as if you did absolutely nothing to protect your users, so please don't do that.

mnapoli added a commit that referenced this issue Mar 16, 2015
@mattab
Copy link
Member

mattab commented Mar 16, 2015

I'm releasing 2.12.0-b4 which includes this code - then we will release 2.12.0-b5 to check the update works well over HTTPS

@mattab
Copy link
Member

mattab commented Mar 17, 2015

It's working, well done! 👍

FYI here is how the HTTPS -> HTTP fallback screen looks like: https://github.com/piwik/piwik-ui-tests/blob/master/CoreUpdaterCode_httpsUpdateFail.png

@mattab mattab closed this as completed Mar 17, 2015
@defuse
Copy link
Author

defuse commented Mar 17, 2015

@mattab Awesome!

I would change the verbiage on the fallback page to mention the possibility of an attack (and, specifically, that if an attacker is intercepting the connection, they could take over your system by inserting malicious code into the download). As it stands, there's no mention of an attack at all. It's implied by the HTTPS error, but maybe that's not so obvious, and might be lost on a less-knowledgeable user.

@taoeffect
Copy link

+1 to what @defuse said.

Also, "network error" and "slow network speeds" are not reasons to "OK" an HTTPS failure.

The only legit error for HTTPS is if the system doesn't support it (which ... never happens on modern systems). True, it's possible that the piwik server has some sort of config problem with its certificate, or nobody remembered to update the certificate. Those errors, however, should be treated as attacks.

Users must not be given a "continue anyway" button to click on in any circumstance in other words (at least without being forced to type into a text field, "I do not care if malware gets installed on my server, download anyway." Or something comparably challenging.)

@mattab
Copy link
Member

mattab commented Mar 17, 2015

Agreed, we will add a sentence to explain the risk of an attack, it makes sense.

We'll leave the rest as it is though, because it's clear and if users care about having a secure Piwik thethey will never click on http upgrade. For others who don't care as much (maybe 95% of users don't care about being target of mitm), these guys are still of utmost importance to us, and we want them to upgrade their Piwik in one click as before 2.12.0.

The security risk factor of running an old out-of-date Piwik version is much higher than risk of having its Piwik Mitm and tampered with... The actual risk of Mitm is quite low due to higher complexity of establishing Mitm, but Piwik likely will have critical security bugs fixed in next versions (though we run a successful bug bounty program)...

I know you guys may disagree with our "laxed" approach here, but overall we are happy with striking the balance.

@mattab mattab reopened this Mar 17, 2015
@mnapoli
Copy link
Contributor

mnapoli commented Mar 17, 2015

@taoeffect I agree we should warn about the possibility of an attack.

However please be aware of this (that I explained already in the pull request): the download via HTTPS can fail because of a network error, timeout, etc. and we have no way to differentiate that from a HTTPS certificate verification error.

So now you understand that 99.9% of the people seeing this screen will simply have a random download error. We should make clear about the risk of an attack, but we shouldn't bee too much alarming or we risk panicking thousands of people over a simple network timeout. We need to find the correct balance.

@taoeffect
Copy link

Remember that users always have the option to download piwik from your website and upgrade manually that way.

@mnapoli wrote:

the download via HTTPS can fail because of a network error, timeout, etc. and we have no way to differentiate that from a HTTPS certificate verification error.

If it fails because of "network error" or "timeout", then HTTP should fail for the exact same reason.

There is no situation where HTTPS will fail for one of those and HTTP will not fail, unless an attack is occurring, or a hosting provider is blocking HTTPS, which they shouldn't be. If that's taking place, the correct course of action is not to allow HTTP, but for the user to switch to a different hosting provider and perhaps sue their previous one.

We need to find the correct balance.

What you are doing here is not about balance. There is no legitimate technical reason to allow HTTP. Allowing HTTP is to make it simple for malware to infect the system, period. As the screen is designed right now, I bet that I could still infect 99% of piwik users because they will simply click the "Update Automatically" button, and if that is the case you have achieved little with this fix.

Myself and @defuse will know not to click on that button, but what about the rest of your users?

@taoeffect
Copy link

My girlfriend was curious as to what sorts of damage an attacker could do thanks to your platform, so, here is a small list:

  • They would use Piwik to disseminate malware to visitors of websites that are using Piwik, thereby infecting not only the site itself, but the computers of anyone who visits them.
  • They could deface websites and get them to show whatever they want.
  • They could stay hidden in the background and install backdoors on the server that would allow hackers to come in and steal all data off of the server (passwords, private data, etc.)
  • They could create a bot network and perform DDoS attacks against random websites.
  • EDIT: the NSA/GCHQ or some other intelligence agency could use it to expand their network of mass-surveillance technologies.

etc. etc. etc.

Why are you allowing this...?

@defuse
Copy link
Author

defuse commented Mar 17, 2015

When in doubt, run a usability test. It's never a bad thing to do to give that page to 5-10 people and ask them what they think it means and what they would do (and it's sometimes surprising the effect a little change of wording can have).

@defuse
Copy link
Author

defuse commented Mar 17, 2015

Agreed on the technical point of it not being due to a timeout or network error.

How about:

We could not make a secure connection to the download server. This could mean that your connection is under attack and someone is trying to replace the update with a malicious (backdoored) version of Piwik. It could also mean that there is something wrong with the Piwik download server, or that your system does not support making secure connections.

Please try again later, or update Piwik manually.

[I'll try again later.] - green button
[Update Piwik Manually] - green button
[Update over an insecure connection (DANGEROUS)] - red button

edit: added a button

@taoeffect
Copy link

The actual risk of Mitm is quite low due to higher complexity of establishing Mitm

Depending on who you are, this attack is either simple or difficult. You'd need to position yourself somewhere in front of builds.piwik.org (by hacking into your hosting provider, or whatever your hosting provider is connected to). The NSA/GCHQ/etc can do this in their sleep, but it's possible for even a lone, skilled individual to pull this off as well.

The reward for doing this is huge.

(though we run a successful bug bounty program)...

Can I send in this bug to the bounty for $$ then? ;)

@mnapoli
Copy link
Contributor

mnapoli commented Mar 18, 2015

Thanks for the feedback, I have updated the screen following discussions with @mattab:

coreupdatercode_httpsupdatefail 1

@taoeffect
Copy link

That's definitely an improvement.

How are you checking whether there is an update available btw? Is that done over HTTP or HTTPS?

Cause if it's done over HTTP, I will simply say there's a new version available, prevent HTTPS, and wait for the user to click the second button to update over HTTP, at which point they will have malware on their system.

If I was the attacker, I'd target about 10% of piwik users with that technique (making sure that @defuse and @taoeffect's servers are not targeted) and I'd probably infect at least 95% of those users, giving me a good sized bot network to do whatever I want with. 😄

@gaumondp
Copy link

About the labels of buttons from last mockup, few ideas :

  1. Shouldn't be "Retry update" ?
  2. "Insecure, continue using HTTP" .maybe add a link/popup with "Why doing this a bad idea?" and reuse @taoeffect text.
  3. "Do not update, continue to Piwik"

@mattab
Copy link
Member

mattab commented Mar 18, 2015

How are you checking whether there is an update available btw? Is that done over HTTP or HTTPS?

The check for new version is done over HTTP so far. that's maybe an issue, so feel free to create a new issue for that

@joepie91
Copy link

joepie91 commented Apr 1, 2015

@mnapoli The button styling in that screenshot is not sufficient. The 'non-secure manual update' button should be red, while the others should be some other non-cautionary color, such as green.

The color distinction is important to make people think twice about the button they are clicking.

EDIT: Here is a good resource from the Chrome team on designing correct "security error" pages.

@mnapoli
Copy link
Contributor

mnapoli commented Apr 1, 2015

@joepie91 in Piwik buttons have only one color for now, they are all red :) Changing that could be done but not in the scope of this issue.

@joepie91
Copy link

joepie91 commented Apr 2, 2015

@mnapoli I don't see how that is not in scope of this issue? Correct error UX is a fundamental part of error handling for SSL connections - and updates over SSL are what the issue is about.

@mattab
Copy link
Member

mattab commented Apr 2, 2015

Firstly this issue is already closed, so it cannot have a scope. Secondly it is our practise to have separate ideas in separate issues. So if you care about that you can create an issue and we can discuss it there.

@joepie91
Copy link

joepie91 commented Apr 2, 2015

Done.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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. Task Indicates an issue is neither a feature nor a bug and it's purely a "technical" change.
Projects
None yet
Development

No branches or pull requests

9 participants