From 7c8d76c4589c298ecae9a071bc6a149f488a613e Mon Sep 17 00:00:00 2001 From: Artem Boldariev Date: Thu, 21 Apr 2022 15:29:45 +0300 Subject: [PATCH 01/12] Add TLS client session cache implementation This commit adds an implementation of a client TLS session cache. TLS client session cache is an object which allows efficient storing and retrieval of previously saved TLS sessions so that they can be resumed. This object is supposed to be a foundation for implementing TLS session resumption - a standard technique to reduce the cost of re-establishing a connection to the remote server endpoint. OpenSSL does server-side TLS session caching transparently by default. However, on the client-side, a TLS session to resume must be manually specified when establishing the TLS connection. The TLS client session cache is precisely the foundation for that. (cherry picked from commit 4ef40988f3e059236f77472368e92222bf397094) --- lib/isc/include/isc/tls.h | 147 +++++++++++++++++++ lib/isc/tls.c | 294 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 441 insertions(+) diff --git a/lib/isc/include/isc/tls.h b/lib/isc/include/isc/tls.h index e05e2c4228..ab44a23484 100644 --- a/lib/isc/include/isc/tls.h +++ b/lib/isc/include/isc/tls.h @@ -267,6 +267,153 @@ isc_tls_cert_store_free(isc_tls_cert_store_t **pstore); *\li 'pstore' is a valid pointer to a pointer containing a non-'NULL' value. */ +typedef struct isc_tlsctx_client_session_cache isc_tlsctx_client_session_cache_t; +/*%< + * TLS client session cache is an object which allows efficient + * storing and retrieval of previously saved TLS sessions so that they + * can be resumed. This object is supposed to be a foundation for + * implementing TLS session resumption - a standard technique to + * reduce the cost of re-establishing a connection to the remote + * server endpoint. + * + * OpenSSL does server-side TLS session caching transparently by + * default. However, on the client-side, a TLS session to resume must + * be manually specified when establishing the TLS connection. The TLS + * client session cache is precisely the foundation for that. + * + * The cache has been designed to have the following characteristics: + * + * - Fixed maximal number of entries to not keep too many obsolete + * sessions within the cache; + * + * - The cache is indexed by character string keys. Each string is a + * key representing a remote endpoint (unique remote endpoint name, + * e.g. combination of the remote IP address and port); + * + * - Least Recently Used (LRU) cache replacement policy while allowing + * easy removal of obsolete entries; + * + * - Ability to store multiple TLS sessions associated with the given + * key (remote endpoint name). This characteristic is important if + * multiple connections to the same remote server can be established; + * + * - Ability to efficiently retrieve the most recent TLS sessions + * associated with the key (remote endpoint name). + * + * Because of these characteristics, the cache will end up having the + * necessary amount of resumable TLS session parameters to the most + * used remote endpoints ("hot entries") after a short period of + * initial use ("warmup"). + * + * Attempting to resume TLS sessions is an optimisation, which is not + * guaranteed to succeed because it requires the same session to be + * present in the server session caches. If it is not the case, the + * usual handshake procedure takes place. However, when session + * resumption is successful, it reduces the amount of the + * computational resources required as well as the amount of data to + * be transmitted when (re)establishing the connection. Also, it + * reduces round trip time (by reducing the number of packets to + * transmit). + * + * This optimisation is important in the context of DNS because the + * amount of data within the full handshake messages might be + * comparable to or surpass the size of a typical DNS message. + */ + +isc_tlsctx_client_session_cache_t * +isc_tlsctx_client_session_cache_new(isc_mem_t *mctx, isc_tlsctx_t *ctx, + const size_t max_entries); +/*%< + * Create a new TLS client session cache object. + * + * Requires: + *\li 'mctx' is a valid memory context object; + *\li 'ctx' is a valid TLS context object; + *\li 'max_entries' is a positive number; + */ + +void +isc_tlsctx_client_session_cache_attach( + isc_tlsctx_client_session_cache_t *source, + isc_tlsctx_client_session_cache_t **targetp); +/*%< + * Create a reference to the TLS client session cache object. + * + * Requires: + *\li 'source' is a valid TLS client session cache object; + *\li 'targetp' is a valid pointer to a pointer which must equal NULL. + */ + +void +isc_tlsctx_client_session_cache_detach( + isc_tlsctx_client_session_cache_t **cachep); +/*%< + * Remove a reference to the TLS client session cache object. + * + * Requires: + *\li 'cachep' is a pointer to a pointer to a valid TLS client session cache + *object. + */ + +void +isc_tlsctx_client_session_cache_keep(isc_tlsctx_client_session_cache_t *cache, + char *remote_peer_name, isc_tls_t *tls); +/*%< + * Add a resumable TLS client session within 'tls' to the TLS client + * session cache object 'cache' and associate it with + * 'remote_server_name' string. Also, the oldest entry from the cache + * might get removed if the cache is full. + * + * Requires: + *\li 'cache' is a pointer to a valid TLS client session cache object; + *\li 'remote_peer_name' is a pointer to a non empty character string. + *\li 'tls' is a valid, non-'NULL' pointer to a TLS connection state object. + */ + +void +isc_tlsctx_client_session_cache_keep_sockaddr( + isc_tlsctx_client_session_cache_t *cache, isc_sockaddr_t *remote_peer, + isc_tls_t *tls); +/*%< + * The same as 'isc_tlsctx_client_session_cache_keep()', but using a + * 'isc_sockaddr_t' as a key, instead of a character string. + * + * Requires: + *\li 'remote_peer' is a valid, non-'NULL' pointer to an 'isc_sockaddr_t' + *object. + */ + +void +isc_tlsctx_client_session_cache_reuse(isc_tlsctx_client_session_cache_t *cache, + char *remote_server_name, isc_tls_t *tls); +/*% + * Try to restore a previously stored TLS session denoted by a remote + * server name as a key ('remote_server_name') into the given TLS + * connection state object ('tls'). The successfully restored session + * gets removed from the cache. + * + * Requires: + *\li 'cache' is a pointer to a valid TLS client session cache object; + *\li 'remote_peer_name' is a pointer to a non empty character string; + *\li 'tls' is a valid, non-'NULL', pointer to a TLS connection state object. + */ + +void +isc_tlsctx_client_session_cache_reuse_sockaddr( + isc_tlsctx_client_session_cache_t *cache, isc_sockaddr_t *remote_peer, + isc_tls_t *tls); +/*%< + * The same as 'isc_tlsctx_client_session_cache_reuse()', but uses a + * 'isc_sockaddr_t' as a key, instead of a character string. + * + * Requires: + *\li 'remote_peer' is a valid, non-'NULL' pointer to an 'isc_sockaddr_t' + *object. + */ + +const isc_tlsctx_t * +isc_tlsctx_client_session_cache_getctx(isc_tlsctx_client_session_cache_t *cache); + typedef struct isc_tlsctx_cache isc_tlsctx_cache_t; /*%< * The TLS context cache is an object which allows retrieving a diff --git a/lib/isc/tls.c b/lib/isc/tls.c index 640fe2442c..a7b9d1b40a 100644 --- a/lib/isc/tls.c +++ b/lib/isc/tls.c @@ -43,6 +43,7 @@ #include #include #include +#include #include #include #include @@ -1074,6 +1075,10 @@ isc_tls_cert_store_free(isc_tls_cert_store_t **pstore) { #define TLSCTX_CACHE_MAGIC ISC_MAGIC('T', 'l', 'S', 'c') #define VALID_TLSCTX_CACHE(t) ISC_MAGIC_VALID(t, TLSCTX_CACHE_MAGIC) +#define TLSCTX_CLIENT_SESSION_CACHE_MAGIC ISC_MAGIC('T', 'l', 'C', 'c') +#define VALID_TLSCTX_CLIENT_SESSION_CACHE(t) \ + ISC_MAGIC_VALID(t, TLSCTX_CLIENT_SESSION_CACHE_MAGIC) + typedef struct isc_tlsctx_cache_entry { /* * We need a TLS context entry for each transport on both IPv4 and @@ -1299,3 +1304,292 @@ isc_tlsctx_cache_find(isc_tlsctx_cache_t *cache, const char *name, return (result); } + +typedef struct client_session_cache_entry client_session_cache_entry_t; + +typedef struct client_session_cache_bucket { + char *bucket_key; + size_t bucket_key_len; + /* Cache entries within the bucket (from the oldest to the newest). */ + ISC_LIST(client_session_cache_entry_t) entries; +} client_session_cache_bucket_t; + +struct client_session_cache_entry { + SSL_SESSION *session; + client_session_cache_bucket_t *bucket; /* "Parent" bucket pointer. */ + ISC_LINK(client_session_cache_entry_t) bucket_link; + ISC_LINK(client_session_cache_entry_t) cache_link; +}; + +struct isc_tlsctx_client_session_cache { + uint32_t magic; + isc_refcount_t references; + isc_mem_t *mctx; + + /* + * We need to keep a reference to the related TLS context in order + * to ensure that it remains valid while the TLS client sessions + * cache object is valid, as every TLS session object + * (SSL_SESSION) is "tied" to a particular context. + */ + isc_tlsctx_t *ctx; + + /* + * The idea is to have one bucket per remote server. Each bucket, + * can maintain multiple TLS sessions to that server, as BIND + * might want to establish multiple TLS connections to the remote + * server at once. + */ + isc_ht_t *buckets; + + /* + * The list of all current entries within the cache maintained in + * LRU-manner, so that the oldest entry might be efficiently + * removed. + */ + ISC_LIST(client_session_cache_entry_t) lru_entries; + /* Number of the entries within the cache. */ + size_t nentries; + /* Maximum number of the entries within the cache. */ + size_t max_entries; + + isc_mutex_t lock; +}; + +isc_tlsctx_client_session_cache_t * +isc_tlsctx_client_session_cache_new(isc_mem_t *mctx, isc_tlsctx_t *ctx, + const size_t max_entries) { + isc_tlsctx_client_session_cache_t *nc; + + REQUIRE(ctx != NULL); + REQUIRE(max_entries > 0); + + nc = isc_mem_get(mctx, sizeof(*nc)); + + *nc = (isc_tlsctx_client_session_cache_t){ .max_entries = max_entries }; + isc_refcount_init(&nc->references, 1); + isc_mem_attach(mctx, &nc->mctx); + isc_tlsctx_attach(ctx, &nc->ctx); + + isc_ht_init(&nc->buckets, mctx, 5); + ISC_LIST_INIT(nc->lru_entries); + isc_mutex_init(&nc->lock); + + nc->magic = TLSCTX_CLIENT_SESSION_CACHE_MAGIC; + + return (nc); +} + +void +isc_tlsctx_client_session_cache_attach( + isc_tlsctx_client_session_cache_t *source, + isc_tlsctx_client_session_cache_t **targetp) { + REQUIRE(VALID_TLSCTX_CLIENT_SESSION_CACHE(source)); + REQUIRE(targetp != NULL && *targetp == NULL); + + isc_refcount_increment(&source->references); + + *targetp = source; +} + +static void +client_cache_entry_delete(isc_tlsctx_client_session_cache_t *restrict cache, + client_session_cache_entry_t *restrict entry) { + client_session_cache_bucket_t *restrict bucket = entry->bucket; + + /* Unlink and free the cache entry */ + ISC_LIST_UNLINK(bucket->entries, entry, bucket_link); + ISC_LIST_UNLINK(cache->lru_entries, entry, cache_link); + cache->nentries--; + (void)SSL_SESSION_free(entry->session); + isc_mem_put(cache->mctx, entry, sizeof(*entry)); + + /* The bucket is empty - let's remove it */ + if (ISC_LIST_EMPTY(bucket->entries)) { + RUNTIME_CHECK(isc_ht_delete(cache->buckets, + (const uint8_t *)bucket->bucket_key, + bucket->bucket_key_len) == + ISC_R_SUCCESS); + + isc_mem_free(cache->mctx, bucket->bucket_key); + isc_mem_put(cache->mctx, bucket, sizeof(*bucket)); + } +} + +void +isc_tlsctx_client_session_cache_detach( + isc_tlsctx_client_session_cache_t **cachep) { + isc_tlsctx_client_session_cache_t *cache = NULL; + client_session_cache_entry_t *entry = NULL, *next = NULL; + + REQUIRE(cachep != NULL); + + cache = *cachep; + *cachep = NULL; + + REQUIRE(VALID_TLSCTX_CLIENT_SESSION_CACHE(cache)); + + if (isc_refcount_decrement(&cache->references) != 1) { + return; + } + + cache->magic = 0; + + isc_refcount_destroy(&cache->references); + + entry = ISC_LIST_HEAD(cache->lru_entries); + while (entry != NULL) { + next = ISC_LIST_NEXT(entry, cache_link); + client_cache_entry_delete(cache, entry); + entry = next; + } + + RUNTIME_CHECK(isc_ht_count(cache->buckets) == 0); + isc_ht_destroy(&cache->buckets); + + isc_mutex_destroy(&cache->lock); + isc_tlsctx_free(&cache->ctx); + isc_mem_putanddetach(&cache->mctx, cache, sizeof(*cache)); +} + +void +isc_tlsctx_client_session_cache_keep(isc_tlsctx_client_session_cache_t *cache, + char *remote_peer_name, isc_tls_t *tls) { + size_t name_len; + isc_result_t result; + SSL_SESSION *sess; + client_session_cache_bucket_t *restrict bucket = NULL; + client_session_cache_entry_t *restrict entry = NULL; + + REQUIRE(VALID_TLSCTX_CLIENT_SESSION_CACHE(cache)); + REQUIRE(remote_peer_name != NULL && *remote_peer_name != '\0'); + REQUIRE(tls != NULL); + + sess = SSL_get1_session(tls); + if (sess == NULL) { + return; + } else if (SSL_SESSION_is_resumable(sess) == 0) { + SSL_SESSION_free(sess); + return; + } + + isc_mutex_lock(&cache->lock); + + name_len = strlen(remote_peer_name); + result = isc_ht_find(cache->buckets, (const uint8_t *)remote_peer_name, + name_len, (void **)&bucket); + + if (result != ISC_R_SUCCESS) { + /* Let's create a new bucket */ + INSIST(bucket == NULL); + bucket = isc_mem_get(cache->mctx, sizeof(*bucket)); + *bucket = (client_session_cache_bucket_t){ + .bucket_key = isc_mem_strdup(cache->mctx, + remote_peer_name), + .bucket_key_len = name_len + }; + ISC_LIST_INIT(bucket->entries); + RUNTIME_CHECK(isc_ht_add(cache->buckets, + (const uint8_t *)remote_peer_name, + name_len, + (void *)bucket) == ISC_R_SUCCESS); + } + + /* Let's add a new cache entry to the new/found bucket */ + entry = isc_mem_get(cache->mctx, sizeof(*entry)); + *entry = (client_session_cache_entry_t){ .session = sess, + .bucket = bucket }; + ISC_LINK_INIT(entry, bucket_link); + ISC_LINK_INIT(entry, cache_link); + + ISC_LIST_APPEND(bucket->entries, entry, bucket_link); + + ISC_LIST_APPEND(cache->lru_entries, entry, cache_link); + cache->nentries++; + + if (cache->nentries > cache->max_entries) { + /* + * Cache overrun. We need to remove the oldest entry from the + * cache + */ + client_session_cache_entry_t *restrict oldest; + INSIST((cache->nentries - 1) == cache->max_entries); + + oldest = ISC_LIST_HEAD(cache->lru_entries); + client_cache_entry_delete(cache, oldest); + } + + isc_mutex_unlock(&cache->lock); +} + +void +isc_tlsctx_client_session_cache_reuse(isc_tlsctx_client_session_cache_t *cache, + char *remote_peer_name, isc_tls_t *tls) { + client_session_cache_bucket_t *restrict bucket = NULL; + client_session_cache_entry_t *restrict entry; + size_t name_len; + isc_result_t result; + + REQUIRE(VALID_TLSCTX_CLIENT_SESSION_CACHE(cache)); + REQUIRE(remote_peer_name != NULL && *remote_peer_name != '\0'); + REQUIRE(tls != NULL); + + isc_mutex_lock(&cache->lock); + + /* Let's find the bucket */ + name_len = strlen(remote_peer_name); + result = isc_ht_find(cache->buckets, (const uint8_t *)remote_peer_name, + name_len, (void **)&bucket); + + if (result != ISC_R_SUCCESS) { + goto exit; + } + + INSIST(bucket != NULL); + + /* + * If the bucket has been found, let's use the newest session from + * the bucket, as it has the highest chance to be successfully + * resumed. + */ + INSIST(!ISC_LIST_EMPTY(bucket->entries)); + entry = ISC_LIST_TAIL(bucket->entries); + RUNTIME_CHECK(SSL_set_session(tls, entry->session) == 1); + client_cache_entry_delete(cache, entry); + +exit: + isc_mutex_unlock(&cache->lock); +} + +void +isc_tlsctx_client_session_cache_keep_sockaddr( + isc_tlsctx_client_session_cache_t *cache, isc_sockaddr_t *remote_peer, + isc_tls_t *tls) { + char peername[ISC_SOCKADDR_FORMATSIZE] = { 0 }; + + REQUIRE(remote_peer != NULL); + + isc_sockaddr_format(remote_peer, peername, sizeof(peername)); + + isc_tlsctx_client_session_cache_keep(cache, peername, tls); +} + +void +isc_tlsctx_client_session_cache_reuse_sockaddr( + isc_tlsctx_client_session_cache_t *cache, isc_sockaddr_t *remote_peer, + isc_tls_t *tls) { + char peername[ISC_SOCKADDR_FORMATSIZE] = { 0 }; + + REQUIRE(remote_peer != NULL); + + isc_sockaddr_format(remote_peer, peername, sizeof(peername)); + + isc_tlsctx_client_session_cache_reuse(cache, peername, tls); +} + +const isc_tlsctx_t * +isc_tlsctx_client_session_cache_getctx( + isc_tlsctx_client_session_cache_t *cache) { + REQUIRE(VALID_TLSCTX_CLIENT_SESSION_CACHE(cache)); + return (cache->ctx); +} From 6ec48f1e7883c35365c1a7735e2d08fce927161f Mon Sep 17 00:00:00 2001 From: Artem Boldariev Date: Fri, 22 Apr 2022 11:41:14 +0300 Subject: [PATCH 02/12] Extend TLS context cache with TLS client session cache This commit extends TLS context cache with TLS client session cache so that an associated session cache can be stored alongside the TLS context within the context cache. (cherry picked from commit 987892d113053f40b94f589f745e0203f097f15c) --- bin/dig/dighost.c | 37 ++++++++++++++++----- lib/dns/xfrin.c | 23 ++++++++++--- lib/isc/include/isc/tls.h | 69 ++++++++++++++++++++++++++++----------- lib/isc/tls.c | 56 ++++++++++++++++++++++++------- lib/ns/listenlist.c | 5 +-- 5 files changed, 145 insertions(+), 45 deletions(-) diff --git a/bin/dig/dighost.c b/bin/dig/dighost.c index d63adac344..bb9e13dcdb 100644 --- a/bin/dig/dighost.c +++ b/bin/dig/dighost.c @@ -2772,7 +2772,8 @@ _cancel_lookup(dig_lookup_t *lookup, const char *file, unsigned int line) { } static isc_tlsctx_t * -get_create_tls_context(dig_query_t *query, const bool is_https) { +get_create_tls_context(dig_query_t *query, const bool is_https, + isc_tlsctx_client_session_cache_t **psess_cache) { isc_result_t result; isc_tlsctx_t *ctx = NULL, *found_ctx = NULL; isc_tls_cert_store_t *store = NULL, *found_store = NULL; @@ -2783,6 +2784,8 @@ get_create_tls_context(dig_query_t *query, const bool is_https) { isc_tlsctx_cache_transport_t transport = is_https ? isc_tlsctx_cache_https : isc_tlsctx_cache_tls; const bool hostname_ignore_subject = !is_https; + isc_tlsctx_client_session_cache_t *sess_cache = NULL, + *found_sess_cache = NULL; if (query->lookup->tls_key_file_set != query->lookup->tls_cert_file_set) { @@ -2793,7 +2796,7 @@ get_create_tls_context(dig_query_t *query, const bool is_https) { result = isc_tlsctx_cache_find(query->lookup->tls_ctx_cache, tlsctxname, transport, family, &found_ctx, - &found_store); + &found_store, &found_sess_cache); if (result != ISC_R_SUCCESS) { if (query->lookup->tls_ca_set) { if (found_store == NULL) { @@ -2852,13 +2855,26 @@ get_create_tls_context(dig_query_t *query, const bool is_https) { } #endif /* HAVE_LIBNGHTTP2 */ - result = isc_tlsctx_cache_add(query->lookup->tls_ctx_cache, - tlsctxname, transport, family, - ctx, store, NULL, NULL); + sess_cache = isc_tlsctx_client_session_cache_new( + mctx, ctx, + ISC_TLSCTX_CLIENT_SESSION_CACHE_DEFAULT_SIZE); + + result = isc_tlsctx_cache_add( + query->lookup->tls_ctx_cache, tlsctxname, transport, + family, ctx, store, sess_cache, NULL, NULL, NULL); RUNTIME_CHECK(result == ISC_R_SUCCESS); + if (psess_cache != NULL) { + INSIST(*psess_cache == NULL); + *psess_cache = sess_cache; + } return (ctx); } + if (psess_cache != NULL) { + INSIST(*psess_cache == NULL); + *psess_cache = found_sess_cache; + } + INSIST(!query->lookup->tls_ca_set || found_store != NULL); return (found_ctx); failure: @@ -2868,6 +2884,9 @@ failure: if (store != NULL && store != found_store) { isc_tls_cert_store_free(&store); } + if (sess_cache != NULL && sess_cache != found_sess_cache) { + isc_tlsctx_client_session_cache_detach(&sess_cache); + } return (NULL); } @@ -2886,6 +2905,7 @@ start_tcp(dig_query_t *query) { dig_query_t *connectquery = NULL; isc_tlsctx_t *tlsctx = NULL; bool tls_mode = false; + isc_tlsctx_client_session_cache_t *sess_cache = NULL; REQUIRE(DIG_VALID_QUERY(query)); debug("start_tcp(%p)", query); @@ -2980,7 +3000,8 @@ start_tcp(dig_query_t *query) { query_attach(query, &connectquery); if (tls_mode) { - tlsctx = get_create_tls_context(connectquery, false); + tlsctx = get_create_tls_context(connectquery, false, + &sess_cache); if (tlsctx == NULL) { goto failure_tls; } @@ -2997,8 +3018,8 @@ start_tcp(dig_query_t *query) { uri, sizeof(uri)); if (!query->lookup->http_plain) { - tlsctx = get_create_tls_context(connectquery, - true); + tlsctx = get_create_tls_context( + connectquery, true, &sess_cache); if (tlsctx == NULL) { goto failure_tls; } diff --git a/lib/dns/xfrin.c b/lib/dns/xfrin.c index 6c3f8cbd52..6701174525 100644 --- a/lib/dns/xfrin.c +++ b/lib/dns/xfrin.c @@ -934,6 +934,8 @@ xfrin_start(dns_xfrin_ctx_t *xfr) { dns_transport_type_t transport_type = DNS_TRANSPORT_TCP; isc_tlsctx_t *tlsctx = NULL, *found = NULL; isc_tls_cert_store_t *store = NULL, *found_store = NULL; + isc_tlsctx_client_session_cache_t *sess_cache = NULL, + *found_sess_cache = NULL; (void)isc_refcount_increment0(&xfr->connects); dns_xfrin_attach(xfr, &connect_xfr); @@ -972,9 +974,9 @@ xfrin_start(dns_xfrin_ctx_t *xfr) { * full TLS handshake procedure, making establishing * subsequent TLS connections for XoT faster. */ - result = isc_tlsctx_cache_find(xfr->tlsctx_cache, tlsname, - isc_tlsctx_cache_tls, family, - &tlsctx, &found_store); + result = isc_tlsctx_cache_find( + xfr->tlsctx_cache, tlsname, isc_tlsctx_cache_tls, + family, &tlsctx, &found_store, &found_sess_cache); if (result != ISC_R_SUCCESS) { const char *hostname = dns_transport_get_remote_hostname( @@ -1079,11 +1081,16 @@ xfrin_start(dns_xfrin_ctx_t *xfr) { isc_tlsctx_enable_dot_client_alpn(tlsctx); + sess_cache = isc_tlsctx_client_session_cache_new( + xfr->mctx, tlsctx, + ISC_TLSCTX_CLIENT_SESSION_CACHE_DEFAULT_SIZE); + found_store = NULL; result = isc_tlsctx_cache_add( xfr->tlsctx_cache, tlsname, isc_tlsctx_cache_tls, family, tlsctx, store, - &found, &found_store); + sess_cache, &found, &found_store, + &found_sess_cache); if (result == ISC_R_EXISTS) { /* * It seems the entry has just been created @@ -1101,7 +1108,10 @@ xfrin_start(dns_xfrin_ctx_t *xfr) { INSIST(found != NULL); isc_tlsctx_free(&tlsctx); isc_tls_cert_store_free(&store); + isc_tlsctx_client_session_cache_detach( + &sess_cache); tlsctx = found; + sess_cache = found_sess_cache; } else { INSIST(result == ISC_R_SUCCESS); } @@ -1129,6 +1139,11 @@ failure: if (store != NULL && store != found_store) { isc_tls_cert_store_free(&store); } + + if (sess_cache != NULL && sess_cache != found_sess_cache) { + isc_tlsctx_client_session_cache_detach(&sess_cache); + } + isc_refcount_decrement0(&xfr->connects); dns_xfrin_detach(&connect_xfr); return (result); diff --git a/lib/isc/include/isc/tls.h b/lib/isc/include/isc/tls.h index ab44a23484..86265ddb7c 100644 --- a/lib/isc/include/isc/tls.h +++ b/lib/isc/include/isc/tls.h @@ -366,7 +366,7 @@ isc_tlsctx_client_session_cache_keep(isc_tlsctx_client_session_cache_t *cache, * * Requires: *\li 'cache' is a pointer to a valid TLS client session cache object; - *\li 'remote_peer_name' is a pointer to a non empty character string. + *\li 'remote_peer_name' is a pointer to a non empty character string; *\li 'tls' is a valid, non-'NULL' pointer to a TLS connection state object. */ @@ -375,11 +375,11 @@ isc_tlsctx_client_session_cache_keep_sockaddr( isc_tlsctx_client_session_cache_t *cache, isc_sockaddr_t *remote_peer, isc_tls_t *tls); /*%< - * The same as 'isc_tlsctx_client_session_cache_keep()', but using a + * The same as 'isc_tlsctx_client_session_cache_keep()', but uses a * 'isc_sockaddr_t' as a key, instead of a character string. * * Requires: - *\li 'remote_peer' is a valid, non-'NULL' pointer to an 'isc_sockaddr_t' + *\li 'remote_peer' is a valid, non-'NULL', pointer to an 'isc_sockaddr_t' *object. */ @@ -395,7 +395,7 @@ isc_tlsctx_client_session_cache_reuse(isc_tlsctx_client_session_cache_t *cache, * Requires: *\li 'cache' is a pointer to a valid TLS client session cache object; *\li 'remote_peer_name' is a pointer to a non empty character string; - *\li 'tls' is a valid, non-'NULL', pointer to a TLS connection state object. + *\li 'tls' is a valid, non-'NULL' pointer to a TLS connection state object. */ void @@ -413,6 +413,22 @@ isc_tlsctx_client_session_cache_reuse_sockaddr( const isc_tlsctx_t * isc_tlsctx_client_session_cache_getctx(isc_tlsctx_client_session_cache_t *cache); +/*%< + * Returns a TLS context associated with the given TLS client + * session cache object. The function is intended to be used to + * implement the sanity checks ('INSIST()'s and 'REQUIRE()'s). + * + * Requires: + *\li 'cache' is a pointer to a valid TLS client session cache object. + */ + +#define ISC_TLSCTX_CLIENT_SESSION_CACHE_DEFAULT_SIZE (150) +/*%< + * The default maximum size of a TLS client session cache. The value + * should be large enough to hold enough sessions to successfully + * re-establish connections to the most remote TLS servers, but not + * too big to avoid keeping too much obsolete sessions. + */ typedef struct isc_tlsctx_cache isc_tlsctx_cache_t; /*%< @@ -481,28 +497,39 @@ isc_tlsctx_cache_detach(isc_tlsctx_cache_t **cachep); */ isc_result_t -isc_tlsctx_cache_add(isc_tlsctx_cache_t *cache, const char *name, - const isc_tlsctx_cache_transport_t transport, - const uint16_t family, isc_tlsctx_t *ctx, - isc_tls_cert_store_t *store, isc_tlsctx_t **pfound, - isc_tls_cert_store_t **pfound_store); +isc_tlsctx_cache_add( + isc_tlsctx_cache_t *cache, const char *name, + const isc_tlsctx_cache_transport_t transport, const uint16_t family, + isc_tlsctx_t *ctx, isc_tls_cert_store_t *store, + isc_tlsctx_client_session_cache_t *client_sess_cache, + isc_tlsctx_t **pfound, isc_tls_cert_store_t **pfound_store, + isc_tlsctx_client_session_cache_t **pfound_client_sess_cache); /*%< * - * Add a new TLS context to the TLS context cache. 'pfound' is an - * optional pointer, which can be used to retrieve an already - * existing TLS context object in a case it exists. + * Add a new TLS context and its associated data to the TLS context + * cache. 'pfound' is an optional pointer, which can be used to + * retrieve an already existing TLS context object in a case it + * exists. * * The passed certificates store object ('store') possession is * transferred to the cache object in a case of success. In some cases * it might be destroyed immediately upon the call completion. * + * The possession of the passed TLS client session cache + * ('client_sess_cache') is also transferred to the cache object in a + * case of success. + * * Requires: *\li 'cache' is a valid pointer to a TLS context cache object; *\li 'name' is a valid pointer to a non-empty string; *\li 'transport' is a valid transport identifier (currently only * TLS/DoT and HTTPS/DoH are supported); *\li 'family' - either 'AF_INET' or 'AF_INET6'; - *\li 'ctx' - a valid pointer to a valid TLS context object. + *\li 'ctx' - a valid pointer to a valid TLS context object; + *\li 'store' - a valid pointer to a valid TLS certificates store object or + * 'NULL'; + *\li 'client_sess_cache' - a valid pointer to a valid TLS client sessions + *cache object or 'NULL. * * Returns: *\li #ISC_R_EXISTS - node of the same key already exists; @@ -510,12 +537,13 @@ isc_tlsctx_cache_add(isc_tlsctx_cache_t *cache, const char *name, */ isc_result_t -isc_tlsctx_cache_find(isc_tlsctx_cache_t *cache, const char *name, - const isc_tlsctx_cache_transport_t transport, - const uint16_t family, isc_tlsctx_t **pctx, - isc_tls_cert_store_t **pstore); +isc_tlsctx_cache_find( + isc_tlsctx_cache_t *cache, const char *name, + const isc_tlsctx_cache_transport_t transport, const uint16_t family, + isc_tlsctx_t **pctx, isc_tls_cert_store_t **pstore, + isc_tlsctx_client_session_cache_t **pfound_client_sess_cache); /*%< - * Look up a TLS context in the TLS context cache. + * Look up a TLS context and its associated data in the TLS context cache. * * Requires: *\li 'cache' is a valid pointer to a TLS context cache object; @@ -523,7 +551,10 @@ isc_tlsctx_cache_find(isc_tlsctx_cache_t *cache, const char *name, *\li 'transport' - a valid transport identifier (currently only * TLS/DoT and HTTPS/DoH are supported; *\li 'family' - either 'AF_INET' or 'AF_INET6'; - *\li 'pctx' - a valid pointer to a non-NULL pointer. + *\li 'pctx' - a valid pointer to a non-NULL pointer; + *\li 'pstore' - a valid pointer to a non-NULL pointer or 'NULL'. + *\li 'pfound_client_sess_cache' - a valid pointer to a non-NULL pointer or + *'NULL'. * * Returns: *\li #ISC_R_SUCCESS - the context has been found; diff --git a/lib/isc/tls.c b/lib/isc/tls.c index a7b9d1b40a..2a6923b73d 100644 --- a/lib/isc/tls.c +++ b/lib/isc/tls.c @@ -1086,6 +1086,8 @@ typedef struct isc_tlsctx_cache_entry { * session-resumption cache. */ isc_tlsctx_t *ctx[isc_tlsctx_cache_count - 1][2]; + isc_tlsctx_client_session_cache_t + *client_sess_cache[isc_tlsctx_cache_count - 1][2]; /* * One certificate store is enough for all the contexts defined * above. We need that for peer validation. @@ -1138,6 +1140,11 @@ tlsctx_cache_entry_destroy(isc_mem_t *mctx, isc_tlsctx_cache_entry_t *entry) { if (entry->ctx[i][k] != NULL) { isc_tlsctx_free(&entry->ctx[i][k]); } + + if (entry->client_sess_cache[i][k] != NULL) { + isc_tlsctx_client_session_cache_detach( + &entry->client_sess_cache[i][k]); + } } } if (entry->ca_store != NULL) { @@ -1187,17 +1194,21 @@ isc_tlsctx_cache_detach(isc_tlsctx_cache_t **cachep) { } isc_result_t -isc_tlsctx_cache_add(isc_tlsctx_cache_t *cache, const char *name, - const isc_tlsctx_cache_transport_t transport, - const uint16_t family, isc_tlsctx_t *ctx, - isc_tls_cert_store_t *store, isc_tlsctx_t **pfound, - isc_tls_cert_store_t **pfound_store) { +isc_tlsctx_cache_add( + isc_tlsctx_cache_t *cache, const char *name, + const isc_tlsctx_cache_transport_t transport, const uint16_t family, + isc_tlsctx_t *ctx, isc_tls_cert_store_t *store, + isc_tlsctx_client_session_cache_t *client_sess_cache, + isc_tlsctx_t **pfound, isc_tls_cert_store_t **pfound_store, + isc_tlsctx_client_session_cache_t **pfound_client_sess_cache) { isc_result_t result = ISC_R_FAILURE; size_t name_len, tr_offset; isc_tlsctx_cache_entry_t *entry = NULL; bool ipv6; REQUIRE(VALID_TLSCTX_CACHE(cache)); + REQUIRE(client_sess_cache == NULL || + VALID_TLSCTX_CLIENT_SESSION_CACHE(client_sess_cache)); REQUIRE(name != NULL && *name != '\0'); REQUIRE(transport > isc_tlsctx_cache_none && transport < isc_tlsctx_cache_count); @@ -1213,6 +1224,7 @@ isc_tlsctx_cache_add(isc_tlsctx_cache_t *cache, const char *name, result = isc_ht_find(cache->data, (const uint8_t *)name, name_len, (void **)&entry); if (result == ISC_R_SUCCESS && entry->ctx[tr_offset][ipv6] != NULL) { + isc_tlsctx_client_session_cache_t *found_client_sess_cache; /* The entry exists. */ if (pfound != NULL) { INSIST(*pfound == NULL); @@ -1223,6 +1235,14 @@ isc_tlsctx_cache_add(isc_tlsctx_cache_t *cache, const char *name, INSIST(*pfound_store == NULL); *pfound_store = entry->ca_store; } + + found_client_sess_cache = + entry->client_sess_cache[tr_offset][ipv6]; + if (pfound_client_sess_cache != NULL && + found_client_sess_cache != NULL) { + INSIST(*pfound_client_sess_cache == NULL); + *pfound_client_sess_cache = found_client_sess_cache; + } result = ISC_R_EXISTS; } else if (result == ISC_R_SUCCESS && entry->ctx[tr_offset][ipv6] == NULL) { @@ -1231,10 +1251,11 @@ isc_tlsctx_cache_add(isc_tlsctx_cache_t *cache, const char *name, * particular transport/IP type combination. */ entry->ctx[tr_offset][ipv6] = ctx; + entry->client_sess_cache[tr_offset][ipv6] = client_sess_cache; /* - * As the passed certificates store object is supposed to be - * internally managed by the cache object anyway, we might - * destroy the unneeded store object right now. + * As the passed certificates store object is supposed + * to be internally managed by the cache object anyway, + * we might destroy the unneeded store object right now. */ if (store != NULL && store != entry->ca_store) { isc_tls_cert_store_free(&store); @@ -1249,6 +1270,7 @@ isc_tlsctx_cache_add(isc_tlsctx_cache_t *cache, const char *name, /* Oracle/Red Hat Linux, GCC bug #53119 */ memset(entry, 0, sizeof(*entry)); entry->ctx[tr_offset][ipv6] = ctx; + entry->client_sess_cache[tr_offset][ipv6] = client_sess_cache; entry->ca_store = store; RUNTIME_CHECK(isc_ht_add(cache->data, (const uint8_t *)name, name_len, @@ -1262,10 +1284,11 @@ isc_tlsctx_cache_add(isc_tlsctx_cache_t *cache, const char *name, } isc_result_t -isc_tlsctx_cache_find(isc_tlsctx_cache_t *cache, const char *name, - const isc_tlsctx_cache_transport_t transport, - const uint16_t family, isc_tlsctx_t **pctx, - isc_tls_cert_store_t **pstore) { +isc_tlsctx_cache_find( + isc_tlsctx_cache_t *cache, const char *name, + const isc_tlsctx_cache_transport_t transport, const uint16_t family, + isc_tlsctx_t **pctx, isc_tls_cert_store_t **pstore, + isc_tlsctx_client_session_cache_t **pfound_client_sess_cache) { isc_result_t result = ISC_R_FAILURE; size_t tr_offset; isc_tlsctx_cache_entry_t *entry = NULL; @@ -1292,7 +1315,16 @@ isc_tlsctx_cache_find(isc_tlsctx_cache_t *cache, const char *name, } if (result == ISC_R_SUCCESS && entry->ctx[tr_offset][ipv6] != NULL) { + isc_tlsctx_client_session_cache_t *found_client_sess_cache = + entry->client_sess_cache[tr_offset][ipv6]; + *pctx = entry->ctx[tr_offset][ipv6]; + + if (pfound_client_sess_cache != NULL && + found_client_sess_cache != NULL) { + INSIST(*pfound_client_sess_cache == NULL); + *pfound_client_sess_cache = found_client_sess_cache; + } } else if (result == ISC_R_SUCCESS && entry->ctx[tr_offset][ipv6] == NULL) { result = ISC_R_NOTFOUND; diff --git a/lib/ns/listenlist.c b/lib/ns/listenlist.c index 32a72888d5..f852f06d56 100644 --- a/lib/ns/listenlist.c +++ b/lib/ns/listenlist.c @@ -49,7 +49,7 @@ listenelt_create(isc_mem_t *mctx, in_port_t port, isc_dscp_t dscp, */ result = isc_tlsctx_cache_find(tlsctx_cache, tls_params->name, transport, family, &sslctx, - &found_store); + &found_store, NULL); if (result != ISC_R_SUCCESS) { /* * The lookup failed, let's try to create a new context @@ -150,7 +150,8 @@ listenelt_create(isc_mem_t *mctx, in_port_t port, isc_dscp_t dscp, RUNTIME_CHECK(isc_tlsctx_cache_add( tlsctx_cache, tls_params->name, transport, family, sslctx, store, - NULL, NULL) == ISC_R_SUCCESS); + NULL, NULL, NULL, + NULL) == ISC_R_SUCCESS); } else { INSIST(sslctx != NULL); } From 0a4a76ff7aaeab30e5b619231321af6f9b18c21e Mon Sep 17 00:00:00 2001 From: Artem Boldariev Date: Fri, 22 Apr 2022 15:59:11 +0300 Subject: [PATCH 03/12] TLS stream/DoH: implement TLS client session resumption This commit extends TLS stream code and DoH code with TLS client session resumption support implemented on top of the TLS client session cache. (cherry picked from commit 90bc13a5d52a7997a5cb977d62489d034b344b73) --- bin/dig/dighost.c | 2 +- bin/tests/test_client.c | 2 +- lib/isc/include/isc/netmgr.h | 6 ++- lib/isc/netmgr/http.c | 7 +-- lib/isc/netmgr/netmgr-int.h | 5 ++ lib/isc/netmgr/netmgr.c | 21 ++++++++ lib/isc/netmgr/tlsstream.c | 97 +++++++++++++++++++++++++++++------- tests/isc/doh_test.c | 13 +++-- tests/isc/netmgr_test.c | 13 ++++- 9 files changed, 136 insertions(+), 30 deletions(-) diff --git a/bin/dig/dighost.c b/bin/dig/dighost.c index bb9e13dcdb..a4490de8ae 100644 --- a/bin/dig/dighost.c +++ b/bin/dig/dighost.c @@ -3028,7 +3028,7 @@ start_tcp(dig_query_t *query) { isc_nm_httpconnect(netmgr, &localaddr, &query->sockaddr, uri, !query->lookup->https_get, tcp_connected, connectquery, tlsctx, - local_timeout, 0); + sess_cache, 0, local_timeout); #endif } else { isc_nm_tcpdnsconnect(netmgr, &localaddr, diff --git a/bin/tests/test_client.c b/bin/tests/test_client.c index ee62a6dfb9..10b2103193 100644 --- a/bin/tests/test_client.c +++ b/bin/tests/test_client.c @@ -428,7 +428,7 @@ run(void) { } isc_nm_httpconnect(netmgr, &sockaddr_local, &sockaddr_remote, req_url, is_post, connect_cb, NULL, tls_ctx, - timeout, 0); + NULL, timeout, 0); } break; #endif default: diff --git a/lib/isc/include/isc/netmgr.h b/lib/isc/include/isc/netmgr.h index f2ead2688d..c21e832f33 100644 --- a/lib/isc/include/isc/netmgr.h +++ b/lib/isc/include/isc/netmgr.h @@ -559,13 +559,15 @@ isc_nm_listentls(isc_nm_t *mgr, isc_sockaddr_t *iface, void isc_nm_tlsconnect(isc_nm_t *mgr, isc_sockaddr_t *local, isc_sockaddr_t *peer, isc_nm_cb_t cb, void *cbarg, isc_tlsctx_t *ctx, + isc_tlsctx_client_session_cache_t *client_sess_cache, unsigned int timeout, size_t extrahandlesize); void isc_nm_httpconnect(isc_nm_t *mgr, isc_sockaddr_t *local, isc_sockaddr_t *peer, const char *uri, bool POST, isc_nm_cb_t cb, void *cbarg, - isc_tlsctx_t *ctx, unsigned int timeout, - size_t extrahandlesize); + isc_tlsctx_t *ctx, + isc_tlsctx_client_session_cache_t *client_sess_cache, + unsigned int timeout, size_t extrahandlesize); isc_result_t isc_nm_listenhttp(isc_nm_t *mgr, isc_sockaddr_t *iface, int backlog, diff --git a/lib/isc/netmgr/http.c b/lib/isc/netmgr/http.c index e66d37e8ac..b227947cca 100644 --- a/lib/isc/netmgr/http.c +++ b/lib/isc/netmgr/http.c @@ -1424,8 +1424,9 @@ error: void isc_nm_httpconnect(isc_nm_t *mgr, isc_sockaddr_t *local, isc_sockaddr_t *peer, const char *uri, bool post, isc_nm_cb_t cb, void *cbarg, - isc_tlsctx_t *tlsctx, unsigned int timeout, - size_t extrahandlesize) { + isc_tlsctx_t *tlsctx, + isc_tlsctx_client_session_cache_t *client_sess_cache, + unsigned int timeout, size_t extrahandlesize) { isc_sockaddr_t local_interface; isc_nmsocket_t *sock = NULL; @@ -1487,7 +1488,7 @@ isc_nm_httpconnect(isc_nm_t *mgr, isc_sockaddr_t *local, isc_sockaddr_t *peer, if (tlsctx != NULL) { isc_nm_tlsconnect(mgr, local, peer, transport_connect_cb, sock, - tlsctx, timeout, 0); + tlsctx, client_sess_cache, timeout, 0); } else { isc_nm_tcpconnect(mgr, local, peer, transport_connect_cb, sock, timeout, 0); diff --git a/lib/isc/netmgr/netmgr-int.h b/lib/isc/netmgr/netmgr-int.h index 55eff3ce24..68fd43f339 100644 --- a/lib/isc/netmgr/netmgr-int.h +++ b/lib/isc/netmgr/netmgr-int.h @@ -971,6 +971,8 @@ struct isc_nmsocket { isc_tlsctx_t **listener_tls_ctx; /*%< A context reference per worker */ size_t n_listener_tls_ctx; + isc_tlsctx_client_session_cache_t *client_sess_cache; + bool client_session_saved; isc_nmsocket_t *tlslistener; isc_nmsocket_t *tlssocket; atomic_bool result_updated; @@ -2175,3 +2177,6 @@ isc__nmsocket_writetimeout_cb(void *data, isc_result_t eresult); isc_error_fatal(__FILE__, __LINE__, "%s failed: %s\n", #func, \ uv_strerror(ret)); \ } + +void +isc__nmsocket_log_tls_session_reuse(isc_nmsocket_t *sock, isc_tls_t *tls); diff --git a/lib/isc/netmgr/netmgr.c b/lib/isc/netmgr/netmgr.c index 6494e99743..0661621b99 100644 --- a/lib/isc/netmgr/netmgr.c +++ b/lib/isc/netmgr/netmgr.c @@ -3729,6 +3729,27 @@ isc_nm_verify_tls_peer_result_string(const isc_nmhandle_t *handle) { return (NULL); } +void +isc__nmsocket_log_tls_session_reuse(isc_nmsocket_t *sock, isc_tls_t *tls) { + const int log_level = ISC_LOG_DEBUG(1); + char client_sabuf[ISC_SOCKADDR_FORMATSIZE]; + char local_sabuf[ISC_SOCKADDR_FORMATSIZE]; + + REQUIRE(tls != NULL); + + if (!isc_log_wouldlog(isc_lctx, log_level)) { + return; + }; + + isc_sockaddr_format(&sock->peer, client_sabuf, sizeof(client_sabuf)); + isc_sockaddr_format(&sock->iface, local_sabuf, sizeof(local_sabuf)); + isc_log_write(isc_lctx, ISC_LOGCATEGORY_GENERAL, ISC_LOGMODULE_NETMGR, + log_level, "TLS %s session %s for %s on %s", + SSL_is_server(tls) ? "server" : "client", + SSL_session_reused(tls) ? "resumed" : "created", + client_sabuf, local_sabuf); +} + #ifdef NETMGR_TRACE /* * Dump all active sockets in netmgr. We output to stderr diff --git a/lib/isc/netmgr/tlsstream.c b/lib/isc/netmgr/tlsstream.c index 4fd909d228..ff6aff8829 100644 --- a/lib/isc/netmgr/tlsstream.c +++ b/lib/isc/netmgr/tlsstream.c @@ -86,6 +86,12 @@ tls_cleanup_listener_tlsctx(isc_nmsocket_t *listener); static isc_tlsctx_t * tls_get_listener_tlsctx(isc_nmsocket_t *listener, const int tid); +static void +tls_keep_client_tls_session(isc_nmsocket_t *sock); + +static void +tls_try_shutdown(isc_tls_t *tls, const bool quite); + /* * The socket is closing, outerhandle has been detached, listener is * inactive, or the netmgr is closing: any operation on it should abort @@ -128,6 +134,10 @@ tls_senddone(isc_nmhandle_t *handle, isc_result_t eresult, void *cbarg) { tlssock = send_req->tlssock; send_req->tlssock = NULL; + if (finish) { + tls_try_shutdown(tlssock->tlsstream.tls, true); + } + if (send_req->cb != NULL) { INSIST(VALID_NMHANDLE(tlssock->statichandle)); send_req->cb(send_req->handle, eresult, send_req->cbarg); @@ -241,10 +251,9 @@ tls_send_outgoing(isc_nmsocket_t *sock, bool finish, isc_nmhandle_t *tlshandle, return (0); } - if (finish && (SSL_get_shutdown(sock->tlsstream.tls) & - SSL_SENT_SHUTDOWN) != SSL_SENT_SHUTDOWN) - { - (void)SSL_shutdown(sock->tlsstream.tls); + if (finish) { + tls_try_shutdown(sock->tlsstream.tls, false); + tls_keep_client_tls_session(sock); } pending = BIO_pending(sock->tlsstream.bio_out); @@ -293,22 +302,21 @@ tls_process_outgoing(isc_nmsocket_t *sock, bool finish, isc__nm_uvreq_t *send_data) { int pending; + bool received_shutdown = ((SSL_get_shutdown(sock->tlsstream.tls) & + SSL_RECEIVED_SHUTDOWN) != 0); + bool sent_shutdown = ((SSL_get_shutdown(sock->tlsstream.tls) & + SSL_SENT_SHUTDOWN) != 0); + + if (received_shutdown && !sent_shutdown) { + finish = true; + } + /* Data from TLS to network */ if (send_data != NULL) { pending = tls_send_outgoing(sock, finish, send_data->handle, send_data->cb.send, send_data->cbarg); } else { - bool received_shutdown = - ((SSL_get_shutdown(sock->tlsstream.tls) & - SSL_RECEIVED_SHUTDOWN) != 0); - bool sent_shutdown = ((SSL_get_shutdown(sock->tlsstream.tls) & - SSL_SENT_SHUTDOWN) != 0); - - if (received_shutdown && !sent_shutdown) { - finish = true; - (void)SSL_shutdown(sock->tlsstream.tls); - } pending = tls_send_outgoing(sock, finish, NULL, NULL, NULL); } @@ -331,6 +339,7 @@ tls_try_handshake(isc_nmsocket_t *sock) { isc_result_t result = ISC_R_SUCCESS; INSIST(SSL_is_init_finished(sock->tlsstream.tls) == 1); INSIST(sock->statichandle == NULL); + isc__nmsocket_log_tls_session_reuse(sock, sock->tlsstream.tls); tlshandle = isc__nmhandle_get(sock, &sock->peer, &sock->iface); if (sock->tlsstream.server) { sock->listener->accept_cb(tlshandle, result, @@ -889,7 +898,8 @@ tcp_connected(isc_nmhandle_t *handle, isc_result_t result, void *cbarg); void isc_nm_tlsconnect(isc_nm_t *mgr, isc_sockaddr_t *local, isc_sockaddr_t *peer, - isc_nm_cb_t cb, void *cbarg, SSL_CTX *ctx, + isc_nm_cb_t cb, void *cbarg, isc_tlsctx_t *ctx, + isc_tlsctx_client_session_cache_t *client_sess_cache, unsigned int timeout, size_t extrahandlesize) { isc_nmsocket_t *nsock = NULL; #if defined(NETMGR_TRACE) && defined(NETMGR_TRACE_VERBOSE) @@ -907,6 +917,13 @@ isc_nm_tlsconnect(isc_nm_t *mgr, isc_sockaddr_t *local, isc_sockaddr_t *peer, nsock->connect_cbarg = cbarg; nsock->connect_timeout = timeout; isc_tlsctx_attach(ctx, &nsock->tlsstream.ctx); + atomic_init(&nsock->client, true); + if (client_sess_cache != NULL) { + INSIST(isc_tlsctx_client_session_cache_getctx( + client_sess_cache) == ctx); + isc_tlsctx_client_session_cache_attach( + client_sess_cache, &nsock->tlsstream.client_sess_cache); + } isc_nm_tcpconnect(mgr, local, peer, tcp_connected, nsock, nsock->connect_timeout, 0); @@ -949,6 +966,12 @@ tcp_connected(isc_nmhandle_t *handle, isc_result_t result, void *cbarg) { isc_nmhandle_attach(handle, &tlssock->outerhandle); atomic_store(&tlssock->active, true); + if (tlssock->tlsstream.client_sess_cache != NULL) { + isc_tlsctx_client_session_cache_reuse_sockaddr( + tlssock->tlsstream.client_sess_cache, &tlssock->peer, + tlssock->tlsstream.tls); + } + /* * Hold a reference to tlssock in the TCP socket: it will * detached in isc__nm_tls_cleanup_data(). @@ -1026,15 +1049,26 @@ isc__nm_tls_cleanup_data(isc_nmsocket_t *sock) { } else if (sock->type == isc_nm_tlslistener) { tls_cleanup_listener_tlsctx(sock); } else if (sock->type == isc_nm_tlssocket) { - if (sock->tlsstream.ctx != NULL) { - isc_tlsctx_free(&sock->tlsstream.ctx); - } if (sock->tlsstream.tls != NULL) { + /* + * Let's shutdown the TLS session properly so that the + * session will remain resumable, if required. + */ + tls_try_shutdown(sock->tlsstream.tls, true); + tls_keep_client_tls_session(sock); isc_tls_free(&sock->tlsstream.tls); /* These are destroyed when we free SSL */ sock->tlsstream.bio_out = NULL; sock->tlsstream.bio_in = NULL; } + if (sock->tlsstream.ctx != NULL) { + isc_tlsctx_free(&sock->tlsstream.ctx); + } + if (sock->tlsstream.client_sess_cache != NULL) { + INSIST(atomic_load(&sock->client)); + isc_tlsctx_client_session_cache_detach( + &sock->tlsstream.client_sess_cache); + } } else if (sock->type == isc_nm_tcpsocket && sock->tlsstream.tlssocket != NULL) { /* @@ -1164,3 +1198,30 @@ isc__nm_async_tls_set_tlsctx(isc_nmsocket_t *listener, isc_tlsctx_t *tlsctx, isc_tlsctx_free(&listener->tlsstream.listener_tls_ctx[tid]); isc_tlsctx_attach(tlsctx, &listener->tlsstream.listener_tls_ctx[tid]); } + +static void +tls_keep_client_tls_session(isc_nmsocket_t *sock) { + /* + * Ensure that the isc_tls_t is being accessed from + * within the worker thread the socket is bound to. + */ + REQUIRE(sock->tid == isc_nm_tid()); + if (sock->tlsstream.client_sess_cache != NULL && + sock->tlsstream.client_session_saved == false) + { + INSIST(atomic_load(&sock->client)); + isc_tlsctx_client_session_cache_keep_sockaddr( + sock->tlsstream.client_sess_cache, &sock->peer, + sock->tlsstream.tls); + sock->tlsstream.client_session_saved = true; + } +} + +static void +tls_try_shutdown(isc_tls_t *tls, const bool force) { + if (force) { + (void)SSL_set_shutdown(tls, SSL_SENT_SHUTDOWN); + } else if ((SSL_get_shutdown(tls) & SSL_SENT_SHUTDOWN) == 0) { + (void)SSL_shutdown(tls); + } +} diff --git a/tests/isc/doh_test.c b/tests/isc/doh_test.c index 6fa2d21588..ce3854cfca 100644 --- a/tests/isc/doh_test.c +++ b/tests/isc/doh_test.c @@ -79,6 +79,7 @@ static atomic_bool slowdown = false; static atomic_bool use_TLS = false; static isc_tlsctx_t *server_tlsctx = NULL; static isc_tlsctx_t *client_tlsctx = NULL; +static isc_tlsctx_client_session_cache_t *client_sess_cache = NULL; static isc_quota_t listener_quota; static atomic_bool check_listener_quota = false; @@ -176,7 +177,8 @@ connect_send_request(isc_nm_t *mgr, const char *uri, bool post, } isc_nm_httpconnect(mgr, NULL, &tcp_listen_addr, uri, post, - connect_send_cb, data, ctx, timeout, 0); + connect_send_cb, data, ctx, client_sess_cache, + timeout, 0); } static int @@ -321,6 +323,9 @@ setup_test(void **state) { client_tlsctx = NULL; isc_tlsctx_createclient(&client_tlsctx); isc_tlsctx_enable_http2client_alpn(client_tlsctx); + client_sess_cache = isc_tlsctx_client_session_cache_new( + mctx, client_tlsctx, + ISC_TLSCTX_CLIENT_SESSION_CACHE_DEFAULT_SIZE); isc_quota_init(&listener_quota, 0); atomic_store(&check_listener_quota, false); @@ -350,6 +355,8 @@ teardown_test(void **state) { isc_tlsctx_free(&client_tlsctx); } + isc_tlsctx_client_session_cache_detach(&client_sess_cache); + isc_quota_destroy(&listener_quota); isc_nm_http_endpoints_detach(&endpoints); @@ -644,7 +651,7 @@ doh_timeout_recovery(void **state) { ISC_NM_HTTP_DEFAULT_PATH); isc_nm_httpconnect(connect_nm, NULL, &tcp_listen_addr, req_url, atomic_load(&POST), timeout_request_cb, NULL, ctx, - T_SOFT, 0); + client_sess_cache, T_SOFT, 0); /* * Sleep until sends reaches 5. @@ -932,7 +939,7 @@ doh_recv_two(void **state) { isc_nm_httpconnect(connect_nm, NULL, &tcp_listen_addr, req_url, atomic_load(&POST), doh_connect_send_two_requests_cb, - NULL, ctx, 5000, 0); + NULL, ctx, client_sess_cache, 5000, 0); while (atomic_load(&nsends) > 0) { if (atomic_load(&was_error)) { diff --git a/tests/isc/netmgr_test.c b/tests/isc/netmgr_test.c index 38b147b43f..07d5f6fc4d 100644 --- a/tests/isc/netmgr_test.c +++ b/tests/isc/netmgr_test.c @@ -28,6 +28,7 @@ #include #include #include +#include #include #include "uv_wrap.h" @@ -62,6 +63,7 @@ static isc_sockaddr_t tcp_listen_addr; static isc_sockaddr_t tcp_connect_addr; static isc_tlsctx_t *tcp_listen_tlsctx = NULL; static isc_tlsctx_t *tcp_connect_tlsctx = NULL; +static isc_tlsctx_client_session_cache_t *tcp_tlsctx_client_sess_cache = NULL; static uint64_t send_magic = 0; static uint64_t stop_magic = 0; @@ -333,6 +335,10 @@ setup_test(void **state __attribute__((unused))) { isc_tlsctx_enable_dot_client_alpn(tcp_connect_tlsctx); + tcp_tlsctx_client_sess_cache = isc_tlsctx_client_session_cache_new( + mctx, tcp_connect_tlsctx, + ISC_TLSCTX_CLIENT_SESSION_CACHE_DEFAULT_SIZE); + return (0); } @@ -361,6 +367,8 @@ teardown_test(void **state __attribute__((unused))) { isc_refcount_destroy(&active_ssends); isc_refcount_destroy(&active_sreads); + isc_tlsctx_client_session_cache_detach(&tcp_tlsctx_client_sess_cache); + return (0); } @@ -1156,7 +1164,8 @@ stream_connect(isc_nm_cb_t cb, void *cbarg, unsigned int timeout, if (stream_use_TLS) { isc_nm_tlsconnect(connect_nm, &tcp_connect_addr, &tcp_listen_addr, cb, cbarg, - tcp_connect_tlsctx, timeout, extrahandlesize); + tcp_connect_tlsctx, + tcp_tlsctx_client_sess_cache, timeout, 0); return; } #endif @@ -2059,7 +2068,7 @@ static void tls_connect(isc_nm_t *nm) { isc_nm_tlsconnect(nm, &tcp_connect_addr, &tcp_listen_addr, connect_connect_cb, NULL, tcp_connect_tlsctx, - T_CONNECT, 0); + tcp_tlsctx_client_sess_cache, T_CONNECT, 0); } ISC_RUN_TEST_IMPL(tls_noop) { From e02284354abdc5056d17ef2a854d363c0c543000 Mon Sep 17 00:00:00 2001 From: Artem Boldariev Date: Mon, 25 Apr 2022 16:47:06 +0300 Subject: [PATCH 04/12] DoT: implement TLS client session resumption This commit extends DoT code with TLS client session resumption support implemented on top of the TLS client session cache. (cherry picked from commit 86465c1dacb704a5e95cd3311595f27c323e0507) --- bin/dig/dighost.c | 2 +- bin/tests/test_client.c | 3 +- lib/dns/xfrin.c | 2 +- lib/isc/include/isc/netmgr.h | 3 +- lib/isc/netmgr/netmgr-int.h | 2 + lib/isc/netmgr/tlsdns.c | 78 +++++++++++++++++++++++++++++++----- tests/isc/netmgr_test.c | 18 ++++----- 7 files changed, 86 insertions(+), 22 deletions(-) diff --git a/bin/dig/dighost.c b/bin/dig/dighost.c index a4490de8ae..833b31953a 100644 --- a/bin/dig/dighost.c +++ b/bin/dig/dighost.c @@ -3008,7 +3008,7 @@ start_tcp(dig_query_t *query) { isc_nm_tlsdnsconnect(netmgr, &localaddr, &query->sockaddr, tcp_connected, connectquery, local_timeout, 0, - tlsctx); + tlsctx, sess_cache); #if HAVE_LIBNGHTTP2 } else if (query->lookup->https_mode) { char uri[4096] = { 0 }; diff --git a/bin/tests/test_client.c b/bin/tests/test_client.c index 10b2103193..ad6a4c7f41 100644 --- a/bin/tests/test_client.c +++ b/bin/tests/test_client.c @@ -407,7 +407,8 @@ run(void) { isc_tlsctx_createclient(&tls_ctx); isc_nm_tlsdnsconnect(netmgr, &sockaddr_local, &sockaddr_remote, - connect_cb, NULL, timeout, 0, tls_ctx); + connect_cb, NULL, timeout, 0, tls_ctx, + NULL); break; } #if HAVE_LIBNGHTTP2 diff --git a/lib/dns/xfrin.c b/lib/dns/xfrin.c index 6701174525..f14324618a 100644 --- a/lib/dns/xfrin.c +++ b/lib/dns/xfrin.c @@ -1118,7 +1118,7 @@ xfrin_start(dns_xfrin_ctx_t *xfr) { } isc_nm_tlsdnsconnect(xfr->netmgr, &xfr->sourceaddr, &xfr->primaryaddr, xfrin_connect_done, - connect_xfr, 30000, 0, tlsctx); + connect_xfr, 30000, 0, tlsctx, sess_cache); } break; default: UNREACHABLE(); diff --git a/lib/isc/include/isc/netmgr.h b/lib/isc/include/isc/netmgr.h index c21e832f33..f979635398 100644 --- a/lib/isc/include/isc/netmgr.h +++ b/lib/isc/include/isc/netmgr.h @@ -519,7 +519,8 @@ isc_nm_tcpdnsconnect(isc_nm_t *mgr, isc_sockaddr_t *local, isc_sockaddr_t *peer, void isc_nm_tlsdnsconnect(isc_nm_t *mgr, isc_sockaddr_t *local, isc_sockaddr_t *peer, isc_nm_cb_t cb, void *cbarg, unsigned int timeout, - size_t extrahandlesize, isc_tlsctx_t *sslctx); + size_t extrahandlesize, isc_tlsctx_t *sslctx, + isc_tlsctx_client_session_cache_t *client_sess_cache); /*%< * Establish a DNS client connection via a TCP or TLS connection, bound to * the address 'local' and connected to the address 'peer'. diff --git a/lib/isc/netmgr/netmgr-int.h b/lib/isc/netmgr/netmgr-int.h index 68fd43f339..6b88638011 100644 --- a/lib/isc/netmgr/netmgr-int.h +++ b/lib/isc/netmgr/netmgr-int.h @@ -940,6 +940,8 @@ struct isc_nmsocket { struct tls { isc_tls_t *tls; isc_tlsctx_t *ctx; + isc_tlsctx_client_session_cache_t *client_sess_cache; + bool client_session_saved; BIO *app_rbio; BIO *app_wbio; BIO *ssl_rbio; diff --git a/lib/isc/netmgr/tlsdns.c b/lib/isc/netmgr/tlsdns.c index a66cba45f7..0c8e91fce8 100644 --- a/lib/isc/netmgr/tlsdns.c +++ b/lib/isc/netmgr/tlsdns.c @@ -80,6 +80,14 @@ tls_cycle(isc_nmsocket_t *sock); static void call_pending_send_callbacks(isc_nmsocket_t *sock, const isc_result_t result); +static void +tlsdns_keep_client_tls_session(isc_nmsocket_t *sock); + +static void +tlsdns_set_tls_shutdown(isc_tls_t *tls) { + (void)SSL_set_shutdown(tls, SSL_SENT_SHUTDOWN); +} + static bool peer_verification_has_failed(isc_nmsocket_t *sock) { if (sock->tls.tls != NULL && sock->tls.state == TLS_STATE_HANDSHAKE && @@ -295,11 +303,17 @@ tlsdns_connect_cb(uv_connect_t *uvreq, int status) { SSL_set_bio(sock->tls.tls, sock->tls.ssl_rbio, sock->tls.ssl_wbio); #endif - SSL_set_connect_state(sock->tls.tls); - result = isc_sockaddr_fromsockaddr(&sock->peer, (struct sockaddr *)&ss); RUNTIME_CHECK(result == ISC_R_SUCCESS); + if (sock->tls.client_sess_cache != NULL) { + isc_tlsctx_client_session_cache_reuse_sockaddr( + sock->tls.client_sess_cache, &sock->peer, + sock->tls.tls); + } + + SSL_set_connect_state(sock->tls.tls); + /* Setting pending req */ sock->tls.pending_req = req; @@ -324,7 +338,8 @@ error: void isc_nm_tlsdnsconnect(isc_nm_t *mgr, isc_sockaddr_t *local, isc_sockaddr_t *peer, isc_nm_cb_t cb, void *cbarg, unsigned int timeout, - size_t extrahandlesize, isc_tlsctx_t *sslctx) { + size_t extrahandlesize, isc_tlsctx_t *sslctx, + isc_tlsctx_client_session_cache_t *client_sess_cache) { isc_result_t result = ISC_R_SUCCESS; isc_nmsocket_t *sock = NULL; isc__netievent_tlsdnsconnect_t *ievent = NULL; @@ -355,6 +370,13 @@ isc_nm_tlsdnsconnect(isc_nm_t *mgr, isc_sockaddr_t *local, isc_sockaddr_t *peer, req->local = *local; req->handle = isc__nmhandle_get(sock, &req->peer, &sock->iface); + if (client_sess_cache != NULL) { + INSIST(isc_tlsctx_client_session_cache_getctx( + client_sess_cache) == sslctx); + isc_tlsctx_client_session_cache_attach( + client_sess_cache, &sock->tls.client_sess_cache); + } + result = isc__nm_socket(sa_family, SOCK_STREAM, 0, &sock->fd); if (result != ISC_R_SUCCESS) { goto failure; @@ -1014,6 +1036,11 @@ isc__nm_tlsdns_processbuffer(isc_nmsocket_t *sock) { isc_nmhandle_detach(&handle); + if (isc__nmsocket_closing(sock)) { + tlsdns_set_tls_shutdown(sock->tls.tls); + tlsdns_keep_client_tls_session(sock); + } + return (ISC_R_SUCCESS); } @@ -1116,6 +1143,8 @@ tls_cycle_input(isc_nmsocket_t *sock) { const unsigned char *alpn = NULL; unsigned int alpnlen = 0; + isc__nmsocket_log_tls_session_reuse(sock, sock->tls.tls); + isc_tls_get_selected_alpn(sock->tls.tls, &alpn, &alpnlen); if (alpn != NULL && alpnlen == ISC_TLS_DOT_PROTO_ALPN_ID_LEN && memcmp(ISC_TLS_DOT_PROTO_ALPN_ID, alpn, @@ -1841,6 +1870,12 @@ tlsdns_close_sock(isc_nmsocket_t *sock) { atomic_store(&sock->connected, false); if (sock->tls.tls != NULL) { + /* + * Let's shutdown the TLS session properly so that the session + * will remain resumable, if required. + */ + tlsdns_set_tls_shutdown(sock->tls.tls); + tlsdns_keep_client_tls_session(sock); isc_tls_free(&sock->tls.tls); } @@ -2021,7 +2056,7 @@ isc__nm_tlsdns_shutdown(isc_nmsocket_t *sock) { if (sock->tls.tls) { /* Shutdown any active TLS connections */ - (void)SSL_shutdown(sock->tls.tls); + tlsdns_set_tls_shutdown(sock->tls.tls); } if (atomic_load(&sock->accepting)) { @@ -2152,10 +2187,35 @@ isc__nm_async_tlsdns_set_tlsctx(isc_nmsocket_t *listener, isc_tlsctx_t *tlsctx, void isc__nm_tlsdns_cleanup_data(isc_nmsocket_t *sock) { - if ((sock->type == isc_nm_tlsdnslistener || - sock->type == isc_nm_tlsdnssocket) && - sock->tls.ctx != NULL) - { - isc_tlsctx_free(&sock->tls.ctx); + if (sock->type == isc_nm_tlsdnslistener || + sock->type == isc_nm_tlsdnssocket) { + if (sock->tls.client_sess_cache != NULL) { + INSIST(atomic_load(&sock->client)); + INSIST(sock->type == isc_nm_tlsdnssocket); + isc_tlsctx_client_session_cache_detach( + &sock->tls.client_sess_cache); + } + if (sock->tls.ctx != NULL) { + INSIST(ISC_LIST_EMPTY(sock->tls.sendreqs)); + isc_tlsctx_free(&sock->tls.ctx); + } + } +} + +static void +tlsdns_keep_client_tls_session(isc_nmsocket_t *sock) { + /* + * Ensure that the isc_tls_t is being accessed from + * within the worker thread the socket is bound to. + */ + REQUIRE(sock->tid == isc_nm_tid()); + if (sock->tls.client_sess_cache != NULL && + sock->tls.client_session_saved == false) + { + INSIST(atomic_load(&sock->client)); + isc_tlsctx_client_session_cache_keep_sockaddr( + sock->tls.client_sess_cache, &sock->peer, + sock->tls.tls); + sock->tls.client_session_saved = true; } } diff --git a/tests/isc/netmgr_test.c b/tests/isc/netmgr_test.c index 07d5f6fc4d..f407a3647b 100644 --- a/tests/isc/netmgr_test.c +++ b/tests/isc/netmgr_test.c @@ -2229,7 +2229,7 @@ static void tlsdns_connect(isc_nm_t *nm) { isc_nm_tlsdnsconnect(nm, &tcp_connect_addr, &tcp_listen_addr, connect_connect_cb, NULL, T_CONNECT, 0, - tcp_connect_tlsctx); + tcp_connect_tlsctx, tcp_tlsctx_client_sess_cache); } ISC_RUN_TEST_IMPL(tlsdns_noop) { @@ -2249,7 +2249,7 @@ ISC_RUN_TEST_IMPL(tlsdns_noop) { isc_refcount_increment0(&active_cconnects); isc_nm_tlsdnsconnect(connect_nm, &tcp_connect_addr, &tcp_listen_addr, connect_connect_cb, NULL, T_CONNECT, 0, - tcp_connect_tlsctx); + tcp_connect_tlsctx, tcp_tlsctx_client_sess_cache); isc__netmgr_shutdown(connect_nm); @@ -2276,7 +2276,7 @@ ISC_RUN_TEST_IMPL(tlsdns_noresponse) { isc_refcount_increment0(&active_cconnects); isc_nm_tlsdnsconnect(connect_nm, &connect_addr, &tcp_listen_addr, connect_connect_cb, NULL, T_CONNECT, 0, - tcp_connect_tlsctx); + tcp_connect_tlsctx, tcp_tlsctx_client_sess_cache); WAIT_FOR_EQ(cconnects, 1); WAIT_FOR_EQ(csends, 1); @@ -2330,7 +2330,7 @@ ISC_RUN_TEST_IMPL(tlsdns_timeout_recovery) { isc_refcount_increment0(&active_cconnects); isc_nm_tlsdnsconnect(connect_nm, &tcp_connect_addr, &tcp_listen_addr, connect_connect_cb, NULL, T_SOFT, 0, - tcp_connect_tlsctx); + tcp_connect_tlsctx, tcp_tlsctx_client_sess_cache); WAIT_FOR_EQ(cconnects, 1); WAIT_FOR_GE(csends, 1); @@ -2361,7 +2361,7 @@ ISC_RUN_TEST_IMPL(tlsdns_recv_one) { isc_refcount_increment0(&active_cconnects); isc_nm_tlsdnsconnect(connect_nm, &tcp_connect_addr, &tcp_listen_addr, connect_connect_cb, NULL, T_CONNECT, 0, - tcp_connect_tlsctx); + tcp_connect_tlsctx, tcp_tlsctx_client_sess_cache); WAIT_FOR_EQ(cconnects, 1); WAIT_FOR_LE(nsends, 0); @@ -2403,14 +2403,14 @@ ISC_RUN_TEST_IMPL(tlsdns_recv_two) { isc_refcount_increment0(&active_cconnects); isc_nm_tlsdnsconnect(connect_nm, &tcp_connect_addr, &tcp_listen_addr, connect_connect_cb, NULL, T_CONNECT, 0, - tcp_connect_tlsctx); + tcp_connect_tlsctx, tcp_tlsctx_client_sess_cache); WAIT_FOR_EQ(cconnects, 1); isc_refcount_increment0(&active_cconnects); isc_nm_tlsdnsconnect(connect_nm, &tcp_connect_addr, &tcp_listen_addr, connect_connect_cb, NULL, T_CONNECT, 0, - tcp_connect_tlsctx); + tcp_connect_tlsctx, tcp_tlsctx_client_sess_cache); WAIT_FOR_EQ(cconnects, 2); @@ -2673,7 +2673,7 @@ ISC_RUN_TEST_IMPL(tlsdns_connect_noalpn) { isc_refcount_increment0(&active_cconnects); isc_nm_tlsdnsconnect(connect_nm, &connect_addr, &tcp_listen_addr, tlsdns_connect_connect_noalpn, NULL, T_CONNECT, 0, - connect_tlsctx_noalpn); + connect_tlsctx_noalpn, NULL); WAIT_FOR_EQ(active_cconnects, 0); @@ -2739,7 +2739,7 @@ ISC_RUN_TEST_IMPL(tlsdns_listen_noalpn) { isc_refcount_increment0(&active_cconnects); isc_nm_tlsdnsconnect(connect_nm, &connect_addr, &tcp_listen_addr, connect_connect_cb, NULL, T_CONNECT, 0, - tcp_connect_tlsctx); + tcp_connect_tlsctx, tcp_tlsctx_client_sess_cache); WAIT_FOR_EQ(saccepts, 1); WAIT_FOR_EQ(cconnects, 1); From 5154bac7c52ef3fda7d860891ff9769b183007fa Mon Sep 17 00:00:00 2001 From: Artem Boldariev Date: Mon, 9 May 2022 19:08:29 +0300 Subject: [PATCH 05/12] Add SSL_SESSION_is_resumable() implementation shim This commit adds SSL_SESSION_is_resumable() implementation if it is missing. (cherry picked from commit 35338b41058b0bcebb137eb098e785f171d0476f) --- configure.ac | 1 + lib/isc/openssl_shim.c | 8 ++++++++ lib/isc/openssl_shim.h | 5 +++++ 3 files changed, 14 insertions(+) diff --git a/configure.ac b/configure.ac index c6e2877362..6d3e5e7bda 100644 --- a/configure.ac +++ b/configure.ac @@ -663,6 +663,7 @@ AC_CHECK_FUNCS([SSL_CTX_set_min_proto_version]) AC_CHECK_FUNCS([SSL_CTX_up_ref]) AC_CHECK_FUNCS([SSL_read_ex SSL_peek_ex SSL_write_ex]) AC_CHECK_FUNCS([SSL_CTX_set1_cert_store X509_STORE_up_ref]) +AC_CHECK_FUNCS([SSL_SESSION_is_resumable]) # # Check for algorithm support in OpenSSL diff --git a/lib/isc/openssl_shim.c b/lib/isc/openssl_shim.c index b8dbfaa71b..1099a4a92f 100644 --- a/lib/isc/openssl_shim.c +++ b/lib/isc/openssl_shim.c @@ -196,3 +196,11 @@ SSL_CTX_set1_cert_store(SSL_CTX *ctx, X509_STORE *store) { } #endif /* !HAVE_SSL_CTX_SET1_CERT_STORE */ + +#if !HAVE_SSL_SESSION_IS_RESUMABLE +int +SSL_SESSION_is_resumable(const SSL_SESSION *sess) { + return (!sess->not_resumable && + (sess->session_id_length > 0 || sess->tlsext_ticklen > 0)); +} +#endif /* HAVE_SSL_SESSION_IS_RESUMABLE */ diff --git a/lib/isc/openssl_shim.h b/lib/isc/openssl_shim.h index c0abd14467..005055c3e8 100644 --- a/lib/isc/openssl_shim.h +++ b/lib/isc/openssl_shim.h @@ -135,3 +135,8 @@ X509_STORE_up_ref(X509_STORE *v); void SSL_CTX_set1_cert_store(SSL_CTX *ctx, X509_STORE *store); #endif /* !HAVE_SSL_CTX_SET1_CERT_STORE */ + +#if !HAVE_SSL_SESSION_IS_RESUMABLE +int +SSL_SESSION_is_resumable(const SSL_SESSION *s); +#endif /* HAVE_SSL_SESSION_IS_RESUMABLE */ From 3393ec19c27da6ddd3b1a9b7405b3df9255ff799 Mon Sep 17 00:00:00 2001 From: Artem Boldariev Date: Mon, 9 May 2022 19:40:01 +0300 Subject: [PATCH 06/12] Modify CHANGES Mention that TLS session resumption is now fully supported in the client side code. (cherry picked from commit aa8c258fba51b1e5761b167f55b6bca2a0dd479e) --- CHANGES | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGES b/CHANGES index 78ea4cb640..5c1930db74 100644 --- a/CHANGES +++ b/CHANGES @@ -32,6 +32,9 @@ 5895. [test] Add new set of unit test macros and move the unit tests under single namespace in /tests/. [GL !6243] +5893. [func] Add TLS session resumption support to the client-side + TLS code. [GL !6274] + 5891. [func] Key timing options for `dnssec-settime` and related utilities now accept "UNSET" times as printed by `dnssec-settime -p`. [GL #3361] From cb6591f277bd5d0f8fad5ff4426d4bde1a1659ce Mon Sep 17 00:00:00 2001 From: Artem Boldariev Date: Tue, 10 May 2022 19:44:28 +0300 Subject: [PATCH 07/12] Avoid aborting when uv_timer_start() is used on a closing socket In such a case it will return UV_EINVAL (-EINVAL), leading to aborting, as the code expects the function to succeed. (cherry picked from commit 245f7cec2ea9005a360478959e4a446c666eab75) --- lib/isc/netmgr/netmgr.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/lib/isc/netmgr/netmgr.c b/lib/isc/netmgr/netmgr.c index 0661621b99..8bad215f9d 100644 --- a/lib/isc/netmgr/netmgr.c +++ b/lib/isc/netmgr/netmgr.c @@ -2077,6 +2077,10 @@ void isc__nmsocket_timer_restart(isc_nmsocket_t *sock) { REQUIRE(VALID_NMSOCK(sock)); + if (uv_is_closing((uv_handle_t *)&sock->read_timer)) { + return; + } + if (atomic_load(&sock->connecting)) { int r; From 0cec9cca37f45e5bd028337af30b6660e36aa5a3 Mon Sep 17 00:00:00 2001 From: Artem Boldariev Date: Tue, 10 May 2022 19:46:12 +0300 Subject: [PATCH 08/12] Fix an abort in DoH (client-side) when writing on closing sock The commit fixes a corner case in client-side DoH code, when a write attempt is done on a closing socket (session). The change ensures that the write call-back will be called with a proper error code (see failed_send_cb() call in client_httpsend()). (cherry picked from commit 9abb00bb5f66c1d365fb0e69bc96fa2e49bd37cf) --- lib/isc/netmgr/http.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/lib/isc/netmgr/http.c b/lib/isc/netmgr/http.c index b227947cca..a3738106dd 100644 --- a/lib/isc/netmgr/http.c +++ b/lib/isc/netmgr/http.c @@ -1508,7 +1508,12 @@ client_send(isc_nmhandle_t *handle, const isc_region_t *region) { REQUIRE(region != NULL); REQUIRE(region->base != NULL); REQUIRE(region->length <= MAX_DNS_MESSAGE_SIZE); - REQUIRE(cstream != NULL); + + if (session->closed) { + return (ISC_R_CANCELED); + } + + INSIST(cstream != NULL); if (cstream->post) { /* POST */ From 7bd5f4972f9586671da7a81c0aab6c51761bde02 Mon Sep 17 00:00:00 2001 From: Artem Boldariev Date: Tue, 10 May 2022 23:09:59 +0300 Subject: [PATCH 09/12] Dig: Do not call isc_nm_cancelread() for HTTP sockets This commit ensures that isc_nm_cancelread() is not called from within dig code for HTTP sockets, as these lack its implementation. It does not have much sense to have it due to transactional nature of HTTP. Every HTTP request-response pair is represented by a virtual socket, where read callback is called only when full DNS message is received or when an error code is being passed there. That is, there is nothing to cancel at the time of the call. (cherry picked from commit 90c52ca12b24345d22739c1277ab28170811deb8) --- bin/dig/dighost.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/bin/dig/dighost.c b/bin/dig/dighost.c index 833b31953a..9eb6b54571 100644 --- a/bin/dig/dighost.c +++ b/bin/dig/dighost.c @@ -2760,7 +2760,8 @@ _cancel_lookup(dig_lookup_t *lookup, const char *file, unsigned int line) { debug("canceling pending query %p, belonging to %p", query, query->lookup); query->canceled = true; - if (query->readhandle != NULL) { + if (query->readhandle != NULL && + !isc_nm_is_http_handle(query->readhandle)) { isc_nm_cancelread(query->readhandle); } query_detach(&query); @@ -4618,7 +4619,8 @@ cancel_all(void) { debug("canceling pending query %p, belonging to %p", q, current_lookup); q->canceled = true; - if (q->readhandle != NULL) { + if (q->readhandle != NULL && + !isc_nm_is_http_handle(q->readhandle)) { isc_nm_cancelread(q->readhandle); } query_detach(&q); From 334eeef5a1b2861355c731493e7bbc5152165e49 Mon Sep 17 00:00:00 2001 From: Artem Boldariev Date: Mon, 23 May 2022 13:41:06 +0300 Subject: [PATCH 10/12] Do not provide a shim for SSL_SESSION_is_resumable() The recently added TLS client session cache used SSL_SESSION_is_resumable() to avoid polluting the cache with non-resumable sessions. However, it turned out that we cannot provide a shim for this function across the whole range of OpenSSL versions due to the fact that OpenSSL 1.1.0 does uses opaque pointers for SSL_SESSION objects. The commit replaces the shim for SSL_SESSION_is_resumable() with a non public approximation of it on systems shipped with OpenSSL 1.1.0. It is not turned into a proper shim because it does not fully emulate the behaviour of SSL_SESSION_is_resumable(), but in our case it is good enough, as it still helps to protect the cache from pollution. For systems shipped with OpenSSL 1.0.X and derivatives (e.g. older versions of LibreSSL), the provided replacement perfectly mimics the function it is intended to replace. (cherry picked from commit 40be3c926309867ebaeb4b1dcd7e1199473dea4d) --- lib/isc/openssl_shim.c | 8 -------- lib/isc/openssl_shim.h | 5 ----- lib/isc/tls.c | 29 ++++++++++++++++++++++++++++- 3 files changed, 28 insertions(+), 14 deletions(-) diff --git a/lib/isc/openssl_shim.c b/lib/isc/openssl_shim.c index 1099a4a92f..b8dbfaa71b 100644 --- a/lib/isc/openssl_shim.c +++ b/lib/isc/openssl_shim.c @@ -196,11 +196,3 @@ SSL_CTX_set1_cert_store(SSL_CTX *ctx, X509_STORE *store) { } #endif /* !HAVE_SSL_CTX_SET1_CERT_STORE */ - -#if !HAVE_SSL_SESSION_IS_RESUMABLE -int -SSL_SESSION_is_resumable(const SSL_SESSION *sess) { - return (!sess->not_resumable && - (sess->session_id_length > 0 || sess->tlsext_ticklen > 0)); -} -#endif /* HAVE_SSL_SESSION_IS_RESUMABLE */ diff --git a/lib/isc/openssl_shim.h b/lib/isc/openssl_shim.h index 005055c3e8..c0abd14467 100644 --- a/lib/isc/openssl_shim.h +++ b/lib/isc/openssl_shim.h @@ -135,8 +135,3 @@ X509_STORE_up_ref(X509_STORE *v); void SSL_CTX_set1_cert_store(SSL_CTX *ctx, X509_STORE *store); #endif /* !HAVE_SSL_CTX_SET1_CERT_STORE */ - -#if !HAVE_SSL_SESSION_IS_RESUMABLE -int -SSL_SESSION_is_resumable(const SSL_SESSION *s); -#endif /* HAVE_SSL_SESSION_IS_RESUMABLE */ diff --git a/lib/isc/tls.c b/lib/isc/tls.c index 2a6923b73d..a2264a3945 100644 --- a/lib/isc/tls.c +++ b/lib/isc/tls.c @@ -1484,6 +1484,33 @@ isc_tlsctx_client_session_cache_detach( isc_mem_putanddetach(&cache->mctx, cache, sizeof(*cache)); } +static bool +ssl_session_seems_resumable(const SSL_SESSION *sess) { +#ifdef HAVE_SSL_SESSION_IS_RESUMABLE + /* + * If SSL_SESSION_is_resumable() is available, let's use that. It + * is expected to be available on OpenSSL >= 1.1.1 and its modern + * siblings. + */ + return (SSL_SESSION_is_resumable(sess) != 0); +#elif (OPENSSL_VERSION_NUMBER >= 0x10100000L) + /* + * Taking into consideration that OpenSSL 1.1.0 uses opaque + * pointers for SSL_SESSION, we cannot implement a replacement for + * SSL_SESSION_is_resumable() manually. Let's use a sensible + * approximation for that, then: if there is an associated session + * ticket or session ID, then, most likely, the session is + * resumable. + */ + unsigned int session_id_len = 0; + (void)SSL_SESSION_get_id(sess, &session_id_len); + return (SSL_SESSION_has_ticket(sess) || session_id_len > 0); +#else + return (!sess->not_resumable && + (sess->session_id_length > 0 || sess->tlsext_ticklen > 0)); +#endif +} + void isc_tlsctx_client_session_cache_keep(isc_tlsctx_client_session_cache_t *cache, char *remote_peer_name, isc_tls_t *tls) { @@ -1500,7 +1527,7 @@ isc_tlsctx_client_session_cache_keep(isc_tlsctx_client_session_cache_t *cache, sess = SSL_get1_session(tls); if (sess == NULL) { return; - } else if (SSL_SESSION_is_resumable(sess) == 0) { + } else if (!ssl_session_seems_resumable(sess)) { SSL_SESSION_free(sess); return; } From a954eacbacf826324c928e614a4323980d5718e9 Mon Sep 17 00:00:00 2001 From: Artem Boldariev Date: Tue, 24 May 2022 11:25:30 +0300 Subject: [PATCH 11/12] CID 352849: refactor get_create_tls_context() within dighost.c This commit removes dead code from cleanup handling part of the get_create_tls_context(). In particular, currently: * there is no way 'found_ctx' might equal 'ctx'; * there is no way 'session_cache' might equal a non-NULL value while cleaning up after a TLS initialisation error. (cherry picked from commit 095b608412b94ae1437458714e9e2461ff998a48) --- bin/dig/dighost.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/bin/dig/dighost.c b/bin/dig/dighost.c index 9eb6b54571..ca3a406232 100644 --- a/bin/dig/dighost.c +++ b/bin/dig/dighost.c @@ -2879,15 +2879,19 @@ get_create_tls_context(dig_query_t *query, const bool is_https, INSIST(!query->lookup->tls_ca_set || found_store != NULL); return (found_ctx); failure: - if (ctx != NULL && found_ctx != ctx) { + if (ctx != NULL) { isc_tlsctx_free(&ctx); } + /* + * The 'found_store' is being managed by the TLS context + * cache. Thus, we should keep it as it is, as it will get + * destroyed alongside the cache. As there is one store per + * multiple TLS contexts, we need to handle store deletion in a + * special way. + */ if (store != NULL && store != found_store) { isc_tls_cert_store_free(&store); } - if (sess_cache != NULL && sess_cache != found_sess_cache) { - isc_tlsctx_client_session_cache_detach(&sess_cache); - } return (NULL); } From b3490213eb518be574e5ac0ace710811c622dbd2 Mon Sep 17 00:00:00 2001 From: Artem Boldariev Date: Tue, 24 May 2022 11:39:26 +0300 Subject: [PATCH 12/12] CID 352848: split xfrin_start() and remove dead code This commit separates TLS context creation code from xfrin_start() as it has become too large and hard to follow into a new function (similarly how it is done in dighost.c) The dead code has been removed from the cleanup section of the TLS creation code: * there is no way 'tlsctx' can equal 'found'; * there is no way 'sess_cache' can be non-NULL in the cleanup section. Also, it fixes a bug in the older version of the code, where TLS client session context fetched from the cache would not get passed to isc_nm_tlsdnsconnect(). (cherry picked from commit 98f758ed4fdc1ed21516bf3c053ce9fde6c5ce35) --- lib/dns/xfrin.c | 395 +++++++++++++++++++++----------------- lib/isc/include/isc/tls.h | 4 +- 2 files changed, 218 insertions(+), 181 deletions(-) diff --git a/lib/dns/xfrin.c b/lib/dns/xfrin.c index f14324618a..1b2b79a1af 100644 --- a/lib/dns/xfrin.c +++ b/lib/dns/xfrin.c @@ -928,14 +928,222 @@ xfrin_create(isc_mem_t *mctx, dns_zone_t *zone, dns_db_t *db, isc_nm_t *netmgr, } static isc_result_t -xfrin_start(dns_xfrin_ctx_t *xfr) { - isc_result_t result; - dns_xfrin_ctx_t *connect_xfr = NULL; - dns_transport_type_t transport_type = DNS_TRANSPORT_TCP; +get_create_tlsctx(const dns_xfrin_ctx_t *xfr, isc_tlsctx_t **pctx, + isc_tlsctx_client_session_cache_t **psess_cache) { + isc_result_t result = ISC_R_FAILURE; isc_tlsctx_t *tlsctx = NULL, *found = NULL; isc_tls_cert_store_t *store = NULL, *found_store = NULL; isc_tlsctx_client_session_cache_t *sess_cache = NULL, *found_sess_cache = NULL; + uint32_t tls_versions; + const char *ciphers = NULL; + bool prefer_server_ciphers; + const uint16_t family = isc_sockaddr_pf(&xfr->primaryaddr) == PF_INET6 + ? AF_INET6 + : AF_INET; + const char *tlsname = NULL; + + REQUIRE(psess_cache != NULL && *psess_cache == NULL); + REQUIRE(pctx != NULL && *pctx == NULL); + + INSIST(xfr->transport != NULL); + tlsname = dns_transport_get_tlsname(xfr->transport); + INSIST(tlsname != NULL && *tlsname != '\0'); + + /* + * Let's try to re-use the already created context. This way + * we have a chance to resume the TLS session, bypassing the + * full TLS handshake procedure, making establishing + * subsequent TLS connections for XoT faster. + */ + result = isc_tlsctx_cache_find(xfr->tlsctx_cache, tlsname, + isc_tlsctx_cache_tls, family, &found, + &found_store, &found_sess_cache); + if (result != ISC_R_SUCCESS) { + const char *hostname = + dns_transport_get_remote_hostname(xfr->transport); + const char *ca_file = dns_transport_get_cafile(xfr->transport); + const char *cert_file = + dns_transport_get_certfile(xfr->transport); + const char *key_file = + dns_transport_get_keyfile(xfr->transport); + char primary_addr_str[INET6_ADDRSTRLEN] = { 0 }; + isc_netaddr_t primary_netaddr = { 0 }; + bool hostname_ignore_subject; + /* + * So, no context exists. Let's create one using the + * parameters from the configuration file and try to + * store it for further reuse. + */ + result = isc_tlsctx_createclient(&tlsctx); + if (result != ISC_R_SUCCESS) { + goto failure; + } + tls_versions = dns_transport_get_tls_versions(xfr->transport); + if (tls_versions != 0) { + isc_tlsctx_set_protocols(tlsctx, tls_versions); + } + ciphers = dns_transport_get_ciphers(xfr->transport); + if (ciphers != NULL) { + isc_tlsctx_set_cipherlist(tlsctx, ciphers); + } + + if (dns_transport_get_prefer_server_ciphers( + xfr->transport, &prefer_server_ciphers)) + { + isc_tlsctx_prefer_server_ciphers(tlsctx, + prefer_server_ciphers); + } + + if (hostname != NULL || ca_file != NULL) { + /* + * The situation when 'found_store != NULL' while 'found + * == NULL' might appear as there is one to many + * relation between per transport TLS contexts and cert + * stores. That is, there could be one store shared + * between multiple contexts. + */ + if (found_store == NULL) { + /* + * 'ca_file' can equal 'NULL' here, in + * that case the store with system-wide + * CA certificates will be created, just + * as planned. + */ + result = isc_tls_cert_store_create(ca_file, + &store); + + if (result != ISC_R_SUCCESS) { + goto failure; + } + } else { + store = found_store; + } + + INSIST(store != NULL); + if (hostname == NULL) { + /* + * If CA bundle file is specified, but + * hostname is not, then use the primary + * IP address for validation, just like + * dig does. + */ + INSIST(ca_file != NULL); + isc_netaddr_fromsockaddr(&primary_netaddr, + &xfr->primaryaddr); + isc_netaddr_format(&primary_netaddr, + primary_addr_str, + sizeof(primary_addr_str)); + hostname = primary_addr_str; + } + /* + * According to RFC 8310, Subject field MUST NOT + * be inspected when verifying hostname for DoT. + * Only SubjectAltName must be checked. + */ + hostname_ignore_subject = true; + result = isc_tlsctx_enable_peer_verification( + tlsctx, false, store, hostname, + hostname_ignore_subject); + if (result != ISC_R_SUCCESS) { + goto failure; + } + + /* + * Let's load client certificate and enable + * Mutual TLS. We do that only in the case when + * Strict TLS is enabled, because Mutual TLS is + * an extension of it. + */ + if (cert_file != NULL) { + INSIST(key_file != NULL); + + result = isc_tlsctx_load_certificate( + tlsctx, key_file, cert_file); + if (result != ISC_R_SUCCESS) { + goto failure; + } + } + } + + isc_tlsctx_enable_dot_client_alpn(tlsctx); + + sess_cache = isc_tlsctx_client_session_cache_new( + xfr->mctx, tlsctx, + ISC_TLSCTX_CLIENT_SESSION_CACHE_DEFAULT_SIZE); + + found_store = NULL; + result = isc_tlsctx_cache_add(xfr->tlsctx_cache, tlsname, + isc_tlsctx_cache_tls, family, + tlsctx, store, sess_cache, &found, + &found_store, &found_sess_cache); + if (result == ISC_R_EXISTS) { + /* + * It seems the entry has just been created from within + * another thread while we were initialising + * ours. Although this is unlikely, it could happen + * after startup/re-initialisation. In such a case, + * discard the new context and associated data and use + * the already established one from now on. + * + * Such situation will not occur after the + * initial 'warm-up', so it is not critical + * performance-wise. + */ + INSIST(found != NULL); + isc_tlsctx_free(&tlsctx); + isc_tls_cert_store_free(&store); + isc_tlsctx_client_session_cache_detach(&sess_cache); + /* Let's return the data from the cache. */ + *psess_cache = found_sess_cache; + *pctx = found; + } else { + /* + * Adding the fresh values into the cache has been + * successful, let's return them + */ + INSIST(result == ISC_R_SUCCESS); + *psess_cache = sess_cache; + *pctx = tlsctx; + } + } else { + /* + * The cache lookup has been successful, let's return the + * results. + */ + INSIST(result == ISC_R_SUCCESS); + *psess_cache = found_sess_cache; + *pctx = found; + } + + return (ISC_R_SUCCESS); + +failure: + if (tlsctx != NULL) { + isc_tlsctx_free(&tlsctx); + } + + /* + * The 'found_store' is being managed by the TLS context + * cache. Thus, we should keep it as it is, as it will get + * destroyed alongside the cache. As there is one store per + * multiple TLS contexts, we need to handle store deletion in a + * special way. + */ + if (store != NULL && store != found_store) { + isc_tls_cert_store_free(&store); + } + + return (result); +} + +static isc_result_t +xfrin_start(dns_xfrin_ctx_t *xfr) { + isc_result_t result; + dns_xfrin_ctx_t *connect_xfr = NULL; + dns_transport_type_t transport_type = DNS_TRANSPORT_TCP; + isc_tlsctx_t *tlsctx = NULL; + isc_tlsctx_client_session_cache_t *sess_cache = NULL; (void)isc_refcount_increment0(&xfr->connects); dns_xfrin_attach(xfr, &connect_xfr); @@ -955,167 +1163,11 @@ xfrin_start(dns_xfrin_ctx_t *xfr) { connect_xfr, 30000, 0); break; case DNS_TRANSPORT_TLS: { - uint32_t tls_versions; - const char *ciphers = NULL; - bool prefer_server_ciphers; - const uint16_t family = isc_sockaddr_pf(&xfr->primaryaddr) == - PF_INET6 - ? AF_INET6 - : AF_INET; - const char *tlsname = NULL; - - INSIST(xfr->transport != NULL); - tlsname = dns_transport_get_tlsname(xfr->transport); - INSIST(tlsname != NULL && *tlsname != '\0'); - - /* - * Let's try to re-use the already created context. This way - * we have a chance to resume the TLS session, bypassing the - * full TLS handshake procedure, making establishing - * subsequent TLS connections for XoT faster. - */ - result = isc_tlsctx_cache_find( - xfr->tlsctx_cache, tlsname, isc_tlsctx_cache_tls, - family, &tlsctx, &found_store, &found_sess_cache); + result = get_create_tlsctx(xfr, &tlsctx, &sess_cache); if (result != ISC_R_SUCCESS) { - const char *hostname = - dns_transport_get_remote_hostname( - xfr->transport); - const char *ca_file = - dns_transport_get_cafile(xfr->transport); - const char *cert_file = - dns_transport_get_certfile(xfr->transport); - const char *key_file = - dns_transport_get_keyfile(xfr->transport); - char primary_addr_str[INET6_ADDRSTRLEN] = { 0 }; - isc_netaddr_t primary_netaddr = { 0 }; - bool hostname_ignore_subject; - /* - * So, no context exists. Let's create one using the - * parameters from the configuration file and try to - * store it for further reuse. - */ - CHECK(isc_tlsctx_createclient(&tlsctx)); - tls_versions = - dns_transport_get_tls_versions(xfr->transport); - if (tls_versions != 0) { - isc_tlsctx_set_protocols(tlsctx, tls_versions); - } - ciphers = dns_transport_get_ciphers(xfr->transport); - if (ciphers != NULL) { - isc_tlsctx_set_cipherlist(tlsctx, ciphers); - } - - if (dns_transport_get_prefer_server_ciphers( - xfr->transport, &prefer_server_ciphers)) - { - isc_tlsctx_prefer_server_ciphers( - tlsctx, prefer_server_ciphers); - } - - if (hostname != NULL || ca_file != NULL) { - if (found_store == NULL) { - /* - * 'ca_file' can equal 'NULL' here, in - * that case the store with system-wide - * CA certificates will be created, just - * as planned. - */ - result = isc_tls_cert_store_create( - ca_file, &store); - - if (result != ISC_R_SUCCESS) { - goto failure; - } - } else { - store = found_store; - } - - INSIST(store != NULL); - if (hostname == NULL) { - /* - * If CA bundle file is specified, but - * hostname is not, then use the primary - * IP address for validation, just like - * dig does. - */ - INSIST(ca_file != NULL); - isc_netaddr_fromsockaddr( - &primary_netaddr, - &xfr->primaryaddr); - isc_netaddr_format( - &primary_netaddr, - primary_addr_str, - sizeof(primary_addr_str)); - hostname = primary_addr_str; - } - /* - * According to RFC 8310, Subject field MUST NOT - * be inspected when verifying hostname for DoT. - * Only SubjectAltName must be checked. - */ - hostname_ignore_subject = true; - result = isc_tlsctx_enable_peer_verification( - tlsctx, false, store, hostname, - hostname_ignore_subject); - if (result != ISC_R_SUCCESS) { - goto failure; - } - - /* - * Let's load client certificate and enable - * Mutual TLS. We do that only in the case when - * Strict TLS is enabled, because Mutual TLS is - * an extension of it. - */ - if (cert_file != NULL) { - INSIST(key_file != NULL); - - result = isc_tlsctx_load_certificate( - tlsctx, key_file, cert_file); - if (result != ISC_R_SUCCESS) { - goto failure; - } - } - } - - isc_tlsctx_enable_dot_client_alpn(tlsctx); - - sess_cache = isc_tlsctx_client_session_cache_new( - xfr->mctx, tlsctx, - ISC_TLSCTX_CLIENT_SESSION_CACHE_DEFAULT_SIZE); - - found_store = NULL; - result = isc_tlsctx_cache_add( - xfr->tlsctx_cache, tlsname, - isc_tlsctx_cache_tls, family, tlsctx, store, - sess_cache, &found, &found_store, - &found_sess_cache); - if (result == ISC_R_EXISTS) { - /* - * It seems the entry has just been created - * from within another thread while we were - * initialising ours. Although this is - * unlikely, it could happen after - * startup/re-initialisation. In such a case, - * discard the new context and use the already - * established one from now on. - * - * Such situation will not occur after the - * initial 'warm-up', so it is not critical - * performance-wise. - */ - INSIST(found != NULL); - isc_tlsctx_free(&tlsctx); - isc_tls_cert_store_free(&store); - isc_tlsctx_client_session_cache_detach( - &sess_cache); - tlsctx = found; - sess_cache = found_sess_cache; - } else { - INSIST(result == ISC_R_SUCCESS); - } + goto failure; } + INSIST(tlsctx != NULL); isc_nm_tlsdnsconnect(xfr->netmgr, &xfr->sourceaddr, &xfr->primaryaddr, xfrin_connect_done, connect_xfr, 30000, 0, tlsctx, sess_cache); @@ -1127,23 +1179,6 @@ xfrin_start(dns_xfrin_ctx_t *xfr) { return (ISC_R_SUCCESS); failure: - /* - * The 'found' context is being managed by the TLS context cache. - * Thus, we should keep it as it is, as it will get destroyed - * alongside the cache. - */ - if (tlsctx != NULL && found != tlsctx) { - isc_tlsctx_free(&tlsctx); - } - - if (store != NULL && store != found_store) { - isc_tls_cert_store_free(&store); - } - - if (sess_cache != NULL && sess_cache != found_sess_cache) { - isc_tlsctx_client_session_cache_detach(&sess_cache); - } - isc_refcount_decrement0(&xfr->connects); dns_xfrin_detach(&connect_xfr); return (result); diff --git a/lib/isc/include/isc/tls.h b/lib/isc/include/isc/tls.h index 86265ddb7c..e960e04913 100644 --- a/lib/isc/include/isc/tls.h +++ b/lib/isc/include/isc/tls.h @@ -558,5 +558,7 @@ isc_tlsctx_cache_find( * * Returns: *\li #ISC_R_SUCCESS - the context has been found; - *\li #ISC_R_NOTFOUND - the context has not been found. + *\li #ISC_R_NOTFOUND - the context has not been found. In such a case, + * 'pstore' still might get initialised as there is one to many + * relation between stores and contexts. */