From: Remi Gacogne Date: Mon, 16 Sep 2024 10:27:00 +0000 (+0200) Subject: dnsdist: Set the ALPN of TLS contexts right away X-Git-Tag: rec-5.2.0-alpha1~78^2~1 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=37c4e8e9f3f0c6e35f5a4102e43b0816d149de6f;p=thirdparty%2Fpdns.git dnsdist: Set the ALPN of TLS contexts right away --- diff --git a/pdns/dnsdistdist/dnsdist-backend.cc b/pdns/dnsdistdist/dnsdist-backend.cc index 11b00667b9..d493d3f9aa 100644 --- a/pdns/dnsdistdist/dnsdist-backend.cc +++ b/pdns/dnsdistdist/dnsdist-backend.cc @@ -306,26 +306,19 @@ DownstreamState::DownstreamState(DownstreamState::Config&& config, std::shared_p setName(d_config.name); - if (d_tlsCtx) { - if (!d_config.d_dohPath.empty()) { + if (d_tlsCtx && !d_config.d_dohPath.empty()) { #ifdef HAVE_NGHTTP2 - setupDoHClientProtocolNegotiation(d_tlsCtx); - - auto outgoingDoHWorkerThreads = dnsdist::configuration::getImmutableConfiguration().d_outgoingDoHWorkers; - if (dnsdist::configuration::isImmutableConfigurationDone() && outgoingDoHWorkerThreads && *outgoingDoHWorkerThreads == 0) { - throw std::runtime_error("Error: setOutgoingDoHWorkerThreads() is set to 0 so no outgoing DoH worker thread is available to serve queries"); - } - - if (!dnsdist::configuration::isImmutableConfigurationDone() && (!outgoingDoHWorkerThreads || *outgoingDoHWorkerThreads == 0)) { - dnsdist::configuration::updateImmutableConfiguration([](dnsdist::configuration::ImmutableConfiguration& immutableConfig) { - immutableConfig.d_outgoingDoHWorkers = 1; - }); - } -#endif /* HAVE_NGHTTP2 */ + auto outgoingDoHWorkerThreads = dnsdist::configuration::getImmutableConfiguration().d_outgoingDoHWorkers; + if (dnsdist::configuration::isImmutableConfigurationDone() && outgoingDoHWorkerThreads && *outgoingDoHWorkerThreads == 0) { + throw std::runtime_error("Error: setOutgoingDoHWorkerThreads() is set to 0 so no outgoing DoH worker thread is available to serve queries"); } - else { - setupDoTProtocolNegotiation(d_tlsCtx); + + if (!dnsdist::configuration::isImmutableConfigurationDone() && (!outgoingDoHWorkerThreads || *outgoingDoHWorkerThreads == 0)) { + dnsdist::configuration::updateImmutableConfiguration([](dnsdist::configuration::ImmutableConfiguration& immutableConfig) { + immutableConfig.d_outgoingDoHWorkers = 1; + }); } +#endif /* HAVE_NGHTTP2 */ } if (connect && !isTCPOnly()) { diff --git a/pdns/dnsdistdist/dnsdist-lua.cc b/pdns/dnsdistdist/dnsdist-lua.cc index 2233e8aa0e..2d939e3c2c 100644 --- a/pdns/dnsdistdist/dnsdist-lua.cc +++ b/pdns/dnsdistdist/dnsdist-lua.cc @@ -576,7 +576,6 @@ static void setupLuaConfig(LuaContext& luaCtx, bool client, bool configCheck) if (getOptionalValue(vars, "tls", valueStr) > 0) { serverPort = 853; config.d_tlsParams.d_provider = valueStr; - tlsCtx = getTLSContext(config.d_tlsParams); if (getOptionalValue(vars, "dohPath", valueStr) > 0) { #if !defined(HAVE_DNS_OVER_HTTPS) || !defined(HAVE_NGHTTP2) @@ -585,9 +584,15 @@ static void setupLuaConfig(LuaContext& luaCtx, bool client, bool configCheck) serverPort = 443; config.d_dohPath = valueStr; + config.d_tlsParams.d_alpn = TLSFrontend::ALPN::DoH; getOptionalValue(vars, "addXForwardedHeaders", config.d_addXForwardedHeaders); } + else { + config.d_tlsParams.d_alpn = TLSFrontend::ALPN::DoT; + } + + tlsCtx = getTLSContext(config.d_tlsParams); } try { diff --git a/pdns/dnsdistdist/dnsdist-nghttp2.cc b/pdns/dnsdistdist/dnsdist-nghttp2.cc index 07033f04ec..e42ff741af 100644 --- a/pdns/dnsdistdist/dnsdist-nghttp2.cc +++ b/pdns/dnsdistdist/dnsdist-nghttp2.cc @@ -1032,21 +1032,6 @@ bool initDoHWorkers() #endif /* HAVE_DNS_OVER_HTTPS && HAVE_NGHTTP2 */ } -bool setupDoHClientProtocolNegotiation(std::shared_ptr& ctx) -{ - if (ctx == nullptr) { - return false; - } -#if defined(HAVE_DNS_OVER_HTTPS) && defined(HAVE_NGHTTP2) - /* we want to set the ALPN to h2, if only to mitigate the ALPACA attack */ - const std::vector> h2Alpns = {{'h', '2'}}; - ctx->setALPNProtos(h2Alpns); - return true; -#else /* HAVE_DNS_OVER_HTTPS && HAVE_NGHTTP2 */ - return false; -#endif /* HAVE_DNS_OVER_HTTPS && HAVE_NGHTTP2 */ -} - bool sendH2Query(const std::shared_ptr& ds, std::unique_ptr& mplexer, std::shared_ptr& sender, InternalQuery&& query, bool healthCheck) { #if defined(HAVE_DNS_OVER_HTTPS) && defined(HAVE_NGHTTP2) diff --git a/pdns/dnsdistdist/dnsdist-nghttp2.hh b/pdns/dnsdistdist/dnsdist-nghttp2.hh index 4027c26aef..043b4977e1 100644 --- a/pdns/dnsdistdist/dnsdist-nghttp2.hh +++ b/pdns/dnsdistdist/dnsdist-nghttp2.hh @@ -61,7 +61,6 @@ extern std::atomic g_dohStatesDumpRequested; class TLSCtx; bool initDoHWorkers(); -bool setupDoHClientProtocolNegotiation(std::shared_ptr& ctx); /* opens a new HTTP/2 connection to the supplied backend (attached to the supplied multiplexer), sends the query, waits for the response to come back or an error to occur then notifies the sender, closing the connection. */ diff --git a/pdns/tcpiohandler.cc b/pdns/tcpiohandler.cc index 83aad570b6..2d8e0f7e7f 100644 --- a/pdns/tcpiohandler.cc +++ b/pdns/tcpiohandler.cc @@ -26,6 +26,26 @@ bool shouldDoVerboseLogging() TLSCtx::tickets_key_added_hook TLSCtx::s_ticketsKeyAddedHook{nullptr}; +static std::vector> getALPNVector(TLSFrontend::ALPN alpn, bool client) +{ + if (alpn == TLSFrontend::ALPN::DoT) { + /* we want to set the ALPN to dot (RFC7858), if only to mitigate the ALPACA attack */ + return std::vector>{{'d', 'o', 't'}}; + } + if (alpn == TLSFrontend::ALPN::DoH) { + if (client) { + /* we want to set the ALPN to h2, if only to mitigate the ALPACA attack */ + return std::vector>{{'h', '2'}}; + } + /* For server contexts, we want to set the ALPN for DoH (note that h2o sets it own ALPN values): + - HTTP/1.1 so that the OpenSSL callback ALPN accepts it, letting us later return a static response + - HTTP/2 + */ + return std::vector>{{'h', '2'},{'h', 't', 't', 'p', '/', '1', '.', '1'}}; + } + return {}; +} + #if defined(HAVE_DNS_OVER_TLS) || defined(HAVE_DNS_OVER_HTTPS) #ifdef HAVE_LIBSSL @@ -592,13 +612,13 @@ private: static LockGuarded s_initTLSConnIndex; static int s_tlsConnIndex; std::vector> d_tlsSessions; - std::shared_ptr d_tlsCtx; // we need to hold a reference to this to make sure that the context exists for as long as the connection, even if a reload happens in the meantime + const std::shared_ptr d_tlsCtx; // we need to hold a reference to this to make sure that the context exists for as long as the connection, even if a reload happens in the meantime std::unique_ptr d_conn; - std::string d_hostname; - struct timeval d_timeout; + const std::string d_hostname; + const timeval d_timeout; bool d_connected{false}; bool d_ktls{false}; - bool d_isClient{false}; + const bool d_isClient{false}; }; LockGuarded OpenSSLTLSConnection::s_initTLSConnIndex{false}; @@ -623,7 +643,7 @@ public: } /* server side context */ - OpenSSLTLSIOCtx(TLSFrontend& frontend, [[maybe_unused]] Private priv): d_feContext(std::make_unique(frontend.d_addr, frontend.d_tlsConfig)) + OpenSSLTLSIOCtx(TLSFrontend& frontend, [[maybe_unused]] Private priv): d_alpnProtos(getALPNVector(frontend.d_alpn, false)), d_feContext(std::make_unique(frontend.d_addr, frontend.d_tlsConfig)) { OpenSSLTLSConnection::generateConnectionIndexIfNeeded(); @@ -652,6 +672,8 @@ public: libssl_set_error_counters_callback(d_feContext->d_tlsCtx, &frontend.d_tlsCounters); + libssl_set_alpn_select_callback(d_feContext->d_tlsCtx.get(), alpnServerSelectCallback, this); + if (!frontend.d_tlsConfig.d_keyLogFile.empty()) { d_feContext->d_keyLogFile = libssl_set_key_log_file(d_feContext->d_tlsCtx, frontend.d_tlsConfig.d_keyLogFile); } @@ -754,6 +776,8 @@ public: SSL_CTX_set_session_cache_mode(d_tlsCtx.get(), SSL_SESS_CACHE_CLIENT | SSL_SESS_CACHE_NO_INTERNAL_STORE); SSL_CTX_sess_set_new_cb(d_tlsCtx.get(), &OpenSSLTLSIOCtx::newTicketFromServerCb); + libssl_set_alpn_protos(d_tlsCtx.get(), getALPNVector(params.d_alpn, true)); + #ifdef SSL_MODE_RELEASE_BUFFERS if (params.d_releaseBuffers) { SSL_CTX_set_mode(d_tlsCtx.get(), SSL_MODE_RELEASE_BUFFERS); @@ -880,22 +904,6 @@ public: return d_feContext != nullptr; } - bool setALPNProtos(const std::vector>& protos) override - { - auto* openSSLContext = getOpenSSLContext(); - if (openSSLContext == nullptr) { - return false; - } - - if (isServerContext()) { - d_alpnProtos = protos; - libssl_set_alpn_select_callback(openSSLContext, alpnServerSelectCallback, this); - return true; - } - - return libssl_set_alpn_protos(openSSLContext, protos); - } - private: /* called in a client context, if the client advertised more than one ALPN value and the server returned more than one as well, to select the one to use. */ static int alpnServerSelectCallback(SSL*, const unsigned char** out, unsigned char* outlen, const unsigned char* in, unsigned int inlen, void* arg) @@ -930,10 +938,9 @@ private: return SSL_TLSEXT_ERR_NOACK; } - std::vector> d_alpnProtos; // store the supported ALPN protocols, so that the server can select based on what the client sent + const std::vector> d_alpnProtos; // store the supported ALPN protocols, so that the server can select based on what the client sent std::shared_ptr d_tlsCtx{nullptr}; // client context, on a server-side the context is stored in d_feContext->d_tlsCtx std::unique_ptr d_feContext{nullptr}; - bool (*d_nextProtocolSelectCallback)(unsigned char** out, unsigned char* outlen, const unsigned char* in, unsigned int inlen){nullptr}; bool d_ktls{false}; }; @@ -1596,7 +1603,7 @@ private: std::unique_ptr d_conn; std::vector> d_tlsSessions; std::string d_host; - bool d_client{false}; + const bool d_client{false}; bool d_handshakeDone{false}; }; @@ -1604,7 +1611,7 @@ class GnuTLSIOCtx: public TLSCtx { public: /* server side context */ - GnuTLSIOCtx(TLSFrontend& fe): d_enableTickets(fe.d_tlsConfig.d_enableTickets) + GnuTLSIOCtx(TLSFrontend& fe): d_protos(getALPNVector(fe.d_alpn, false)), d_enableTickets(fe.d_tlsConfig.d_enableTickets) { int rc = 0; d_ticketsKeyRotationDelay = fe.d_tlsConfig.d_ticketsKeyRotationDelay; @@ -1662,7 +1669,7 @@ public: } /* client side context */ - GnuTLSIOCtx(const TLSContextParameters& params): d_contextParameters(std::make_unique(params)), d_enableTickets(true), d_validateCerts(params.d_validateCertificates) + GnuTLSIOCtx(const TLSContextParameters& params): d_protos(getALPNVector(params.d_alpn, true)), d_contextParameters(std::make_unique(params)), d_enableTickets(true), d_validateCerts(params.d_validateCertificates) { int rc = 0; @@ -1817,21 +1824,11 @@ public: return "gnutls"; } - bool setALPNProtos(const std::vector>& protos) override - { -#ifdef HAVE_GNUTLS_ALPN_SET_PROTOCOLS - d_protos = protos; - return true; -#else - return false; -#endif - } - private: /* client context parameters */ - std::unique_ptr d_contextParameters{nullptr}; std::shared_ptr d_creds; - std::vector> d_protos; + const std::vector> d_protos; + std::unique_ptr d_contextParameters{nullptr}; gnutls_priority_t d_priorityCache{nullptr}; SharedLockGuarded> d_ticketsKey{nullptr}; bool d_enableTickets{true}; @@ -1842,34 +1839,6 @@ private: #endif /* HAVE_DNS_OVER_TLS || HAVE_DNS_OVER_HTTPS */ -bool setupDoTProtocolNegotiation(std::shared_ptr& ctx) -{ - if (ctx == nullptr) { - return false; - } - /* we want to set the ALPN to dot (RFC7858), if only to mitigate the ALPACA attack */ - const std::vector> dotAlpns = {{'d', 'o', 't'}}; - ctx->setALPNProtos(dotAlpns); - return true; -} - -bool setupDoHProtocolNegotiation(std::shared_ptr& ctx) -{ - if (ctx == nullptr) { - return false; - } - /* This code is only called for incoming/server TLS contexts (not outgoing/client), - and h2o sets it own ALPN values. - We want to set the ALPN for DoH: - - HTTP/1.1 so that the OpenSSL callback ALPN accepts it, letting us later return a static response - - HTTP/2 - */ - const std::vector> dohAlpns{{'h', '2'},{'h', 't', 't', 'p', '/', '1', '.', '1'}}; - ctx->setALPNProtos(dohAlpns); - - return true; -} - bool TLSFrontend::setupTLS() { #if defined(HAVE_DNS_OVER_TLS) || defined(HAVE_DNS_OVER_HTTPS) @@ -1896,13 +1865,6 @@ bool TLSFrontend::setupTLS() #endif } - if (d_alpn == ALPN::DoT) { - setupDoTProtocolNegotiation(newCtx); - } - else if (d_alpn == ALPN::DoH) { - setupDoHProtocolNegotiation(newCtx); - } - std::atomic_store_explicit(&d_ctx, std::move(newCtx), std::memory_order_release); #endif /* HAVE_DNS_OVER_TLS || HAVE_DNS_OVER_HTTPS */ return true; diff --git a/pdns/tcpiohandler.hh b/pdns/tcpiohandler.hh index 9e0aa09137..f24735cc88 100644 --- a/pdns/tcpiohandler.hh +++ b/pdns/tcpiohandler.hh @@ -111,12 +111,6 @@ public: virtual size_t getTicketsKeysCount() = 0; virtual std::string getName() const = 0; - /* set the advertised ALPN protocols, in client or server context */ - virtual bool setALPNProtos(const std::vector>& /* protos */) - { - return false; - } - using tickets_key_added_hook = std::function; static void setTicketsKeyAddedHook(const tickets_key_added_hook& hook) @@ -577,6 +571,7 @@ struct TLSContextParameters std::string d_ciphers; std::string d_ciphers13; std::string d_caStore; + TLSFrontend::ALPN d_alpn{TLSFrontend::ALPN::Unset}; bool d_validateCertificates{true}; bool d_releaseBuffers{true}; bool d_enableRenegotiation{false};