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

[leleuj]

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]

@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]

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]

Pushing this back as 4.2 is focussed on PRs.

[darioseidl]

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]

@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 #9361 to address that - are you able to provide a PR to add documentation?

[darioseidl]

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