[master] Merge branch 'trac4317'

src/bin/dhcp4/dhcp4_srv.cc

@@ -269,7 +269,6 @@ Dhcpv4Exchange::setHostIdentifiers() {
                                                     std::vector<uint8_t>(vec.begin() + 5,
                                                                          vec.end()));
                     }
-                    /// @todo Add support for other client identifiers (see #4317).
                 }
             }
             break;
@@ -289,6 +288,15 @@ Dhcpv4Exchange::setHostIdentifiers() {
             }
             break;
 
+        case Host::IDENT_CLIENT_ID:
+            if (context_->clientid_) {
+                const std::vector<uint8_t>& vec = context_->clientid_->getDuid();
+                if (!vec.empty()) {
+                    context_->addHostIdentifier(id_type, vec);
+                }
+            }
+            break;
+
         default:
             ;
         }

src/bin/dhcp4/tests/config_parser_unittest.cc

@@ -3457,6 +3457,11 @@ TEST_F(Dhcp4ParserTest, reservations) {
         "        \"circuit-id\": \"060504030201\","
         "        \"ip-address\": \"192.0.4.102\","
         "        \"hostname\": \"\""
+        "      },"
+        "      {"
+        "        \"client-id\": \"05:01:02:03:04:05:06\","
+        "        \"ip-address\": \"192.0.4.103\","
+        "        \"hostname\": \"\""
         "      }"
         "    ]"
         " } ],"
@@ -3542,6 +3547,7 @@ TEST_F(Dhcp4ParserTest, reservations) {
     EXPECT_FALSE(hosts_cfg->get4(234, Host::IDENT_CIRCUIT_ID,
                                  &circuit_id[0], circuit_id.size()));
 
+
     // Repeat the test for the DUID based reservation in this subnet.
     duid.reset(new DUID(std::vector<uint8_t>(duid_vec.rbegin(),
                                              duid_vec.rend())));
@@ -3559,6 +3565,18 @@ TEST_F(Dhcp4ParserTest, reservations) {
     opt_ttl = retrieveOption<OptionUint8Ptr>(*host, DHO_DEFAULT_IP_TTL);
     ASSERT_TRUE(opt_ttl);
     EXPECT_EQ(95, static_cast<int>(opt_ttl->getValue()));
+
+    // Check that we can find reservation using client identifier.
+    ClientIdPtr client_id = ClientId::fromText("05:01:02:03:04:05:06");
+    host = hosts_cfg->get4(542, Host::IDENT_CLIENT_ID, &client_id->getDuid()[0],
+                           client_id->getDuid().size());
+    ASSERT_TRUE(host);
+    EXPECT_EQ("192.0.4.103", host->getIPv4Reservation().toText());
+
+    // But this reservation should not be returned for other subnet.
+    host = hosts_cfg->get4(234, Host::IDENT_CLIENT_ID, &client_id->getDuid()[0],
+                           client_id->getDuid().size());
+    EXPECT_FALSE(host);
 }
 
 // This test checks that it is possible to configure option data for a

src/bin/dhcp4/tests/dora_unittest.cc

@@ -147,6 +147,10 @@ const char* DORA_CONFIGS[] = {
         "       {"
         "         \"circuit-id\": \"'charter950'\","
         "         \"ip-address\": \"10.0.0.9\""
+        "       },"
+        "       {"
+        "         \"client-id\": \"01:11:22:33:44:55:66\","
+        "         \"ip-address\": \"10.0.0.1\""
         "       }"
         "    ]"
         "} ]"
@@ -173,7 +177,8 @@ const char* DORA_CONFIGS[] = {
     "{ \"interfaces-config\": {"
         "      \"interfaces\": [ \"*\" ]"
         "},"
-        "\"host-reservation-identifiers\": [ \"circuit-id\", \"hw-address\", \"duid\" ],"
+        "\"host-reservation-identifiers\": [ \"circuit-id\", \"hw-address\","
+        "                                    \"duid\", \"client-id\" ],"
         "\"valid-lifetime\": 600,"
         "\"subnet4\": [ { "
         "    \"subnet\": \"10.0.0.0/24\", "
@@ -190,6 +195,10 @@ const char* DORA_CONFIGS[] = {
         "       {"
         "         \"circuit-id\": \"'charter950'\","
         "         \"ip-address\": \"10.0.0.9\""
+        "       },"
+        "       {"
+        "         \"client-id\": \"01:11:22:33:44:55:66\","
+        "         \"ip-address\": \"10.0.0.1\""
         "       }"
         "    ]"
         "} ]"
@@ -199,7 +208,8 @@ const char* DORA_CONFIGS[] = {
     "{ \"interfaces-config\": {"
         "      \"interfaces\": [ \"*\" ]"
         "},"
-        "\"host-reservation-identifiers\": [ \"duid\", \"circuit-id\", \"hw-address\" ],"
+        "\"host-reservation-identifiers\": [ \"duid\", \"client-id\","
+        "                                    \"circuit-id\", \"hw-address\" ],"
         "\"valid-lifetime\": 600,"
         "\"subnet4\": [ { "
         "    \"subnet\": \"10.0.0.0/24\", "
@@ -216,6 +226,10 @@ const char* DORA_CONFIGS[] = {
         "       {"
         "         \"circuit-id\": \"'charter950'\","
         "         \"ip-address\": \"10.0.0.9\""
+        "       },"
+        "       {"
+        "         \"client-id\": \"01:11:22:33:44:55:66\","
+        "         \"ip-address\": \"10.0.0.1\""
         "       }"
         "    ]"
         "} ]"
@@ -852,7 +866,7 @@ TEST_F(DORATest, hostIdentifiersOrder) {
 
     // Reconfigure the server to change the preference order of the
     // host identifiers. The 'circuit-id' should now take precedence over
-    // the hw-address and duid.
+    // the hw-address, duid and client-id.
     configure(DORA_CONFIGS[4], *client.getServer());
     ASSERT_NO_THROW(client.doDORA(boost::shared_ptr<
                                   IOAddress>(new IOAddress("0.0.0.0"))));
@@ -866,7 +880,7 @@ TEST_F(DORATest, hostIdentifiersOrder) {
 
     // Reconfigure the server to change the preference order of the
     // host identifiers. The 'duid' should now take precedence over
-    // the hw-address and circuit-id
+    // the client-id, hw-address and circuit-id
     configure(DORA_CONFIGS[5], *client.getServer());
     ASSERT_NO_THROW(client.doDORA(boost::shared_ptr<
                                   IOAddress>(new IOAddress("0.0.0.0"))));
@@ -877,6 +891,23 @@ TEST_F(DORATest, hostIdentifiersOrder) {
     ASSERT_EQ(DHCPACK, static_cast<int>(resp->getType()));
     // Make sure that the client has got the lease for the reserved address.
     ASSERT_EQ("10.0.0.8", client.config_.lease_.addr_.toText());
+
+    // Replace the client identifier with the one for which address
+    // 10.0.0.1 is reserved. Because the DUID is a special type of
+    // client identifier, this change effectively removes the association
+    // of the client with the DUID for which address 10.0.0.8 is reserved.
+    // The next identifier type to be used by the server (after DUID) is
+    // client-id and thus the server should assign address 10.0.0.1.
+    client.includeClientId("01:11:22:33:44:55:66");
+    ASSERT_NO_THROW(client.doDORA(boost::shared_ptr<
+                                  IOAddress>(new IOAddress("0.0.0.0"))));
+    // Make sure that the server responded.
+    ASSERT_TRUE(client.getContext().response_);
+    resp = client.getContext().response_;
+    // Make sure that the server has responded with DHCPACK.
+    ASSERT_EQ(DHCPACK, static_cast<int>(resp->getType()));
+    // Make sure that the client has got the lease for the reserved address.
+    ASSERT_EQ("10.0.0.1", client.config_.lease_.addr_.toText());
 }
 
 // This test checks that setting the match-client-id value to false causes

src/lib/dhcpsrv/host.cc

@@ -136,6 +136,9 @@ Host::getIdentifierType(const std::string& identifier_name) {
     } else if (identifier_name == "circuit-id") {
         return (IDENT_CIRCUIT_ID);
 
+    } else if (identifier_name == "client-id") {
+        return (IDENT_CLIENT_ID);
+
     } else {
         isc_throw(isc::BadValue, "invalid client identifier type '"
                   << identifier_name << "'");
@@ -176,6 +179,9 @@ Host::getIdentifierAsText(const IdentifierType& type, const uint8_t* value,
     case IDENT_CIRCUIT_ID:
         s << "circuit-id";
         break;
+    case IDENT_CLIENT_ID:
+        s << "client-id";
+        break;
     default:
         // This should never happen actually, unless we add new identifier
         // and forget to add a case for it above.
@@ -198,6 +204,9 @@ Host::getIdentifierName(const IdentifierType& type) {
     case Host::IDENT_CIRCUIT_ID:
         return ("circuit-id");
 
+    case Host::IDENT_CLIENT_ID:
+        return ("client-id");
+
     default:
         ;
     }

src/lib/dhcpsrv/host.h

@@ -175,17 +175,21 @@ public:
 
     /// @brief Type of the host identifier.
     ///
-    /// Currently hardware address assigned to an interface and the
-    /// DHCPv6 client's DUID are supported.
+    /// Currently supported identifiers are:
+    /// - hardware address (DHCPv4 and DHCPv6) (identifier name: "hw-address"),
+    /// - DUID (DHCPv4 and DHCPv6) (identifier name: "duid"),
+    /// - circuit identifier (DHCPv4) (identifier name: "circuit-id"),
+    /// - client identifier (DHCPv4) (identifier name: "client-id")
     enum IdentifierType {
         IDENT_HWADDR,
         IDENT_DUID,
-        IDENT_CIRCUIT_ID
+        IDENT_CIRCUIT_ID,
+        IDENT_CLIENT_ID
     };
 
     /// @brief Constant pointing to the last identifier of the
     /// @ref IdentifierType enumeration.
-    static const IdentifierType LAST_IDENTIFIER_TYPE = IDENT_CIRCUIT_ID;
+    static const IdentifierType LAST_IDENTIFIER_TYPE = IDENT_CLIENT_ID;
 
     /// @brief Constructor.
     ///
@@ -240,7 +244,8 @@ public:
     ///
     /// @param identifier Identifier in the textual format. The expected formats
     /// for the hardware address and other identifiers are provided above.
-    /// @param identifier_name One of "hw-address", "duid", "circuit-id".
+    /// @param identifier_name One of the supported identifiers in the text form as
+    /// described for @ref IdentifierType.
     /// @param ipv4_subnet_id Identifier of the IPv4 subnet to which the host
     /// is connected.
     /// @param ipv6_subnet_id Identifier of the IPv6 subnet to which the host
@@ -282,7 +287,8 @@ public:
     /// This method is called by the @c Host constructor.
     ///
     /// @param identifier Reference to a new identifier in the textual format.
-    /// @param name One of "hw-address", "duid", "circuit-id".
+    /// @param name One of the supported identifiers in the text form as
+    /// described for @ref IdentifierType.
     ///
     /// @throw BadValue if the identifier is invalid.
     void setIdentifier(const std::string& identifier, const std::string& name);

src/lib/dhcpsrv/parsers/host_reservation_parser.cc

@@ -41,6 +41,7 @@ getSupportedParams4(const bool identifiers_only = false) {
         identifiers_set.insert("hw-address");
         identifiers_set.insert("duid");
         identifiers_set.insert("circuit-id");
+        identifiers_set.insert("client-id");
     }
     // Copy identifiers and add all other parameters.
     if (params_set.empty()) {

src/lib/dhcpsrv/tests/host_reservation_parser_unittest.cc

@@ -182,6 +182,9 @@ protected:
 
     /// @brief Vector holding circuit id used by tests.
     std::vector<uint8_t> circuit_id_;
+
+    /// @brief Vector holding client id used by tests.
+    std::vector<uint8_t> client_id_;
 };
 
 void
@@ -199,6 +202,11 @@ HostReservationParserTest::SetUp() {
 
     const std::string circuit_id_str = "howdy";
     circuit_id_.assign(circuit_id_str.begin(), circuit_id_str.end());
+
+    client_id_.push_back(0x01); // Client identifier type.
+    // Often client id comprises HW address.
+    client_id_.insert(client_id_.end(), hwaddr_->hwaddr_.begin(),
+                      hwaddr_->hwaddr_.end());
 }
 
 void
@@ -252,6 +260,22 @@ TEST_F(HostReservationParserTest, dhcp4CircuitIdHexWithPrefix) {
                     circuit_id_);
 }
 
+// This test verifies that the parser can parse a reservation entry for
+// which client-id is an identifier. The client-id is specified in
+// hexadecimal format.
+TEST_F(HostReservationParserTest, dhcp4ClientIdHex) {
+    testIdentifier4("client-id", "01010203040506", Host::IDENT_CLIENT_ID,
+                    client_id_);
+}
+
+// This test verifies that the parser can parse a reservation entry for
+// which client-id is an identifier. The client-id is specified in
+// hexadecimal format with a '0x' prefix.
+TEST_F(HostReservationParserTest, dhcp4ClientIdHexWithPrefix) {
+    testIdentifier4("client-id", "0x01010203040506", Host::IDENT_CLIENT_ID,
+                    client_id_);
+}
+
 // This test verifies that the parser can parse the reservation entry
 // when IPv4 address is specified, but hostname is not.
 TEST_F(HostReservationParserTest, dhcp4NoHostname) {
@@ -465,6 +489,17 @@ TEST_F(HostReservationParserTest, dhcp6CircuitId) {
     testInvalidConfig<HostReservationParser6>(config);
 }
 
+// This test verifies that host reservation parser for DHCPv6 rejects
+// "client-id" as a host identifier.
+TEST_F(HostReservationParserTest, dhcp6ClientId) {
+    // Use DHCPv4 specific identifier 'client-id' with DHCPv6 parser.
+    std::string config = "{ \"client-id\": \"01010203040506\","
+        "\"ip-addresses\": [ \"2001:db8:1::100\", \"2001:db8:1::200\" ],"
+        "\"prefixes\": [ ],"
+        "\"hostname\": \"foo.example.com\" }";
+    testInvalidConfig<HostReservationParser6>(config);
+}
+
 // This test verfies that the parser can parse the IPv6 reservation entry
 // which lacks hostname parameter.
 TEST_F(HostReservationParserTest, dhcp6NoHostname) {
@@ -818,7 +853,8 @@ public:
 // Test that list of supported DHCPv4 identifiers list is correctly
 // parsed.
 TEST_F(HostReservationIdsParserTest, dhcp4Identifiers) {
-    std::string config = "[ \"circuit-id\", \"duid\", \"hw-address\" ]";
+    std::string config =
+        "[ \"circuit-id\", \"duid\", \"hw-address\", \"client-id\" ]";
 
     ElementPtr config_element = Element::fromJSON(config);
 
@@ -828,12 +864,13 @@ TEST_F(HostReservationIdsParserTest, dhcp4Identifiers) {
     ConstCfgHostOperationsPtr cfg = CfgMgr::instance().getStagingCfg()->
         getCfgHostOperations4();
     const CfgHostOperations::IdentifierTypes& ids = cfg->getIdentifierTypes();
-    ASSERT_EQ(3, ids.size());
+    ASSERT_EQ(4, ids.size());
 
     CfgHostOperations::IdentifierTypes::const_iterator id = ids.begin();
     EXPECT_EQ(*id++, Host::IDENT_CIRCUIT_ID);
     EXPECT_EQ(*id++, Host::IDENT_DUID);
     EXPECT_EQ(*id++, Host::IDENT_HWADDR);
+    EXPECT_EQ(*id++, Host::IDENT_CLIENT_ID);
 }
 
 // Test that list of supported DHCPv6 identifiers list is correctly
@@ -883,12 +920,13 @@ TEST_F(HostReservationIdsParserTest, dhcp4AutoIdentifiers) {
     ConstCfgHostOperationsPtr cfg = CfgMgr::instance().getStagingCfg()->
         getCfgHostOperations4();
     const CfgHostOperations::IdentifierTypes& ids = cfg->getIdentifierTypes();
-    ASSERT_EQ(3, ids.size());
+    ASSERT_EQ(4, ids.size());
 
     CfgHostOperations::IdentifierTypes::const_iterator id = ids.begin();
     EXPECT_EQ(*id++, Host::IDENT_HWADDR);
     EXPECT_EQ(*id++, Host::IDENT_DUID);
     EXPECT_EQ(*id++, Host::IDENT_CIRCUIT_ID);
+    EXPECT_EQ(*id++, Host::IDENT_CLIENT_ID);
 }
 
 // This test verifies that use of "auto" together with an explicit

src/lib/dhcpsrv/tests/host_unittest.cc

@@ -23,7 +23,7 @@ namespace {
 /// @brief Holds a type of the last identifier in @c IdentifierType enum.
 ///
 /// This value must be updated when new identifiers are added to the enum.
-const Host::IdentifierType LAST_IDENTIFIER_TYPE = Host::IDENT_CIRCUIT_ID;
+const Host::IdentifierType LAST_IDENTIFIER_TYPE = Host::IDENT_CLIENT_ID;
 
 // This test verifies that it is possible to create IPv6 address
 // reservation.
@@ -180,6 +180,7 @@ TEST_F(HostTest, getIdentifier) {
     EXPECT_EQ(Host::IDENT_HWADDR, Host::getIdentifierType("hw-address"));
     EXPECT_EQ(Host::IDENT_DUID, Host::getIdentifierType("duid"));
     EXPECT_EQ(Host::IDENT_CIRCUIT_ID, Host::getIdentifierType("circuit-id"));
+    EXPECT_EQ(Host::IDENT_CLIENT_ID, Host::getIdentifierType("client-id"));
 
     EXPECT_THROW(Host::getIdentifierType("unuspported"), isc::BadValue);
 }