paragonie-scott/crypto-wrong-answers.md

Last active August 17, 2024 06:33

Star () You must be signed in to star a gist
Fork () You must be signed in to fork a gist

Learn more about clone URLs
Clone this repository at <script src="https://gist.github.com/paragonie-scott/e9319254c8ecbad4f227.js"></script>
Save paragonie-scott/e9319254c8ecbad4f227 to your computer and use it in GitHub Desktop.

Download ZIP

An Open Letter to Developers Everywhere (About Cryptography)

Raw

crypto-wrong-answers.md

Hello software developers,

Please check your code to ensure you're not making one of the following mistakes related to cryptography.

I. General Mistakes

Writing your own home-grown cryptography primitives (For example: Mifare Classic)
- Exception: For the sake of learning, but don't deploy it in production.
Using a fast hash function (e.g. MD5, SHA256) for storing passwords. Use bcrypt instead.
Not using a cryptographically secure random number generator
Hashing small identifiers and hoping the hash hides the identifier (easily brute-forced)

II. Symmetric-key Cryptography Mistakes

Not using authenticated encryption
Not following an Encrypt then MAC construction
Using ECB mode (ciphertext blocks will repeat, degrading confidentiality)
Not using a random IV for CBC mode
Ever reusing a nonce in CTR mode with the same key
Using a human-readable password (i.e. "Password123") directly as an encryption key instead of a randomly generated string (or rather a byte array if appropriate to your language)
Using the same key for encryption and message authentication
- This isn't really a vulnerability, just a bad practice; use HKDF-SHA256 to split your key into one for encryption and one for authentication.
Hard-coding an encryption key or password into your client software

III. Asymmetric-key Cryptography Mistakes

Using RSA with PKCS1v1.5 padding
- Yes, that scheme was broken in 1998. I don't give a damn what your "legacy support" concerns are. Stop using it already.
Using an unsafe curve for ECC (e.g. many NIST curves)

If you're looking for cryptography right answers, check out the linked gist.

Thank you for your time.

Signed,
A friend who wants your application to be secure

Author

paragonie-scott commented Nov 18, 2015

If you happen to be a PHP developer and aren't sure how to solve your cryptography requirements for your project, I've got you covered there too: secure PHP cryptography library recommendations.

Ivoz commented Nov 18, 2015

Generating anything but >= 2048 bit RSA keys; accepting <1024 bit RSA keys (<2048 bit for new applications)
Accepting <1024 bit DH parameters; Not deploying >= 2048 DH parameters only in new applications
Thinking the security benefits of any salt you're using should rely on it being kept secret
Using a library written only in a dynamic language for cryptography, this will 99% of the time mean you are now open to side channel attacks (surprisingly easy if you're running virtual hosts in a shared environment)
Comparing two cryptography-related values without using a constant-time comparison function
Not trying to use the highest-level cryptography API you can possibly get away with (e.g: going straight for OpenSSL instead of libsodium)
Demanding all sorts of requirements for passwords - if necessary, just ask for a long one.

atoponce commented Nov 18, 2015

No need to add it to your article. I'll add this here- there is a single edge case exception for ECB mode- using it with a block cipher as a userspace CSPRNG, such as AES. Of course, the only reason you're writing a userspace CSPRNG is because a kernelspace CSPRNG does not exist (embedded, whatever).

The problems with CBC, OFB, CTR, etc. is that the all rely on other a separate IV or nonce object. This is another "thing that can go wrong" and possibly compromise the security of your block cipher CSPRNG. Instead, ECB mode doesn't need anything of these things- just the keyed data.

As a psuedocode example:

key = <16-bytes of sufficient entropy data>
i = <16-bytes of zero>
while true:
    print AES-ECB(i, key)
    i++

This looks a lot like AES-CTR, but doesn't require a nonce to start. The only thing that needs to be provided by the end user, is the key. The security of the userspace CSPRNG rests entirely on the entropy of the key. Further, if the data is only 16-bytes, and never larger, then you'll only be encrypting a single block, and you'll never need to worry about data structure leaks.

Of course, the counter in AES-CTR could be randomly generated by system, but then you have a chicken-and-egg problem. How do you guarantee the entropy of the counter, if you don't have a kernelspace CSPRNG? You could statically set the counter to zero, but then you're no better off than AES-ECB.

Just a thought.

wsargent commented Nov 18, 2015

I'd really like references to Keyczar, Kalium, cryptlib or libsodium in there (as well as any other high level crypto libraries in there). Blog post: https://tersesystems.com/2015/10/05/effective-cryptography-in-the-jvm/

Also links to http://saweis.net/crypto.html and https://github.com/sweis/crypto-might-not-suck

Nov 18, 2015

@atoponce:

How is your system any different than using CTR mode with the nonce set to 0? This isn't even a "they are similar" comparison: the output should be identical. Both are AES(0, key) + AES(1, key) + ... + AES(n, key)

Copy link

atoponce commented Nov 18, 2015

@Matir:

The only argument is simplicity. AES-ECB(data, key) is simpler than AES-CTR(data, key, counter).

Copy link

AntonKueltz commented Nov 18, 2015

Please take a look at Thomas Pornin's post on elliptic curves in regard to your last point on safe curves.

Copy link

Nov 19, 2015

I like your post. However, claiming "Not using authenticated encryption" is a mistake is a bit alarmist - "encrypt then MAC" works fine. Also, it might be worth having a separate section for password hashing; Failing to use a salt is another mistake. Personally, I'd remote the "Exception: for the sake of learning" because that actually applies to everything here, not just home-grown ciphers.

Copy link

creshal commented Nov 19, 2015

With the PHC over, I'd recommend adding its Argon2 password hashing together with bcrypt.

Copy link

atoponce commented Nov 19, 2015

@creshal I wouldn't. Argon2 is new and given the suite of current algorithms, relatively untested and unused. I would let Argon2 bake for at least 5 years before giving it considerable attention for production use. We need mathematicians and cryptographers analyzing Argon2 for a good long while, before it becomes an every day recommendation. In the meantime, bcrypt is a sound choice.

Copy link

vdzhuvinov commented Nov 20, 2015

You'll be spared all these problems of home grown solutions if you use standard JOSE and JWT, elegant crypto designed primarily for web and mobile apps :) JOSE stands for JavaScript Object Signing and Encryption, JWT for JSON Web Tokens. For Java and Android check out the Nimbus JWT lib: http://www.connect2id.com/products/nimbus-jose-jwt

Copy link

Author

paragonie-scott commented Nov 20, 2015

@vdzhuvinov No!

https://twitter.com/bascule/status/625773581448777728

https://www.reddit.com/r/ruby/comments/3erwqx/proof_dead_simple_security_for_ruby/cthw4ho

Copy link

tarcieri commented Nov 26, 2015

@vdzhuvinov there is nothing "elegant" about JOSE and JWT. They had the opportunity to greenfield, but ended up effectively creating JSON-ized SAML full of design mistakes which really shouldn't exist if they had properly studied the attacks on SAML:

alg is mandatory for tokens but not for keys: this specifically lead to this class of vulnerabilities which shouldn't exist in modern standards: https://auth0.com/blog/2015/03/31/critical-vulnerabilities-in-json-web-token-libraries/ This is practically backwards: alg should be mandatory for keys, and tokens should carry only a kid to associate a token with a particular key, and NEVER carry an alg field (instead kid is optional and alg is mandatory for tokens). This way, the verifier always knows the algorithm, and doesn't need to consult the (malleable) part of the token to infer the algorithm. That's just asking for attacks which trick the verifier into using the wrong algorithm (see earlier mentioned link).

Transferring claims between multiple principals (i.e. 3+ principal interactions) are a traditionally difficult problem that leads to ambiguity and confusion (i.e. "confused deputy problem"). With SAML we saw confusion between claims for different principals being misused in the wrong context, "fixed" in SAML 2.0 with AudienceRestrictions. This was carried over in JWT as the aud attribute, but it could've been solved with a cryptographic binding between principals that absolutely ensures claims can't be confused (as seen in e.g. Macaroons). Instead JWT merely carried over SAML's half-solution, and worse, they chose not to make it mandatory, meaning it will not necessarily be automatically checked, so we have two more sharp edges: failure to include an aud attribute leading to attributes being confused in different contexts, and a potential failure to check aud even if it's present. Macaroons, on the other hand, not only cryptographically bind credentials acquired by multiple principals together (i.e. third party caveats / "holder-of-key proofs"), but provide a formal authorization logic for proving them correct.

JOSE and COSE did not specify signatures in a way that's interoperable. This is not a problem with other bearer credential formats (e.g. Macaroons) where the tokens can be transcoded between serialization formats and signatures will still verify. COSE couldn't do this due to a lack of forethought on JOSE's part. Anyone who is using JOSE now who wants to transition to COSE must make a wholesale switch: they can't incrementally upgrade.

JOSE is an ugly, design-by-committee standard. It is an unopinionated, one-size-fits-all solution. About the best I can say is that JWE and JWS are better than CMS, but that's not saying much.