From: Marcin Siodelski Date: Tue, 21 May 2013 13:18:58 +0000 (+0200) Subject: [2902] Adjust the destination address for response. X-Git-Tag: bind10-1.2.0beta1-release~440^2~5 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=e7a483b3f53323d03b64f49385d173ceb595c39c;p=thirdparty%2Fkea.git [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. --- diff --git a/src/bin/dhcp4/dhcp4_srv.cc b/src/bin/dhcp4/dhcp4_srv.cc index 3eb187bfa7..f7881971c3 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 9d6bc96aa4..2f89f3c175 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 8591eefb62..ba1f1f0fcc 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); @@ -512,6 +513,28 @@ public: // Check that the requested options are returned. optionsCheck(rsp); + } + + 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 srv(new NakedDhcpv4Srv(0)); + + boost::shared_ptr req(new Pkt4(in_msg_type, 1234)); + req->setGiaddr(IOAddress(giaddr)); + req->setCiaddr(IOAddress(ciaddr)); + req->setFlags(flags); + + boost::shared_ptr resp(new Pkt4(out_msg_type, 1234)); + resp->setYiaddr(IOAddress(yiaddr)); + + ASSERT_NO_THROW(srv->adjustRemoteAddr(req, resp)); + + + } ~Dhcpv4SrvTest() { @@ -558,6 +581,22 @@ TEST_F(Dhcpv4SrvTest, basic) { delete naked_srv; } +TEST_F(Dhcpv4SrvTest, adjustRemoteAddressRelay) { + boost::scoped_ptr srv(new NakedDhcpv4Srv(0)); + + boost::shared_ptr 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 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.