[#1706] Added restrict-commands feature

diff --git a/doc/sphinx/arm/hooks-ha.rst b/doc/sphinx/arm/hooks-ha.rst
index 7e77a7003f1d337e741db8d16b14ec602d1d4cf1..3f19626b459a437c32a59276454e789d4239bc07 100644 (file)
--- a/doc/sphinx/arm/hooks-ha.rst
+++ b/doc/sphinx/arm/hooks-ha.rst
@@ -237,6 +237,10 @@ Since Kea 2.2.0 the HTTPS server side is supported:
   are required and verified, i.e. like ``cert-required``. It defaults
   to true and is a HA config (vs peer config) parameter.
 
+Kea 2.2.0 adds to a new security feature with the ``restrict-commands``
+HA config parameter: when it is configured to ``true`` (default is
+``false``) commands which are not used by the hook are rejected.
+
 Following is an example of HA server pair and Control Agent
 configuration for Hot-Standby with TLS.
 

diff --git a/src/hooks/dhcp/high_availability/command_creator.cc b/src/hooks/dhcp/high_availability/command_creator.cc
index 0b481f540a8001330e5e52f0e246c78871495392..19af10b466e642276f618ff2f093a503735873d5 100644 (file)
--- a/src/hooks/dhcp/high_availability/command_creator.cc
+++ b/src/hooks/dhcp/high_availability/command_creator.cc
@@ -1,4 +1,4 @@
-// Copyright (C) 2018-2021 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2018-2022 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
@@ -13,10 +13,26 @@
 
 using namespace isc::data;
 using namespace isc::dhcp;
+using namespace std;
 
 namespace isc {
 namespace ha {
 
+unordered_set<string> CommandCreator::ha_commands4_ = {
+    "list-commands", "status-get",
+    "dhcp-disable", "dhcp-enable", "ha-reset", "ha-heartbeat",
+    "lease4-update", "lease4-del", "lease4-get-all", "lease4-get-page",
+    "ha-maintenance-notify", "ha-sync-complete-notify"
+};
+
+unordered_set<string> CommandCreator::ha_commands6_ = {
+    "list-commands", "status-get",
+    "dhcp-disable", "dhcp-enable", "ha-reset", "ha-heartbeat",
+    "lease6-bulk-apply", "lease6-update", "lease6-del", "lease6-get-all",
+    "lease6-get-page",
+    "ha-maintenance-notify", "ha-sync-complete-notify"
+};
+
 ConstElementPtr
 CommandCreator::createDHCPDisable(const unsigned int max_period,
                                   const HAServerType& server_type) {
@@ -248,7 +264,7 @@ void
 CommandCreator::insertService(ConstElementPtr& command,
                               const HAServerType& server_type) {
     ElementPtr service = Element::createList();
-    const std::string service_name = (server_type == HAServerType::DHCPv4 ? "dhcp4" : "dhcp6");
+    const string service_name = (server_type == HAServerType::DHCPv4 ? "dhcp4" : "dhcp6");
     service->add(Element::create(service_name));
 
     // We have no better way of setting a new element here than

diff --git a/src/hooks/dhcp/high_availability/command_creator.h b/src/hooks/dhcp/high_availability/command_creator.h
index a65d575e35826f7ece6741fcaa285f043558c930..276f405df519bb12ca1d1395563ddbe077531a0f 100644 (file)
--- a/src/hooks/dhcp/high_availability/command_creator.h
+++ b/src/hooks/dhcp/high_availability/command_creator.h
@@ -1,4 +1,4 @@
-// Copyright (C) 2018-2021 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2018-2022 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
@@ -11,6 +11,7 @@
 #include <ha_server_type.h>
 #include <cc/data.h>
 #include <dhcpsrv/lease.h>
+#include <unordered_set>
 #include <string>
 
 namespace isc {
@@ -172,6 +173,12 @@ public:
     static data::ConstElementPtr
     createSyncCompleteNotify(const HAServerType& server_type);
 
+    /// @brief List of commands used by the High Availability in v4.
+    static std::unordered_set<std::string> ha_commands4_;
+
+    /// @brief List of commands used by the High Availability in v6.
+    static std::unordered_set<std::string> ha_commands6_;
+
 private:
 
     /// @brief Replaces "cltt" with "expire" value within the lease.

diff --git a/src/hooks/dhcp/high_availability/ha_config.cc b/src/hooks/dhcp/high_availability/ha_config.cc
index 3d355412e4f67a7b0b126a25ba9abef1ac4ed2c5..1e3a8c684dbf586db65d66f83167fabdd5bf8c27 100644 (file)
--- a/src/hooks/dhcp/high_availability/ha_config.cc
+++ b/src/hooks/dhcp/high_availability/ha_config.cc
@@ -167,7 +167,8 @@ HAConfig::HAConfig()
       enable_multi_threading_(false), http_dedicated_listener_(false),
       http_listener_threads_(0), http_client_threads_(0),
       trust_anchor_(), cert_file_(), key_file_(), require_client_certs_(true),
-      peers_(), state_machine_(new StateMachineConfig()) {
+      restrict_commands_(false), peers_(),
+      state_machine_(new StateMachineConfig()) {
 }
 
 HAConfig::PeerConfigPtr

diff --git a/src/hooks/dhcp/high_availability/ha_config.h b/src/hooks/dhcp/high_availability/ha_config.h
index fd7710c7ad7049f8ad256650625d2128ab3f72f9..bb4857777e22aa49b9c15c06542fb27384ca0dc0 100644 (file)
--- a/src/hooks/dhcp/high_availability/ha_config.h
+++ b/src/hooks/dhcp/high_availability/ha_config.h
@@ -680,6 +680,18 @@ public:
         require_client_certs_ = flag;
     }
 
+    /// @brief Returns restrict-commands.
+    bool getRestrictCommands() const {
+        return (restrict_commands_);
+    }
+
+    /// @brief Sets restrict-commands.
+    ///
+    /// @param flag Restrict commands to HA flag value.
+    void setRestrictCommands(bool flag) {
+        restrict_commands_ = flag;
+    }
+
     /// @brief Returns configuration of the specified server.
     ///
     /// @param name Server name.
@@ -765,6 +777,7 @@ public:
     util::Optional<std::string> cert_file_;    ///< Certificate file.
     util::Optional<std::string> key_file_;     ///< Private key file.
     bool require_client_certs_;                ///< Require client certs flag.
+    bool restrict_commands_;                   ///< Restrict commands to HA flag.
     PeerConfigMap peers_;                      ///< Map of peers' configurations.
     StateMachineConfigPtr state_machine_;      ///< State machine configuration.
 };

diff --git a/src/hooks/dhcp/high_availability/ha_config_parser.cc b/src/hooks/dhcp/high_availability/ha_config_parser.cc
index 4f44a7d0c1161304401e8ba2d3c4b0186ed9115f..8bc0aaa5ba78ed8529c77c73bf20d9782698ea15 100644 (file)
--- a/src/hooks/dhcp/high_availability/ha_config_parser.cc
+++ b/src/hooks/dhcp/high_availability/ha_config_parser.cc
@@ -32,6 +32,7 @@ const SimpleDefaults HA_CONFIG_DEFAULTS = {
     { "max-response-delay",      Element::integer, "60000" },
     { "max-unacked-clients",     Element::integer, "10" },
     { "require-client-certs",    Element::boolean, "true" },
+    { "restrict-commands",       Element::boolean, "false" },
     { "send-lease-updates",      Element::boolean, "true" },
     { "sync-leases",             Element::boolean, "true" },
     { "sync-timeout",            Element::integer, "60000" },
@@ -233,6 +234,9 @@ HAConfigParser::parseInternal(const HAConfigPtr& config_storage,
     // Get 'require-client-certs'.
     config_storage->setRequireClientCerts(getBoolean(c, "require-client-certs"));
 
+    // Get 'restrict-commands'.
+    config_storage->setRestrictCommands(getBoolean(c, "restrict-commands"));
+
     // Peers configuration parsing.
     const auto& peers_vec = peers->listValue();
 

diff --git a/src/hooks/dhcp/high_availability/ha_service.cc b/src/hooks/dhcp/high_availability/ha_service.cc
index 416bf052f3703229177e22b4e55acf8b1b40e58a..f844b33b044f81e4e378f7eed5ec3d9fcab086dd 100644 (file)
--- a/src/hooks/dhcp/high_availability/ha_service.cc
+++ b/src/hooks/dhcp/high_availability/ha_service.cc
@@ -12,6 +12,7 @@
 #include <ha_service_states.h>
 #include <cc/command_interpreter.h>
 #include <cc/data.h>
+#include <config/cmd_response_creator.h>
 #include <config/timeouts.h>
 #include <dhcp/iface_mgr.h>
 #include <dhcpsrv/cfgmgr.h>
@@ -114,6 +115,16 @@ HAService::HAService(const IOServicePtr& io_service, const NetworkStatePtr& netw
             // Instantiate the listener.
             listener_.reset(new CmdHttpListener(server_address, my_url.getPort(),
                                                 listener_threads, tls_context));
+            // Set the command filter when enabled.
+            if (config_->getRestrictCommands()) {
+                if (server_type == HAServerType::DHCPv4) {
+                    CmdResponseCreator::command_accept_list_ =
+                        CommandCreator::ha_commands4_;
+                } else {
+                    CmdResponseCreator::command_accept_list_ =
+                        CommandCreator::ha_commands6_;
+                }
+            }
         }
     }
 

diff --git a/src/hooks/dhcp/high_availability/tests/ha_config_unittest.cc b/src/hooks/dhcp/high_availability/tests/ha_config_unittest.cc
index 35b4cfbb9c865cd05d7c1c124cff9b060d7a1a8f..5c773a149ff9166d7e84f66e4a51142ca6451e68 100644 (file)
--- a/src/hooks/dhcp/high_availability/tests/ha_config_unittest.cc
+++ b/src/hooks/dhcp/high_availability/tests/ha_config_unittest.cc
@@ -1333,6 +1333,7 @@ TEST_F(HAConfigTest, tlsParameterInheritance) {
         "        \"cert-file\": \"!CA!/kea-client.crt\","
         "        \"key-file\": \"!CA!/kea-client.key\","
         "        \"require-client-certs\": false,"
+        "        \"restrict-commands\": true,"
         "        \"peers\": ["
         "            {"
         "                \"name\": \"my-server\","
@@ -1381,6 +1382,7 @@ TEST_F(HAConfigTest, tlsParameterInheritance) {
     expected += "/kea-client.key";
     EXPECT_EQ(expected, impl->getConfig()->getKeyFile().get());
     EXPECT_FALSE(impl->getConfig()->getRequireClientCerts());
+    EXPECT_TRUE(impl->getConfig()->getRestrictCommands());
 
     // Check the first peer parameters: it inherits them from the global level.
     HAConfig::PeerConfigPtr cfg = impl->getConfig()->getThisServerConfig();

diff --git a/src/hooks/dhcp/high_availability/tests/ha_mt_unittest.cc b/src/hooks/dhcp/high_availability/tests/ha_mt_unittest.cc
index 3c3f32d5419a8f53457b49dd61713d32c4c77731..eded087b63140308f99e1a67f7b2ced705359e36 100644 (file)
--- a/src/hooks/dhcp/high_availability/tests/ha_mt_unittest.cc
+++ b/src/hooks/dhcp/high_availability/tests/ha_mt_unittest.cc
@@ -7,6 +7,7 @@
 #include <config.h>
 
 #include <asiolink/asio_wrapper.h>
+#include <config/cmd_response_creator.h>
 #include <ha_test.h>
 #include <ha_config.h>
 #include <ha_service.h>
@@ -125,6 +126,7 @@ public:
     HAMtServiceTest()
         : HATest() {
         MultiThreadingMgr::instance().setMode(true);
+        CmdResponseCreator::command_accept_list_.clear();
     }
 
     /// @brief Destructor.
@@ -134,6 +136,7 @@ public:
         io_service_->get_io_service().reset();
         io_service_->poll();
         MultiThreadingMgr::instance().setMode(false);
+        CmdResponseCreator::command_accept_list_.clear();
     }
 
     /// @brief Callback function invoke upon test timeout.
@@ -162,6 +165,7 @@ TEST_F(HAMtServiceTest, multiThreadingBasics) {
         "        \"this-server-name\": \"server1\","
         "        \"mode\": \"passive-backup\","
         "        \"wait-backup-ack\": true,"
+        "        \"restrict-commands\": true,"
         "        \"peers\": ["
         "            {"
         "                \"name\": \"server1\","
@@ -195,6 +199,9 @@ TEST_F(HAMtServiceTest, multiThreadingBasics) {
     // Multi-threading should be enabled.
     ASSERT_TRUE(ha_config->getEnableMultiThreading());
 
+    // Command filtering is enabled.
+    EXPECT_FALSE(CmdResponseCreator::command_accept_list_.empty());
+
     // Now we'll start, pause, resume and stop a few times.
     for (int i = 0; i < 3; ++i) {
         // Verify we're stopped.