From: Remi Gacogne Date: Tue, 8 Feb 2022 09:35:19 +0000 (+0100) Subject: TCPIOHandler: Handle validation of IP addresses in certificates X-Git-Tag: rec-4.7.0-alpha1~9^2~14 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=74b08b2a7d0c561cd8d55e8d94d73a3fb89d4e36;p=thirdparty%2Fpdns.git TCPIOHandler: Handle validation of IP addresses in certificates --- diff --git a/pdns/dnsdist-lua.cc b/pdns/dnsdist-lua.cc index 6c3d66dc6a..23f549e13d 100644 --- a/pdns/dnsdist-lua.cc +++ b/pdns/dnsdist-lua.cc @@ -565,6 +565,17 @@ static void setupLuaConfig(LuaContext& luaCtx, bool client, bool configCheck) if (vars.count("subjectName")) { config.d_tlsSubjectName = boost::get(vars.at("subjectName")); } + else if (vars.count("subjectAddr")) { + try { + ComboAddress ca(boost::get(vars.at("subjectAddr"))); + config.d_tlsSubjectName = ca.toString(); + config.d_tlsSubjectIsAddr = true; + } + catch (const std::exception& e) { + errlog("Error creating new server: downstream subjectAddr value must be a valid IP address"); + return std::shared_ptr(); + } + } if (vars.count("tls")) { config.d_tlsParams.d_provider = boost::get(vars.at("tls")); diff --git a/pdns/dnsdist.hh b/pdns/dnsdist.hh index 4c51a6842b..f03522db86 100644 --- a/pdns/dnsdist.hh +++ b/pdns/dnsdist.hh @@ -739,6 +739,7 @@ struct DownstreamState: public std::enable_shared_from_this uint8_t maxCheckFailures{1}; uint8_t minRiseSuccesses{1}; Availability availability{Availability::Auto}; + bool d_tlsSubjectIsAddr{false}; bool mustResolve{false}; bool useECS{false}; bool useProxyProtocol{false}; diff --git a/pdns/dnsdistdist/dnsdist-discovery.cc b/pdns/dnsdistdist/dnsdist-discovery.cc index 58a6b025bf..8821f4ea81 100644 --- a/pdns/dnsdistdist/dnsdist-discovery.cc +++ b/pdns/dnsdistdist/dnsdist-discovery.cc @@ -369,7 +369,7 @@ static bool checkBackendUsability(std::shared_ptr& ds) } time_t now = time(nullptr); - auto handler = std::make_unique(ds->d_config.d_tlsSubjectName, sock.releaseHandle(), timeval{ds->d_config.checkTimeout, 0}, ds->d_tlsCtx, now); + auto handler = std::make_unique(ds->d_config.d_tlsSubjectName, ds->d_config.d_tlsSubjectIsAddr, sock.releaseHandle(), timeval{ds->d_config.checkTimeout, 0}, ds->d_tlsCtx, now); handler->connect(ds->d_config.tcpFastOpen, ds->d_config.remote, timeval{ds->d_config.checkTimeout, 0}); return true; } @@ -424,6 +424,7 @@ bool ServiceDiscovery::tryToUpgradeBackend(const UpgradeableBackend& backend) extension." */ config.d_tlsSubjectName = backend.d_ds->d_config.remote.toString(); + config.d_tlsSubjectIsAddr = true; } if (!backend.d_poolAfterUpgrade.empty()) { diff --git a/pdns/dnsdistdist/dnsdist-healthchecks.cc b/pdns/dnsdistdist/dnsdist-healthchecks.cc index d46ec532b3..a68e1ef0ae 100644 --- a/pdns/dnsdistdist/dnsdist-healthchecks.cc +++ b/pdns/dnsdistdist/dnsdist-healthchecks.cc @@ -420,7 +420,7 @@ bool queueHealthCheck(std::unique_ptr& mplexer, const std::shared } else { time_t now = time(nullptr); - data->d_tcpHandler = std::make_unique(ds->d_config.d_tlsSubjectName, sock.releaseHandle(), timeval{ds->d_config.checkTimeout,0}, ds->d_tlsCtx, now); + data->d_tcpHandler = std::make_unique(ds->d_config.d_tlsSubjectName, ds->d_config.d_tlsSubjectIsAddr, sock.releaseHandle(), timeval{ds->d_config.checkTimeout,0}, ds->d_tlsCtx, now); data->d_ioState = std::make_unique(*mplexer, data->d_tcpHandler->getDescriptor()); if (ds->d_tlsCtx) { try { diff --git a/pdns/dnsdistdist/dnsdist-tcp-downstream.cc b/pdns/dnsdistdist/dnsdist-tcp-downstream.cc index ebb63a99c8..511ed30f32 100644 --- a/pdns/dnsdistdist/dnsdist-tcp-downstream.cc +++ b/pdns/dnsdistdist/dnsdist-tcp-downstream.cc @@ -95,7 +95,7 @@ bool ConnectionToBackend::reconnect() socket->setNonBlocking(); gettimeofday(&d_connectionStartTime, nullptr); - auto handler = std::make_unique(d_ds->d_config.d_tlsSubjectName, socket->releaseHandle(), timeval{0,0}, d_ds->d_tlsCtx, d_connectionStartTime.tv_sec); + auto handler = std::make_unique(d_ds->d_config.d_tlsSubjectName, d_ds->d_config.d_tlsSubjectIsAddr, socket->releaseHandle(), timeval{0,0}, d_ds->d_tlsCtx, d_connectionStartTime.tv_sec); if (!tlsSession && d_ds->d_tlsCtx) { tlsSession = g_sessionCache.getSession(d_ds->getID(), d_connectionStartTime.tv_sec); } diff --git a/pdns/dnsdistdist/docs/reference/config.rst b/pdns/dnsdistdist/docs/reference/config.rst index 62b7b3cf85..ede565291e 100644 --- a/pdns/dnsdistdist/docs/reference/config.rst +++ b/pdns/dnsdistdist/docs/reference/config.rst @@ -533,7 +533,7 @@ Servers Added ``addXForwardedHeaders``, ``caStore``, ``checkTCP``, ``ciphers``, ``ciphers13``, ``dohPath``, ``enableRenegotiation``, ``releaseBuffers``, ``subjectName``, ``tcpOnly``, ``tls`` and ``validateCertificates`` to server_table. .. versionchanged:: 1.8.0 - Added ``autoUpgrade``, ``autoUpgradeDoHKey``, ``autoUpgradeInterval``, ``autoUpgradeKeep`` and ``autoUpgradePool`` to server_table. + Added ``autoUpgrade``, ``autoUpgradeDoHKey``, ``autoUpgradeInterval``, ``autoUpgradeKeep``, ``autoUpgradePool`` and ``subjectAddr`` to server_table. Add a new backend server. Call this function with either a string:: @@ -587,7 +587,8 @@ Servers caStore=STRING, -- Specifies the path to the CA certificate file, in PEM format, to use to check the certificate presented by the backend. Default is an empty string, which means to use the system CA store. Note that this directive is only used if ``validateCertificates`` is set. ciphers=STRING, -- The TLS ciphers to use. The exact format depends on the provider used. When the OpenSSL provider is used, ciphers for TLS 1.3 must be specified via ``ciphersTLS13``. ciphersTLS13=STRING, -- The ciphers to use for TLS 1.3, when the OpenSSL provider is used. When the GnuTLS provider is used, ``ciphers`` applies regardless of the TLS protocol and this setting is not used. - subjectName=STRING, -- The subject name passed in the SNI value of the TLS handshake, and against which to validate the certificate presented by the backend. Default is empty. + subjectName=STRING, -- The subject name passed in the SNI value of the TLS handshake, and against which to validate the certificate presented by the backend. Default is empty. If set this value supersedes any ``subjectAddr`` one. + subjectAddr=STRING, -- The subject IP address passed in the SNI value of the TLS handshake, and against which to validate the certificate presented by the backend. Default is empty. validateCertificates=BOOL,-- Whether the certificate presented by the backend should be validated against the CA store (see ``caStore``). Default is true. dohPath=STRING, -- Enable DNS over HTTPS communication for this backend, using POST queries to the HTTP host supplied as ``subjectName`` and the HTTP path supplied in this parameter. addXForwardedHeaders=BOOL,-- Whether to add X-Forwarded-For, X-Forwarded-Port and X-Forwarded-Proto headers to a DNS over HTTPS backend. diff --git a/pdns/dnsdistdist/test-dnsdistnghttp2_cc.cc b/pdns/dnsdistdist/test-dnsdistnghttp2_cc.cc index 8e1bd912c7..7bf4e2c8ef 100644 --- a/pdns/dnsdistdist/test-dnsdistnghttp2_cc.cc +++ b/pdns/dnsdistdist/test-dnsdistnghttp2_cc.cc @@ -498,7 +498,7 @@ public: return std::make_unique(socket); } - std::unique_ptr getClientConnection(const std::string& host, int socket, const struct timeval& timeout) override + std::unique_ptr getClientConnection(const std::string& host, bool hostIsAddr, int socket, const struct timeval& timeout) override { return std::make_unique(socket, true, d_needProxyProtocol); } diff --git a/pdns/dnsdistdist/test-dnsdisttcp_cc.cc b/pdns/dnsdistdist/test-dnsdisttcp_cc.cc index 9f24135848..79c5cec57b 100644 --- a/pdns/dnsdistdist/test-dnsdisttcp_cc.cc +++ b/pdns/dnsdistdist/test-dnsdisttcp_cc.cc @@ -306,7 +306,7 @@ public: return std::make_unique(socket); } - std::unique_ptr getClientConnection(const std::string& host, int socket, const struct timeval& timeout) override + std::unique_ptr getClientConnection(const std::string& host, bool hostIsAddr, int socket, const struct timeval& timeout) override { return std::make_unique(socket, true); } diff --git a/pdns/lwres.cc b/pdns/lwres.cc index 0e42dc1dc3..933b469b87 100644 --- a/pdns/lwres.cc +++ b/pdns/lwres.cc @@ -275,7 +275,7 @@ static bool tcpconnect(const struct timeval& now, const ComboAddress& ip, TCPOut dnsOverTLS = false; } } - connection.d_handler = std::make_shared(nsName, s.releaseHandle(), timeout, tlsCtx, now.tv_sec); + connection.d_handler = std::make_shared(nsName, false, s.releaseHandle(), timeout, tlsCtx, now.tv_sec); // Returned state ignored // This can throw an exception, retry will need to happen at higher level connection.d_handler->tryConnect(SyncRes::s_tcp_fast_open_connect, ip); diff --git a/pdns/rec-carbon.cc b/pdns/rec-carbon.cc index 723e447296..a8e67a6bae 100644 --- a/pdns/rec-carbon.cc +++ b/pdns/rec-carbon.cc @@ -48,7 +48,7 @@ void doCarbonDump(void*) { g_networkTimeoutMsec / 1000, static_cast(g_networkTimeoutMsec) % 1000 * 1000 }; - auto handler = std::make_shared("", s.releaseHandle(), timeout, tlsCtx, time(nullptr)); + auto handler = std::make_shared("", false, s.releaseHandle(), timeout, tlsCtx, time(nullptr)); handler->tryConnect(SyncRes::s_tcp_fast_open_connect, remote); // we do the connect so the first attempt happens while we gather stats if (msg.empty()) { diff --git a/pdns/sdig.cc b/pdns/sdig.cc index 6e9d77635b..917f75038e 100644 --- a/pdns/sdig.cc +++ b/pdns/sdig.cc @@ -418,7 +418,7 @@ try { Socket sock(dest.sin4.sin_family, SOCK_STREAM); sock.setNonBlocking(); setTCPNoDelay(sock.getHandle()); // disable NAGLE, which does not play nicely with delayed ACKs - TCPIOHandler handler(subjectName, sock.releaseHandle(), timeout, tlsCtx, time(nullptr)); + TCPIOHandler handler(subjectName, false, sock.releaseHandle(), timeout, tlsCtx, time(nullptr)); handler.connect(fastOpen, dest, timeout); // we are writing the proxyheader inside the TLS connection. Is that right? if (proxyheader.size() > 0 && handler.write(proxyheader.data(), proxyheader.size(), timeout) != proxyheader.size()) { diff --git a/pdns/tcpiohandler.cc b/pdns/tcpiohandler.cc index c85ef7d77c..84ac3aef76 100644 --- a/pdns/tcpiohandler.cc +++ b/pdns/tcpiohandler.cc @@ -102,7 +102,7 @@ public: } /* client-side connection */ - OpenSSLTLSConnection(const std::string& hostname, int socket, const struct timeval& timeout, std::shared_ptr& tlsCtx): d_tlsCtx(tlsCtx), d_conn(std::unique_ptr(SSL_new(tlsCtx.get()), SSL_free)), d_hostname(hostname), d_timeout(timeout) + OpenSSLTLSConnection(const std::string& hostname, bool hostIsAddr, int socket, const struct timeval& timeout, std::shared_ptr& tlsCtx): d_tlsCtx(tlsCtx), d_conn(std::unique_ptr(SSL_new(tlsCtx.get()), SSL_free)), d_hostname(hostname), d_timeout(timeout) { d_socket = socket; @@ -131,21 +131,36 @@ public: throw std::runtime_error("Error setting TLS SNI to " + d_hostname); } -#if (OPENSSL_VERSION_NUMBER >= 0x1010000fL) && HAVE_SSL_SET_HOSTFLAGS // grrr libressl - SSL_set_hostflags(d_conn.get(), X509_CHECK_FLAG_NO_PARTIAL_WILDCARDS); - if (SSL_set1_host(d_conn.get(), d_hostname.c_str()) != 1) { - throw std::runtime_error("Error setting TLS hostname for certificate validation"); + if (hostIsAddr) { +#if (OPENSSL_VERSION_NUMBER >= 0x10002000L) + X509_VERIFY_PARAM *param = SSL_get0_param(d_conn.get()); + /* Enable automatic IP checks */ + X509_VERIFY_PARAM_set_hostflags(param, X509_CHECK_FLAG_NO_PARTIAL_WILDCARDS); + if (X509_VERIFY_PARAM_set1_ip_asc(param, d_hostname.c_str()) != 1) { + throw std::runtime_error("Error setting TLS IP for certificate validation"); + } +#else + /* no validation for you, see https://wiki.openssl.org/index.php/Hostname_validation */ +#endif } + else { +#if (OPENSSL_VERSION_NUMBER >= 0x1010000fL) && HAVE_SSL_SET_HOSTFLAGS // grrr libressl + SSL_set_hostflags(d_conn.get(), X509_CHECK_FLAG_NO_PARTIAL_WILDCARDS); + if (SSL_set1_host(d_conn.get(), d_hostname.c_str()) != 1) { + throw std::runtime_error("Error setting TLS hostname for certificate validation"); + } #elif (OPENSSL_VERSION_NUMBER >= 0x10002000L) - X509_VERIFY_PARAM *param = SSL_get0_param(d_conn.get()); - /* Enable automatic hostname checks */ - X509_VERIFY_PARAM_set_hostflags(param, X509_CHECK_FLAG_NO_PARTIAL_WILDCARDS); - if (X509_VERIFY_PARAM_set1_host(param, d_hostname.c_str(), d_hostname.size()) != 1) { - throw std::runtime_error("Error setting TLS hostname for certificate validation"); - } + X509_VERIFY_PARAM *param = SSL_get0_param(d_conn.get()); + /* Enable automatic hostname checks */ + X509_VERIFY_PARAM_set_hostflags(param, X509_CHECK_FLAG_NO_PARTIAL_WILDCARDS); + if (X509_VERIFY_PARAM_set1_host(param, d_hostname.c_str(), d_hostname.size()) != 1) { + throw std::runtime_error("Error setting TLS hostname for certificate validation"); + } #else - /* no hostname validation for you, see https://wiki.openssl.org/index.php/Hostname_validation */ + /* no hostname validation for you, see https://wiki.openssl.org/index.php/Hostname_validation */ #endif + } + SSL_set_ex_data(d_conn.get(), s_tlsConnIndex, this); } @@ -734,9 +749,9 @@ public: return std::make_unique(socket, timeout, d_feContext); } - std::unique_ptr getClientConnection(const std::string& host, int socket, const struct timeval& timeout) override + std::unique_ptr getClientConnection(const std::string& host, bool hostIsAddr, int socket, const struct timeval& timeout) override { - return std::make_unique(host, socket, timeout, d_tlsCtx); + return std::make_unique(host, hostIsAddr, socket, timeout, d_tlsCtx); } void rotateTicketsKey(time_t now) override @@ -1654,7 +1669,7 @@ public: return entry; } - std::unique_ptr getClientConnection(const std::string& host, int socket, const struct timeval& timeout) override + std::unique_ptr getClientConnection(const std::string& host, bool, int socket, const struct timeval& timeout) override { auto creds = getPerThreadCredentials(d_contextParameters->d_validateCertificates, d_contextParameters->d_caStore); auto connection = std::make_unique(host, socket, timeout, creds, d_priorityCache, d_validateCerts); diff --git a/pdns/tcpiohandler.hh b/pdns/tcpiohandler.hh index 48968e41bc..92faa6909a 100644 --- a/pdns/tcpiohandler.hh +++ b/pdns/tcpiohandler.hh @@ -78,7 +78,7 @@ public: } virtual ~TLSCtx() {} virtual std::unique_ptr getConnection(int socket, const struct timeval& timeout, time_t now) = 0; - virtual std::unique_ptr getClientConnection(const std::string& host, int socket, const struct timeval& timeout) = 0; + virtual std::unique_ptr getClientConnection(const std::string& host, bool hostIsAddr, int socket, const struct timeval& timeout) = 0; virtual void rotateTicketsKey(time_t now) = 0; virtual void loadTicketsKeys(const std::string& file) { @@ -233,10 +233,10 @@ class TCPIOHandler public: enum class Type : uint8_t { Client, Server }; - TCPIOHandler(const std::string& host, int socket, const struct timeval& timeout, std::shared_ptr ctx, time_t now): d_socket(socket) + TCPIOHandler(const std::string& host, bool hostIsAddr, int socket, const struct timeval& timeout, std::shared_ptr ctx, time_t now): d_socket(socket) { if (ctx) { - d_conn = ctx->getClientConnection(host, d_socket, timeout); + d_conn = ctx->getClientConnection(host, hostIsAddr, d_socket, timeout); } } diff --git a/pdns/ws-recursor.cc b/pdns/ws-recursor.cc index 17f0b85369..f3372e9c38 100644 --- a/pdns/ws-recursor.cc +++ b/pdns/ws-recursor.cc @@ -1374,7 +1374,7 @@ void AsyncWebServer::serveConnection(std::shared_ptr client) const g_networkTimeoutMsec / 1000, static_cast(g_networkTimeoutMsec) % 1000 * 1000 }; std::shared_ptr tlsCtx{nullptr}; - auto handler = std::make_shared("", client->releaseHandle(), timeout, tlsCtx, time(nullptr)); + auto handler = std::make_shared("", false, client->releaseHandle(), timeout, tlsCtx, time(nullptr)); PacketBuffer data; try {