Lots of issues about pysheeet

[crazyguitar]

I really really appreciate that you open this issue!! thanks. 👍

Unfortunately, the security cheat sheet was too simple that programmers SHOULD NOT just copy past the code. It will case lots of security issues. I will give a warning on the top of cheat sheet soon.

based on karanlyons said:

The AES examples use CBC with PKCS7 and no MAC, which is the classic example of a scheme open to padding oracle attacks. Nowadays I’d recommend GCM (or EAX) as your block mode if it’s available, and otherwise CTR or CBC with a MAC on the IV & ciphertext using a separate key. And I wouldn’t consider myself within even spitting distance of a crypto expert.

What’s weird is that cryptography has all of those primitives, so it’s concerning that the author didn’t use them. The key stretching is also using a custom function as opposed to one of the KDFs in cryptography as well (it looks like the function is basically HKDF, but the first rule of crypto is “don’t roll your own crypto,” so at the very least we could use the primitives).

There’s also no guarding against nonce/IV reuse (which is a bad idea in general, but thankfully not catastrophic for CBC) in the symmetric crypto. Worse, the key & IV are derived from the initial key, guaranteeing that any given key will always have the same IV, which opens you up to all sorts of attacks based around inference of plaintexts from a corpus of ciphertexts (With CBC if (Key₁, IV₁) == (Key₂, IV₂) then if C₁ == C₂, P₁ == P₂. Additionally the first block is now effectively in ECB mode. With CTR/GCM/EAX/etc. the results are way worse).

There’s probably more stuff wrong, too (like the stuff around certificates seemingly ignoring any trust model, and the implication of a lack of understanding as to the maximum message length given a key size), but that’s what I found from a quick glance. The one bit of good news is that if you just use TLS with strong cipher suites (and a trusted cert with pinning) almost all of this will be done properly for you.

I get the intent behind this, and I think that intent is genuinely good, but crypto is really not a place where you want to be giving any advice unless you’re either an expert or willing to couch your advice in a bunch of caveats.

(Also, and somewhat tangential: if we’re going to put together a “this is how you do things” cheatsheet, it wouldn’t hurt to espouse the most modern stuff we’ve got: ECDSA, AES-GCM, ECDHE, ECC and AEAD in general, scrypt, etc.)

I agree that there still have a lot of work to do ...