BIND 10 trac3231, updated. 756857b003804fe4a869fd52cf837e4e70bf5f0b [3231] Implemented function which checks if the v4 packet is relayed.
BIND 10 source code commits
bind10-changes at lists.isc.org
Wed Jan 8 20:09:25 UTC 2014
The branch, trac3231 has been updated
via 756857b003804fe4a869fd52cf837e4e70bf5f0b (commit)
from c994af9f1877b0a62b56cb34ac3d7e0528b2fa07 (commit)
Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.
- Log -----------------------------------------------------------------
commit 756857b003804fe4a869fd52cf837e4e70bf5f0b
Author: Marcin Siodelski <marcin at isc.org>
Date: Wed Jan 8 21:09:15 2014 +0100
[3231] Implemented function which checks if the v4 packet is relayed.
-----------------------------------------------------------------------
Summary of changes:
src/bin/dhcp4/dhcp4_srv.cc | 12 +++++----
src/bin/dhcp4/dhcp4_srv.h | 3 +--
src/bin/dhcp4/tests/dhcp4_srv_unittest.cc | 38 ++++++++++++++++++++++++++++-
src/bin/dhcp4/tests/dhcp4_test_utils.cc | 3 ++-
src/lib/dhcp/pkt4.cc | 20 +++++++++++++++
src/lib/dhcp/pkt4.h | 18 ++++++++++++++
src/lib/dhcp/tests/pkt4_unittest.cc | 23 +++++++++++++++++
7 files changed, 108 insertions(+), 9 deletions(-)
-----------------------------------------------------------------------
diff --git a/src/bin/dhcp4/dhcp4_srv.cc b/src/bin/dhcp4/dhcp4_srv.cc
index 5237554..3c6afc5 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 510aae8..487000e 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 31db4e5..d3880b9 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<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.
diff --git a/src/bin/dhcp4/tests/dhcp4_test_utils.cc b/src/bin/dhcp4/tests/dhcp4_test_utils.cc
index 26c611c..341717f 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 8a5b8ab..e9c2093 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<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
diff --git a/src/lib/dhcp/pkt4.h b/src/lib/dhcp/pkt4.h
index c8015cd..3d9e0ab 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 72ffff7..b1bb852 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
More information about the bind10-changes
mailing list