nw-updater icon indicating copy to clipboard operation
nw-updater copied to clipboard

Security of packages

Open sashahilton00 opened this issue 11 years ago • 16 comments

Just noticed that the packages are being delivered over HTTP, which makes sense, but there is no verification of the actual file that is delivered. This could lead to MITM attacks, where the attacker loads his own package into the updater, then intercepts other computer's traffic and redirects to his updater, which, once installed, gives him full access to the user's computer. May I suggest that we use the Crypto plugin (http://nodejs.org/api/crypto.html) to implement DSA signing of packages, which ensures they are the original package, with the advantage of it still being possible to deliver it securely over HTTP. This feature is an absolute must if this updater is to ever be implemented as a service/process, and certainly in a program with many users.

sashahilton00 avatar Oct 07 '14 00:10 sashahilton00

Is a nice idea- though wouldn't using SSL provide the same security without the need to generate any app specific keys etc?

4ver avatar Oct 07 '14 09:10 4ver

Yes it would, though to a lesser extent (256bit vs. 2048bit) and it would also require people to buy SSL certificates, whereas the DSA signature is free and not subject to the same warnings/errors abt. Self signed certificates. Also, unless the HTTPS uses Extended Validation, it is possible to forge HTTPS certificates. With DSA, short of brute forcing the key, there is not much one can do to crack it. Though if we were to use HTTPS as opposed to DSA, then I suggest we implement HTTPS fingerprinting...

sashahilton00 avatar Oct 07 '14 09:10 sashahilton00

I'm using node-webkit-updaterto create a desktop variant of Cryptocat (ticket), hence this issue is of high priority for me (I'm no security expert).

