From 86220b1c80f829693edb21cc58dded72704459be Mon Sep 17 00:00:00 2001 From: Mike Drob Date: Fri, 7 Sep 2018 10:28:30 -0500 Subject: [PATCH] HBASE-21168 Insecure Randomness in BloomFilterUtil Flagged by Fortify static analysis --- .../apache/hadoop/hbase/util/BloomFilterUtil.java | 55 ++++++++++++---------- .../regionserver/TestCompoundBloomFilter.java | 6 ++- 2 files changed, 35 insertions(+), 26 deletions(-) diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/util/BloomFilterUtil.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/util/BloomFilterUtil.java index b1b3bccdcb..33bea7a498 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/util/BloomFilterUtil.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/util/BloomFilterUtil.java @@ -21,9 +21,11 @@ import java.text.NumberFormat; import java.util.Random; import org.apache.hadoop.hbase.Cell; -import org.apache.yetus.audience.InterfaceAudience; import org.apache.hadoop.hbase.nio.ByteBuff; import org.apache.hadoop.hbase.regionserver.BloomType; +import org.apache.yetus.audience.InterfaceAudience; + +import org.apache.hbase.thirdparty.com.google.common.annotations.VisibleForTesting; /** * Utility methods related to BloomFilters @@ -75,12 +77,17 @@ public final class BloomFilterUtil { return (long) Math.ceil(maxKeys * (-Math.log(errorRate) / LOG2_SQUARED)); } - public static void setFakeLookupMode(boolean enabled) { - if (enabled) { - randomGeneratorForTest = new Random(283742987L); - } else { - randomGeneratorForTest = null; - } + /** + * Sets a random generator to be used for look-ups instead of computing hashes. Can be used to + * simulate uniformity of accesses better in a test environment. Should not be set in a real + * environment where correctness matters! + *

+ * This gets used in {@link #contains(ByteBuff, int, int, Hash, int, HashKey)} + * @param random The random number source to use, or null to compute actual hashes + */ + @VisibleForTesting + public static void setRandomGeneratorForTest(Random random) { + randomGeneratorForTest = random; } /** @@ -205,26 +212,26 @@ public final class BloomFilterUtil { private static boolean contains(ByteBuff bloomBuf, int bloomOffset, int bloomSize, Hash hash, int hashCount, HashKey hashKey) { int hash1 = hash.hash(hashKey, 0); - int hash2 = hash.hash(hashKey, hash1); int bloomBitSize = bloomSize << 3; + int hash2 = 0; + int compositeHash = 0; + if (randomGeneratorForTest == null) { - // Production mode. - int compositeHash = hash1; - for (int i = 0; i < hashCount; i++) { - int hashLoc = Math.abs(compositeHash % bloomBitSize); - compositeHash += hash2; - if (!checkBit(hashLoc, bloomBuf, bloomOffset)) { - return false; - } - } - } else { - // Test mode with "fake lookups" to estimate "ideal false positive rate". - for (int i = 0; i < hashCount; i++) { - int hashLoc = randomGeneratorForTest.nextInt(bloomBitSize); - if (!checkBit(hashLoc, bloomBuf, bloomOffset)){ - return false; - } + // Production mode + compositeHash = hash1; + hash2 = hash.hash(hashKey, hash1); + } + + for (int i = 0; i < hashCount; i++) { + int hashLoc = (randomGeneratorForTest == null + // Production mode + ? Math.abs(compositeHash % bloomBitSize) + // Test mode with "fake look-ups" to estimate "ideal false positive rate" + : randomGeneratorForTest.nextInt(bloomBitSize)); + compositeHash += hash2; + if (!checkBit(hashLoc, bloomBuf, bloomOffset)) { + return false; } } return true; diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestCompoundBloomFilter.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestCompoundBloomFilter.java index 0b17d286cc..424a78807e 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestCompoundBloomFilter.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestCompoundBloomFilter.java @@ -225,7 +225,9 @@ public class TestCompoundBloomFilter { // Test for false positives (some percentage allowed). We test in two modes: // "fake lookup" which ignores the key distribution, and production mode. for (boolean fakeLookupEnabled : new boolean[] { true, false }) { - BloomFilterUtil.setFakeLookupMode(fakeLookupEnabled); + if (fakeLookupEnabled) { + BloomFilterUtil.setRandomGeneratorForTest(new Random(283742987L)); + } try { String fakeLookupModeStr = ", fake lookup is " + (fakeLookupEnabled ? "enabled" : "disabled"); @@ -275,7 +277,7 @@ public class TestCompoundBloomFilter { validateFalsePosRate(falsePosRate, nTrials, -2.58, cbf, fakeLookupModeStr); } finally { - BloomFilterUtil.setFakeLookupMode(false); + BloomFilterUtil.setRandomGeneratorForTest(null); } } -- 2.16.1