Use SHA256 in the replaycache, rather than SHA1

This migrates away from SHA1, and provides further hash flooding
protection on top of the randomised siphash implementation.

Add unit tests to make sure that different inputs don't have the
same hash.

diff --git a/changes/feature8961-replaycache-sha256 b/changes/feature8961-replaycache-sha256
new file mode 100644 (file)
index 0000000..c5b5c85
--- /dev/null
+++ b/changes/feature8961-replaycache-sha256
@@ -0,0 +1,4 @@
+  o Minor enhancement (replaycache):
+    - The replay cache now uses SHA256 instead of SHA1.
+      Implements feature #8961.
+      Patch by "teor", issue reported by "rransom".

diff --git a/src/or/replaycache.c b/src/or/replaycache.c
index 569e0736cb043ae91880180455d09b800b4c18e6..82e5c44d3d6e60122b152aa0d54172aa250130ab 100644 (file)
--- a/src/or/replaycache.c
+++ b/src/or/replaycache.c
@@ -23,7 +23,7 @@ replaycache_free(replaycache_t *r)
     return;
   }
 
-  if (r->digests_seen) digestmap_free(r->digests_seen, tor_free_);
+  if (r->digests_seen) digest256map_free(r->digests_seen, tor_free_);
 
   tor_free(r);
 }
@@ -54,7 +54,7 @@ replaycache_new(time_t horizon, time_t interval)
   r->scrub_interval = interval;
   r->scrubbed = 0;
   r->horizon = horizon;
-  r->digests_seen = digestmap_new();
+  r->digests_seen = digest256map_new();
 
  err:
   return r;
