spring-projects / spring-security

Spring Security
http://spring.io/projects/spring-security
Apache License 2.0
8.55k stars 5.79k forks source link

The default initialization vector size of the AesBytesEncryptor should be 12 bytes for GCM #3879

Open leleuj opened 8 years ago

leleuj commented 8 years ago

Summary

When trying to decrypt data with Node.js (AES256, GCM), I realized the initialization vector was of 12 bytes while it's 16 bytes length in the AesBytesEncryptor: https://github.com/spring-projects/spring-security/blob/master/crypto/src/main/java/org/springframework/security/crypto/encrypt/AesBytesEncryptor.java#L63

The spec recommends to use initialization vectors of 96 bits length: http://csrc.nist.gov/groups/ST/toolkit/BCM/documents/proposedmodes/gcm/gcm-revised-spec.pdf

An initialization vector IV , that can have any number of bits between 1 and 264. For a fixed value of the key, each IV value must be distinct, but need not have equal lengths. 96-bit IV values can be processed more efficiently, so that length is recommended for situations in which efficiency is critical.

Actual Behavior

Default size of the initialization vector for GCM cipher algorithm: 16 bytes

Expected Behavior

Default size of the initialization vector for GCM cipher algorithm: 12 bytes. Use: KeyGenerators.secureRandom(12).

Version

4.0.2.RELEASE

rwinch commented 8 years ago

@leleuj Thank you for the report

tl;dr - I think we need to provide an improvement for adjusting the IV size. However, the default value cannot change.

Thoughts on the ideal default value

The value you provided is really about efficiency (which may not be what a user wants to optimize around). Many users may be more interested in how secure the value is. For example, OWASP is optimizing for security rather than performance. OWASP states the following about AE:

In general, the tag sizes and the IV sizes should be set to maximum values.

Keep in mind that not all implementations of GCM are even optimized. JDK8's GCM is is notoriously slow. Fortunately, improvements to use CPU instructions for GHASH which is going to drastically improve performance (and can take advantage of the IV of 12 bytes). See JDK-8046943

Why the default won't change

We cannot change the default value because this will break passivity and (as mentioned above) is not currently broken.

Thoughts on improvements

That said I do see why users may want to be able to tune the IV. What's more is the static factory methods are misleading. The method was created multiple years ago in which time cryptography has evolved. Ideally we should create an implementation that can evolve with the times.

The tricky part is providing hooks into the parameters without making things too complicated for users as cryptography is very difficult to get right.

leleuj commented 8 years ago

I perfectly understand that the default value cannot change.

I don't know all platforms, but I think implementations tend to follow the 96 bits recommendation, "whether the efficiency is critical or not". At least, that's the case for OpenSSL on which the Node.js crypto module is based. And it seems to be the case for PHP, Ruby... which also relies on OpenSSL.

As an improvement, it would be great indeed that the Encryptors factory allows to build a AesBytesEncryptor with a specific IV generator (like KeyGenerators.secureRandom(12)).

rwinch commented 7 years ago

Pushing this back as 4.2 is focussed on PRs.

darioseidl commented 3 years ago

I was also surprised to see a 16 byte IV used in AesBytesEncryptor, where 12 bytes are usually recommended for AES/GCM. I think the JavaDoc of the class should mention that.

jzheaux commented 3 years ago

@darioseidl, I think it makes sense to update the JavaDoc for AesBytesEncryptor, as you indicated. Specifically, the constructors could be documented to explain 1. that a 16-byte IV is used by default and 2. that it can be changed by using one of the constructors that takes a BytesKeyGenerator. I've added https://github.com/spring-projects/spring-security/issues/9361 to address that - are you able to provide a PR to add documentation?

darioseidl commented 2 years ago

Oof, sorry, it took me so long, I came back to it now, but @prashanttholia was quicker. Thanks!