diff options
| -rw-r--r-- | src/bin/dhcp4/dhcp4_srv.cc | 16 | ||||
| -rw-r--r-- | src/bin/dhcp4/tests/dhcp4_srv_unittest.cc | 442 | ||||
| -rw-r--r-- | src/bin/dhcp4/tests/dhcp4_test_utils.h | 1 |
3 files changed, 449 insertions, 10 deletions
diff --git a/src/bin/dhcp4/dhcp4_srv.cc b/src/bin/dhcp4/dhcp4_srv.cc index 4c83daec67..13ed480b76 100644 --- a/src/bin/dhcp4/dhcp4_srv.cc +++ b/src/bin/dhcp4/dhcp4_srv.cc @@ -4362,11 +4362,7 @@ Dhcpv4Srv::recoverStashedAgentOption(const Pkt4Ptr& query) { if (!isc || (isc->getType() != Element::map)) { return; } - ConstElementPtr extended_info = isc->get("relay-agent-info"); - if (!extended_info || (extended_info->getType() != Element::map)) { - return; - } - ConstElementPtr relay_agent_info = extended_info->get("relay-agent-info"); + ConstElementPtr relay_agent_info = isc->get("relay-agent-info"); if (!relay_agent_info) { return; } @@ -4391,12 +4387,11 @@ Dhcpv4Srv::recoverStashedAgentOption(const Pkt4Ptr& query) { } // Extract the RAI. string rai_hex = relay_agent_info->stringValue(); - vector<uint8_t> rai_data; - str::decodeFormattedHexString(rai_hex, rai_data); - if (rai_data.size() <= Option::OPTION4_HDR_LEN) { - // RAI is empty. + if (rai_hex.empty()) { return; } + vector<uint8_t> rai_data; + str::decodeFormattedHexString(rai_hex, rai_data); static OptionDefinitionPtr rai_def; if (!rai_def) { rai_def = LibDHCP::getOptionDef(DHCP4_OPTION_SPACE, @@ -4407,7 +4402,8 @@ Dhcpv4Srv::recoverStashedAgentOption(const Pkt4Ptr& query) { return; } OptionCustomPtr rai(new OptionCustom(*rai_def, Option::V4, rai_data)); - if (!rai) { + // unpackOptions is a bit too flexible so check if it got something... + if (!rai || rai->getOptions().empty()) { return; } // Remove an existing empty RAI. diff --git a/src/bin/dhcp4/tests/dhcp4_srv_unittest.cc b/src/bin/dhcp4/tests/dhcp4_srv_unittest.cc index de5b17e441..d385812690 100644 --- a/src/bin/dhcp4/tests/dhcp4_srv_unittest.cc +++ b/src/bin/dhcp4/tests/dhcp4_srv_unittest.cc @@ -5250,6 +5250,448 @@ TEST_F(Dhcpv4SrvTest, fixedFieldsInClassOrder) { } } +/// @brief Test fixture for recoverStashedAgentOption. +class StashAgentOptionTest : public Dhcpv4SrvTest { +public: + /// @brief Constructor. + StashAgentOptionTest() : addr_("192.0.1.2") { + query_.reset(new Pkt4(DHCPREQUEST, 1234)); + query_->addOption(generateClientId()); + hwaddr_ = generateHWAddr(); + query_->setHWAddr(hwaddr_); + query_->setCiaddr(addr_); + + if (!rai_def_) { + isc_throw(Unexpected, "RAI definition can't be found"); + } + rai_.reset(new OptionCustom(*rai_def_, Option::V4)); + rai_sub_.reset(new Option(Option::V4, RAI_OPTION_LINK_SELECTION, + addr_.toBytes())); + rai_->addOption(rai_sub_); + + lease_.reset(new Lease4(addr_, hwaddr_, client_id_, 100, time(0), 1)); + user_context_ = Element::createMap(); + lease_->setContext(user_context_); + isc_ = Element::createMap(); + user_context_->set("ISC", isc_); + relay_agent_info_ = Element::createMap(); + isc_->set("relay-agent-info", relay_agent_info_); + sub_options_ = Element::create(rai_->toHexString()); + relay_agent_info_->set("sub-options", sub_options_); + + LeaseMgrFactory::destroy(); + LeaseMgrFactory::create("type=memfile persist=false universe=4"); + + CfgMgr::instance().clear(); + CfgMgr::instance().getStagingCfg()-> + addConfiguredGlobal("stash-agent-options", Element::create(true)); + } + + /// @brief Destructor. + virtual ~StashAgentOptionTest() { + CfgMgr::instance().clear(); + LeaseMgrFactory::destroy(); + } + + /// RAI definition. + static OptionDefinitionPtr rai_def_; + + /// Client address. + IOAddress addr_; + + /// Query. + Pkt4Ptr query_; + + /// Hardware address. + HWAddrPtr hwaddr_; + + /// RAI option. + OptionPtr rai_; + + /// RAI suboption. + OptionPtr rai_sub_; + + /// Lease. + Lease4Ptr lease_; + + /// Lease user context. + ElementPtr user_context_; + + /// ISC map. + ElementPtr isc_; + + /// Relay agent info (map or string). + ElementPtr relay_agent_info_; + + /// Sub-options i.e. RAI content (hexstring). + ElementPtr sub_options_; +}; + +OptionDefinitionPtr +StashAgentOptionTest::rai_def_ = LibDHCP::getOptionDef(DHCP4_OPTION_SPACE, + DHO_DHCP_AGENT_OPTIONS); + +// Verify that RAI has a definition and can be built. +TEST(StashAgentOptionTestTest, checkRAI) { + ASSERT_TRUE(StashAgentOptionTest::rai_def_); +} + +// Verify the basic positive case. +TEST_F(StashAgentOptionTest, basic) { + CfgMgr::instance().commit(); + EXPECT_NO_THROW(LeaseMgrFactory::instance().addLease(lease_)); + + EXPECT_NO_THROW(srv_.recoverStashedAgentOption(query_)); + OptionPtr rai_query = query_->getOption(DHO_DHCP_AGENT_OPTIONS); + ASSERT_TRUE(rai_query); + EXPECT_EQ(rai_query->toHexString(true), rai_->toHexString(true)); + EXPECT_TRUE(query_->inClass("STASH_AGENT_OPTIONS")); +} + +// Verify that the client id has priority i.e. when the client id +// is present the hardware address is ignored. +TEST_F(StashAgentOptionTest, clientId) { + // Change the hardware address. + auto hwaddr2 = generateHWAddr(8); + ASSERT_NE(hwaddr_, hwaddr2); + query_->setHWAddr(hwaddr2); + + CfgMgr::instance().commit(); + EXPECT_NO_THROW(LeaseMgrFactory::instance().addLease(lease_)); + + EXPECT_NO_THROW(srv_.recoverStashedAgentOption(query_)); + OptionPtr rai_query = query_->getOption(DHO_DHCP_AGENT_OPTIONS); + ASSERT_TRUE(rai_query); + EXPECT_EQ(rai_query->toHexString(true), rai_->toHexString(true)); + EXPECT_TRUE(query_->inClass("STASH_AGENT_OPTIONS")); +} + +// Verify that the hardware address is used when there is no client id. +TEST_F(StashAgentOptionTest, hardwareAddress) { + // No longer use the client id. + query_->delOption(DHO_DHCP_CLIENT_IDENTIFIER); + lease_.reset(new Lease4(addr_, hwaddr_, ClientIdPtr(), 100, time(0), 1)); + lease_->setContext(user_context_); + + CfgMgr::instance().commit(); + EXPECT_NO_THROW(LeaseMgrFactory::instance().addLease(lease_)); + + EXPECT_NO_THROW(srv_.recoverStashedAgentOption(query_)); + OptionPtr rai_query = query_->getOption(DHO_DHCP_AGENT_OPTIONS); + ASSERT_TRUE(rai_query); + EXPECT_EQ(rai_query->toHexString(true), rai_->toHexString(true)); + EXPECT_TRUE(query_->inClass("STASH_AGENT_OPTIONS")); +} + +// Verify that old extended info format is supported. +TEST_F(StashAgentOptionTest, oldExtendedInfo) { + // Use the old extended info format. + isc_->set("relay-agent-info", sub_options_); + + CfgMgr::instance().commit(); + EXPECT_NO_THROW(LeaseMgrFactory::instance().addLease(lease_)); + + EXPECT_NO_THROW(srv_.recoverStashedAgentOption(query_)); + OptionPtr rai_query = query_->getOption(DHO_DHCP_AGENT_OPTIONS); + ASSERT_TRUE(rai_query); + EXPECT_EQ(rai_query->toHexString(true), rai_->toHexString(true)); + EXPECT_TRUE(query_->inClass("STASH_AGENT_OPTIONS")); +} + +// Verify that empty RAI is supported. +TEST_F(StashAgentOptionTest, emptyRelayAgentInfo) { + // Add an empty RAI. + OptionPtr empty_rai(new OptionCustom(*StashAgentOptionTest::rai_def_, + Option::V4)); + query_->addOption(empty_rai); + + CfgMgr::instance().commit(); + EXPECT_NO_THROW(LeaseMgrFactory::instance().addLease(lease_)); + + EXPECT_NO_THROW(srv_.recoverStashedAgentOption(query_)); + OptionPtr rai_query = query_->getOption(DHO_DHCP_AGENT_OPTIONS); + ASSERT_TRUE(rai_query); + EXPECT_FALSE(rai_query->getOptions().empty()); + EXPECT_EQ(rai_query->toHexString(true), rai_->toHexString(true)); + EXPECT_TRUE(query_->inClass("STASH_AGENT_OPTIONS")); +} + +// Verify that the client address must be not zero. +TEST_F(StashAgentOptionTest, clientAddress) { + // Set the client address to 0.0.0.0. + query_->setCiaddr(IOAddress::IPV4_ZERO_ADDRESS()); + + CfgMgr::instance().commit(); + EXPECT_NO_THROW(LeaseMgrFactory::instance().addLease(lease_)); + + EXPECT_NO_THROW(srv_.recoverStashedAgentOption(query_)); + OptionPtr rai_query = query_->getOption(DHO_DHCP_AGENT_OPTIONS); + EXPECT_FALSE(rai_query); + EXPECT_FALSE(query_->inClass("STASH_AGENT_OPTIONS")); +} + +// Verify that the gateway address must be zero. +TEST_F(StashAgentOptionTest, gatewayAddress) { + // Set the gateway address to not zero. + query_->setGiaddr(IOAddress("192.0.2.1")); + + CfgMgr::instance().commit(); + EXPECT_NO_THROW(LeaseMgrFactory::instance().addLease(lease_)); + + EXPECT_NO_THROW(srv_.recoverStashedAgentOption(query_)); + OptionPtr rai_query = query_->getOption(DHO_DHCP_AGENT_OPTIONS); + EXPECT_FALSE(rai_query); + EXPECT_FALSE(query_->inClass("STASH_AGENT_OPTIONS")); + + // Even broadcast is not accepted. + query_->setGiaddr(IOAddress::IPV4_BCAST_ADDRESS()); + EXPECT_NO_THROW(srv_.recoverStashedAgentOption(query_)); + rai_query = query_->getOption(DHO_DHCP_AGENT_OPTIONS); + EXPECT_FALSE(rai_query); + EXPECT_FALSE(query_->inClass("STASH_AGENT_OPTIONS")); +} + +// Verify that the stash-agent-options is required. +TEST_F(StashAgentOptionTest, stashAgentOption) { + // Not commit the config. + + EXPECT_NO_THROW(LeaseMgrFactory::instance().addLease(lease_)); + + EXPECT_NO_THROW(srv_.recoverStashedAgentOption(query_)); + OptionPtr rai_query = query_->getOption(DHO_DHCP_AGENT_OPTIONS); + EXPECT_FALSE(rai_query); + EXPECT_FALSE(query_->inClass("STASH_AGENT_OPTIONS")); +} + +// Verify that the message type must be DHCPREQUEST. +TEST_F(StashAgentOptionTest, request) { + // Build a DISCOVER query. + query_.reset(new Pkt4(DHCPDISCOVER, 1234)); + query_->addOption(generateClientId()); + hwaddr_ = generateHWAddr(); + query_->setHWAddr(hwaddr_); + query_->setCiaddr(addr_); + + CfgMgr::instance().commit(); + EXPECT_NO_THROW(LeaseMgrFactory::instance().addLease(lease_)); + + EXPECT_NO_THROW(srv_.recoverStashedAgentOption(query_)); + OptionPtr rai_query = query_->getOption(DHO_DHCP_AGENT_OPTIONS); + EXPECT_FALSE(rai_query); + EXPECT_FALSE(query_->inClass("STASH_AGENT_OPTIONS")); +} + +// Verify that if there is a not empty RAI it will fail. +TEST_F(StashAgentOptionTest, notEmptyRelayAgentInfo) { + // Add a not empty RAI. + query_->addOption(rai_); + + CfgMgr::instance().commit(); + EXPECT_NO_THROW(LeaseMgrFactory::instance().addLease(lease_)); + + EXPECT_NO_THROW(srv_.recoverStashedAgentOption(query_)); + EXPECT_FALSE(query_->inClass("STASH_AGENT_OPTIONS")); +} + +// Verify that if the query is already in STASH_AGENT_OPTIONS it will fail. +TEST_F(StashAgentOptionTest, inClass) { + // Add the STASH_AGENT_OPTIONS class. + query_->addClass("STASH_AGENT_OPTIONS"); + + CfgMgr::instance().commit(); + EXPECT_NO_THROW(LeaseMgrFactory::instance().addLease(lease_)); + + EXPECT_NO_THROW(srv_.recoverStashedAgentOption(query_)); + OptionPtr rai_query = query_->getOption(DHO_DHCP_AGENT_OPTIONS); + EXPECT_FALSE(rai_query); + EXPECT_TRUE(query_->inClass("STASH_AGENT_OPTIONS")); +} + +// Verify that the lease is required. +TEST_F(StashAgentOptionTest, lease) { + CfgMgr::instance().commit(); + // Not add the lease. + + EXPECT_NO_THROW(srv_.recoverStashedAgentOption(query_)); + OptionPtr rai_query = query_->getOption(DHO_DHCP_AGENT_OPTIONS); + EXPECT_FALSE(rai_query); + EXPECT_FALSE(query_->inClass("STASH_AGENT_OPTIONS")); +} + +// Verify that if the lease is expired it will fail. +TEST_F(StashAgentOptionTest, expired) { + // Expired the lease. + lease_.reset(new Lease4(addr_, hwaddr_, client_id_, 100, + time(0) - 1000, 1)); + lease_->setContext(user_context_); + + CfgMgr::instance().commit(); + EXPECT_NO_THROW(LeaseMgrFactory::instance().addLease(lease_)); + + EXPECT_NO_THROW(srv_.recoverStashedAgentOption(query_)); + OptionPtr rai_query = query_->getOption(DHO_DHCP_AGENT_OPTIONS); + EXPECT_FALSE(rai_query); + EXPECT_FALSE(query_->inClass("STASH_AGENT_OPTIONS")); +} + +// Verify that the lease user context is required. +TEST_F(StashAgentOptionTest, userContext) { + // Remove lease user context. + lease_->setContext(ElementPtr()); + + CfgMgr::instance().commit(); + EXPECT_NO_THROW(LeaseMgrFactory::instance().addLease(lease_)); + + EXPECT_NO_THROW(srv_.recoverStashedAgentOption(query_)); + OptionPtr rai_query = query_->getOption(DHO_DHCP_AGENT_OPTIONS); + EXPECT_FALSE(rai_query); + EXPECT_FALSE(query_->inClass("STASH_AGENT_OPTIONS")); +} + +// Verify that the lease user context must be a map. +// getLeaseClientClasses throws BadValue on addLease. + +// Verify that the ISC entry in the lease user context is required. +TEST_F(StashAgentOptionTest, iscEntry) { + // Remove the isc entry. + user_context_ = Element::createMap(); + user_context_->set("foo", Element::create("bar")); + lease_->setContext(user_context_); + + CfgMgr::instance().commit(); + EXPECT_NO_THROW(LeaseMgrFactory::instance().addLease(lease_)); + + EXPECT_NO_THROW(srv_.recoverStashedAgentOption(query_)); + OptionPtr rai_query = query_->getOption(DHO_DHCP_AGENT_OPTIONS); + EXPECT_FALSE(rai_query); + EXPECT_FALSE(query_->inClass("STASH_AGENT_OPTIONS")); +} + +// Verify that the ISC entry in the lease user context must be a map. +// getLeaseClientClasses throws BadValue on addLease.} + +// Verify that the relay-agent-info entry is required. +TEST_F(StashAgentOptionTest, relayAgentInfoEntry) { + // Remove the relay-agent-info entry. + isc_ = Element::createMap(); + isc_->set("foo", Element::create("bar")); + user_context_->set("ISC", isc_); + + CfgMgr::instance().commit(); + EXPECT_NO_THROW(LeaseMgrFactory::instance().addLease(lease_)); + + EXPECT_NO_THROW(srv_.recoverStashedAgentOption(query_)); + OptionPtr rai_query = query_->getOption(DHO_DHCP_AGENT_OPTIONS); + EXPECT_FALSE(rai_query); + EXPECT_FALSE(query_->inClass("STASH_AGENT_OPTIONS")); +} + +// Verify the relay-agent-info entry must be a map or a string. +TEST_F(StashAgentOptionTest, badRelayAgentInfoEntry) { + // Set the relay-agent-info entry to a boolean. + relay_agent_info_ = Element::create(true); + isc_->set("relay-agent-info", relay_agent_info_); + + CfgMgr::instance().commit(); + EXPECT_NO_THROW(LeaseMgrFactory::instance().addLease(lease_)); + + EXPECT_NO_THROW(srv_.recoverStashedAgentOption(query_)); + OptionPtr rai_query = query_->getOption(DHO_DHCP_AGENT_OPTIONS); + EXPECT_FALSE(rai_query); + EXPECT_FALSE(query_->inClass("STASH_AGENT_OPTIONS")); +} + +// Verify the sub-options entry is required in new extended info format. +TEST_F(StashAgentOptionTest, subOptionsEntry) { + // Remove the sub-options entry. + relay_agent_info_ = Element::createMap(); + relay_agent_info_->set("foo", Element::create("bar")); + isc_->set("relay-agent-info", relay_agent_info_); + + CfgMgr::instance().commit(); + EXPECT_NO_THROW(LeaseMgrFactory::instance().addLease(lease_)); + + EXPECT_NO_THROW(srv_.recoverStashedAgentOption(query_)); + OptionPtr rai_query = query_->getOption(DHO_DHCP_AGENT_OPTIONS); + EXPECT_FALSE(rai_query); + EXPECT_FALSE(query_->inClass("STASH_AGENT_OPTIONS")); +} + +// Verify the sub-options entry must be a string. +TEST_F(StashAgentOptionTest, badSubOptionsEntry) { + // Set the sub-options entry to a boolean. + sub_options_ = Element::create(true); + relay_agent_info_->set("sub-options", sub_options_); + + CfgMgr::instance().commit(); + EXPECT_NO_THROW(LeaseMgrFactory::instance().addLease(lease_)); + + EXPECT_NO_THROW(srv_.recoverStashedAgentOption(query_)); + OptionPtr rai_query = query_->getOption(DHO_DHCP_AGENT_OPTIONS); + EXPECT_FALSE(rai_query); + EXPECT_FALSE(query_->inClass("STASH_AGENT_OPTIONS")); +} + +// Verify the sub-options entry must be not empty. +TEST_F(StashAgentOptionTest, emptySubOptionsEntry) { + // Set the sub-options entry to empty. + sub_options_ = Element::create(""); + relay_agent_info_->set("sub-options", sub_options_); + + CfgMgr::instance().commit(); + EXPECT_NO_THROW(LeaseMgrFactory::instance().addLease(lease_)); + + EXPECT_NO_THROW(srv_.recoverStashedAgentOption(query_)); + OptionPtr rai_query = query_->getOption(DHO_DHCP_AGENT_OPTIONS); + EXPECT_FALSE(rai_query); + EXPECT_FALSE(query_->inClass("STASH_AGENT_OPTIONS")); +} + +// Verify the sub-options entry must be a valid hexstring. +TEST_F(StashAgentOptionTest, hexString) { + // Set the sub-options entry to invalid hexstring. + sub_options_ = Element::create("foobar"); + relay_agent_info_->set("sub-options", sub_options_); + + CfgMgr::instance().commit(); + EXPECT_NO_THROW(LeaseMgrFactory::instance().addLease(lease_)); + + EXPECT_THROW(srv_.recoverStashedAgentOption(query_), BadValue); + EXPECT_FALSE(query_->inClass("STASH_AGENT_OPTIONS")); +} + +// Verify the sub-options entry must be a valid RAI content. +TEST_F(StashAgentOptionTest, badRelayAgentInfo) { + // Set the sub-options entry to truncated RAI content. + string content = sub_options_->stringValue(); + ASSERT_LT(2, content.size()); + content.resize(content.size() - 2); + sub_options_ = Element::create(content); + relay_agent_info_->set("sub-options", sub_options_); + + CfgMgr::instance().commit(); + EXPECT_NO_THROW(LeaseMgrFactory::instance().addLease(lease_)); + + EXPECT_NO_THROW(srv_.recoverStashedAgentOption(query_)); + OptionPtr rai_query = query_->getOption(DHO_DHCP_AGENT_OPTIONS); + EXPECT_FALSE(rai_query); + EXPECT_FALSE(query_->inClass("STASH_AGENT_OPTIONS")); + + // Set the sub-options entry to invalid RAI content. + content = rai_->toHexString(); + // length is in the second octet so patch the third hexdigit. + ASSERT_LE(5, content.size()); + ASSERT_EQ('4', content[5]); + content[5] = '3'; + sub_options_ = Element::create(content); + relay_agent_info_->set("sub-options", sub_options_); + + EXPECT_THROW(srv_.recoverStashedAgentOption(query_), InvalidOptionValue); + EXPECT_FALSE(query_->inClass("STASH_AGENT_OPTIONS")); +} + +/// @todo: lease ownership + /// @todo: Implement proper tests for MySQL lease/host database, /// see ticket #4214. diff --git a/src/bin/dhcp4/tests/dhcp4_test_utils.h b/src/bin/dhcp4/tests/dhcp4_test_utils.h index 249429709e..c576c1d674 100644 --- a/src/bin/dhcp4/tests/dhcp4_test_utils.h +++ b/src/bin/dhcp4/tests/dhcp4_test_utils.h @@ -316,6 +316,7 @@ public: using Dhcpv4Srv::srvidToString; using Dhcpv4Srv::classifyPacket; using Dhcpv4Srv::deferredUnpack; + using Dhcpv4Srv::recoverStashedAgentOption; using Dhcpv4Srv::accept; using Dhcpv4Srv::acceptMessageType; using Dhcpv4Srv::selectSubnet; |