From 9f8c7a3aaaf60763085aeeef2e314ce4ad673a63 Mon Sep 17 00:00:00 2001 From: Yutong Sean Date: Tue, 21 Dec 2021 10:16:01 +0800 Subject: [PATCH 1/3] HBASE-26613 The logic of the method incrementIV in Encryption class has problem --- .../hadoop/hbase/io/crypto/Encryption.java | 24 +++++++++---------- .../hbase/io/crypto/TestEncryption.java | 13 ++++++++++ 2 files changed, 25 insertions(+), 12 deletions(-) diff --git a/hbase-common/src/main/java/org/apache/hadoop/hbase/io/crypto/Encryption.java b/hbase-common/src/main/java/org/apache/hadoop/hbase/io/crypto/Encryption.java index 6adcae5b22e3..504f38110507 100644 --- a/hbase-common/src/main/java/org/apache/hadoop/hbase/io/crypto/Encryption.java +++ b/hbase-common/src/main/java/org/apache/hadoop/hbase/io/crypto/Encryption.java @@ -21,6 +21,8 @@ import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; +import java.math.BigInteger; +import java.nio.ByteBuffer; import java.security.Key; import java.security.MessageDigest; import java.security.NoSuchAlgorithmException; @@ -33,6 +35,7 @@ import javax.crypto.spec.PBEKeySpec; import javax.crypto.spec.SecretKeySpec; +import org.apache.commons.codec.binary.Hex; import org.apache.commons.io.IOUtils; import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.hbase.HBaseConfiguration; @@ -640,20 +643,17 @@ public static void incrementIv(byte[] iv) { } public static void incrementIv(byte[] iv, int v) { + // v should be > 0 int length = iv.length; - boolean carry = true; - // TODO: Optimize for v > 1, e.g. 16, 32 - do { - for (int i = 0; i < length; i++) { - if (carry) { - iv[i] = (byte) ((iv[i] + 1) & 0xFF); - carry = 0 == iv[i]; - } else { - break; - } + int sum = 0; + for (int i = 0; i < length; i++) { + if (v <= 0) { + break; } - v--; - } while (v > 0); + sum = v + iv[i]; + v = sum / 256; + iv[i] = (byte)(sum % 256); + } } /** diff --git a/hbase-common/src/test/java/org/apache/hadoop/hbase/io/crypto/TestEncryption.java b/hbase-common/src/test/java/org/apache/hadoop/hbase/io/crypto/TestEncryption.java index 829be39f6120..f7835b3d18de 100644 --- a/hbase-common/src/test/java/org/apache/hadoop/hbase/io/crypto/TestEncryption.java +++ b/hbase-common/src/test/java/org/apache/hadoop/hbase/io/crypto/TestEncryption.java @@ -89,6 +89,19 @@ public void testTypicalHFileBlocks() throws Exception { } } + @Test + public void testIncrementIV() { + byte[] iv = new byte[] {1, 2, 3}; + Encryption.incrementIv(iv); + assertTrue(Bytes.equals(iv, new byte[] {2, 2, 3})); + + Encryption.incrementIv(iv, 255); + assertTrue(Bytes.equals(iv, new byte[] {1, 3, 3})); + + Encryption.incrementIv(iv, 1024); + assertTrue(Bytes.equals(iv, new byte[] {1, 7, 3})); + } + private void checkTransformSymmetry(byte[] keyBytes, byte[] iv, byte[] plaintext) throws Exception { LOG.info("checkTransformSymmetry: AES, plaintext length = " + plaintext.length); From 2dcf24f5b260492bc26c42fc1614a538688abf37 Mon Sep 17 00:00:00 2001 From: Yutong Sean Date: Tue, 21 Dec 2021 10:30:36 +0800 Subject: [PATCH 2/3] Remove useless imports --- .../java/org/apache/hadoop/hbase/io/crypto/Encryption.java | 3 --- 1 file changed, 3 deletions(-) diff --git a/hbase-common/src/main/java/org/apache/hadoop/hbase/io/crypto/Encryption.java b/hbase-common/src/main/java/org/apache/hadoop/hbase/io/crypto/Encryption.java index 504f38110507..537917f5efaa 100644 --- a/hbase-common/src/main/java/org/apache/hadoop/hbase/io/crypto/Encryption.java +++ b/hbase-common/src/main/java/org/apache/hadoop/hbase/io/crypto/Encryption.java @@ -21,8 +21,6 @@ import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; -import java.math.BigInteger; -import java.nio.ByteBuffer; import java.security.Key; import java.security.MessageDigest; import java.security.NoSuchAlgorithmException; @@ -35,7 +33,6 @@ import javax.crypto.spec.PBEKeySpec; import javax.crypto.spec.SecretKeySpec; -import org.apache.commons.codec.binary.Hex; import org.apache.commons.io.IOUtils; import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.hbase.HBaseConfiguration; From acb46fd118f4916878fd3b226f1cbb3a0d3d99fe Mon Sep 17 00:00:00 2001 From: Yutong Sean Date: Tue, 21 Dec 2021 10:58:11 +0800 Subject: [PATCH 3/3] Regard iv as unsigned bytes --- .../java/org/apache/hadoop/hbase/io/crypto/Encryption.java | 2 +- .../org/apache/hadoop/hbase/io/crypto/TestEncryption.java | 7 +++++++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/hbase-common/src/main/java/org/apache/hadoop/hbase/io/crypto/Encryption.java b/hbase-common/src/main/java/org/apache/hadoop/hbase/io/crypto/Encryption.java index 537917f5efaa..807758958d0a 100644 --- a/hbase-common/src/main/java/org/apache/hadoop/hbase/io/crypto/Encryption.java +++ b/hbase-common/src/main/java/org/apache/hadoop/hbase/io/crypto/Encryption.java @@ -647,7 +647,7 @@ public static void incrementIv(byte[] iv, int v) { if (v <= 0) { break; } - sum = v + iv[i]; + sum = v + (iv[i] & 0xFF); v = sum / 256; iv[i] = (byte)(sum % 256); } diff --git a/hbase-common/src/test/java/org/apache/hadoop/hbase/io/crypto/TestEncryption.java b/hbase-common/src/test/java/org/apache/hadoop/hbase/io/crypto/TestEncryption.java index f7835b3d18de..8d850a7aa4e2 100644 --- a/hbase-common/src/test/java/org/apache/hadoop/hbase/io/crypto/TestEncryption.java +++ b/hbase-common/src/test/java/org/apache/hadoop/hbase/io/crypto/TestEncryption.java @@ -92,6 +92,7 @@ public void testTypicalHFileBlocks() throws Exception { @Test public void testIncrementIV() { byte[] iv = new byte[] {1, 2, 3}; + byte[] iv_neg = new byte[] {-3, -13, 25}; Encryption.incrementIv(iv); assertTrue(Bytes.equals(iv, new byte[] {2, 2, 3})); @@ -100,6 +101,12 @@ public void testIncrementIV() { Encryption.incrementIv(iv, 1024); assertTrue(Bytes.equals(iv, new byte[] {1, 7, 3})); + + Encryption.incrementIv(iv_neg); + assertTrue(Bytes.equals(iv_neg, new byte[] {-2, -13, 25})); + + Encryption.incrementIv(iv_neg, 5); + assertTrue(Bytes.equals(iv_neg, new byte[] {3, -12, 25})); } private void checkTransformSymmetry(byte[] keyBytes, byte[] iv, byte[] plaintext)