BUG/MEDIUM: cache: always verify the primary hash in get_secondary_entry()

When checking for secondary entries, the tree is walked within duplicates
of the primary key, only indexed on the first 32 bits, which means that
in case of hash collision, we could start looking for an object and
switch to another one while visiting secondaries. In order to avoid this
we simply need to always check the full primary hash of the entry that
was found.

This should be backported to all stable versions.

diff --git a/src/cache.c b/src/cache.c
index 75ccc3c4df0186125d93da72e339408df85bd0c1..8349bdd173d675968fad4bd2e1be81caf2a98871 100644 (file)
--- a/src/cache.c
+++ b/src/cache.c
@@ -374,7 +374,7 @@ static int secondary_key_cmp(const char *ref_key, const char *new_key)
  * delete_expired==0, write otherwise.
  */
 struct cache_entry *get_secondary_entry(struct cache_tree *cache, struct cache_entry *entry,
-                                        const char *secondary_key, int delete_expired)
+                                        const char *primary_hash, const char *secondary_key, int delete_expired)
 {
        struct eb32_node *node = &entry->eb;
 
@@ -395,6 +395,12 @@ struct cache_entry *get_secondary_entry(struct cache_tree *cache, struct cache_e
                entry = node ? eb32_entry(node, struct cache_entry, eb) : NULL;
        }
 
+       /* Now verify the full primary hash matches: eb32 only compares 32 bits so
+        * we could have ended up on a different, unrelated entry.
+        */
+       if (entry && primary_hash && memcmp(entry->hash, primary_hash, sizeof(entry->hash)))
+               entry = NULL;
+
        /* Expired entry */
        if (entry && entry->expire <= date.tv_sec) {
                if (delete_expired) {
@@ -1303,7 +1309,7 @@ enum act_return http_action_store_cache(struct act_rule *rule, struct proxy *px,
        if (old) {
                if (vary_signature)
                        old = get_secondary_entry(cache_tree, old,
-                                                 txn->cache_secondary_hash, 1);
+                                                 txn->cache_hash, txn->cache_secondary_hash, 1);
                if (old) {
                        if (!old->complete) {
                                /* An entry with the same primary key is already being
@@ -2178,6 +2184,7 @@ enum act_return http_action_req_cache_use(struct act_rule *rule, struct proxy *p
                        if (!http_request_build_secondary_key(s, res->secondary_key_signature)) {
                                cache_rdlock(cache_tree);
                                sec_entry = get_secondary_entry(cache_tree, res,
+                                                               s->txn.http->cache_hash,
                                                                s->txn.http->cache_secondary_hash,
                                                                0);
                                if (!sec_entry) {