Index: trunk/src/VBox/NetworkServices/DHCP/VBoxNetDHCP.cpp
===================================================================
--- trunk/src/VBox/NetworkServices/DHCP/VBoxNetDHCP.cpp	(revision 17783)
+++ trunk/src/VBox/NetworkServices/DHCP/VBoxNetDHCP.cpp	(revision 17786)
@@ -38,4 +38,5 @@
 #include <iprt/param.h>
 #include <iprt/getopt.h>
+#include <iprt/string.h>
 
 #include <VBox/sup.h>
@@ -379,6 +380,6 @@
     void                makeDhcpReply(uint8_t uMsgType, VBoxNetDhcpLease *pLease, PCRTNETBOOTP pDhcpMsg, size_t cb);
 
-    VBoxNetDhcpLease   *findLeaseByMacAddress(PCRTMAC pMacAddress);
-    VBoxNetDhcpLease   *findLeaseByIpv4AndMacAddresses(RTNETADDRIPV4 IPv4Addr, PCRTMAC pMacAddress);
+    VBoxNetDhcpLease   *findLeaseByMacAddress(PCRTMAC pMacAddress, bool fAnyState);
+    VBoxNetDhcpLease   *findLeaseByIpv4AndMacAddresses(RTNETADDRIPV4 IPv4Addr, PCRTMAC pMacAddress, bool fAnyState);
     VBoxNetDhcpLease   *newLease(PCRTNETBOOTP pDhcpMsg, size_t cb);
 