Let's assert that the most up-to-date application's package.json - which includes needed update manifest data - is hosted within a GitHub repository (requested via https://raw.githubusercontent.com/REPO/package.json) and releases are then downloaded via https://github.com/REPO/releases/download/VERSION/APP.zip.

How secure is this scenario with node-webkit-updater (if HTTPS fingerprinting ev. gets implemented)?

majodev avatar Nov 10 '14 12:11 majodev

@majodev If it has EV, then it is pretty secure. The weakest link in this scenario would be if someone got hold of your git password and used it to upload a malicious version. This is why I suggested the DSA on top of HTTPS; even if someone gets hold of your password for git by some other means, as long as you generate to private key on your computer, make it a decent length (2048/4096 bit), and never transfer it over the internet, only store it on your computer/USB stick, then even if your git is compromised, the attacker will still be unable to push malicious software, as the package will need to be cryptographically signed with the private key that you hold in a safe place.

sashahilton00 avatar Nov 10 '14 14:11 sashahilton00

@sashahilton00 OK, big thanks for providing this important information!

I think the Sparkle Update Framework handles signing + checking in a same manner as described by you.

So, to accomplish signed updates, a whole deployment process may look like this (given that someone actually contributes DSA checking capabilities to node-webkit-updater):

  1. Borrow Sparkles' sh scripts generate_keys.sh and sign_update.sh.
  2. Use generate_keys.sh to generate public (dsa_pub.pem) and private (dsa_priv.pem) DSA 4096bit key files.
  3. The file dsa_pub.pem must always get redistributed with the app binaries (hence include it within your node-webkit-builder task).
  4. The (securely kept) private key dsa_priv.pem will be used to sign the app's .zip via sign_update.sh. The generated value of this script must be included within the manifest update data (see the example update entry below).
  5. node-webkit-updater get's the DSA key from the manifest and downloads the new app .zip.
  6. Before unpacking the downloaded .zip, the DSA key entry from the remote manifest must be validated against the redistributed dsa_pub.pem and the newly downloaded .zip.
  7. If OK, proceed with unpacking + installing.

Example update entry with DSA keys in manifest:

"packages": {
  "mac": {
    "url": "http://localhost:3000/releases/updapp/mac/updapp.zip",
    "dsa": "MCwCFFPIOejIR0uCackc1QcthLju/apTAhQed6mytl+bddBn+fqSRBAWL7dnCg=="
  },
  ...
}

Right? I really like this discussion so far. :+1:

majodev avatar Nov 10 '14 16:11 majodev

@majodev Yes, that is pretty much the flow that I was talking about, and this is pretty much the workflow I use for applications with a built-in updater. This is probably unnecessary, but if you want to safeguard it further, you might want to include the dsa_pub.pem in the actual zip update, so that if the private key was ever to be compromised, then you could hopefully push out a new public key before any damage was done, thus making the compromised private key useless, though this may be a bit extreme. The solution you have proposed above is still very secure. If you want to make it slightly more secure, I would suggest also including the file checksum in the manifest and to verify that also, though it is almost impossible that two different files could share the same signature, so this is probably unnecessary too.

sashahilton00 avatar Nov 10 '14 17:11 sashahilton00

If anyone is interested in contributing this to node-webkit-updater (or securing your own application), I've implemented DSA signing and verifying today in my fork of Cryptocat.

The DSA Signature gets verified after node-webkit-updater has successfully downloaded an update. The process is very straightforward (but currently synchronous) done here.

How-to:

  • Use bin/generate_keys.sh from Sparkle to generate your private and public key files
  • Bundle the public key file dsa_pub.pem with your applications.
  • Add a verification step after an update is completely downloaded (like here and here).
  • After packaging: Use bin/sign_update.sh (again from Sparkle) to get the DSA signatures of your zipped apps.
  • Host the urls of your zipped updates + its DSA signatures the within your remote package.json (e.g. like here).

majodev avatar Nov 12 '14 20:11 majodev

Looks good, are you planning on integrating it into his repo?

sashahilton00 avatar Nov 12 '14 23:11 sashahilton00

@sashahilton00 Currently not, but I'll change my mind maybe if I have some free time in the future (and nobody had time to contribute it already). To integrate it here, it'll have to be fully test covered, more decoupled and well documented, also I have no idea if this kind of (optional?) security checking is in the interest of @edjafarov. Furthermore I've had problems running the tests of this repo on my current machine, hence I don't want to push anything that has flaws / I can't guarantee is working.

majodev avatar Nov 13 '14 12:11 majodev

node-webkit-updater gives low level api to update apps. I am not sure we should add features in package. This could be implemented separately like @majodev did. Though I think this thread is really awesome and should be highlighted in readme/wiki. @adam-lynch what do you think?

edjafarov avatar Nov 13 '14 13:11 edjafarov

I also felt that providing such a implementation is a bit over the top within this package. It depends on too many side factors + I don't want to force users to use DSA only. Furthermore, the most comfortable way to compute these DSA signatures is during a product's build-task (e.g. after bundling via node-webkit-builder and compressing the new releases via grunt-contrib-compress) + appending them to package.json (or update-manifest) you are about to host publicly. However, this is totally out of the scope of this package.

@edjafarov One thing that would be "nice-to-have" is exposing your platform-variable (e.g. a public getter) from your app/updater.js, so I don't have to borrow it like here. This way one could easily provide more sub-fields (e.g. my dsa at the mac and winsubfield within the packages-field like here) in the update-manifest and safely get the additional needed ones from the remote manifest during the update procedure.

majodev avatar Nov 13 '14 13:11 majodev

@sashahilton00 @majodev Popcorntime seem to have implemented dsa signing of packages in their updater. Copy, paste, pr :smile:

https://git.popcorntime.io/stash/projects/PT/repos/popcorn-app/browse/src/app/lib/updater/index.js#137

4ver avatar Dec 02 '14 23:12 4ver

They have, I have actually done a bit of work on that updater myself actually :) The entire updater is a bit buggy, but the verifying part is fine, I will take a look at making a PR if I can find the time, applying to Uni atm.

sashahilton00 avatar Dec 02 '14 23:12 sashahilton00

@sashahilton00 I'd love to pick your brain on that updater (about the other steps). I just had a look around too.

adam-lynch avatar Dec 03 '14 00:12 adam-lynch

What would you like to know? (I didn't do all of it, it was a collaborative effort, but will try to answer as best I can :)

sashahilton00 avatar Dec 03 '14 00:12 sashahilton00

@sashahilton00 see #63. Thanks! :smile:

adam-lynch avatar Dec 03 '14 00:12 adam-lynch