[master] handle unknown client in the INIT-REBOOT state. #3656

ChangeLog

+879.	[bug]		fdupont
+	Drop DHCPREQUEST message from an unknown client in the
+	INIT-REBOOT state.
+	(Trac #3656, git xxx)
+
 878.	[func]		marcin
 	DHCPv4 and DHCPv6 server now support the lfc-interval
 	parameter which configures the interval in which the

src/bin/dhcp4/dhcp4_messages.mes

@@ -158,6 +158,10 @@ 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_NO_LEASE_INIT_REBOOT no lease for address %1 requested by INIT-REBOOT client (id: %2, hwaddr: %3)
+This debug message is issued when the client being in the INIT-REBOOT state
+requested an address but this client is unknown. The server will not respond.
+
 % DHCP4_INVALID_RELAY_INFO malformed packet received from client-id %1, hwaddr %2: %3
 This message is logged when the client sends invalid combination of
 values in the giaddr and hops fields. If the packet is relayed it should

src/bin/dhcp4/dhcp4_srv.cc

@@ -1002,10 +1002,20 @@ Dhcpv4Srv::assignLease(const Pkt4Ptr& question, Pkt4Ptr& answer) {
 
     // 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.
+    // determine whether the client's notion of the address is correct
+    // and whether the client is known, i.e., has a lease.
     if (!fake_allocation && !opt_serverid && opt_requested_address) {
-        Lease4Ptr lease = LeaseMgrFactory::instance().getLease4(hint);
-        if (!lease) {
+        Lease4Ptr lease;
+        if (hwaddr) {
+            lease = LeaseMgrFactory::instance().getLease4(*hwaddr,
+                                                          subnet->getID());
+        }
+        if (!lease && client_id) {
+            lease = LeaseMgrFactory::instance().getLease4(*client_id,
+                                                          subnet->getID());
+        }
+        // Got a lease so we can check the address.
+        if (lease && (lease->addr_ != hint)) {
             LOG_DEBUG(dhcp4_logger, DBG_DHCP4_DETAIL,
                       DHCP4_INVALID_ADDRESS_INIT_REBOOT)
                 .arg(hint.toText())
@@ -1016,6 +1026,17 @@ Dhcpv4Srv::assignLease(const Pkt4Ptr& question, Pkt4Ptr& answer) {
             answer->setYiaddr(IOAddress("0.0.0.0"));
             return;
         }
+        // Now check the second error case: unknown client.
+        if (!lease) {
+            LOG_DEBUG(dhcp4_logger, DBG_DHCP4_DETAIL,
+                      DHCP4_NO_LEASE_INIT_REBOOT)
+                .arg(hint.toText())
+                .arg(client_id ? client_id->toText():"(no client-id)")
+                .arg(hwaddr ? hwaddr->toText():"(no hwaddr info)");
+
+            answer.reset();
+            return;
+        }
     }
 
 
@@ -1329,6 +1350,11 @@ Dhcpv4Srv::processDiscover(Pkt4Ptr& discover) {
 
     assignLease(discover, offer);
 
+    if (!offer) {
+        // The offer is empty so return it *now*!
+        return (offer);
+    }
+
     // Adding any other options makes sense only when we got the lease.
     if (offer->getYiaddr() != IOAddress("0.0.0.0")) {
         appendRequestedOptions(discover, offer);
@@ -1376,6 +1402,11 @@ Dhcpv4Srv::processRequest(Pkt4Ptr& request) {
     // or even rebinding.
     assignLease(request, ack);
 
+    if (!ack) {
+        // The ack is empty so return it *now*!
+        return (ack);
+    }
+
     // Adding any other options makes sense only when we got the lease.
     if (ack->getYiaddr() != IOAddress("0.0.0.0")) {
         appendRequestedOptions(request, ack);

src/bin/dhcp4/dhcp4_srv.h

-// Copyright (C) 2011-2014 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2011-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
@@ -359,7 +359,13 @@ protected:
     /// are added to specific message.
     ///
     /// @param question DISCOVER or REQUEST message from client
-    /// @param answer OFFER or ACK/NAK message (lease options will be added here)
+
+    /// @param answer OFFER or ACK/NAK message (lease options will be
+    /// added here)
+    /// 
+    /// This method may reset the @c answer shared pointer to indicate
+    /// that the response should not be sent to the client. The caller
+    /// must check if the @c answer is null after calling this method.
     void assignLease(const Pkt4Ptr& question, Pkt4Ptr& answer);
 
     /// @brief Append basic options if they are not present.

src/bin/dhcp4/tests/dora_unittest.cc

-// Copyright (C) 2014 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2014-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
@@ -327,7 +327,7 @@ TEST_F(DORATest, selectingRequestNonMatchingAddress) {
 
 // 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.
+// if the client requests invalid address the server sends a DHCPNAK.
 TEST_F(DORATest, initRebootRequest) {
     Dhcp4Client client(Dhcp4Client::SELECTING);
     // Configure DHCP server.
@@ -373,6 +373,12 @@ TEST_F(DORATest, initRebootRequest) {
     ASSERT_TRUE(client.getContext().response_);
     resp = client.getContext().response_;
     EXPECT_EQ(DHCPNAK, static_cast<int>(resp->getType()));
+
+    // Try to request from a different client.
+    client.modifyHWAddr();
+    ASSERT_NO_THROW(client.doRequest());
+    // The server should not respond.
+    EXPECT_FALSE(client.getContext().response_);
 }
 
 // Check that the ciaddr returned by the server is correct for DHCPOFFER and