You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hbase.apache.org by md...@apache.org on 2018/09/12 14:58:07 UTC

[2/3] hbase git commit: HBASE-21168 Insecure Randomness in BloomFilterUtil

HBASE-21168 Insecure Randomness in BloomFilterUtil

Flagged by Fortify static analysis

Signed-off-by: Andrew Purtell <ap...@apache.org>
Signed-off-by: Mingliang Liu <li...@apache.org>


Project: http://git-wip-us.apache.org/repos/asf/hbase/repo
Commit: http://git-wip-us.apache.org/repos/asf/hbase/commit/e9d23d5d
Tree: http://git-wip-us.apache.org/repos/asf/hbase/tree/e9d23d5d
Diff: http://git-wip-us.apache.org/repos/asf/hbase/diff/e9d23d5d

Branch: refs/heads/branch-2
Commit: e9d23d5d25571f4bf682f53303c40e4a18284e66
Parents: e1548d3
Author: Mike Drob <md...@apache.org>
Authored: Fri Sep 7 10:28:30 2018 -0500
Committer: Mike Drob <md...@apache.org>
Committed: Wed Sep 12 09:51:57 2018 -0500

----------------------------------------------------------------------
 .../hadoop/hbase/util/BloomFilterUtil.java      | 55 +++++++++++---------
 .../regionserver/TestCompoundBloomFilter.java   |  6 ++-
 2 files changed, 35 insertions(+), 26 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/hbase/blob/e9d23d5d/hbase-server/src/main/java/org/apache/hadoop/hbase/util/BloomFilterUtil.java
----------------------------------------------------------------------
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 b1b3bcc..33bea7a 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!
+   * <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,
       int hashCount, HashKey<T> 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;

http://git-wip-us.apache.org/repos/asf/hbase/blob/e9d23d5d/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestCompoundBloomFilter.java
----------------------------------------------------------------------
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 0b17d28..424a788 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);
       }
     }