[#1662] Checkpoint: addressing comments

configure.ac

@@ -1403,6 +1403,20 @@ AM_CONDITIONAL(KEA_SHELL, test x$enable_shell != xno)
 AC_SUBST(DISTCHECK_KEA_SHELL_CONFIGURE_FLAG)
 AC_SUBST(PKGPYTHONDIR)
 
+# Decide if the control agent TLS test can work.
+ca_tls_test=no
+if test "x$enable_shell" != "xno"; then
+  major="${PYTHON_VERSION:0:1}"
+  if test "x$major" = "x3"; then
+    if test "x${CRYPTO_NAME}" = "xOpenSSL"; then
+      ca_tls_test=yes
+    fi
+  else
+    AC_MSG_WARN("python2 kea-shell does not support HTTPS")
+  fi
+fi
+AM_CONDITIONAL(CA_TLS_TEST, test x$ca_tls_test != xno)
+
 AC_ARG_WITH([sphinx],
   AC_HELP_STRING([--with-sphinx=PATH], [path to sphinx-build tool]),
   [sphinx_path="$withval"])

src/bin/agent/ca_messages.cc

@@ -13,6 +13,7 @@ extern const isc::log::MessageID CTRL_AGENT_COMMAND_FORWARD_FAILED = "CTRL_AGENT
 extern const isc::log::MessageID CTRL_AGENT_CONFIG_CHECK_FAIL = "CTRL_AGENT_CONFIG_CHECK_FAIL";
 extern const isc::log::MessageID CTRL_AGENT_CONFIG_FAIL = "CTRL_AGENT_CONFIG_FAIL";
 extern const isc::log::MessageID CTRL_AGENT_FAILED = "CTRL_AGENT_FAILED";
+extern const isc::log::MessageID CTRL_AGENT_HTTPS_SERVICE_STARTED = "CTRL_AGENT_HTTPS_SERVICE_STARTED";
 extern const isc::log::MessageID CTRL_AGENT_HTTP_SERVICE_STARTED = "CTRL_AGENT_HTTP_SERVICE_STARTED";
 extern const isc::log::MessageID CTRL_AGENT_RUN_EXIT = "CTRL_AGENT_RUN_EXIT";
 extern const isc::log::MessageID CTRL_AGENT_STARTED = "CTRL_AGENT_STARTED";
@@ -29,6 +30,7 @@ const char* values[] = {
     "CTRL_AGENT_CONFIG_CHECK_FAIL", "Control Agent configuration check failed: %1",
     "CTRL_AGENT_CONFIG_FAIL", "Control Agent configuration failed: %1",
     "CTRL_AGENT_FAILED", "application experienced a fatal error: %1",
+    "CTRL_AGENT_HTTPS_SERVICE_STARTED", "HTTPS service bound to address %1:%2",
     "CTRL_AGENT_HTTP_SERVICE_STARTED", "HTTP service bound to address %1:%2",
     "CTRL_AGENT_RUN_EXIT", "application is exiting the event loop",
     "CTRL_AGENT_STARTED", "Kea Control Agent version %1 started",

src/bin/agent/ca_messages.h

@@ -14,6 +14,7 @@ extern const isc::log::MessageID CTRL_AGENT_COMMAND_FORWARD_FAILED;
 extern const isc::log::MessageID CTRL_AGENT_CONFIG_CHECK_FAIL;
 extern const isc::log::MessageID CTRL_AGENT_CONFIG_FAIL;
 extern const isc::log::MessageID CTRL_AGENT_FAILED;
+extern const isc::log::MessageID CTRL_AGENT_HTTPS_SERVICE_STARTED;
 extern const isc::log::MessageID CTRL_AGENT_HTTP_SERVICE_STARTED;
 extern const isc::log::MessageID CTRL_AGENT_RUN_EXIT;
 extern const isc::log::MessageID CTRL_AGENT_STARTED;

src/bin/agent/ca_messages.mes

@@ -1,4 +1,4 @@
-# Copyright (C) 2016-2018 Internet Systems Consortium, Inc. ("ISC")
+# Copyright (C) 2016-2021 Internet Systems Consortium, Inc. ("ISC")
 #
 # This Source Code Form is subject to the terms of the Mozilla Public
 # License, v. 2.0. If a copy of the MPL was not distributed with this
@@ -33,6 +33,11 @@ in earlier log entries, possibly on lower levels.
 This is a fatal error message issued when the Control Agent application
 encounters an unrecoverable error from within the event loop.
 
+% CTRL_AGENT_HTTPS_SERVICE_STARTED HTTPS service bound to address %1:%2
+This informational message indicates that the server has started HTTPS service
+on the specified address and port. All control commands should be sent to this
+address and port over a TLS channel.
+
 % CTRL_AGENT_HTTP_SERVICE_STARTED HTTP service bound to address %1:%2
 This informational message indicates that the server has started HTTP service
 on the specified address and port. All control commands should be sent to this

src/bin/agent/ca_process.cc

@@ -134,6 +134,7 @@ CtrlAgentProcess::configure(isc::data::ConstElementPtr config_set,
         }
 
         uint16_t server_port = ctx->getHttpPort();
+        bool use_https = false;
 
         // Only open a new listener if the configuration has changed.
         if (http_listeners_.empty() ||
@@ -149,6 +150,7 @@ CtrlAgentProcess::configure(isc::data::ConstElementPtr config_set,
                                       ctx->getCertFile(),
                                       ctx->getKeyFile(),
                                       ctx->getCertRequired());
+                use_https = true;
             }
 
             // Create response creator factory first. It will be used to
@@ -175,9 +177,13 @@ CtrlAgentProcess::configure(isc::data::ConstElementPtr config_set,
         }
 
         // Ok, seems we're good to go.
-        LOG_INFO(agent_logger, CTRL_AGENT_HTTP_SERVICE_STARTED)
+        if (use_https) {
-            .arg(server_address.toText()).arg(server_port);
+          LOG_INFO(agent_logger, CTRL_AGENT_HTTPS_SERVICE_STARTED)
-
+              .arg(server_address.toText()).arg(server_port);
+        } else {
+            LOG_INFO(agent_logger, CTRL_AGENT_HTTP_SERVICE_STARTED)
+                .arg(server_address.toText()).arg(server_port);
+        }
     });
 
     int rcode = 0;

