HBASE-21168 Insecure Randomness in BloomFilterUtil

Flagged by Fortify static analysis

Signed-off-by: Andrew Purtell <apurtell@apache.org>
Signed-off-by: Mingliang Liu <liuml07@apache.org>
This commit is contained in:
Mike Drob 2018-09-07 10:28:30 -05:00
parent 3810ba2c6e
commit 0075093d21
No known key found for this signature in database
GPG Key ID: 3E48C0C6EF362B9E
2 changed files with 35 additions and 26 deletions

View File

@ -21,9 +21,11 @@ import java.text.NumberFormat;
import java.util.Random; import java.util.Random;
import org.apache.hadoop.hbase.Cell; import org.apache.hadoop.hbase.Cell;
import org.apache.yetus.audience.InterfaceAudience;
import org.apache.hadoop.hbase.nio.ByteBuff; import org.apache.hadoop.hbase.nio.ByteBuff;
import org.apache.hadoop.hbase.regionserver.BloomType; 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 * Utility methods related to BloomFilters
@ -75,12 +77,17 @@ public final class BloomFilterUtil {
return (long) Math.ceil(maxKeys * (-Math.log(errorRate) / LOG2_SQUARED)); return (long) Math.ceil(maxKeys * (-Math.log(errorRate) / LOG2_SQUARED));
} }
public static void setFakeLookupMode(boolean enabled) { /**
if (enabled) { * Sets a random generator to be used for look-ups instead of computing hashes. Can be used to
randomGeneratorForTest = new Random(283742987L); * simulate uniformity of accesses better in a test environment. Should not be set in a real
} else { * environment where correctness matters!
randomGeneratorForTest = null; * <p>
} * 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 <T> boolean contains(ByteBuff bloomBuf, int bloomOffset, int bloomSize, Hash hash, private static <T> boolean contains(ByteBuff bloomBuf, int bloomOffset, int bloomSize, Hash hash,
int hashCount, HashKey<T> hashKey) { int hashCount, HashKey<T> hashKey) {
int hash1 = hash.hash(hashKey, 0); int hash1 = hash.hash(hashKey, 0);
int hash2 = hash.hash(hashKey, hash1);
int bloomBitSize = bloomSize << 3; int bloomBitSize = bloomSize << 3;
int hash2 = 0;
int compositeHash = 0;
if (randomGeneratorForTest == null) { if (randomGeneratorForTest == null) {
// Production mode. // Production mode
int compositeHash = hash1; compositeHash = hash1;
for (int i = 0; i < hashCount; i++) { hash2 = hash.hash(hashKey, hash1);
int hashLoc = Math.abs(compositeHash % bloomBitSize); }
compositeHash += hash2;
if (!checkBit(hashLoc, bloomBuf, bloomOffset)) { for (int i = 0; i < hashCount; i++) {
return false; int hashLoc = (randomGeneratorForTest == null
} // Production mode
} ? Math.abs(compositeHash % bloomBitSize)
} else { // Test mode with "fake look-ups" to estimate "ideal false positive rate"
// Test mode with "fake lookups" to estimate "ideal false positive rate". : randomGeneratorForTest.nextInt(bloomBitSize));
for (int i = 0; i < hashCount; i++) { compositeHash += hash2;
int hashLoc = randomGeneratorForTest.nextInt(bloomBitSize); if (!checkBit(hashLoc, bloomBuf, bloomOffset)) {
if (!checkBit(hashLoc, bloomBuf, bloomOffset)){ return false;
return false;
}
} }
} }
return true; return true;

View File

@ -225,7 +225,9 @@ public class TestCompoundBloomFilter {
// Test for false positives (some percentage allowed). We test in two modes: // Test for false positives (some percentage allowed). We test in two modes:
// "fake lookup" which ignores the key distribution, and production mode. // "fake lookup" which ignores the key distribution, and production mode.
for (boolean fakeLookupEnabled : new boolean[] { true, false }) { for (boolean fakeLookupEnabled : new boolean[] { true, false }) {
BloomFilterUtil.setFakeLookupMode(fakeLookupEnabled); if (fakeLookupEnabled) {
BloomFilterUtil.setRandomGeneratorForTest(new Random(283742987L));
}
try { try {
String fakeLookupModeStr = ", fake lookup is " + (fakeLookupEnabled ? String fakeLookupModeStr = ", fake lookup is " + (fakeLookupEnabled ?
"enabled" : "disabled"); "enabled" : "disabled");
@ -275,7 +277,7 @@ public class TestCompoundBloomFilter {
validateFalsePosRate(falsePosRate, nTrials, -2.58, cbf, validateFalsePosRate(falsePosRate, nTrials, -2.58, cbf,
fakeLookupModeStr); fakeLookupModeStr);
} finally { } finally {
BloomFilterUtil.setFakeLookupMode(false); BloomFilterUtil.setRandomGeneratorForTest(null);
} }
} }