ericelliott/credential

Easy password hashing and verification in Node. Protects against brute force, rainbow tables, and timing attacks.

credential's Issues

Support bcrypt hashing method?

Not sure how can I use bcrypt in stead of pbkdf2

Follow the WebCrypto API

It would be good to mimic, insofar as sensible, the WebCrypto API. Consistency across APIs reduces the likelihood of developer misunderstanding and error.

The API is still changing, but it looks like it'll be landing soon.

Update deps, freshen code, release new major version.

Cut a release

It has been a while, and even though we have a basic agreement in #16 that you should avoid passwords if possible, this package still seems like a solid implementation, and I would rather have people using it than rolling their own.

I would really like people to have the latest additions (especially e3a7bd4), so I propose we cut a 3.0.0 release.

I have back-ported as much as possible (including e3a7bd4) from the master branch onto a 2.x compatible branch for those who have legacy databases: https://github.com/srcagency/credential/tree/v0.2.x
A neat thing is that passwords will be stored in the "new" format (with iterations - see the legacy test case) meaning that users with existing databases can track this branch for e.g. six months and then do an upgrade to 3.x, then only users who hasn't changed password since will be invalidated.
Of course, this should be done with extreme caution as I haven't actually had the chance to test this on a real database.

Side note:
As an experiment, to gain greater knowledge of my own preferences and code style, I did a complete rewrite of the package (still passing existing tests). If you're interested it is here: https://github.com/srcagency/credential/tree/rewrite and comments are welcome.
See this specific commit's message for a brief summary of changes: srcagency/credentials@48b689d.

nice credential hasher

Hi Eric,

Neat module! I only have some suggestions from observation:

A hashing implementation that I had used on a non-node project based it's iterations of PBKDF2 on the current year which seemed convenient.

The relevant c#
here to here, to here

This implementation also uses the storage technique described here, and doesn't worry about obfuscation of an explicit iteration count. This could be naive but I don't think an the iteration count being declared on the hash would hurt anything. And it could certainly help when determining if a specific credential needs to be reset so a stronger hash can be created.

Disclaimer: I'm no expert!

-caleb

The Road to 1.0

Let's get 1.0 released!

• Update docs
• Warn about how passwords are obsolete, and users should consider alternatives if possible. Link to other open source library options for 2FA (Google Authenticator, Twilio SMS verification, etc..)

Original post

I'm using credential in a personal project and really like the focused approach you've taken. Are you still planning on moving ahead to 1.0? Are any contributions are needed from a non-security expert, open source novice?

Hash and Verify taking too much time

Hi there ! I hope I'm not putting this message at the wrong place.

I'm working in a team and our project is using your package "credential": "^2.0.0"
I was investigating our login feature taking too much time and it appears that it is coming from the verify function (from 3s to 5s)
I've written this little test to confirm that:

const credential = require('credential')()

const start = async () => {
  let start = new Date()
  const pwd = 'myLittlePassword!#56'
  const hash = await credential.hash(pwd)
  console.log(hash, new Date() - start)
  start = new Date()
  const isValid = await credential.verify(hash, pwd)
  console.log(isValid, new Date() - start)
}

start()

And both the hash and verify functions take around 5 seconds each.
Is it normal to take that long ?
Thanks !

having hash in logs

As I understand it, given all the magic of one-way functions, an attacker wouldn't get much from just the hash object, but do you consider it safe to have in logs or should I try to avoid logging it?
Thanks for this great tool!

Passwords are obsolete

The biggest threat to password security right now is passwords that have already been stolen from systems that aren't secure.

The runner up is brute force, and brute force can be distributed and optimized with frequency-sorted password dictionaries.

We should encourage all credential users to also employ:

• 2-factor auth
• dictionary-based password blacklist

How can we better encourage them in the docs?

Is it possible to encourage them programatically, as well?

Ideas

Add a password validation function which:

1. Makes sure the password is sufficiently secure (based on config)
2. Asynchronously tests against a password blacklist

This idea complicates security, because we'd need a database of blacklisted passwords, and we'd need to keep a candidate password in memory long enough to check against the blacklist. If an attacker got access to our blacklist, they could just filter their frequency sorted dictionary. If the attacker gained access to program memory, they could sniff the pending passwords.

I still think a small enough blacklist would be a good idea.

The software architect side of me is convinced this belongs in a different module. The lazy developer side of me is convinced that if it's in a separate module, few developers will bother with it.

However, maybe we could put it in a separate module, and refer to it in all the docs. Show it in the installation steps, show it in the example sequences, etc... Make it really obvious that these modules are intended to be used together.

We could do the same thing with a 2FA solution, though I'm inclined to use an off-the-shelf 2FA instead of rolling our own.

