BIND 10 trac2902, updated. e7a483b3f53323d03b64f49385d173ceb595c39c [2902] Adjust the destination address for response.
BIND 10 source code commits
bind10-changes at lists.isc.org
Tue May 21 13:19:11 UTC 2013
The branch, trac2902 has been updated
via e7a483b3f53323d03b64f49385d173ceb595c39c (commit)
from 5162bd7739ccfab83a0e81e44ba5e46314eae0cf (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 e7a483b3f53323d03b64f49385d173ceb595c39c
Author: Marcin Siodelski <marcin at isc.org>
Date: Tue May 21 15:18:58 2013 +0200
[2902] Adjust the destination address for response.
Packet will be sent to broadcast address if broadcast flag is set by the
client. Also, DHCPNAK is always sent to broadcast address.
-----------------------------------------------------------------------
Summary of changes:
src/bin/dhcp4/dhcp4_srv.cc | 90 ++++++++++++++++++-----------
src/bin/dhcp4/dhcp4_srv.h | 17 ++++++
src/bin/dhcp4/tests/dhcp4_srv_unittest.cc | 43 +++++++++++++-
3 files changed, 115 insertions(+), 35 deletions(-)
-----------------------------------------------------------------------
diff --git a/src/bin/dhcp4/dhcp4_srv.cc b/src/bin/dhcp4/dhcp4_srv.cc
index 3eb187b..f788197 100644
--- a/src/bin/dhcp4/dhcp4_srv.cc
+++ b/src/bin/dhcp4/dhcp4_srv.cc
@@ -205,9 +205,9 @@ Dhcpv4Srv::run() {
}
if (rsp) {
- if (rsp->getRemoteAddr().toText() == "0.0.0.0") {
- rsp->setRemoteAddr(query->getRemoteAddr());
- }
+
+ adjustRemoteAddr(query, rsp);
+
if (!rsp->getHops()) {
rsp->setRemotePort(DHCP4_CLIENT_PORT);
} else {
@@ -358,14 +358,6 @@ Dhcpv4Srv::copyDefaultFields(const Pkt4Ptr& question, Pkt4Ptr& answer) {
// relay address
answer->setGiaddr(question->getGiaddr());
- if (question->getGiaddr().toText() != "0.0.0.0") {
- // relayed traffic
- answer->setRemoteAddr(question->getGiaddr());
- } else {
- // direct traffic
- answer->setRemoteAddr(question->getRemoteAddr());
- }
-
// Let's copy client-id to response. See RFC6842.
OptionPtr client_id = question->getOption(DHO_DHCP_CLIENT_IDENTIFIER);
if (client_id) {
@@ -539,28 +531,6 @@ Dhcpv4Srv::assignLease(const Pkt4Ptr& question, Pkt4Ptr& answer) {
answer->setYiaddr(lease->addr_);
- // If remote address is not set, we are dealing with a directly
- // connected client requesting new lease. We can send response to
- // the address assigned in the lease, but first we have to make sure
- // that IfaceMgr supports responding directly to the client when
- // client doesn't have address assigned to its interface yet.
- if (answer->getRemoteAddr().toText() == "0.0.0.0") {
- if (IfaceMgr::instance().isDirectResponseSupported()) {
- answer->setRemoteAddr(lease->addr_);
- } else {
- // Since IfaceMgr does not support direct responses to
- // clients not having IP addresses, we have to send response
- // to broadcast. We don't check whether the use_bcast flag
- // was set in the constructor, because this flag is only used
- // by unit tests to prevent opening broadcast sockets, as
- // it requires root privileges. If this function is invoked by
- // unit tests, we expect that it sets broadcast address if
- // direct response is not supported, so as a test can verify
- // function's behavior, regardless of the use_bcast flag's value.
- answer->setRemoteAddr(IOAddress("255.255.255.255"));
- }
- }
-
// IP Address Lease time (type 51)
opt = OptionPtr(new Option(Option::V4, DHO_DHCP_LEASE_TIME));
opt->setUint32(lease->valid_lft_);
@@ -595,6 +565,60 @@ Dhcpv4Srv::assignLease(const Pkt4Ptr& question, Pkt4Ptr& answer) {
}
}
+void
+Dhcpv4Srv::adjustRemoteAddr(const Pkt4Ptr& question, Pkt4Ptr& msg) {
+ // Let's create static objects representing zeroed and broadcast
+ // addresses. We will use them further in this function to test
+ // other addresses against them. Since they are static, they will
+ // be created only once.
+ static const IOAddress zero_addr("0.0.0.0");
+ static const IOAddress bcast_addr("255.255.255.255");
+
+ // If received relayed message, server responds to the relay address.
+ if (question->getGiaddr() != zero_addr) {
+ msg->setRemoteAddr(question->getGiaddr());
+
+ // If giaddr is 0 but client set ciaddr, server should unicast the
+ // response to ciaddr.
+ } else if (question->getCiaddr() != zero_addr) {
+ msg->setRemoteAddr(question->getCiaddr());
+
+ // We can't unicast the response to the client when sending NAK,
+ // because we haven't allocated address for him. Therefore,
+ // NAK is broadcast.
+ } else if (msg->getType() == DHCPNAK) {
+ msg->setRemoteAddr(bcast_addr);
+
+ // If yiaddr is set it means that we have created a lease for a client.
+ } else if (question->getYiaddr() != zero_addr) {
+ // If the broadcast bit is set in the flags field, we have to
+ // send the response to broadcast address. Client may have requested it
+ // because it doesn't support reception of messages on the interface
+ // which doesn't have an address assigned. The other case when response
+ // must be broadcasted is when our server does not support responding
+ // directly to a client without address assigned.
+ bool bcast_flag = (question->getFlags() >> 0xF) ? true : false;
+ if (!IfaceMgr::instance().isDirectResponseSupported() || bcast_flag) {
+ msg->setRemoteAddr(bcast_addr);
+
+ // Client cleared the broadcast bit and we support direct responses
+ // so we should unicast the response to a newly allocated address -
+ // yiaddr.
+ } else {
+ msg->setRemoteAddr(question->getYiaddr());
+
+ }
+
+ // In most cases, we should have the remote address found already. If we
+ // found ourselves at this point, the rational thing to do is to respond
+ // to the address we got the query from.
+ } else {
+ msg->setRemoteAddr(question->getRemoteAddr());
+
+ }
+}
+
+
OptionPtr
Dhcpv4Srv::getNetmaskOption(const Subnet4Ptr& subnet) {
uint32_t netmask = getNetmask4(subnet->get().second);
diff --git a/src/bin/dhcp4/dhcp4_srv.h b/src/bin/dhcp4/dhcp4_srv.h
index 9d6bc96..2f89f3c 100644
--- a/src/bin/dhcp4/dhcp4_srv.h
+++ b/src/bin/dhcp4/dhcp4_srv.h
@@ -225,6 +225,23 @@ protected:
/// @param msg_type specifies message type
void appendDefaultOptions(Pkt4Ptr& msg, uint8_t msg_type);
+ /// @brief Sets remote addresses for outgoing packet.
+ ///
+ /// This method sets the local and remote addresses on outgoing packet.
+ /// The addresses being set depend on the following conditions:
+ /// - has incoming packet been relayed,
+ /// - is direct response to a client without address supported,
+ /// - type of the outgoing packet,
+ /// - broadcast flag set in the incoming packet.
+ ///
+ /// @warning This method does not check whether provided packet pointers
+ /// are valid. Make sure that pointers are correct before calling this
+ /// function.
+ ///
+ /// @param question instance of a packet received by a server.
+ /// @param [out] msg response packet which addresses are to be adjusted.
+ void adjustRemoteAddr(const Pkt4Ptr& question, Pkt4Ptr& msg);
+
/// @brief Returns server-identifier option
///
/// @return server-id option
diff --git a/src/bin/dhcp4/tests/dhcp4_srv_unittest.cc b/src/bin/dhcp4/tests/dhcp4_srv_unittest.cc
index 8591eef..ba1f1f0 100644
--- a/src/bin/dhcp4/tests/dhcp4_srv_unittest.cc
+++ b/src/bin/dhcp4/tests/dhcp4_srv_unittest.cc
@@ -72,6 +72,7 @@ public:
: Dhcpv4Srv(port, "type=memfile", false, false) {
}
+ using Dhcpv4Srv::adjustRemoteAddr;
using Dhcpv4Srv::processDiscover;
using Dhcpv4Srv::processRequest;
using Dhcpv4Srv::processRelease;
@@ -450,7 +451,7 @@ public:
}
- if (relay_addr.toText() != "0.0.0.0") {
+ /* if (relay_addr.toText() != "0.0.0.0") {
// This is relayed message. It should be sent brsp to relay address.
EXPECT_EQ(req->getGiaddr().toText(),
rsp->getRemoteAddr().toText());
@@ -475,7 +476,7 @@ public:
}
- }
+ } */
messageCheck(req, rsp);
@@ -514,6 +515,28 @@ public:
}
+ void testAdjustRemoteAddress(const uint8_t in_msg_type,
+ const uint8_t out_msg_type,
+ const std::string& giaddr,
+ const std::string& ciaddr,
+ const std::string& yiaddr,
+ const uint16_t flags) {
+ boost::scoped_ptr<NakedDhcpv4Srv> srv(new NakedDhcpv4Srv(0));
+
+ boost::shared_ptr<Pkt4> req(new Pkt4(in_msg_type, 1234));
+ req->setGiaddr(IOAddress(giaddr));
+ req->setCiaddr(IOAddress(ciaddr));
+ req->setFlags(flags);
+
+ boost::shared_ptr<Pkt4> resp(new Pkt4(out_msg_type, 1234));
+ resp->setYiaddr(IOAddress(yiaddr));
+
+ ASSERT_NO_THROW(srv->adjustRemoteAddr(req, resp));
+
+
+
+ }
+
~Dhcpv4SrvTest() {
CfgMgr::instance().deleteSubnets4();
@@ -558,6 +581,22 @@ TEST_F(Dhcpv4SrvTest, basic) {
delete naked_srv;
}
+TEST_F(Dhcpv4SrvTest, adjustRemoteAddressRelay) {
+ boost::scoped_ptr<NakedDhcpv4Srv> srv(new NakedDhcpv4Srv(0));
+
+ boost::shared_ptr<Pkt4> req(new Pkt4(DHCPDISCOVER, 1234));
+ req->setGiaddr(IOAddress("192.0.2.1"));
+ req->setCiaddr(IOAddress("0.0.0.0"));
+ req->setFlags(flags);
+
+ boost::shared_ptr<Pkt4> resp(new Pkt4(OFFER, 1234));
+ resp->setYiaddr(IOAddress("192.0.2.100"));
+
+ ASSERT_NO_THROW(srv->adjustRemoteAddr(req, resp));
+
+ EXPECT_EQ("192.0.2.1", resp->getRemoteAddr().toText());
+}
+
// Verifies that DISCOVER received via relay can be processed correctly,
// that the OFFER message generated in response is valid and
// contains necessary options.
More information about the bind10-changes
mailing list