[
https://issues.apache.org/jira/browse/HADOOP-11343?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14232399#comment-14232399
]
Jerry Chen commented on HADOOP-11343:
-------------------------------------
Thanks Mike for commenting.
The code above is not the solution. It was copied from the source code.
The risk of the existing implementation is actually, when generating the
initIV, it was filled with 16 bytes random number. And in the calculateIV
algorithm, it adds a long counter to the 8 bytes of initIV. This means that
even the counter is not very big, long += counter still has a good possibility
to overflow. For example, as the initIV is random, it might happens to be some
big value in the 8 bytes, and a smaller counter will cause the result to be
overflow.
When saying that "the overflow get lost", the carry doesn't add up to the above
8 bytes. Since we have 16 bytes, but we are doing 8 bytes arithmetic. While for
"standard aes-ctr", I mean that the increment of the counter is happening on
128bits (16 bytes) space, other than on 8 bytes space. Checking the following
code from openssl:
http://www.mcs.anl.gov/~bester/gsi/coverage/4.0.6/source-aes_ctr.c.html
You can see that the AES_ctr128_inc increment the counter with 128bytes add
arithmetic.
We have two approaches for solving this:
1. When generating 16 bytes initIV, we fill the lower 8 bytes with random IV
and leave the higher 8 bytes zero for counter This is actually suggested in
http://www.mcs.anl.gov/~bester/gsi/coverage/4.0.6/source-aes_ctr.c.html as
following:
* This algorithm assumes that the counter is in the x lower bits
* of the IV (ivec), and that the application has full control over
* overflow and the rest of the IV. This implementation takes NO
* responsability for checking that the counter doesn't overflow
* into the rest of the IV when incremented
We choose the x = 8
2. for calculateIV, we can always do a identical IV + Counter algorithm on 16
bytes other than on high (big endian) 8 bytes. If solution #1 is not done, from
the probability, it is still possible that the IV + counter will overflow the
whole 16 bytes if the initIV happens randomized to very large number. But it
still has two benefits, a. the possibility would be much much lower for a 16
bytes number overflow with a long counter. b. Even the 16 bytes overflow, the
algorithm is identical with the AES_ctr128_inc which means the encrypted block
can be still decrypted by a "standard AES-ctr" cipher such as Java Cipher. The
current calculateIV implementation cannot achieve this if the 8 bytes
overflowed because the final IV used for the block will be different between a
Java Cipher and our implementation, even with the same input if initIV and
counter.
We have get some initial code done and can provided the patch for review.
Please help a review if possible.
Thanks again for sharing the opinion.
> Overflow is not properly handled in caclulating final iv for AES CTR
> --------------------------------------------------------------------
>
> Key: HADOOP-11343
> URL: https://issues.apache.org/jira/browse/HADOOP-11343
> Project: Hadoop Common
> Issue Type: Bug
> Components: security
> Affects Versions: trunk-win
> Reporter: Jerry Chen
>
> In the AesCtrCryptoCodec calculateIV, as the init IV is a random generated 16
> bytes,
> final byte[] iv = new byte[cc.getCipherSuite().getAlgorithmBlockSize()];
> cc.generateSecureRandom(iv);
> Then the following calculation of iv and counter on 8 bytes (64bit) space
> would easily cause overflow and this overflow gets lost. The result would be
> the 128 bit data block was encrypted with a wrong counter and cannot be
> decrypted by standard aes-ctr.
> /**
> * The IV is produced by adding the initial IV to the counter. IV length
> * should be the same as {@link #AES_BLOCK_SIZE}
> */
> @Override
> public void calculateIV(byte[] initIV, long counter, byte[] IV) {
> Preconditions.checkArgument(initIV.length == AES_BLOCK_SIZE);
> Preconditions.checkArgument(IV.length == AES_BLOCK_SIZE);
>
> System.arraycopy(initIV, 0, IV, 0, CTR_OFFSET);
> long l = 0;
> for (int i = 0; i < 8; i++) {
> l = ((l << 8) | (initIV[CTR_OFFSET + i] & 0xff));
> }
> l += counter;
> IV[CTR_OFFSET + 0] = (byte) (l >>> 56);
> IV[CTR_OFFSET + 1] = (byte) (l >>> 48);
> IV[CTR_OFFSET + 2] = (byte) (l >>> 40);
> IV[CTR_OFFSET + 3] = (byte) (l >>> 32);
> IV[CTR_OFFSET + 4] = (byte) (l >>> 24);
> IV[CTR_OFFSET + 5] = (byte) (l >>> 16);
> IV[CTR_OFFSET + 6] = (byte) (l >>> 8);
> IV[CTR_OFFSET + 7] = (byte) (l);
> }
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
