dnsdist: Skip invalid OCSP files after issuing a warning

Contrary to certificates and keys, OCSP files are never required to
provide a working DoT or DoH service, so it's better to start even
if would not load all, or any, OCSP files.

diff --git a/pdns/dnsdistdist/doh.cc b/pdns/dnsdistdist/doh.cc
index 799462898a6a9bf8efc79e78a7984d7620d93ff4..6912b79fa083b65ba5b25eb0a415389d1ba8d1a6 100644 (file)
--- a/pdns/dnsdistdist/doh.cc
+++ b/pdns/dnsdistdist/doh.cc
@@ -1425,7 +1425,10 @@ static void setupTLSContext(DOHAcceptContext& acceptCtx,
     tlsConfig.d_ciphers = DOH_DEFAULT_CIPHERS;
   }
 
-  auto ctx = libssl_init_server_context(tlsConfig, acceptCtx.d_ocspResponses);
+  auto [ctx, warnings] = libssl_init_server_context(tlsConfig, acceptCtx.d_ocspResponses);
+  for (const auto& warning : warnings) {
+    warnlog("%s", warning);
+  }
 
   if (tlsConfig.d_enableTickets && tlsConfig.d_numberOfTicketsKeys > 0) {
     acceptCtx.d_ticketKeys = std::make_unique<OpenSSLTLSTicketKeysRing>(tlsConfig.d_numberOfTicketsKeys);

diff --git a/pdns/dolog.hh b/pdns/dolog.hh
index 1767cd4c03810995e735526871c82c2059f6f162..a28777ab810618f0ba3291ecd2f90526b2b74794 100644 (file)
--- a/pdns/dolog.hh
+++ b/pdns/dolog.hh
@@ -209,4 +209,3 @@ void errlog(const char* s, Args... args)
 }
 
 #endif
-

diff --git a/pdns/libssl.cc b/pdns/libssl.cc
index e9fcba6cefc3ba77219c3670194389a77759e41a..2966e2b1b4f42f1c661db29ee8156b48ad480ef3 100644 (file)
--- a/pdns/libssl.cc
+++ b/pdns/libssl.cc
@@ -396,7 +396,7 @@ static bool libssl_validate_ocsp_response(const std::string& response)
   return true;
 }
 
