From: Neil Horman Date: Wed, 15 May 2024 13:20:30 +0000 (-0400) Subject: alternate collision checking support X-Git-Tag: openssl-3.4.0-alpha1~89 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=435531ec24ecdf00a5904f277bf3d2d9c6d63dd9;p=thirdparty%2Fopenssl.git alternate collision checking support Add full key matching to hashtable the idea is that on a hash value match we do a full memory comparison of the unhashed key to validate that its actually the key we're looking for Reviewed-by: Paul Dale Reviewed-by: Tomas Mraz (Merged from https://github.com/openssl/openssl/pull/24504) --- diff --git a/crypto/hashtable/hashtable.c b/crypto/hashtable/hashtable.c index c7ceafd6dcf..77c546dbb00 100644 --- a/crypto/hashtable/hashtable.c +++ b/crypto/hashtable/hashtable.c @@ -514,6 +514,18 @@ static void free_old_ht_value(void *arg) OPENSSL_free(h); } +static ossl_inline int match_key(HT_KEY *a, HT_KEY *b) +{ + /* + * keys match if they are both present, the same size + * and compare equal in memory + */ + if (a != NULL && b != NULL && a->keysize == b->keysize) + return !memcmp(a->keybuf, b->keybuf, a->keysize); + + return 1; +} + static int ossl_ht_insert_locked(HT *h, uint64_t hash, struct ht_internal_value_st *newval, HT_VALUE **olddata) @@ -537,9 +549,12 @@ static int ossl_ht_insert_locked(HT *h, uint64_t hash, if (ival == NULL) empty_idx = j; if (compare_hash(hash, ihash)) { - if (olddata == NULL) { - /* invalid */ - return 0; + /* Its the same hash, lets make sure its not a collision */ + if (match_key(&newval->value.key, &ival->key)) { + if (olddata == NULL) { + /* invalid */ + return 0; + } } /* Do a replacement */ if (!CRYPTO_atomic_store(&md->neighborhoods[neigh_idx].entries[j].hash, @@ -570,18 +585,27 @@ static struct ht_internal_value_st *alloc_new_value(HT *h, HT_KEY *key, void *data, uintptr_t *type) { - struct ht_internal_value_st *new; struct ht_internal_value_st *tmp; + size_t nvsize = sizeof(*tmp); - new = OPENSSL_malloc(sizeof(*new)); + if (h->config.collision_check == 1) + nvsize += key->keysize; - if (new == NULL) + tmp = OPENSSL_malloc(nvsize); + + if (tmp == NULL) return NULL; - tmp = (struct ht_internal_value_st *)ossl_rcu_deref(&new); tmp->ht = h; tmp->value.value = data; tmp->value.type_id = type; + tmp->value.key.keybuf = NULL; + if (h->config.collision_check) { + tmp->value.key.keybuf = (uint8_t *)(tmp+1); + tmp->value.key.keysize = key->keysize; + memcpy(tmp->value.key.keybuf, key->keybuf, key->keysize); + } + return tmp; } @@ -643,9 +667,9 @@ HT_VALUE *ossl_ht_get(HT *h, HT_KEY *key) for (j = 0; j < NEIGHBORHOOD_LEN; j++) { if (!CRYPTO_atomic_load(&md->neighborhoods[neigh_idx].entries[j].hash, &ehash, h->atomic_lock)) - break; - if (compare_hash(hash, ehash)) { - vidx = ossl_rcu_deref(&md->neighborhoods[neigh_idx].entries[j].value); + return NULL; + vidx = ossl_rcu_deref(&md->neighborhoods[neigh_idx].entries[j].value); + if (compare_hash(hash, ehash) && match_key(&vidx->value.key, key)) { ret = (HT_VALUE *)vidx; break; } @@ -674,16 +698,18 @@ int ossl_ht_delete(HT *h, HT_KEY *key) hash = h->config.ht_hash_fn(key->keybuf, key->keysize); - neigh_idx = hash & md->neighborhood_mask; - PREFETCH_NEIGHBORHOOD(md->neighborhoods[neigh_idx]); + neigh_idx = hash & h->md->neighborhood_mask; + PREFETCH_NEIGHBORHOOD(h->md->neighborhoods[neigh_idx]); for (j = 0; j < NEIGHBORHOOD_LEN; j++) { - if (compare_hash(hash, md->neighborhoods[neigh_idx].entries[j].hash)) { - h->wpd.value_count--; - if (!CRYPTO_atomic_store(&md->neighborhoods[neigh_idx].entries[j].hash, + v = (struct ht_internal_value_st *)h->md->neighborhoods[neigh_idx].entries[j].value; + if (compare_hash(hash, h->md->neighborhoods[neigh_idx].entries[j].hash)) { + if (!match_key(key, &v->value.key)) + continue; + if (!CRYPTO_atomic_store(&h->md->neighborhoods[neigh_idx].entries[j].hash, 0, h->atomic_lock)) break; - v = (struct ht_internal_value_st *)md->neighborhoods[neigh_idx].entries[j].value; - ossl_rcu_assign_ptr(&md->neighborhoods[neigh_idx].entries[j].value, + h->wpd.value_count--; + ossl_rcu_assign_ptr(&h->md->neighborhoods[neigh_idx].entries[j].value, &nv); rc = 1; break; diff --git a/fuzz/hashtable.c b/fuzz/hashtable.c index b58d48b2250..9d911d86c29 100644 --- a/fuzz/hashtable.c +++ b/fuzz/hashtable.c @@ -99,7 +99,7 @@ static void fuzz_free_cb(HT_VALUE *v) int FuzzerInitialize(int *argc, char ***argv) { - HT_CONFIG fuzz_conf = {NULL, fuzz_free_cb, NULL, 0}; + HT_CONFIG fuzz_conf = {NULL, fuzz_free_cb, NULL, 1, 0}; OPENSSL_init_crypto(OPENSSL_INIT_LOAD_CRYPTO_STRINGS, NULL); ERR_clear_error(); diff --git a/include/internal/hashtable.h b/include/internal/hashtable.h index 873612d8279..e486397ae9f 100644 --- a/include/internal/hashtable.h +++ b/include/internal/hashtable.h @@ -19,12 +19,21 @@ typedef struct ht_internal_st HT; +/* + * Represents a key to a hashtable + */ +typedef struct ht_key_header_st { + size_t keysize; + uint8_t *keybuf; +} HT_KEY; + /* * Represents a value in the hash table */ typedef struct ht_value_st { void *value; uintptr_t *type_id; + HT_KEY key; } HT_VALUE; /* @@ -42,17 +51,10 @@ typedef struct ht_config_st { OSSL_LIB_CTX *ctx; void (*ht_free_fn)(HT_VALUE *obj); uint64_t (*ht_hash_fn)(uint8_t *key, size_t keylen); + uint32_t collision_check; uint32_t init_neighborhoods; } HT_CONFIG; -/* - * Key value for a hash lookup - */ -typedef struct ht_key_header_st { - size_t keysize; - uint8_t *keybuf; -} HT_KEY; - /* * Hashtable key rules * Any struct can be used to formulate a hash table key, as long as the diff --git a/test/lhash_test.c b/test/lhash_test.c index 76bed5d7e52..d3e929e2e06 100644 --- a/test/lhash_test.c +++ b/test/lhash_test.c @@ -230,6 +230,7 @@ static int test_int_hashtable(void) NULL, NULL, NULL, + 1, 0, }; INTKEY key; @@ -407,6 +408,7 @@ static int test_hashtable_stress(void) NULL, /* use default context */ hashtable_intfree, /* our free function */ hashtable_hash, /* our hash function */ + 1, /* Check collisions */ 625000, /* preset hash size */ }; HT *h; @@ -625,6 +627,7 @@ static int test_hashtable_multithread(void) NULL, /* use default context */ hashtable_mt_free, /* our free function */ NULL, /* default hash function */ + 1, /* Check collisions */ 0, /* default hash size */ }; int ret = 0;