-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Signing appcasts #971
Comments
Our main focus is to protect against installation of unauthorized code, and for that particular purpose signed appcast doesn't offer significant improvement. We already support multiple checks for the payload: HTTPS + DSA + Apple Code signing. Using the same keys for the appcast wouldn't create any new security barrier (i.e. if author leaks their DSA private key and HTTPS is compromised, then the attacker will replace the appcast as easily as the payload). |
Thanks, @pornel -- so would a PR be considered because you just don't have time to work on stuff that isn't your main focus? Or would you not want one because of the additional complexity it would entail (for the code and for application authors)? One reason for using signed appcasts is that they can make sure that updates are presented to the user properly. A forged appcast can prevent a user from being presented with a critical update, or it can include release notes with malicious links like "Don't click the 'Update' button, instead, please click here to update"). Signed appcasts can also allow an author to include Javascript in the update notes without it being a security risk. Signed appcasts would have mitigated the problems associated with not using https: that caused such an uproar a year ago (or whenever it was), too, but of course that's 20-20 hindsight. |
I think signing appcasts could be valuable. However it should be an option developers would have to opt into, and we would still want to be secure as possible when developers don't sign the appcast. So there would be an Info.plist key/value in the bundle that specifies the feed should be signed. (It would be inconvenient if we required appcast signing). Do you download another file from a different URL to obtain the signature of the appcast, and then validate it? Or do you do it some other way? What I am concerned at the moment is maintainability and merging in new features at the moment, whilst I'm trying to merge changes from my fork at this time. |
Also what if the appcast has external file references (release notes, images, ..)? Do we just not care, or take precautions around that? |
@zorgiepoo - Thanks. An optional Info.plist key/value pair (boolean SUAppcastMustBeSigned, defaulting to false) is fine with me. If true and if the appcast's signature wasn't correct, the appcast fetch would fail hard. If not present, or if false, no signature checking would be done. Note that this is a dangerous option -- if you lose your private key you won't be able to create a new appcast with display notes such as "Please download a copy of XXX from the website", which you could do if appcasts aren't signed. All you can do is create an appcast that causes an error. The error message for an invalid appcast signature should mention that as a possibility. Regarding the mechanism of the signature, Tunnelblick calculates the signature of the appcast, then prepends an HTML/XML comment which includes the signature to the appcast. The start of a Tunnelblick appcast looks like this:
When my patched Sparkle gets the appcast, it removes the comment and checks the rest of the appcast against the signature. How might signatures work for dynamically-generated appcasts? The webserver would have to sign them on-the-fly, which means the webserver would need to have the private DSA key available, which is a really bad idea because of the public-facing nature of a website. So although they could, I don't imagine anyone would want to do that. I don't think the Sparkle developers should care about external links in an appcast. An appcast does not need to have external references except the update link [1] . Everything else is up to the appcast's author. The release notes, for example, can be inlined so they are protected by the appcast's signature, and so could images. Sparkle requires the appcast; that's why I think it should be protected. [1] I'm not sure if the update link is actually required. If missing, Sparkle might reasonably show the update notes with a disabled "Update" button. The update notes could have a link to a download or something if the author wanted. That wouldn't be very user-friendly, but it might be better than complaining (if that's what Sparkle does now). |
There is no absolute security, and we have to choose which attacks we protect from, and which we give up on. We face trade-offs in usability, reliability and complexity of updating. For example, we don't guarantee that the app will be updated. An active MITM can prevent any updates from being shown at all by breaking the network at any level, and the breakage can be done in a way that's indistinguishable from common failures (e.g. an attacker can simulate a captive portal, firewall, etc.). If we took seriously a scenario that an out-of-date app may contain exploitable vulnerabilities, we'd have to assume that any problem with the appcast or network connectivity can be a sign of an attack in progress that has exploited or is about to exploit the host app, so the app would have to be immediately stopped or even deleted. Thats obviously a very drastic user-hostile measure for an unlikely attack, so we choose not to protect from it.
We rely on HTTPS to protect against that on the network level, and I think that's perfectly adequate — our threat model is "not-Mossad". If we chose to fight adversaries that can hack/infiltrate/intimidate a CA, we'd first have to worry that it's even easier to do the same to the developers. The CA system is frighteningly large and only as strong as the weakest link, but realistically it's probably still the strongest link in Sparkle. We tell developers to keep the private DSA key, without a passphrase, on disk. The Flash plugin has privileges to sign Sparkle updates. |
One scenario that I think is realistic, and that we could protect from is when the web server is compromised, but developer's computer is not. In such case signed appcast would prevent phishing. However, I think that's still a low priority, because a phishing attack is the least effective way to exploit the server. It massively increases chance of getting compromise discovered and killing the golden goose for the attacker. It's much better to backdoor regular download archives (and the DSA pub key in them) and stay under the radar for as long as possible. AFAIK that how it's been when Transmission got hacked, so Sparkle's security turned out to be good enough. |
@pornel - Thanks. If you can't get a validly-signed appcast, then, as you say, it could be a network problem or it could be an attack. But if you do get a validly-signed appcast, you know whether or not an update is available with a bit more assurance that you would if it is not signed. It's a low priority, though, so I'll skip making a PR for now. |
Afaik, signed appcast should be useful excactly in the scenario where the server is compromised but not the developers computer. I personally think as a software update framework, our security shouldn't rely on whether it is served over http or https. It should be agnostic. I vaguely recall reading about the "Software Update Framework" online which went into more greater detail. |
Not even that. If there's no expiration date in the signature, an attacker could have saved an old version of your signed appcast and perform a replay attack (and if you do have an expiration date, the attacker may spoof NTP server response and change system clock to allow an expired one :) |
I'm warming up to the idea, but I'd like to think this trough to avoid making it a problem for developers.
|
I'm thinking:
|
I'd like to bump this issue in light of the Handbrake breach, which circumvented Sparkle through social-engineering (emphasis mine):
Source: https://panic.com/blog/stolen-source-code/. A signed appcast may have blocked off this attack vector by preventing the attacker from adding this note, assuming the update signing keys weren't compromised (which, judging by the attack method, seems to be the case). |
pornel wrote:
I modified the Tunnelblick implementation to include an expiration date (protected by the signature). Tthat means the appcasts have to be "refreshed" with a new signature before they expire. Other developers could set the expiration far into the future, I suppose, to avoid that, if they aren't concerned with long-term replay attacks. I'm not particularly concerned about NTP spoofing; really paranoid folks don't use NTP or have very tight controls on what time deltas are allowed. |
Adding this to future milestone as it's a potential interest for exploring. It's not an easy enhancement. It should be easy to use, be safe to opt into, and offer more security. There are several desirable constraints I can think of, many are from this discussion:
I'm not too concerned about expiration dates. We need to continue leveraging not allowing downgrades or showing updates that have already been installed. |
In Issue #962, @pornel said "We don't sign appcasts and don't intend to".
Can I ask why you "don't intend to"?
Signed appcasts would allow Sparkle to trust them (although Sparkle must still protect itself from authors' errors, of course). (This was just mentioned in #969.) It's easy to script the creation of the appcast so it gets signed. I do that in Tunnelblick, and generate_appcast (PR #953) could presumably be modified to sign the appcast.
I can create a PR to check the appcast signature if there is any interest but if there is some reason the Sparkle developers won't consider it I won't bother.
Would a PR be more acceptable if it only required signed appcasts if some flag was set by the application? (Perhaps via an "appcastMustBeSigned" delegate?)
The text was updated successfully, but these errors were encountered: