[2.4] [SECURITY] Review of Crypto library

[v6ak]

I'd like to discuss the crypto library security and purposes. I find it easy to misuse the library. Moreover, recent 2.4 update *changed* some security properties. That is, some previously insecure usages are secure now, but also some previously secure usages are insecure since 2.4. This means users of Crypto library should consider the security impact before migrating to 2.4.

What has been changed?

The ECB mode has been replaced by CTR mode. I'll quote a misleading claim from the official documentation:

“The CTR mode is much more secure than the ECB mode.” ( https://www.playframework.com/documentation/2.4.x/Migration24#Crypto-APIs )

The ECB mode is generally non-recommended (see https://en.wikipedia.org/wiki/Block_cipher_mode_of_operation#Electronic_Codebook_.28ECB.29 ), so this might look like a good decision at first sight. While the CTR mode can be more secure if properly used, it has some different pitfalls. Because some of these CTR mode pitfalls are not present in ECB, some previously secure code might become insecure.

There are some more changes, e.g. better entropy of key (higher effective key size). The old Crypto library uses first 16 characters of a string key (i.e. application.secret by default) as a key, which is wrong, especially when the string (application.secret) is hexadecimal (=> 64b effective key size) or so. See https://github.com/playframework/playframework/issues/2879 for more details The new Crypto uses a hash function for deriving the key, which is much better. A PKDF would be even better for some purposes, but even now I don't see any significant issue with the new key derivation approach. (But it depends on usage! I'll discuss it later.)

What might be wrong for some usages?

Unlike ECB, the CTR mode is a stream cipher mode. Stream ciphers have usually two issues that are not present in ECB mode:
1. Malleability. This one is not specific for stream ciphers, but stream ciphers are ultimately malleable. An adversary without the secret key can modify the cryptotext to mean something different. For more details on malleability, see https://en.wikipedia.org/wiki/Malleability_%28cryptography%29#Example_malleable_cryptosystems
2. Insecure when a key+IV is reused. If you, however, use one key with the same IV twice, some details about both plaintexts are leaked, potentially revealing both of them. See https://en.wikipedia.org/wiki/Stream_cipher_attack#Reused_key_attack for more details.

The malleability can be mitigated by authenticated encryption, but Play! does not it implicitly. This would be correct for a completely new API if this was mentioned in the documentation. In Play!, the Crypto API is not completely new (so one might consider it as a BC break with some bad security implications) and the documentation even don't mention it.

The key+IV resuse attack (“keystream reuse attack”) can be mitigated by using random unpredictable IVs. The documentation is unclear about usage of IVs. It just states that both using an IV and not using an IV is supported, but it is not clear what is the default.

What else is wrong with the documentation?

I've found also some relict in documentation, ECB doc relicts. It is a minor issue: The documentation just states that some usage is insecure, although the issue is not true for CTR mode: https://github.com/playframework/playframework/issues/4407#issuecomment-106936942

What/who is the Play! Crypto library intended for?

A proper mode of operation must be selected for ensuring desired level of security for desired type of usage. There are various properties that can be considered neither good nor bad without defining the correct usage. I am also not sure if the library is intended for crypto-newbies (it is easy to use it wrong for them) or crypto-experts (they would want to choose the mode of operation themselves).

In addition to two CTR-related issues mentioned above, it is questionable if PKDF should be used. It is unneeded in some cases (e.g. if the key is application.secret), but it is welcome if you are using a potentially weak password (e.g. user password), because they slow bruteforce attacks down by some factor.

Well, I admit one can configure the mode. But I don't think that global config (i.e. play.crypto.aes.transformation config option) is a good idea. It is generally unclear what code is affected by changing this property. Is some library code affected? I don't know until I analyze all the libraries I use.

I'd like to hear answer to the question from the developers. It should be also noted in the documentation. Without it, one might assume that almost any behavior is OK.

Why do I disclose it publicly?

I respect responsible disclosure objective, but I don't think that keeping this issue private makes any sense now, especially when 2.4 is fresh. I feel it is better to warn programmers that they should think twice between 2.3 -> 2.4 migration if they are suing Play! Crypto library.

Regards,
Vít Šesták 'v6ak'

[Ben McCann]

Relevant issue: https://github.com/playframework/playframework/issues/4407

[v6ak]

I'd like to bump this issue. I am happy to suggest a solution, but I need an answer to the question “What/who is the Play! Crypto library intended for?”. Without that, I might suggest something unsuitable.

At this time, it looks for me to use some AEAD mode instead of CTR, but that's rather some guess.

Regards,
Vít Šesták 'v6ak'