[3279] The server identifier should be encapsulated by the OptionCustom.

diff --git a/src/bin/dhcp4/dhcp4_srv.cc b/src/bin/dhcp4/dhcp4_srv.cc
index 9fcbd4e02dc7f54d1ea82d320997a1ee21fbeb3c..22ab6536bba378b2fdaa709bccdcd8c45f29152a 100644 (file)
--- a/src/bin/dhcp4/dhcp4_srv.cc
+++ b/src/bin/dhcp4/dhcp4_srv.cc
@@ -1551,22 +1551,26 @@ Dhcpv4Srv::acceptServerId(const Pkt4Ptr& pkt) const {
     }
     // Server identifier is present. Let's convert it to 4-byte address
     // and try to match with server identifiers used by the server.
-    Option4AddrLstPtr option_addrs =
-        boost::dynamic_pointer_cast<Option4AddrLst>(option);
+    OptionCustomPtr option_custom =
+        boost::dynamic_pointer_cast<OptionCustom>(option);
     // Unable to convert the option to the option type which encapsulates it.
     // We treat this as non-matching server id.
-    if (!option_addrs) {
+    if (!option_custom) {
         return (false);
     }
-    Option4AddrLst::AddressContainer addrs = option_addrs->getAddresses();
-
     // The server identifier option should carry exactly one IPv4 address.
-    // This option is encapsulated by the class which accepts a list of
-    // IPv4 addresses. So, there is a potential risk that the client has sent
-    // a server identifier option with multiple addresses and it has been
-    // parsed by the server. Here we are filtering out such malformed
-    // messages here.
-    if (addrs.size() != 1) {
+    // If the option definition for the server identifier doesn't change,
+    // the OptionCustom object should have exactly one IPv4 address and
+    // this check is somewhat redundant. On the other hand, if someone
+    // breaks option it may be better to check that here.
+    if (option_custom->getDataFieldsNum() != 1) {
+        return (false);
+    }
+
+    // The server identifier MUST be an IPv4 address. If given address is
+    // v6, it is wrong.
+    IOAddress server_id = option_custom->readAddress();
+    if (!server_id.isV4()) {
         return (false);
     }
 
@@ -1578,7 +1582,7 @@ Dhcpv4Srv::acceptServerId(const Pkt4Ptr& pkt) const {
     // performance hit should be acceptable. If it turns out to
     // be significant, we will have to cache server identifiers
     // when sockets are opened.
-    return (IfaceMgr::instance().hasOpenSocket(addrs[0]));
+    return (IfaceMgr::instance().hasOpenSocket(server_id));
 }
 
 void

diff --git a/src/bin/dhcp4/tests/dhcp4_srv_unittest.cc b/src/bin/dhcp4/tests/dhcp4_srv_unittest.cc
index 2871c903d3c4ba09a083c6ff3970d84ebf335327..057847ceda2abb621344407837cbfe8e0aa4a3c8 100644 (file)
--- a/src/bin/dhcp4/tests/dhcp4_srv_unittest.cc
+++ b/src/bin/dhcp4/tests/dhcp4_srv_unittest.cc
@@ -1046,12 +1046,16 @@ TEST_F(Dhcpv4SrvFakeIfaceTest, acceptServerId) {
     // accepted.
     EXPECT_TRUE(srv.acceptServerId(pkt));
 
+    // Create definition of the server identifier option.
+    OptionDefinition def("server-identifier", DHO_DHCP_SERVER_IDENTIFIER,
+                         "ipv4-address", false);
+
     // Add a server identifier option which doesn't match server ids being
     // used by the server. The accepted server ids are the IPv4 addresses
     // configured on the interfaces. The 10.1.2.3 is not configured on
-    // any interface.
-    OptionPtr other_serverid(new Option4AddrLst(DHO_DHCP_SERVER_IDENTIFIER,
-                                                IOAddress("10.1.2.3")));
+    // any interfaces.
+    OptionCustomPtr other_serverid(new OptionCustom(def, Option::V6));
+    other_serverid->writeAddress(IOAddress("10.1.2.3"));
     pkt->addOption(other_serverid);
     EXPECT_FALSE(srv.acceptServerId(pkt));
 
@@ -1060,8 +1064,8 @@ TEST_F(Dhcpv4SrvFakeIfaceTest, acceptServerId) {
 
     // Add a server id being an IPv4 address configured on eth0 interface.
     // A DHCPv4 message holding this server identifier should be accepted.
-    OptionPtr eth0_serverid(new Option4AddrLst(DHO_DHCP_SERVER_IDENTIFIER,
-                                               IOAddress("192.0.3.1")));
+    OptionCustomPtr eth0_serverid(new OptionCustom(def, Option::V6));
+    eth0_serverid->writeAddress(IOAddress("192.0.3.1"));
     ASSERT_NO_THROW(pkt->addOption(eth0_serverid));
     EXPECT_TRUE(srv.acceptServerId(pkt));
 
@@ -1070,8 +1074,8 @@ TEST_F(Dhcpv4SrvFakeIfaceTest, acceptServerId) {
 
     // Add a server id being an IPv4 address configured on eth1 interface.
     // A DHCPv4 message holding this server identifier should be accepted.
-    OptionPtr eth1_serverid(new Option4AddrLst(DHO_DHCP_SERVER_IDENTIFIER,
-                                               IOAddress("10.0.0.1")));
+    OptionCustomPtr eth1_serverid(new OptionCustom(def, Option::V6));
+    eth1_serverid->writeAddress(IOAddress("10.0.0.1"));
     ASSERT_NO_THROW(pkt->addOption(eth1_serverid));
     EXPECT_TRUE(srv.acceptServerId(pkt));