From 730c4de28dbe69a8e74d8c26f79a4a4022637828 Mon Sep 17 00:00:00 2001 From: Marcin Siodelski Date: Thu, 12 Oct 2017 11:40:49 +0200 Subject: [PATCH] [5376] Assign configured server identifier in outbound messages. --- src/bin/dhcp4/dhcp4_srv.cc | 9 ++- src/bin/dhcp4/dhcp4_srv.h | 12 ++-- src/bin/dhcp4/tests/dora_unittest.cc | 84 ++++++++++++++++++++++++---- 3 files changed, 89 insertions(+), 16 deletions(-) diff --git a/src/bin/dhcp4/dhcp4_srv.cc b/src/bin/dhcp4/dhcp4_srv.cc index 874d85449e..6e05c28bc0 100644 --- a/src/bin/dhcp4/dhcp4_srv.cc +++ b/src/bin/dhcp4/dhcp4_srv.cc @@ -1142,6 +1142,12 @@ Dhcpv4Srv::srvidToString(const OptionPtr& srvid) { void Dhcpv4Srv::appendServerID(Dhcpv4Exchange& ex) { + // Do not append generated server identifier if there is one appended already. + // This is when explicitly configured server identifier option is present. + if (ex.getResponse()->getOption(DHO_DHCP_SERVER_IDENTIFIER)) { + return; + } + // Use local address on which the packet has been received as a // server identifier. In some cases it may be a different address, // e.g. broadcast packet or DHCPv4o6 packet. @@ -1386,7 +1392,8 @@ Dhcpv4Srv::appendBasicOptions(Dhcpv4Exchange& ex) { static const uint16_t required_options[] = { DHO_ROUTERS, DHO_DOMAIN_NAME_SERVERS, - DHO_DOMAIN_NAME }; + DHO_DOMAIN_NAME, + DHO_DHCP_SERVER_IDENTIFIER }; static size_t required_options_size = sizeof(required_options) / sizeof(required_options[0]); diff --git a/src/bin/dhcp4/dhcp4_srv.h b/src/bin/dhcp4/dhcp4_srv.h index 0f07121a85..2fa90f127e 100644 --- a/src/bin/dhcp4/dhcp4_srv.h +++ b/src/bin/dhcp4/dhcp4_srv.h @@ -525,7 +525,8 @@ protected: /// - Subnet Mask, /// - Router, /// - Name Server, - /// - Domain Name. + /// - Domain Name, + /// - Server Identifier. /// /// @param ex DHCPv4 exchange holding the client's message to be checked. void appendBasicOptions(Dhcpv4Exchange& ex); @@ -681,10 +682,11 @@ protected: /// @brief Adds server identifier option to the server's response. /// - /// This method adds a server identifier to the DHCPv4 message. This is set - /// to the local address on which the client's query has been received with - /// the exception of broadcast traffic and DHCPv4o6 query for which a socket - /// on the particular interface is found and its address is used as server id. + /// This method adds a server identifier to the DHCPv4 message if it doesn't + /// exist yet. This is set to the local address on which the client's query has + /// been received with the exception of broadcast traffic and DHCPv4o6 query for + /// which a socket on the particular interface is found and its address is used + /// as server id. /// /// @note This method doesn't throw exceptions by itself but the underlying /// classes being used my throw. The reason for this method to not sanity diff --git a/src/bin/dhcp4/tests/dora_unittest.cc b/src/bin/dhcp4/tests/dora_unittest.cc index f6a23c4f73..691e6e3eb7 100644 --- a/src/bin/dhcp4/tests/dora_unittest.cc +++ b/src/bin/dhcp4/tests/dora_unittest.cc @@ -87,14 +87,19 @@ namespace { /// - boot-file-name = "bootfile.efi" /// /// - Configuration 7: +/// - Used for testing custom value of dhcp-server-identifier option. +/// - 1 subnets: 10.0.0.0/24 and 192.0.2.0/24 +/// - Custom server identifier specified for each subnet. +/// +/// - Configuration 8: /// - Simple configuration with a single subnet and single pool /// - Using MySQL lease database backend to store leases /// -/// - Configuration 8: +/// - Configuration 9: /// - Simple configuration with a single subnet and single pool /// - Using PostgreSQL lease database backend to store leases /// -/// - Configuration 9: +/// - Configuration 10: /// - Simple configuration with a single subnet and single pool /// - Using Cassandra lease database backend to store leases const char* DORA_CONFIGS[] = { @@ -282,6 +287,38 @@ const char* DORA_CONFIGS[] = { "}", // Configuration 7 + "{ \"interfaces-config\": {" + " \"interfaces\": [ \"*\" ]" + "}," + "\"valid-lifetime\": 600," + "\"subnet4\": [" + " {" + " \"subnet\": \"10.0.0.0/24\", " + " \"pools\": [ { \"pool\": \"10.0.0.10-10.0.0.100\" } ]," + " \"interface\": \"eth0\"," + " \"option-data\": [" + " {" + " \"name\": \"dhcp-server-identifier\"," + " \"data\": \"1.2.3.4\"" + " }" + " ]" + " }," + " {" + " \"subnet\": \"192.0.2.0/24\", " + " \"pools\": [ { \"pool\": \"192.0.2.10-192.0.2.100\" } ]," + " \"interface\": \"eth1\"," + " \"option-data\": [" + " {" + " \"name\": \"dhcp-server-identifier\"," + " \"data\": \"2.3.4.5\"" + " }" + " ]" + + " }" + "]" + "}", + +// Configuration 8 "{ \"interfaces-config\": {" " \"interfaces\": [ \"*\" ]" "}," @@ -299,7 +336,7 @@ const char* DORA_CONFIGS[] = { " } ]" "}", -// Configuration 8 +// Configuration 9 "{ \"interfaces-config\": {" " \"interfaces\": [ \"*\" ]" "}," @@ -317,7 +354,7 @@ const char* DORA_CONFIGS[] = { " } ]" "}", -// Configuration 9 +// Configuration 10 "{ \"interfaces-config\": {" " \"interfaces\": [ \"*\" ]" "}," @@ -1568,6 +1605,33 @@ TEST_F(DORATest, multiStageBoot) { testMultiStageBoot(0); } +// This test verifies that custom server identifier can be specified for +// a subnet. +TEST_F(DORATest, customServerIdentifier) { + Dhcp4Client client1(Dhcp4Client::SELECTING); + // Configure DHCP server. + ASSERT_NO_THROW(configure(DORA_CONFIGS[7], *client1.getServer())); + + ASSERT_NO_THROW(client1.doDORA()); + // Make sure that the server responded. + ASSERT_TRUE(client1.getContext().response_); + Pkt4Ptr resp = client1.getContext().response_; + // Make sure that the server has responded with DHCPACK. + ASSERT_EQ(DHCPACK, static_cast(resp->getType())); + // The explicitly configured server identifier should take precedence + // over generated server identifier. + EXPECT_EQ("1.2.3.4", client1.config_.serverid_.toText()); + + // Repeat the test for different subnet. + Dhcp4Client client2(client1.getServer(), Dhcp4Client::SELECTING); + client2.setIfaceName("eth1"); + ASSERT_NO_THROW(client2.doDORA()); + ASSERT_TRUE(client2.getContext().response_); + resp = client2.getContext().response_; + ASSERT_EQ(DHCPACK, static_cast(resp->getType())); + EXPECT_EQ("2.3.4.5", client2.config_.serverid_.toText()); +} + // Starting tests which require MySQL backend availability. Those tests // will not be executed if Kea has been compiled without the // --with-dhcp-mysql. @@ -1595,8 +1659,8 @@ public: // Test that the client using the same hardware address but multiple // client identifiers will obtain multiple leases (MySQL lease database). TEST_F(DORAMySQLTest, multiStageBoot) { - // DORA_CONFIGS[7] to be used for server configuration. - testMultiStageBoot(7); + // DORA_CONFIGS[9] to be used for server configuration. + testMultiStageBoot(8); } #endif @@ -1628,8 +1692,8 @@ public: // Test that the client using the same hardware address but multiple // client identifiers will obtain multiple leases (PostgreSQL lease database). TEST_F(DORAPgSQLTest, multiStageBoot) { - // DORA_CONFIGS[8] to be used for server configuration. - testMultiStageBoot(8); + // DORA_CONFIGS[9] to be used for server configuration. + testMultiStageBoot(9); } #endif @@ -1658,8 +1722,8 @@ public: // Test that the client using the same hardware address but multiple // client identifiers will obtain multiple leases (CQL lease database). TEST_F(DORACQLTest, multiStageBoot) { - // DORA_CONFIGS[9] to be used for server configuration. - testMultiStageBoot(9); + // DORA_CONFIGS[10] to be used for server configuration. + testMultiStageBoot(10); } #endif -- 2.47.2