[3231] Implemented function which checks if the v4 packet is relayed.

src/bin/dhcp4/dhcp4_srv.cc

@@ -1154,11 +1154,13 @@ Dhcpv4Srv::adjustIfaceData(const Pkt4Ptr& query, const Pkt4Ptr& response) {
 
     // For the non-relayed message, the destination port is the client's port.
     // For the relayed message, the server/relay port is a destination.
-    if (!response->getHops()) {
-        response->setRemotePort(DHCP4_CLIENT_PORT);
-    } else {
-        response->setRemotePort(DHCP4_SERVER_PORT);
-    }
+    // 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.
+    response->setRemotePort(query->isRelayed() ? DHCP4_SERVER_PORT :
+                            DHCP4_CLIENT_PORT);
+
     // In many cases the query is sent to a broadcast address. This address
     // appears as a local address in the query message. Therefore we can't
     // simply copy local address from the query and use it as a source

src/bin/dhcp4/dhcp4_srv.h

@@ -438,8 +438,7 @@ protected:
     /// address).
     ///
     /// The destination port is always DHCPv4 client (68) or relay (67) port,
-    /// depending if the response will be sent directly to a client (hops = 0),
-    /// or through a relay (hops > 0).
+    /// depending if the response will be sent directly to a client.
     ///
     /// The source port is always set to DHCPv4 server port (67).
     ///

src/bin/dhcp4/tests/dhcp4_srv_unittest.cc

@@ -322,6 +322,40 @@ TEST_F(Dhcpv4SrvFakeIfaceTest, adjustIfaceDataBroadcast) {
 
 }
 
+// This test verifies that exception is thrown of the invalid combination
+// of giaddr and hops is specified in a client's message.
+TEST_F(Dhcpv4SrvFakeIfaceTest, adjustIfaceDataInvalid) {
+    boost::shared_ptr<Pkt4> req(new Pkt4(DHCPDISCOVER, 1234));
+
+    // The hops and giaddr values are used to determine if the client's
+    // message has been relayed or sent directly. The allowed combinations
+    // are (giaddr = 0 and hops = 0) or (giaddr != 0 and hops != 0). Any
+    // other combination is invalid and the adjustIfaceData should throw
+    // an exception. We will test that exception is indeed thrown.
+    req->setGiaddr(IOAddress("0.0.0.0"));
+    req->setHops(1);
+
+    // Clear client address as it hasn't got any address configured yet.
+    req->setCiaddr(IOAddress("0.0.0.0"));
+    // The query is sent to the broadcast address in the Select state.
+    req->setLocalAddr(IOAddress("255.255.255.255"));
+    // The query has been received on the DHCPv4 server port 67.
+    req->setLocalPort(DHCP4_SERVER_PORT);
+    // Set the interface. The response should be sent via the same interface.
+    req->setIface("eth0");
+    req->setIndex(1);
+
+    // Create a response.
+    boost::shared_ptr<Pkt4> resp(new Pkt4(DHCPOFFER, 1234));
+    // Assign some new address for this client.
+    resp->setYiaddr(IOAddress("192.0.2.13"));
+
+    // Clear the remote address.
+    resp->setRemoteAddr(IOAddress("0.0.0.0"));
+
+    EXPECT_THROW(NakedDhcpv4Srv::adjustIfaceData(req, resp), isc::BadValue);
+}
+
 // This test verifies that the server identifier option is appended to
 // a specified DHCPv4 message and the server identifier is correct.
 TEST_F(Dhcpv4SrvTest, appendServerID) {
@@ -2933,8 +2967,10 @@ TEST_F(Dhcpv4SrvFakeIfaceTest, vendorOptionsORO) {
     ASSERT_EQ(0, rcode_);
 
     boost::shared_ptr<Pkt4> dis(new Pkt4(DHCPDISCOVER, 1234));
-    // Set the giaddr to non-zero address as if it was relayed.
+    // Set the giaddr and hops to non-zero address as if it was relayed.
     dis->setGiaddr(IOAddress("192.0.2.1"));
+    dis->setHops(1);
+
     OptionPtr clientid = generateClientId();
     dis->addOption(clientid);
     // Set interface. It is required by the server to generate server id.

src/bin/dhcp4/tests/dhcp4_test_utils.cc

@@ -484,8 +484,9 @@ Dhcpv4SrvFakeIfaceTest::testDiscoverRequest(const uint8_t msg_type) {
     req->setLocalHWAddr(1, 6, mac);
     // Set target IP address.
     req->setRemoteAddr(IOAddress("192.0.2.55"));
-    // Set relay address.
+    // Set relay address and hops.
     req->setGiaddr(IOAddress("192.0.2.10"));
+    req->setHops(1);
 
     // We are going to test that certain options are returned
     // in the response message when requested using 'Parameter

src/lib/dhcp/pkt4.cc

@@ -463,6 +463,26 @@ Pkt4::updateTimestamp() {
     timestamp_ = boost::posix_time::microsec_clock::universal_time();
 }
 
+bool
+Pkt4::isRelayed() const {
+    static const IOAddress zero_addr("0.0.0.0");
+    // For non-relayed message both Giaddr and Hops are zero.
+    if (getGiaddr() == zero_addr && getHops() == 0) {
+        return (false);
+
+    // For relayed message, both Giaddr and Hops are non-zero.
+    } else if (getGiaddr() != zero_addr && getHops() > 0) {
+        return (true);
+    }
+    // In any other case, the packet is considered malformed.
+    isc_throw(isc::BadValue, "invalid combination of giaddr = "
+              << getGiaddr().toText() << " and hops = "
+              << static_cast<int>(getHops()) << ". Valid values"
+              " are: (giaddr = 0 and hops = 0) or (giaddr != 0 and"
+              "hops != 0)");
+
+}
+
 } // end of namespace isc::dhcp
 
 } // end of namespace isc

src/lib/dhcp/pkt4.h

@@ -484,6 +484,24 @@ public:
     /// @return remote port
     uint16_t getRemotePort() const { return (remote_port_); }
 
+    /// @brief Checks if a DHCPv4 message has been relayed.
+    ///
+    /// This function returns a boolean value which indicates whether a DHCPv4
+    /// message has been relayed (if true is returned) or not (if false).
+    ///
+    /// This function uses a combination of Giaddr and Hops. It is expected that
+    /// if Giaddr is not 0, the Hops is greater than 0. In this case the message
+    /// is considered relayed. If Giaddr is 0, the Hops value must also be 0. In
+    /// this case the message is considered non-relayed. For any other
+    /// combination of Giaddr and Hops, an exception is thrown to indicate that
+    /// the message is malformed.
+    ///
+    /// @return Boolean value which indicates whether the message is relayed
+    /// (true) or non-relayed (false).
+    /// @throw isc::BadValue if invalid combination of Giaddr and Hops values is
+    /// found.
+    bool isRelayed() const;
+
     /// @brief Set callback function to be used to parse options.
     ///
     /// @param callback An instance of the callback function or NULL to

src/lib/dhcp/tests/pkt4_unittest.cc

@@ -798,4 +798,27 @@ TEST_F(Pkt4Test, hwaddrSrcRemote) {
                            remote_addr->hwaddr_.begin()));
 }
 
+// This test verifies that the check for a message being relayed is correct.
+// It also checks that the exception is thrown if the combination of hops and
+// giaddr is invalid.
+TEST_F(Pkt4Test, isRelayed) {
+    Pkt4 pkt(DHCPDISCOVER, 1234);
+    // By default, the hops and giaddr should be 0.
+    ASSERT_EQ("0.0.0.0", pkt.getGiaddr().toText());
+    ASSERT_EQ(0, pkt.getHops());
+    // For hops = 0 and giaddr = 0, the message is non-relayed.
+    EXPECT_FALSE(pkt.isRelayed());
+    // Set giaddr but leave hops = 0. This should result in exception.
+    pkt.setGiaddr(IOAddress("10.0.0.1"));
+    EXPECT_THROW(pkt.isRelayed(), isc::BadValue);
+    // Set hops. Now both hops and giaddr is set. The message is relayed.
+    pkt.setHops(10);
+    EXPECT_TRUE(pkt.isRelayed());
+    // Set giaddr to 0. For hops being set to non-zero value the function
+    // should throw an exception.
+    pkt.setGiaddr(IOAddress("0.0.0.0"));
+    EXPECT_THROW(pkt.isRelayed(), isc::BadValue);
+
+}
+
 } // end of anonymous namespace