From: Tomek Mrugalski Date: Mon, 3 Apr 2017 11:33:34 +0000 (+0200) Subject: [5132] Changes after review (comment #9): X-Git-Tag: trac5196_base~12^2~4 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=cf9be2f7fca8536ed5b2dc860d5ac8315e3e44f7;p=thirdparty%2Fkea.git [5132] Changes after review (comment #9): - new tests added - needed => required - spaced added where needed - CfgHosts::toElement6 updated - --- diff --git a/src/bin/dhcp4/dhcp4_hooks.dox b/src/bin/dhcp4/dhcp4_hooks.dox index 3e0a120a75..7dae8825e5 100644 --- a/src/bin/dhcp4/dhcp4_hooks.dox +++ b/src/bin/dhcp4/dhcp4_hooks.dox @@ -118,7 +118,7 @@ to the end of this list. - @b Description: this callout is executed only if flexible identifiers are enabled, i.e. host-reservation-identifiers contain 'flex-id' value. This callout enables external library to provide values for flexible identifiers. - To be able to use this feature, flex_id hook library is needed. + To be able to use this feature, flex_id hook library is required. - Next step status: If a callout installed on the "host4_identifier" hook point sets the next step status to value other than NEXT_STEP_CONTINUE, the diff --git a/src/bin/dhcp4/tests/hooks_unittest.cc b/src/bin/dhcp4/tests/hooks_unittest.cc index d84e291ff1..944927736e 100644 --- a/src/bin/dhcp4/tests/hooks_unittest.cc +++ b/src/bin/dhcp4/tests/hooks_unittest.cc @@ -36,13 +36,13 @@ TEST_F(Dhcpv4SrvTest, Hooks) { NakedDhcpv4Srv srv(0); // check if appropriate hooks are registered - int hook_index_buffer4_receive = -1; - int hook_index_pkt4_receive = -1; - int hook_index_select_subnet = -1; - int hook_index_lease4_release = -1; - int hook_index_pkt4_send = -1; - int hook_index_buffer4_send = -1; - int hook_index_host4_identifier= -1; + int hook_index_buffer4_receive = -1; + int hook_index_pkt4_receive = -1; + int hook_index_select_subnet = -1; + int hook_index_lease4_release = -1; + int hook_index_pkt4_send = -1; + int hook_index_buffer4_send = -1; + int hook_index_host4_identifier = -1; // check if appropriate indexes are set EXPECT_NO_THROW(hook_index_buffer4_receive = ServerHooks::getServerHooks() @@ -555,16 +555,54 @@ public: host4_identifier_foo_callout(CalloutHandle& handle) { callback_name_ = string("host4_identifier"); - std::vector id(3); - id[0] = 0x66; // f - id[1] = 0x6f; // o - id[2] = 0x6f; // o + // Make sure the query4 parameter is passed. + handle.getArgument("query4", callback_qry_pkt4_); + + // Make sure id_type parameter is passed. + Host::IdentifierType type = Host::IDENT_FLEX; + handle.getArgument("id_type", type); + + // Make sure id_value parameter is passed. + std::vector id_test; + handle.getArgument("id_value", id_test); + + std::vector id = { 0x66, 0x6f, 0x6f }; // foo handle.setArgument("id_value", id); handle.setArgument("id_type", Host::IDENT_FLEX); return (0); } + /// @brief Test host4_identifier callout by setting identifier to hwaddr + /// + /// This callout always returns fixed HWADDR: 00:01:02:03:04:05 + /// + /// @param callout_handle handle passed by the hooks framework + /// @return always 0 + static int + host4_identifier_hwaddr_callout(CalloutHandle& handle) { + callback_name_ = string("host4_identifier"); + + // Make sure the query4 parameter is passed. + handle.getArgument("query4", callback_qry_pkt4_); + + // Make sure id_type parameter is passed. + Host::IdentifierType type = Host::IDENT_FLEX; + handle.getArgument("id_type", type); + + // Make sure id_value parameter is passed. + std::vector id_test; + handle.getArgument("id_value", id_test); + + // Ok, now set the identifier to 00:01:02:03:04:05 + std::vector id = { 0x00, 0x01, 0x02, 0x03, 0x04, 0x05 }; + handle.setArgument("id_value", id); + handle.setArgument("id_type", Host::IDENT_HWADDR); + + return (0); + } + + /// resets buffers used to store data received by callouts void resetCalloutBuffers() { callback_name_ = string(""); @@ -1793,6 +1831,75 @@ TEST_F(HooksDhcpv4SrvTest, host4_identifier) { EXPECT_EQ("192.0.2.201", adv->getYiaddr().toText()); } +// Checks if callout installed on host4_identifier can generate identifier of +// other type. This particular callout always returns hwaddr. +TEST_F(HooksDhcpv4SrvTest, host4_identifier_hwaddr) { + IfaceMgrTestConfig test_config(true); + IfaceMgr::instance().openSockets4(); + + // Configure a subnet with host reservation. The reservation is based on + // flexible identifier value of 'foo'. That's exactly what the + // host4_identifier_foo_callout sets. + string config = "{ \"interfaces-config\": {" + " \"interfaces\": [ \"*\" ]" + "}," + "\"rebind-timer\": 2000, " + "\"renew-timer\": 1000, " + "\"host-reservation-identifiers\": [ \"flex-id\" ], " + "\"subnet4\": [ { " + " \"pools\": [ { \"pool\": \"192.0.2.0/25\" } ]," + " \"subnet\": \"192.0.2.0/24\", " + " \"interface\": \"eth0\", " + " \"reservations\": [" + " {" + " \"hw-address\": \"00:01:02:03:04:05\"," + " \"ip-address\": \"192.0.2.201\"" + " }" + " ]" + "} ]," + "\"valid-lifetime\": 4000 }"; + + ConstElementPtr json; + EXPECT_NO_THROW(json = parseDHCP4(config)); + ASSERT_TRUE(json); + ConstElementPtr status; + + // Configure the server and make sure the config is accepted + EXPECT_NO_THROW(status = configureDhcp4Server(*srv_, json)); + ASSERT_TRUE(status); + comment_ = parseAnswer(rcode_, status); + ASSERT_EQ(0, rcode_); + + CfgMgr::instance().commit(); + + // Install host4_identifier_hwaddr_callout + EXPECT_NO_THROW(HooksManager::preCalloutsLibraryHandle().registerCallout( + "host4_identifier", host4_identifier_hwaddr_callout)); + + // Let's create a simple DISCOVER + Pkt4Ptr sol = generateSimpleDiscover(); + + // Simulate that we have received that traffic + srv_->fakeReceive(sol); + + // Server will now process to run its normal loop, but instead of calling + // IfaceMgr::receive4(), it will read all packets from the list set by + // fakeReceive() + // In particular, it should call registered pkt4_receive callback. + srv_->run(); + + // check that the server did send a response + ASSERT_EQ(1, srv_->fake_sent_.size()); + + // Make sure that we received a response + Pkt4Ptr adv = srv_->fake_sent_.front(); + ASSERT_TRUE(adv); + + // Make sure the address offered is the one that was reserved. + EXPECT_EQ("192.0.2.201", adv->getYiaddr().toText()); +} + + // Verifies that libraries are unloaded by server destruction // The callout libraries write their library index number to a marker // file upon load and unload, making it simple to test whether or not diff --git a/src/bin/dhcp6/tests/hooks_unittest.cc b/src/bin/dhcp6/tests/hooks_unittest.cc index 7ce8baf9ad..dffbb207f2 100644 --- a/src/bin/dhcp6/tests/hooks_unittest.cc +++ b/src/bin/dhcp6/tests/hooks_unittest.cc @@ -53,17 +53,16 @@ TEST_F(Dhcpv6SrvTest, Hooks) { NakedDhcpv6Srv srv(0); // check if appropriate hooks are registered - int hook_index_buffer6_receive = -1; - int hook_index_buffer6_send = -1; - int hook_index_lease6_renew = -1; - int hook_index_lease6_release = -1; - int hook_index_lease6_rebind = -1; - int hook_index_lease6_decline = -1; - int hook_index_pkt6_received = -1; - int hook_index_select_subnet = -1; - int hook_index_pkt6_send = -1; - int hook_index_host6_identifier= -1; - + int hook_index_buffer6_receive = -1; + int hook_index_buffer6_send = -1; + int hook_index_lease6_renew = -1; + int hook_index_lease6_release = -1; + int hook_index_lease6_rebind = -1; + int hook_index_lease6_decline = -1; + int hook_index_pkt6_received = -1; + int hook_index_select_subnet = -1; + int hook_index_pkt6_send = -1; + int hook_index_host6_identifier = -1; // check if appropriate indexes are set EXPECT_NO_THROW(hook_index_buffer6_receive = ServerHooks::getServerHooks() @@ -666,19 +665,46 @@ public: handle.getArgument("id_type", type); // Make sure id_value parameter is passed. - std::vector id; - handle.getArgument("id_value", id); + std::vector id_test; + handle.getArgument("id_value", id_test); - id.resize(3); - id[0] = 0x66; // f - id[1] = 0x6f; // o - id[2] = 0x6f; // o + // Ok, now set the identifier. + std::vector id = { 0x66, 0x6f, 0x6f }; // foo handle.setArgument("id_value", id); handle.setArgument("id_type", Host::IDENT_FLEX); return (0); } + /// @brief Test host4_identifier callout by setting identifier to hwaddr + /// + /// This callout always returns fixed HWADDR: 00:01:02:03:04:05 + /// + /// @param callout_handle handle passed by the hooks framework + /// @return always 0 + static int + host6_identifier_hwaddr_callout(CalloutHandle& handle) { + callback_name_ = string("host6_identifier"); + + // Make sure the query6 parameter is passed. + handle.getArgument("query6", callback_qry_pkt6_); + + // Make sure id_type parameter is passed. + Host::IdentifierType type = Host::IDENT_FLEX; + handle.getArgument("id_type", type); + + // Make sure id_value parameter is passed. + std::vector id_test; + handle.getArgument("id_value", id_test); + + // Ok, now set the identifier to 00:01:02:03:04:05 + std::vector id = { 0x00, 0x01, 0x02, 0x03, 0x04, 0x05 }; + handle.setArgument("id_value", id); + handle.setArgument("id_type", Host::IDENT_HWADDR); + + return (0); + } + /// Resets buffers used to store data received by callouts void resetCalloutBuffers() { @@ -2264,7 +2290,6 @@ TEST_F(HooksDhcpv6SrvTest, lease6DeclineDrop) { EXPECT_EQ(Lease::STATE_DEFAULT, from_mgr->state_); } - // Checks if callout installed on host6_identifier can generate an // identifier and whether that identifier is actually used. TEST_F(HooksDhcpv6SrvTest, host6Identifier) { @@ -2338,6 +2363,79 @@ TEST_F(HooksDhcpv6SrvTest, host6Identifier) { ASSERT_EQ("2001:db8::f00", addr_opt->getAddress().toText()); } +// Checks if callout installed on host6_identifier can generate an identifier +// other type. This particular callout always returns hwaddr. +TEST_F(HooksDhcpv6SrvTest, host6Identifier_hwaddr) { + + // Configure 2 subnets, both directly reachable over local interface + // (let's not complicate the matter with relays) + string config = "{ \"interfaces-config\": {\n" + " \"interfaces\": [ \"*\" ]\n" + "},\n" + "\"preferred-lifetime\": 3000,\n" + "\"rebind-timer\": 2000,\n" + "\"renew-timer\": 1000,\n" + "\"host-reservation-identifiers\": [ \"flex-id\" ],\n" + "\"subnet6\": [ {\n" + " \"pools\": [ { \"pool\": \"2001:db8::/64\" } ],\n" + " \"subnet\": \"2001:db8::/48\", \n" + " \"interface\": \"" + valid_iface_ + "\",\n" + " \"reservations\": [\n" + " {\n" + " \"hw-address\": \"00:01:02:03:04:05\",\n" + " \"ip-addresses\": [ \"2001:db8::f00\" ]\n" + " }\n" + " ]\n" + " } ]\n," + "\"valid-lifetime\": 4000 }"; + + ConstElementPtr json; + EXPECT_NO_THROW(json = parseDHCP6(config)); + ConstElementPtr status; + + // Configure the server and make sure the config is accepted + EXPECT_NO_THROW(status = configureDhcp6Server(*srv_, json)); + ASSERT_TRUE(status); + comment_ = isc::config::parseAnswer(rcode_, status); + ASSERT_EQ(0, rcode_); + + CfgMgr::instance().commit(); + + // Install host6_identifier_foo_callout + EXPECT_NO_THROW(HooksManager::preCalloutsLibraryHandle().registerCallout( + "host6_identifier", host6_identifier_hwaddr_callout)); + + // Prepare solicit packet. Server should select first subnet for it + Pkt6Ptr sol = Pkt6Ptr(new Pkt6(DHCPV6_SOLICIT, 1234)); + sol->setRemoteAddr(IOAddress("fe80::abcd")); + sol->setIface(valid_iface_); + sol->addOption(generateIA(D6O_IA_NA, 234, 1500, 3000)); + OptionPtr clientid = generateClientId(); + sol->addOption(clientid); + + // Pass it to the server and get an advertise + Pkt6Ptr adv = srv_->processSolicit(sol); + + // Check if we get response at all + ASSERT_TRUE(adv); + + // Check that the callback called is indeed the one we installed + EXPECT_EQ("host6_identifier", callback_name_); + + // Check that pkt6 argument passing was successful and returned proper value + EXPECT_TRUE(callback_qry_pkt6_.get() == sol.get()); + + // Now check if we got the reserved address + OptionPtr tmp = adv->getOption(D6O_IA_NA); + ASSERT_TRUE(tmp); + + // Check that IA_NA was returned and that there's an address included + boost::shared_ptr addr_opt = checkIA_NA(adv, 234, 1000, 2000); + + ASSERT_TRUE(addr_opt); + ASSERT_EQ("2001:db8::f00", addr_opt->getAddress().toText()); +} + // Verifies that libraries are unloaded by server destruction // The callout libraries write their library index number to a marker diff --git a/src/lib/dhcpsrv/cfg_hosts.cc b/src/lib/dhcpsrv/cfg_hosts.cc index c16fb85633..935be7fbd9 100644 --- a/src/lib/dhcpsrv/cfg_hosts.cc +++ b/src/lib/dhcpsrv/cfg_hosts.cc @@ -775,6 +775,10 @@ CfgHosts::toElement6() const { isc_throw(ToElementError, "unexpected circuit-id DUID type"); } else if (id_type == Host::IDENT_CLIENT_ID) { isc_throw(ToElementError, "unexpected client-id DUID type"); + } else if (id_type == Host::IDENT_FLEX) { + const std::vector& bin = (*host)->getIdentifier(); + std::string flex = util::encode::encodeHex(bin); + map->set("flex-id", Element::create(flex)); } else { isc_throw(ToElementError, "invalid DUID type: " << id_type); }