diff options
| -rw-r--r-- | ssl/dtls_conn_lookup.c | 78 | ||||
| -rw-r--r-- | ssl/ssl_lib.c | 3 | ||||
| -rw-r--r-- | test/dtlsssllistenertest.c | 153 |
3 files changed, 191 insertions, 43 deletions
diff --git a/ssl/dtls_conn_lookup.c b/ssl/dtls_conn_lookup.c index 27f4c66076f..fea8c7d277b 100644 --- a/ssl/dtls_conn_lookup.c +++ b/ssl/dtls_conn_lookup.c @@ -40,63 +40,54 @@ typedef struct addr_lookup_data_st { uint64_t hash_key[2]; /* 128-bit SipHash key */ } ADDR_LOOKUP_DATA; +/* BIO_ADDR contains the raw address for every supported address family. */ +#define ADDR_HASHKEY_MAX_LEN (sizeof(int) + sizeof(uint16_t) + sizeof(BIO_ADDR)) + /* - * Build a hash key from a BIO_ADDR. + * Build a hash key from a BIO_ADDR in caller-provided storage. * The key format is: family (int) + port (uint16_t) + raw address bytes. * - * Returns allocated hashkey buffer on success, NULL on failure. + * Returns 1 on success, 0 on failure. */ -static uint8_t *build_hashkey(const BIO_ADDR *peer, size_t *out_len) +static int build_hashkey_into(const BIO_ADDR *peer, uint8_t *hashkey, + size_t capacity, size_t *out_len) { - size_t hashkey_len = 0; size_t addr_len = 0; - int family; + size_t header_len = sizeof(int) + sizeof(uint16_t); + int family = BIO_ADDR_family(peer); uint16_t port; - uint8_t *hashkey; - int *famptr; - uint16_t *portptr; - uint8_t *addrptr; - - family = BIO_ADDR_family(peer); /* For AF_UNSPEC (no peer address), use a minimal key */ if (family == AF_UNSPEC) { - hashkey_len = sizeof(int); - hashkey = OPENSSL_zalloc(hashkey_len); - if (hashkey == NULL) - return NULL; - famptr = (int *)hashkey; - *famptr = family; - *out_len = hashkey_len; - return hashkey; + if (capacity < sizeof(family)) + return 0; + memcpy(hashkey, &family, sizeof(family)); + *out_len = sizeof(family); + return 1; } - if (!BIO_ADDR_rawaddress(peer, NULL, &addr_len)) - return NULL; + if (!BIO_ADDR_rawaddress(peer, NULL, &addr_len) + || capacity < header_len || addr_len > capacity - header_len) + return 0; port = BIO_ADDR_rawport(peer); + memcpy(hashkey, &family, sizeof(family)); + memcpy(hashkey + sizeof(family), &port, sizeof(port)); + if (!BIO_ADDR_rawaddress(peer, hashkey + header_len, NULL)) + return 0; - hashkey_len += sizeof(int); /* family */ - hashkey_len += sizeof(uint16_t); /* port */ - hashkey_len += addr_len; /* address */ - - hashkey = OPENSSL_zalloc(hashkey_len); - if (hashkey == NULL) - return NULL; + *out_len = header_len + addr_len; + return 1; +} - famptr = (int *)hashkey; - portptr = (uint16_t *)(famptr + 1); - addrptr = (uint8_t *)(portptr + 1); +/* Return an allocated hash key for callers which need owned storage. */ +static uint8_t *build_hashkey(const BIO_ADDR *peer, size_t *out_len) +{ + uint8_t hashkey[ADDR_HASHKEY_MAX_LEN]; - *famptr = family; - *portptr = port; - if (!BIO_ADDR_rawaddress(peer, addrptr, NULL)) { - OPENSSL_free(hashkey); + if (!build_hashkey_into(peer, hashkey, sizeof(hashkey), out_len)) return NULL; - } - - *out_len = hashkey_len; - return hashkey; + return OPENSSL_memdup(hashkey, *out_len); } /* @@ -313,6 +304,7 @@ static int addr_unregister_conn(DGRAM_CONN_LOOKUP *lookup, const BIO_ADDR *peer) ADDR_LOOKUP_DATA *data; DGRAM_CONN_ENTRY lookup_key; DGRAM_CONN_ENTRY *removed; + uint8_t hashkey[ADDR_HASHKEY_MAX_LEN]; if (lookup == NULL || lookup->impl_data == NULL || peer == NULL) return 0; @@ -322,17 +314,17 @@ static int addr_unregister_conn(DGRAM_CONN_LOOKUP *lookup, const BIO_ADDR *peer) memset(&lookup_key, 0, sizeof(lookup_key)); BIO_ADDR_copy(&lookup_key.peer, peer); - lookup_key.hashkey = build_hashkey(peer, &lookup_key.hashkey_len); - if (lookup_key.hashkey == NULL) + /* Cleanup must not depend on allocating a temporary lookup key. */ + if (!build_hashkey_into(peer, hashkey, sizeof(hashkey), + &lookup_key.hashkey_len)) return 0; + lookup_key.hashkey = hashkey; /* Set SipHash key pointer for hash function */ lookup_key.siphash_key = data->hash_key; removed = lh_DGRAM_CONN_ENTRY_delete(data->htable, &lookup_key); - OPENSSL_free(lookup_key.hashkey); - if (removed != NULL) { conn_entry_free(removed); return 1; diff --git a/ssl/ssl_lib.c b/ssl/ssl_lib.c index b40783a21e1..03914f1c1aa 100644 --- a/ssl/ssl_lib.c +++ b/ssl/ssl_lib.c @@ -684,6 +684,9 @@ int ossl_ssl_connection_reset(SSL *s) if (is_dtls_listener_conn) { DTLS_LISTENER *dl = (DTLS_LISTENER *)saved_listener; + /* The detached connection cannot unregister itself on free. */ + ossl_dtls_listener_unregister_established_conn(saved_listener, + &saved_peer_addr); ossl_crypto_mutex_lock(dl->mutex); sc->listener_rx = NULL; ossl_crypto_mutex_unlock(dl->mutex); diff --git a/test/dtlsssllistenertest.c b/test/dtlsssllistenertest.c index 79bf863d006..44d8934f5f4 100644 --- a/test/dtlsssllistenertest.c +++ b/test/dtlsssllistenertest.c @@ -5583,6 +5583,156 @@ static int drive_until_connection_queued(SSL *listener, SSL *clientssl) return 0; } +#ifndef OPENSSL_NO_DTLS1_2 +static CRYPTO_malloc_fn ssl_clear_malloc; +static CRYPTO_realloc_fn ssl_clear_realloc; +static int ssl_clear_oom_active; + +/* + * Preserve MFAIL's first-failure selection, then keep normal allocations + * failing until SSL_clear() returns. Error/debug allocations use a NULL file + * and remain available for reporting the original failure. + */ +static void *ssl_clear_oom_malloc(size_t num, const char *file, int line) +{ + if (ssl_clear_oom_active && num > 0 && file != NULL + && mfail_was_triggered()) + return NULL; + return ssl_clear_malloc(num, file, line); +} + +static void *ssl_clear_oom_realloc(void *ptr, size_t num, + const char *file, int line) +{ + if (ssl_clear_oom_active && num > 0 && file != NULL + && mfail_was_triggered()) + return NULL; + return ssl_clear_realloc(ptr, num, file, line); +} + +/* + * A failed replacement of the DTLS state must not leave a listener entry. + * idx == 0 injects a single failure; idx == 1 keeps allocations failing + * through reset cleanup, including any allocation needed to unregister. + */ +static int test_dtls_listener_ssl_clear_mfail(int idx) +{ + SSL_CTX *sctx = NULL, *cctx = NULL; + SSL *listener = NULL, *clientssl = NULL, *serverssl = NULL; + BIO_ADDR *client_addr = NULL; + SSL_CONNECTION *sc; + DTLS_LISTENER *dl = NULL; + DGRAM_URXE probe = { 0 }; + CRYPTO_free_fn saved_free = NULL; + int have_peer = 0, wrapped = 0, ret; + /* Invariant failures must fail even when an allocation was injected. */ + int testresult = -1; + + if (!TEST_true(create_ssl_ctx_pair(NULL, DTLS_server_method(), + DTLS_client_method(), DTLS1_2_VERSION, DTLS1_2_VERSION, + &sctx, &cctx, cert, privkey)) + || !TEST_true(create_dtls_listener_and_client_mem(sctx, cctx, + SSL_LISTENER_FLAG_NO_VALIDATE | SSL_LISTENER_FLAG_SINGLE_THREAD, + &listener, &clientssl, &client_addr)) + || !drive_until_connection_queued(listener, clientssl) + || !TEST_ptr(serverssl = SSL_accept_connection(listener, + SSL_ACCEPT_CONNECTION_NO_BLOCK)) + || !TEST_ptr(sc = SSL_CONNECTION_FROM_SSL_ONLY(serverssl)) + || !TEST_ptr(sc->d1) + || !TEST_ptr_eq(sc->d1->listener, listener) + || !TEST_ptr(sc->d1->rx) + || !TEST_ptr_eq(sc->listener_rx, sc->d1->rx) + || !TEST_ptr_ne(serverssl->method, serverssl->defltmeth)) + goto end; + + dl = (DTLS_LISTENER *)listener; + probe.peer = sc->d1->peer_addr; + have_peer = 1; + if (!TEST_int_eq(BIO_ADDR_family(&probe.peer), AF_INET) + || !TEST_ptr_eq(ossl_dtls_listener_find_established_conn(dl, &probe), + serverssl) + || !TEST_size_t_eq( + ossl_dgram_conn_lookup_num_items(dl->established_conns), 1)) + goto end; + + /* Wrap only the installed MFAIL allocator, never the default allocator. */ + if (idx == 1 && mfail_is_installed()) { + CRYPTO_get_mem_functions(&ssl_clear_malloc, &ssl_clear_realloc, + &saved_free); + if (!TEST_true(CRYPTO_set_mem_functions(ssl_clear_oom_malloc, + ssl_clear_oom_realloc, saved_free))) + goto end; + wrapped = 1; + } + + /* Only the reset is subject to allocation-failure injection. */ + ssl_clear_oom_active = wrapped; + MFAIL_start(); + ret = SSL_clear(serverssl); + MFAIL_end(); + ssl_clear_oom_active = 0; + if (wrapped) { + if (!TEST_true(CRYPTO_set_mem_functions(ssl_clear_malloc, + ssl_clear_realloc, saved_free))) + goto end; + wrapped = 0; + } + ERR_clear_error(); + + if (!TEST_ptr_eq(serverssl->method, serverssl->defltmeth) + || (ret == 0 && !TEST_true(mfail_was_triggered()))) + goto end; + + if (ret == 0 && (sc->d1 == NULL || sc->d1->listener == NULL)) { + /* + * ssl_init failed before restoring the listener association. Check + * the saved peer while the SSL is still alive: freeing it first would + * turn a stale registration into a dangling pointer. + */ + if (!TEST_ptr_null(sc->listener_rx) + || !TEST_size_t_eq( + ossl_dgram_conn_lookup_num_items(dl->established_conns), 0) + || !TEST_ptr_null( + ossl_dtls_listener_find_established_conn(dl, &probe))) + goto end; + } else { + /* + * A successful reset, or a failure after ssl_init restored the + * association, must preserve the connection's registration. + */ + if (!TEST_ptr(sc->d1) + || !TEST_ptr_eq(sc->d1->listener, listener) + || !TEST_ptr(sc->d1->rx) + || !TEST_ptr_eq(sc->listener_rx, sc->d1->rx) + || !TEST_size_t_eq( + ossl_dgram_conn_lookup_num_items(dl->established_conns), 1) + || !TEST_ptr_eq( + ossl_dtls_listener_find_established_conn(dl, &probe), serverssl)) + goto end; + } + + testresult = 1; +end: + ssl_clear_oom_active = 0; + MFAIL_end(); + if (wrapped + && !TEST_true(CRYPTO_set_mem_functions(ssl_clear_malloc, + ssl_clear_realloc, saved_free))) + testresult = -1; + /* Remove any stale entry before freeing the object on a failing run. */ + if (have_peer) + ossl_dtls_listener_unregister_established_conn(listener, &probe.peer); + SSL_free(serverssl); + SSL_free(clientssl); + SSL_free(listener); + BIO_ADDR_free(client_addr); + SSL_CTX_free(sctx); + SSL_CTX_free(cctx); + ERR_clear_error(); + return testresult; +} +#endif + /* * Test the blocking mode of a DTLS listener and of the connections it creates. * @@ -6879,6 +7029,9 @@ int setup_tests(void) ADD_TEST(test_ssl_ownership_set_rbio_incoming_leak); ADD_TEST(test_ssl_ownership_multiple_pending_leak); ADD_TEST(test_ssl_ownership_pending_timeout_cleanup); +#ifndef OPENSSL_NO_DTLS1_2 + ADD_MFAIL_ALL_NO_CHECK_TESTS(test_dtls_listener_ssl_clear_mfail, 2); +#endif /* Max number of pending connections tests */ ADD_TEST(test_dtls_listener_max_pending_conns_api); |