From: Marcin Siodelski Date: Wed, 8 Jan 2014 20:09:15 +0000 (+0100) Subject: [3231] Implemented function which checks if the v4 packet is relayed. X-Git-Tag: bind10-1.2.0beta1-release~159^2~2 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=756857b003804fe4a869fd52cf837e4e70bf5f0b;p=thirdparty%2Fkea.git [3231] Implemented function which checks if the v4 packet is relayed. --- diff --git a/src/bin/dhcp4/dhcp4_srv.cc b/src/bin/dhcp4/dhcp4_srv.cc index 5237554a36..3c6afc50df 100644 --- a/src/bin/dhcp4/dhcp4_srv.cc +++ b/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 diff --git a/src/bin/dhcp4/dhcp4_srv.h b/src/bin/dhcp4/dhcp4_srv.h index 510aae831c..487000e807 100644 --- a/src/bin/dhcp4/dhcp4_srv.h +++ b/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). /// diff --git a/src/bin/dhcp4/tests/dhcp4_srv_unittest.cc b/src/bin/dhcp4/tests/dhcp4_srv_unittest.cc index 31db4e58f5..d3880b9071 100644 --- a/src/bin/dhcp4/tests/dhcp4_srv_unittest.cc +++ b/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 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 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 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. diff --git a/src/bin/dhcp4/tests/dhcp4_test_utils.cc b/src/bin/dhcp4/tests/dhcp4_test_utils.cc index 26c611c7bb..341717f6e4 100644 --- a/src/bin/dhcp4/tests/dhcp4_test_utils.cc +++ b/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 diff --git a/src/lib/dhcp/pkt4.cc b/src/lib/dhcp/pkt4.cc index 8a5b8abc47..e9c2093974 100644 --- a/src/lib/dhcp/pkt4.cc +++ b/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(getHops()) << ". Valid values" + " are: (giaddr = 0 and hops = 0) or (giaddr != 0 and" + "hops != 0)"); + +} + } // end of namespace isc::dhcp } // end of namespace isc diff --git a/src/lib/dhcp/pkt4.h b/src/lib/dhcp/pkt4.h index c8015cd598..3d9e0abf45 100644 --- a/src/lib/dhcp/pkt4.h +++ b/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 diff --git a/src/lib/dhcp/tests/pkt4_unittest.cc b/src/lib/dhcp/tests/pkt4_unittest.cc index 72ffff7048..b1bb85275c 100644 --- a/src/lib/dhcp/tests/pkt4_unittest.cc +++ b/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