interledgerjs / ilp-plugin-xrp-asym-client

Client to Asymmetric XRP Paychan
4 stars 3 forks source link

Check if password is compromised #3

Closed dappelt closed 6 years ago

michielbdejong commented 6 years ago

it would take a lot of deliberate work to make a password that is base64url and compromised and exactly 32 bytes

Smart! :+1:

justmoon commented 6 years ago

Aside from performance issues, it doesn't seem wise to send a SHA-1 hash of all passwords to an external service. You're just giving them a list of weakly hashed passwords they can then go and steal from. haveibeenpwned.com should always return true because once you send them your weakly hashed password - if you weren't pwned before, you are now.

I could get behind @sharafian's proposal to require a random fixed-length secret. Although it sure was nice when you could use a short password for testing.

Perhaps a compromise would be to still allow the short password-like thing for mini-accounts and have this plugin require the long random-y value.

emschwartz commented 6 years ago

I could get behind @sharafian's proposal to require a random fixed-length secret. Although it sure was nice when you could use a short password for testing.

My suggestion when talking to @dappelt about this was to have a length requirement by default, but have a config option to turn that off specifically for that testing use case.

dappelt commented 6 years ago

Thanks for your input!

Before replying in detail, let me start by saying that what we are discussing here is just part of the problem. Another, and arguably more severe, problem is that mini-accounts weakly hashes (potentially bad) passwords and informs all the world how bad the password is by putting the hash in the ILP address. And if an attacker can reverse a weak hash, he does not even have to try out BTP usernames, since there are none ...

If mini-accounts would require a combination of user/pass and would do some better hashing, I would not insist so much on strong passwords ;-)


it could make sure that the user's password is exactly 32 bytes in base64url (or something similar; I've been using 16 bytes in hex). Then it would take a lot of deliberate work to make a password that is base64url and compromised and exactly 32 bytes

@sharafian. The most important lesson from the recently updated NIST password guidelines is that imposing password complexity rules, e.g. fixed-length, makes for weaker passwords. In essence, if such rules are in place, users will have a harder time memorizing their password and, hence, choose easy to remember passwords. For example, the password passwordpasswordpassword conforms to your rule, yet it is obviously not a very good password. And this is of course not the only example. In fact, I found 3.854.533 similar "easy to remember" passwords/phrases that conform to your rule in a single password dictionary. Furthermore, by mandating fixed-length passwords the password space is significantly reduced and dictionary attacks become more efficient.

What about password managers? Users with a password manager will most likely have a strong, random password regardless of any imposed password policy. On the other hand, users without password manager are unlikely to start using one just because of the password policy and will resort to easy to remember passwords.

What about base64 encoding the passwords to make them obscure? In short, it does not stop dictionary attacks. An attacker would simply recompute the dictionary. By reducing passwords to a fixed length you make it even easier for the attacker. He needs to recompute only a fraction of the dictionary.

We have the choice: We can cook up our own password policies (and fail) or follow guidelines and best practices. I vividly remember how Evan and you were discussing who's bank implements worse password policies. Let's not make the same mistake :-)

Aside from performance issues, it doesn't seem wise to send an SHA-1 hash of all passwords to an external service. You're just giving them a list of weakly hashed passwords they can then go and steal from. haveibeenpwned.com should always return true because once you send them your weakly hashed password - if you weren't pwned before, you are now.

@justmoon. Remember there is a difference between preimage resistance and collision resistance. Shattered demonstrated that SHA1 is vulnerable to collision attacks, but preimage resistance remains intact. In other words, haveibeenpwned.com cannot reverse a (strong) password from its hash and a password does not become pwned by submitting it.

dappelt commented 6 years ago

My suggestion when talking to @dappelt about this was to have a length requirement by default, but have a config option to turn that off specifically for that testing use case.

Length requirement is good, but should not be too strict. The recommendation is that passwords have at least 8 characters and the server accepts at least up to 64 character passwords.

sharafian commented 6 years ago

Maybe the problem is that we're thinking of this as a password at all. Like you mentioned, it's not associated with any username and it's not stored in any database, so it doesn't really make sense to treat it the same way.

If anything, it's like a secret for a cryptocurrency. It's a private value that maps to a public one. Technically, wallet software allows you to hand-enter a password, so someone might use a weak brain wallet, but that's why software always makes sure to give you a securely generated secret and warns you how important it is to use a random value.

Furthermore, most of the times we use mini-accounts we generate the secret on the fly for each instance (assuming it's an infinite balance mini-accounts which is what your local ILP provider will probably expose). For a case like ilp-plugin-asym-client where we want it to be persistent, we could easily provide a config generator that creates a random secret or else generate one and store it in a file for the user when one isn't supplied through the config.

I think having values like http://:secret@localhost give the wrong idea too, because they make it look more like a password. If we switch our examples to use random strings like g6TuNjtoWqIHmm9IneK8G4_R0DNJONtJ32WykI7pZmQ that could also help.

dappelt commented 6 years ago

For a case like ilp-plugin-asym-client where we want it to be persistent, we could easily provide a config generator that creates a random secret or else generate one and store it in a file for the user when one isn't supplied through the config.

This made me realize that subclasses of mini-accounts/plugin-btp are in a better position to do authentication. For example, ilp-plugin-xrp-asym-client already has a private key for the paychan, which can be used for authentication. There is no need to create another secret value. The client could authenticate to the server by proving that he has the private key for a specific paychan. The ILP address the server assigns to the client could be the server's ILP prefix + the clients XRP address.

emschwartz commented 6 years ago

The other advantage of using the payment channel (and its corresponding keypair) for authentication is that it limits the set of people you need to worry about signing up to those that have XRP accounts. If anyone can create a token and effectively sign up, you need some other way to prevent someone from creating loads of fake accounts and stealing whatever you set the bandwidth to, multiple times over.

dappelt commented 6 years ago

No consensus was reached for this PR