From: Marcin Siodelski Date: Tue, 16 Jul 2024 16:22:44 +0000 (+0200) Subject: [#3485] Backported fix #3481 to Kea 2.6.1 X-Git-Tag: Kea-2.6.1~14 X-Git-Url: http://git.ipfire.org/gitweb/gitweb.cgi?a=commitdiff_plain;h=89ab047f6ccd697810d6cc81b3cd9c0b14b26836;p=thirdparty%2Fkea.git [#3485] Backported fix #3481 to Kea 2.6.1 The fix removes an issue with not returning some suboptions specified in the Kea config backend to the DHCP clients. --- diff --git a/src/lib/dhcpsrv/cb_ctl_dhcp4.cc b/src/lib/dhcpsrv/cb_ctl_dhcp4.cc index 211681bce4..741f122aca 100644 --- a/src/lib/dhcpsrv/cb_ctl_dhcp4.cc +++ b/src/lib/dhcpsrv/cb_ctl_dhcp4.cc @@ -215,6 +215,7 @@ CBControlDHCPv4::databaseConfigApply(const BackendSelector& backend_selector, // Class options also need to be created when returned from the config backend. client_classes.createOptions(external_cfg->getCfgOptionDef()); + client_classes.encapsulateOptions(); external_cfg->setClientClassDictionary(boost::make_shared(client_classes)); } diff --git a/src/lib/dhcpsrv/cb_ctl_dhcp6.cc b/src/lib/dhcpsrv/cb_ctl_dhcp6.cc index 1ed4820810..577b23ceee 100644 --- a/src/lib/dhcpsrv/cb_ctl_dhcp6.cc +++ b/src/lib/dhcpsrv/cb_ctl_dhcp6.cc @@ -214,6 +214,7 @@ CBControlDHCPv6::databaseConfigApply(const db::BackendSelector& backend_selector // Class options also need to be created when returned from the config backend. client_classes.createOptions(external_cfg->getCfgOptionDef()); + client_classes.encapsulateOptions(); external_cfg->setClientClassDictionary(boost::make_shared(client_classes)); } diff --git a/src/lib/dhcpsrv/cfg_option.cc b/src/lib/dhcpsrv/cfg_option.cc index 27ae10b14e..61137ed5eb 100644 --- a/src/lib/dhcpsrv/cfg_option.cc +++ b/src/lib/dhcpsrv/cfg_option.cc @@ -256,6 +256,7 @@ CfgOption::copyTo(CfgOption& other) const { other.options_.clearItems(); other.vendor_options_.clearItems(); mergeTo(other); + other.encapsulated_ = isEncapsulated(); } void diff --git a/src/lib/dhcpsrv/cfg_shared_networks.h b/src/lib/dhcpsrv/cfg_shared_networks.h index 7123426e8c..7a0a57148f 100644 --- a/src/lib/dhcpsrv/cfg_shared_networks.h +++ b/src/lib/dhcpsrv/cfg_shared_networks.h @@ -200,7 +200,11 @@ public: // Create the network's options based on the given definitions. other_network->getCfgOption()->createOptions(cfg_def); - // Add the new/updated nework. + // Encapsulate options, so that the DHCP server can effectively return + // them to the clients without having to encapsulate them for each request. + other_network->getCfgOption()->encapsulate(); + + // Add the new/updated network. static_cast(networks_.push_back(other_network)); } } diff --git a/src/lib/dhcpsrv/cfg_subnets4.cc b/src/lib/dhcpsrv/cfg_subnets4.cc index 681f4c36f0..d462fca501 100644 --- a/src/lib/dhcpsrv/cfg_subnets4.cc +++ b/src/lib/dhcpsrv/cfg_subnets4.cc @@ -154,9 +154,14 @@ CfgSubnets4::merge(CfgOptionDefPtr cfg_def, CfgSharedNetworks4Ptr networks, // Create the subnet's options based on the given definitions. other_subnet->getCfgOption()->createOptions(cfg_def); + // Encapsulate options, so that the DHCP server can effectively return + // them to the clients without having to encapsulate them for each request. + other_subnet->getCfgOption()->encapsulate(); + // Create the options for pool based on the given definitions. for (auto const& pool : other_subnet->getPoolsWritable(Lease::TYPE_V4)) { pool->getCfgOption()->createOptions(cfg_def); + pool->getCfgOption()->encapsulate(); } // Add the "other" subnet to the our collection of subnets. diff --git a/src/lib/dhcpsrv/cfg_subnets6.cc b/src/lib/dhcpsrv/cfg_subnets6.cc index 704cd42cb9..faf20bbd80 100644 --- a/src/lib/dhcpsrv/cfg_subnets6.cc +++ b/src/lib/dhcpsrv/cfg_subnets6.cc @@ -151,14 +151,20 @@ CfgSubnets6::merge(CfgOptionDefPtr cfg_def, CfgSharedNetworks6Ptr networks, // Create the subnet's options based on the given definitions. other_subnet->getCfgOption()->createOptions(cfg_def); + // Encapsulate options, so that the DHCP server can effectively return + // them to the clients without having to encapsulate them for each request. + other_subnet->getCfgOption()->encapsulate(); + // Create the options for pool based on the given definitions. for (auto const& pool : other_subnet->getPoolsWritable(Lease::TYPE_NA)) { pool->getCfgOption()->createOptions(cfg_def); + pool->getCfgOption()->encapsulate(); } // Create the options for pd pool based on the given definitions. for (auto const& pool : other_subnet->getPoolsWritable(Lease::TYPE_PD)) { pool->getCfgOption()->createOptions(cfg_def); + pool->getCfgOption()->encapsulate(); } // Add the "other" subnet to the our collection of subnets. diff --git a/src/lib/dhcpsrv/cfgmgr.cc b/src/lib/dhcpsrv/cfgmgr.cc index c457fb7bde..ef156dd22c 100644 --- a/src/lib/dhcpsrv/cfgmgr.cc +++ b/src/lib/dhcpsrv/cfgmgr.cc @@ -197,6 +197,7 @@ CfgMgr::mergeIntoCurrentCfg(const uint32_t seq) { // First we need to remove statistics. getCurrentCfg()->removeStatistics(); mergeIntoCfg(getCurrentCfg(), seq); + LibDHCP::setRuntimeOptionDefs(getCurrentCfg()->getCfgOptionDef()->getContainer()); } catch (...) { // Make sure the statistics is updated even if the merge failed. diff --git a/src/lib/dhcpsrv/client_class_def.cc b/src/lib/dhcpsrv/client_class_def.cc index d9b69f14b8..166d66e4b3 100644 --- a/src/lib/dhcpsrv/client_class_def.cc +++ b/src/lib/dhcpsrv/client_class_def.cc @@ -553,6 +553,16 @@ ClientClassDictionary::createOptions(const CfgOptionDefPtr& external_defs) { } } +void +ClientClassDictionary::encapsulateOptions() const { + for (auto const& c : *list_) { + CfgOptionPtr class_options = c->getCfgOption(); + if (class_options) { + class_options->encapsulate(); + } + } +} + ElementPtr ClientClassDictionary::toElement() const { ElementPtr result = Element::createList(); diff --git a/src/lib/dhcpsrv/client_class_def.h b/src/lib/dhcpsrv/client_class_def.h index da5a452e56..c9158e1107 100644 --- a/src/lib/dhcpsrv/client_class_def.h +++ b/src/lib/dhcpsrv/client_class_def.h @@ -471,6 +471,10 @@ public: /// @param cfg_option_def set of option definitions to use. void createOptions(const CfgOptionDefPtr& cfg_option_def); + /// @brief Iterates over the classes in the dictionary and encapsulates + /// suboptions. + void encapsulateOptions() const; + /// @brief Equality operator. /// /// @param other Other client class dictionary to compare to. diff --git a/src/lib/dhcpsrv/tests/cb_ctl_dhcp_unittest.cc b/src/lib/dhcpsrv/tests/cb_ctl_dhcp_unittest.cc index ec0e7b9ef0..3cdf633577 100644 --- a/src/lib/dhcpsrv/tests/cb_ctl_dhcp_unittest.cc +++ b/src/lib/dhcpsrv/tests/cb_ctl_dhcp_unittest.cc @@ -587,6 +587,7 @@ public: (getTimestamp("dhcp4_options") > lb_modification_time)) { ASSERT_TRUE(found_opt.option_); EXPECT_EQ("new.example.com", found_opt.option_->toString()); + EXPECT_TRUE(options->isEncapsulated()); } else { EXPECT_FALSE(found_opt.option_); @@ -601,6 +602,7 @@ public: (getTimestamp("dhcp4_shared_network") > lb_modification_time)) { ASSERT_TRUE(found_network); EXPECT_TRUE(found_network->hasFetchGlobalsFn()); + EXPECT_TRUE(found_network->getCfgOption()->isEncapsulated()); } else { EXPECT_FALSE(found_network); @@ -616,6 +618,7 @@ public: EXPECT_TRUE(found_subnet->hasFetchGlobalsFn()); EXPECT_TRUE(boost::dynamic_pointer_cast (found_subnet->getAllocator(Lease::TYPE_V4))); + EXPECT_TRUE(found_subnet->getCfgOption()->isEncapsulated()); } else { EXPECT_FALSE(found_subnet); @@ -647,6 +650,9 @@ public: EXPECT_EQ(OptionBuffer(option_desc.formatted_value_.begin(), option_desc.formatted_value_.end()), option->getData()); + + // Make sure that the options have been encapsulated. + EXPECT_TRUE(cfg_option_desc->isEncapsulated()); } else { EXPECT_FALSE(found_class); } @@ -1500,6 +1506,7 @@ public: (getTimestamp("dhcp6_options") > lb_modification_time)) { ASSERT_TRUE(found_opt.option_); EXPECT_EQ("some.bootfile", found_opt.option_->toString()); + EXPECT_TRUE(options->isEncapsulated()); } else { EXPECT_FALSE(found_opt.option_); @@ -1514,6 +1521,7 @@ public: (getTimestamp("dhcp6_shared_network") > lb_modification_time)) { ASSERT_TRUE(found_network); EXPECT_TRUE(found_network->hasFetchGlobalsFn()); + EXPECT_TRUE(found_network->getCfgOption()->isEncapsulated()); } else { EXPECT_FALSE(found_network); @@ -1531,6 +1539,7 @@ public: EXPECT_TRUE(boost::dynamic_pointer_cast (found_subnet->getAllocator(Lease::TYPE_PD))); EXPECT_TRUE(found_subnet->hasFetchGlobalsFn()); + EXPECT_TRUE(found_subnet->getCfgOption()->isEncapsulated()); } else { EXPECT_FALSE(found_subnet); @@ -1554,6 +1563,8 @@ public: EXPECT_EQ(OptionBuffer(option_desc.formatted_value_.begin(), option_desc.formatted_value_.end()), option->getData()); + EXPECT_TRUE(found_class->getCfgOption()->isEncapsulated()); + } else { EXPECT_FALSE(found_class); } diff --git a/src/lib/dhcpsrv/tests/cfg_option_unittest.cc b/src/lib/dhcpsrv/tests/cfg_option_unittest.cc index 573899b7ab..0fabbec3d1 100644 --- a/src/lib/dhcpsrv/tests/cfg_option_unittest.cc +++ b/src/lib/dhcpsrv/tests/cfg_option_unittest.cc @@ -474,6 +474,14 @@ TEST_F(CfgOptionTest, copy) { container = cfg_dst.getAll("foo"); ASSERT_TRUE(container); EXPECT_EQ(10, container->size()); + + // Source config wasn't encapsulated, so the destination shouldn't be too. + EXPECT_FALSE(cfg_dst.isEncapsulated()); + + // Now let's make sure that the encapsulation flag is correctly set. + cfg_src.encapsulate(); + ASSERT_NO_THROW(cfg_src.copyTo(cfg_dst)); + EXPECT_TRUE(cfg_dst.isEncapsulated()); } // This test verifies that DHCP options from one configuration diff --git a/src/lib/dhcpsrv/tests/cfg_shared_networks4_unittest.cc b/src/lib/dhcpsrv/tests/cfg_shared_networks4_unittest.cc index bf0ba21bb3..1ed822d7a4 100644 --- a/src/lib/dhcpsrv/tests/cfg_shared_networks4_unittest.cc +++ b/src/lib/dhcpsrv/tests/cfg_shared_networks4_unittest.cc @@ -370,6 +370,7 @@ TEST(CfgSharedNetworks4Test, mergeNetworks) { OptionStringPtr opstr = boost::dynamic_pointer_cast(desc.option_); ASSERT_TRUE(opstr); EXPECT_EQ("Yay!", opstr->getValue()); + EXPECT_TRUE(network->getCfgOption()->isEncapsulated()); // No changes to network2. ASSERT_NO_FATAL_FAILURE(checkMergedNetwork(cfg_to, "network2", Triplet(200), diff --git a/src/lib/dhcpsrv/tests/cfg_shared_networks6_unittest.cc b/src/lib/dhcpsrv/tests/cfg_shared_networks6_unittest.cc index 065829932b..ac684dd1f6 100644 --- a/src/lib/dhcpsrv/tests/cfg_shared_networks6_unittest.cc +++ b/src/lib/dhcpsrv/tests/cfg_shared_networks6_unittest.cc @@ -378,6 +378,7 @@ TEST(CfgSharedNetworks6Test, mergeNetworks) { OptionStringPtr opstr = boost::dynamic_pointer_cast(desc.option_); ASSERT_TRUE(opstr); EXPECT_EQ("Yay!", opstr->getValue()); + EXPECT_TRUE(network->getCfgOption()->isEncapsulated()); // No changes to network2. ASSERT_NO_FATAL_FAILURE(checkMergedNetwork(cfg_to, "network2", Triplet(200), diff --git a/src/lib/dhcpsrv/tests/cfg_subnets4_unittest.cc b/src/lib/dhcpsrv/tests/cfg_subnets4_unittest.cc index 2e15407159..8f6e563be9 100644 --- a/src/lib/dhcpsrv/tests/cfg_subnets4_unittest.cc +++ b/src/lib/dhcpsrv/tests/cfg_subnets4_unittest.cc @@ -390,6 +390,7 @@ TEST(CfgSubnets4Test, mergeSubnets) { opstr = boost::dynamic_pointer_cast(desc.option_); ASSERT_TRUE(opstr); EXPECT_EQ("Team!", opstr->getValue()); + EXPECT_TRUE(subnet->getCfgOption()->isEncapsulated()); // subnet4 should be replaced by subnet4b and moved to network1. ASSERT_NO_FATAL_FAILURE(checkMergedSubnet(cfg_to, "192.0.4.0/26", @@ -407,6 +408,7 @@ TEST(CfgSubnets4Test, mergeSubnets) { opstr = boost::dynamic_pointer_cast(desc.option_); ASSERT_TRUE(opstr); EXPECT_EQ("POOLS", opstr->getValue()); + EXPECT_TRUE(merged_pool->getCfgOption()->isEncapsulated()); const PoolPtr merged_pool2 = subnet->getPool(Lease::TYPE_V4, IOAddress("192.0.5.30")); ASSERT_TRUE(merged_pool2); @@ -414,6 +416,7 @@ TEST(CfgSubnets4Test, mergeSubnets) { opstr = boost::dynamic_pointer_cast(desc.option_); ASSERT_TRUE(opstr); EXPECT_EQ("RULE!", opstr->getValue()); + EXPECT_TRUE(merged_pool2->getCfgOption()->isEncapsulated()); } // This test verifies that it is possible to retrieve a subnet using an diff --git a/src/lib/dhcpsrv/tests/cfg_subnets6_unittest.cc b/src/lib/dhcpsrv/tests/cfg_subnets6_unittest.cc index 38f8b255e0..fdbfa124ce 100644 --- a/src/lib/dhcpsrv/tests/cfg_subnets6_unittest.cc +++ b/src/lib/dhcpsrv/tests/cfg_subnets6_unittest.cc @@ -20,6 +20,7 @@ #include #include #include + #include #include #include @@ -1076,6 +1077,7 @@ TEST(CfgSubnets6Test, mergeSubnets) { opstr = boost::dynamic_pointer_cast(desc.option_); ASSERT_TRUE(opstr); EXPECT_EQ("Team!", opstr->getValue()); + EXPECT_TRUE(subnet->getCfgOption()->isEncapsulated()); // subnet4 should be replaced by subnet4b and moved to network1. ASSERT_NO_FATAL_FAILURE(checkMergedSubnet(cfg_to, "2001:4::/64", @@ -1093,6 +1095,7 @@ TEST(CfgSubnets6Test, mergeSubnets) { opstr = boost::dynamic_pointer_cast(desc.option_); ASSERT_TRUE(opstr); EXPECT_EQ("POOLS", opstr->getValue()); + EXPECT_TRUE(merged_pool->getCfgOption()->isEncapsulated()); const PoolPtr merged_pool2 = subnet->getPool(Lease::TYPE_PD, IOAddress("2001:1::")); ASSERT_TRUE(merged_pool2); @@ -1100,6 +1103,7 @@ TEST(CfgSubnets6Test, mergeSubnets) { opstr = boost::dynamic_pointer_cast(desc.option_); ASSERT_TRUE(opstr); EXPECT_EQ("RULE!", opstr->getValue()); + EXPECT_TRUE(merged_pool2->getCfgOption()->isEncapsulated()); } // This test verifies the Subnet6 parser's validation logic for id parameter. diff --git a/src/lib/dhcpsrv/tests/cfgmgr_unittest.cc b/src/lib/dhcpsrv/tests/cfgmgr_unittest.cc index 1be8c68456..13ec6a1147 100644 --- a/src/lib/dhcpsrv/tests/cfgmgr_unittest.cc +++ b/src/lib/dhcpsrv/tests/cfgmgr_unittest.cc @@ -8,6 +8,7 @@ #include #include +#include #include #include #include @@ -1054,6 +1055,13 @@ TEST_F(CfgMgrTest, mergeIntoCurrentCfg) { // Those must be two separate instances. ASSERT_FALSE(ext_cfg1 == ext_cfg2); + // Add an option definition. + ext_cfg1->getCfgOptionDef()->add(OptionDefinition::create("option-foo", + 1, + "isc", + OPT_EMPTY_TYPE, + "")); + // Add a subnet which will be merged from first configuration. Subnet4Ptr subnet1(new Subnet4(IOAddress("192.1.2.0"), 24, 1, 2, 3, 123)); ext_cfg1->getCfgSubnets4()->add(subnet1); @@ -1073,6 +1081,10 @@ TEST_F(CfgMgrTest, mergeIntoCurrentCfg) { ASSERT_TRUE(cfg_mgr.getCurrentCfg()->getCfgSubnets4()->getBySubnetId(123)); ASSERT_FALSE(cfg_mgr.getCurrentCfg()->getCfgSubnets4()->getBySubnetId(124)); + // Ensure that the runtime option definitions have been updated. + auto runtime_def = LibDHCP::getRuntimeOptionDef("isc", "option-foo"); + ASSERT_TRUE(runtime_def); + // Create another configuration instance to check what sequence it would // pick. It should pick the first available one. SrvConfigPtr ext_cfg3;