@@ -440,4 +441,5 @@
 {
     m_enmState = kState_Offer;
+    m_xid = xid;
     RTTimeNow(&m_ExpireTime);
     RTTimeSpecAddSeconds(&m_ExpireTime, 60);
@@ -538,5 +540,9 @@
     pDefCfg->m_DNSes.push_back(Addr);
     pDefCfg->m_DomainName     = "vboxnetdhcp.org";
+#if 0
     pDefCfg->m_cSecLease      = 60*60; /* 1 hour */
+#else
+    pDefCfg->m_cSecLease      = 30; /* sec */
+#endif
     pDefCfg->m_TftpServer     = "10.0.2.3"; //??
     this->addConfig(pDefCfg);
@@ -942,5 +948,5 @@
         {
             size_t  cb;
-            void   *pv = VBoxNetUDPMatch(m_pIfBuf, 67 /* bootps */, &m_MacAddress,
+            void   *pv = VBoxNetUDPMatch(m_pIfBuf, RTNETIPV4_PORT_BOOTPS, &m_MacAddress,
                                          VBOXNETUDP_MATCH_UNICAST | VBOXNETUDP_MATCH_BROADCAST | VBOXNETUDP_MATCH_CHECKSUM
                                          | (m_cVerbosity > 2 ? VBOXNETUDP_MATCH_PRINT_STDERR : 0),
@@ -1027,12 +1033,8 @@
 {
     /*
-     * First, see if there is already a lease for this client. It may have rebooted,
-     * crashed or whatever that have caused it to forget its existing lease.
-     * If none was found, create a new lease for it and then construct a reply.
-     */
-    VBoxNetDhcpLease *pLease = findLeaseByMacAddress(&pDhcpMsg->bp_chaddr.Mac);
-    if (    !pLease
-        ||  !pLease->isInCurrentConfig())
-        pLease = newLease(pDhcpMsg, cb);
+     * The newLease() method contains logic for finding current leases
+     * and reusing them in case the client is forgetful.
+     */
+    VBoxNetDhcpLease *pLease = newLease(pDhcpMsg, cb);
     if (!pLease)
         return false;
@@ -1063,72 +1065,95 @@
     /** @todo Probably need to match the server IP here to work correctly with
      *        other servers. */
-    /** @todo check this against the RFC and real code. the code is kind of
-     *        fishy... */
-
-    /*
-     * Windows will reissue these requests when rejoining a network if it thinks it
-     * already has an address on the network. If we cannot find a valid lease,
-     * make a new one and return NAC.
-     */
-    RTNETADDRIPV4 IPv4Addr;
-    if (findOptionIPv4Addr(RTNET_DHCP_OPT_REQ_ADDR, pDhcpMsg, cb, &IPv4Addr))
-    {
-        VBoxNetDhcpLease *pLease = findLeaseByIpv4AndMacAddresses(IPv4Addr, &pDhcpMsg->bp_chaddr.Mac);
-        if (    pLease
-            &&  pLease->isInCurrentConfig())
-        {
-            if (pLease->isBeingOffered())
+    /** @todo This code isn't entirely correct and quite a bit of a hack, but it
+     *        will have to do for now as the right thing (tm) is very complex.
+     *        Part of the fun is verifying that the request is something we can
+     *        and should handle. */
+
+    /*
+     * Try find the lease by the requested address + client MAC address.
+     */
+    VBoxNetDhcpLease   *pLease = NULL;
+    RTNETADDRIPV4       IPv4Addr;
+    bool                fReqAddr = findOptionIPv4Addr(RTNET_DHCP_OPT_REQ_ADDR, pDhcpMsg, cb, &IPv4Addr);
+    if (fReqAddr)
+    {
+        fReqAddr = true;
+        pLease = findLeaseByIpv4AndMacAddresses(IPv4Addr, &pDhcpMsg->bp_chaddr.Mac, true /* fAnyState */);
+    }
+
+    /*
+     * Try find the lease by the client IP address + client MAC address.
+     */
+    if (    !pLease
+        &&  pDhcpMsg->bp_ciaddr.u)
+        pLease = findLeaseByIpv4AndMacAddresses(pDhcpMsg->bp_ciaddr, &pDhcpMsg->bp_chaddr.Mac, true /* fAnyState */);
+
+#if 0 /** @todo client id stuff - it doesn't make sense here imho, we need IP + MAC. What would make sense
+                though is to compare the client id with what we've got in the lease and use it to root out
+                bad requests. */
+    /*
+     * Try find the lease by using the client id.
+     */
+    if (!pLease)
+    {
+        size_t          cbClientID = 0;
+        uint8_t const  *pbClientID  = findOption(RTNET_DHCP_OPT_CLIENT_ID, pDhcpMsg, cb, &cbClientID);
+        if (    pbClientID
+            &&  cbClientID == sizeof(RTMAC) + 1
+            &&  pbClientID[0] == RTNET_ARP_ETHER
+            &&
+                )
+        {
+            pLease = findLeaseByIpv4AndMacAddresses(pDhcpMsg->bp_ciaddr, &pDhcpMsg->bp_chaddr.Mac, true /* fAnyState */);
+        }
+    }
+#endif
+
+    /*
+     * Validate the lease that's requested.
+     * We've already check the MAC and IP addresses.
+     */
+    bool fAckIt = false;
+    if (pLease)
+    {
+        if (pLease->isBeingOffered())
+        {
+            if (pLease->m_xid == pDhcpMsg->bp_xid)
+            {
+                fAckIt = true;
                 debugPrint(2, true, "REQUEST for offered lease.");
-            else
-                debugPrint(1, true, "REQUEST for lease not on offer.");
-
-            /* Check if the xid matches, if it doesn't it's not our call. */
-#if 0      /** @todo check how windows treats bp_xid here, it should match I think. If
-            *        it doesn't we've no way of filtering out broadcast replies to other
-            *        DHCP servers. Fix this later.
-            */
-            if (pDhcpMsg->bp_xid != pLease->m_xid)
-            {
-                debugPrint(1, true, "bp_xid %#x != lease %#x", pDhcpMsg->bp_xid, pLease->m_xid);
-                return true;
-            }
-#endif
-            /* Check if the config has changed since the offer was given? NAK it then? */
-
-            /*
-             * Ack it.
-             */
-            pLease->activate(pDhcpMsg->bp_xid);
-            makeDhcpReply(RTNET_DHCP_MT_ACK, pLease, pDhcpMsg, cb);
-        }
-        else
-        {
-            /*
-             * Try make a new offer and see if we get the requested IP and config, that
-             * will make the (windows) client happy apparently...
-             */
-            pLease = newLease(pDhcpMsg, cb);
-            if (    pLease
-                &&  pLease->m_IPv4Address.u == IPv4Addr.u
-                /** @todo match requested config later */)
-            {
-                /* ACK it. */
-                debugPrint(1, true, "REQUEST for lease not on offer, new lease matches.");
                 pLease->activate();
-                makeDhcpReply(RTNET_DHCP_MT_ACK, pLease, pDhcpMsg, cb);
             }
             else
-            {
-                /* NAK it */
-                debugPrint(1, true, "REQUEST for lease not on offer, NACnig it.");
-                if (pLease)
-                    pLease->release();
-                pLease->activate();
-                makeDhcpReply(RTNET_DHCP_MT_NAC, NULL, pDhcpMsg, cb);
-            }
-        }
+                debugPrint(2, true, "REQUEST for offered lease, xid mismatch. Expected %#x, got %#x.",
+                           pLease->m_xid, pDhcpMsg->bp_xid);
+        }
+        else if (!pLease->isInCurrentConfig())
+            debugPrint(1, true, "REQUEST for obsolete lease -> NAK");
+        else if (fReqAddr != (pDhcpMsg->bp_ciaddr.u != 0)) // ???
+        {
+            /** @todo this ain't safe. */
+            debugPrint(1, true, "REQUEST for lease not on offer, assuming renewal. lease_xid=%#x bp_xid=%#x",
+                       pLease->m_xid, pDhcpMsg->bp_xid);
+            fAckIt = true;
+            pLease->activate(pDhcpMsg->bp_xid);
+        }
+        else
+            debugPrint(1, true, "REQUEST for lease not on offer, NAK it.");
+    }
+
+    /*
+     * NAK if if no lease was found.
+     */
+    if (fAckIt)
+    {
+        debugPrint(1, false, "ACK'ing DHCP_REQUEST");
+        makeDhcpReply(RTNET_DHCP_MT_ACK, pLease, pDhcpMsg, cb);
     }
     else
-        debugPrint(1, true, "No requested address option");
+    {
+        debugPrint(1, false, "NAK'ing DHCP_REQUEST");
+        makeDhcpReply(RTNET_DHCP_MT_NAC, NULL, pDhcpMsg, cb);
+    }
 
     return true;
@@ -1156,4 +1181,5 @@
 
     /** @todo this is not required in the initial implementation, do it later. */
+    debugPrint(1, true, "DECLINE is not implemented");
     return true;
 }
@@ -1192,4 +1218,5 @@
 
     /** @todo this is not required in the initial implementation, do it later. */
+    debugPrint(1, true, "RELEASE is not implemented");
     return true;
 }
@@ -1205,4 +1232,5 @@
     uint8_t        *m_pbEnd;       /**< The end the current option space. */
     uint8_t        *m_pfOverload;  /**< Pointer to the flags of the overload option. */
+    uint8_t         m_fUsed;       /**< Overload fields that have been used. */
     PRTNETDHCPOPT   m_pOpt;        /**< The current option. */
     PRTNETBOOTP     m_pDhcp;       /**< The DHCP packet. */
@@ -1215,4 +1243,5 @@
         m_pbEnd((uint8_t *)pDhcp + cbDhcp),
         m_pfOverload(NULL),
+        m_fUsed(0),
         m_pOpt(NULL),
         m_pDhcp(pDhcp),
@@ -1232,4 +1261,18 @@
 
     /**
+     * Try use the bp_file field.
+     * @returns true if not overloaded, false otherwise.
+     */
+    bool useBpFile(void)
+    {
+        if (    m_pfOverload
+            &&  (*m_pfOverload & 1))
+            return false;
+        m_fUsed |= 1 /* bp_file flag*/;
+        return true;
+    }
+
+
+    /**
      * Try overload more BOOTP fields
      */
@@ -1239,24 +1282,30 @@
         uint8_t    *pbNew;
         uint8_t    *pbNewEnd;
+        uint8_t     fField;
+        if (!(m_fUsed & 1))
+        {
+            fField     = 1;
+            pbNew      = &m_pDhcp->bp_file[0];
+            pbNewEnd   = &m_pDhcp->bp_file[sizeof(m_pDhcp->bp_file)];
+        }
+        else if (!(m_fUsed & 2))
+        {
+            fField     = 2;
+            pbNew      = &m_pDhcp->bp_sname[0];
+            pbNewEnd   = &m_pDhcp->bp_sname[sizeof(m_pDhcp->bp_sname)];
+        }
+        else
+            return false;
+
         if (!m_pfOverload)
         {
             /* Add an overload option. */
             *m_pbCur++ = RTNET_DHCP_OPT_OPTION_OVERLOAD;
-            *m_pbCur++ = 1;
+            *m_pbCur++ = fField;
             m_pfOverload = m_pbCur;
             *m_pbCur++ = 1;     /* bp_file flag */
-
-            pbNew      = &m_pDhcp->bp_file[0];
-            pbNewEnd   = &m_pDhcp->bp_file[sizeof(m_pDhcp->bp_file)];
-        }
-        else if (!(*m_pfOverload & 2))
-        {
-            *m_pfOverload |= 2; /* bp_sname flag */
-
-            pbNew      = &m_pDhcp->bp_sname[0];
-            pbNewEnd   = &m_pDhcp->bp_sname[sizeof(m_pDhcp->bp_sname)];
         }
         else
-            return false;
+            *m_pfOverload |= fField;
 
         /* pad current option field */
@@ -1264,4 +1313,5 @@
             *m_pbCur++ = RTNET_DHCP_OPT_PAD; /** @todo not sure if this stuff is at all correct... */
 
+        /* switch */
         m_pbCur = pbNew;
         m_pbEnd = pbNewEnd;
@@ -1285,5 +1335,5 @@
 
         /* Check if we need to overload more stuff. */
-        if ((uintptr_t)(m_pbEnd - m_pbCur) < cb + 2 + 3)
+        if ((uintptr_t)(m_pbEnd - m_pbCur) < cb + 2 + (m_pfOverload ? 1 : 3))
         {
             m_pOpt = NULL;
@@ -1293,5 +1343,5 @@
                 AssertMsgFailedReturn(("%u %#x\n", uOption, cb), false);
             }
-            if ((uintptr_t)(m_pbEnd - m_pbCur) < cb + 2 + 3)
+            if ((uintptr_t)(m_pbEnd - m_pbCur) < cb + 2 + 1)
             {
                 m_fOverflowed = true;
@@ -1511,5 +1561,5 @@
     pReply->bp_ciaddr.u = 0;
     pReply->bp_yiaddr.u = pLease ? pLease->m_IPv4Address.u : 0xffffffff;
-    pReply->bp_siaddr.u = pLease && pLease->m_pCfg ? pLease->m_pCfg->m_TftpServerAddr.u : 0;
+    pReply->bp_siaddr.u = pLease && pLease->m_pCfg ? pLease->m_pCfg->m_TftpServerAddr.u : 0; /* (next server == TFTP)*/
     pReply->bp_giaddr.u = 0;
     memset(&pReply->bp_chaddr, '\0', sizeof(pReply->bp_chaddr));
@@ -1543,4 +1593,11 @@
 
         /* The PXE config. */
+        if (pCfg->m_BootfileName.size())
+        {
+            if (Cursor.useBpFile())
+                RTStrPrintf((char *)&pReply->bp_file[0], sizeof(pReply->bp_file), "%s", pCfg->m_BootfileName.c_str());
+            else
+                Cursor.optStr(RTNET_DHCP_OPT_BOOTFILE_NAME, pCfg->m_BootfileName);
+        }
     }
 
@@ -1554,5 +1611,5 @@
     int rc;
 #if 0
-    if (pDhcpMsg->bp_flags & RTNET_DHCP_FLAGS_NO_BROADCAST)
+    if (!(pDhcpMsg->bp_flags & RTNET_DHCP_FLAGS_NO_BROADCAST)) /** @todo need to see someone set this flag to check that it's correct. */
     {
         RTNETADDRIPV4 IPv4AddrBrdCast;
@@ -1574,5 +1631,4 @@
 
 
-
 /**
  * Look up a lease by MAC address.
@@ -1580,6 +1636,7 @@
  * @returns Pointer to the lease if found, NULL if not found.
  * @param   pMacAddress             The mac address.
- */
-VBoxNetDhcpLease *VBoxNetDhcp::findLeaseByMacAddress(PCRTMAC pMacAddress)
+ * @param   fAnyState       Any state.
+ */
+VBoxNetDhcpLease *VBoxNetDhcp::findLeaseByMacAddress(PCRTMAC pMacAddress, bool fAnyState)
 {
     size_t iLease = m_Leases.size();
@@ -1590,5 +1647,7 @@
             &&  pLease->m_MacAddress.au16[0] == pMacAddress->au16[0]
             &&  pLease->m_MacAddress.au16[1] == pMacAddress->au16[1]
-            &&  pLease->m_MacAddress.au16[2] == pMacAddress->au16[2])
+            &&  pLease->m_MacAddress.au16[2] == pMacAddress->au16[2]
+            &&  (   fAnyState
+                 || (pLease->m_enmState != VBoxNetDhcpLease::kState_Free)) )
             return pLease;
     }
@@ -1604,6 +1663,7 @@
  * @param   IPv4Addr        The IPv4 address.
  * @param   pMacAddress     The mac address.
- */
-VBoxNetDhcpLease *VBoxNetDhcp::findLeaseByIpv4AndMacAddresses(RTNETADDRIPV4 IPv4Addr, PCRTMAC pMacAddress)
+ * @param   fAnyState       Any state.
+ */
+VBoxNetDhcpLease *VBoxNetDhcp::findLeaseByIpv4AndMacAddresses(RTNETADDRIPV4 IPv4Addr, PCRTMAC pMacAddress, bool fAnyState)
 {
     size_t iLease = m_Leases.size();
@@ -1615,5 +1675,7 @@
             &&  pLease->m_MacAddress.au16[0] == pMacAddress->au16[0]
             &&  pLease->m_MacAddress.au16[1] == pMacAddress->au16[1]
-            &&  pLease->m_MacAddress.au16[2] == pMacAddress->au16[2])
+            &&  pLease->m_MacAddress.au16[2] == pMacAddress->au16[2]
+            &&  (   fAnyState
+                 || (pLease->m_enmState != VBoxNetDhcpLease::kState_Free)) )
             return pLease;
     }
@@ -1844,10 +1906,11 @@
         {
             const char *pszMsg = m_uCurMsgType != UINT8_MAX ? debugDhcpName(m_uCurMsgType) : "";
-            RTStrmPrintf(g_pStdErr, "VBoxNetDHCP: debug: %8s chaddr=%.6Rhxs ciaddr=%d.%d.%d.%d yiaddr=%d.%d.%d.%d siaddr=%d.%d.%d.%d\n",
+            RTStrmPrintf(g_pStdErr, "VBoxNetDHCP: debug: %8s chaddr=%.6Rhxs ciaddr=%d.%d.%d.%d yiaddr=%d.%d.%d.%d siaddr=%d.%d.%d.%d xid=%#x\n",
                          pszMsg,
                          &m_pCurMsg->bp_chaddr,
                          m_pCurMsg->bp_ciaddr.au8[0], m_pCurMsg->bp_ciaddr.au8[1], m_pCurMsg->bp_ciaddr.au8[2], m_pCurMsg->bp_ciaddr.au8[3],
                          m_pCurMsg->bp_yiaddr.au8[0], m_pCurMsg->bp_yiaddr.au8[1], m_pCurMsg->bp_yiaddr.au8[2], m_pCurMsg->bp_yiaddr.au8[3],
-                         m_pCurMsg->bp_siaddr.au8[0], m_pCurMsg->bp_siaddr.au8[1], m_pCurMsg->bp_siaddr.au8[2], m_pCurMsg->bp_siaddr.au8[3]);
+                         m_pCurMsg->bp_siaddr.au8[0], m_pCurMsg->bp_siaddr.au8[1], m_pCurMsg->bp_siaddr.au8[2], m_pCurMsg->bp_siaddr.au8[3],
+                         m_pCurMsg->bp_xid);
         }
     }