Is there a good off the shelf password validation library we could use?

prevent travis from randomly failing

travis is randomly failing because of: constantTimeCompare

I guess we don't want to increase the alpha time

What about we change the travis script to
npm run check || npm run check || npm run check
So the tests are running at least three time before failing

can't bind hash

this code is throwing an exception, and doesn't execute the toHash function:

import credential from 'credential'

credential.hash.bind(this, 'foo', function(err, hashedPassword){
    // never get called
})

Checkout this branch for a failing test: https://github.com/mastilver/credential/tree/feature/fix-not-able-to-call-bind

Note: I don't thing hash is the only function affected

Node v6 deprecation - "crypto.pbkdf2 without specifying a digest is deprecated"

I just installed Node v6 and noticed a new deprecation warning: "crypto.pbkdf2 without specifying a digest is deprecated". crypto.pbkdf2 is called here, and it leaves digest undefined. According to the Node v4 docs, an unspecified digest algorithm defaults to "SHA1".

I'm happy to open a PR for this, I was just wondering whether to add an explicit reference to the existing default (i.e. "SHA1") or to switch it out with SHA256 or SHA512. All the information I can find says even though SHA1 has vulnerabilities, they don't affect its use in pbkdf2; so SHA1 should be fine. That being said, all the sources of information I could find (and by that I mean Stack Overflow questions) were several years old, and I know next to nothing about cryptography. So some guidance there would help.

Release v2.0.0 on npm

verify/parseHash should accept object

If you are using document based database like MondoDB or OrientDB you can store the json-hash-string directly as embedded document.

So why not pass the stored object to verify/parseHash instead the JSON representation of the Object?

Is JSON the business of credential?

Is it really necessary to have the JSON.parse/stringify business in this module? It seems rather arbitrary to me. If using a store like MongoDB it makes little sense to save it as a string - and then I would be able to do lookups of potentially weak passwords (in the sense of iterations).

If it's an interface thing, we might just as well use array.join and string.split - not that that's any better.

A backwards compatible approach might be to return the object, but attach a prototype with a toString method that's basically JSON.stringify. It would be fairly easy to have verify support both formats. Of course I'd rather see a breaking change and complete strip of the stringifying and parsing.

function for hardening hashes

Imagine a function which takes an hash/hash-object and returns an hash/hash-object. The function does nothing more then increase the iteration count and rehash the old one.

This is useful if you want keep your hashes difficult to crack as computer performance is increasing. (e.g with a cronjob)

What do you thing about it?

Git tag

@tjconcept

Hey, I wanted to reference a line of code from the 0.2.6 but the tag isn't defined.

the 'verify' function takes too much time, about 1.3 seconds

the 'verify' function takes too much time, about 1.3 seconds

performance optimisation

What could i do to enhance Encryption performance? The Hash function seems to slow down my MEAN Stack server at a high level. Normal requests happen in about 20ms, creating a user and logging in (--> using the hash function on my server) slows it down to 3 to 4 seconds!

Does it really needs webpack?

Webpack seems like an overkill
Why do we need to have a minified version?
Isn't it enough to use babel?

Why are you trying to hide the iteration count?

That's going to make it much more difficult to impliment in another language.

Upgrade QA tools

On the road to 1.0.

See the scripts in prod-module-boilerplate for reference.

• Upgrade build process and tools
• Verify Travis build
• Replace Mout with native tools or lodash
• Upgrade outdated dependencies
• Double check pre-commit hook

Double check build status, tests

• Check the Travis build status
• Verify that the tests are completing

Bad default settings

Default settings of PBKDF2-SHA1 with output length of 66 bytes is bad. The way PBKDF2 works the defender is doing 4 times more work than an attacker. There should be a limit of 20 bytes to prevent this.

