Hacker News Reader

Comment by iancarroll | Hacker News Reader

I am pretty sure the code is reusing IVs for AES-CTR[0] (i.e. the encryption is worthless), because it doesn't specify an IV and Node seems to derive the IV from the key.[1] This seems like a massive footgun in Node.

I don't think their scheme results in different keys for each site, but I could be wrong.

[0] https://github.com/HainaLi/horcrux_password_manager/blob/mas...

[1] https://nodejs.org/api/crypto.html#crypto_crypto_createciphe...

View on HN

Wow. This is yet another example of the fatal combination of Rolling Your Own Crypto + Using OpenSSL Directly And Blowing Your Own Foot Off Because It Lets You.

  var cipher = crypto.createCipher('aes-256-ctr', key.toString('hex'))

Besides the completely fatal error of using derived and non-unique IVs (fatal as in, if you encrypt more than 1 item with it, it is exactly as good as plaintext because any two items encrypted with the same key+iv in CTR mode cancels out to plaintext), isn't using hex encoding vastly constraining the possible complexity-per-byte of the key?

A single hard-coded salt for key derivation:

  const key = crypto.pbkdf2Sync(auth, '0945jv209j252x5', 100000, 512, 'sha512');

Again, the salt is only lowercase alphanumeric. This makes this 120-bit salt really just a 77-bit salt. But since it's hard-coded and not randomly generated, it's a 0-bit salt.

Can everyone who is developing crypto apps Just Use NaCl/Libsodium?

As an infosec guy, I'm honestly getting really tired of this. Virtually every time someone submits some new security tool to Hacker News, the author has made trivial, catastrophic, and what should be completely avoidable security mistakes.

So for the hundredth time, if you're not a cryptographer or experienced security engineer, please stop releasing and promoting your crypto-related projects before they have been vetted by someone who is. If this is something you intend to release, ideally run the basic idea by someone qualified first. By not doing so, you are doing active harm. Someone's life and/or liberty may very well depend on the software you write, and when you fail them in this regard you are ethically and morally responsible when these things are taken from them.

What blows my mind is that most of these errors we see in these tools submitted to HN are errors that were all covered in my 'Introduction to Cryptography' course I took when getting my CS degree.

People seriously need to stop rolling their own crypto.

Is there a good way to find people who are qualified to do such a review? This paper was written by a Ph.D. student and professor of Computer Science at a respected university. The professor teaches a crypto course on Udacity (https://www.udacity.com/course/applied-cryptography--cs387). If they don't meet the criteria for being cryptographers, I wonder how many people in the world do?

Not many. There's a Venn diagram to draw about academic cryptographers and practicing cryptographic engineers and people qualified to do cryptographic reviews and the overlap is less than you might think.

I'm going to push back on this a bit.

Thomas responded alongside this comment to talk about how academic cryptographers are not necessarily qualified to implement original crypto, and I largely agree with that; however, I don't actually think that's the issue here. Rather I would pin this on a lack of peer review.

I could be wrong, but I don't believe the author of this paper has had it published or at least accepted in any journal or conference proceedings. Being an eprint format with endorsement rather than peer review, you can expect mistakes like this to happen often, even if the authors are ostensibly qualified. When you submit original research for publication you generally go back and forth a bit with adjustments as needed, and as long as there is nothing egregious you don't need to redo it all.

In this specific case, I believe the author fully understands the issue (or would, were it presented to them) and is fully capable of fixing it. A qualified peer review would (hopefully :) have caught this and other latent issues if an HN commenter did.

We see this in the broader mathematics and computer science communities, and we especially see it in sub-disciplines like machine learning as well. It's absolutely true that academic cryptographers should not be assumed capable of rolling their own crypto a priori, but in my (educated) opinion I would certainly place far more weight on crypto developed by an academic cryptographer than a software engineer without any particular training.

My platonic ideal for someone who is capable of developing original crypto is something like an academic with a PhD in math or computer science (focusing on crypto), who can develop software very well and who joins an applied lab for crypto engineering and development (like NCC's) or a top cryptanalysis firm like Riscure. Failing that, I'd probably place the most weight on someone who had a lot of training in crypto engineering or practical cryptanalysis over an academic with no implementation experience.

(I apologize if any of this is patronizing, I don't know what your background or familiarity with the academic process is w/r/t peer review, etc).

> A qualified peer review would (hopefully :) have caught this and other latent issues if an HN commenter did.

Would the qualified peer-review necessarily be reading the NodeJS code, or just checking the theoretical soundness of the paper? I'm not so certain about the former...

You absolutely need both skills.

Almost certainly not the former, no. At best the code would be "supplementary material", which reviewers are not required to go over.

5c from me:

1) why CTR mode was chosen? I would probably go with something like GCM: privacy + integrity check.

2) IV ideally should be re-generated on every re-encryption. It doesn't have to be secret, but has to be random (securely random).

GCM has exactly the same problem with respect to nonces (GCM, like CTR, has a nonce, not an IV, but the terms are unfortunately used interchangeably).

The secrecy/predictability/uniqueness rules for IVs and nonces depend on the specific cipher mode you're using, so be careful about writing generic recommendations. Also, be very careful with the word "ideally", because if you get an IV or nonce wrong, chances are your problems are much worse than "not ideal".

The good news appears to be that this has prompted some upstream discussion.

https://github.com/nodejs/node/issues/13801

I opened an issue on the topic. https://github.com/HainaLi/horcrux_password_manager/issues/1

Alas, this is the very reason I wrote this blog post on stronger encryption in node.js:

http://vancelucas.com/blog/stronger-encryption-and-decryptio...

Reply on news.ycombinator.com