src/bin/agent/tests/Makefile.am

@@ -1,7 +1,10 @@
 SHTESTS =
 # The test of dynamic reconfiguration based on signals will work only
 # if we are using file based configuration approach.
-SHTESTS += ca_process_tests.sh #tls_ca_process_tests.sh
+SHTESTS += ca_process_tests.sh
+if CA_TLS_TEST
+SHTESTS += #tls_ca_process_tests.sh
+endif
 
 noinst_SCRIPTS = ca_process_tests.sh tls_ca_process_tests.sh
 

src/lib/asiolink/common_tls.cc

@@ -43,12 +43,32 @@ TlsContextBase::configure(TlsContextPtr& context,
         context.reset(new TlsContext(role));
         context->setCertRequired(cert_required);
         if (isDir(ca_file)) {
-            context->loadCaPath(ca_file);
+            try {
+                context->loadCaPath(ca_file);
+            } catch (const std::exception& ex) {
+                isc_throw(isc::BadValue, "load of CA directory '"
+                          << ca_file << "' failed: " << ex.what());
+            }
         } else {
-            context->loadCaFile(ca_file);
+            try {
+                context->loadCaFile(ca_file);
+            } catch (const std::exception& ex) {
+                isc_throw(isc::BadValue, "load of CA file '"
+                          << ca_file << "' failed: " << ex.what());
+            }
+        }
+        try {
+            context->loadCertFile(cert_file);
+        } catch (const std::exception& ex) {
+            isc_throw(isc::BadValue, "load of cert file '"
+                      << cert_file << "' failed: " << ex.what());
+        }
+        try {
+            context->loadKeyFile(key_file);
+        } catch (const std::exception& ex) {
+            isc_throw(isc::BadValue, "load of private key file '"
+                      << cert_file << "' failed: " << ex.what());
         }
-        context->loadCertFile(cert_file);
-        context->loadKeyFile(key_file);
     } catch (...) {
         context.reset();
         throw;

src/lib/asiolink/common_tls.h

@@ -65,9 +65,7 @@ public:
     /// @param key_file The private key file name.
     /// @param cert_required True if peer certificates are required,
     /// false if they are optional. This is a server specific parameter.
-    /// @throw isc::cryptolink::LibraryError on various errors from
+    /// @throw isc::BadValue on error.
-    /// the cryptographic backend.
-    /// @throw isc::BadValue when cert_required is set to false for a client.
     static void configure(TlsContextPtr& context,
                           TlsRole role,
                           const std::string& ca_file,

src/lib/asiolink/tests/tls_unittest.cc

@@ -304,10 +304,13 @@ public:
         } catch (const LibraryError& ex) {
             thrown = true;
             errmsg_ = ex.what();
+        } catch (const isc::BadValue& ex) {
+            thrown = true;
+            errmsg_ = ex.what();
         } catch (const exception& ex) {
             thrown = true;
             errmsg_ = ex.what();
-            ADD_FAILURE() << "expect only LibraryError exception";
+            ADD_FAILURE() << "expect only LibraryError or BadValue exception";
         }
 
         // Check the no error case.
@@ -659,10 +662,14 @@ TEST(TLSTest, configure) {
 TEST(TLSTest, configureError) {
     // The error case.
     Expecteds exps;
+    // Common part of the error message.
+    string common_error = "load of cert file '/no-such-file' failed: ";
     // Botan error.
-    exps.addThrow("I/O error: DataSource: Failure opening file /no-such-file");
+    string botan_error = "I/O error: DataSource: Failure opening file /no-such-file";
+    exps.addThrow(common_error + botan_error);
     // OpenSSL error.
-    exps.addThrow("No such file or directory");
+    string openssl_error = "No such file or directory";
+    exps.addThrow(common_error + openssl_error);
     exps.runCanThrow([] {
         TlsContextPtr ctx1;
         string ca(string(TEST_CA_DIR) + "/kea-ca.crt");