CASSANDRA-11452: Improved resistence to hash collision attacks

When the candidate and victim's hash code are equal they have the same
estimated frequency. This results in the candidate being rejected. If
a hash collision attack then the cache will not admit any new entries
and the hit rate will be very low (causing poor response times).

Ideally we'd use a 64-bit hash, but Java does not provide that natively
and, for now, I'd prefer not exposing a builder function (K -> long). A
longHashCode() would not fix the problem, but greatly reduce its
likelihood.

This patch evicts the victim in the case of a collision. This should
allow candidates to flow through the eden -> probation segments. This
should have a similar effect as using freq1 >= freq2 comparision (not
strictly >) without degrading the hit rate in the LIRS traces.

@blambov please review so that we can iterate and find a good
compromise solution

caffeine/src/javaPoet/java/com/github/benmanes/caffeine/cache/Specifications.java

@@ -103,8 +103,7 @@ public final class Specifications {
   public static final TypeName WRITE_QUEUE = ParameterizedTypeName.get(
       WRITE_QUEUE_TYPE, ClassName.get(Runnable.class));
 
-  public static final TypeName FREQUENCY_SKETCH = ParameterizedTypeName.get(
-      ClassName.get("com.github.benmanes.caffeine.cache", "FrequencySketch"), kTypeVar);
+  public static final TypeName FREQUENCY_SKETCH = ClassName.get(PACKAGE_NAME, "FrequencySketch");
 
   private Specifications() {}
 

caffeine/src/jmh/java/com/github/benmanes/caffeine/cache/FrequencySketchBenchmark.java

@@ -38,12 +38,12 @@ public class FrequencySketchBenchmark {
 
   int index = 0;
   Integer[] ints;
-  FrequencySketch<Integer> sketch;
+  FrequencySketch sketch;
 
   @Setup
   public void setup() {
     ints = new Integer[SIZE];
-    sketch = new FrequencySketch<>();
+    sketch = new FrequencySketch();
     sketch.ensureCapacity(ITEMS);
 
     NumberGenerator generator = new ScrambledZipfianGenerator(ITEMS);

caffeine/src/jmh/java/com/github/benmanes/caffeine/cache/impl/Cache2k.java

@@ -26,11 +26,11 @@
 public final class Cache2k<K, V> implements BasicCache<K, V> {
   private final Cache<K, V> cache;
 
-  @SuppressWarnings("unchecked")
+  @SuppressWarnings({"unchecked", "deprecation"})
   public Cache2k(Class<?> implementation, int maximumSize) {
     cache = (Cache<K, V>) CacheBuilder.newCache(Object.class, Object.class)
         .implementation(implementation)
-        .maxSize(maximumSize)
+        .entryCapacity(maximumSize)
         .eternal(true)
         .build();
   }

caffeine/src/main/java/com/github/benmanes/caffeine/cache/BoundedLocalCache.java

@@ -359,7 +359,7 @@ protected boolean isWeighted() {
     return (weigher != Weigher.singletonWeigher());
   }
 
-  protected FrequencySketch<K> frequencySketch() {
+  protected FrequencySketch frequencySketch() {
     throw new UnsupportedOperationException();
   }
 
@@ -578,10 +578,21 @@ void evictFromMain(int candidates) {
         continue;
       }
 
+      // Evict the victim on a potential hash collision attack
+      int victimHash = victimKey.hashCode();
+      int candidateHash = candidateKey.hashCode();
+      if (victimHash == candidateHash) {
+        candidates--;
+        Node<K, V> evict = victim;
+        victim = victim.getNextInAccessOrder();
+        evictEntry(evict, RemovalCause.SIZE, 0L);
+        continue;
+      }
+
       // Evict the entry with the lowest frequency
       candidates--;
-      int victimFreq = frequencySketch().frequency(victimKey);
-      int candidateFreq = frequencySketch().frequency(candidateKey);
+      int victimFreq = frequencySketch().frequency(victimHash);
+      int candidateFreq = frequencySketch().frequency(candidateHash);
       if (candidateFreq > victimFreq) {
         Node<K, V> evict = victim;
         victim = victim.getNextInAccessOrder();
@@ -976,7 +987,7 @@ void onAccess(Node<K, V> node) {
       if (key == null) {
         return;
       }
-      frequencySketch().increment(key);
+      frequencySketch().increment(key.hashCode());
       if (node.inEden()) {
         reorder(accessOrderEdenDeque(), node);
       } else if (node.inMainProbation()) {
@@ -1095,7 +1106,7 @@ public void run() {
 
         K key = node.getKey();
         if (key != null) {
-          frequencySketch().increment(key);
+          frequencySketch().increment(key.hashCode());
         }
       }
 
@@ -2059,7 +2070,7 @@ public Set<Entry<K, V>> entrySet() {
   @SuppressWarnings("GuardedByChecker")
   Map<K, V> evictionOrder(int limit, Function<V, V> transformer, boolean ascending) {
     Comparator<Node<K, V>> comparator = Comparator.comparingInt(node ->
-        frequencySketch().frequency(node.getKey()));
+        frequencySketch().frequency(node.getKey().hashCode()));
     comparator = ascending ? comparator : comparator.reversed();
     PeekingIterator<Node<K, V>> eden;
     PeekingIterator<Node<K, V>> main;

caffeine/src/main/java/com/github/benmanes/caffeine/cache/FrequencySketch.java

@@ -18,7 +18,6 @@
 import java.util.concurrent.ThreadLocalRandom;
 
 import javax.annotation.Nonnegative;
-import javax.annotation.Nonnull;
 import javax.annotation.concurrent.NotThreadSafe;
 
 /**
@@ -29,7 +28,7 @@
  * @author [email protected] (Ben Manes)
  */
 @NotThreadSafe
-final class FrequencySketch<E> {
+final class FrequencySketch {
 
   /*
    * This class maintains a 4-bit CountMinSketch [1] with periodic aging to provide the popularity
@@ -115,16 +114,16 @@ public boolean isNotInitialized() {
   /**
    * Returns the estimated number of occurrences of an element, up to the maximum (15).
    *
-   * @param e the element to count occurrences of
+   * @param hashCode the hash code of the element to count occurrences of
    * @return the estimated number of occurrences of the element; possibly zero but never negative
    */
   @Nonnegative
-  public int frequency(@Nonnull E e) {
+  public int frequency(int hashCode) {
     if (isNotInitialized()) {
       return 0;
     }
 
-    int hash = spread(e.hashCode());
+    int hash = spread(hashCode);
     int start = (hash & 3) << 2;
     int frequency = Integer.MAX_VALUE;
     for (int i = 0; i < 4; i++) {
@@ -140,14 +139,14 @@ public int frequency(@Nonnull E e) {
    * of all elements will be periodically down sampled when the observed events exceeds a threshold.
    * This process provides a frequency aging to allow expired long term entries to fade away.
    *
-   * @param e the element to add
+   * @param hashCode the hash code of the element to count occurrences of
    */
-  public void increment(@Nonnull E e) {
+  public void increment(int hashCode) {
     if (isNotInitialized()) {
       return;
     }
 
-    int hash = spread(e.hashCode());
+    int hash = spread(hashCode);
     int start = (hash & 3) << 2;
 
     // Loop unrolling improves throughput by 5m ops/s

caffeine/src/test/java/com/github/benmanes/caffeine/cache/FrequencySketchTest.java

@@ -30,21 +30,21 @@
  * @author [email protected] (Ben Manes)
  */
 public final class FrequencySketchTest {
-  final Integer item = ThreadLocalRandom.current().nextInt();
+  final int item = ThreadLocalRandom.current().nextInt();
 
   @Test
   public void construc() {
-    FrequencySketch<Integer> sketch = new FrequencySketch<>();
+    FrequencySketch sketch = new FrequencySketch();
     assertThat(sketch.table, is(nullValue()));
   }
 
   @Test(dataProvider = "sketch", expectedExceptions = IllegalArgumentException.class)
-  public void ensureCapacity_negative(FrequencySketch<Integer> sketch) {
+  public void ensureCapacity_negative(FrequencySketch sketch) {
     sketch.ensureCapacity(-1);
   }
 
   @Test(dataProvider = "sketch")
-  public void ensureCapacity_smaller(FrequencySketch<Integer> sketch) {
+  public void ensureCapacity_smaller(FrequencySketch sketch) {
     int size = sketch.table.length;
     sketch.ensureCapacity(size / 2);
     assertThat(sketch.table.length, is(size));
@@ -53,7 +53,7 @@ public void ensureCapacity_smaller(FrequencySketch<Integer> sketch) {
   }
 
   @Test(dataProvider = "sketch")
-  public void ensureCapacity_larger(FrequencySketch<Integer> sketch) {
+  public void ensureCapacity_larger(FrequencySketch sketch) {
     int size = sketch.table.length;
     sketch.ensureCapacity(size * 2);
     assertThat(sketch.table.length, is(size * 2));
@@ -62,21 +62,21 @@ public void ensureCapacity_larger(FrequencySketch<Integer> sketch) {
   }
 
   @Test(dataProvider = "sketch")
-  public void increment_once(FrequencySketch<Integer> sketch) {
+  public void increment_once(FrequencySketch sketch) {
     sketch.increment(item);
     assertThat(sketch.frequency(item), is(1));
   }
 
   @Test(dataProvider = "sketch")
-  public void increment_max(FrequencySketch<Integer> sketch) {
+  public void increment_max(FrequencySketch sketch) {
     for (int i = 0; i < 20; i++) {
       sketch.increment(item);
     }
     assertThat(sketch.frequency(item), is(15));
   }
 
   @Test(dataProvider = "sketch")
-  public void increment_distinct(FrequencySketch<Integer> sketch) {
+  public void increment_distinct(FrequencySketch sketch) {
     sketch.increment(item);
     sketch.increment(item + 1);
     assertThat(sketch.frequency(item), is(1));
@@ -87,7 +87,7 @@ public void increment_distinct(FrequencySketch<Integer> sketch) {
   @Test
   public void reset() {
     boolean reset = false;
-    FrequencySketch<Integer> sketch = new FrequencySketch<>();
+    FrequencySketch sketch = new FrequencySketch();
     sketch.ensureCapacity(64);
 
     for (int i = 1; i < 20 * sketch.table.length; i++) {
@@ -103,20 +103,20 @@ public void reset() {
 
   @Test
   public void heavyHitters() {
-    FrequencySketch<Double> sketch = makeSketch(512);
+    FrequencySketch sketch = makeSketch(512);
     for (int i = 100; i < 100_000; i++) {
-      sketch.increment((double) i);
+      sketch.increment(Double.hashCode(i));
     }
     for (int i = 0; i < 10; i += 2) {
       for (int j = 0; j < i; j++) {
-        sketch.increment((double) i);
+        sketch.increment(Double.hashCode(i));
       }
     }
 
     // A perfect popularity count yields an array [0, 0, 2, 0, 4, 0, 6, 0, 8, 0]
     int[] popularity = new int[10];
     for (int i = 0; i < 10; i++) {
-      popularity[i] = sketch.frequency((double) i);
+      popularity[i] = sketch.frequency(Double.hashCode(i));
     }
     for (int i = 0; i < popularity.length; i++) {
       if ((i == 0) || (i == 1) || (i == 3) || (i == 5) || (i == 7) || (i == 9)) {
@@ -136,8 +136,8 @@ public Object[][] providesSketch() {
     return new Object[][] {{ makeSketch(512) }};
   }
 
-  private static <T> FrequencySketch<T> makeSketch(long maximumSize) {
-    FrequencySketch<T> sketch = new FrequencySketch<>();
+  private static FrequencySketch makeSketch(long maximumSize) {
+    FrequencySketch sketch = new FrequencySketch();
     sketch.ensureCapacity(maximumSize);
     ensureRandomSeed(sketch);
     return sketch;

caffeine/src/test/java/com/github/benmanes/caffeine/cache/RandomSeedEnforcer.java

@@ -42,7 +42,7 @@ public static void ensureRandomSeed(Cache<?, ?> cache) {
   }
 
   /** Force the random seed to a predictable value. */
-  public static void ensureRandomSeed(FrequencySketch<?> sketch) {
+  public static void ensureRandomSeed(FrequencySketch sketch) {
     try {
       Field field = FrequencySketch.class.getDeclaredField("randomSeed");
       field.setAccessible(true);