oortcloud / node-ddp-client

A callback style DDP (Meteor's Distributed Data Protocol) node client.
Other
263 stars 80 forks source link

Meteor Accounts is moving away from login with SRP #39

Closed emlama closed 10 years ago

emlama commented 10 years ago

I just noticed this while reading the 0.8.2 release notes, but am not sure if this is going to affect how this package works. Any thoughts on how this will affect node-ddp before I upgrade to 0.8.2?

emgee3 commented 10 years ago

I did a little testing and the 0.8.2 release will break logging in from the current version of the ddp client. Refactoring out the SRP package is pretty straightforward — the bcrypt password storage is mostly a server-side change — the client just has to SHA256 the password, which is simpler than the current SRP challenge-response.

emgee3 commented 10 years ago

As a note, even though the SRP needs to be refactored, you can log in using:

ddpclient.call("login", [
    { user : { username : "username" }, password : "password" }
  ], function (err, result) {
    ...
vsivsi commented 10 years ago

I can verify that the above workaround works for the current release-0.8.2 branch of meteor. It also works for email based login:

ddpclient.call("login", [
    { user : { email : "user@domain.com" }, password : "password" }
  ], function (err, result) { ... });

Adding SHA-256 support should be trivial (rather than sending the plaintext password), but I'm dubious about the value of this. It seems sort of pointless without the use of Salt, since it will be vulnerable to replay attacks and/or super-fast password cracking when not used over SSL. And if you are using SSL, then you shouldn't really need this... Seems like Meteor is watering down the password security in order to drive installations to use SSL (for authentication at least). Or is there some fundamental vulnerability in SRP that I'm unaware of? I know there was some controversy back when the SRP decision was announced, but I don't recall why and can't immediately find a link to it.

The thing that's a drag about this is that once SRP is refactored out of meteor-ddp, dependent packages that need to work with both older and newer Meteor server versions will be in a bit of a pickle.
I'm adding code right now to the ddp-login npm package to add the fallback method as an affirmative option.

vsivsi commented 10 years ago

It looks like the plaintext password method should work with all 0.8.x servers (haven't tested with older). So that seems to be the least common denominator.

Adding to the "replay attack" concern above: Meteor authTokens have always been vulnerable to this when SSL isn't used... Which isn't great, but at least authTokens are easy to expire/revoke and can't be reversed back to a password using dictionary + brute-force methods.

emgee3 commented 10 years ago

On the ddp-login package, that's nice @vsivsi. I've been toying with the idea of a major cleanup of the ddp client once mrt is deprecated. Being able to refactor out all of the login logic would be nice.

And I agree on the whole SRP vs bcrypt, MDG is saying you just need to use SSL.

vsivsi commented 10 years ago

Also: the nodejs crypto module has built-in support for SHA-256

crypto = require('crypto');
hash = crypto.createHash('sha256');
hash.update('password','utf8');
digest = hash.digest('hex');
emlama commented 10 years ago

Well this is all great :+1:

Thanks for maintaining this package!

emgee3 commented 10 years ago

Oh, forgot to mention, the ddpclient.call('login' ... technique above works all the way back to version 0.6 of Meteor and possibly 0.5.

vsivsi commented 10 years ago

@emgee3 Do you know how far back the sha-256 digest support goes? Is that new or has it been there all along?

emgee3 commented 10 years ago

The SHA-256 is in 0.8.2 for the first time. Fallback for plain text has been around for ever.

vsivsi commented 10 years ago

Okay thanks. Here's a wrinkle I'm seeing:

However, SHA256 digest login fails with:

{
   "error":400,
   "reason":"old password format",
      [...]
   "message":"old password format [400]",
   "errorType":"Meteor.Error"
}

Unless that user account has been authenticated at least once using the Meteor client. In other words, neither the "plaintext" nor "digest SHA256" methods will transition an old account from SRP to bcrypt, and the "digest SHA256" method won't work until the account has been transitioned.

Also, for the "digest" method to work at all, they must be bcrypting the SHA256 digest of a password, and not the password itself, right?

emgee3 commented 10 years ago

Oh, I didn't notice that it's not transitioning the password after a plaintext login. But you're correct, that is what's happening.

For digest, yes, they are bcrypting the SHA256 digest as opposed to the actual password.

vsivsi commented 10 years ago

FYI, I've implemented the entire new login scheme in ddp-login v1.0.0. Just working up the unit tests now. It depends on node-ddp-client for the connection and ddp.call() but nothing else. So if you want to strip out all authentication functionality and point users to ddp-login for that, I'd be fine with it. I still need to change the unit-tests for the new scheme, but it should be ready to go by tomorrow.

vsivsi commented 10 years ago

I just ran across this discussion of the motivation for this change and lots of details. https://meteor.hackpad.com/SRP-bcrypt-J5mdBojeVfe

Also, for anyone who wants to try it out, the new version of ddp-login with updates for Meteor v0.8.2 (including the use of the SHA256 digest) is here: https://github.com/vsivsi/ddp-login/tree/meteor-0.8.2-auth

I'm still looking into the possibility of triggering the conversion from SRP accounts to Bcrypt remotely. I think it should be possible.

Other than that, it's in pretty good shape, but I won't push it to npm until MDG merges release-v0.8.2 to the master branch.

emgee3 commented 10 years ago

I just published node-ddp-client version 0.7.0 which removes built-in authentication support. As discussed, I'm referring people to https://github.com/vsivsi/ddp-login as an alternative.

vsivsi commented 10 years ago

I just bumped the ddp-login package to v1.0.0 with full support for Meteor v0.8.2 bcrypt accounts, including auto account migration from SRP to Bcrypt style authentication on first login and optional plaintext authentication for backward compatibility with older servers.

xanatas commented 9 years ago

Can you maybe explain why (do you know why) no salt is being used? I got this possible attack vector regarding from a friend regarding securtiy and salt:

1- Login to the application 2- Intercept the request 3- Observe the SHA 256 Hash digest 4- Now use any online SHA 256 decoder and found the cracked hash

Just how possible is this?

vsivsi commented 9 years ago

It's actually worse than that. All you need to do is send the same intercepted password hash to re-authenticate as that user (no hash reversal required). Meteor (and by extension its authentication package) is really only secure over SSL.

I can't speak for the MDG, but the old SRP scheme was complicated and didn't really increase security much because presumably if you can intercept the authentication exchange, you can also intercept (and replace) responses to the browser HTTP requests that load the javascript code implementing the client-side of the exchange. And of course that would give you access to everything including the plaintext password.

So HTTPS / SSL connections are the only real answer for a system like Meteor, and IMO the MDG finally acknowledged that when they made the switch to SHA256 in 0.8.2.

You can read a bit more about this here: https://www.npmjs.com/package/ddp-login#security

xanatas commented 9 years ago

Thanks for clearing that up! Im no expert but would a salt not make in more secure then or is this not possible because of the server side end?

vsivsi commented 9 years ago

Without SSL, the Meteor authentication token, which is issued as a result of a successful login, is also sent back and forth in the clear. Possession of this token allows the attacker to fully impersonate a given user, even with no knowledge of their password.

The only benefit of salt is it makes it much more difficult to use pre-computed tables to reverse a hashed password back into the plaintext. Salting passwords (along with expensive hashing functions like bcrypt) helps a lot to protect them from being reversed when the authentication database from a server is compromised. Meteor does both of those things on the server-side.

But in this case, salting the password during the exchange does very little to help because a "Man-in-the-middle" attacker with access to the unencrypted communications between browser and server can simply substitute or add any javascript code he wants to the Meteor client bundle. Among other things, that code can send plaintext passwords the user enters to a server anywhere on the internet. So this is the weakest link, and the only practical way to defend against it is to use SSL. Also, users shouldn't be reusing passwords between different services, but that's a different issue...