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
Comments
Thanks for suggestion - code signing is covered in #1757 |
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 |
@mattab can we work on this during 2.12.0? Probably too short to get it into 2.11.0? |
+1, moved in 2.12.0 |
So, from skimming #1757, unless I missed something, it sounds like updates are not being signed & verified either. Thanks for adding the |
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. |
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? |
I am not understanding the difficulty. What beyond the following do you need to do?
|
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). |
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. |
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. |
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 :-) |
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 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. |
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. |
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. |
Guys please don't need to comment on this issue it does not help - it was already assigned to the next version milestone 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). |
Thanks for providing more info on what's involved in fixing this issue, @mattab. Regarding:
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. |
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 |
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 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. |
+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.) |
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. |
@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. |
Remember that users always have the option to download piwik from your website and upgrade manually that way. @mnapoli wrote:
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.
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? |
My girlfriend was curious as to what sorts of damage an attacker could do thanks to your platform, so, here is a small list:
etc. etc. etc. Why are you allowing this...? |
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). |
Agreed on the technical point of it not being due to a timeout or network error. How about:
edit: added a button |
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.
Can I send in this bug to the bounty for $$ then? ;) |
Thanks for the feedback, I have updated the screen following discussions with @mattab: |
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. 😄 |
About the labels of buttons from last mockup, few ideas :
|
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 |
@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. |
@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. |
@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. |
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. |
Done. |
The one-click updater downloads the latest
.zip
version over from anhttp://
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: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).The text was updated successfully, but these errors were encountered: