From e4483bbbd1fa632362c3ba08cc79783dac2d5e30 Mon Sep 17 00:00:00 2001 From: Yorgos Thessalonikefs Date: Mon, 20 Jan 2025 15:43:44 +0100 Subject: [PATCH] Unique DoT and DoH SSL contexts to allow for different ALPN (#1222) --- daemon/daemon.c | 7 ++- daemon/daemon.h | 10 +++- daemon/unbound.c | 94 +++++++++++++++++------------ daemon/worker.c | 7 ++- dnstap/unbound-dnstap-socket.c | 3 +- services/listen_dnsport.c | 9 ++- services/listen_dnsport.h | 6 +- testcode/fake_event.c | 3 +- util/net_help.c | 104 ++++++++++++++++++++++++++------- util/net_help.h | 15 +++-- 10 files changed, 181 insertions(+), 77 deletions(-) diff --git a/daemon/daemon.c b/daemon/daemon.c index 9e1d5d4f7..76607fe52 100644 --- a/daemon/daemon.c +++ b/daemon/daemon.c @@ -954,11 +954,12 @@ daemon_delete(struct daemon* daemon) free(daemon->env); #ifdef HAVE_SSL listen_sslctx_delete_ticket_keys(); - SSL_CTX_free((SSL_CTX*)daemon->listen_sslctx); - SSL_CTX_free((SSL_CTX*)daemon->connect_sslctx); + SSL_CTX_free((SSL_CTX*)daemon->listen_dot_sslctx); + SSL_CTX_free((SSL_CTX*)daemon->listen_doh_sslctx); + SSL_CTX_free((SSL_CTX*)daemon->connect_dot_sslctx); #endif #ifdef HAVE_NGTCP2 - SSL_CTX_free((SSL_CTX*)daemon->quic_sslctx); + SSL_CTX_free((SSL_CTX*)daemon->listen_quic_sslctx); #endif free(daemon); /* lex cleanup */ diff --git a/daemon/daemon.h b/daemon/daemon.h index f93887e3d..54ab97b2d 100644 --- a/daemon/daemon.h +++ b/daemon/daemon.h @@ -97,10 +97,14 @@ struct daemon { struct listen_port* rc_ports; /** remote control connections management (for first worker) */ struct daemon_remote* rc; - /** ssl context for listening to dnstcp over ssl, and connecting ssl */ - void* listen_sslctx, *connect_sslctx; + /** ssl context for listening to dnstcp over ssl */ + void* listen_dot_sslctx; + /** ssl context for connecting to dnstcp over ssl */ + void* connect_dot_sslctx; + /** ssl context for listening to DoH */ + void* listen_doh_sslctx; /** ssl context for listening to quic */ - void* quic_sslctx; + void* listen_quic_sslctx; /** num threads allocated */ int num; /** num threads allocated in the previous config or 0 at first */ diff --git a/daemon/unbound.c b/daemon/unbound.c index f394fbde3..feea43180 100644 --- a/daemon/unbound.c +++ b/daemon/unbound.c @@ -463,6 +463,62 @@ detach(void) #endif /* HAVE_DAEMON */ } +/* setup a listening ssl context, fatal_exit() on any failure */ +static void +setup_listen_sslctx(void** ctx, int is_dot, int is_doh, struct config_file* cfg) +{ +#ifdef HAVE_SSL + if(!(*ctx = listen_sslctx_create( + cfg->ssl_service_key, cfg->ssl_service_pem, NULL, + cfg->tls_ciphers, cfg->tls_ciphersuites, + (cfg->tls_session_ticket_keys.first && + cfg->tls_session_ticket_keys.first->str[0] != 0), + is_dot, is_doh))) { + fatal_exit("could not set up listen SSL_CTX"); + } +#else /* HAVE_SSL */ + (void)ctx;(void)is_dot;(void)is_doh;(void)cfg; +#endif /* HAVE_SSL */ +} + +/* setups the needed ssl contexts, fatal_exit() on any failure */ +static void +setup_sslctxs(struct daemon* daemon, struct config_file* cfg) +{ +#ifdef HAVE_SSL + if(!(daemon->rc = daemon_remote_create(cfg))) + fatal_exit("could not set up remote-control"); + if(cfg->ssl_service_key && cfg->ssl_service_key[0]) { + /* setup the session keys; the callback to use them will be + * attached to each sslctx separately */ + if(cfg->tls_session_ticket_keys.first && + cfg->tls_session_ticket_keys.first->str[0] != 0) { + if(!listen_sslctx_setup_ticket_keys( + cfg->tls_session_ticket_keys.first)) { + fatal_exit("could not set session ticket SSL_CTX"); + } + } + (void)setup_listen_sslctx(&daemon->listen_dot_sslctx, 1, 0, cfg); +#ifdef HAVE_NGHTTP2_NGHTTP2_H + if(cfg_has_https(cfg)) { + (void)setup_listen_sslctx(&daemon->listen_doh_sslctx, 0, 1, cfg); + } +#endif +#ifdef HAVE_NGTCP2 + if(!(daemon->listen_quic_sslctx = quic_sslctx_create( + cfg->ssl_service_key, cfg->ssl_service_pem, NULL))) { + fatal_exit("could not set up quic SSL_CTX"); + } +#endif /* HAVE_NGTCP2 */ + } + if(!(daemon->connect_dot_sslctx = connect_sslctx_create(NULL, NULL, + cfg->tls_cert_bundle, cfg->tls_win_cert))) + fatal_exit("could not set up connect SSL_CTX"); +#else /* HAVE_SSL */ + (void)daemon;(void)cfg; +#endif /* HAVE_SSL */ +} + /** daemonize, drop user privileges and chroot if needed */ static void perform_setup(struct daemon* daemon, struct config_file* cfg, int debug_mode, @@ -489,43 +545,7 @@ perform_setup(struct daemon* daemon, struct config_file* cfg, int debug_mode, #endif /* read ssl keys while superuser and outside chroot */ -#ifdef HAVE_SSL - if(!(daemon->rc = daemon_remote_create(cfg))) - fatal_exit("could not set up remote-control"); - if(cfg->ssl_service_key && cfg->ssl_service_key[0]) { - if(!(daemon->listen_sslctx = listen_sslctx_create( - cfg->ssl_service_key, cfg->ssl_service_pem, NULL))) { - fatal_exit("could not set up listen SSL_CTX"); - } - if(cfg->tls_ciphers && cfg->tls_ciphers[0]) { - if (!SSL_CTX_set_cipher_list(daemon->listen_sslctx, cfg->tls_ciphers)) { - fatal_exit("failed to set tls-cipher %s", cfg->tls_ciphers); - } - } -#ifdef HAVE_SSL_CTX_SET_CIPHERSUITES - if(cfg->tls_ciphersuites && cfg->tls_ciphersuites[0]) { - if (!SSL_CTX_set_ciphersuites(daemon->listen_sslctx, cfg->tls_ciphersuites)) { - fatal_exit("failed to set tls-ciphersuites %s", cfg->tls_ciphersuites); - } - } -#endif /* HAVE_SSL_CTX_SET_CIPHERSUITES */ - if(cfg->tls_session_ticket_keys.first && - cfg->tls_session_ticket_keys.first->str[0] != 0) { - if(!listen_sslctx_setup_ticket_keys(daemon->listen_sslctx, cfg->tls_session_ticket_keys.first)) { - fatal_exit("could not set session ticket SSL_CTX"); - } - } -#ifdef HAVE_NGTCP2 - if(!(daemon->quic_sslctx = quic_sslctx_create( - cfg->ssl_service_key, cfg->ssl_service_pem, NULL))) { - fatal_exit("could not set up quic SSL_CTX"); - } -#endif /* HAVE_NGTCP2 */ - } - if(!(daemon->connect_sslctx = connect_sslctx_create(NULL, NULL, - cfg->tls_cert_bundle, cfg->tls_win_cert))) - fatal_exit("could not set up connect SSL_CTX"); -#endif /* HAVE_SSL */ + (void)setup_sslctxs(daemon, cfg); /* init syslog (as root) if needed, before daemonize, otherwise * a fork error could not be printed since daemonize closed stderr.*/ diff --git a/daemon/worker.c b/daemon/worker.c index 3985d1108..ba5118a74 100644 --- a/daemon/worker.c +++ b/daemon/worker.c @@ -2173,8 +2173,9 @@ worker_init(struct worker* worker, struct config_file *cfg, : cfg->tcp_idle_timeout, cfg->harden_large_queries, cfg->http_max_streams, cfg->http_endpoint, cfg->http_notls_downstream, - worker->daemon->tcl, worker->daemon->listen_sslctx, - worker->daemon->quic_sslctx, + worker->daemon->tcl, worker->daemon->listen_dot_sslctx, + worker->daemon->listen_doh_sslctx, + worker->daemon->listen_quic_sslctx, dtenv, worker->daemon->doq_table, worker->env.rnd, cfg, worker_handle_request, worker); if(!worker->front) { @@ -2191,7 +2192,7 @@ worker_init(struct worker* worker, struct config_file *cfg, cfg->unwanted_threshold, cfg->outgoing_tcp_mss, &worker_alloc_cleanup, worker, cfg->do_udp || cfg->udp_upstream_without_downstream, - worker->daemon->connect_sslctx, cfg->delay_close, + worker->daemon->connect_dot_sslctx, cfg->delay_close, cfg->tls_use_sni, dtenv, cfg->udp_connect, cfg->max_reuse_tcp_queries, cfg->tcp_reuse_timeout, cfg->tcp_auth_query_timeout); diff --git a/dnstap/unbound-dnstap-socket.c b/dnstap/unbound-dnstap-socket.c index 7f8be4965..cfa0c8f95 100644 --- a/dnstap/unbound-dnstap-socket.c +++ b/dnstap/unbound-dnstap-socket.c @@ -346,7 +346,8 @@ static struct tap_socket* tap_socket_new_tlsaccept(char* ip, s->fd = -1; s->ev_cb = ev_cb; s->data = data; - s->sslctx = listen_sslctx_create(server_key, server_cert, verifypem); + s->sslctx = listen_sslctx_create(server_key, server_cert, verifypem, + NULL, NULL, 0, 0, 0); if(!s->sslctx) { log_err("could not create ssl context"); free(s->ip); diff --git a/services/listen_dnsport.c b/services/listen_dnsport.c index 8a5cf2a18..2c4b28abf 100644 --- a/services/listen_dnsport.c +++ b/services/listen_dnsport.c @@ -1512,7 +1512,8 @@ listen_create(struct comm_base* base, struct listen_port* ports, size_t bufsize, int tcp_accept_count, int tcp_idle_timeout, int harden_large_queries, uint32_t http_max_streams, char* http_endpoint, int http_notls, struct tcl_list* tcp_conn_limit, - void* sslctx, void* quic_sslctx, struct dt_env* dtenv, + void* dot_sslctx, void* doh_sslctx, void* quic_sslctx, + struct dt_env* dtenv, struct doq_table* doq_table, struct ub_randstate* rnd,struct config_file* cfg, comm_point_callback_type* cb, void *cb_arg) @@ -1566,7 +1567,7 @@ listen_create(struct comm_base* base, struct listen_port* ports, ports->ftype, ports->pp2_enabled, cb, cb_arg, ports->socket); if(ports->ftype == listen_type_http) { - if(!sslctx && !http_notls) { + if(!doh_sslctx && !http_notls) { log_warn("HTTPS port configured, but " "no TLS tls-service-key or " "tls-service-pem set"); @@ -1612,8 +1613,10 @@ listen_create(struct comm_base* base, struct listen_port* ports, cp->ssl = NULL; } else if(ports->ftype == listen_type_doq) { cp->ssl = quic_sslctx; + } else if(ports->ftype == listen_type_http) { + cp->ssl = doh_sslctx; } else { - cp->ssl = sslctx; + cp->ssl = dot_sslctx; } cp->dtenv = dtenv; cp->do_not_close = 1; diff --git a/services/listen_dnsport.h b/services/listen_dnsport.h index 629f20bd4..f6275f805 100644 --- a/services/listen_dnsport.h +++ b/services/listen_dnsport.h @@ -194,7 +194,8 @@ int resolve_interface_names(char** ifs, int num_ifs, * @param http_endpoint: HTTP endpoint to service queries on * @param http_notls: no TLS for http downstream * @param tcp_conn_limit: TCP connection limit info. - * @param sslctx: nonNULL if ssl context. + * @param dot_sslctx: nonNULL if dot ssl context. + * @param doh_sslctx: nonNULL if doh ssl context. * @param quic_sslctx: nonNULL if quic ssl context. * @param dtenv: nonNULL if dnstap enabled. * @param doq_table: the doq connection table, with shared information. @@ -210,7 +211,8 @@ listen_create(struct comm_base* base, struct listen_port* ports, size_t bufsize, int tcp_accept_count, int tcp_idle_timeout, int harden_large_queries, uint32_t http_max_streams, char* http_endpoint, int http_notls, struct tcl_list* tcp_conn_limit, - void* sslctx, void* quic_sslctx, struct dt_env* dtenv, + void* dot_sslctx, void* doh_sslctx, void* quic_sslctx, + struct dt_env* dtenv, struct doq_table* doq_table, struct ub_randstate* rnd,struct config_file* cfg, comm_point_callback_type* cb, void *cb_arg); diff --git a/testcode/fake_event.c b/testcode/fake_event.c index 840a687d9..48843f1db 100644 --- a/testcode/fake_event.c +++ b/testcode/fake_event.c @@ -938,7 +938,8 @@ listen_create(struct comm_base* base, struct listen_port* ATTR_UNUSED(ports), char* ATTR_UNUSED(http_endpoint), int ATTR_UNUSED(http_notls), struct tcl_list* ATTR_UNUSED(tcp_conn_limit), - void* ATTR_UNUSED(sslctx), void* ATTR_UNUSED(quic_ssl), + void* ATTR_UNUSED(dot_sslctx), void* ATTR_UNUSED(doh_sslctx), + void* ATTR_UNUSED(quic_ssl), struct dt_env* ATTR_UNUSED(dtenv), struct doq_table* ATTR_UNUSED(table), struct ub_randstate* ATTR_UNUSED(rnd), diff --git a/util/net_help.c b/util/net_help.c index fbae91d9c..81499f228 100644 --- a/util/net_help.c +++ b/util/net_help.c @@ -1162,8 +1162,29 @@ log_cert(unsigned level, const char* str, void* cert) } #endif /* HAVE_SSL */ +#if defined(HAVE_SSL) && defined(HAVE_SSL_CTX_SET_ALPN_SELECT_CB) +static int +dot_alpn_select_cb(SSL* ATTR_UNUSED(ssl), const unsigned char** out, + unsigned char* outlen, const unsigned char* in, unsigned int inlen, + void* ATTR_UNUSED(arg)) +{ + static const unsigned char alpns[] = { 3, 'd', 'o', 't' }; + unsigned char* tmp_out; + int ret; + ret = SSL_select_next_proto(&tmp_out, outlen, alpns, sizeof(alpns), in, inlen); + if(ret == OPENSSL_NPN_NO_OVERLAP) { + /* Client sent ALPN but no overlap. Should have been error, + * but for privacy we continue without ALPN (e.g., if certain + * ALPNs are blocked) */ + return SSL_TLSEXT_ERR_NOACK; + } + *out = tmp_out; + return SSL_TLSEXT_ERR_OK; +} +#endif + #if defined(HAVE_SSL) && defined(HAVE_NGHTTP2) && defined(HAVE_SSL_CTX_SET_ALPN_SELECT_CB) -static int alpn_select_cb(SSL* ATTR_UNUSED(ssl), const unsigned char** out, +static int doh_alpn_select_cb(SSL* ATTR_UNUSED(ssl), const unsigned char** out, unsigned char* outlen, const unsigned char* in, unsigned int inlen, void* ATTR_UNUSED(arg)) { @@ -1177,6 +1198,23 @@ static int alpn_select_cb(SSL* ATTR_UNUSED(ssl), const unsigned char** out, } #endif +/* setup the callback for ticket keys */ +static int +setup_ticket_keys_cb(void* sslctx) +{ +# ifdef HAVE_SSL_CTX_SET_TLSEXT_TICKET_KEY_EVP_CB + if(SSL_CTX_set_tlsext_ticket_key_evp_cb(sslctx, tls_session_ticket_key_cb) == 0) { + return 0; + } +# else + if(SSL_CTX_set_tlsext_ticket_key_cb(sslctx, tls_session_ticket_key_cb) == 0) { + return 0; + } +# endif + return 1; +} + + int listen_sslctx_setup(void* ctxt) { @@ -1248,9 +1286,6 @@ listen_sslctx_setup(void* ctxt) #ifdef HAVE_SSL_CTX_SET_SECURITY_LEVEL SSL_CTX_set_security_level(ctx, 0); #endif -#if defined(HAVE_SSL_CTX_SET_ALPN_SELECT_CB) && defined(HAVE_NGHTTP2) - SSL_CTX_set_alpn_select_cb(ctx, alpn_select_cb, NULL); -#endif #else (void)ctxt; #endif /* HAVE_SSL */ @@ -1285,7 +1320,10 @@ listen_sslctx_setup_2(void* ctxt) #endif /* HAVE_SSL */ } -void* listen_sslctx_create(char* key, char* pem, char* verifypem) +void* listen_sslctx_create(const char* key, const char* pem, + const char* verifypem, const char* tls_ciphers, + const char* tls_ciphersuites, int set_ticket_keys_cb, + int is_dot, int is_doh) { #ifdef HAVE_SSL SSL_CTX* ctx = SSL_CTX_new(SSLv23_server_method()); @@ -1336,11 +1374,50 @@ void* listen_sslctx_create(char* key, char* pem, char* verifypem) verifypem)); SSL_CTX_set_verify(ctx, SSL_VERIFY_PEER|SSL_VERIFY_FAIL_IF_NO_PEER_CERT, NULL); } + if(tls_ciphers && tls_ciphers[0]) { + if (!SSL_CTX_set_cipher_list(ctx, tls_ciphers)) { + log_err("failed to set tls-cipher %s", + tls_ciphers); + log_crypto_err("Error in SSL_CTX_set_cipher_list"); + SSL_CTX_free(ctx); + return NULL; + } + } +#ifdef HAVE_SSL_CTX_SET_CIPHERSUITES + if(tls_ciphersuites && tls_ciphersuites[0]) { + if (!SSL_CTX_set_ciphersuites(ctx, tls_ciphersuites)) { + log_err("failed to set tls-ciphersuites %s", + tls_ciphersuites); + log_crypto_err("Error in SSL_CTX_set_ciphersuites"); + SSL_CTX_free(ctx); + return NULL; + } + } +#endif /* HAVE_SSL_CTX_SET_CIPHERSUITES */ + if(set_ticket_keys_cb) { + if(!setup_ticket_keys_cb(ctx)) { + log_crypto_err("no support for TLS session ticket"); + SSL_CTX_free(ctx); + return NULL; + } + } + /* setup ALPN */ +#if defined(HAVE_SSL_CTX_SET_ALPN_SELECT_CB) + if(is_dot) { + SSL_CTX_set_alpn_select_cb(ctx, dot_alpn_select_cb, NULL); + } else if(is_doh) { +#if defined(HAVE_NGHTTP2) + SSL_CTX_set_alpn_select_cb(ctx, doh_alpn_select_cb, NULL); +#endif + } +#endif /* HAVE_SSL_CTX_SET_ALPN_SELECT_CB */ return ctx; #else (void)key; (void)pem; (void)verifypem; + (void)tls_ciphers; (void)tls_ciphersuites; + (void)tls_session_ticket_keys; return NULL; -#endif +#endif /* HAVE_SSL */ } #ifdef USE_WINSOCK @@ -1700,7 +1777,7 @@ void ub_openssl_lock_delete(void) #endif /* OPENSSL_THREADS */ } -int listen_sslctx_setup_ticket_keys(void* sslctx, struct config_strlist* tls_session_ticket_keys) { +int listen_sslctx_setup_ticket_keys(struct config_strlist* tls_session_ticket_keys) { #ifdef HAVE_SSL size_t s = 1; struct config_strlist* p; @@ -1746,24 +1823,11 @@ int listen_sslctx_setup_ticket_keys(void* sslctx, struct config_strlist* tls_ses } /* terminate array with NULL key name entry */ keys->key_name = NULL; -# ifdef HAVE_SSL_CTX_SET_TLSEXT_TICKET_KEY_EVP_CB - if(SSL_CTX_set_tlsext_ticket_key_evp_cb(sslctx, tls_session_ticket_key_cb) == 0) { - log_err("no support for TLS session ticket"); - return 0; - } -# else - if(SSL_CTX_set_tlsext_ticket_key_cb(sslctx, tls_session_ticket_key_cb) == 0) { - log_err("no support for TLS session ticket"); - return 0; - } -# endif return 1; #else - (void)sslctx; (void)tls_session_ticket_keys; return 0; #endif - } #ifdef HAVE_SSL diff --git a/util/net_help.h b/util/net_help.h index 4365c9a6a..278e370a2 100644 --- a/util/net_help.h +++ b/util/net_help.h @@ -493,9 +493,18 @@ void listen_sslctx_setup_2(void* ctxt); * @param key: private key file. * @param pem: public key cert. * @param verifypem: if nonNULL, verifylocation file. + * @param tls_ciphers: if non empty string, tls ciphers to use. + * @param tls_ciphersuites: if non empty string, tls ciphersuites to use. + * @param set_ticket_keys_cb: if the callback for configured ticket keys needs + * to be set. + * @param is_dot: if the TLS connection is for DoT to set the appropriate ALPN. + * @param is_doh: if the TLS connection is for DoH to set the appropriate ALPN. * return SSL_CTX* or NULL on failure (logged). */ -void* listen_sslctx_create(char* key, char* pem, char* verifypem); +void* listen_sslctx_create(const char* key, const char* pem, + const char* verifypem, const char* tls_ciphers, + const char* tls_ciphersuites, int set_ticket_keys_cb, + int is_dot, int is_doh); /** * create SSL connect context @@ -553,12 +562,10 @@ void ub_openssl_lock_delete(void); /** * setup TLS session ticket - * @param sslctx: the SSL_CTX to use (from connect_sslctx_create()) * @param tls_session_ticket_keys: TLS ticket secret filenames * @return false on failure (alloc failure). */ -int listen_sslctx_setup_ticket_keys(void* sslctx, - struct config_strlist* tls_session_ticket_keys); +int listen_sslctx_setup_ticket_keys(struct config_strlist* tls_session_ticket_keys); /** Free memory used for TLS session ticket keys */ void listen_sslctx_delete_ticket_keys(void);