3 ms·

Thanks for sharing! As somebody who has been bitten before by copy-pasting a key I appreciate that as the headlining benefit :) Thoughts from an ergonomic pers

by jffry 4y ago

Thanks for sharing! As somebody who has been bitten before by copy-pasting a key I appreciate that as the headlining benefit :) Thoughts from an ergonomic perspective: - Base58 is nice and makes easy-to-handle values which is nice - Naming of shortToken, longToken, and token are confusing to me, because it makes it sound like all of these values are tokens/secrets of some sort, rather than components of the token. Not to bikeshed, but what jumps to my mind is more like "prefix_id_secret", which helps make it clear what role each one plays. - I don't think keyPrefix should have a default, I think your function should raise an error if it's not provided. Otherwise more than one user of your library is going to end up with "mycompany" keys in the wild. - You probably ought to validate that keyPrefix does not contain an underscore and raise an exception if it does, otherwise that way lies pain for people trying to handle these keys. Thoughts from a security perspective: - You should probably use crypto.timingSafeEquals(buffer, buffer) [1] instead of comparing strings - If you're trying to store the secret's hash, isn't something like bcrypt/scrypt preferred over raw sha256 these days? [1] https://nodejs.org/api/crypto.html#cryptotimingsafeequala-b https://nodejs.org/api/crypto.html#cryptotimingsafeequala-b

koolba 4y ago
> If you're trying to store the secret's hash, isn't something like bcrypt/scrypt preferred over raw sha256 these days? Not for this type of use case. If a secret is long and generated from a cryptographically secure source (eg /dev/urandom), then any cryptographic hash function is fine as brute forcing the secret itself is not feasible. A single pass of sha256 is fine and presumably you’d be doing many of these operations each second in a high throughput application. “Slow” hash functions like bcrypt or scrypt are for user provided secrets that might not have a large amount of entropy. It’s fine for something like user authentication but would be way too slow and pointless for API keys.
- benatkin 4y ago
  > If a secret is long It seems this one, at 24 base-58 characters, is only roughly twice as long as it needs to be, right? How short is too short?
seveibar 4y ago
Appreciate the thoughts on the naming/exceptions and thanks for taking a look at the implementation! Definitely making some adjustments tonight. I think your thought on bcrypt/scrypt vs SHA256 is super interesting here. The long token is treated a lot like a password, so we should treat it similarly and use slow hashing. However, unlike a password an API key is repeatedly used for authentication instead of being exchanged for a session token. I don't think this meaningfully changes anything- so I think you're right that bcrypt/scrypt would be a better choice! Edit: Also see koomla's answer!
- benatkin 4y ago
  What meaningfully changes the requirements is that takes many more attempts to brute force an API key because they're longer. I'd consider using a weaker bcrypt/scrypt/argon2 for API keys than would be used for login. Perhaps one that takes a hundredth or a thousandth of the time. It could be unnecessary though. Here's the main scenario: someone snagged a hashed long token from a database backup and wants to get the unhashed long token so they can use it to access something behind the API. They can do all the brute forcing they want and the server owner will never know about it. There are 1 with 42 zeroes worth of potential long tokens to try (58 * '51FwqftsmMDHHbJAMEXXHCgG'.length). Seems unlikely even though it's very very cheap to hash a potential long token. The tokens this is using are pretty short, but still not short enough to make cracking it feasible. If you used argon2 maybe you could cut mycompany_BRTRKFsL_51FwqftsmMDHHbJAMEXXHCgG down to mycompany_BRTRKFsL_51FwqftsmMDH. Shorter token! I would perhaps make the long token 58 digits long just because it would have more than a googol (10^100) possible values but still be shorter than 80 characters with the prefix and short token, and maybe swap the SHA hashing for a low cost (memory and CPU) argon2. If you wanted to have really short API keys you could get creative with argon2. This is relevant for magic links. https://startdebugging.net/2013/10/counting-up-to-one-trillion/ https://startdebugging.net/2013/10/counting-up-to-one-trilli...

by jffry 4y ago

koolba 4y ago

> If you're trying to store the secret's hash, isn't something like bcrypt/scrypt preferred over raw sha256 these days? Not for this type of use case. If a secret is long and generated from a cryptographically secure source (eg /dev/urandom), then any cryptographic hash function is fine as brute forcing the secret itself is not feasible. A single pass of sha256 is fine and presumably you’d be doing many of these operations each second in a high throughput application. “Slow” hash functions like bcrypt or scrypt are for user provided secrets that might not have a large amount of entropy. It’s fine for something like user authentication but would be way too slow and pointless for API keys.

benatkin 4y ago
> If a secret is long It seems this one, at 24 base-58 characters, is only roughly twice as long as it needs to be, right? How short is too short?

seveibar 4y ago

Appreciate the thoughts on the naming/exceptions and thanks for taking a look at the implementation! Definitely making some adjustments tonight. I think your thought on bcrypt/scrypt vs SHA256 is super interesting here. The long token is treated a lot like a password, so we should treat it similarly and use slow hashing. However, unlike a password an API key is repeatedly used for authentication instead of being exchanged for a session token. I don't think this meaningfully changes anything- so I think you're right that bcrypt/scrypt would be a better choice! Edit: Also see koomla's answer!

benatkin 4y ago
What meaningfully changes the requirements is that takes many more attempts to brute force an API key because they're longer. I'd consider using a weaker bcrypt/scrypt/argon2 for API keys than would be used for login. Perhaps one that takes a hundredth or a thousandth of the time. It could be unnecessary though. Here's the main scenario: someone snagged a hashed long token from a database backup and wants to get the unhashed long token so they can use it to access something behind the API. They can do all the brute forcing they want and the server owner will never know about it. There are 1 with 42 zeroes worth of potential long tokens to try (58 * '51FwqftsmMDHHbJAMEXXHCgG'.length). Seems unlikely even though it's very very cheap to hash a potential long token. The tokens this is using are pretty short, but still not short enough to make cracking it feasible. If you used argon2 maybe you could cut mycompany_BRTRKFsL_51FwqftsmMDHHbJAMEXXHCgG down to mycompany_BRTRKFsL_51FwqftsmMDH. Shorter token! I would perhaps make the long token 58 digits long just because it would have more than a googol (10^100) possible values but still be shorter than 80 characters with the prefix and short token, and maybe swap the SHA hashing for a low cost (memory and CPU) argon2. If you wanted to have really short API keys you could get creative with argon2. This is relevant for magic links. https://startdebugging.net/2013/10/counting-up-to-one-trillion/ https://startdebugging.net/2013/10/counting-up-to-one-trilli...