[3320] Covered the case where the client is in INIT-REBOOT state.

src/bin/dhcp4/dhcp4_messages.mes

@@ -147,6 +147,11 @@ point, the setting of the flag instructs the server not to choose a
 subnet, an action that severely limits further processing; the server
 will be only able to offer global options - no addresses will be assigned.
 
+% DHCP4_INVALID_ADDRESS_INIT_REBOOT client having client-id %1, hwaddr %2 and being in the INIT-REBOOT state requested invalid address %3
+This debug message is issued when the client being in the INIT-REBOOT state
+requested an address which is not assigned to him. The server will respond
+to this client with DHCPNAK.
+
 % DHCP4_LEASE_ADVERT lease %1 advertised (client client-id %2, hwaddr %3)
 This debug message indicates that the server successfully advertised
 a lease. It is up to the client to choose one server out of othe advertised

src/bin/dhcp4/dhcp4_srv.cc

@@ -940,8 +940,14 @@ Dhcpv4Srv::assignLease(const Pkt4Ptr& question, Pkt4Ptr& answer) {
     if (opt) {
         client_id = ClientIdPtr(new ClientId(opt->getData()));
     }
+
     // client-id is not mandatory in DHCPv4
 
+    // Get the server identifier. It will be used to determine the state
+    // of the client.
+    OptionCustomPtr opt_serverid = boost::dynamic_pointer_cast<
+        OptionCustom>(question->getOption(DHO_DHCP_SERVER_IDENTIFIER));
+
     // Try to get the Requested IP Address option and use the address as a hint
     // for the allocation engine. If the server doesn't already have a lease
     // for this client it will try to allocate the one requested.
@@ -962,6 +968,25 @@ Dhcpv4Srv::assignLease(const Pkt4Ptr& question, Pkt4Ptr& answer) {
     // allocation.
     bool fake_allocation = (question->getType() == DHCPDISCOVER);
 
+    // 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 has to be verified.
+    if (!fake_allocation && !opt_serverid && opt_requested_address) {
+        Lease4Ptr lease = LeaseMgrFactory::instance().getLease4(hint);
+        if (!lease) {
+            LOG_DEBUG(dhcp4_logger, DBG_DHCP4_DETAIL,
+                      DHCP4_INVALID_ADDRESS_INIT_REBOOT)
+                .arg(client_id ? client_id->toText():"(no client-id)")
+                .arg(hwaddr ? hwaddr->toText():"(no hwaddr info)")
+                .arg(hint.toText());
+
+            answer->setType(DHCPNAK);
+            answer->setYiaddr(IOAddress("0.0.0.0"));
+            return;
+        }
+    }
+
+
     CalloutHandlePtr callout_handle = getCalloutHandle(question);
 
     std::string hostname;
@@ -1001,8 +1026,8 @@ Dhcpv4Srv::assignLease(const Pkt4Ptr& question, Pkt4Ptr& answer) {
     /// @todo pass the actual FQDN data.
     Lease4Ptr old_lease;
     Lease4Ptr lease = alloc_engine_->allocateLease4(subnet, client_id, hwaddr,
-                                                      hint, fqdn_fwd, fqdn_rev,
-                                                      hostname,
+                                                    hint, fqdn_fwd, fqdn_rev,
+                                                    hostname,
                                                     fake_allocation,
                                                     callout_handle,
                                                     old_lease);

src/bin/dhcp4/tests/dhcp4_client.cc

@@ -71,6 +71,15 @@ Dhcp4Client::Dhcp4Client(boost::shared_ptr<NakedDhcpv4Srv>& srv,
 }
 
 void
+Dhcp4Client::addRequestedAddress(const asiolink::IOAddress& addr) {
+    if (context_.query_) {
+        Option4AddrLstPtr opt(new Option4AddrLst(DHO_DHCP_REQUESTED_ADDRESS,
+                                                 addr));
+        context_.query_->addOption(opt);
+    }
+}
+
+void
 Dhcp4Client::applyConfiguration() {
     Pkt4Ptr resp = context_.response_;
     if (!resp) {
@@ -140,10 +149,7 @@ Dhcp4Client::doDiscover(const boost::shared_ptr<IOAddress>& requested_addr) {
     // Request options if any.
     includePRL();
     if (requested_addr) {
-        Option4AddrLstPtr
-            opt_requested_addr(new Option4AddrLst(DHO_DHCP_REQUESTED_ADDRESS,
-                                                  IOAddress(*requested_addr)));
-        context_.query_->addOption(opt_requested_addr);
+        addRequestedAddress(*requested_addr);
     }
     // Send the message to the server.
     sendMsg(context_.query_);
@@ -155,11 +161,10 @@ void
 Dhcp4Client::doDORA(const boost::shared_ptr<IOAddress>& requested_addr) {
     doDiscover(requested_addr);
     if (context_.response_ && (context_.response_->getType() == DHCPOFFER)) {
-        doRequest(requested_addr);
+        doRequest();
     }
 }
 
-
 void
 Dhcp4Client::doInform(const bool set_ciaddr) {
     context_.query_ = createMsg(DHCPINFORM);
@@ -187,7 +192,7 @@ Dhcp4Client::doInform(const bool set_ciaddr) {
 }
 
 void
-Dhcp4Client::doRequest(const boost::shared_ptr<IOAddress>& requested_addr) {
+Dhcp4Client::doRequest() {
     context_.query_ = createMsg(DHCPREQUEST);
 
     // Set ciaddr.
@@ -198,18 +203,17 @@ Dhcp4Client::doRequest(const boost::shared_ptr<IOAddress>& requested_addr) {
     }
 
     // Requested IP address.
-    if ((state_ == SELECTING) || (state_ == INIT_REBOOT)) {
+    if (state_ == SELECTING) {
         if (context_.response_ &&
             (context_.response_->getType() == DHCPOFFER) &&
             (context_.response_->getYiaddr() != IOAddress("0.0.0.0"))) {
-            Option4AddrLstPtr
-                opt_requested_addr(new Option4AddrLst(DHO_DHCP_REQUESTED_ADDRESS,
-                                                      IOAddress(context_.response_->getYiaddr())));
-            context_.query_->addOption(opt_requested_addr);
+            addRequestedAddress(context_.response_->getYiaddr());
         } else {
             isc_throw(Dhcp4ClientError, "error sending the DHCPREQUEST because"
                       " the received DHCPOFFER message was invalid");
         }
+    } else if (state_ == INIT_REBOOT) {
+        addRequestedAddress(config_.lease_.addr_);
     }
 
     // Server identifier.

src/bin/dhcp4/tests/dhcp4_client.h

@@ -122,6 +122,13 @@ public:
 
     /// @brief Sends DHCPDISCOVER message to the server and receives response.
     ///
+    /// The message being sent to the server includes Parameter Request List
+    /// option if any options to be requested have been specified using the
+    /// @c requestOptions or @c requestOption methods.
+    ///
+    /// The configuration returned by the server in the DHCPOFFER message is
+    /// NOT stored in the client configuration: @c config_.
+    ///
     /// @param requested_addr A pointer to the IP Address to be sent in the
     /// Requested IP Address option or NULL if the option should not be
     /// included.
@@ -130,6 +137,9 @@ public:
 
     /// @brief Perform 4-way exchange with a server.
     ///
+    /// This method calls @c doDiscover and @c doRequest to perform the 4-way
+    /// exchange with the server.
+    ///
     /// @param requested_addr A pointer to the address to be requested using the
     /// Requested IP Address option.
     void doDORA(const boost::shared_ptr<asiolink::IOAddress>&
@@ -157,11 +167,29 @@ public:
 
     /// @brief Sends DHCPREQUEST Message to the server and receives a response.
     ///
-    /// @param requested_addr A pointer to the IP Address to be sent in the
-    /// Requested IP Address option or NULL if the option should not be
-    /// included.
-    void doRequest(const boost::shared_ptr<asiolink::IOAddress>&
-                   requested_addr = boost::shared_ptr<asiolink::IOAddress>());
+    /// This method simulates sending the DHCPREQUEST message to the server and
+    /// receiving a response. The DHCPREQUEST message can be used by the client
+    /// being in various states:
+    /// - SELECTING - client is trying to obtain a new lease and it has selected
+    /// the server using the DHCPDISCOVER.
+    /// - INIT-REBOOT - client cached an address it was previously using and is
+    /// now trying to verify if this addres is still valid.
+    /// - RENEW - client's renewal timer has passed and the client is trying to
+    /// extend the lifetime of the lease.
+    /// - REBIND - client's rebind timer has passed and the client is trying to
+    /// extend the lifetime of the lease from any server.
+    ///
+    /// Depending on the state that the client is in, different combinations of
+    /// - ciaddr
+    /// - Requested IP Address option
+    /// - server identifier
+    /// are used (as per RFC2131, section 4.3.2). Therefore, the unit tests
+    /// must setthe appropriate state of the client prior to calling this
+    /// method using the @c setState function.
+    ///
+    /// When the server returns the DHCPACK the configuration carried in the
+    /// DHCPACK message is applied and can be obtained from the @c config_.
+    void doRequest();
 
     /// @brief Generates a hardware address used by the client.
     ///
@@ -268,6 +296,12 @@ public:
 
 private:
 
+    /// @brief Creates and addds Requested IP Address option to the client's
+    /// query.
+    ///
+    /// @param addr Address to be added in the Requested IP Address option.
+    void addRequestedAddress(const asiolink::IOAddress& addr);
+
     /// @brief Stores configuration received from the server.
     ///
     /// This methods stores the configuration obtained from the DHCP server

src/bin/dhcp4/tests/dhcp4_srv_unittest.cc

@@ -596,92 +596,6 @@ TEST_F(Dhcpv4SrvTest, DiscoverNoTimers) {
 //
 // constructed very simple DISCOVER message with:
 // - client-id option
-// - address set to specific value as hint
-//
-// expected returned OFFER message:
-// - copy of client-id
-// - server-id
-// - offered address
-TEST_F(Dhcpv4SrvTest, DiscoverHint) {
-    IfaceMgrTestConfig test_config(true);
-    IfaceMgr::instance().openSockets4();
-
-    boost::scoped_ptr<NakedDhcpv4Srv> srv;
-    ASSERT_NO_THROW(srv.reset(new NakedDhcpv4Srv(0)));
-    IOAddress hint("192.0.2.107");
-
-    Pkt4Ptr dis = Pkt4Ptr(new Pkt4(DHCPDISCOVER, 1234));
-    dis->setRemoteAddr(IOAddress("192.0.2.1"));
-    OptionPtr clientid = generateClientId();
-    dis->addOption(clientid);
-    dis->setYiaddr(hint);
-    dis->setIface("eth1");
-
-    // Pass it to the server and get an offer
-    Pkt4Ptr offer = srv->processDiscover(dis);
-
-    // Check if we get response at all
-    checkResponse(offer, DHCPOFFER, 1234);
-
-    // Check that address was returned from proper range, that its lease
-    // lifetime is correct, that T1 and T2 are returned properly
-    checkAddressParams(offer, subnet_, true, true);
-
-    EXPECT_EQ(offer->getYiaddr(), hint);
-
-    // Check identifiers
-    checkServerId(offer, srv->getServerID());
-    checkClientId(offer, clientid);
-}
-
-
-// This test verifies that incoming DISCOVER can be handled properly, that an
-// OFFER is generated, that the response has an address and that address
-// really belongs to the configured pool.
-//
-// constructed very simple DISCOVER message with:
-// - address set to specific value as hint
-//
-// expected returned OFFER message:
-// - copy of client-id
-// - server-id
-// - offered address
-TEST_F(Dhcpv4SrvTest, DiscoverNoClientId) {
-    IfaceMgrTestConfig test_config(true);
-    IfaceMgr::instance().openSockets4();
-
-    boost::scoped_ptr<NakedDhcpv4Srv> srv;
-    ASSERT_NO_THROW(srv.reset(new NakedDhcpv4Srv(0)));
-    IOAddress hint("192.0.2.107");
-
-    Pkt4Ptr dis = Pkt4Ptr(new Pkt4(DHCPDISCOVER, 1234));
-    dis->setRemoteAddr(IOAddress("192.0.2.1"));
-    dis->setYiaddr(hint);
-    dis->setHWAddr(generateHWAddr(6));
-    dis->setIface("eth1");
-
-    // Pass it to the server and get an offer
-    Pkt4Ptr offer = srv->processDiscover(dis);
-
-    // Check if we get response at all
-    checkResponse(offer, DHCPOFFER, 1234);
-
-    // Check that address was returned from proper range, that its lease
-    // lifetime is correct, that T1 and T2 are returned properly
-    checkAddressParams(offer, subnet_, true, true);
-
-    EXPECT_EQ(offer->getYiaddr(), hint);
-
-    // Check identifiers
-    checkServerId(offer, srv->getServerID());
-}
-
-// This test verifies that incoming DISCOVER can be handled properly, that an
-// OFFER is generated, that the response has an address and that address
-// really belongs to the configured pool.
-//
-// constructed very simple DISCOVER message with:
-// - client-id option
 // - address set to specific value as hint, but that hint is invalid
 //
 // expected returned OFFER message:
@@ -825,58 +739,6 @@ TEST_F(Dhcpv4SrvTest, discoverEchoClientId) {
     checkClientId(offer, clientid);
 }
 
-// This test verifies that incoming REQUEST can be handled properly, that an
-// ACK is generated, that the response has an address and that address
-// really belongs to the configured pool.
-//
-// constructed a single REQUEST message with:
-// - client-id option
-// - hwaddr information
-// - requested address (that the client received in DISCOVER/OFFER exchange)
-//
-// expected returned ACK message:
-// - copy of client-id
-// - server-id
-// - assigned address
-//
-// Test verifies that the lease is actually in the database.
-TEST_F(Dhcpv4SrvTest, RequestBasic) {
-    IfaceMgrTestConfig test_config(true);
-    IfaceMgr::instance().openSockets4();
-
-    boost::scoped_ptr<NakedDhcpv4Srv> srv;
-    ASSERT_NO_THROW(srv.reset(new NakedDhcpv4Srv(0)));
-
-    IOAddress hint("192.0.2.107");
-    Pkt4Ptr req = Pkt4Ptr(new Pkt4(DHCPREQUEST, 1234));
-    req->setRemoteAddr(IOAddress("192.0.2.1"));
-    OptionPtr clientid = generateClientId();
-    req->addOption(clientid);
-    req->setYiaddr(hint);
-    req->setIface("eth1");
-
-    // Pass it to the server and get an advertise
-    Pkt4Ptr ack = srv->processRequest(req);
-
-    // Check if we get response at all
-    checkResponse(ack, DHCPACK, 1234);
-    EXPECT_EQ(hint, ack->getYiaddr());
-
-    // Check that address was returned from proper range, that its lease
-    // lifetime is correct, that T1 and T2 are returned properly
-    checkAddressParams(ack, subnet_, true, true);
-
-    // Check identifiers
-    checkServerId(ack, srv->getServerID());
-    checkClientId(ack, clientid);
-
-    // Check that the lease is really in the database
-    Lease4Ptr l = checkLease(ack, clientid, req->getHWAddr(), hint);
-
-    ASSERT_TRUE(l);
-    LeaseMgrFactory::instance().deleteLease(l->addr_);
-}
-
 // Check that option 58 and 59 are not included if they are not specified.
 TEST_F(Dhcpv4SrvTest, RequestNoTimers) {
     IfaceMgrTestConfig test_config(true);

src/bin/dhcp4/tests/dora_unittest.cc

@@ -140,16 +140,86 @@ public:
 
 };
 
+/// This test verifies that the client in the SELECTING state can get
+/// an address when it doesn't request any specific address in the
+/// DHCPDISCOVER message.
+TEST_F(DORATest, selectingDoNotRequestAddress) {
+    Dhcp4Client client(Dhcp4Client::SELECTING);
+    // Configure DHCP server.
+    configure(DORA_CONFIGS[0], *client.getServer());
+
+    // 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");
+}
+
 // This test verifies that the client in a SELECTING state can request
 // a specific address and that this address will be assigned when
-// available.
-TEST_F(DORATest, selectingRequestAvailableAddress) {
+// available. It also tests that if the client requests an address which
+// is in use the client will get a different address.
+TEST_F(DORATest, selectingRequestAddress) {
+    Dhcp4Client client(Dhcp4Client::SELECTING);
+    // Configure DHCP server.
+    configure(DORA_CONFIGS[0], *client.getServer());
+
+    // Perform 4-way exchange with the server.
+    ASSERT_NO_THROW(client.doDORA(boost::shared_ptr<
+                                  IOAddress>(new IOAddress("10.0.0.50"))));
+
+    // 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_EQ("10.0.0.50", client.config_.lease_.addr_.toText());
+
+    // Simulate different client requesting the same address.
+    client.modifyHWAddr();
+    ASSERT_NO_THROW(client.doDORA(boost::shared_ptr<
+                                  IOAddress>(new IOAddress("10.0.0.50"))));
+    resp = client.getContext().response_;
+    // Make sure that the server responded.
+    ASSERT_TRUE(resp);
+    // 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 some address.
+    EXPECT_NE(client.config_.lease_.addr_.toText(), "0.0.0.0");
+    // Make sure that the client has got a different address than requested
+    // as the requested one is already in use.
+    EXPECT_NE(client.config_.lease_.addr_.toText(), "10.0.0.50");
+}
+
+// This test verifies that the client will get the address that it has
+// been allocated when the client requests a different address.
+TEST_F(DORATest, selectingRequestNonMatchingAddress) {
     Dhcp4Client client(Dhcp4Client::SELECTING);
     // Configure DHCP server.
     configure(DORA_CONFIGS[0], *client.getServer());
 
     // Perform 4-way exchange with the server.
-    ASSERT_NO_THROW(client.doDORA(boost::shared_ptr<IOAddress>(new IOAddress("10.0.0.50"))));
+    ASSERT_NO_THROW(client.doDORA(boost::shared_ptr<
+                                  IOAddress>(new IOAddress("10.0.0.50"))));
 
     // Make sure that the server responded.
     ASSERT_TRUE(client.getContext().response_);
@@ -161,7 +231,76 @@ TEST_F(DORATest, selectingRequestAvailableAddress) {
     // 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_EQ("10.0.0.50", client.config_.lease_.addr_.toText());
+
+    // Let's request a different address. The server should respond with
+    // the one that the client already has allocated.
+    ASSERT_NO_THROW(client.doDORA(boost::shared_ptr<
+                                  IOAddress>(new IOAddress("10.0.0.80"))));
+    // 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()));
+    // 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 address that
+    // the client has recorded in the lease database.
     EXPECT_EQ("10.0.0.50", client.config_.lease_.addr_.toText());
 }
 
+// Test that the client in the INIT-REBOOT state can request the IP
+// address it has and the address is returned. Also, check that if
+// if the client requests in valid address the server sends a DHCPNAK.
+TEST_F(DORATest, InitRebootRequest) {
+    Dhcp4Client client(Dhcp4Client::SELECTING);
+    // Configure DHCP server.
+    configure(DORA_CONFIGS[0], *client.getServer());
+    // 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"))));
+    // 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_EQ("10.0.0.50", client.config_.lease_.addr_.toText());
+
+    // Client has a lease in the database. Let's transition the client
+    // to the INIT_REBOOT state so as the client can request the cached
+    // lease using the DHCPREQUEST message.
+    client.setState(Dhcp4Client::INIT_REBOOT);
+    ASSERT_NO_THROW(client.doRequest(boost::shared_ptr<
+                                     IOAddress>(new IOAddress("10.0.0.50"))));
+
+    // 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()));
+    // 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_EQ("10.0.0.50", client.config_.lease_.addr_.toText());
+
+    // Try to request a different address than the client has. The server
+    // should respond with DHCPNAK.
+    client.config_.lease_.addr_ = IOAddress("10.0.0.30");
+    ASSERT_NO_THROW(client.doRequest(boost::shared_ptr<
+                                     IOAddress>(new IOAddress("10.0.0.50"))));
+    // Make sure that the server responded.
+    ASSERT_TRUE(client.getContext().response_);
+    resp = client.getContext().response_;
+    EXPECT_EQ(DHCPNAK, static_cast<int>(resp->getType()));
+}
+
 } // end of anonymous namespace