Skip to content

Instantly share code, notes, and snippets.

@v6ak
Last active August 29, 2015 14:23
Show Gist options
  • Star 0 You must be signed in to star a gist
  • Fork 0 You must be signed in to fork a gist
  • Save v6ak/2f7f3c4f18cfe4d1be9e to your computer and use it in GitHub Desktop.
Save v6ak/2f7f3c4f18cfe4d1be9e to your computer and use it in GitHub Desktop.

I'd like to suggest something, but the correct solution depends on the purpose of the Crypto library. On one hand, it looks somewhat high-level (e.g. the output is String of printable characters and the API is simple), on some others, is does not look so high-level (the need of knowing the specifics of the mode of operation). It also depends on how much do we want to be backward compatible.

What output overhead is acceptable? In this case, I am mainly talking about some overhead with a constant upper bound. This overhead might be needed for padding, authentication and IV. The choice of CTR suggested that such several-bytes constant overhead might be too much, but I don't know the reasoning about the choice.

Random read/write: The CTR choice looks also like intended for some random read or write operations (note that random write with CTR is insecure except some special cases), but the API does not provide anything like this. If random access is a concern, it adds some limitations to mode of operations, so this is an important design decision. (The case of authenticated random write is the most tricky one, but it is still feasible at some cost.) I feel that random access is rather outside of the scope of the Play! Crypto.

Is authentication needed? Generaly yes (e.g. POODLE is an exploitation of an imperfect authentication), but users might have solved it separately, so authentication would bring an extra overhead (both computational and the output). Maybe authentication should be enabled (and required) by default, with an option to disable it.

Key derivation: If the key is strong, key derivation like PBKDF2 is rather an overhead with little-to-no security benefit. In some cases, such overhead might be a kind of DoS. On the other hand, when there is a weak password, a key derivation function like PBKDF2 might be a critical improvement of security. Unfortunately, if we want to address this issue, we seem to have to change the API. But this can be done in a conservative way by deprecating parts of the old API. (I might elaborate more about new API design.)

One of my major complaints about the current state is the global config. The user can configure the security settings for the whole app, but various parts of the app (some of them might be hidden in a 3rd party library) might have different needs. So either all of the parts of the app must be secure under any such configuration (which makes writing such code hard, requiring to be able to handle the lowest-common-denominator of all the modes) or user must be very careful when changing such global option (i.e. careful about all the requirements of all the code using the Crypto). So, I generally don't recommend changing the global config, as this might be very tricky.

My preference is against a versioned approach (i.e. a Crypto implementation that can look at "version 1" vs "version 2" strings), because it's easy to weaken crypto by including old versions (i.e. FREAK) and adding extra logic to deal with it.

I see you point, but I disagree in some degree. First, if you want to be backward compatible, you need versioning. Second, versioning does not imply the ability to downgrade the cryptography. There might be some potential to perform some oracle-based attacks (I might elaborate), but they are not much likely to be practical. Maybe a good tradeoff between complete versioning and not versioning at all is versioning when required. The user might configure the engines to be Seq(LegacyEngine1, LegacyEngine2, NewSuperSecureEngine). Such config would assign version numbers 1->LegacyEngine1, 2->LegacyEngine2, 3->NewSuperSecureEngine. The last one (i.e. the NewSuperSecureEngine) would be the one used for encryption. The first one (i.e. the LegacyEngine1) would be the one used for decryption if no version is available. When LegacyEngine1 is about to be switched off, one would just replace it by DisabledEngine (i.e. Seq(DisabledEngine, LegacyEngine2, NewSuperSecureEngine)), which would fail at all the decryption (and encryption) requests. This approach allows to balance backward compatibility and downgrade prevention. In the case of backward compatibility is required, I don't think that versioning would weaken the cryptography to lower degree than before.

Just a note about 3rd party crypto libraries: I'd like to have all the crypto primitived implemented in a native code if possible. The main reason for me is not the performance (although AES-NI can bring a serious speedup), but rather side channel resistance. This will probably favorize JCA and its wrappers. While JCA probably does not guarrant to be implemented in native code, the users are likely to get some well-reviewed native code implementation. (This might be false for some not-so-common JREs, but Play! can't address the issue without requiring a native platform-specific library.)

A comment on linked article http://security.blogoverflow.com/2013/06/qotw-47-lessons-learned-and-misconceptions-regarding-encryption-and-cryptology/:

Don’t use the same key for both encryption and authentication. Don’t use the same key for both encryption and signing.

I don't take this advice too generally. In fact, AEAD modes (which essentially do this) are rather recommended. I know two arguments against using the same key for both encryption and signing, none of them seems to be critical there. But there is some room for improvement. If we don't want to have multiple keys (which essentially requires application.secret/play.crypto.secret to be split to multiple config options), we might use some subkey generation for that. This will require versioning when the backward compatibility is needed, but in such case, the versioning would be probably needed anyway. (I may elaborate.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment