Merge branch '3375-cids-352848-352849-handle-deadcode-flow-errors' into 'main'

Resolve: CID 352848, CID 352849: Control flow issues (DEADCODE)

Closes #3375

See merge request isc-projects/bind9!6351
This commit is contained in:
Artem Boldariev 2022-05-25 10:20:05 +00:00
commit b6f1c242a4
3 changed files with 226 additions and 185 deletions

View file

@ -2871,15 +2871,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);
}

View file

@ -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);
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, 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);

View file

@ -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.
*/