From: Marcin Siodelski <marcin@isc.org>
Date: Mon, 25 Jul 2016 17:35:12 +0000 (+0200)
Subject: [github23] Corrected setting port number for DHCPv4 server.
X-Git-Tag: trac4551_base~15
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=4216a4bedf8a884df087c68d48c88ee9ffa444dc;p=thirdparty%2Fkea.git

[github23] Corrected setting port number for DHCPv4 server.

For the DHCPINFORM case set destination port 68 if:
- direct message sent without ciaddr,
- ciaddr present

Also updated unit test to verify port number for the directly
sent DHCPINFORM without ciaddr.
---

diff --git a/src/bin/dhcp4/dhcp4_srv.cc b/src/bin/dhcp4/dhcp4_srv.cc
index 6faab3dca4..1ef9d7f0f8 100644
--- a/src/bin/dhcp4/dhcp4_srv.cc
+++ b/src/bin/dhcp4/dhcp4_srv.cc
@@ -1776,15 +1776,25 @@ Dhcpv4Srv::adjustIfaceData(Dhcpv4Exchange& ex) {
     Pkt4Ptr query = ex.getQuery();
     Pkt4Ptr response = ex.getResponse();
 
-    // For the non-relayed message, the destination port is the client's port.
-    // For the relayed message, the server/relay port is a destination unless
-    // the message is DHCPINFORM.
+    // The DHCPINFORM is generally unicast to the client. The only situation
+    // when the server is unable to unicast to the client is when the client
+    // doesn't include ciaddr and the message is relayed. In this case the
+    // server has to reply via relay agent. For other messages we send back
+    // through relay if message is relayed, and unicast to the client if the
+    // message is not relayed.
     // Note that the call to this function may throw if invalid combination
     // of hops and giaddr is found (hops = 0 if giaddr = 0 and hops != 0 if
     // giaddr != 0). The exception will propagate down and eventually cause the
     // packet to be discarded.
-    const bool server_port = query->isRelayed() && ((query->getType() == DHCPINFORM) ? query->getCiaddr().isV4Zero() : true);
-    response->setRemotePort(server_port ? DHCP4_SERVER_PORT : DHCP4_CLIENT_PORT);
+    if (((query->getType() == DHCPINFORM) &&
+         ((!query->getCiaddr().isV4Zero()) ||
+          (!query->isRelayed() && !query->getRemoteAddr().isV4Zero()))) ||
+        ((query->getType() != DHCPINFORM) && !query->isRelayed())) {
+        response->setRemotePort(DHCP4_CLIENT_PORT);
+
+    } else {
+        response->setRemotePort(DHCP4_SERVER_PORT);
+    }
 
     IOAddress local_addr = query->getLocalAddr();
 
diff --git a/src/bin/dhcp4/tests/dhcp4_client.cc b/src/bin/dhcp4/tests/dhcp4_client.cc
index 82c78e7a60..28b6cc08bf 100644
--- a/src/bin/dhcp4/tests/dhcp4_client.cc
+++ b/src/bin/dhcp4/tests/dhcp4_client.cc
@@ -467,6 +467,8 @@ Dhcp4Client::receiveOneMsg() {
                               msg->getBuffer().getLength()));
     msg_copy->setRemoteAddr(msg->getLocalAddr());
     msg_copy->setLocalAddr(msg->getRemoteAddr());
+    msg_copy->setRemotePort(msg->getLocalPort());
+    msg_copy->setLocalPort(msg->getRemotePort());
     msg_copy->setIface(msg->getIface());
 
     msg_copy->unpack();
diff --git a/src/bin/dhcp4/tests/inform_unittest.cc b/src/bin/dhcp4/tests/inform_unittest.cc
index e00f7c78b5..5ed038b4dc 100644
--- a/src/bin/dhcp4/tests/inform_unittest.cc
+++ b/src/bin/dhcp4/tests/inform_unittest.cc
@@ -268,6 +268,8 @@ TEST_F(InformTest, directClientNoCiaddr) {
     EXPECT_EQ(IOAddress("10.0.0.56"), resp->getLocalAddr());
     // Response must not be relayed.
     EXPECT_FALSE(resp->isRelayed());
+    EXPECT_EQ(DHCP4_CLIENT_PORT, resp->getLocalPort());
+    EXPECT_EQ(DHCP4_SERVER_PORT, resp->getRemotePort());
     // Make sure that the server id is present.
     EXPECT_EQ("10.0.0.1", client.config_.serverid_.toText());
     // Make sure that the Routers option has been received.