[#3694] addressed review comments

Razvan Becheriu · Razvan Becheriu · commit 1569148fa309 · 2025-02-11T11:05:17.000+02:00
diff --git a/ChangeLog b/ChangeLog
@@ -1,7 +1,8 @@
 2324.	[func]		razvan
 	It is not necessary to restart the server to apply changes in the
 	TLS configuration. Running the "config-reload" command is
-	sufficient.
+	sufficient. A different address or port must be specified when
+	switching from HTTP to HTTPS or from HTTPS to HTTP.
 	(Gitlab #3694)
 
 Kea 2.7.6 (development) released on January 29, 2025
diff --git a/doc/sphinx/arm/agent.rst b/doc/sphinx/arm/agent.rst
@@ -259,8 +259,13 @@ Configuring only one or two string parameters results in an error.
    mutually authenticated, but there is no proof they are the same as
    for the HTTP authentication.
 
+The server will issue an error when changing the socket type from HTTP to HTTPS
+or from HTTPS to HTTP using the same address and port. This action is not
+allowed as it might introduce a security issue accidentally caused by a user
+mistake.
 A different address or port must be specified when using the "config-set"
-command to switch from HTTP to HTTPS or from HTTPS to HTTP.
+command to switch from HTTP to HTTPS or from HTTPS to HTTP. The same applies
+when modyfying the configuration file and then running "config-reload" command.
 
 The :iscman:`kea-shell` tool also supports TLS.
 
diff --git a/doc/sphinx/arm/ddns.rst b/doc/sphinx/arm/ddns.rst
@@ -428,8 +428,13 @@ password, these values can be read from files. The syntax is extended by:
 
 Since Kea-2.7.6 Kea supports multiple HTTP/HTTPS connections.
 Both IPv4 and IPv6 addresses can be used.
+The server will issue an error when changing the socket type from HTTP to HTTPS
+or from HTTPS to HTTP using the same address and port. This action is not
+allowed as it might introduce a security issue accidentally caused by a user
+mistake.
 A different address or port must be specified when using the "config-set"
-command to switch from HTTP to HTTPS or from HTTPS to HTTP.
+command to switch from HTTP to HTTPS or from HTTPS to HTTP. The same applies
+when modyfying the configuration file and then running "config-reload" command.
 
 When files are used, they are read when the configuration is loaded,
 to detect configuration errors as soon as possible.
diff --git a/doc/sphinx/arm/dhcp4-srv.rst b/doc/sphinx/arm/dhcp4-srv.rst
@@ -7922,8 +7922,13 @@ password, these values can be read from files. The syntax is extended by:
 
 Since Kea-2.7.6 Kea supports multiple HTTP/HTTPS connections.
 Both IPv4 and IPv6 addresses can be used.
+The server will issue an error when changing the socket type from HTTP to HTTPS
+or from HTTPS to HTTP using the same address and port. This action is not
+allowed as it might introduce a security issue accidentally caused by a user
+mistake.
 A different address or port must be specified when using the "config-set"
-command to switch from HTTP to HTTPS or from HTTPS to HTTP.
+command to switch from HTTP to HTTPS or from HTTPS to HTTP. The same applies
+when modyfying the configuration file and then running "config-reload" command.
 
 When files are used, they are read when the configuration is loaded,
 to detect configuration errors as soon as possible.
diff --git a/doc/sphinx/arm/dhcp6-srv.rst b/doc/sphinx/arm/dhcp6-srv.rst
@@ -7723,8 +7723,13 @@ password, these values can be read from files. The syntax is extended by:
 
 Since Kea-2.7.6 Kea supports multiple HTTP/HTTPS connections.
 Both IPv4 and IPv6 addresses can be used.
+The server will issue an error when changing the socket type from HTTP to HTTPS
+or from HTTPS to HTTP using the same address and port. This action is not
+allowed as it might introduce a security issue accidentally caused by a user
+mistake.
 A different address or port must be specified when using the "config-set"
-command to switch from HTTP to HTTPS or from HTTPS to HTTP.
+command to switch from HTTP to HTTPS or from HTTPS to HTTP. The same applies
+when modyfying the configuration file and then running "config-reload" command.
 
 When files are used, they are read when the configuration is loaded,
 to detect configuration errors as soon as possible.
diff --git a/src/bin/agent/ca_messages.cc b/src/bin/agent/ca_messages.cc
@@ -20,6 +20,7 @@ extern const isc::log::MessageID CTRL_AGENT_HTTPS_SERVICE_STARTED = "CTRL_AGENT_
 extern const isc::log::MessageID CTRL_AGENT_HTTPS_SERVICE_UPDATED = "CTRL_AGENT_HTTPS_SERVICE_UPDATED";
 extern const isc::log::MessageID CTRL_AGENT_HTTP_SERVICE_REUSE_FAILED = "CTRL_AGENT_HTTP_SERVICE_REUSE_FAILED";
 extern const isc::log::MessageID CTRL_AGENT_HTTP_SERVICE_STARTED = "CTRL_AGENT_HTTP_SERVICE_STARTED";
+extern const isc::log::MessageID CTRL_AGENT_HTTP_SERVICE_UPDATED = "CTRL_AGENT_HTTP_SERVICE_UPDATED";
 extern const isc::log::MessageID CTRL_AGENT_RUN_EXIT = "CTRL_AGENT_RUN_EXIT";
 extern const isc::log::MessageID CTRL_AGENT_STARTED = "CTRL_AGENT_STARTED";
 
@@ -37,11 +38,12 @@ const char* values[] = {
     "CTRL_AGENT_CONFIG_FAIL", "Control Agent configuration failed: %1",
     "CTRL_AGENT_CONFIG_SYNTAX_WARNING", "Control Agent configuration syntax warning: %1",
     "CTRL_AGENT_FAILED", "application experienced a fatal error: %1",
-    "CTRL_AGENT_HTTPS_SERVICE_REUSE_FAILED", "failed to reuse HTTPS service bound to address %1:%2",
-    "CTRL_AGENT_HTTPS_SERVICE_STARTED", "HTTPS service bound to address %1:%2",
-    "CTRL_AGENT_HTTPS_SERVICE_UPDATED", "reused HTTPS service bound to address %1:%2 and updated TLS settings",
-    "CTRL_AGENT_HTTP_SERVICE_REUSE_FAILED", "failed to reused HTTP service bound to address %1:%2",
-    "CTRL_AGENT_HTTP_SERVICE_STARTED", "HTTP service bound to address %1:%2",
+    "CTRL_AGENT_HTTPS_SERVICE_REUSE_FAILED", "failed to reuse HTTPS service bound to address: %1 port: %2",
+    "CTRL_AGENT_HTTPS_SERVICE_STARTED", "HTTPS service bound to address: %1 port: %2",
+    "CTRL_AGENT_HTTPS_SERVICE_UPDATED", "reused HTTPS service bound to address: %1 port: %2 and updated TLS settings",
+    "CTRL_AGENT_HTTP_SERVICE_REUSE_FAILED", "failed to reused HTTP service bound to address: %1 port: %2",
+    "CTRL_AGENT_HTTP_SERVICE_STARTED", "HTTP service bound to address: %1 port: %2",
+    "CTRL_AGENT_HTTP_SERVICE_UPDATED", "reused HTTP service bound to address: %1 port: %2",
     "CTRL_AGENT_RUN_EXIT", "application is exiting the event loop",
     "CTRL_AGENT_STARTED", "Kea Control Agent version %1 started",
     NULL
diff --git a/src/bin/agent/ca_messages.h b/src/bin/agent/ca_messages.h
@@ -21,6 +21,7 @@ extern const isc::log::MessageID CTRL_AGENT_HTTPS_SERVICE_STARTED;
 extern const isc::log::MessageID CTRL_AGENT_HTTPS_SERVICE_UPDATED;
 extern const isc::log::MessageID CTRL_AGENT_HTTP_SERVICE_REUSE_FAILED;
 extern const isc::log::MessageID CTRL_AGENT_HTTP_SERVICE_STARTED;
+extern const isc::log::MessageID CTRL_AGENT_HTTP_SERVICE_UPDATED;
 extern const isc::log::MessageID CTRL_AGENT_RUN_EXIT;
 extern const isc::log::MessageID CTRL_AGENT_STARTED;
 
diff --git a/src/bin/agent/ca_messages.mes b/src/bin/agent/ca_messages.mes
@@ -43,31 +43,35 @@ error. The error was displayed and the configuration parsing resumed.
 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_REUSE_FAILED failed to reuse HTTPS service bound to address %1:%2
+% CTRL_AGENT_HTTPS_SERVICE_REUSE_FAILED failed to reuse HTTPS service bound to address: %1 port: %2
 This error message indicates that the server has failed reusing existing
 HTTPS service on the specified address and port. The server can not swith from
 HTTPS to HTTP sockets using the same address and port.
 
-% CTRL_AGENT_HTTPS_SERVICE_STARTED HTTPS service bound to address %1:%2
+% CTRL_AGENT_HTTPS_SERVICE_STARTED HTTPS service bound to address: %1 port: %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_HTTPS_SERVICE_UPDATED reused HTTPS service bound to address %1:%2 and updated TLS settings
+% CTRL_AGENT_HTTPS_SERVICE_UPDATED reused HTTPS service bound to address: %1 port: %2 and updated TLS settings
 This informational message indicates that the server has reused existing
 HTTPS service on the specified address and port. Note that any change in
 the TLS setup has been applied.
 
-% CTRL_AGENT_HTTP_SERVICE_REUSE_FAILED failed to reused HTTP service bound to address %1:%2
+% CTRL_AGENT_HTTP_SERVICE_REUSE_FAILED failed to reused HTTP service bound to address: %1 port: %2
 This error message indicates that the server has failed reusing existing
 HTTP service on the specified address and port. The server can not swith from
 HTTP to HTTPS sockets using the same address and port.
 
-% CTRL_AGENT_HTTP_SERVICE_STARTED HTTP service bound to address %1:%2
+% CTRL_AGENT_HTTP_SERVICE_STARTED HTTP service bound to address: %1 port: %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
 address and port.
 
+% CTRL_AGENT_HTTP_SERVICE_UPDATED reused HTTP service bound to address: %1 port: %2
+This informational message indicates that the server has reused existing
+HTTP service on the specified address and port.
+
 % CTRL_AGENT_RUN_EXIT application is exiting the event loop
 Logged at debug log level 0.
 This is a debug message issued when the Control Agent exits its
diff --git a/src/bin/agent/ca_process.cc b/src/bin/agent/ca_process.cc
@@ -162,18 +162,27 @@ CtrlAgentProcess::configure(isc::data::ConstElementPtr config_set,
                         // Overwrite the authentication setup and the http headers in the response creator config.
                         it->second->config_->setAuthConfig(ctx->getAuthConfig());
                         it->second->config_->setHttpHeaders(ctx->getHttpHeaders());
-                        getIOService()->post([listener, tls_context]() { listener->setTlsContext(tls_context); });
+                        listener->setTlsContext(tls_context);
                         LOG_INFO(agent_logger, CTRL_AGENT_HTTPS_SERVICE_UPDATED)
                             .arg(server_address.toText())
                             .arg(server_port);
                     }
-                } else if (!ctx->getTrustAnchor().empty()) {
-                    // Can not switch from HTTP to HTTPS
-                    LOG_ERROR(agent_logger, CTRL_AGENT_HTTP_SERVICE_REUSE_FAILED)
-                        .arg(server_address.toText())
-                        .arg(server_port);
-                    isc_throw(BadValue,
-                              "Can not switch from HTTP to HTTPS sockets using the same address and port.");
+                } else {
+                    if (!ctx->getTrustAnchor().empty()) {
+                        // Can not switch from HTTP to HTTPS
+                        LOG_ERROR(agent_logger, CTRL_AGENT_HTTP_SERVICE_REUSE_FAILED)
+                            .arg(server_address.toText())
+                            .arg(server_port);
+                        isc_throw(BadValue,
+                                  "Can not switch from HTTP to HTTPS sockets using the same address and port.");
+                    } else {
+                        // Overwrite the authentication setup and the http headers in the response creator config.
+                        it->second->config_->setAuthConfig(ctx->getAuthConfig());
+                        it->second->config_->setHttpHeaders(ctx->getHttpHeaders());
+                        LOG_INFO(agent_logger, CTRL_AGENT_HTTP_SERVICE_UPDATED)
+                            .arg(server_address.toText())
+                            .arg(server_port);
+                    }
                 }
             }
             // If the connection can be reused, mark it as usable.
diff --git a/src/bin/agent/tests/ca_controller_unittests.cc b/src/bin/agent/tests/ca_controller_unittests.cc
@@ -421,6 +421,9 @@ TEST_F(CtrlAgentControllerTest, noListenerChangeHttp) {
         "  }"
         "}";
 
+    // Pointer used to store the listener instance. It is set after 50 ms the
+    // process has started (using a timer) and it's value is checked on server
+    // shutdown using the callback.
     const HttpListener* listener_ptr = 0;
 
     // This check callback is called before the shutdown.
@@ -522,6 +525,9 @@ TEST_F(CtrlAgentControllerTest, noListenerChangeHttps) {
              << "  }"
              << "}";
 
+    // Pointer used to store the listener instance. It is set after 50 ms the
+    // process has started (using a timer) and it's value is checked on server
+    // shutdown using the callback.
     const HttpListener* listener_ptr = 0;
     TlsContext* context = 0;
 
@@ -607,6 +613,9 @@ TEST_F(CtrlAgentControllerTest, handleHttpToHttpsSwitch) {
              << "  }"
              << "}";
 
+    // Pointer used to store the listener instance. It is set after 50 ms the
+    // process has started (using a timer) and it's value is checked on server
+    // shutdown using the callback.
     const HttpListener* listener_ptr = 0;
 
     // This check callback is called before the shutdown.
@@ -703,6 +712,9 @@ TEST_F(CtrlAgentControllerTest, handleHttpsToHttpSwitch) {
              << "  }"
              << "}";
 
+    // Pointer used to store the listener instance. It is set after 50 ms the
+    // process has started (using a timer) and it's value is checked on server
+    // shutdown using the callback.
     const HttpListener* listener_ptr = 0;
     TlsContext* context = 0;
 
diff --git a/src/lib/config/config_messages.cc b/src/lib/config/config_messages.cc
@@ -37,6 +37,7 @@ extern const isc::log::MessageID COMMAND_WATCH_SOCKET_MARK_READY_ERROR = "COMMAN
 extern const isc::log::MessageID HTTP_COMMAND_MGR_HTTPS_SERVICE_REUSE_FAILED = "HTTP_COMMAND_MGR_HTTPS_SERVICE_REUSE_FAILED";
 extern const isc::log::MessageID HTTP_COMMAND_MGR_HTTPS_SERVICE_UPDATED = "HTTP_COMMAND_MGR_HTTPS_SERVICE_UPDATED";
 extern const isc::log::MessageID HTTP_COMMAND_MGR_HTTP_SERVICE_REUSE_FAILED = "HTTP_COMMAND_MGR_HTTP_SERVICE_REUSE_FAILED";
+extern const isc::log::MessageID HTTP_COMMAND_MGR_HTTP_SERVICE_UPDATED = "HTTP_COMMAND_MGR_HTTP_SERVICE_UPDATED";
 extern const isc::log::MessageID HTTP_COMMAND_MGR_SERVICE_STARTED = "HTTP_COMMAND_MGR_SERVICE_STARTED";
 extern const isc::log::MessageID HTTP_COMMAND_MGR_SERVICE_STOPPING = "HTTP_COMMAND_MGR_SERVICE_STOPPING";
 
@@ -50,9 +51,9 @@ const char* values[] = {
     "COMMAND_DEREGISTERED", "Command %1 deregistered",
     "COMMAND_EXTENDED_REGISTERED", "Command %1 registered",
     "COMMAND_HTTP_LISTENER_COMMAND_REJECTED", "Command HTTP listener rejected command '%1' from '%2'",
-    "COMMAND_HTTP_LISTENER_STARTED", "Command HTTP listener started with %1 threads, listening on %2:%3, use TLS: %4",
-    "COMMAND_HTTP_LISTENER_STOPPED", "Command HTTP listener for %1:%2 stopped.",
-    "COMMAND_HTTP_LISTENER_STOPPING", "Stopping Command HTTP listener for %1:%2",
+    "COMMAND_HTTP_LISTENER_STARTED", "Command HTTP listener started with %1 threads, listening on address: %2 port: %3, use TLS: %4",
+    "COMMAND_HTTP_LISTENER_STOPPED", "Command HTTP listener for address: %1 port: %2 stopped.",
+    "COMMAND_HTTP_LISTENER_STOPPING", "Stopping Command HTTP listener for address: %1 port: %2",
     "COMMAND_PROCESS_ERROR1", "Error while processing command: %1",
     "COMMAND_PROCESS_ERROR2", "Error while processing command: %1",
     "COMMAND_RECEIVED", "Received command '%1'",
@@ -73,10 +74,11 @@ const char* values[] = {
     "COMMAND_WATCH_SOCKET_CLEAR_ERROR", "watch socket failed to clear: %1",
     "COMMAND_WATCH_SOCKET_CLOSE_ERROR", "watch socket failed to close: %1",
     "COMMAND_WATCH_SOCKET_MARK_READY_ERROR", "watch socket failed to mark ready: %1",
-    "HTTP_COMMAND_MGR_HTTPS_SERVICE_REUSE_FAILED", "failed to reused HTTPS service bound to address %1:%2",
-    "HTTP_COMMAND_MGR_HTTPS_SERVICE_UPDATED", "reused HTTPS service bound to address %1:%2 and updated TLS settings",
-    "HTTP_COMMAND_MGR_HTTP_SERVICE_REUSE_FAILED", "failed to reused HTTP service bound to address %1:%2",
-    "HTTP_COMMAND_MGR_SERVICE_STARTED", "started %1 service bound to address %2 port %3",
+    "HTTP_COMMAND_MGR_HTTPS_SERVICE_REUSE_FAILED", "failed to reused HTTPS service bound to address: %1 port: %2",
+    "HTTP_COMMAND_MGR_HTTPS_SERVICE_UPDATED", "reused HTTPS service bound to address: %1 port: %2 and updated TLS settings",
+    "HTTP_COMMAND_MGR_HTTP_SERVICE_REUSE_FAILED", "failed to reused HTTP service bound to address: %1 port: %2",
+    "HTTP_COMMAND_MGR_HTTP_SERVICE_UPDATED", "reused HTTP service bound to address: %1 port: %2",
+    "HTTP_COMMAND_MGR_SERVICE_STARTED", "started %1 service bound to address: %2 port: %3",
     "HTTP_COMMAND_MGR_SERVICE_STOPPING", "Server is stopping %1 service %2",
     NULL
 };
diff --git a/src/lib/config/config_messages.h b/src/lib/config/config_messages.h
@@ -38,6 +38,7 @@ extern const isc::log::MessageID COMMAND_WATCH_SOCKET_MARK_READY_ERROR;
 extern const isc::log::MessageID HTTP_COMMAND_MGR_HTTPS_SERVICE_REUSE_FAILED;
 extern const isc::log::MessageID HTTP_COMMAND_MGR_HTTPS_SERVICE_UPDATED;
 extern const isc::log::MessageID HTTP_COMMAND_MGR_HTTP_SERVICE_REUSE_FAILED;
+extern const isc::log::MessageID HTTP_COMMAND_MGR_HTTP_SERVICE_UPDATED;
 extern const isc::log::MessageID HTTP_COMMAND_MGR_SERVICE_STARTED;
 extern const isc::log::MessageID HTTP_COMMAND_MGR_SERVICE_STOPPING;
 
diff --git a/src/lib/config/config_messages.mes b/src/lib/config/config_messages.mes
@@ -30,20 +30,20 @@ Logged at debug log level 10.
 This debug messages is issued when a command is rejected. Arguments detail
 the command and the address the request was received from.
 
-% COMMAND_HTTP_LISTENER_STARTED Command HTTP listener started with %1 threads, listening on %2:%3, use TLS: %4
+% COMMAND_HTTP_LISTENER_STARTED Command HTTP listener started with %1 threads, listening on address: %2 port: %3, use TLS: %4
 Logged at debug log level 10.
 This debug messages is issued when an HTTP listener has been started to
 accept connections from Command API clients through which commands can be
 received and responses sent.  Arguments detail the number of threads
 that the listener is using, the address and port at which it is listening,
 and if HTTPS/TLS is used or not.
 
-% COMMAND_HTTP_LISTENER_STOPPED Command HTTP listener for %1:%2 stopped.
+% COMMAND_HTTP_LISTENER_STOPPED Command HTTP listener for address: %1 port: %2 stopped.
 Logged at debug log level 10.
 This debug messages is issued when the Command HTTP listener, listening
 at the given address and port, has completed shutdown.
 
-% COMMAND_HTTP_LISTENER_STOPPING Stopping Command HTTP listener for %1:%2
+% COMMAND_HTTP_LISTENER_STOPPING Stopping Command HTTP listener for address: %1 port: %2
 Logged at debug log level 10.
 This debug messages is issued when the Command HTTP listener, listening
 at the given address and port, has begun to shutdown.
@@ -153,22 +153,26 @@ ready status after scheduling asynchronous send. This is programmatic error
 that should be reported. The command manager may or may not continue
 to operate correctly.
 
-% HTTP_COMMAND_MGR_HTTPS_SERVICE_REUSE_FAILED failed to reused HTTPS service bound to address %1:%2
+% HTTP_COMMAND_MGR_HTTPS_SERVICE_REUSE_FAILED failed to reused HTTPS service bound to address: %1 port: %2
 This error message indicates that the server has failed reusing existing
 HTTPS service on the specified address and port. The server can not swith from
 HTTPS to HTTP sockets using the same address and port.
 
-% HTTP_COMMAND_MGR_HTTPS_SERVICE_UPDATED reused HTTPS service bound to address %1:%2 and updated TLS settings
+% HTTP_COMMAND_MGR_HTTPS_SERVICE_UPDATED reused HTTPS service bound to address: %1 port: %2 and updated TLS settings
 This informational message indicates that the server has reused existing
 HTTPS service on the specified address and port. Note that any change in
 the TLS setup has been applied.
 
-% HTTP_COMMAND_MGR_HTTP_SERVICE_REUSE_FAILED failed to reused HTTP service bound to address %1:%2
+% HTTP_COMMAND_MGR_HTTP_SERVICE_UPDATED reused HTTP service bound to address: %1 port: %2
+This informational message indicates that the server has reused existing
+HTTP service on the specified address and port.
+
+% HTTP_COMMAND_MGR_HTTP_SERVICE_REUSE_FAILED failed to reused HTTP service bound to address: %1 port: %2
 This error message indicates that the server has failed reusing existing
 HTTP service on the specified address and port. The server can not swith from
 HTTP to HTTPS sockets using the same address and port.
 
-% HTTP_COMMAND_MGR_SERVICE_STARTED started %1 service bound to address %2 port %3
+% HTTP_COMMAND_MGR_SERVICE_STARTED started %1 service bound to address: %2 port: %3
 This informational message indicates that the server has started
 HTTP/HTTPS service on the specified address and port for receiving
 control commands.
diff --git a/src/lib/config/http_command_mgr.cc b/src/lib/config/http_command_mgr.cc
diff --git a/src/lib/config/http_command_mgr.h b/src/lib/config/http_command_mgr.h
diff --git a/src/lib/config/unix_command_mgr.h b/src/lib/config/unix_command_mgr.h
