[master] Merge branch 'trac3747'

doc/guide/dhcp4-srv.xml

@@ -1950,6 +1950,170 @@ temporarily override a list of interface names and listen on all interfaces.
 </screen>
     </section>
 
+    <section id="dhcp4-match-client-id">
+      <title>Using Client Identifier and Hardware Address</title>
+      <para>DHCP server must be able to identify the client (distinguish it from
+      other clients) from which it receives the message. There are many reasons
+      why this identification is required and the most important ones are listed
+      below.
+      <itemizedlist>
+        <listitem><simpara>When the client contacts the server to allocate a new
+        lease, the server must store the client identification information in
+        the lease database as a search key.</simpara></listitem>
+        <listitem><simpara>When the client is trying to renew or release the existing
+        lease, the server must be able to find the existing lease entry in the
+        database for this client, using the client identification information as a
+        search key.</simpara></listitem>
+        <listitem><simpara>Some configurations use static reservations for the IP
+        addreses and other configuration information. The server's administrator
+        uses client identification information to create these static assignments.
+        </simpara></listitem>
+        <listitem><simpara>In the dual stack networks there is often a need to
+        correlate the lease information stored in DHCPv4 and DHCPv6 server for
+        a particular host. Using common identification information by the DHCPv4
+        and DHCPv6 client allows the network administrator to achieve this
+        correlation and better administer the network.</simpara></listitem>
+      </itemizedlist>
+      </para>
+
+      <para>DHCPv4 makes use of two distinct identifiers which are placed
+      by the client in the queries sent to the server and copied by the server
+      to its responses to the client: 'chaddr' and 'client identifier'. The
+      former was introduced as a part of the BOOTP specification and it is also
+      used by DHCP to carry the hardware address of the interface used to send
+      the query to the server (MAC address for the Ethernet). The latter is
+      carried in the Client-identifier option, introduced in the
+      <ulink url="http://tools.ietf.org/html/rfc2132">RFC 2132</ulink>.
+      </para>
+
+      <para>The <ulink url="http://tools.ietf.org/html/rfc2131">RFC 2131</ulink>
+      indicates that the server may use both of these identifiers to identify
+      the client but the 'client identifier', if present, takes precedence
+      over 'chaddr'. One of the reasons for this is that 'client identifier'
+      is independent from the hardware used by the client to communicate with
+      the server. For example, if the client obtained the lease using one
+      network card and then the network card is moved to another host, the
+      server will wrongly identify this host is the one which has obtained
+      the lease. Moreover, the
+      <ulink url="https://tools.ietf.org/html/rfc4361">RFC 4361</ulink> gives
+      the recommendation to use DUID
+      (see <ulink url="https://tools.ietf.org/html/rfc3315">DHCPv6 specification</ulink>)
+      carried as 'client identifier' when dual stack networks are in use,
+      to provide consistent identification information of the client, regardless
+      of the protocol type it is using. Kea adheres to these specifications and
+      the 'client identifier' by default takes precedence over the value carried
+      in 'chaddr' field when the server searches, creates, updates or removes
+      the client's lease.
+      </para>
+
+      <para>When the server receives a DHCPDISCOVER or DHCPREQUEST message from the
+      client, it will try to find out if the client already has a lease in the
+      database and will hand out the existing lease rather than allocate
+      a new one. Each lease in the lease database is associated with the
+      'client identifier' and/or 'chaddr'. The server will first use the
+      'client identifer' (if present) to search the lease. If the lease is
+      found, the server will treat this lease as belonging to the client
+      even if the current 'chaddr' and the 'chaddr' associated with
+      the lease do not match. This facilitates the scenario when the network card
+      on the client system has been replaced and thus the new MAC address
+      appears in the messages sent by the DHCP client. If the server fails
+      to find the lease using the 'client identifier' it will perform another lookup
+      using the 'chaddr'. If this lookup returns no result, the client is
+      considered as not having a lease and the new lease will be created.
+      </para>
+
+      <para>A common problem reported by network operators is that bogus
+      client implementations do not use stable client identifiers such as
+      generating a new 'client identifier' each time the client connects
+      to the network. Another well known case is when the client changes its
+      'client identifier' during the multi-stage boot process (PXE). In such
+      cases, the MAC address of the client's interface remains stable and
+      using 'chaddr' field to identify the client guarantees that the
+      particular system is considered to be the same client, even though its
+      'client identifier' changes.
+      </para>
+
+      <para>To address this problem, Kea includes a configuration option
+      which enables client identification using 'chaddr' only by instructing
+      the server to disregard server to "ignore" the 'client identifier' during
+      lease lookups and allocations for a particular subnet. Consider the following
+      simplified server configuration:</para>
+<screen>
+"Dhcp4": {
+    ...
+    <userinput>"match-client-id": true,</userinput>
+    ...
+    "subnet4": [
+    {
+        "subnet": "192.0.10.0/24",
+        "pools": [ { "pool": "192.0.2.23-192.0.2.87" } ],
+        <userinput>"match-client-id": false</userinput>
+    },
+    {
+        "subnet": "10.0.0.0/8",
+        "pools": [ { "pool": "10.0.0.23-10.0.2.99" } ],
+    }
+    ]
+}
+</screen>
+
+     <para>The <command>match-client-id</command> is a boolean value which
+     controls this behavior. The default value of <userinput>true</userinput>
+     indicates that the server will use the 'client identifier' for lease
+     lookups and 'chaddr' if the first lookup returns no results. The
+     <command>false</command> means that the server will only
+     use the 'chaddr' to search for client's lease. Whether the DHCID for
+     DNS updates is generated from the 'client identifier' or 'chaddr' is
+     controlled through the same parameter accordingly.</para>
+
+     <para>The <command>match-client-id</command> parameter may appear
+     both in the global configuration scope and/or under any subnet
+     declaration. In the example shown above, the effective value of the
+     <command>match-client-id</command> will be <userinput>false</userinput>
+     for the subnet 192.0.10.0/24, because the subnet specific setting
+     of the parameter overrides the global value of the parameter. The
+     effective value of the <command>match-client-id</command> for the subnet
+     10.0.0.0/8 will be set to <userinput>true</userinput> because the
+     subnet declaration lacks this parameter and the global setting is
+     by default used for this subnet. In fact, the global entry for this
+     parameter could be omitted in this case, because
+     <userinput>true</userinput> is the default value.
+     </para>
+
+     <para>It is important to explain what happens when the client obtains
+     its lease for one setting of the <command>match-client-id</command>
+     and then renews when the setting has been changed. Let's first consider
+     the case when the client obtains the lease when the
+     <command>match-client-id</command> is set to <userinput>true</userinput>.
+     The server will store the lease information including 'client identifier'
+     (if supplied) and 'chaddr' in the lease database. When the setting is
+     changed and the client renews the lease the server will determine that
+     it should use the 'chaddr' to search for the existing lease. If the
+     client hasn't changed its MAC address the server should successfully
+     find the existing lease. The 'client identifier' associated with the
+     returned lease is ignored and the client is allowed to use this lease.
+     When the lease is renewed only the 'chaddr' is recorded for this
+     lease according to the new server setting.
+     </para>
+
+     <para>In the second case the client has the lease with only a 'chaddr'
+     value recorded. When the setting is changed to
+     <command>match-client-id</command> set to <userinput>true</userinput>
+     the server will first try to use the 'client identifier' to find the
+     existing client's lease. This will return no results because the
+     'client identifier' was not recorded for this lease. The server will
+     then use the 'chaddr' and the lease will be found. If the lease appears
+     to have no 'client identifier' recorded, the server will assume that
+     this lease belongs to the client and that it was created with the previous
+     setting of the <command>match-client-id</command>.
+     However, if the lease contains 'client identifier' which is different
+     from the 'client identifier' used by the client the lease will be
+     assumed to belong to another client and the new lease will be
+     allocated.
+     </para>
+
+    </section>
+
   </section> <!-- end of configuring kea-dhcp4 server section with many subsections -->
 
   <!-- Host reservation is a large topic. There will be many subsections,
@@ -2255,18 +2419,18 @@ temporarily override a list of interface names and listen on all interfaces.
 
       <para>
         An example configuration that disables reservation looks like follows:
-	<screen>
+<screen>
 "Dhcp4": {
     "subnet4": [
-	{
+    {
         "subnet": "192.0.2.0/24",
         <userinput>"reservation-mode": "disabled"</userinput>,
         ...
-	}
+    }
     ]
 }
 </screen>
-      </para>        
+      </para>
    </section>
 
   </section>

src/bin/admin/scripts/mysql/.gitignore

@@ -1 +1,2 @@
 /upgrade_1.0_to_2.0.sh
+/upgrade_2.0_to_3.0.sh

src/bin/dhcp4/dhcp4.spec

@@ -74,6 +74,12 @@
         "item_default": true
       },
 
+      { "item_name": "match-client-id",
+        "item_type": "boolean",
+        "item_optional": true,
+        "item_default": true
+      },
+
       { "item_name": "option-def",
         "item_type": "list",
         "item_optional": false,
@@ -269,6 +275,12 @@
                   "item_default": "0.0.0.0"
                 },
 
+                { "item_name": "match-client-id",
+                  "item_type": "boolean",
+                  "item_optional": true,
+                  "item_default": true
+                },
+
                 { "item_name": "pool",
                   "item_type": "list",
                   "item_optional": false,

src/bin/dhcp4/dhcp4_messages.mes

@@ -71,6 +71,20 @@ The first argument specifies the client and transaction identification
 information. The second argument includes all classes to which the
 packet has been assigned.
 
+% DHCP4_CLIENTID_IGNORED_FOR_LEASES %1: not using client identifier for lease allocation for subnet %2
+This debug message is issued when the server is processing the DHCPv4 message
+for which client identifier will not be used when allocating new lease or
+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.
+
 % DHCP4_CLIENT_FQDN_PROCESS %1: processing Client FQDN option
 This debug message is issued when the server starts processing the Client
 FQDN option sent in the client's query. The argument includes the
@@ -496,22 +510,13 @@ but no such lease is known to the server. The first argument contains
 the client and transaction identification information. The second
 argument contains the address which the client is trying to release.
 
-% DHCP4_RELEASE_FAIL_WRONG_CLIENT_ID %1: client tried to release address %2, but this address is leased to different client using client id %3
-This warning message indicates that client tried to release an address
-that belongs to a different client. This should not happen in normal
-circumstances and may indicate a misconfiguration of the client.  However,
-since the client releasing the address will stop using it anyway, there
-is a good chance that the situation will correct itself.
-
-% DHCP4_RELEASE_FAIL_WRONG_HWADDR %1: client tried to release address %2, but this address is leased to different client using HW address %3
-This warning message indicates that client tried to release an address
-that does belong to it, but the lease information was associated with
-a different hardware address. One possible reason for using different
-hardware address is that a cloned virtual machine was not updated and
-both clones use the same client-id. The first argument includes the
-client and the transaction identification information. The second
-argument includes the address which release was attempted. The
-third argumnet includes the HW address of the lease owner.
+% DHCP4_RELEASE_FAIL_WRONG_CLIENT %1: client is trying to release the lease %2 which belongs to a different client
+This debug message is issued when a client is trying to release the
+lease for the address which is currently used by another client, i.e.
+the 'client identifier' or 'chaddr' doesn't match between the client
+and the lease. The first argument includes the client and the
+transaction identification information. The second argument specifies
+the leased address.
 
 % DHCP4_RESPONSE_FQDN_DATA %1: including FQDN option in the server's response: %2
 This debug message is issued when the server is adding the Client FQDN

src/bin/dhcp4/dhcp4_srv.cc

@@ -108,10 +108,22 @@ Dhcpv4Exchange::Dhcpv4Exchange(const AllocEnginePtr& alloc_engine,
     context_->subnet_ = subnet;
     // Hardware address.
     context_->hwaddr_ = query->getHWAddr();
-    // Client Identifier
-    OptionPtr opt_clientid = query->getOption(DHO_DHCP_CLIENT_IDENTIFIER);
-    if (opt_clientid) {
-        context_->clientid_.reset(new ClientId(opt_clientid->getData()));
+
+    // 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) {
+        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());
+        }
     }
     // Check for static reservations.
     alloc_engine->findReservation(*context_);
