From ede152ecfb9d112df6d3718e2b805e62aac8c93f Mon Sep 17 00:00:00 2001 From: Remi Gacogne Date: Tue, 16 Apr 2019 15:03:48 +0200 Subject: [PATCH] dnsdist: Implement DoH certificate reloading --- pdns/Makefile.am | 1 + pdns/dnsdistdist/Makefile.am | 1 + pdns/dnsdistdist/doh.cc | 166 ++++++++++++++++++++++--------- pdns/dnsdistdist/doh.hh | 69 +------------ pdns/dnsdistdist/libssl.cc | 94 +++++++++++++++++ pdns/dnsdistdist/libssl.hh | 4 + pdns/dnsdistdist/tcpiohandler.cc | 68 +------------ pdns/doh.hh | 68 +++++++++++++ 8 files changed, 292 insertions(+), 179 deletions(-) mode change 100644 => 120000 pdns/dnsdistdist/doh.hh create mode 100644 pdns/dnsdistdist/libssl.cc create mode 100644 pdns/dnsdistdist/libssl.hh create mode 100644 pdns/doh.hh diff --git a/pdns/Makefile.am b/pdns/Makefile.am index 6749e4cf6..6c845d9e7 100644 --- a/pdns/Makefile.am +++ b/pdns/Makefile.am @@ -1483,6 +1483,7 @@ fuzz_target_dnsdistcache_SOURCES = \ dnsname.cc dnsname.hh \ dnsparser.cc dnsparser.hh \ dnswriter.cc dnswriter.hh \ + doh.hh \ ednsoptions.cc ednsoptions.hh \ ednssubnet.cc ednssubnet.hh \ iputils.cc iputils.hh \ diff --git a/pdns/dnsdistdist/Makefile.am b/pdns/dnsdistdist/Makefile.am index f15cf5af4..35600a2ea 100644 --- a/pdns/dnsdistdist/Makefile.am +++ b/pdns/dnsdistdist/Makefile.am @@ -143,6 +143,7 @@ dnsdist_SOURCES = \ gettime.cc gettime.hh \ htmlfiles.h \ iputils.cc iputils.hh \ + libssl.cc libssl.hh \ lock.hh \ misc.cc misc.hh \ mplexer.hh \ diff --git a/pdns/dnsdistdist/doh.cc b/pdns/dnsdistdist/doh.cc index 5456685ac..22e92bcea 100644 --- a/pdns/dnsdistdist/doh.cc +++ b/pdns/dnsdistdist/doh.cc @@ -1,21 +1,25 @@ #define H2O_USE_EPOLL 1 + #include #include -#include "h2o.h" -#include "h2o/http1.h" -#include "h2o/http2.h" +#include + +#include +#include +//#include +#include + #include "base64.hh" #include "dnsname.hh" #undef CERT #include "dnsdist.hh" #include "misc.hh" -#include #include "dns.hh" #include "dolog.hh" #include "dnsdist-ecs.hh" #include "dnsdist-rules.hh" #include "dnsdist-xpf.hh" -#include +#include "libssl.hh" using namespace std; @@ -41,14 +45,43 @@ using namespace std; They are not in HTTP1. So you MUST use the length field! */ +class DOHAcceptContext +{ +public: + DOHAcceptContext() + { + memset(&d_h2o_accept_ctx, 0, sizeof(d_h2o_accept_ctx)); + } + DOHAcceptContext(const DOHAcceptContext&) = delete; + DOHAcceptContext& operator=(const DOHAcceptContext&) = delete; + + h2o_accept_ctx_t* get() + { + ++d_refcnt; + return &d_h2o_accept_ctx; + } + + void release() + { + --d_refcnt; + if (d_refcnt == 0) { + SSL_CTX_free(d_h2o_accept_ctx.ssl_ctx); + d_h2o_accept_ctx.ssl_ctx = nullptr; + delete this; + } + } + +private: + h2o_accept_ctx_t d_h2o_accept_ctx; + std::atomic d_refcnt{1}; +}; + // we create one of these per thread, and pass around a pointer to it // through the bowels of h2o struct DOHServerConfig { - DOHServerConfig(uint32_t idleTimeout) + DOHServerConfig(uint32_t idleTimeout): accept_ctx(new DOHAcceptContext) { - memset(&h2o_accept_ctx, 0, sizeof(h2o_accept_ctx)); - if(socketpair(AF_LOCAL, SOCK_DGRAM, 0, dohquerypair) < 0) { unixDie("Creating a socket pair for DNS over HTTPS"); } @@ -62,16 +95,32 @@ struct DOHServerConfig h2o_config_init(&h2o_config); h2o_config.http2.idle_timeout = idleTimeout * 1000; } + DOHServerConfig(const DOHServerConfig&) = delete; + DOHServerConfig& operator=(const DOHServerConfig&) = delete; + + ~DOHServerConfig() + { + if (accept_ctx) { + accept_ctx->release(); + } + } + LocalHolders holders; h2o_globalconf_t h2o_config; h2o_context_t h2o_ctx; - h2o_accept_ctx_t h2o_accept_ctx; + DOHAcceptContext* accept_ctx{nullptr}; ClientState* cs{nullptr}; std::shared_ptr df{nullptr}; int dohquerypair[2]{-1,-1}; int dohresponsepair[2]{-1,-1}; }; +static void on_socketclose(void *data) +{ + DOHAcceptContext* ctx = reinterpret_cast(data); + ctx->release(); +} + /* this duplicates way too much from the UDP handler. Sorry. this function calls 'return -1' to drop a query without sending it caller should make sure HTTPS thread hears of that @@ -79,14 +128,16 @@ struct DOHServerConfig static int processDOHQuery(DOHUnit* du) { - LocalHolders holders; uint16_t queryId = 0; + ComboAddress remote; try { if(!du->req) { // we got closed meanwhile. XXX small race condition here return -1; } + remote = du->remote; DOHServerConfig* dsc = reinterpret_cast(du->req->conn->ctx->storage.entries[0].data); + auto& holders = dsc->holders; ClientState& cs = *dsc->cs; if (du->query.size() < sizeof(dnsheader)) { @@ -182,17 +233,18 @@ static int processDOHQuery(DOHUnit* du) dh->id = idOffset; int fd = pickBackendSocketForSending(ss); + /* you can't touch du after this line, because it might already have been freed */ ssize_t ret = udpClientSendRequestToBackend(ss, fd, query, dq.len); + vinfolog("Got query for %s|%s from %s (https), relayed to %s", ids->qname.toString(), QType(ids->qtype).getName(), remote.toStringWithPort(), ss->getName()); + if(ret < 0) { ++ss->sendErrors; ++g_stats.downstreamSendErrors; } - - vinfolog("Got query for %s|%s from %s (https), relayed to %s", ids->qname.toString(), QType(ids->qtype).getName(), du->remote.toStringWithPort(), ss->getName()); } catch(const std::exception& e) { - vinfolog("Got an error in DOH question thread while parsing a query from %s, id %d: %s", du->remote.toStringWithPort(), queryId, e.what()); + vinfolog("Got an error in DOH question thread while parsing a query from %s, id %d: %s", remote.toStringWithPort(), queryId, e.what()); return -1; } return 0; @@ -488,8 +540,9 @@ static void on_accept(h2o_socket_t *listener, const char *err) return; } // do some dnsdist rules here to filter based on IP address - if ((sock = h2o_evloop_socket_accept(listener)) == nullptr) + if ((sock = h2o_evloop_socket_accept(listener)) == nullptr) { return; + } ComboAddress remote; @@ -497,8 +550,11 @@ static void on_accept(h2o_socket_t *listener, const char *err) // cout<<"New HTTP accept for client "<data << endl; sock->data = dsc; + sock->on_close.cb = on_socketclose; + auto accept_ctx = dsc->accept_ctx->get(); + sock->on_close.data = dsc->accept_ctx; ++dsc->df->d_httpconnects; - h2o_accept(&dsc->h2o_accept_ctx, sock); + h2o_accept(accept_ctx, sock); } static int create_listener(const ComboAddress& addr, std::shared_ptr& dsc, int fd) @@ -510,57 +566,77 @@ static int create_listener(const ComboAddress& addr, std::shared_ptr& dsc, const std::string& cert_file, const std::string& key_file, const std::string& ciphers, const std::string& ciphers13) +static std::unique_ptr getTLSContext(const std::string& cert_file, const std::string& key_file, const std::string& ciphers, const std::string& ciphers13) { - SSL_load_error_strings(); - SSL_library_init(); - OpenSSL_add_all_algorithms(); - - dsc->h2o_accept_ctx.ssl_ctx = SSL_CTX_new(SSLv23_server_method()); + auto ctx = std::unique_ptr(SSL_CTX_new(SSLv23_server_method()), SSL_CTX_free); - SSL_CTX_set_options(dsc->h2o_accept_ctx.ssl_ctx, SSL_OP_NO_SSLv2); + SSL_CTX_set_options(ctx.get(), SSL_OP_NO_SSLv2); #ifdef SSL_CTX_set_ecdh_auto - SSL_CTX_set_ecdh_auto(dsc->h2o_accept_ctx.ssl_ctx, 1); + SSL_CTX_set_ecdh_auto(ctx.get(), 1); #endif /* load certificate and private key */ - if (SSL_CTX_use_certificate_chain_file(dsc->h2o_accept_ctx.ssl_ctx, cert_file.c_str()) != 1) { - errlog("An error occurred while trying to load the DOH server certificate file: %s", cert_file); - return -1; + if (SSL_CTX_use_certificate_chain_file(ctx.get(), cert_file.c_str()) != 1) { + throw std::runtime_error("Failed to setup SSL/TLS for DoH listener, an error occurred while trying to load the DOH server certificate file: " + cert_file); } - if (SSL_CTX_use_PrivateKey_file(dsc->h2o_accept_ctx.ssl_ctx, key_file.c_str(), SSL_FILETYPE_PEM) != 1) { - errlog("An error occurred while trying to load the DOH server private key file: %s", key_file); - return -1; + if (SSL_CTX_use_PrivateKey_file(ctx.get(), key_file.c_str(), SSL_FILETYPE_PEM) != 1) { + throw std::runtime_error("Failed to setup SSL/TLS for DoH listener, an error occurred while trying to load the DOH server private key file: " + key_file); } - if (SSL_CTX_set_cipher_list(dsc->h2o_accept_ctx.ssl_ctx, ciphers.c_str()) != 1) { - errlog("DOH ciphers could not be set: %s\n", ciphers); - return -1; + if (SSL_CTX_set_cipher_list(ctx.get(), ciphers.c_str()) != 1) { + throw std::runtime_error("Failed to setup SSL/TLS for DoH listener, DOH ciphers could not be set: " + ciphers); } #ifdef HAVE_SSL_CTX_SET_CIPHERSUITES - if (!ciphers13.empty() && SSL_CTX_set_ciphersuites(dsc->h2o_accept_ctx.ssl_ctx, ciphers13.c_str()) != 1) { - errlog("DOH TLS 1.3 ciphers could not be set: %s", ciphers13); - return -1; + if (!ciphers13.empty() && SSL_CTX_set_ciphersuites(ctx.get(), ciphers13.c_str()) != 1) { + throw std::runtime_error("Failed to setup SSL/TLS for DoH listener, DOH TLS 1.3 ciphers could not be set: " + ciphers13); } #endif /* HAVE_SSL_CTX_SET_CIPHERSUITES */ - h2o_ssl_register_alpn_protocols(dsc->h2o_accept_ctx.ssl_ctx, h2o_http2_alpn_protocols); + h2o_ssl_register_alpn_protocols(ctx.get(), h2o_http2_alpn_protocols); - return 0; + return ctx; +} + +static void setupAcceptContext(DOHAcceptContext& ctx, DOHServerConfig& dsc, bool setupTLS) +{ + auto nativeCtx = ctx.get(); + nativeCtx->ctx = &dsc.h2o_ctx; + nativeCtx->hosts = dsc.h2o_config.hosts; + if (setupTLS) { + auto tlsCtx = getTLSContext(dsc.df->d_certFile, dsc.df->d_keyFile, + dsc.df->d_ciphers.empty() ? "DEFAULT:!MD5:!DSS:!DES:!RC4:!RC2:!SEED:!IDEA:!NULL:!ADH:!EXP:!SRP:!PSK" : dsc.df->d_ciphers, + dsc.df->d_ciphers13); + + nativeCtx->ssl_ctx = tlsCtx.release(); + } + + ctx.release(); +} + +void DOHFrontend::reloadCertificate() +{ + auto newAcceptContext = std::unique_ptr(new DOHAcceptContext()); + setupAcceptContext(*newAcceptContext, *d_dsc, true); + DOHAcceptContext* oldCtx = d_dsc->accept_ctx; + d_dsc->accept_ctx = newAcceptContext.release(); + oldCtx->release(); } void DOHFrontend::setup() { + registerOpenSSLUser(); + d_dsc = std::make_shared(d_idleTimeout); - // we should probably make that hash, algorithm etc line configurable too - if (setup_ssl(d_dsc, d_certFile, d_keyFile, - d_ciphers.empty() ? "DEFAULT:!MD5:!DSS:!DES:!RC4:!RC2:!SEED:!IDEA:!NULL:!ADH:!EXP:!SRP:!PSK" : d_ciphers, - d_ciphers13) != 0) { - throw std::runtime_error("Failed to setup SSL/TLS for DoH listener"); - } + auto tlsCtx = getTLSContext(d_certFile, d_keyFile, + d_ciphers.empty() ? "DEFAULT:!MD5:!DSS:!DES:!RC4:!RC2:!SEED:!IDEA:!NULL:!ADH:!EXP:!SRP:!PSK" : d_ciphers, + d_ciphers13); + + auto accept_ctx = d_dsc->accept_ctx->get(); + accept_ctx->ssl_ctx = tlsCtx.release(); + d_dsc->accept_ctx->release(); } // this is the entrypoint from dnsdist.cc @@ -596,9 +672,7 @@ try // this listens to responses from dnsdist to turn into http responses h2o_socket_read_start(sock, on_dnsdist); - // as one does - dsc->h2o_accept_ctx.ctx = &dsc->h2o_ctx; - dsc->h2o_accept_ctx.hosts = dsc->h2o_config.hosts; + setupAcceptContext(*dsc->accept_ctx, *dsc, false); if (create_listener(df->d_local, dsc, cs->tcpFD) != 0) { throw std::runtime_error("DOH server failed to listen on " + df->d_local.toStringWithPort() + ": " + strerror(errno)); diff --git a/pdns/dnsdistdist/doh.hh b/pdns/dnsdistdist/doh.hh deleted file mode 100644 index cf0c04fa7..000000000 --- a/pdns/dnsdistdist/doh.hh +++ /dev/null @@ -1,68 +0,0 @@ -#pragma once -#include "iputils.hh" - -struct DOHServerConfig; - -struct DOHFrontend -{ - std::shared_ptr d_dsc{nullptr}; - std::string d_certFile; - std::string d_keyFile; - std::string d_ciphers; - std::string d_ciphers13; - ComboAddress d_local; - - uint32_t d_idleTimeout{30}; // HTTP idle timeout in seconds - std::vector d_urls; - std::string d_errortext; - std::atomic d_httpconnects; // number of TCP/IP connections established - std::atomic d_http1queries; // valid DNS queries received via HTTP1 - std::atomic d_http2queries; // valid DNS queries received via HTTP2 - std::atomic d_tls10queries; // valid DNS queries received via TLSv1.0 - std::atomic d_tls11queries; // valid DNS queries received via TLSv1.1 - std::atomic d_tls12queries; // valid DNS queries received via TLSv1.2 - std::atomic d_tls13queries; // valid DNS queries received via TLSv1.3 - std::atomic d_tlsUnknownqueries; // valid DNS queries received via unknown TLS version - - std::atomic d_getqueries; // valid DNS queries received via GET - std::atomic d_postqueries; // valid DNS queries received via POST - std::atomic d_badrequests; // request could not be converted to dns query - std::atomic d_errorresponses; // dnsdist set 'error' on response - std::atomic d_validresponses; // valid responses sent out - - void reloadCertificate() - { - // XXX: not implemented yet - } - -#ifndef HAVE_DNS_OVER_HTTPS - void setup() - { - } -#else - void setup(); -#endif /* HAVE_DNS_OVER_HTTPS */ -}; - -#ifndef HAVE_DNS_OVER_HTTPS -struct DOHUnit -{ -}; - -#else /* HAVE_DNS_OVER_HTTPS */ -struct st_h2o_req_t; - -struct DOHUnit -{ - std::string query; - ComboAddress remote; - ComboAddress dest; - st_h2o_req_t* req{nullptr}; - DOHUnit** self{nullptr}; - int rsock; - uint16_t qtype; - bool error{false}; - bool ednsAdded{false}; -}; - -#endif /* HAVE_DNS_OVER_HTTPS */ diff --git a/pdns/dnsdistdist/doh.hh b/pdns/dnsdistdist/doh.hh new file mode 120000 index 000000000..3b4ad59bc --- /dev/null +++ b/pdns/dnsdistdist/doh.hh @@ -0,0 +1 @@ +../doh.hh \ No newline at end of file diff --git a/pdns/dnsdistdist/libssl.cc b/pdns/dnsdistdist/libssl.cc new file mode 100644 index 000000000..0114743a5 --- /dev/null +++ b/pdns/dnsdistdist/libssl.cc @@ -0,0 +1,94 @@ + +#include "config.h" +#include "libssl.hh" + +#ifdef HAVE_LIBSSL + +#include +#include +#include +#include +#include +#include + +#if (OPENSSL_VERSION_NUMBER < 0x1010000fL || defined LIBRESSL_VERSION_NUMBER) +/* OpenSSL < 1.1.0 needs support for threading/locking in the calling application. */ +static pthread_mutex_t *openssllocks{nullptr}; + +extern "C" { +static void openssl_pthreads_locking_callback(int mode, int type, const char *file, int line) +{ + if (mode & CRYPTO_LOCK) { + pthread_mutex_lock(&(openssllocks[type])); + + } else { + pthread_mutex_unlock(&(openssllocks[type])); + } +} + +static unsigned long openssl_pthreads_id_callback() +{ + return (unsigned long)pthread_self(); +} +} + +static void openssl_thread_setup() +{ + openssllocks = (pthread_mutex_t*)OPENSSL_malloc(CRYPTO_num_locks() * sizeof(pthread_mutex_t)); + + for (int i = 0; i < CRYPTO_num_locks(); i++) + pthread_mutex_init(&(openssllocks[i]), NULL); + + CRYPTO_set_id_callback(openssl_pthreads_id_callback); + CRYPTO_set_locking_callback(openssl_pthreads_locking_callback); +} + +static void openssl_thread_cleanup() +{ + CRYPTO_set_locking_callback(NULL); + + for (int i=0; i s_users; + +void registerOpenSSLUser() +{ + if (s_users.fetch_add(1) == 0) { + SSL_load_error_strings(); + OpenSSL_add_ssl_algorithms(); + openssl_thread_setup(); + } +} + +void unregisterOpenSSLUser() +{ + if (s_users.fetch_sub(1) == 1) { + ERR_free_strings(); + + EVP_cleanup(); + + CONF_modules_finish(); + CONF_modules_free(); + CONF_modules_unload(1); + + CRYPTO_cleanup_all_ex_data(); + openssl_thread_cleanup(); + } +} + +#endif /* HAVE_LIBSSL */ diff --git a/pdns/dnsdistdist/libssl.hh b/pdns/dnsdistdist/libssl.hh new file mode 100644 index 000000000..a735815d5 --- /dev/null +++ b/pdns/dnsdistdist/libssl.hh @@ -0,0 +1,4 @@ +#pragma once + +void registerOpenSSLUser(); +void unregisterOpenSSLUser(); diff --git a/pdns/dnsdistdist/tcpiohandler.cc b/pdns/dnsdistdist/tcpiohandler.cc index 32731c85b..8d9a8e1bd 100644 --- a/pdns/dnsdistdist/tcpiohandler.cc +++ b/pdns/dnsdistdist/tcpiohandler.cc @@ -19,58 +19,7 @@ #include -#if (OPENSSL_VERSION_NUMBER < 0x1010000fL || defined LIBRESSL_VERSION_NUMBER) -/* OpenSSL < 1.1.0 needs support for threading/locking in the calling application. */ -static pthread_mutex_t *openssllocks{nullptr}; - -extern "C" { -static void openssl_pthreads_locking_callback(int mode, int type, const char *file, int line) -{ - if (mode & CRYPTO_LOCK) { - pthread_mutex_lock(&(openssllocks[type])); - - } else { - pthread_mutex_unlock(&(openssllocks[type])); - } -} - -static unsigned long openssl_pthreads_id_callback() -{ - return (unsigned long)pthread_self(); -} -} - -static void openssl_thread_setup() -{ - openssllocks = (pthread_mutex_t*)OPENSSL_malloc(CRYPTO_num_locks() * sizeof(pthread_mutex_t)); - - for (int i = 0; i < CRYPTO_num_locks(); i++) - pthread_mutex_init(&(openssllocks[i]), NULL); - - CRYPTO_set_id_callback(openssl_pthreads_id_callback); - CRYPTO_set_locking_callback(openssl_pthreads_locking_callback); -} - -static void openssl_thread_cleanup() -{ - CRYPTO_set_locking_callback(NULL); - - for (int i=0; i d_dsc{nullptr}; + std::string d_certFile; + std::string d_keyFile; + std::string d_ciphers; + std::string d_ciphers13; + ComboAddress d_local; + + uint32_t d_idleTimeout{30}; // HTTP idle timeout in seconds + std::vector d_urls; + + std::atomic d_httpconnects; // number of TCP/IP connections established + std::atomic d_http1queries; // valid DNS queries received via HTTP1 + std::atomic d_http2queries; // valid DNS queries received via HTTP2 + std::atomic d_tls10queries; // valid DNS queries received via TLSv1.0 + std::atomic d_tls11queries; // valid DNS queries received via TLSv1.1 + std::atomic d_tls12queries; // valid DNS queries received via TLSv1.2 + std::atomic d_tls13queries; // valid DNS queries received via TLSv1.3 + std::atomic d_tlsUnknownqueries; // valid DNS queries received via unknown TLS version + + std::atomic d_getqueries; // valid DNS queries received via GET + std::atomic d_postqueries; // valid DNS queries received via POST + std::atomic d_badrequests; // request could not be converted to dns query + std::atomic d_errorresponses; // dnsdist set 'error' on response + std::atomic d_validresponses; // valid responses sent out + +#ifndef HAVE_DNS_OVER_HTTPS + void setup() + { + } + + void reloadCertificate() + { + } +#else + void setup(); + void reloadCertificate(); +#endif /* HAVE_DNS_OVER_HTTPS */ +}; + +#ifndef HAVE_DNS_OVER_HTTPS +struct DOHUnit +{ +}; + +#else /* HAVE_DNS_OVER_HTTPS */ +struct st_h2o_req_t; + +struct DOHUnit +{ + std::string query; + ComboAddress remote; + ComboAddress dest; + st_h2o_req_t* req{nullptr}; + DOHUnit** self{nullptr}; + int rsock; + uint16_t qtype; + bool error{false}; + bool ednsAdded{false}; +}; + +#endif /* HAVE_DNS_OVER_HTTPS */ -- 2.40.0