[5306] Use match-client-id defined on shared network level.

src/bin/dhcp4/dhcp4_messages.mes

@@ -66,12 +66,10 @@ renewing existing lease. The server is explicitly configured to not use
 client identifier to lookup existing leases for the client and will not
 record client identifier in the lease database. This mode of operation
 is useful when clients don't use stable client identifiers, e.g. multi
-stage booting. Note that the client identifier may be used for other
-operations than lease allocation, e.g. identifying host reservations
-for the client using client identifier. The first argument includes the
-client and transaction identification information. The second argument
-specifies the identifier of the subnet where the client is connected
-and for which this mode of operation is configured on the server.
+stage booting. The first argument includes the client and transaction
+identification information. The second argument specifies the identifier
+of the subnet where the client is connected and for which this mode of
+operation is configured on the server.
 
 % DHCP4_CLIENT_FQDN_DATA %1: Client sent FQDN option: %2
 This debug message includes the detailed information extracted from the

src/bin/dhcp4/dhcp4_srv.cc

@@ -132,20 +132,13 @@ Dhcpv4Exchange::Dhcpv4Exchange(const AllocEnginePtr& alloc_engine,
     // Pointer to client's query.
     context_->query_ = query;
 
-    // Set client identifier if the match-client-id flag is enabled (default).
-    // If the subnet wasn't found it doesn't matter because we will not be
-    // able to allocate a lease anyway so this context will not be used.
+    // If subnet found, retrieve client identifier which will be needed
+    // for allocations and search for reservations associated with a
+    // subnet/shared network.
     if (subnet) {
-        if (subnet->getMatchClientId()) {
-            OptionPtr opt_clientid = query->getOption(DHO_DHCP_CLIENT_IDENTIFIER);
-            if (opt_clientid) {
-                context_->clientid_.reset(new ClientId(opt_clientid->getData()));
-            }
-        } else {
-            /// @todo When merging with #3806 use different logger.
-            LOG_DEBUG(dhcp4_logger, DBG_DHCP4_DETAIL, DHCP4_CLIENTID_IGNORED_FOR_LEASES)
-                .arg(query->getLabel())
-                .arg(subnet->getID());
+        OptionPtr opt_clientid = query->getOption(DHO_DHCP_CLIENT_IDENTIFIER);
+        if (opt_clientid) {
+            context_->clientid_.reset(new ClientId(opt_clientid->getData()));
         }
 
         // Find static reservations if not disabled for our subnet.
@@ -1847,6 +1840,16 @@ Dhcpv4Srv::assignLease(Dhcpv4Exchange& ex) {
             .arg(query->getLabel())
             .arg(lease->addr_.toText());
 
+        // We're logging this here, because this is the place where we know
+        // which subnet has been actually used for allocation. If the
+        // client identifier matching is disabled, we want to make sure that
+        // the user is notified.
+        if (!ctx->subnet_->getMatchClientId()) {
+            LOG_DEBUG(dhcp4_logger, DBG_DHCP4_DETAIL, DHCP4_CLIENTID_IGNORED_FOR_LEASES)
+                .arg(ctx->query_->getLabel())
+                .arg(ctx->subnet_->getID());
+        }
+
         resp->setYiaddr(lease->addr_);
 
         /// @todo The server should check what ciaddr the client has supplied

src/bin/dhcp4/tests/shared_network_unittest.cc

@@ -596,6 +596,70 @@ const char* NETWORKS_CONFIG[] = {
     "            ]"
     "        }"
     "    ]"
+    "}",
+
+    // Configuration #11.
+    "{"
+    "    \"interfaces-config\": {"
+    "        \"interfaces\": [ \"*\" ]"
+    "    },"
+    "    \"valid-lifetime\": 600,"
+    "    \"match-client-id\": false,"
+    "    \"shared-networks\": ["
+    "        {"
+    "            \"name\": \"frog\","
+    "            \"interface\": \"eth1\","
+    "            \"match-client-id\": true,"
+    "            \"subnet4\": ["
+    "                {"
+    "                    \"subnet\": \"192.0.2.0/26\","
+    "                    \"id\": 10,"
+    "                    \"match-client-id\": false"
+    "                },"
+    "                {"
+    "                    \"subnet\": \"192.0.2.64/26\","
+    "                    \"id\": 100,"
+    "                    \"pools\": ["
+    "                        {"
+    "                            \"pool\": \"192.0.2.65 - 192.0.2.127\""
+    "                        }"
+    "                    ]"
+    "                }"
+    "            ]"
+    "        }"
+    "    ]"
+    "}",
+
+    // Configuration #12.
+    "{"
+    "    \"interfaces-config\": {"
+    "        \"interfaces\": [ \"*\" ]"
+    "    },"
+    "    \"valid-lifetime\": 600,"
+    "    \"match-client-id\": false,"
+    "    \"shared-networks\": ["
+    "        {"
+    "            \"name\": \"frog\","
+    "            \"interface\": \"eth1\","
+    "            \"match-client-id\": false,"
+    "            \"subnet4\": ["
+    "                {"
+    "                    \"subnet\": \"192.0.2.0/26\","
+    "                    \"id\": 10,"
+    "                    \"match-client-id\": false"
+    "                },"
+    "                {"
+    "                    \"subnet\": \"192.0.2.64/26\","
+    "                    \"id\": 100,"
+    "                    \"pools\": ["
+    "                        {"
+    "                            \"pool\": \"192.0.2.65 - 192.0.2.127\""
+    "                        }"
+    "                    ]"
+    "                }"
+    "            ]"
+    "        }"
+    "    ]"
     "}"
 };
 
@@ -1196,4 +1260,41 @@ TEST_F(Dhcpv4SharedNetworkTest, sharedNetworkSelectionByRelay) {
     EXPECT_EQ("10.0.0", resp2->getYiaddr().toText().substr(0, 6));
 }
 
+// Client id matching gets disabled on the shared network level.
+TEST_F(Dhcpv4SharedNetworkTest, matchClientId) {
+    // Create client using client identifier besides MAC address.
+    Dhcp4Client client(Dhcp4Client::SELECTING);
+    client.includeClientId("01:02:03:04");
+    client.setIfaceName("eth1");
+
+    // Create server configuration with match-client-id value initially
+    // set to true. The client should be allocated a lease and the
+    // client identifier should be included in this lease.
+    configure(NETWORKS_CONFIG[11], *client.getServer());
+
+    // Perform 4-way exchange.
+    ASSERT_NO_THROW(client.doDORA());
+    Pkt4Ptr resp1 = client.getContext().response_;
+    ASSERT_TRUE(resp1);
+    ASSERT_EQ(DHCPACK, resp1->getType());
+
+    // Reconfigure the server and turn off client identifier matching
+    // on the shared network level. The subnet from which the client
+    // is allocated an address should derive the match-client-id value
+    // and ignore the fact that the client identifier is not matching.
+    configure(NETWORKS_CONFIG[12], *client.getServer());
+
+    client.includeClientId("01:01:01:01");
+    client.setState(Dhcp4Client::RENEWING);
+
+    // Try to renew the lease with modified MAC address.
+    ASSERT_NO_THROW(client.doRequest());
+    Pkt4Ptr resp2 = client.getContext().response_;
+    ASSERT_TRUE(resp2);
+    ASSERT_EQ(DHCPACK, resp2->getType());
+
+    // The lease should get rewnewed.
+    EXPECT_EQ(resp2->getYiaddr().toText(), resp1->getYiaddr().toText());
+}
+
 } // end of anonymous namespace

src/lib/dhcpsrv/alloc_engine.cc

@@ -2224,13 +2224,18 @@ void findClientLease(AllocEngine::ClientContext4& ctx, Lease4Ptr& client_lease)
 
     while (subnet) {
 
+        ClientIdPtr client_id;
+        if (subnet->getMatchClientId()) {
+            client_id = ctx.clientid_;
+        }
+
         // If client identifier has been supplied, use it to lookup the lease. This
         // search will return no lease if the client doesn't have any lease in the
         // database or if the client didn't use client identifier to allocate the
         // existing lease (this include cases when the server was explicitly
         // configured to ignore client identifier).
-        if (ctx.clientid_) {
-            client_lease = lease_mgr.getLease4(*ctx.clientid_, subnet->getID());
+        if (client_id) {
+            client_lease = lease_mgr.getLease4(*client_id, subnet->getID());
         }
 
         // If no lease found using the client identifier, try the lookup using
@@ -2246,7 +2251,7 @@ void findClientLease(AllocEngine::ClientContext4& ctx, Lease4Ptr& client_lease)
                  client_lease_it != client_leases.end(); ++client_lease_it) {
                 Lease4Ptr existing_lease = *client_lease_it;
                 if ((existing_lease->subnet_id_ == subnet->getID()) &&
-                    existing_lease->belongsToClient(ctx.hwaddr_, ctx.clientid_)) {
+                    existing_lease->belongsToClient(ctx.hwaddr_, client_id)) {
                     // Found the lease of this client, so return it.
                     client_lease = existing_lease;
                     // We got a lease but the subnet it belongs to may differ from
@@ -2551,7 +2556,8 @@ AllocEngine::requestLease4(AllocEngine::ClientContext4& ctx) {
         // If it is in use by another client, the address can't be
         // allocated.
         if (existing && !existing->expired() &&
-            !existing->belongsToClient(ctx.hwaddr_, ctx.clientid_)) {
+            !existing->belongsToClient(ctx.hwaddr_, ctx.subnet_->getMatchClientId() ?
+                                       ctx.clientid_ : ClientIdPtr())) {
 
             LOG_DEBUG(alloc_engine_logger, ALLOC_ENGINE_DBG_TRACE,
                       ALLOC_ENGINE_V4_REQUEST_IN_USE)
@@ -2689,7 +2695,7 @@ AllocEngine::createLease4(const ClientContext4& ctx, const IOAddress& addr) {
 
     // @todo: remove this kludge after ticket #2590 is implemented
     std::vector<uint8_t> local_copy;
-    if (ctx.clientid_) {
+    if (ctx.clientid_ && ctx.subnet_->getMatchClientId()) {
         local_copy = ctx.clientid_->getDuid();
     }
     const uint8_t* local_copy0 = local_copy.empty() ? 0 : &local_copy[0];
@@ -2832,7 +2838,8 @@ AllocEngine::renewLease4(const Lease4Ptr& lease,
         // Pass the parameters
         ctx.callout_handle_->setArgument("query4", ctx.query_);
         ctx.callout_handle_->setArgument("subnet4", subnet4);
-        ctx.callout_handle_->setArgument("clientid", ctx.clientid_);
+        ctx.callout_handle_->setArgument("clientid", subnet4->getMatchClientId() ?
+                                         ctx.clientid_ : ClientIdPtr());
         ctx.callout_handle_->setArgument("hwaddr", ctx.hwaddr_);
 
         // Pass the lease to be updated
@@ -2999,10 +3006,15 @@ AllocEngine::allocateUnreservedLease4(ClientContext4& ctx) {
     uint64_t total_attempts = 0;
     while (subnet) {
 
+        ClientIdPtr client_id;
+        if (subnet->getMatchClientId()) {
+            client_id = ctx.clientid_;
+        }
+
         const uint64_t max_attempts = (attempts_ > 0 ? attempts_ :
                                        subnet->getPoolCapacity(Lease::TYPE_V4));
         for (uint64_t i = 0; i < max_attempts; ++i) {
-            IOAddress candidate = allocator->pickAddress(subnet, ctx.clientid_,
+            IOAddress candidate = allocator->pickAddress(subnet, client_id,
                                                          ctx.requested_address_);
             // If address is not reserved for another client, try to allocate it.
             if (!addressReserved(candidate, ctx)) {
@@ -3046,7 +3058,7 @@ AllocEngine::updateLease4Information(const Lease4Ptr& lease,
                                      AllocEngine::ClientContext4& ctx) const {
     lease->subnet_id_ = ctx.subnet_->getID();
     lease->hwaddr_ = ctx.hwaddr_;
-    lease->client_id_ = ctx.clientid_;
+    lease->client_id_ = ctx.subnet_->getMatchClientId() ? ctx.clientid_ : ClientIdPtr();
     lease->cltt_ = time(NULL);
     lease->t1_ = ctx.subnet_->getT1();
     lease->t2_ = ctx.subnet_->getT2();