@@ -1055,29 +1067,21 @@ Dhcpv4Srv::createNameChangeRequests(const Lease4Ptr& lease,
     // We may also decide not to generate any requests at all. This is when
     // we discover that nothing has changed in the client's FQDN data.
     if (old_lease) {
-        if (!lease->matches(*old_lease)) {
-            isc_throw(isc::Unexpected,
-                      "there is no match between the current instance of the"
-                      " lease: " << lease->toText() << ", and the previous"
-                      " instance: " << lease->toText());
-        } else {
-            // There will be a NameChangeRequest generated to remove existing
-            // DNS entries if the following conditions are met:
-            // - The hostname is set for the existing lease, we can't generate
-            //   removal request for non-existent hostname.
-            // - A server has performed reverse, forward or both updates.
-            // - FQDN data between the new and old lease do not match.
-            if (!lease->hasIdenticalFqdn(*old_lease)) {
-                queueNameChangeRequest(isc::dhcp_ddns::CHG_REMOVE,
-                                       old_lease);
+        // There will be a NameChangeRequest generated to remove existing
+        // DNS entries if the following conditions are met:
+        // - The hostname is set for the existing lease, we can't generate
+        //   removal request for non-existent hostname.
+        // - A server has performed reverse, forward or both updates.
+        // - FQDN data between the new and old lease do not match.
+        if (!lease->hasIdenticalFqdn(*old_lease)) {
+            queueNameChangeRequest(isc::dhcp_ddns::CHG_REMOVE, old_lease);
 
             // If FQDN data from both leases match, there is no need to update.
-            } else if (lease->hasIdenticalFqdn(*old_lease)) {
-                return;
-
-            }
+        } else if (lease->hasIdenticalFqdn(*old_lease)) {
+            return;
 
         }
+
     }
 
     // We may need to generate the NameChangeRequest for the new lease. It
@@ -1159,9 +1163,6 @@ Dhcpv4Srv::assignLease(Dhcpv4Exchange& ex) {
     /// @todo: move subnet selection to a common code
     resp->setSiaddr(subnet->getSiaddr());
 
-    // Get client-id. It is not mandatory in DHCPv4.
-    ClientIdPtr client_id = ex.getContext()->clientid_;
-
     // Get the server identifier. It will be used to determine the state
     // of the client.
     OptionCustomPtr opt_serverid = boost::dynamic_pointer_cast<
@@ -1190,6 +1191,9 @@ Dhcpv4Srv::assignLease(Dhcpv4Exchange& ex) {
     // allocation.
     bool fake_allocation = (query->getType() == DHCPDISCOVER);
 
+    // Get client-id. It is not mandatory in DHCPv4.
+    ClientIdPtr client_id = ex.getContext()->clientid_;
+
     // If there is no server id and there is a Requested IP Address option
     // the client is in the INIT-REBOOT state in which the server has to
     // determine whether the client's notion of the address is correct
@@ -1201,33 +1205,37 @@ Dhcpv4Srv::assignLease(Dhcpv4Exchange& ex) {
             .arg(hint.toText());
 
         Lease4Ptr lease;
-        if (hwaddr) {
-            lease = LeaseMgrFactory::instance().getLease4(*hwaddr,
-                                                          subnet->getID());
+        if (client_id) {
+            lease = LeaseMgrFactory::instance().getLease4(*client_id, subnet->getID());
         }
-        if (!lease && client_id) {
-            lease = LeaseMgrFactory::instance().getLease4(*client_id,
-                                                          subnet->getID());
+
+        if (!lease && hwaddr) {
+            lease = LeaseMgrFactory::instance().getLease4(*hwaddr, subnet->getID());
         }
-        // Got a lease so we can check the address.
-        if (lease && (lease->addr_ != hint)) {
+
+        // Check the first error case: unknown client. We check this before
+        // validating the address sent because we don't want to respond if
+        // we don't know this client.
+        if (!lease || !lease->belongsToClient(hwaddr, client_id)) {
             LOG_DEBUG(bad_packet_logger, DBG_DHCP4_DETAIL,
-                      DHCP4_PACKET_NAK_0002)
+                      DHCP4_NO_LEASE_INIT_REBOOT)
                 .arg(query->getLabel())
                 .arg(hint.toText());
 
-            resp->setType(DHCPNAK);
-            resp->setYiaddr(IOAddress::IPV4_ZERO_ADDRESS());
+            ex.deleteResponse();
             return;
         }
-        // Now check the second error case: unknown client.
-        if (!lease) {
+
+        // We know this client so we can now check if his notion of the
+        // IP address is correct.
+        if (lease && (lease->addr_ != hint)) {
             LOG_DEBUG(bad_packet_logger, DBG_DHCP4_DETAIL,
-                      DHCP4_NO_LEASE_INIT_REBOOT)
+                      DHCP4_PACKET_NAK_0002)
                 .arg(query->getLabel())
                 .arg(hint.toText());
 
-            ex.deleteResponse();
+            resp->setType(DHCPNAK);
+            resp->setYiaddr(IOAddress::IPV4_ZERO_ADDRESS());
             return;
         }
     }
@@ -1661,7 +1669,16 @@ Dhcpv4Srv::processRelease(Pkt4Ptr& release) {
     /// @todo Uncomment this (see ticket #3116)
     /// sanityCheck(release, MANDATORY);
 
-    // Try to find client-id
+    // Try to find client-id. Note that for the DHCPRELEASE we don't check if the
+    // match-client-id configuration parameter is disabled because this parameter
+    // is configured for subnets and we don't select subnet for the DHCPRELEASE.
+    // Bogus clients usually generate new client identifiers when they first
+    // connect to the network, so whatever client identifier has been used to
+    // acquire the lease, the client identifier carried in the DHCPRELEASE is
+    // likely to be the same and the lease will be correctly identified in the
+    // lease database. If supplied client identifier differs from the one used
+    // to acquire the lease then the lease will remain in the database and
+    // simply expire.
     ClientIdPtr client_id;
     OptionPtr opt = release->getOption(DHO_DHCP_CLIENT_IDENTIFIER);
     if (opt) {
@@ -1680,25 +1697,10 @@ Dhcpv4Srv::processRelease(Pkt4Ptr& release) {
             return;
         }
 
-        // Does the hardware address match? We don't want one client releasing
-        // another client's leases. Note that we're comparing the hardware
-        // addresses only, not hardware types or sources of the hardware
-        // addresses. Thus we don't use HWAddr::equals().
-        if (lease->hwaddr_->hwaddr_ != release->getHWAddr()->hwaddr_) {
-            LOG_DEBUG(lease_logger, DBG_DHCP4_DETAIL, DHCP4_RELEASE_FAIL_WRONG_HWADDR)
+        if (!lease->belongsToClient(release->getHWAddr(), client_id)) {
+            LOG_DEBUG(lease_logger, DBG_DHCP4_DETAIL, DHCP4_RELEASE_FAIL_WRONG_CLIENT)
                 .arg(release->getLabel())
-                .arg(release->getCiaddr().toText())
-                .arg(lease->hwaddr_->toText(false));
-            return;
-        }
-
-        // Does the lease have client-id info? If it has, then check it with what
-        // the client sent us.
-        if (lease->client_id_ && client_id && *lease->client_id_ != *client_id) {
-            LOG_DEBUG(lease_logger, DBG_DHCP4_DETAIL, DHCP4_RELEASE_FAIL_WRONG_CLIENT_ID)
-                .arg(release->getLabel())
-                .arg(release->getCiaddr().toText())
-                .arg(lease->client_id_->toText());
+                .arg(release->getCiaddr().toText());
             return;
         }
 

src/bin/dhcp4/dhcp4_srv.h

@@ -378,7 +378,7 @@ protected:
     /// @return ACK or NAK message
     Pkt4Ptr processRequest(Pkt4Ptr& request);
 
-    /// @brief Stub function that will handle incoming RELEASE messages.
+    /// @brief Processes incoming DHCPRELEASE messages.
     ///
     /// In DHCPv4, server does not respond to RELEASE messages, therefore
     /// this function does not return anything.
@@ -391,9 +391,11 @@ protected:
     /// @param decline message received from client
     void processDecline(Pkt4Ptr& decline);
 
-    /// @brief Stub function that will handle incoming INFORM messages.
+    /// @brief Processes incoming DHCPINFORM messages.
     ///
     /// @param inform message received from client
+    ///
+    /// @return DHCPACK to be sent to the client.
     Pkt4Ptr processInform(Pkt4Ptr& inform);
 
     /// @brief Appends options requested by client.

src/bin/dhcp4/json_config_parser.cc

@@ -194,6 +194,8 @@ protected:
             parser = new RelayInfoParser(config_id, relay_info_, Option::V4);
         } else if (config_id.compare("option-data") == 0) {
             parser = new OptionDataListParser(config_id, options_, AF_INET);
+        } else if (config_id.compare("match-client-id") == 0) {
+            parser = new BooleanParser(config_id, boolean_values_);
         } else {
             isc_throw(NotImplemented, "unsupported parameter: " << config_id);
         }
@@ -250,7 +252,28 @@ protected:
         Subnet4Ptr subnet4(new Subnet4(addr, len, t1, t2, valid, subnet_id));
         subnet_ = subnet4;
 
-        // Try global value first
+        // match-client-id
+        isc::util::OptionalValue<bool> match_client_id;
+        try {
+            match_client_id = boolean_values_->getParam("match-client-id");
+
+        } catch (...) {
+            // Ignore because this parameter is optional and it may be specified
+            // in the global scope.
+        }
+
+        // If the match-client-id wasn't specified as a subnet specific parameter
+        // check if there is global value specified.
+        if (!match_client_id.isSpecified()) {
+            // If not specified, use false.
+            match_client_id.specify(globalContext()->boolean_values_->
+                                    getOptionalParam("match-client-id", true));
+        }
+
+        // Set the match-client-id value for the subnet.
+        subnet4->setMatchClientId(match_client_id.get());
+
+        // next-server
         try {
             string next_server = globalContext()->string_values_->getParam("next-server");
             if (!next_server.empty()) {
@@ -374,6 +397,8 @@ namespace dhcp {
         parser = new BooleanParser(config_id, globalContext()->boolean_values_);
     } else if (config_id.compare("dhcp-ddns") == 0) {
         parser = new D2ClientConfigParser(config_id);
+    } else if (config_id.compare("match-client-id") == 0) {
+        parser = new BooleanParser(config_id, globalContext()->boolean_values_);
     } else {
         isc_throw(DhcpConfigError,
                 "unsupported global configuration parameter: "
@@ -408,6 +433,9 @@ configureDhcp4Server(Dhcpv4Srv&, isc::data::ConstElementPtr config_set) {
     LOG_DEBUG(dhcp4_logger, DBG_DHCP4_COMMAND,
               DHCP4_CONFIG_START).arg(config_set->str());
 
+    // Reset global context.
+    globalContext().reset(new ParserContext(Option::V4));
+
     // Before starting any subnet operations, let's reset the subnet-id counter,
     // so newly recreated configuration starts with first subnet-id equal 1.
     Subnet::resetSubnetID();

src/bin/dhcp4/tests/Makefile.am

@@ -87,6 +87,7 @@ dhcp4_unittests_SOURCES += marker_file.cc
 dhcp4_unittests_SOURCES += dhcp4_client.cc dhcp4_client.h
 dhcp4_unittests_SOURCES += inform_unittest.cc
 dhcp4_unittests_SOURCES += dora_unittest.cc
+dhcp4_unittests_SOURCES += release_unittest.cc
 
 if CONFIG_BACKEND_BUNDY
 # For Bundy backend, we only need to run the usual tests. There are no

src/bin/dhcp4/tests/config_parser_unittest.cc

@@ -1102,6 +1102,77 @@ TEST_F(Dhcp4ParserTest, echoClientId) {
     CfgMgr::instance().echoClientId(true);
 }
 
+// This test checks that the global match-client-id parameter is optional
+// and that values under the subnet are used.
+TEST_F(Dhcp4ParserTest, matchClientIdNoGlobal) {
+    ConstElementPtr status;
+
+    std::string config = "{ " + genIfaceConfig() + "," +
+        "\"rebind-timer\": 2000, "
+        "\"renew-timer\": 1000, "
+        "\"subnet4\": [ "
+        "{"
+        "    \"match-client-id\": true,"
+        "    \"pools\": [ { \"pool\": \"192.0.2.1 - 192.0.2.100\" } ],"
+        "    \"subnet\": \"192.0.2.0/24\""
+        "},"
+        "{"
+        "    \"match-client-id\": false,"
+        "    \"pools\": [ { \"pool\": \"192.0.3.1 - 192.0.3.100\" } ],"
+        "    \"subnet\": \"192.0.3.0/24\""
+        "} ],"
+        "\"valid-lifetime\": 4000 }";
+
+    ElementPtr json = Element::fromJSON(config);
+    ASSERT_NO_THROW(status = configureDhcp4Server(*srv_, json));
+    checkResult(status, 0);
+
+    CfgSubnets4Ptr cfg = CfgMgr::instance().getStagingCfg()->getCfgSubnets4();
+    Subnet4Ptr subnet1 = cfg->selectSubnet(IOAddress("192.0.2.1"));
+    ASSERT_TRUE(subnet1);
+    EXPECT_TRUE(subnet1->getMatchClientId());
+
+    Subnet4Ptr subnet2 = cfg->selectSubnet(IOAddress("192.0.3.1"));
+    ASSERT_TRUE(subnet2);
+    EXPECT_FALSE(subnet2->getMatchClientId());
+}
+
+// This test checks that the global match-client-id parameter is used
+// when there is no such parameter under subnet and that the parameter
+// specified for a subnet overrides the global setting.
+TEST_F(Dhcp4ParserTest, matchClientIdGlobal) {
+    ConstElementPtr status;
+
+    std::string config = "{ " + genIfaceConfig() + "," +
+        "\"rebind-timer\": 2000, "
+        "\"renew-timer\": 1000, "
+        "\"match-client-id\": true,"
+        "\"subnet4\": [ "
+        "{"
+        "    \"match-client-id\": false,"
+        "    \"pools\": [ { \"pool\": \"192.0.2.1 - 192.0.2.100\" } ],"
+        "    \"subnet\": \"192.0.2.0/24\""
+        "},"
+        "{"
+        "    \"pools\": [ { \"pool\": \"192.0.3.1 - 192.0.3.100\" } ],"
+        "    \"subnet\": \"192.0.3.0/24\""
+        "} ],"
+        "\"valid-lifetime\": 4000 }";
+
+    ElementPtr json = Element::fromJSON(config);
+    ASSERT_NO_THROW(status = configureDhcp4Server(*srv_, json));
+    checkResult(status, 0);
+
+    CfgSubnets4Ptr cfg = CfgMgr::instance().getStagingCfg()->getCfgSubnets4();
+    Subnet4Ptr subnet1 = cfg->selectSubnet(IOAddress("192.0.2.1"));
+    ASSERT_TRUE(subnet1);
+    EXPECT_FALSE(subnet1->getMatchClientId());
+
+    Subnet4Ptr subnet2 = cfg->selectSubnet(IOAddress("192.0.3.1"));
+    ASSERT_TRUE(subnet2);
+    EXPECT_TRUE(subnet2->getMatchClientId());
+}
+
 // This test checks if it is possible to override global values
 // on a per subnet basis.
 TEST_F(Dhcp4ParserTest, subnetLocal) {
@@ -2005,6 +2076,7 @@ TEST_F(Dhcp4ParserTest, optionDataEncapsulate) {
     // configuration manager sends whole configuration for the lists
     // where at least one element is being modified or added.
     config = "{ " + genIfaceConfig() + "," +
+        "\"valid-lifetime\": 3000,"
         "\"rebind-timer\": 2000,"
         "\"renew-timer\": 1000,"
         "\"option-data\": [ {"
@@ -2585,6 +2657,7 @@ TEST_F(Dhcp4ParserTest, stdOptionDataEncapsulate) {
     // they should be included as sub-options in the 'vendor-opts'
     // option.
     config = "{ " + genIfaceConfig() + "," +
+        "\"valid-lifetime\": 3000,"
         "\"rebind-timer\": 2000,"
         "\"renew-timer\": 1000,"
         "\"option-data\": [ {"

src/bin/dhcp4/tests/dhcp4_client.cc

@@ -254,6 +254,25 @@ Dhcp4Client::doInform(const bool set_ciaddr) {
 }
 
 void
+Dhcp4Client::doRelease() {
+    if (config_.lease_.addr_ == IOAddress::IPV4_ZERO_ADDRESS()) {
+        isc_throw(Dhcp4ClientError, "failed to send the release"
+                  " message because client doesn't have a lease");
+    }
+    context_.query_ = createMsg(DHCPRELEASE);
+    // Set ciaddr to the address which we want to release.
+    context_.query_->setCiaddr(config_.lease_.addr_);
+    // Include client identifier.
+    appendClientId();
+
+    // Remove configuration.
+    config_.reset();
+
+    // Send the message to the server.
+    sendMsg(context_.query_);
+}
+
+void
 Dhcp4Client::doRequest() {
     context_.query_ = createMsg(DHCPREQUEST);
 

src/bin/dhcp4/tests/dhcp4_client.h

@@ -166,6 +166,12 @@ public:
     /// an error occurs.
     void doInform(const bool set_ciaddr = true);
 
+    /// @brief Sends DHCPRELEASE Message to the server.
+    ///
+    /// This method simulates sending the DHCPRELEASE message to the server.
+    /// The released lease is removed from the client's configuration.
+    void doRelease();
+
     /// @brief Sends DHCPREQUEST Message to the server and receives a response.
     ///
     /// This method simulates sending the DHCPREQUEST message to the server and

src/bin/dhcp4/tests/dhcp4_srv_unittest.cc

@@ -911,169 +911,6 @@ TEST_F(Dhcpv4SrvTest, sanityCheck) {
                  RFCViolation);
 }
 
-// This test verifies that incoming (positive) RELEASE can be handled properly.
-// As there is no REPLY in DHCPv4, the only thing to verify here is that
-// the lease is indeed removed from the database.
-TEST_F(Dhcpv4SrvTest, ReleaseBasic) {
-    boost::scoped_ptr<NakedDhcpv4Srv> srv;
-    ASSERT_NO_THROW(srv.reset(new NakedDhcpv4Srv(0)));
-
-    const IOAddress addr("192.0.2.106");
-    const uint32_t temp_t1 = 50;
-    const uint32_t temp_t2 = 75;
-    const uint32_t temp_valid = 100;
-    const time_t temp_timestamp = time(NULL) - 10;
-
-    // Generate client-id also duid_
-    OptionPtr clientid = generateClientId();
-
-    // Check that the address we are about to use is indeed in pool
-    ASSERT_TRUE(subnet_->inPool(Lease::TYPE_V4, addr));
-
-    // Let's create a lease and put it in the LeaseMgr
-    uint8_t hwaddr_data[] = { 0, 0xfe, 0xfe, 0xfe, 0xfe, 0xfe};
-    HWAddrPtr hwaddr(new HWAddr(hwaddr_data, sizeof(hwaddr_data), HTYPE_ETHER));
-    Lease4Ptr used(new Lease4(addr, hwaddr,
-                              &client_id_->getDuid()[0], client_id_->getDuid().size(),
-                              temp_valid, temp_t1, temp_t2, temp_timestamp,
-                              subnet_->getID()));
-    ASSERT_TRUE(LeaseMgrFactory::instance().addLease(used));
-
-    // Check that the lease is really in the database
-    Lease4Ptr l = LeaseMgrFactory::instance().getLease4(addr);
-    ASSERT_TRUE(l);
-
-    // Let's create a RELEASE
-    // Generate client-id also duid_
-    Pkt4Ptr rel = Pkt4Ptr(new Pkt4(DHCPRELEASE, 1234));
-    rel->setRemoteAddr(addr);
-    rel->setCiaddr(addr);
-    rel->addOption(clientid);
-    rel->addOption(srv->getServerID());
-    rel->setHWAddr(hwaddr);
-    rel->setIface("eth0");
-
-    // Pass it to the server and hope for a REPLY
-    // Note: this is no response to RELEASE in DHCPv4
-    EXPECT_NO_THROW(srv->processRelease(rel));
-
-    // The lease should be gone from LeaseMgr
-    l = LeaseMgrFactory::instance().getLease4(addr);
-    EXPECT_FALSE(l);
-
-    // Try to get the lease by hardware address
-    Lease4Collection leases = LeaseMgrFactory::instance().getLease4(*hwaddr);
-    EXPECT_EQ(leases.size(), 0);
-
-    // Try to get it by hw/subnet_id combination
-    l = LeaseMgrFactory::instance().getLease4(*hwaddr, subnet_->getID());
-    EXPECT_FALSE(l);
-
-    // Try by client-id
-    leases = LeaseMgrFactory::instance().getLease4(*client_id_);
-    EXPECT_EQ(leases.size(), 0);
-
-    // Try by client-id/subnet-id
-    l = LeaseMgrFactory::instance().getLease4(*client_id_, subnet_->getID());
-    EXPECT_FALSE(l);
-
-    // Ok, the lease is *really* not there.
-}
-
-// This test verifies that incoming (invalid) RELEASE can be handled properly.
-//
-// This test checks 3 scenarios:
-// 1. there is no such lease at all
-// 2. there is such a lease, but it is assigned to a different IAID
-// 3. there is such a lease, but it belongs to a different client
-TEST_F(Dhcpv4SrvTest, ReleaseReject) {
-    boost::scoped_ptr<NakedDhcpv4Srv> srv;
-    ASSERT_NO_THROW(srv.reset(new NakedDhcpv4Srv(0)));
-
-    const IOAddress addr("192.0.2.106");
-    const uint32_t t1 = 50;
-    const uint32_t t2 = 75;
-    const uint32_t valid = 100;
-    const time_t timestamp = time(NULL) - 10;
-
-    // Let's create a lease and put it in the LeaseMgr
-    uint8_t bogus_mac_addr[] = { 0, 0xfe, 0xfe, 0xfe, 0xfe, 0xfe};
-    HWAddrPtr bogus_hw(new HWAddr(bogus_mac_addr, sizeof(bogus_mac_addr), HTYPE_ETHER));
-    OptionPtr bogus_clientid = generateClientId(7); // different length
-
-    // Generate client-id also duid_
-    OptionPtr clientid = generateClientId();
-
-    // Check that the address we are about to use is indeed in pool
-    ASSERT_TRUE(subnet_->inPool(Lease::TYPE_V4, addr));
-
-    // Let's create a RELEASE
-    // Generate client-id also duid_
-    Pkt4Ptr rel = Pkt4Ptr(new Pkt4(DHCPRELEASE, 1234));
-    rel->setRemoteAddr(addr);
-    rel->setCiaddr(addr);
-    rel->addOption(clientid);
-    rel->addOption(srv->getServerID());
-    rel->setHWAddr(bogus_hw);
-    rel->setIface("eth0");
-
-    // Case 1: No lease known to server
-    SCOPED_TRACE("CASE 1: Lease is not known to the server");
-
-    // There is nothing to check here. The lease is not there and server does
-    // not send anything back. This case is enumerated here just for keeping
-    // parity with similar test in DHCPv6.
-    EXPECT_NO_THROW(srv->processRelease(rel));
-
-    // CASE 2: Lease is known and belongs to this client, but to a different hardware
-    SCOPED_TRACE("CASE 2: Lease is known and belongs to this client, but uses different HW addr");
-
-    // Let's create a lease and put it in the LeaseMgr
-    uint8_t mac_addr[] = { 0, 0x1, 0x2, 0x3, 0x4, 0x5};
-    HWAddrPtr hw(new HWAddr(mac_addr, sizeof(mac_addr), HTYPE_ETHER));
-    Lease4Ptr used(new Lease4(addr, hw,
-                              &client_id_->getDuid()[0], client_id_->getDuid().size(),
-                              valid, t1, t2, timestamp, subnet_->getID()));
-    ASSERT_TRUE(LeaseMgrFactory::instance().addLease(used));
-    // Check that the lease is really in the database
-    Lease4Ptr l = LeaseMgrFactory::instance().getLease4(addr);
-    ASSERT_TRUE(l);
-
-    rel->setHWAddr(bogus_hw);
-
-    EXPECT_NO_THROW(srv->processRelease(rel));
-
-    // Check that the lease was not removed (due to hardware address mis-match)
-    l = LeaseMgrFactory::instance().getLease4(addr);
-    ASSERT_TRUE(l);
-
-    // CASE 3: Lease belongs to a client with different client-id
-    SCOPED_TRACE("CASE 3: Lease belongs to a client with different client-id");
-
-    rel->setHWAddr(hw); // proper HW address this time
-    rel->delOption(DHO_DHCP_CLIENT_IDENTIFIER);
-    rel->addOption(bogus_clientid); // but invalid client-id
-
-    OptionPtr x = rel->getOption(DHO_DHCP_CLIENT_IDENTIFIER);
-
-    EXPECT_NO_THROW(srv->processRelease(rel));
-
-    // Check that the lease is still there
-    l = LeaseMgrFactory::instance().getLease4(addr);
-    ASSERT_TRUE(l);
-
-    // Final sanity check. Verify that with valid hw and client-id release is successful
-    rel->delOption(DHO_DHCP_CLIENT_IDENTIFIER);
-    rel->addOption(clientid);
-
-    // It should work this time
-    EXPECT_NO_THROW(srv->processRelease(rel));
-
-    // Check that the lease is not there
-    l = LeaseMgrFactory::instance().getLease4(addr);
-    EXPECT_FALSE(l);
-}
-
 // Checks if received relay agent info option is echoed back to the client
 TEST_F(Dhcpv4SrvTest, relayAgentInfoEcho) {
     IfaceMgrTestConfig test_config(true);

src/bin/dhcp4/tests/dora_unittest.cc

@@ -57,6 +57,13 @@ namespace {
 ///   - 1 subnet: 10.0.0.0/24
 ///   - One reservation for the client using MAC address:
 ///     aa:bb:cc:dd:ee:ff, reserved address 10.0.0.7
+///
+/// - Configuration 3:
+///   - Use for testing match-client-id flag
+///   - 1 subnet: 10.0.0.0/24
+///   - 1 pool: 10.0.0.10-10.0.0.100
+///   - match-client-id flag is set to false, thus the server
+///     uses HW address for lease lookup, rather than client id
 const char* DORA_CONFIGS[] = {
 // Configuration 0
     "{ \"interfaces-config\": {"
@@ -152,7 +159,27 @@ const char* DORA_CONFIGS[] = {
         "       }"
         "    ]"
         "} ]"
-    "}"
+    "}",
+
+// Configuration 3
+    "{ \"interfaces-config\": {"
+        "      \"interfaces\": [ \"*\" ]"
+        "},"
+        "\"valid-lifetime\": 600,"
+        "\"match-client-id\": false,"
+        "\"subnet4\": [ { "
+        "    \"subnet\": \"10.0.0.0/24\", "
+        "    \"id\": 1,"
+        "    \"pools\": [ { \"pool\": \"10.0.0.10-10.0.0.100\" } ],"
+        "    \"option-data\": [ {"
+        "        \"name\": \"routers\","
+        "        \"code\": 3,"
+        "        \"data\": \"10.0.0.200,10.0.0.201\","
+        "        \"csv-format\": true,"
+        "        \"space\": \"dhcp4\""
+        "    } ]"
+        " } ]"
+    "}",
 };
 
 /// @brief Test fixture class for testing 4-way (DORA) exchanges.
@@ -173,71 +200,30 @@ public:
         IfaceMgr::instance().openSockets4();
     }
 
-    /// @brief Test that server doesn't allocate the lease for a client
-    /// which has the same address or client identifier as another client.
+    /// @brief Test that server returns the same lease for the client which is
+    /// sometimes using client identifier, sometimes not.
     ///
     /// This test checks the server's behavior in the following scenario:
-    /// - Client A identifies itself to the server using the hardware address
-    ///   and client identifier or only one of those.
-    /// - Client A performs the 4-way exchange and obtains a lease from the server.
-    /// - Client B uses the same HW address or client identifier as the client A.
-    /// - Client B uses both HW address and client identifier if the client A is using
-    ///   only one of them. Client B uses one of the HW address or client
-    ///   identifier if the client A is using both.
-    /// - Client B sends the DHCPDISCOVER to the server.
-    ///   The server determines that there is a lease for the client A using the
-    ///   same HW address as the client B. Server discards the client's message and
-    ///   doesn't offer the lease for the client B to prevent allocation of the
-    ///   lease without a unique identifier.
-    /// - The client sends the DHCPREQUEST and the server sends the DHCPNAK for the
-    ///   same reason.
-    /// - The client A renews its address successfully.
+    /// - Client identifies itself to the server using HW address, and may use
+    ///   client identifier.
+    /// - Client performs the 4-way exchange and obtains a lease from the server.
+    /// - If the client identifier was in use when the client has acquired the lease,
+    ///   the client uses null client identifier in the next exchange with the server.
+    /// - If the client identifier was not in use when the client has acquired the
+    ///   lease, the client uses client identifier in the next exchange with the
+    ///   server.
+    /// - When the client contacts the server for the second time using the
+    ///   DHCPDISCOVER the server determines (using HW address) that the client
+    ///   already has a lease and returns this lease to the client.
+    /// - The client renews the existing lease.
     ///
-    /// The specific test cases using this test must make sure that one of the
-    /// provided parameters is an empty string. This simulates the situation where
-    /// one of the clients has only one of the identifiers and the other one has
-    /// two.
-    ///
-    /// @param hwaddr_a HW address of client A.
-    /// @param clientid_a Client id of client A.
-    /// @param hwaddr_b HW address of client B.
-    /// @param clientid_b Client id of client B.
-    void oneAllocationOverlapTest(const std::string& hwaddr_a,
-                                  const std::string& clientid_a,
-                                  const std::string& hwaddr_b,
+    /// @param clientid_a Client identifier when the client initially allocates
+    /// the lease. An empty value means "no client identifier".
+    /// @param clientid_b Client identifier when the client sends the DHCPDISCOVER
+    /// and then DHCPREQUEST to renew lease.
+    void oneAllocationOverlapTest(const std::string& clientid_a,
                                   const std::string& clientid_b);
 
-    /// @brief Test that server can allocate the lease for a client having
-    /// the same HW Address or client id as another client.
-    ///
-    /// This test checks the server behavior in the following situation:
-    /// - Client A identifies itself to the server using client identifier
-    ///   and the hardware address and requests allocation of the new lease.
-    /// - Server allocates the lease to the client.
-    /// - Client B has a different hardware address or client identifier than
-    ///   the client A, but the other identifier is equal to the corresponding
-    ///   identifier of the client A.
-    /// - Client B sends DHCPDISCOVER.
-    /// - Server should determine that the client B is not client A, because
-    ///   it is using a different hadrware address or client identifier.
-    ///   As a consequence, the server should offer a different address to the
-    ///   client B.
-    /// - The client B performs the 4-way exchange again, and the server
-    ///   allocates a new address to the client, which should be different
-    ///   than the address used by the client A.
-    /// - Client B is in the renewing state and it successfully renews its
-    ///   address.
-    /// - The client A also renews its address successfully.
-    ///
-    /// @param hwaddr_a HW address of client A.
-    /// @param clientid_a Client id of client A.
-    /// @param hwaddr_b HW address of client B.
-    /// @param clientid_b Client id of client B.
-    void twoAllocationsOverlapTest(const std::string& hwaddr_a,
-                                   const std::string& clientid_a,
-                                   const std::string& hwaddr_b,
-                                   const std::string& clientid_b);
-
     /// @brief Interface Manager's fake configuration control.
     IfaceMgrTestConfig iface_mgr_test_config_;
 
@@ -404,6 +390,7 @@ TEST_F(DORATest, initRebootRequest) {
     Dhcp4Client client(Dhcp4Client::SELECTING);
     // Configure DHCP server.
     configure(DORA_CONFIGS[0], *client.getServer());
+    client.includeClientId("11:22");
     // Obtain a lease from the server using the 4-way exchange.
     ASSERT_NO_THROW(client.doDORA(boost::shared_ptr<
                                   IOAddress>(new IOAddress("10.0.0.50"))));
@@ -446,11 +433,32 @@ TEST_F(DORATest, initRebootRequest) {
     resp = client.getContext().response_;
     EXPECT_EQ(DHCPNAK, static_cast<int>(resp->getType()));
 
-    // Try to request from a different client.
+    // Change client identifier. The server should treat the request
+    // as a resquest from unknown client and ignore it.
+    client.includeClientId("12:34");
+    ASSERT_NO_THROW(client.doRequest());
+    ASSERT_FALSE(client.getContext().response_);
+
+    // Now let's fix the IP address. The client identifier is still
+    // invalid so the message should be dropped.
+    client.config_.lease_.addr_ = IOAddress("10.0.0.50");
+    ASSERT_NO_THROW(client.doRequest());
+    ASSERT_FALSE(client.getContext().response_);
+
+    // Restore original client identifier.
+    client.includeClientId("11:22");
+
+    // Try to request from a different HW address. This should be successful
+    // because the client identifier matches.
     client.modifyHWAddr();
     ASSERT_NO_THROW(client.doRequest());
-    // The server should not respond.
-    EXPECT_FALSE(client.getContext().response_);
+    // 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 with the requested address.
+    ASSERT_EQ("10.0.0.50", client.config_.lease_.addr_.toText());
 }
 
 // Check that the ciaddr returned by the server is correct for DHCPOFFER and
@@ -507,14 +515,67 @@ TEST_F(DORATest, ciaddr) {
 }
 
 void
-DORATest::twoAllocationsOverlapTest(const std::string& hwaddr_a,
-                                    const std::string& clientid_a,
-                                    const std::string& hwaddr_b,
-                                    const std::string& clientid_b) {
+DORATest::oneAllocationOverlapTest(const std::string& clientid_a,
+                                   const std::string& clientid_b) {
+    // Allocate a lease by client using the 4-way exchange.
+    Dhcp4Client client(Dhcp4Client::SELECTING);
+    client.includeClientId(clientid_a);
+    client.setHWAddress("01:02:03:04:05:06");
+    configure(DORA_CONFIGS[0], *client.getServer());
+    ASSERT_NO_THROW(client.doDORA());
+    // Make sure that the server responded.
+    ASSERT_TRUE(client.getContext().response_);
+    Pkt4Ptr resp = client.getContext().response_;
+    // Make sure that the server has responded with DHCPACK.
+    ASSERT_EQ(DHCPACK, static_cast<int>(resp->getType()));
+    Lease4Ptr lease_a = LeaseMgrFactory::instance().getLease4(client.config_.lease_.addr_);
+    ASSERT_TRUE(lease_a);
+    // Remember the allocated address.
+    IOAddress yiaddr = lease_a->addr_;
+
+    // Change client identifier. If parameters clientid_a and clientid_b
+    // are specified correctly, this removes the client identifier from
+    // client's requests if the lease has been acquired with the client
+    // identifier, or adds the client identifier otherwise.
+    client.includeClientId(clientid_b);
+
+    // Check if the server will offer the same address.
+    ASSERT_NO_THROW(client.doDiscover());
+    resp = client.getContext().response_;
+    ASSERT_TRUE(resp);
+    EXPECT_EQ(yiaddr, resp->getYiaddr());
+
+    // Client should also be able to renew its address.
+    client.setState(Dhcp4Client::RENEWING);
+    ASSERT_NO_THROW(client.doRequest());
+    ASSERT_TRUE(client.getContext().response_);
+    resp = client.getContext().response_;
+    ASSERT_EQ(DHCPACK, static_cast<int>(resp->getType()));
+    ASSERT_EQ(yiaddr, client.config_.lease_.addr_);
+}
+
+// This test checks the server behavior in the following situation:
+// - Client A identifies itself to the server using client identifier
+//   and the hardware address and requests allocation of the new lease.
+// - Server allocates the lease to the client.
+// - Client B has the same hardware address but is using a different
+//   client identifier then Client A.
+// - Client B sends DHCPDISCOVER.
+// - Server should determine that the client B is not client A, because
+//   it is using a different client identifier, even though they use the
+//   same HW address. As a consequence, the server should offer a
+//    different address to the client B.
+// - The client B performs the 4-way exchange again and the server
+//   allocates a new address to the client, which should be different
+//   than the address used by the client A.
+// - Client B is in the renewing state and it successfully renews its
+//   address.
+// - Client A also renews its address successfully.
+TEST_F(DORATest, twoAllocationsOverlap) {
     // Allocate a lease by client A using the 4-way exchange.
     Dhcp4Client client_a(Dhcp4Client::SELECTING);
-    client_a.includeClientId(clientid_a);
-    client_a.setHWAddress(hwaddr_a);
+    client_a.includeClientId("12:34");
+    client_a.setHWAddress("01:02:03:04:05:06");
     configure(DORA_CONFIGS[0], *client_a.getServer());
     ASSERT_NO_THROW(client_a.doDORA());
     // Make sure that the server responded.
@@ -529,8 +590,8 @@ DORATest::twoAllocationsOverlapTest(const std::string& hwaddr_a,
 
     // Create client B.
     Dhcp4Client client_b(client_a.getServer(), Dhcp4Client::SELECTING);
-    client_b.setHWAddress(hwaddr_b);
-    client_b.includeClientId(clientid_b);
+    client_b.setHWAddress("01:02:03:04:05:06");
+    client_b.includeClientId("45:67");
     // Send DHCPDISCOVER and expect the response.
     ASSERT_NO_THROW(client_b.doDiscover());
     Pkt4Ptr resp_b = client_b.getContext().response_;
@@ -581,86 +642,6 @@ DORATest::twoAllocationsOverlapTest(const std::string& hwaddr_a,
     ASSERT_NE(client_a.config_.lease_.addr_, client_b.config_.lease_.addr_);
 }
 
-void
-DORATest::oneAllocationOverlapTest(const std::string& hwaddr_a,
-                                   const std::string& clientid_a,
-                                   const std::string& hwaddr_b,
-                                   const std::string& clientid_b) {
-    // Allocate a lease by client A using the 4-way exchange.
-    Dhcp4Client client_a(Dhcp4Client::SELECTING);
-    client_a.includeClientId(clientid_a);
-    client_a.setHWAddress(hwaddr_a);
-    configure(DORA_CONFIGS[0], *client_a.getServer());
-    ASSERT_NO_THROW(client_a.doDORA());
-    // Make sure that the server responded.
-    ASSERT_TRUE(client_a.getContext().response_);
-    Pkt4Ptr resp_a = client_a.getContext().response_;
-    // Make sure that the server has responded with DHCPACK.
-    ASSERT_EQ(DHCPACK, static_cast<int>(resp_a->getType()));
-    Lease4Ptr lease_a = LeaseMgrFactory::instance().getLease4(client_a.config_.lease_.addr_);
-    ASSERT_TRUE(lease_a);
-
-    // Client B sends a DHCPDISCOVER.
-    Dhcp4Client client_b(client_a.getServer(), Dhcp4Client::SELECTING);
-    client_b.setHWAddress(hwaddr_b);
-    client_b.includeClientId(clientid_b);
-    // Client A and Client B have one common identifier (HW address
-    // or client identifier) and one of them is missing the other
-    // identifier. The allocation engine can't offer an address for
-    // the client which has the same identifier as the other client and
-    // which doesn't have any other (unique) identifier. It should
-    // discard the DHCPDISCOVER.
-    ASSERT_NO_THROW(client_b.doDiscover());
-    Pkt4Ptr resp_b = client_b.getContext().response_;
-    ASSERT_FALSE(resp_b);
-
-    // Now repeat the same test but send the DHCPREQUEST. This time the
-    // server should send the DHCPNAK.
-    client_b.config_.lease_.addr_ = IOAddress::IPV4_ZERO_ADDRESS();
-    client_b.setState(Dhcp4Client::INIT_REBOOT);
-    ASSERT_NO_THROW(client_b.doRequest());
-    resp_b = client_b.getContext().response_;
-    ASSERT_TRUE(resp_b);
-    ASSERT_EQ(DHCPNAK, static_cast<int>(resp_b->getType()));
-
-    // Client A should also be able to renew its address.
-    client_a.setState(Dhcp4Client::RENEWING);
-    ASSERT_NO_THROW(client_a.doRequest());
-    ASSERT_TRUE(client_a.getContext().response_);
-    resp_b = client_a.getContext().response_;
-    ASSERT_EQ(DHCPACK, static_cast<int>(resp_b->getType()));
-    ASSERT_NE(client_a.config_.lease_.addr_, client_b.config_.lease_.addr_);
-}
-
-// This test checks the server behavior in the following situation:
-// - Client A identifies itself to the server using client identifier
-//   and the hardware address and requests allocation of the new lease.
-// - Server allocates the lease to the client.
-// - Client B has a different hardware address but is using the same
-//   client identifier as Client A.
-// - Client B sends DHCPDISCOVER.
-// - Server should determine that the client B is not client A, because
-//   it is using a different hadrware address, even though they use the
-//   same client identifier. As a consequence, the server should offer
-//   a different address to the client B.
-// - The client B performs the 4-way exchange again and the server
-//   allocates a new address to the client, which should be different
-//   than the address used by the client A.
-// - Client B is in the renewing state and it successfully renews its
-//   address.
-// - Client A also renews its address successfully.
-TEST_F(DORATest, twoAllocationsOverlap1) {
-    twoAllocationsOverlapTest("01:02:03:04:05:06", "12:34",
-                              "02:02:03:03:04:04", "12:34");
-}
-
-// This test is similar to twoAllocationsOverlap1, but the
-// clients differ by client identifier.
-TEST_F(DORATest, twoAllocationsOverlap2) {
-    twoAllocationsOverlapTest("01:02:03:04:05:06", "12:34",
-                              "01:02:03:04:05:06", "22:34");
-}
-
 // This test checks the server behavior in the following situation:
 // - Client A identifies itself to the server using the hardware address
 //   and client identifier.
@@ -676,16 +657,14 @@ TEST_F(DORATest, twoAllocationsOverlap2) {
 //   same reason.
 // - Client A renews its address successfully.
 TEST_F(DORATest, oneAllocationOverlap1) {
-    oneAllocationOverlapTest("01:02:03:04:05:06", "12:34",
-                             "01:02:03:04:05:06", "");
+    oneAllocationOverlapTest("12:34", "");
 }
 
 // This test is similar to oneAllocationOverlap2 but this time the client A
 // uses no client identifier, and the client B uses the HW address and the
 // client identifier. The server behaves as previously.
 TEST_F(DORATest, oneAllocationOverlap2) {
-    oneAllocationOverlapTest("01:02:03:04:05:06", "",
-                             "01:02:03:04:05:06", "12:34");
+    oneAllocationOverlapTest("", "12:34");
 }
 
 // This is a simple test for the host reservation. It creates a reservation
@@ -732,6 +711,85 @@ TEST_F(DORATest, reservation) {
     ASSERT_TRUE(subnet->inPool(Lease::TYPE_V4, clientB.config_.lease_.addr_));
 }
 
+// This test checks that setting the match-client-id value to false causes
+// the server to ignore changing client identifier when the client is
+// using consistent HW address.
+TEST_F(DORATest, ignoreChangingClientId) {
+    Dhcp4Client client(Dhcp4Client::SELECTING);
+    // Configure DHCP server.
+    configure(DORA_CONFIGS[3], *client.getServer());
+    client.includeClientId("12:12");
+    // Obtain the lease using 4-way exchange.
+    ASSERT_NO_THROW(client.doDORA());
+    // Make sure that the server responded.
+    ASSERT_TRUE(client.getContext().response_);
+    Pkt4Ptr resp = client.getContext().response_;
+    // Make sure that the server has responded with DHCPACK.
+    ASSERT_EQ(DHCPACK, static_cast<int>(resp->getType()));
+    EXPECT_FALSE(client.config_.lease_.client_id_);
+
+    // Remember address which the client has obtained.
+    IOAddress leased_address = client.config_.lease_.addr_;
+
+    // Modify client id. Because we have set the configuration flag which
+    // forces the server to lookup leases using the HW address, the
+    // client id modification should not matter and the client should
+    // obtain the same lease.
+    client.includeClientId("14:14");
+    // Obtain the lease using 4-way exchange.
+    ASSERT_NO_THROW(client.doDORA());
+    // 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 server assigned the same address, even though the
+    // client id has changed.
+    EXPECT_EQ(leased_address, client.config_.lease_.addr_);
+    // Check that the client id is not present in the lease.
+    EXPECT_FALSE(client.config_.lease_.client_id_);
+}
+
+// This test checks that the match-client-id parameter doesn't have
+// effect on the lease lookup using the HW address.
+TEST_F(DORATest, changingHWAddress) {
+    Dhcp4Client client(Dhcp4Client::SELECTING);
+    // Configure DHCP server.
+    configure(DORA_CONFIGS[3], *client.getServer());
+    client.includeClientId("12:12");
+    client.setHWAddress("00:01:02:03:04:05");
+    // Obtain the lease using 4-way exchange.
+    ASSERT_NO_THROW(client.doDORA());
+    // Make sure that the server responded.
+    ASSERT_TRUE(client.getContext().response_);
+    Pkt4Ptr resp = client.getContext().response_;
+    // Make sure that the server has responded with DHCPACK.
+    ASSERT_EQ(DHCPACK, static_cast<int>(resp->getType()));
+    // Check that the client id is not present in the lease.
+    EXPECT_FALSE(client.config_.lease_.client_id_);
+
+    // Remember address which the client has obtained.
+    IOAddress leased_address = client.config_.lease_.addr_;
+
+    // Modify HW address but leave client id in place. The value of the
+    // match-client-id set to false must not have any effect on the
+    // case when the HW address is changing. In such case the server will
+    // allocate the new address for the client.
+    client.setHWAddress("01:01:01:01:01:01");
+    // Obtain a lease.
+    ASSERT_NO_THROW(client.doDORA());
+    // 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()));
+    // Client must assign different address because the client id is
+    // ignored and the HW address was changed.
+    EXPECT_NE(client.config_.lease_.addr_, leased_address);
+    // Check that the client id is not present in the lease.
+    EXPECT_FALSE(client.config_.lease_.client_id_);
+}
+
 // This test checks the following scenario:
 // 1. Client A performs 4-way exchange and obtains a lease from the dynamic pool.
 // 2. Reservation is created for the client A using an address out of the dynamic

src/bin/dhcp4/tests/fqdn_unittest.cc

@@ -797,20 +797,6 @@ TEST_F(NameDhcpv4SrvTest, createNameChangeRequestsRenew) {
 
 }
 
-// This test verifies that exception is thrown when leases passed to the
-// createNameChangeRequests function do not match, i.e. they comprise
-// different IP addresses, client ids etc.
-TEST_F(NameDhcpv4SrvTest, createNameChangeRequestsLeaseMismatch) {
-    Lease4Ptr lease1 = createLease(IOAddress("192.0.2.3"),
-                                   "lease1.example.com.",
-                                   true, true);
-    Lease4Ptr lease2 = createLease(IOAddress("192.0.2.4"),
-                                   "lease2.example.com.",
-                                   true, true);
-    EXPECT_THROW(srv_->createNameChangeRequests(lease2, lease1),
-                 isc::Unexpected);
-}
-
 // Test that the OFFER message generated as a result of the DISCOVER message
 // processing will not result in generation of the NameChangeRequests.
 TEST_F(NameDhcpv4SrvTest, processDiscover) {

src/bin/dhcp4/tests/release_unittest.cc

@@ -0,0 +1,262 @@
+// Copyright (C) 2015 Internet Systems Consortium, Inc. ("ISC")
+//
+// Permission to use, copy, modify, and/or distribute this software for any
+// purpose with or without fee is hereby granted, provided that the above
+// copyright notice and this permission notice appear in all copies.
+//
+// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH
+// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
+// AND FITNESS.  IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT,
+// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
+// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE
+// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
+// PERFORMANCE OF THIS SOFTWARE.
+
+#include <config.h>
+#include <asiolink/io_address.h>
+#include <cc/data.h>
+#include <dhcp/dhcp4.h>
+#include <dhcp/tests/iface_mgr_test_config.h>
+#include <dhcpsrv/cfgmgr.h>
+#include <dhcpsrv/subnet_id.h>
+#include <dhcp4/tests/dhcp4_test_utils.h>
+#include <dhcp4/tests/dhcp4_client.h>
+#include <boost/shared_ptr.hpp>
+
+using namespace isc;
+using namespace isc::asiolink;
+using namespace isc::data;
+using namespace isc::dhcp;
+using namespace isc::dhcp::test;
+
+namespace {
+
+/// @brief Set of JSON configurations used throughout the Release tests.
+///
+/// - Configuration 0:
+///   - Used for testing Release message processing
+///   - 1 subnet: 10.0.0.0/24
+///   - 1 pool: 10.0.0.10-10.0.0.100
+///   - Router option present: 10.0.0.200 and 10.0.0.201
+const char* RELEASE_CONFIGS[] = {
+// Configuration 0
+    "{ \"interfaces-config\": {"
+        "      \"interfaces\": [ \"*\" ]"
+        "},"
+        "\"valid-lifetime\": 600,"
+        "\"subnet4\": [ { "
+        "    \"subnet\": \"10.0.0.0/24\", "
+        "    \"id\": 1,"
+        "    \"pools\": [ { \"pool\": \"10.0.0.10-10.0.0.100\" } ],"
+        "    \"option-data\": [ {"
+        "        \"name\": \"routers\","
+        "        \"code\": 3,"
+        "        \"data\": \"10.0.0.200,10.0.0.201\","
+        "        \"csv-format\": true,"
+        "        \"space\": \"dhcp4\""
+        "    } ]"
+        " } ]"
+    "}"
+};
+
+/// @brief Test fixture class for testing 4-way (DORA) exchanges.
+///
+/// @todo Currently there is a limit number of test cases covered here.
+/// In the future it is planned that the tests from the
+/// dhcp4_srv_unittest.cc will be migrated here and will use the
+/// @c Dhcp4Client class.
+class ReleaseTest : public Dhcpv4SrvTest {
+public:
+
+    enum ExpectedResult {
+        SHOULD_PASS,
+        SHOULD_FAIL
+    };
+
+    /// @brief Constructor.
+    ///
+    /// Sets up fake interfaces.
+    ReleaseTest()
+        : Dhcpv4SrvTest(),
+          iface_mgr_test_config_(true) {
+        IfaceMgr::instance().openSockets4();
+    }
+
+    /// @brief Performs 4-way exchange to obtain new lease.
+    ///
+    /// @param client Client to be used to obtain a lease.
+    void acquireLease(Dhcp4Client& client);
+
+    /// @brief Tests if the acquired lease is or is not released.
+    ///
+    /// @param hw_address_1 HW Address to be used to acquire the lease.
+    /// @param client_id_1 Client id to be used to acquire the lease.
+    /// @param hw_address_2 HW Address to be used to release the lease.
+    /// @param client_id_2 Client id to be used to release the lease.
+    /// @param expected_result SHOULD_PASS if the lease is expected to
+    /// be successfully released, or SHOULD_FAIL if the lease is expected
+    /// to not be released.
+    void acquireAndRelease(const std::string& hw_address_1,
+                           const std::string& client_id_1,
+                           const std::string& hw_address_2,
+                           const std::string& client_id_2,
+                           ExpectedResult expected_result);
+
+    /// @brief Interface Manager's fake configuration control.
+    IfaceMgrTestConfig iface_mgr_test_config_;
+
+};
+
+void
+ReleaseTest::acquireLease(Dhcp4Client& client) {
+    // Perform 4-way exchange with the server but to not request any
+    // specific address in the DHCPDISCOVER message.
+    ASSERT_NO_THROW(client.doDORA());
+    // Make sure that the server responded.
+    ASSERT_TRUE(client.getContext().response_);
+    Pkt4Ptr resp = client.getContext().response_;
+    // Make sure that the server has responded with DHCPACK.
+    ASSERT_EQ(DHCPACK, static_cast<int>(resp->getType()));
+    // Response must not be relayed.
+    EXPECT_FALSE(resp->isRelayed());
+    // Make sure that the server id is present.
+    EXPECT_EQ("10.0.0.1", client.config_.serverid_.toText());
+    // Make sure that the client has got the lease with the requested address.
+    ASSERT_NE(client.config_.lease_.addr_.toText(), "0.0.0.0");
+    Lease4Ptr lease = LeaseMgrFactory::instance().getLease4(client.config_.lease_.addr_);
+    ASSERT_TRUE(lease);
+}
+
+void
+ReleaseTest::acquireAndRelease(const std::string& hw_address_1,
+                               const std::string& client_id_1,
+                               const std::string& hw_address_2,
+                               const std::string& client_id_2,
+                               ExpectedResult expected_result) {
+    CfgMgr::instance().clear();
+    Dhcp4Client client(Dhcp4Client::SELECTING);
+    // Configure DHCP server.
+    configure(RELEASE_CONFIGS[0], *client.getServer());
+    // Explicitly set the client id.
+    client.includeClientId(client_id_1);
+    // Explicitly set the HW address.
+    client.setHWAddress(hw_address_1);
+    // Perform 4-way exchange to obtain a new lease.
+    acquireLease(client);
+
+    // Remember the acquired address.
+    IOAddress leased_address = client.config_.lease_.addr_;
+
+    // Explicitly set the client id for DHCPRELEASE.
+    client.includeClientId(client_id_2);
+    // Explicitly set the HW address for DHCPRELEASE.
+    client.setHWAddress(hw_address_2);
+
+    // Send the release and make sure that the lease is removed from the
+    // server.
+    ASSERT_NO_THROW(client.doRelease());
+    Lease4Ptr lease = LeaseMgrFactory::instance().getLease4(leased_address);
+
+    // We check if the release process was successful by checking if the
+    // lease is in the database. It is expected that it is not present,
+    // i.e. has been deleted with the release.
+    if (expected_result == SHOULD_PASS) {
+        EXPECT_FALSE(lease);
+
+    } else {
+        EXPECT_TRUE(lease);
+    }
+}
+
+// This test checks that the client can acquire and release the lease.
+TEST_F(ReleaseTest, releaseNoIdentifierChange) {
+    acquireAndRelease("01:02:03:04:05:06", "12:14",
+                      "01:02:03:04:05:06", "12:14",
+                      SHOULD_PASS);
+}
+
+// This test verifies the release correctness in the following case:
+// - Client acquires new lease using HW address only
+// - Client sends the DHCPRELEASE with valid HW address and without
+//   client identifier.
+// - The server successfully releases the lease.
+TEST_F(ReleaseTest, releaseHWAddressOnly) {
+    acquireAndRelease("01:02:03:04:05:06", "",
+                      "01:02:03:04:05:06", "",
+                      SHOULD_PASS);
+}
+
+// This test verifies the release correctness in the following case:
+// - Client acquires new lease using the client identifier and HW address
+// - Client sends the DHCPRELEASE with valid HW address but with no
+//   client identifier.
+// - The server successfully releases the lease.
+TEST_F(ReleaseTest, releaseNoClientId) {
+    acquireAndRelease("01:02:03:04:05:06", "12:14",
+                      "01:02:03:04:05:06", "",
+                      SHOULD_PASS);
+}
+
+// This test verifies the release correctness in the following case:
+// - Client acquires new lease using HW address
+// - Client sends the DHCPRELEASE with valid HW address and some
+//   client identifier.
+// - The server identifies the lease using HW address and releases
+//   this lease.
+TEST_F(ReleaseTest, releaseNoClientId2) {
+    acquireAndRelease("01:02:03:04:05:06", "",
+                      "01:02:03:04:05:06", "12:14",
+                      SHOULD_PASS);
+}
+
+// This test checks the server's behavior in the following case:
+// - Client acquires new lease using the client identifier and HW address
+// - Client sends the DHCPRELEASE with the valid HW address but with invalid
+//   client identifier.
+// - The server should not remove the lease.
+TEST_F(ReleaseTest, releaseNonMatchingClientId) {
+    acquireAndRelease("01:02:03:04:05:06", "12:14",
+                      "01:02:03:04:05:06", "12:15:16",
+                      SHOULD_FAIL);
+}
+
+// This test checks the server's behavior in the following case:
+// - Client acquires new lease using client identifier and HW address
+// - Client sends the DHCPRELEASE with the same client identifier but
+//   different HW address.
+// - The server uses client identifier to find the client's lease and
+//   releases it.
+TEST_F(ReleaseTest, releaseNonMatchingHWAddress) {
+    acquireAndRelease("01:02:03:04:05:06", "12:14",
+                      "06:06:06:06:06:06", "12:14",
+                      SHOULD_PASS);
+}
+
+// This test checks the server's behavior in the following case:
+// - Client acquires new lease.
+// - Client sends DHCPRELEASE with the ciaddr set to a different
+//   address than it has acquired from the server.
+// - Server determines that the client is trying to release a
+//   wrong address and will refuse to release.
+TEST_F(ReleaseTest, releaseNonMatchingIPAddress) {
+    Dhcp4Client client(Dhcp4Client::SELECTING);
+    // Configure DHCP server.
+    configure(RELEASE_CONFIGS[0], *client.getServer());
+    // Perform 4-way exchange to obtain a new lease.
+    acquireLease(client);
+
+    // Remember the acquired address.
+    IOAddress leased_address = client.config_.lease_.addr_;
+
+    // Modify the client's address to force it to release a different address
+    // than it has obtained from the server.
+    client.config_.lease_.addr_ = IOAddress(static_cast<uint32_t>(leased_address) + 1);
+
+    // Send DHCPRELEASE and make sure it was unsuccessful, i.e. the lease
+    // remains in the database.
+    ASSERT_NO_THROW(client.doRelease());
+    Lease4Ptr lease = LeaseMgrFactory::instance().getLease4(leased_address);
+    ASSERT_TRUE(lease);
+}
+
+} // end of anonymous namespace

src/bin/dhcp6/json_config_parser.cc

@@ -704,6 +704,9 @@ configureDhcp6Server(Dhcpv6Srv&, isc::data::ConstElementPtr config_set) {
     LOG_DEBUG(dhcp6_logger, DBG_DHCP6_COMMAND,
               DHCP6_CONFIG_START).arg(config_set->str());
 
+    // Reset global context.
+    globalContext().reset(new ParserContext(Option::V6));
+
     // Before starting any subnet operations, let's reset the subnet-id counter,
     // so newly recreated configuration starts with first subnet-id equal 1.
     Subnet::resetSubnetID();

src/bin/dhcp6/tests/config_parser_unittest.cc

@@ -2238,6 +2238,8 @@ TEST_F(Dhcp6ParserTest, optionDataEncapsulate) {
     // configuration manager sends whole configuration for the lists
     // where at least one element is being modified or added.
     config = "{ " + genIfaceConfig() + ","
+        "\"preferred-lifetime\": 3000,"
+        "\"valid-lifetime\": 4000,"
         "\"rebind-timer\": 2000,"
         "\"renew-timer\": 1000,"
         "\"option-data\": [ {"

src/bin/dhcp6/tests/dhcp6_test_utils.cc

@@ -17,6 +17,7 @@
 #include <dhcp6/tests/dhcp6_test_utils.h>
 #include <dhcp6/json_config_parser.h>
 #include <config/ccsession.h>
+#include <util/pointer_util.h>
 #include <string.h>
 
 using namespace isc::data;
@@ -134,7 +135,9 @@ Dhcpv6SrvTest::checkLease(const isc::dhcp::Lease6& lease) {
         return (Lease6Ptr());
     }
 
-    EXPECT_TRUE(lease_db->matches(lease));
+    EXPECT_TRUE(util::nullOrEqualValues(lease_db->hwaddr_, lease.hwaddr_));
+    EXPECT_TRUE(util::nullOrEqualValues(lease_db->duid_, lease.duid_));
+
     return (lease_db);
 }
 

src/bin/dhcp6/tests/dhcp6_test_utils.h

@@ -386,7 +386,7 @@ public:
     }
 
     // Checks if the lease sent to client is present in the database
-    // and is valid when checked agasint the configured subnet
+    // and is valid when checked against the configured subnet
     isc::dhcp::Lease6Ptr checkLease
         (const isc::dhcp::DuidPtr& duid, const isc::dhcp::OptionPtr& ia_na,
          boost::shared_ptr<isc::dhcp::Option6IAAddr> addr);

src/lib/dhcp/iface_mgr.cc

@@ -238,7 +238,7 @@ Iface::setActive(const IOAddress& address, const bool active) {
     for (AddressCollection::iterator addr_it = addrs_.begin();
          addr_it != addrs_.end(); ++addr_it) {
         if (address == addr_it->get()) {
-            addr_it->specify(active);
+            addr_it->specify(OptionalValueState(active));
             return;
         }
     }
@@ -250,7 +250,7 @@ void
 Iface::setActive(const bool active) {
     for (AddressCollection::iterator addr_it = addrs_.begin();
          addr_it != addrs_.end(); ++addr_it) {
-        addr_it->specify(active);
+        addr_it->specify(OptionalValueState(active));
     }
 }
 

src/lib/dhcpsrv/alloc_engine.cc

@@ -1228,69 +1228,43 @@ hasAddressReservation(const AllocEngine::ClientContext4& ctx) {
     return (ctx.host_ && !ctx.host_->getIPv4Reservation().isV4Zero());
 }
 
-/// @brief Check if there is a lease for the client which message is processed.
+/// @brief Finds existing lease in the database.
 ///
-/// This function searches the lease database to find existing lease for the client.
-/// It finds the lease using the client's HW address first. If the lease exists and
-/// appears to belong to the client the lease is returned. Otherwise, the function
-/// will search for the lease using the client identifier (if supplied). If the
-/// lease exists and appears to belong to the client, it is returned.
+/// This function searches for the lease in the database which belongs to the
+/// client requesting allocation. If the client has supplied the client
+/// identifier this identifier is used to look up the lease. If the lease is
+/// not found using the client identifier, an additional lookup is performed
+/// using the HW address, if supplied. If the lease is found using the HW
+/// address, the function also checks if the lease belongs to the client, i.e.
+/// there is no conflict between the client identifiers.
 ///
-/// This function also identifies the conflicts between existing leases and the
-/// lease to be allocated for the client, when the client is using a HW address
-/// or client identifier which is already in use by the client having a lease in
-/// the database. If the client uses an identifier which is already used by another
-/// client and no other unique identifier which could be used to identify the client's
-/// lease this function signals the conflict by returning 'true'.
-///
-/// @param ctx Client context.
-/// @param [out] client_lease Client's lease found in the database.
-///
-/// @return true if there is a conflict of identifiers (HW address or client id)
-/// between the client which message is being processed and the client which has
-/// a lease in the database. When the value 'true' is returned, the caller should
-/// cease the lease allocation for the client.
-bool matchClientLease(const AllocEngine::ClientContext4& ctx, Lease4Ptr& client_lease) {
-    // Obtain the sole instance of the LeaseMgr.
+/// @param ctx Context holding data extracted from the client's message,
+/// including the HW address and client identifier.
+/// @param [out] client_lease A pointer to the lease returned by this function
+/// or null value if no has been lease found.
+void findClientLease(const AllocEngine::ClientContext4& ctx, Lease4Ptr& client_lease) {
     LeaseMgr& lease_mgr = LeaseMgrFactory::instance();
-
-    // The server should hand out existing lease to the client, so we have to check
-    // if there is one. First, try to use the client's HW address.
-    client_lease = lease_mgr.getLease4(*ctx.hwaddr_, ctx.subnet_->getID());
-    // If there is no lease for this HW address or the lease doesn't seem to be ours,
-    // we will have to use the client identifier. Note that in some situations two
-    // clients may use the same HW address so even if we find the lease for the HW
-    // address it doesn't mean it is ours, because client identifier may not match.
-    if (ctx.clientid_ && ((!client_lease) || (client_lease && !ctx.myLease(*client_lease)))) {
-        // Check if the lease is in conflict with the lease that we want to allocate.
-        // If the lease is in conflict because of using overlapping HW address or
-        // client identifier, we can't allocate the lease for this client.
-        if (client_lease && ctx.isInConflict(*client_lease)) {
-            return (true);
-        }
-        // There is no lease or the lease we found is not conflicting with the lease
-        // which we have found for the HW address, so there is still a chance that
-        // we will allocate the lease. Check if there is a lease using the client
-        // identifier.
+    // 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_, ctx.subnet_->getID());
     }
 
-    // Check if the lease we have found belongs to us.
-    if (client_lease && !ctx.myLease(*client_lease)) {
-        // If the lease doesn't belong to us, check if we can add new lease for
-        // the client which message we're processing, or its identifiers are
-        // in conflict with this lease.
-        if (ctx.isInConflict(*client_lease)) {
-            return (true);
+    // If no lease found using the client identifier, try the lookup using
+    // the HW address.
+    if (!client_lease && ctx.hwaddr_) {
+        client_lease = lease_mgr.getLease4(*ctx.hwaddr_, ctx.subnet_->getID());
+        // This lookup may return the lease which has conflicting client
+        // identifier and thus is considered to belong to someone else.
+        // If this is the case, we need to toss the result and force the
+        // Allocation Engine to allocate another lease.
+        if (client_lease && !client_lease->belongsToClient(ctx.hwaddr_, ctx.clientid_)) {
+            client_lease.reset();
         }
-        // If there is no conflict we can proceed and try to find the appropriate
-        // lease but we don't use the one we found, because it is assigned to
-        // someone else. Reset the pointer to indicate that we're not
-        // renewing this lease.
-        client_lease.reset();
     }
-
-    return (false);
 }
 
 } // end of anonymous namespace
@@ -1321,37 +1295,6 @@ AllocEngine::ClientContext4::ClientContext4(const Subnet4Ptr& subnet,
       fake_allocation_(fake_allocation), old_lease_(), host_() {
 }
 
-
-
-bool
-AllocEngine::ClientContext4::myLease(const Lease4& lease) const {
-    if ((!hwaddr_ && lease.hwaddr_) || (hwaddr_ && !lease.hwaddr_)) {
-        return (false);
-    }
-
-    if ((hwaddr_ && lease.hwaddr_) && (hwaddr_->hwaddr_ != lease.hwaddr_->hwaddr_)) {
-        return (false);
-    }
-
-    if ((!clientid_ && lease.client_id_) || (clientid_ && !lease.client_id_)) {
-        return (false);
-    }
-
-    if ((clientid_ && lease.client_id_) && (*clientid_ != *lease.client_id_)) {
-        return (false);
-    }
-
-    return (true);
-}
-
-bool
-AllocEngine::ClientContext4::isInConflict(const Lease4& lease) const {
-    return ((!(hwaddr_ && lease.hwaddr_) && (clientid_ && lease.client_id_) &&
-             (*clientid_ == *lease.client_id_)) ||
-            (!(clientid_ && lease.client_id_) && (hwaddr_ && lease.hwaddr_) &&
-             (hwaddr_->hwaddr_ == lease.hwaddr_->hwaddr_)));
-}
-
 Lease4Ptr
 AllocEngine::allocateLease4(ClientContext4& ctx) {
     // The NULL pointer indicates that the old lease didn't exist. It may
@@ -1410,10 +1353,7 @@ AllocEngine::discoverLease4(AllocEngine::ClientContext4& ctx) {
     // if there is a conflict with existing lease and the allocation should
     // not be continued.
     Lease4Ptr client_lease;
-    bool conflict = matchClientLease(ctx, client_lease);
-    if (conflict) {
-        return (Lease4Ptr());
-    }
+    findClientLease(ctx, client_lease);
 
     // new_lease will hold the pointer to the lease that we will offer to the
     // caller.
@@ -1499,10 +1439,7 @@ AllocEngine::requestLease4(AllocEngine::ClientContext4& ctx) {
     // if there is a conflict with existing lease and the allocation should
     // not be continued.
     Lease4Ptr client_lease;
-    bool conflict = matchClientLease(ctx, client_lease);
-    if (conflict) {
-        return (Lease4Ptr());
-    }
+    findClientLease(ctx, client_lease);
 
     // Obtain the sole instance of the LeaseMgr.
     LeaseMgr& lease_mgr = LeaseMgrFactory::instance();
@@ -1537,7 +1474,8 @@ AllocEngine::requestLease4(AllocEngine::ClientContext4& ctx) {
         // if the address is in use by our client or another client.
         // If it is in use by another client, the address can't be
         // allocated.
-        if (existing && !existing->expired() && !ctx.myLease(*existing)) {
+        if (existing && !existing->expired() &&
+            !existing->belongsToClient(ctx.hwaddr_, ctx.clientid_)) {
             return (Lease4Ptr());
         }
 

src/lib/dhcpsrv/alloc_engine.h

@@ -760,45 +760,6 @@ public:
                        const bool fwd_dns_update, const bool rev_dns_update,
                        const std::string& hostname, const bool fake_allocation);
 
-        /// @brief Check if the specified lease belongs to the client.
-        ///
-        /// This method compares the hardware address and the client id
-        /// in the lease with the relevant values in the context. That
-        /// way the method determines whether the lease belongs to the
-        /// client which message the server is processing.
-        ///
-        /// @return true if the lease belongs to the client for which
-        /// the context has been created, false otherwise.
-        bool myLease(const Lease4& lease) const;
-
-        /// @brief Check if the lease conflicts with the context.
-        ///
-        /// This method is used in cases when there is a lease in the lease database
-        /// for the client and the server receives the message from another client
-        /// which may be potentially using the same client identifier or HW address.
-        ///
-        /// Currently the allocation engine allows for allocating a lease for the
-        /// client which is using the same HW address or client identifier as the
-        /// client which already has a lease in the database. However, the value of
-        /// one of the identifiers must be unique. In other words, two clients may
-        /// have the same client identifier but different HW addresses and the
-        /// leases can be allocated for both (there is no conflict).
-        ///
-        /// The other possible case is that one of the clients uses both HW address
-        /// and client identifier, and another client is using only one of those
-        /// and its value is the same as for the former client. The allocation
-        /// engine treats it as a conflict because there is no unique value in the
-        /// lease database by which the server could distinguish the clients.
-        /// Hence, it doesn't allocate the lease from the context in conflict.
-        ///
-        /// This method detects the conflict described above.
-        ///
-        /// @param lease Existing lease to be checked.
-        ///
-        /// @return true if the lease is in conflict with the context, false
-        /// otherwise.
-        bool isInConflict(const Lease4& lease) const;
-
     };
 
     /// @brief Pointer to the @c ClientContext4.

src/lib/dhcpsrv/lease.cc

@@ -1,4 +1,4 @@
-// Copyright (C) 2012-2014 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2012-2015 Internet Systems Consortium, Inc. ("ISC")
 //
 // Permission to use, copy, modify, and/or distribute this software for any
 // purpose with or without fee is hereby granted, provided that the above
@@ -13,8 +13,10 @@
 // PERFORMANCE OF THIS SOFTWARE.
 
 #include <dhcpsrv/lease.h>
+#include <util/pointer_util.h>
 #include <sstream>
 
+using namespace isc::util;
 using namespace std;
 
 namespace isc {
@@ -29,6 +31,7 @@ Lease::Lease(const isc::asiolink::IOAddress& addr, uint32_t t1, uint32_t t2,
      fqdn_fwd_(fqdn_fwd), fqdn_rev_(fqdn_rev), hwaddr_(hwaddr) {
 }
 
+
 std::string
 Lease::typeToText(Lease::Type type) {
    switch (type) {
@@ -88,6 +91,25 @@ Lease4::Lease4(const Lease4& other)
     }
 }
 
+Lease4::Lease4(const isc::asiolink::IOAddress& address,
+               const HWAddrPtr& hw_address,
+               const ClientIdPtr& client_id,
+               const uint32_t valid_lifetime,
+               const uint32_t t1,
+               const uint32_t t2,
+               const time_t cltt,
+               const SubnetID subnet_id,
+               const bool fqdn_fwd,
+               const bool fqdn_rev,
+               const std::string& hostname)
+
+    : Lease(address, t1, t2, valid_lifetime, subnet_id, cltt, fqdn_fwd,
+            fqdn_rev, hostname, hw_address),
+      ext_(0), client_id_(client_id) {
+}
+
+
+
 const std::vector<uint8_t>&
 Lease4::getClientIdVector() const {
     if(!client_id_) {
@@ -108,35 +130,20 @@ Lease::getHWAddrVector() const {
 }
 
 bool
-Lease4::matches(const Lease4& other) const {
-    if ((client_id_ && !other.client_id_) ||
-        (!client_id_ && other.client_id_)) {
-        // One lease has client-id, but the other doesn't
-        return false;
-    }
-
-    if (client_id_ && other.client_id_ &&
-        *client_id_ != *other.client_id_) {
-        // Different client-ids
-        return false;
-    }
-
-    // Note that hwaddr_ is now a poiner to the HWAddr structure.
-    // We can't simply compare smart pointers, we need to compare the
-    // actual objects they point to. If one of the leases had a hardware address
-    // and the other doesn't, they clearly don't match.
-    if ( (!hwaddr_ && other.hwaddr_) ||
-         (hwaddr_ && !other.hwaddr_) ) {
-        return (false);
+Lease4::belongsToClient(const HWAddrPtr& hw_address,
+                        const ClientIdPtr& client_id) const {
+    // If client id matches, lease matches.
+    if (equalValues(client_id, client_id_)) {
+        return (true);
+
+    } else if (!client_id || !client_id_) {
+        // If client id is unspecified, use HW address.
+        if (equalValues(hw_address, hwaddr_)) {
+            return (true);
+        }
     }
 
-    // The lease is equal if addresses and extensions match and there is
-    // either the same hardware address on both or neither have hardware address
-    // specified.
-    return (addr_ == other.addr_ &&
-            ext_ == other.ext_ &&
-            (!hwaddr_ ||
-             *hwaddr_ == *other.hwaddr_) );
+    return (false);
 }
 
 Lease4&
@@ -241,13 +248,13 @@ std::string
 Lease4::toText() const {
     ostringstream stream;
 
-    /// @todo: print out client-id (if present)
     stream << "Address:       " << addr_ << "\n"
            << "Valid life:    " << valid_lft_ << "\n"
            << "T1:            " << t1_ << "\n"
            << "T2:            " << t2_ << "\n"
            << "Cltt:          " << cltt_ << "\n"
-           << "Hardware addr: " << (hwaddr_?hwaddr_->toText(false):"(none)") << "\n"
+           << "Hardware addr: " << (hwaddr_ ? hwaddr_->toText(false) : "(none)") << "\n"
+           << "Client id:     " << (client_id_ ? client_id_->toText() : "(none)") << "\n"
            << "Subnet ID:     " << subnet_id_ << "\n";
 
     return (stream.str());
@@ -256,19 +263,10 @@ Lease4::toText() const {
 
 bool
 Lease4::operator==(const Lease4& other) const {
-    if ( (client_id_ && !other.client_id_) ||
-         (!client_id_ && other.client_id_) ) {
-        // One lease has client-id, but the other doesn't
-        return false;
-    }
-
-    if (client_id_ && other.client_id_ &&
-        *client_id_ != *other.client_id_) {
-        // Different client-ids
-        return false;
-    }
-
-    return (matches(other) &&
+    return (nullOrEqualValues(hwaddr_, other.hwaddr_) &&
+            nullOrEqualValues(client_id_, other.client_id_) &&
+            addr_ == other.addr_ &&
+            ext_ == other.ext_ &&
             subnet_id_ == other.subnet_id_ &&
             t1_ == other.t1_ &&
             t2_ == other.t2_ &&
@@ -282,29 +280,13 @@ Lease4::operator==(const Lease4& other) const {
 }
 
 bool
-Lease6::matches(const Lease6& other) const {
-
-    // One lease has a hardware address, the other doesn't.
-    if ( (!hwaddr_ && other.hwaddr_) ||
-         (hwaddr_ && !other.hwaddr_) ) {
-        return (false);
-    }
-
-    // Both leases have hardware address, but they are not equal.
-    if (hwaddr_ && (*hwaddr_ != *other.hwaddr_)) {
-        return (false);
-    }
-
-    return (addr_ == other.addr_ &&
+Lease6::operator==(const Lease6& other) const {
+    return (nullOrEqualValues(duid_, other.duid_) &&
+            nullOrEqualValues(hwaddr_, other.hwaddr_) &&
+            addr_ == other.addr_ &&
             type_ == other.type_ &&
             prefixlen_ == other.prefixlen_ &&
             iaid_ == other.iaid_ &&
-            *duid_ == *other.duid_);
-}
-
-bool
-Lease6::operator==(const Lease6& other) const {
-    return (matches(other) &&
             preferred_lft_ == other.preferred_lft_ &&
             valid_lft_ == other.valid_lft_ &&
             t1_ == other.t1_ &&

src/lib/dhcpsrv/lease.h

@@ -217,6 +217,32 @@ struct Lease4 : public Lease {
         }
     }
 
+    /// @brief Constructor.
+    ///
+    /// @param address IPv4 address.
+    /// @param hw_address Pointer to client's HW addresss.
+    /// @param client_id  pointer to the client id structure.
+    /// @param valid_lifetime Valid lifetime value.
+    /// @param t1 Renew timer.
+    /// @param t2 Rebind timer.
+    /// @param cltt Timestamp when the lease is acquired, renewed.
+    /// @param subnet_id Subnet identifier.
+    /// @param fqdn_fwd Forward DNS update performed.
+    /// @param fqdn_rev Reverse DNS update performed.
+    /// @param hostname Client's name for the DNS update..
+    Lease4(const isc::asiolink::IOAddress& address,
+           const HWAddrPtr& hw_address,
+           const ClientIdPtr& client_id,
+           const uint32_t valid_lifetime,
+           const uint32_t t1,
+           const uint32_t t2,
+           const time_t cltt,
+           const SubnetID subnet_id,
+           const bool fqdn_fwd = false,
+           const bool fqdn_rev = false,
+           const std::string& hostname = "");
+
+
     /// @brief Default constructor
     ///
     /// Initialize fields that don't have a default constructor.
@@ -239,19 +265,68 @@ struct Lease4 : public Lease {
     /// or an empty vector if client identifier is NULL.
     const std::vector<uint8_t>& getClientIdVector() const;
 
-    /// @brief Check if two objects encapsulate the lease for the same
-    /// client.
-    ///
-    /// Checks if two @c Lease4 objects have the same address, client id,
-    /// HW address and ext_ value.  If these parameters match it is an
-    /// indication that both objects describe the lease for the same
-    /// client but apparently one is a result of renewal of the other. The
-    /// special case of the matching lease is the one that is equal to another.
-    ///
-    /// @param other A lease to compare with.
-    ///
-    /// @return true if the selected parameters of the two leases match.
-    bool matches(const Lease4& other) const;
+    /// @brief Check if the lease belongs to the client with the given
+    /// identifiers.
+    ///
+    /// This method checks if the lease belongs to the client using the
+    /// specified HW address and/or client identifier. Note that any of the
+    /// pointers passed to this method may be set to null, in which case
+    /// they are treated as unspecified and are not used for matching the
+    /// client with the lease.
+    ///
+    /// According to the DHCPv4 specifications, the client identifier takes
+    /// precedence over the HW address when identifying the lease for the
+    /// client on the server side. In particular, the RFC4361 introduces the
+    /// use of DUID for DHCPv4 which should be a stable identifier for the
+    /// client. The use of stable identifier allows for the correlation of the
+    /// DHCPv4 and DHCPv6 clients in the dual stack networks. It also allows
+    /// for allocating the same lease to the client which hardware (and thus
+    /// MAC address) has changed.
+    ///
+    /// By default, Kea respects the precedence of the client identifier over
+    /// MAC address and when this method finds the match of the client
+    /// identifier with the client identifier stored in the lease, it will
+    /// treat the lease as the lease of this client, even when the HW
+    /// address doesn't match.
+    ///
+    /// The HW address is used for matching the client with the lease only
+    /// when the lease is not associated with any client identifier (client
+    /// identifier for the lease is null) or when the client identifier
+    /// parameter passed to this method is null. This facilitates the following
+    /// cases:
+    /// - client didn't generate client identifier and is only using the chaddr
+    ///   field to identify itself.
+    /// - server's administrator configured the server to NOT match client
+    ///   identifiers, the client obtained the new lease, and the administrator
+    ///   reconfigured the server to match the client identifiers. The client
+    ///   is trying to renew its lease and both the client identifier and HW
+    ///   address is used for matching the lease which doesn't have the record
+    ///   of the client identifier.
+    /// - client obtained the lease using the HW address and client identifier,
+    ///   the server's administrator configured the server to NOT match the
+    ///   client identifiers, and the client returns to renew the lease. This
+    ///   time, the lease has a record of both client identifier and the HW
+    ///   address but only the HW address is used for matching the client to
+    ///   the lease.
+    ///
+    /// Note that the typical case when the server's administrator may want to
+    /// disable matching the client identifier passed in the client's message
+    /// is when the client is performing multi-stage boot. In such case, the
+    /// client identifiers may change on various stages of the boot, but the
+    /// HW address will remain stable. The server's administrator prefers
+    /// using the HW address for client identification in this case.
+    ///
+    /// It may also be useful to disable matching client identifiers to
+    /// mitigate the problem of broken client implementations which generate
+    /// new client identifiers every time they connect to the network.
+    ///
+    /// @param hw_address Pointer to the HW address of the client.
+    /// @param client_id Pointer to the client identifier structure.
+    ///
+    /// @return true if the lease belongs to the client using the specified
+    /// hardware address and/or client identifier.
+    bool belongsToClient(const HWAddrPtr& hw_address,
+                         const ClientIdPtr& client_id) const;
 
     /// @brief Assignment operator.
     ///
@@ -374,20 +449,6 @@ struct Lease6 : public Lease {
     /// @return A reference to a vector holding a DUID.
     const std::vector<uint8_t>& getDuidVector() const;
 
-    /// @brief Checks if two lease objects encapsulate the lease for the same
-    /// client.
-    ///
-    /// This function compares address, type, prefix length, IAID and DUID
-    /// parameters between two @c Lease6 objects. If these parameters match
-    /// it is an indication that both objects describe the lease for the same
-    /// client but apparently one is a result of renewal of the other. The
-    /// special case of the matching lease is the one that is equal to another.
-    ///
-    /// @param other A lease to compare to.
-    ///
-    /// @return true if selected parameters of the two leases match.
-    bool matches(const Lease6& other) const;
-
     /// @brief Compare two leases for equality
     ///
     /// @param other lease6 object with which to compare

src/lib/dhcpsrv/parsers/dhcp_parsers.cc

@@ -1030,7 +1030,9 @@ PoolParser::commit() {
 SubnetConfigParser::SubnetConfigParser(const std::string&,
                                        ParserContextPtr global_context,
                                        const isc::asiolink::IOAddress& default_addr)
-    : uint32_values_(new Uint32Storage()), string_values_(new StringStorage()),
+    : uint32_values_(new Uint32Storage()),
+      string_values_(new StringStorage()),
+      boolean_values_(new BooleanStorage()),
       pools_(new PoolStorage()), global_context_(global_context),
       relay_info_(new isc::dhcp::Subnet::RelayInfo(default_addr)),
       options_(new CfgOption()) {

src/lib/dhcpsrv/parsers/dhcp_parsers.h

@@ -1063,6 +1063,9 @@ protected:
     /// Storage for subnet-specific string values.
     StringStoragePtr string_values_;
 
+    /// Storage for subnet-specific boolean values.
+    BooleanStoragePtr boolean_values_;
+
     /// Storage for pools belonging to this subnet.
     PoolStoragePtr pools_;
 

src/lib/dhcpsrv/subnet.cc

@@ -179,8 +179,8 @@ Subnet4::Subnet4(const isc::asiolink::IOAddress& prefix, uint8_t length,
                  const Triplet<uint32_t>& t2,
                  const Triplet<uint32_t>& valid_lifetime,
                  const SubnetID id)
-:Subnet(prefix, length, t1, t2, valid_lifetime,
-        RelayInfo(IOAddress("0.0.0.0")), id), siaddr_(IOAddress("0.0.0.0")) {
+    : Subnet(prefix, length, t1, t2, valid_lifetime, RelayInfo(IOAddress("0.0.0.0")), id),
+      siaddr_(IOAddress("0.0.0.0")), match_client_id_(true) {
     if (!prefix.isV4()) {
         isc_throw(BadValue, "Non IPv4 prefix " << prefix.toText()
                   << " specified in subnet4");

src/lib/dhcpsrv/subnet.h

@@ -542,7 +542,24 @@ public:
     /// @return siaddr value
     isc::asiolink::IOAddress getSiaddr() const;
 
-protected:
+    /// @brief Sets the flag indicating if the client identifier should be
+    /// used to identify the client's lease.
+    ///
+    /// @param match If this value is true, the client identifiers are not
+    /// used for lease lookup.
+    void setMatchClientId(const bool match) {
+        match_client_id_ = match;
+    }
+
+    /// @brief Returns the flag indicating if the client identifiers should
+    /// be used to identify the client's lease.
+    ///
+    /// @return true if client identifiers should be used, false otherwise.
+    bool getMatchClientId() const {
+        return (match_client_id_);
+    }
+
+private:
 
     /// @brief Returns default address for pool selection
     /// @return ANY IPv4 address
@@ -560,6 +577,10 @@ protected:
 
     /// @brief siaddr value for this subnet
     isc::asiolink::IOAddress siaddr_;
+
+    /// @brief Should server use client identifiers for client lease
+    /// lookup.
+    bool match_client_id_;
 };
 
 /// @brief A pointer to a @c Subnet4 object

src/lib/dhcpsrv/tests/alloc_engine4_unittest.cc

@@ -536,6 +536,69 @@ TEST_F(AllocEngine4Test, requestReuseExpiredLease4) {
     EXPECT_TRUE(*ctx.old_lease_ == original_lease);
 }
 
+// This test checks that the Allocation Engine correcly identifies the
+// existing client's lease in the lease database, using the client
+// identifier and HW address.
+TEST_F(AllocEngine4Test, identifyClientLease) {
+    Lease4Ptr lease(new Lease4(IOAddress("192.0.2.101"), hwaddr_, clientid_,
+                               100, 30, 60, time(NULL), subnet_->getID()));
+    LeaseMgrFactory::instance().addLease(lease);
+
+    AllocEngine engine(AllocEngine::ALLOC_ITERATIVE, 100, false);
+    AllocEngine::ClientContext4 ctx(subnet_, clientid_, hwaddr_,
+                                    IOAddress::IPV4_ZERO_ADDRESS(),
+                                    false, false, "", true);
+
+    Lease4Ptr identified_lease = engine.allocateLease4(ctx);
+    ASSERT_TRUE(identified_lease);
+    EXPECT_EQ("192.0.2.101", identified_lease->addr_.toText());
+
+    ctx.hwaddr_ = hwaddr2_;
+    ctx.clientid_ = clientid_;
+    identified_lease = engine.allocateLease4(ctx);
+    ASSERT_TRUE(identified_lease);
+    EXPECT_EQ("192.0.2.101", identified_lease->addr_.toText());
+
+    ctx.hwaddr_ = hwaddr_;
+    ctx.clientid_ = clientid2_;
+    identified_lease = engine.allocateLease4(ctx);
+    ASSERT_TRUE(identified_lease);
+    EXPECT_NE(identified_lease->addr_.toText(), "192.0.2.101");
+
+    ctx.hwaddr_ = hwaddr_;
+    ctx.clientid_.reset();
+    identified_lease = engine.allocateLease4(ctx);
+    ASSERT_TRUE(identified_lease);
+    EXPECT_EQ("192.0.2.101", identified_lease->addr_.toText());
+
+    ctx.hwaddr_ = hwaddr2_;
+    ctx.clientid_.reset();
+    identified_lease = engine.allocateLease4(ctx);
+    ASSERT_TRUE(identified_lease);
+    EXPECT_NE(identified_lease->addr_.toText(), "192.0.2.101");
+
+    lease->client_id_.reset();
+    ASSERT_NO_THROW(LeaseMgrFactory::instance().updateLease4(lease));
+
+    ctx.hwaddr_ = hwaddr_;
+    ctx.clientid_ = clientid_;
+    identified_lease = engine.allocateLease4(ctx);
+    ASSERT_TRUE(identified_lease);
+    EXPECT_EQ("192.0.2.101", identified_lease->addr_.toText());
+
+    ctx.hwaddr_ = hwaddr_;
+    ctx.clientid_.reset();
+    identified_lease = engine.allocateLease4(ctx);
+    ASSERT_TRUE(identified_lease);
+    EXPECT_EQ("192.0.2.101", identified_lease->addr_.toText());
+
+    ctx.hwaddr_ = hwaddr2_;
+    ctx.clientid_ = clientid_;
+    identified_lease = engine.allocateLease4(ctx);
+    ASSERT_TRUE(identified_lease);
+    EXPECT_NE(identified_lease->addr_.toText(), "192.0.2.101");
+}
+
 // This test checks that when the client requests the address which belongs
 // to another client, the allocation engine returns NULL (for the
 // DHCPREQUEST case) or a lease for the address which belongs to this
@@ -924,7 +987,7 @@ TEST_F(AllocEngine4Test, reservedAddressExistingLeaseInvalidHint) {
     CfgMgr::instance().commit();
 
     // Create a lease for the client for a different address than reserved.
-    Lease4Ptr lease(new Lease4(IOAddress("192.0.2.101"), hwaddr_, 0, 0,
+    Lease4Ptr lease(new Lease4(IOAddress("192.0.2.101"), hwaddr_, ClientIdPtr(),
                                100, 30, 60, time(NULL), subnet_->getID(),
                                false, false, ""));
     LeaseMgrFactory::instance().addLease(lease);
@@ -946,6 +1009,7 @@ TEST_F(AllocEngine4Test, reservedAddressExistingLeaseInvalidHint) {
     AllocEngine::ClientContext4 ctx2(subnet_, clientid_, hwaddr_,
                                     IOAddress("192.0.2.101"), false, false,
                                     "", false);
+    AllocEngine::findReservation(ctx2);
     allocated_lease = engine.allocateLease4(ctx2);
     // The client has reservation so the server wants to allocate a
     // reserved address and doesn't want to renew the address that the

src/lib/dhcpsrv/tests/alloc_engine_utils.cc

@@ -351,6 +351,8 @@ AllocEngine4Test::AllocEngine4Test() {
     HostMgr::instance().create();
 
     clientid_ = ClientIdPtr(new ClientId(vector<uint8_t>(8, 0x44)));
+    clientid2_ = ClientIdPtr(new ClientId(vector<uint8_t>(8, 0x56)));
+
     uint8_t mac[] = { 0, 1, 22, 33, 44, 55};
 
     // Let's use odd hardware type to check if there is no Ethernet

src/lib/dhcpsrv/tests/alloc_engine_utils.h

@@ -383,6 +383,7 @@ public:
     }
 
     ClientIdPtr clientid_;      ///< Client-identifier (value used in tests)
+    ClientIdPtr clientid2_;     ///< Alternative client-identifier.
     HWAddrPtr hwaddr_;          ///< Hardware address (value used in tests)
     HWAddrPtr hwaddr2_;         ///< Alternative hardware address.
     Subnet4Ptr subnet_;         ///< Subnet4 (used in tests)

src/lib/dhcpsrv/tests/lease_unittest.cc

@@ -1,4 +1,4 @@
-// Copyright (C) 2013-2014 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2013-2015 Internet Systems Consortium, Inc. ("ISC")
 //
 // Permission to use, copy, modify, and/or distribute this software for any
 // purpose with or without fee is hereby granted, provided that the above
@@ -16,6 +16,7 @@
 #include <asiolink/io_address.h>
 #include <dhcp/duid.h>
 #include <dhcpsrv/lease.h>
+#include <util/pointer_util.h>
 #include <gtest/gtest.h>
 #include <vector>
 #include <sstream>
@@ -57,27 +58,24 @@ Lease4 createLease4(const std::string& hostname, const bool fqdn_fwd,
 class Lease4Test : public ::testing::Test {
 public:
 
-    /// Default constructor
+    /// @brief Default constructor
     ///
     /// Currently it only initializes hardware address.
     Lease4Test() {
         hwaddr_.reset(new HWAddr(HWADDR, sizeof(HWADDR), HTYPE_ETHER));
+        clientid_.reset(new ClientId(CLIENTID, sizeof(CLIENTID)));
     }
 
     /// Hardware address, used by tests.
     HWAddrPtr hwaddr_;
+
+    /// Pointer to the client identifier used by tests.
+    ClientIdPtr clientid_;
 };
 
-/// Lease4 is also defined in lease_mgr.h, so is tested in this file as well.
-// This test checks if the Lease4 structure can be instantiated correctly
+// This test checks if the Lease4 structure can be instantiated correctly.
 TEST_F(Lease4Test, constructor) {
-
-    // Random values for the tests
-    const uint8_t CLIENTID[] = {0x17, 0x34, 0xe2, 0xff, 0x09, 0x92, 0x54};
-    std::vector<uint8_t> clientid_vec(CLIENTID, CLIENTID + sizeof(CLIENTID));
-    ClientId clientid(clientid_vec);
-
-    // ...and a time
+     // Get current time for the use in Lease.
     const time_t current_time = time(NULL);
 
     // Other random constants.
@@ -93,15 +91,14 @@ TEST_F(Lease4Test, constructor) {
     for (int i = 0; i < sizeof(ADDRESS) / sizeof(ADDRESS[0]); ++i) {
 
         // Create the lease
-        Lease4 lease(ADDRESS[i], hwaddr_,
-                     CLIENTID, sizeof(CLIENTID), VALID_LIFETIME, 0, 0,
+        Lease4 lease(ADDRESS[i], hwaddr_, clientid_, VALID_LIFETIME, 0, 0,
                      current_time, SUBNET_ID, true, true,
                      "hostname.example.com.");
 
         EXPECT_EQ(ADDRESS[i], static_cast<uint32_t>(lease.addr_));
         EXPECT_EQ(0, lease.ext_);
-        EXPECT_TRUE(hwaddr_ == lease.hwaddr_);
-        EXPECT_TRUE(clientid == *lease.client_id_);
+        EXPECT_TRUE(util::equalValues(hwaddr_, lease.hwaddr_));
+        EXPECT_TRUE(util::equalValues(clientid_, lease.client_id_));
         EXPECT_EQ(0, lease.t1_);
         EXPECT_EQ(0, lease.t2_);
         EXPECT_EQ(VALID_LIFETIME, lease.valid_lft_);
@@ -118,11 +115,7 @@ TEST_F(Lease4Test, constructor) {
 // This test verfies that copy constructor copies Lease4 fields correctly.
 TEST_F(Lease4Test, copyConstructor) {
 
-    const uint8_t CLIENTID[] = {0x17, 0x34, 0xe2, 0xff, 0x09, 0x92, 0x54};
-    std::vector<uint8_t> clientid_vec(CLIENTID, CLIENTID + sizeof(CLIENTID));
-    ClientId clientid(clientid_vec);
-
-    // ...and a time
+    // Get current time for the use in Lease4.
     const time_t current_time = time(NULL);
 
     // Other random constants.
@@ -130,8 +123,7 @@ TEST_F(Lease4Test, copyConstructor) {
     const uint32_t VALID_LIFETIME = 500;
 
     // Create the lease
-    Lease4 lease(0xffffffff, hwaddr_,
-                 CLIENTID, sizeof(CLIENTID), VALID_LIFETIME, 0, 0, current_time,
+    Lease4 lease(0xffffffff, hwaddr_, clientid_, VALID_LIFETIME, 0, 0, current_time,
                  SUBNET_ID);
 
     // Use copy constructor to copy the lease.
@@ -160,12 +152,7 @@ TEST_F(Lease4Test, copyConstructor) {
 // correctly.
 TEST_F(Lease4Test, operatorAssign) {
 
-    // Random values for the tests
-    const uint8_t CLIENTID[] = {0x17, 0x34, 0xe2, 0xff, 0x09, 0x92, 0x54};
-    std::vector<uint8_t> clientid_vec(CLIENTID, CLIENTID + sizeof(CLIENTID));
-    ClientId clientid(clientid_vec);
-
-    // ...and a time
+    // Get the current time for the use in Lease4.
     const time_t current_time = time(NULL);
 
     // Other random constants.
@@ -173,8 +160,7 @@ TEST_F(Lease4Test, operatorAssign) {
     const uint32_t VALID_LIFETIME = 500;
 
     // Create the lease
-    Lease4 lease(0xffffffff, hwaddr_,
-                 CLIENTID, sizeof(CLIENTID), VALID_LIFETIME, 0, 0, current_time,
+    Lease4 lease(0xffffffff, hwaddr_, clientid_, VALID_LIFETIME, 0, 0, current_time,
                  SUBNET_ID);
 
     // Use assignment operator to assign the lease.
@@ -199,56 +185,66 @@ TEST_F(Lease4Test, operatorAssign) {
     EXPECT_TRUE(lease == copied_lease);
 }
 
-// This test verifies that the matches() returns true if two leases differ
-// by values other than address, HW address, Client ID and ext_.
-TEST_F(Lease4Test, matches) {
-    // Create two leases which share the same address, HW address, client id
-    // and ext_ value.
-    const time_t current_time = time(NULL);
-    Lease4 lease1(IOAddress("192.0.2.3"), hwaddr_, CLIENTID,
-                  sizeof(CLIENTID), VALID_LIFETIME, current_time, 0, 0,
-                  SUBNET_ID);
-    lease1.hostname_ = "lease1.example.com.";
-    lease1.fqdn_fwd_ = true;
-    lease1.fqdn_rev_ = true;
-
-    // We need to make an explicit copy. Otherwise the second lease will just
-    // store a pointer and we'll have two leases pointing to a single HWAddr.
-    // That would make modifications to only one impossible.
-    HWAddrPtr hwcopy(new HWAddr(*hwaddr_));
-
-    Lease4 lease2(IOAddress("192.0.2.3"), hwcopy, CLIENTID,
-                  sizeof(CLIENTID), VALID_LIFETIME + 10, current_time - 10,
-                  100, 200, SUBNET_ID);
-    lease2.hostname_ = "lease2.example.com.";
-    lease2.fqdn_fwd_ = false;
-    lease2.fqdn_rev_ = true;
-
-    // Leases should match.
-    EXPECT_TRUE(lease1.matches(lease2));
-    EXPECT_TRUE(lease2.matches(lease1));
-
-    // Change address, leases should not match anymore.
-    lease1.addr_ = IOAddress("192.0.2.4");
-    EXPECT_FALSE(lease1.matches(lease2));
-    lease1.addr_ = lease2.addr_;
-
-    // Change HW address, leases should not match.
-    lease1.hwaddr_->hwaddr_[1] += 1;
-    EXPECT_FALSE(lease1.matches(lease2));
-    lease1.hwaddr_ = lease2.hwaddr_;
-
-    // Chanage client id, leases should not match.
-    std::vector<uint8_t> client_id = lease1.client_id_->getClientId();
-    client_id[1] += 1;
-    lease1.client_id_.reset(new ClientId(client_id));
-    EXPECT_FALSE(lease1.matches(lease2));
-    lease1.client_id_ = lease2.client_id_;
-
-    // Change ext_, leases should not match.
-    lease1.ext_ += 1;
-    EXPECT_FALSE(lease1.matches(lease2));
-    lease1.ext_ = lease2.ext_;
+// This test verifies that it is correctly determined when the lease
+// belongs to the particular client identified by the client identifier
+// and hw address.
+TEST_F(Lease4Test, leaseBelongsToClient) {
+    // Client identifier that matches the one in the lease.
+    ClientIdPtr matching_client_id = ClientId::fromText("01:02:03:04");
+    // Client identifier that doesn't match the one in the lease.
+    ClientIdPtr diff_client_id = ClientId::fromText("01:02:03:05");
+    // Null (no) client identifier.
+    ClientIdPtr null_client_id;
+
+    // HW Address that matches the one in the lease.
+    HWAddrPtr matching_hw(new HWAddr(HWAddr::fromText("00:01:02:03:04:05",
+                                                      HTYPE_ETHER)));
+    // HW Address that doesn't match the one in the lease.
+    HWAddrPtr diff_hw(new HWAddr(HWAddr::fromText("00:01:02:03:04:06",
+                                                  HTYPE_ETHER)));
+    // Null HW Address.
+    HWAddrPtr null_hw;
+
+    // Create the lease with MAC address and Client Identifier.
+    Lease4 lease(IOAddress("192.0.2.1"), matching_hw, matching_client_id,
+                 60, time(NULL), 0, 0, 1);
+
+    // Verify cases for lease that has both hw address and client identifier.
+    EXPECT_TRUE(lease.belongsToClient(matching_hw, matching_client_id));
+    EXPECT_FALSE(lease.belongsToClient(matching_hw, diff_client_id));
+    EXPECT_TRUE(lease.belongsToClient(matching_hw, null_client_id));
+    EXPECT_TRUE(lease.belongsToClient(diff_hw, matching_client_id));
+    EXPECT_FALSE(lease.belongsToClient(diff_hw, diff_client_id));
+    EXPECT_FALSE(lease.belongsToClient(diff_hw, null_client_id));
+    EXPECT_TRUE(lease.belongsToClient(null_hw, matching_client_id));
+    EXPECT_FALSE(lease.belongsToClient(null_hw, diff_client_id));
+    EXPECT_FALSE(lease.belongsToClient(null_hw, null_client_id));
+
+
+    // Verify cases for lease that has only HW address.
+    lease.client_id_ = null_client_id;
+    EXPECT_TRUE(lease.belongsToClient(matching_hw, matching_client_id));
+    EXPECT_TRUE(lease.belongsToClient(matching_hw, diff_client_id));
+    EXPECT_TRUE(lease.belongsToClient(matching_hw, null_client_id));
+    EXPECT_FALSE(lease.belongsToClient(diff_hw, matching_client_id));
+    EXPECT_FALSE(lease.belongsToClient(diff_hw, diff_client_id));
+    EXPECT_FALSE(lease.belongsToClient(diff_hw, null_client_id));
+    EXPECT_FALSE(lease.belongsToClient(null_hw, matching_client_id));
+    EXPECT_FALSE(lease.belongsToClient(null_hw, diff_client_id));
+    EXPECT_FALSE(lease.belongsToClient(null_hw, null_client_id));
+
+    // Verify cases for lease that has only client identifier.
+    lease.client_id_ = matching_client_id;
+    lease.hwaddr_ = null_hw;
+    EXPECT_TRUE(lease.belongsToClient(matching_hw, matching_client_id));
+    EXPECT_FALSE(lease.belongsToClient(matching_hw, diff_client_id));
+    EXPECT_FALSE(lease.belongsToClient(matching_hw, null_client_id));
+    EXPECT_TRUE(lease.belongsToClient(diff_hw, matching_client_id));
+    EXPECT_FALSE(lease.belongsToClient(diff_hw, diff_client_id));
+    EXPECT_FALSE(lease.belongsToClient(diff_hw, null_client_id));
+    EXPECT_TRUE(lease.belongsToClient(null_hw, matching_client_id));
+    EXPECT_FALSE(lease.belongsToClient(null_hw, diff_client_id));
+    EXPECT_FALSE(lease.belongsToClient(null_hw, null_client_id));
 }
 
 /// @brief Lease4 Equality Test
@@ -260,28 +256,22 @@ TEST_F(Lease4Test, operatorEquals) {
 
     // Random values for the tests
     const uint32_t ADDRESS = 0x01020304;
-    const uint8_t HWADDR[] = {0x08, 0x00, 0x2b, 0x02, 0x3f, 0x4e};
-    std::vector<uint8_t> hwaddr(HWADDR, HWADDR + sizeof(HWADDR));
-    const uint8_t CLIENTID[] = {0x17, 0x34, 0xe2, 0xff, 0x09, 0x92, 0x54};
-    std::vector<uint8_t> clientid_vec(CLIENTID, CLIENTID + sizeof(CLIENTID));
-    ClientId clientid(clientid_vec);
     const time_t current_time = time(NULL);
     const uint32_t SUBNET_ID = 42;
     const uint32_t VALID_LIFETIME = 500;
 
     // Check when the leases are equal.
-    Lease4 lease1(ADDRESS, hwaddr_,
-                  CLIENTID, sizeof(CLIENTID), VALID_LIFETIME, current_time, 0,
+    Lease4 lease1(ADDRESS, hwaddr_, clientid_, VALID_LIFETIME, current_time, 0,
                   0, SUBNET_ID);
 
     // We need to make an explicit copy. Otherwise the second lease will just
-    // store a pointer and we'll have two leases pointing to a single HWAddr.
-    // That would make modifications to only one impossible.
+    // store a pointer and we'll have two leases pointing to a single HWAddr
+    // or client. That would make modifications to only one impossible.
     HWAddrPtr hwcopy(new HWAddr(*hwaddr_));
+    ClientIdPtr clientid_copy(new ClientId(*clientid_));
 
-    Lease4 lease2(ADDRESS, hwcopy,
-                  CLIENTID, sizeof(CLIENTID), VALID_LIFETIME, current_time, 0, 0,
-                  SUBNET_ID);
+    Lease4 lease2(ADDRESS, hwcopy, clientid_copy, VALID_LIFETIME, current_time,
+                  0, 0, SUBNET_ID);
     EXPECT_TRUE(lease1 == lease2);
     EXPECT_FALSE(lease1 != lease2);
 
@@ -308,6 +298,7 @@ TEST_F(Lease4Test, operatorEquals) {
     EXPECT_TRUE(lease1 == lease2);  // Check that the reversion has made the
     EXPECT_FALSE(lease1 != lease2); // ... leases equal
 
+    std::vector<uint8_t> clientid_vec = clientid_->getClientId();
     ++clientid_vec[0];
     lease1.client_id_.reset(new ClientId(clientid_vec));
     EXPECT_FALSE(lease1 == lease2);
@@ -390,7 +381,7 @@ TEST_F(Lease4Test, operatorEquals) {
 
 // Verify that the client id can be returned as a vector object and if client
 // id is NULL the empty vector is returned.
-TEST(Lease4, getClientIdVector) {
+TEST_F(Lease4Test, getClientIdVector) {
     // Create a lease.
     Lease4 lease;
     // By default, the lease should have client id set to NULL. If it doesn't,
@@ -398,18 +389,17 @@ TEST(Lease4, getClientIdVector) {
     ASSERT_FALSE(lease.client_id_);
     // When client id is NULL the vector returned should be empty.
     EXPECT_TRUE(lease.getClientIdVector().empty());
-    // Now, let's set the non NULL client id. Fill it with the 8 bytes, each
-    // holding a value of 0x42.
-    std::vector<uint8_t> client_id_vec(8, 0x42);
-    lease.client_id_ = ClientIdPtr(new ClientId(client_id_vec));
+
+    // Initialize client identifier to non-null value.
+    lease.client_id_ = clientid_;
     // Check that the returned vector, encapsulating client id is equal to
     // the one that has been used to set the client id for the lease.
     std::vector<uint8_t> returned_vec = lease.getClientIdVector();
-    EXPECT_TRUE(returned_vec == client_id_vec);
+    EXPECT_TRUE(returned_vec == clientid_->getClientId());
 }
 
 // Verify the behavior of the function which checks FQDN data for equality.
-TEST(Lease4, hasIdenticalFqdn) {
+TEST_F(Lease4Test, hasIdenticalFqdn) {
     Lease4 lease = createLease4("myhost.example.com.", true, true);
     EXPECT_TRUE(lease.hasIdenticalFqdn(createLease4("myhost.example.com.",
                                                      true, true)));
@@ -429,8 +419,8 @@ TEST(Lease4, hasIdenticalFqdn) {
 TEST_F(Lease4Test, toText) {
 
     const time_t current_time = 12345678;
-    Lease4 lease(IOAddress("192.0.2.3"), hwaddr_, CLIENTID, sizeof(CLIENTID),
-                 3600, 123, 456, current_time, 789);
+    Lease4 lease(IOAddress("192.0.2.3"), hwaddr_, clientid_, 3600, 123,
+                 456, current_time, 789);
     
     std::stringstream expected;
     expected << "Address:       192.0.2.3\n"
@@ -439,12 +429,14 @@ TEST_F(Lease4Test, toText) {
              << "T2:            456\n"
              << "Cltt:          12345678\n"
              << "Hardware addr: " << hwaddr_->toText(false) << "\n"
+             << "Client id:     " << clientid_->toText() << "\n"
              << "Subnet ID:     789\n";
 
     EXPECT_EQ(expected.str(), lease.toText());
 
-    // Now let's try with a lease without hardware address.
+    // Now let's try with a lease without hardware address and client identifier.
     lease.hwaddr_.reset();
+    lease.client_id_.reset();
     expected.str("");
     expected << "Address:       192.0.2.3\n"
              << "Valid life:    3600\n"
@@ -452,6 +444,7 @@ TEST_F(Lease4Test, toText) {
              << "T2:            456\n"
              << "Cltt:          12345678\n"
              << "Hardware addr: (none)\n"
+             << "Client id:     (none)\n"
              << "Subnet ID:     789\n";
     EXPECT_EQ(expected.str(), lease.toText());
 }
@@ -565,88 +558,6 @@ TEST(Lease6, Lease6ConstructorWithFQDN) {
                                          subnet_id)), InvalidOperation);
 }
 
-// This test verifies that the matches() function returns true if two leases
-// differ by values other than address, type, prefix length, IAID and DUID.
-TEST(Lease6, matches) {
-
-    // Create two matching leases.
-    uint8_t llt[] = {0, 1, 2, 3, 4, 5, 6, 0xa, 0xb, 0xc, 0xd, 0xe, 0xf};
-    DuidPtr duid(new DUID(llt, sizeof(llt)));
-
-    Lease6 lease1(Lease6::TYPE_NA, IOAddress("2001:db8:1::1"), duid,
-                  IAID, 100, 200, 50, 80,
-                  SUBNET_ID);
-    lease1.hostname_ = "lease1.example.com.";
-    lease1.fqdn_fwd_ = true;
-    lease1.fqdn_rev_ = true;
-    Lease6 lease2(Lease6::TYPE_NA, IOAddress("2001:db8:1::1"), duid,
-                  IAID, 200, 300, 90, 70,
-                  SUBNET_ID);
-    lease2.hostname_ = "lease1.example.com.";
-    lease2.fqdn_fwd_ = false;
-    lease2.fqdn_rev_ = true;
-
-    EXPECT_TRUE(lease1.matches(lease2));
-
-    // Modify each value used to match both leases, and make sure that
-    // leases don't match.
-
-    // Modify address.
-    lease1.addr_ = IOAddress("2001:db8:1::2");
-    EXPECT_FALSE(lease1.matches(lease2));
-    lease1.addr_ = lease2.addr_;
-
-    // Modify lease type.
-    lease1.type_ = Lease6::TYPE_TA;
-    EXPECT_FALSE(lease1.matches(lease2));
-    lease1.type_ = lease2.type_;
-
-    // Modify prefix length.
-    lease1.prefixlen_ += 1;
-    EXPECT_FALSE(lease1.matches(lease2));
-    lease1.prefixlen_ = lease2.prefixlen_;
-
-    // Modify IAID.
-    lease1.iaid_ += 1;
-    EXPECT_FALSE(lease1.matches(lease2));
-    lease1.iaid_ = lease2.iaid_;
-
-    // Modify DUID.
-    llt[1] += 1;
-    duid.reset(new DUID(llt, sizeof(llt)));
-    lease1.duid_ = duid;
-    EXPECT_FALSE(lease1.matches(lease2));
-    lease1.duid_ = lease2.duid_;
-
-    // Hardware address checks
-    EXPECT_TRUE(lease1.matches(lease2)); // Neither lease have hardware address.
-
-    // Let's add a hardware lease to the first one.
-    HWAddrPtr hwaddr(new HWAddr(HWADDR, sizeof(HWADDR), HTYPE_ETHER));
-    lease1.hwaddr_ = hwaddr;
-
-    // Only the first one has a hardware address, so not equal.
-    EXPECT_FALSE(lease1.matches(lease2));
-
-    // Only the second one has it, so still not equal.
-    lease1.hwaddr_.reset();
-    lease2.hwaddr_ = hwaddr;
-    EXPECT_FALSE(lease1.matches(lease2));
-
-    // Ok, now both have it - they should be equal.
-    lease1.hwaddr_ = hwaddr;
-    EXPECT_TRUE(lease1.matches(lease2));
-
-    // Let's create a second instance that have the same values.
-    HWAddrPtr hwaddr2(new HWAddr(HWADDR, sizeof(HWADDR), HTYPE_ETHER));
-    lease2.hwaddr_ = hwaddr2;
-    EXPECT_TRUE(lease1.matches(lease2));
-
-    // Let's modify the second address and check that they won't be equal anymore.
-    hwaddr2->hwaddr_[0]++;
-    EXPECT_FALSE(lease1.matches(lease2));
-}
-
 /// @brief Lease6 Equality Test
 ///
 /// Checks that the operator==() correctly compares two leases for equality.

src/lib/dhcpsrv/tests/subnet_unittest.cc

@@ -116,6 +116,22 @@ TEST(Subnet4Test, siaddr) {
         BadValue);
 }
 
+// Checks if the match-client-id flag can be set and retrieved.
+TEST(Subnet4Test, matchClientId) {
+    Subnet4 subnet(IOAddress("192.0.2.1"), 24, 1000, 2000, 3000);
+
+    // By default the flag should be set to true.
+    EXPECT_TRUE(subnet.getMatchClientId());
+
+    // Modify it and retrieve.
+    subnet.setMatchClientId(false);
+    EXPECT_FALSE(subnet.getMatchClientId());
+
+    // Modify again.
+    subnet.setMatchClientId(true);
+    EXPECT_TRUE(subnet.getMatchClientId());
+}
+
 TEST(Subnet4Test, Pool4InSubnet4) {
 
     Subnet4Ptr subnet(new Subnet4(IOAddress("192.1.2.0"), 24, 1, 2, 3));

src/lib/util/Makefile.am

@@ -20,6 +20,7 @@ libkea_util_la_SOURCES += memory_segment.h
 libkea_util_la_SOURCES += memory_segment_local.h memory_segment_local.cc
 libkea_util_la_SOURCES += optional_value.h
 libkea_util_la_SOURCES += pid_file.h pid_file.cc
+libkea_util_la_SOURCES += pointer_util.h
 libkea_util_la_SOURCES += process_spawn.h process_spawn.cc
 libkea_util_la_SOURCES += range_utilities.h
 libkea_util_la_SOURCES += signal_set.cc signal_set.h

src/lib/util/optional_value.h

@@ -100,11 +100,7 @@ public:
     /// @param value New actual value.
     void specify(const T& value) {
         set(value);
-        specify(true);
-    }
-
-    void specify(const OptionalValueState& state) {
-        specified_ = state.specified_;
+        specify(OptionalValueState(true));
     }
 
     /// @brief Sets the value to "specified" or "unspecified".
@@ -112,8 +108,8 @@ public:
     /// It does not alter the actual value. It only marks it "specified" or
     /// "unspecified".
     /// @param specified boolean that determined if a value is specified or not
-    void specify(const bool specified) {
-        specified_ = specified;
+    void specify(const OptionalValueState& state) {
+        specified_ = state.specified_;
     }
 
     /// @brief Checks if the value is specified or unspecified.

src/lib/util/pointer_util.h

@@ -0,0 +1,57 @@
+// Copyright (C) 2015  Internet Systems Consortium, Inc. ("ISC")
+//
+// Permission to use, copy, modify, and/or distribute this software for any
+// purpose with or without fee is hereby granted, provided that the above
+// copyright notice and this permission notice appear in all copies.
+//
+// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH
+// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
+// AND FITNESS.  IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT,
+// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
+// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE
+// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
+// PERFORMANCE OF THIS SOFTWARE.
+
+#ifndef POINTER_UTIL_H
+#define POINTER_UTIL_H
+
+namespace isc {
+namespace util {
+
+/// @brief This function checks if two pointers are non-null and values
+/// are equal.
+///
+/// This function performs the typical comparison of values encapsulated by
+/// the smart pointers, with checking if the pointers are non-null.
+///
+/// @param ptr1 First pointer.
+/// @param ptr2 Second pointer.
+///
+/// @tparam T Pointer type, e.g. boost::shared_ptr, or boost::scoped_ptr.
+///
+/// @return true only if both pointers are non-null and the values which they
+/// point to are equal.
+template<typename T>
+inline bool equalValues(const T& ptr1, const T& ptr2) {
+    return (ptr1 && ptr2 && (*ptr1 == *ptr2));
+}
+
+/// @brief This function checks if two pointers are both null or both
+/// are non-null and they point to equal values.
+///
+/// @param ptr1 First pointer.
+/// @param ptr2 Second pointer.
+///
+/// @tparam T Pointer type, e.g. boost::shared_ptr, or boost::scoped_ptr.
+///
+/// @return true if both pointers are null or if they are both non-null
+/// and the values pointed to are equal.
+template<typename T>
+inline bool nullOrEqualValues(const T& ptr1, const T& ptr2) {
+    return ((!ptr1 && !ptr2) || equalValues(ptr1, ptr2));
+}
+
+} // end of namespace isc::util
+} // end of namespace isc
+
+#endif

src/lib/util/tests/optional_value_unittest.cc

@@ -47,7 +47,7 @@ TEST(OptionalValueTest, set) {
     ASSERT_FALSE(value.isSpecified());
 
     // Mark value "specified". The value itself should not change.
-    value.specify(true);
+    value.specify(OptionalValueState(true));
     ASSERT_EQ(100, value.get());
     ASSERT_TRUE(value.isSpecified());
 
@@ -57,7 +57,7 @@ TEST(OptionalValueTest, set) {
     ASSERT_TRUE(value.isSpecified());
 
     // Mark it "unspecified". The value should remain the same.
-    value.specify(false);
+    value.specify(OptionalValueState(false));
     ASSERT_EQ(5, value.get());
     ASSERT_FALSE(value.isSpecified());
 }
@@ -110,7 +110,7 @@ TEST(OptionalValueTest, equalityOperators) {
     EXPECT_FALSE(value == 123);
     EXPECT_TRUE(value != 123);
 
-    value.specify(true);
+    value.specify(OptionalValueState(true));
 
     EXPECT_TRUE(value == 10);
     EXPECT_FALSE(value != 10);
@@ -123,7 +123,7 @@ TEST(OptionalValueTest, equalityOperators) {
     EXPECT_FALSE(value == 10);
     EXPECT_TRUE(value != 10);
 
-    value.specify(false);
+    value.specify(OptionalValueState(false));
 
     EXPECT_FALSE(value == 123);
     EXPECT_TRUE(value != 123);