Also consider changing it to PBKDF2-SHA512 with a max length of 64 bytes. SHA512 is about 2 times better than SHA1 or SHA256 because SHA512 uses 64 bit integers. On a 64 bit processor you gain a 2x advantage over an attacker (it's really that you take back a 2x disadvantage). Although this might change once SHA instructions are common/available. Since this might make SHA1 and SHA256 maybe around 3x faster and thus about 1.5x better than SHA512.

TL;DR Generally speaking, use a 128 bit salt and a 256 bit hash. Larger is pointless and even half those are fine, but don't go lower than half.

There is no point to go more than 256 bits in a password hash as a password has much less entropy. Also 128 bits is fine, but I wouldn't go lower than that. For salt, 128 bits is awesome and 64 bit is "fine". For salt, there is no point to going above 256 bits or even near. Although my inner cryptographer is yelling "use a 256 bit salt"... OK fine for encryption and easier proofs use a 256 bit salt. Now my inner password cracker wants to talk real world. Let's talk global scale (2^33 people) and crazy scale 7 new salts per person per day for 50 years (2^17 salts/person). So 2^50 random 64 bit salts that means you have 99.996948% unique salts or a 0.003052% speed up in cracking. Note there are about 2^35 collisions with 2 or more, but about 32 groups of 3 collisions. So yeah for those 96 salts that collided into groups of 3 it sucks if those are the only hashes targeted, then it's 3x faster. BUT 96/2^50 is negligible. Anyway a 64 bit salt is "fine", but even though I know this, I'd just use 128 bits and you'll "never" collide.

Overrides test bug

https://github.com/ericelliott/credential/blob/master/test/credential-test.js#L171-L172

I guess this should test the resulting hash, not the current settings.
(fix coming in PR)

Needs to be a factory for instance safety.

New hotness:

var credential = require('credential');
var pw = credential(options); // instance safety FTW

Result of hash method

Please add option, which can change result from string into object.
Now I parse it after your stringify. I think is better give to user define type of result

What if an attacker know that I am using this library?

As we know, the hash method is returning a JSON string which contains: hash, salt, keyLength, hashMethod, iterations. Now, without knowing how an app using this library is hashing passwords, the attacker will have no way/will be slowed down when trying(?) to crack the passwords, but what if he/she knows that I am using credential library (and also its version)?

If you don't mind another question, how would you go about rehashing users passwords with more cycles? #23 (comment)

Verifying with a pre-parsed JSON object

When attempting to verify a hash I started off by passing in the hash as a Javascript Object. After looking at the code of verify, it seems that it always attempts to run JSON.parse on the input hash, which in my case was not needed. The solution was to use JSON.stringify on the object before passing it to verify.

In my case I am using a Postgres JSON column in my database and Knex and Bookshelf as an ORM, which means by the time I receive the password hash from the database it has already been parsed. Ideally I'd be able to pass this directly to verify and avoid the step back to a string.

Thanks for the great library!

The use of "time" - a weakness worth noting?

The number of iterations and thus the strength of the hashes depends on the result of Date.now which in turn means that a misconfigured system time would be fatal.

Maybe we should make a note about this in the read me, and possibly some warning if Date.now reports a year less than 2015 - that would prevent complete disaster in the foreseeable future.

I'm not sure about the practical risk this possesses, but some considerations:

• an attacker could intercept NTP
• system time is critical (https, databases, any crypto) which makes me believe it is well protected and on top of most sysadmins checklists

Inconsistent use of bytes length/encodings?

createSalt generates keyLength bytes and then transform it in a base64 string. This string is passed to pbkdf2 as salt, but pbkdf2 treats strings as utf8, so the given salt is not the same random sequence of bytes generated early.
Can be assumed that the salt is still random even if has been expanded by the inconsistent use of the encodings? Are there any security implications? Usually in cryptography when you introduce some fixed points you open yourself to some kind of attacks.

Make errors programmatically processable

Currently when credential resolves with an exception, to distinguish kind of an error (e.g. whether it was that one the other) we need to investigate error message.

That's not very reliable as if message will be reformatted proper detection may stop working.

It'll be great if errors are equipped with code property (as it's in Node.js API's), that way by investigating error code's we can programmatically read the reason of rejection.

I can't make the cli work

this work:
echo -n "my password" | credential hash - | credential verify - "my password"

But if I do:
credential hash password

and then:
credential verify <the previous output> password
It's throwing me:
Unexpected token ':"w6iqo4XwaKPzWn7B541EW+kWeVmJ9bMMv3WNMPPutWuwwSdJecntnXDBFw Xc+MO/dtAFnZKkdg+bBPIjJOqUpKGJ"' in expression or statement.

I can't figure out why...

Default number of iterations seems extreme

With default configuration, we are observing that the number of iterations is around 480000, causing the hashing process to take around 2 seconds on a high spec development machine. This has a few consequences - aside from slow login, integration test suites are very slow, and it is foreseeable that it could cause performance issues in production.

Is this by design?

The recommendation from our security team is to tune it so it takes roughly 0.5 seconds.

Invalid input error should be passed to callback

Great library, I just have one problem so far. When invalid input is passed to the hash function, this error won't be passed to the callback. Instead the application will throw and possibly shutdown.

Steps to reproduce:

tom@tom-UX32VD:~/projects/restorder$ node
> var pw = require('credential')
undefined
> pw.hash(undefined, function(err, hash) { console.dir(arguments); });
undefined
> 
crypto.js:565
    return binding.PBKDF2(password, salt, iterations, keylen, callback);
                   ^
TypeError: Not a buffer
    at pbkdf2 (crypto.js:565:20)
    at Object.exports.pbkdf2 (crypto.js:551:10)
    at Object.pbkdf2 (/home/tom/projects/restorder/node_modules/credential/credential.js:52:12)
    at Object.<anonymous> (/home/tom/projects/restorder/node_modules/credential/credential.js:119:30)
    at Object.ondone (/home/tom/projects/restorder/node_modules/credential/credential.js:81:7)

Checking whether the right parameters were inserted each time I execute pw.hash is cumbersome when I already have error handling based on the err callback parameter.

Do not encourage people to write security issue in public places

From the NPM page:

If you find a security flaw in this code, please report it.

I did not find an exploit, but I was wondering where the link would lead to, and now I am here.

Just writing the exploits here is not really secure and could in fact risk every use of your module out there, Please consider handling security issues in a closed issue queue or mail or something. Some place where others cannot easily see them.

Due to the large number of iterations, Its consuming the full CPU usage.

Hi there,
I am using this library for my project, and till now everything is fine. But, now the project is going to live mode and I have done the load testing of my site. When I had tested my site with 100 users and only 8 users were able to login simultaneously. However, it's not using the system RAM but consumes 100% CPU. Thereafter no user was able to Login.
Please suggest me what to do?

Thank You!

Base iteration count on current year.

For reference, see:

https://github.com/brockallen/BrockAllen.MembershipReboot/blob/master/src/BrockAllen.MembershipReboot/Crypto/DefaultCrypto.cs#L128

The defaults in the current version of credential are based on recommendations for 2014, so we would use that as the start year.

In order to make this work with credential, we will need to be able to detect expired password hashes so that the user can be prompted to reset credentials. This also ensures that the user is forced to change their password annually.

Note that we probably want to implement this functionality such that it would not be a breaking change. There are already many production apps using the existing version of Credential.

See also: #9

Need Password Security expert endorsement

I want to make sure that the Node / io.js community has a password security library they can trust. In 2013, I researched all of the libs I could find and found serious security flaws in all of them. I know it's impossible to make it perfect, but we need to ensure that there is something that at least raises the bar enough that a random script kiddie can't cause multi-million dollar disaster, PR nightmares, and personal loss to users.

I'm not a password security expert. I'm asking for your help. This has already been reviewed by many security experts, but I know there is room for improvement, and I want to make sure that users have a clear indication about which library they can count on to really work to make their users more secure. Please review this code carefully. Attack it with everything you've got, and then file issues here. I'll give you public credit, and you'll be helping millions of people have a more secure online profile.

Please indicate whether you'd be willing to publicly endorse this library when the issues you've filed have been fixed. I'm looking for top security experts to sign off on the security of this library.

Thank you for your help!

Sincerely,

Eric Elliott
Author, "Programming JavaScript Applications" (O'Reilly) &
"Learn JavaScript with Eric Elliott" (online courses)

Additional Hash Function: scrypt

It might be a good idea to implement scrypt as an alternative hash function:

https://github.com/barrysteyn/node-scrypt

Why hash() just return string rather than object?

First of all many thanks for your lib, I have a problem to work with your module. when I use hash() it returns a string that has a object for example

pw.hash(body.password, function (err, hash) {
      if (err) {
        console.error(err);
        return;
      }
console.log(type of hash) // string
}

but I need hash.hash

fix failing CI server

I will try to work on that tonight

In the meantime, @ericelliott do you think you can enable @greenkeeperio-bot on this repo?

constantEquals is not constant time

The function

constantEquals = function constantEquals(x, y) {
    var result = true,
      length = (x.length > y.length) ? x.length : y.length,
      i;

    for (i=0; i<length; i++) {
      if (x.charCodeAt(i) !== y.charCodeAt(i)) {
        result = false;
      }
    }
    return result;
  },

is not constant time. In particular, the assignment of result will add instructions to the loop when the characters are inequal, exposing timing signatures.

I will post a comment momentarily with a thought on a possibly better option.

callback, promise or both

currently the api is using callbacks
What about using Promises instead?

Or we can have both (return Promise if callback is not provided)
I'm sure there is a module that does that, I can't remember witch one...

What do you think?

Remove workUnits, credential_key and workKey options in favor of iterations

It was always my understanding that the iterations in PBKDF2 were the 'workUnits' and it was algorithms like bcrypt that took an arbitrary workUnit which was then used to compute a time-cost within the algorithm. In that light I'm not sure the options credential_key or workKey or workUnits add any additional security (only obscurity) besides saving the attacker a few hash comparisons.