-std::map<int, std::string> libssl_load_ocsp_responses(const std::vector<std::string>& ocspFiles, std::vector<int> keyTypes)
+static std::map<int, std::string> libssl_load_ocsp_responses(const std::vector<std::string>& ocspFiles, std::vector<int> keyTypes, std::vector<std::string>& warnings)
 {
   std::map<int, std::string> ocspResponses;
 
@@ -408,12 +408,13 @@ std::map<int, std::string> libssl_load_ocsp_responses(const std::vector<std::str
   for (const auto& filename : ocspFiles) {
     std::ifstream file(filename, std::ios::binary);
     std::string content;
-    while(file) {
+    while (file) {
       char buffer[4096];
       file.read(buffer, sizeof(buffer));
       if (file.bad()) {
         file.close();
-        throw std::runtime_error("Unable to load OCSP response from '" + filename + "'");
+        warnings.push_back("Unable to load OCSP response from " + filename);
+        continue;
       }
       content.append(buffer, file.gcount());
     }
@@ -424,7 +425,8 @@ std::map<int, std::string> libssl_load_ocsp_responses(const std::vector<std::str
       ocspResponses.insert({keyTypes.at(count), std::move(content)});
     }
     catch (const std::exception& e) {
-      throw std::runtime_error("Error checking the validity of OCSP response from '" + filename + "': " + e.what());
+      warnings.push_back("Error checking the validity of OCSP response from '" + filename + "': " + e.what());
+      continue;
     }
     ++count;
   }
@@ -801,9 +803,10 @@ bool OpenSSLTLSTicketKey::decrypt(const unsigned char* iv, EVP_CIPHER_CTX* ectx,
   return true;
 }
 
-std::unique_ptr<SSL_CTX, void(*)(SSL_CTX*)> libssl_init_server_context(const TLSConfig& config,
-                                                                       std::map<int, std::string>& ocspResponses)
+std::pair<std::unique_ptr<SSL_CTX, void(*)(SSL_CTX*)>, std::vector<std::string>> libssl_init_server_context(const TLSConfig& config,
+                                                                                                            std::map<int, std::string>& ocspResponses)
 {
+  std::vector<std::string> warnings;
   auto ctx = std::unique_ptr<SSL_CTX, decltype(&SSL_CTX_free)>(SSL_CTX_new(SSLv23_server_method()), SSL_CTX_free);
 
   if (!ctx) {
@@ -881,7 +884,7 @@ std::unique_ptr<SSL_CTX, void(*)(SSL_CTX*)> libssl_init_server_context(const TLS
 #ifdef SSL_MODE_ASYNC
     mode |= SSL_MODE_ASYNC;
 #else
-    cerr<<"Warning: TLS async mode requested but not supported"<<endl;
+    warnings.push_back("Warning: TLS async mode requested but not supported");
 #endif
   }
 
@@ -970,7 +973,7 @@ std::unique_ptr<SSL_CTX, void(*)(SSL_CTX*)> libssl_init_server_context(const TLS
 #ifndef DISABLE_OCSP_STAPLING
   if (!config.d_ocspFiles.empty()) {
     try {
-      ocspResponses = libssl_load_ocsp_responses(config.d_ocspFiles, keyTypes);
+      ocspResponses = libssl_load_ocsp_responses(config.d_ocspFiles, keyTypes, warnings);
     }
     catch(const std::exception& e) {
       throw std::runtime_error("Unable to load OCSP responses: " + std::string(e.what()));
@@ -988,7 +991,7 @@ std::unique_ptr<SSL_CTX, void(*)(SSL_CTX*)> libssl_init_server_context(const TLS
   }
 #endif /* HAVE_SSL_CTX_SET_CIPHERSUITES */
 
-  return ctx;
+  return std::make_pair(std::move(ctx), std::move(warnings));
 }
 
 #ifdef HAVE_SSL_CTX_SET_KEYLOG_CALLBACK

diff --git a/pdns/libssl.hh b/pdns/libssl.hh
index d8af451112029fc9058b63e50bc31ccd98834618..ad6bcd8133213753cd21e48ca966b116be4970b5 100644 (file)
--- a/pdns/libssl.hh
+++ b/pdns/libssl.hh
@@ -133,9 +133,6 @@ int libssl_ticket_key_callback(SSL* s, OpenSSLTLSTicketKeysRing& keyring, unsign
 
 #ifndef DISABLE_OCSP_STAPLING
 int libssl_ocsp_stapling_callback(SSL* ssl, const std::map<int, std::string>& ocspMap);
-
-std::map<int, std::string> libssl_load_ocsp_responses(const std::vector<std::string>& ocspFiles, std::vector<int> keyTypes);
-
 #ifdef HAVE_OCSP_BASIC_SIGN
 bool libssl_generate_ocsp_response(const std::string& certFile, const std::string& caCert, const std::string& caKey, const std::string& outFile, int ndays, int nmin);
 #endif
@@ -147,8 +144,10 @@ LibsslTLSVersion libssl_tls_version_from_string(const std::string& str);
 const std::string& libssl_tls_version_to_string(LibsslTLSVersion version);
 bool libssl_set_min_tls_version(std::unique_ptr<SSL_CTX, void(*)(SSL_CTX*)>& ctx, LibsslTLSVersion version);
 
-std::unique_ptr<SSL_CTX, void(*)(SSL_CTX*)> libssl_init_server_context(const TLSConfig& config,
-                                                                       std::map<int, std::string>& ocspResponses);
+/* return the created context, and a list of warning messages for issues not severe enough
+   to trigger raising an exception, like failing to load an OCSP response file */
+std::pair<std::unique_ptr<SSL_CTX, void(*)(SSL_CTX*)>, std::vector<std::string>> libssl_init_server_context(const TLSConfig& config,
+                                                                                                            std::map<int, std::string>& ocspResponses);
 
 std::unique_ptr<FILE, int(*)(FILE*)> libssl_set_key_log_file(std::unique_ptr<SSL_CTX, void(*)(SSL_CTX*)>& ctx, const std::string& logFile);
 

diff --git a/pdns/tcpiohandler.cc b/pdns/tcpiohandler.cc
index fd3a50b7c9f86273a40aa4ff081b0de33cdf59d4..f2817bd90279fabb985d42fe6ae1e35838701694 100644 (file)
--- a/pdns/tcpiohandler.cc
+++ b/pdns/tcpiohandler.cc
@@ -30,7 +30,12 @@ public:
   {
     registerOpenSSLUser();
 
-    d_tlsCtx = libssl_init_server_context(tlsConfig, d_ocspResponses);
+    auto [ctx, warnings] = libssl_init_server_context(tlsConfig, d_ocspResponses);
+    for (const auto& warning : warnings) {
+      warnlog("%s", warning);
+    }
+    d_tlsCtx = std::move(ctx);
+
     if (!d_tlsCtx) {
       ERR_print_errors_fp(stderr);
       throw std::runtime_error("Error creating TLS context on " + addr.toStringWithPort());
@@ -1565,7 +1570,7 @@ public:
     for (const auto& file : fe.d_tlsConfig.d_ocspFiles) {
       rc = gnutls_certificate_set_ocsp_status_request_file(d_creds.get(), file.c_str(), count);
       if (rc != GNUTLS_E_SUCCESS) {
-        throw std::runtime_error("Error loading OCSP response from file '" + file + "' for certificate ('" + fe.d_tlsConfig.d_certKeyPairs.at(count).d_cert + "') and key ('" + fe.d_tlsConfig.d_certKeyPairs.at(count).d_key.value() + "') for TLS context on " + fe.d_addr.toStringWithPort() + ": " + gnutls_strerror(rc));
+        warnlog("Error loading OCSP response from file '%s' for certificate ('%s') and key ('%s') for TLS context on %s: %s", file, fe.d_tlsConfig.d_certKeyPairs.at(count).d_cert, fe.d_tlsConfig.d_certKeyPairs.at(count).d_key.value(), fe.d_addr.toStringWithPort(), gnutls_strerror(rc));
       }
       ++count;
     }

diff --git a/regression-tests.dnsdist/test_OCSP.py b/regression-tests.dnsdist/test_OCSP.py
index 6b29e3df554c2b9403bdcbaa0d66d7095d19e615..a8577baea16e209a92ead763e9e455d9898fdd78 100644 (file)
--- a/regression-tests.dnsdist/test_OCSP.py
+++ b/regression-tests.dnsdist/test_OCSP.py
@@ -91,6 +91,33 @@ class TestOCSPStaplingDOH(DNSDistOCSPStaplingTest):
         self.assertTrue(serialNumber2)
         self.assertNotEqual(serialNumber, serialNumber2)
 
+class TestBrokenOCSPStaplingDoH(DNSDistOCSPStaplingTest):
+
+    _consoleKey = DNSDistTest.generateConsoleKey()
+    _consoleKeyB64 = base64.b64encode(_consoleKey).decode('ascii')
+    _serverKey = 'server.key'
+    _serverCert = 'server.chain'
+    _serverName = 'tls.tests.dnsdist.org'
+    _caCert = 'ca.pem'
+    # invalid OCSP file!
+    _ocspFile = '/dev/null'
+    _tlsServerPort = 8443
+    _config_template = """
+    newServer{address="127.0.0.1:%s"}
+    setKey("%s")
+    controlSocket("127.0.0.1:%s")
+
+    addDOHLocal("127.0.0.1:%s", "%s", "%s", { "/" }, { ocspResponses={"%s"}})
+    """
+    _config_params = ['_testServerPort', '_consoleKeyB64', '_consolePort', '_tlsServerPort', '_serverCert', '_serverKey', '_ocspFile']
+
+    def testBrokenOCSPStapling(self):
+        """
+        OCSP Stapling: Broken (DoH)
+        """
+        output = self.checkOCSPStaplingStatus('127.0.0.1', self._tlsServerPort, self._serverName, self._caCert)
+        self.assertNotIn('OCSP Response Status: successful (0x0)', output)
+
 class TestOCSPStaplingTLSGnuTLS(DNSDistOCSPStaplingTest):
 
     _consoleKey = DNSDistTest.generateConsoleKey()
@@ -134,6 +161,34 @@ class TestOCSPStaplingTLSGnuTLS(DNSDistOCSPStaplingTest):
         self.assertTrue(serialNumber2)
         self.assertNotEqual(serialNumber, serialNumber2)
 
+class TestBrokenOCSPStaplingTLSGnuTLS(DNSDistOCSPStaplingTest):
+
+    _consoleKey = DNSDistTest.generateConsoleKey()
+    _consoleKeyB64 = base64.b64encode(_consoleKey).decode('ascii')
+    _serverKey = 'server.key'
+    _serverCert = 'server.chain'
+    _serverName = 'tls.tests.dnsdist.org'
+    _caCert = 'ca.pem'
+    # invalid OCSP file!
+    _ocspFile = '/dev/null'
+    _tlsServerPort = 8443
+    _config_template = """
+    newServer{address="127.0.0.1:%s"}
+    setKey("%s")
+    controlSocket("127.0.0.1:%s")
+
+    addTLSLocal("127.0.0.1:%s", "%s", "%s", { provider="gnutls", ocspResponses={"%s"}})
+    """
+    _config_params = ['_testServerPort', '_consoleKeyB64', '_consolePort', '_tlsServerPort', '_serverCert', '_serverKey', '_ocspFile']
+
+    def testBrokenOCSPStapling(self):
+        """
+        OCSP Stapling: Broken (GnuTLS)
+        """
+        output = self.checkOCSPStaplingStatus('127.0.0.1', self._tlsServerPort, self._serverName, self._caCert)
+        self.assertNotIn('OCSP Response Status: successful (0x0)', output)
+        self.assertEquals(self.getTLSProvider(), "gnutls")
+
 class TestOCSPStaplingTLSOpenSSL(DNSDistOCSPStaplingTest):
 
     _consoleKey = DNSDistTest.generateConsoleKey()
@@ -176,3 +231,31 @@ class TestOCSPStaplingTLSOpenSSL(DNSDistOCSPStaplingTest):
         serialNumber2 = self.getOCSPSerial(output)
         self.assertTrue(serialNumber2)
         self.assertNotEqual(serialNumber, serialNumber2)
+
+class TestBrokenOCSPStaplingTLSOpenSSL(DNSDistOCSPStaplingTest):
+
+    _consoleKey = DNSDistTest.generateConsoleKey()
+    _consoleKeyB64 = base64.b64encode(_consoleKey).decode('ascii')
+    _serverKey = 'server.key'
+    _serverCert = 'server.chain'
+    _serverName = 'tls.tests.dnsdist.org'
+    _caCert = 'ca.pem'
+    # invalid OCSP file!
+    _ocspFile = '/dev/null'
+    _tlsServerPort = 8443
+    _config_template = """
+    newServer{address="127.0.0.1:%s"}
+    setKey("%s")
+    controlSocket("127.0.0.1:%s")
+
+    addTLSLocal("127.0.0.1:%s", "%s", "%s", { provider="openssl", ocspResponses={"%s"}})
+    """
+    _config_params = ['_testServerPort', '_consoleKeyB64', '_consolePort', '_tlsServerPort', '_serverCert', '_serverKey', '_ocspFile']
+
+    def testBrokenOCSPStapling(self):
+        """
+        OCSP Stapling: Broken (OpenSSL)
+        """
+        output = self.checkOCSPStaplingStatus('127.0.0.1', self._tlsServerPort, self._serverName, self._caCert)
+        self.assertNotIn('OCSP Response Status: successful (0x0)', output)
+        self.assertEquals(self.getTLSProvider(), "openssl")