@@ -69,7 +69,7 @@ replaycache_add_and_test_internal(
     time_t *elapsed)
 {
   int rv = 0;
-  char digest[DIGEST_LEN];
+  uint8_t digest[DIGEST256_LEN];
   time_t *access_time;
 
   /* sanity check */
@@ -80,10 +80,10 @@ replaycache_add_and_test_internal(
   }
 
   /* compute digest */
-  crypto_digest(digest, (const char *)data, len);
+  crypto_digest256((char *)digest, (const char *)data, len, DIGEST_SHA256);
 
   /* check map */
-  access_time = digestmap_get(r->digests_seen, digest);
+  access_time = digest256map_get(r->digests_seen, digest);
 
   /* seen before? */
   if (access_time != NULL) {
@@ -114,7 +114,7 @@ replaycache_add_and_test_internal(
     /* No, so no hit and update the digest map with the current time */
     access_time = tor_malloc(sizeof(*access_time));
     *access_time = present;
-    digestmap_set(r->digests_seen, digest, access_time);
+    digest256map_set(r->digests_seen, digest, access_time);
   }
 
   /* now scrub the cache if it's time */
@@ -130,8 +130,8 @@ replaycache_add_and_test_internal(
 STATIC void
 replaycache_scrub_if_needed_internal(time_t present, replaycache_t *r)
 {
-  digestmap_iter_t *itr = NULL;
-  const char *digest;
+  digest256map_iter_t *itr = NULL;
+  const uint8_t *digest;
   void *valp;
   time_t *access_time;
 
@@ -149,19 +149,19 @@ replaycache_scrub_if_needed_internal(time_t present, replaycache_t *r)
   if (r->horizon == 0) return;
 
   /* okay, scrub time */
-  itr = digestmap_iter_init(r->digests_seen);
-  while (!digestmap_iter_done(itr)) {
-    digestmap_iter_get(itr, &digest, &valp);
+  itr = digest256map_iter_init(r->digests_seen);
+  while (!digest256map_iter_done(itr)) {
+    digest256map_iter_get(itr, &digest, &valp);
     access_time = (time_t *)valp;
     /* aged out yet? */
     if (*access_time < present - r->horizon) {
       /* Advance the iterator and remove this one */
-      itr = digestmap_iter_next_rmv(r->digests_seen, itr);
+      itr = digest256map_iter_next_rmv(r->digests_seen, itr);
       /* Free the value removed */
       tor_free(access_time);
     } else {
       /* Just advance the iterator */
-      itr = digestmap_iter_next(r->digests_seen, itr);
+      itr = digest256map_iter_next(r->digests_seen, itr);
     }
   }
 

diff --git a/src/or/replaycache.h b/src/or/replaycache.h
index 9b9daf383119e78404cd36213f2a562057b8e8f9..9c409f2fd7a2115767a788728fa392a68ac8716a 100644 (file)
--- a/src/or/replaycache.h
+++ b/src/or/replaycache.h
@@ -26,7 +26,7 @@ struct replaycache_s {
   /*
    * Digest map: keys are digests, values are times the digest was last seen
    */
-  digestmap_t *digests_seen;
+  digest256map_t *digests_seen;
 };
 
 #endif /* REPLAYCACHE_PRIVATE */

diff --git a/src/test/test_replay.c b/src/test/test_replay.c
index a02c160365242ed855c4ec9e655514fd1c6be6ff..7a0f0987769ae7b6b5041f609fd5e776b6c3c753 100644 (file)
--- a/src/test/test_replay.c
+++ b/src/test/test_replay.c
@@ -17,6 +17,20 @@ static const char *test_buffer =
   " occaecat cupidatat non proident, sunt in culpa qui officia deserunt"
   " mollit anim id est laborum.";
 
+static const char *test_buffer_2 =
+  "At vero eos et accusamus et iusto odio dignissimos ducimus qui blanditiis"
+  " praesentium voluptatum deleniti atque corrupti quos dolores et quas"
+  " molestias excepturi sint occaecati cupiditate non provident, similique"
+  " sunt in culpa qui officia deserunt mollitia animi, id est laborum et"
+  " dolorum fuga. Et harum quidem rerum facilis est et expedita distinctio."
+  " Nam libero tempore, cum soluta nobis est eligendi optio cumque nihil"
+  " impedit quo minus id quod maxime placeat facere possimus, omnis voluptas"
+  " assumenda est, omnis dolor repellendus. Temporibus autem quibusdam et aut"
+  " officiis debitis aut rerum necessitatibus saepe eveniet ut et voluptates"
+  " repudiandae sint et molestiae non recusandae. Itaque earum rerum hic"
+  " tenetur a sapiente delectus, ut aut reiciendis voluptatibus maiores alias"
+  " consequatur aut perferendis doloribus asperiores repellat.";
+
 static void
 test_replaycache_alloc(void *arg)
 {
@@ -83,6 +97,12 @@ test_replaycache_miss(void *arg)
         strlen(test_buffer), NULL);
   tt_int_op(result,OP_EQ, 0);
 
+  /* make sure a different buffer misses as well */
+  result =
+  replaycache_add_and_test_internal(1200, NULL, test_buffer_2,
+                                    strlen(test_buffer_2), NULL);
+  tt_int_op(result,OP_EQ, 0);
+
   /* poke the bad-parameter error case too */
   result =
     replaycache_add_and_test_internal(1200, NULL, test_buffer,
@@ -115,6 +135,18 @@ test_replaycache_hit(void *arg)
         strlen(test_buffer), NULL);
   tt_int_op(result,OP_EQ, 1);
 
+  /* make sure a different buffer misses then hits as well */
+
+  result =
+  replaycache_add_and_test_internal(1200, r, test_buffer_2,
+                                    strlen(test_buffer_2), NULL);
+  tt_int_op(result,OP_EQ, 0);
+
+  result =
+  replaycache_add_and_test_internal(1300, r, test_buffer_2,
+                                    strlen(test_buffer_2), NULL);
+  tt_int_op(result,OP_EQ, 1);
+
  done:
   if (r) replaycache_free(r);
 
@@ -245,7 +277,7 @@ test_replaycache_scrub(void *arg)
   /* Make sure we hit the aging-out case too */
   replaycache_scrub_if_needed_internal(1500, r);
   /* Assert that we aged it */
-  tt_int_op(digestmap_size(r->digests_seen),OP_EQ, 0);
+  tt_int_op(digest256map_size(r->digests_seen),OP_EQ, 0);
 
  done:
   if (r) replaycache_free(r);