From: Francis Dupont Date: Fri, 11 Oct 2019 12:46:00 +0000 (+0200) Subject: [677-kea4-not-honouring-the-p-switch] Improved -p support X-Git-Tag: Kea-1.7.1~45 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=47e709fb7b89464d4473e8c7cb138f539042d68b;p=thirdparty%2Fkea.git [677-kea4-not-honouring-the-p-switch] Improved -p support --- diff --git a/src/bin/dhcp4/dhcp4_srv.cc b/src/bin/dhcp4/dhcp4_srv.cc index 456c6d9d89..1b4b155561 100644 --- a/src/bin/dhcp4/dhcp4_srv.cc +++ b/src/bin/dhcp4/dhcp4_srv.cc @@ -466,7 +466,7 @@ const std::string Dhcpv4Srv::VENDOR_CLASS_PREFIX("VENDOR_CLASS_"); Dhcpv4Srv::Dhcpv4Srv(uint16_t server_port, uint16_t client_port, const bool use_bcast, const bool direct_response_desired) : io_service_(new IOService()), shutdown_(true), alloc_engine_(), - server_port_(server_port), use_bcast_(use_bcast), + use_bcast_(use_bcast), server_port_(server_port), client_port_(client_port), network_state_(new NetworkState(NetworkState::DHCPv4)), cb_control_(new CBControlDHCPv4()) { @@ -2461,7 +2461,11 @@ Dhcpv4Srv::adjustIfaceData(Dhcpv4Exchange& ex) { response->setIface(query->getIface()); } - response->setLocalPort(DHCP4_SERVER_PORT); + if (server_port_) { + response->setLocalPort(server_port_); + } else { + response->setLocalPort(DHCP4_SERVER_PORT); + } } void diff --git a/src/bin/dhcp4/dhcp4_srv.h b/src/bin/dhcp4/dhcp4_srv.h index 490387a924..13265a5e29 100644 --- a/src/bin/dhcp4/dhcp4_srv.h +++ b/src/bin/dhcp4/dhcp4_srv.h @@ -984,14 +984,14 @@ private: /// @return Option that contains netmask information static OptionPtr getNetmaskOption(const Subnet4Ptr& subnet); - /// UDP port number on which server listens. - uint16_t server_port_; - /// Should broadcast be enabled on sockets (if true). bool use_bcast_; protected: + /// UDP port number on which server listens. + uint16_t server_port_; + /// UDP port number to which server sends responses. uint16_t client_port_; diff --git a/src/bin/dhcp4/tests/dhcp4_srv_unittest.cc b/src/bin/dhcp4/tests/dhcp4_srv_unittest.cc index 07e6fe6d9d..da1621bae3 100644 --- a/src/bin/dhcp4/tests/dhcp4_srv_unittest.cc +++ b/src/bin/dhcp4/tests/dhcp4_srv_unittest.cc @@ -200,8 +200,9 @@ TEST_F(Dhcpv4SrvTest, adjustIfaceDataRelay) { // Clear remote address. resp->setRemoteAddr(IOAddress("0.0.0.0")); - // Set the client port. + // Set the client and server ports. srv_.client_port_ = 1234; + srv_.server_port_ = 2345; ASSERT_NO_THROW(srv_.adjustIfaceData(ex)); @@ -210,6 +211,9 @@ TEST_F(Dhcpv4SrvTest, adjustIfaceDataRelay) { // Remote port was enforced to the client port. EXPECT_EQ(srv_.client_port_, resp->getRemotePort()); + + // Local port was enforced to the server port. + EXPECT_EQ(srv_.server_port_, resp->getLocalPort()); } // This test verifies that the remote port is adjusted when @@ -2125,6 +2129,48 @@ TEST_F(Dhcpv4SrvTest, portsClientPort) { EXPECT_EQ(srv.client_port_, offer->getRemotePort()); } +// Checks if server port can be overridden in packets being sent. +TEST_F(Dhcpv4SrvTest, portsServerPort) { + IfaceMgrTestConfig test_config(true); + IfaceMgr::instance().openSockets4(); + + // Do not use DHCP4_SERVER_PORT here as 0 means don't open sockets. + NakedDhcpv4Srv srv(0); + EXPECT_EQ(0, srv.server_port_); + + // Use of the captured DHCPDISCOVER packet requires that + // subnet 10.254.226.0/24 is in use, because this packet + // contains the giaddr which belongs to this subnet and + // this giaddr is used to select the subnet + configure(CONFIGS[0]); + srv.server_port_ = 1234; + + // Let's create a relayed DISCOVER. This particular relayed DISCOVER has + // added option 82 (relay agent info) with 3 suboptions. The server + // is supposed to echo it back in its response. + Pkt4Ptr dis; + ASSERT_NO_THROW(dis = PktCaptures::captureRelayedDiscover()); + + // Simulate that we have received that traffic + srv.fakeReceive(dis); + + // Server will now process to run its normal loop, but instead of calling + // IfaceMgr::receive4(), it will read all packets from the list set by + // fakeReceive() + // In particular, it should call registered buffer4_receive callback. + srv.run(); + + // Check that the server did send a response + ASSERT_EQ(1, srv.fake_sent_.size()); + + // Make sure that we received a response + Pkt4Ptr offer = srv.fake_sent_.front(); + ASSERT_TRUE(offer); + + // Get Relay Agent Info from query... + EXPECT_EQ(srv.server_port_, offer->getLocalPort()); +} + /// @todo Implement tests for subnetSelect See tests in dhcp6_srv_unittest.cc: /// selectSubnetAddr, selectSubnetIface, selectSubnetRelayLinkaddr, /// selectSubnetRelayInterfaceId. Note that the concept of interface-id is not diff --git a/src/bin/dhcp4/tests/dhcp4_test_utils.h b/src/bin/dhcp4/tests/dhcp4_test_utils.h index 433db9f7b9..231655b42b 100644 --- a/src/bin/dhcp4/tests/dhcp4_test_utils.h +++ b/src/bin/dhcp4/tests/dhcp4_test_utils.h @@ -229,6 +229,7 @@ public: using Dhcpv4Srv::VENDOR_CLASS_PREFIX; using Dhcpv4Srv::shutdown_; using Dhcpv4Srv::alloc_engine_; + using Dhcpv4Srv::server_port_; using Dhcpv4Srv::client_port_; }; diff --git a/src/bin/dhcp6/dhcp6_srv.cc b/src/bin/dhcp6/dhcp6_srv.cc index 6f9236ac64..d1e02821ae 100644 --- a/src/bin/dhcp6/dhcp6_srv.cc +++ b/src/bin/dhcp6/dhcp6_srv.cc @@ -835,7 +835,11 @@ Dhcpv6Srv::processPacket(Pkt6Ptr& query, Pkt6Ptr& rsp) { rsp->setRemotePort(relay_port ? relay_port : DHCP6_SERVER_PORT); } - rsp->setLocalPort(DHCP6_SERVER_PORT); + if (server_port_) { + rsp->setLocalPort(server_port_); + } else { + rsp->setLocalPort(DHCP6_SERVER_PORT); + } rsp->setIndex(query->getIndex()); rsp->setIface(query->getIface()); diff --git a/src/bin/dhcp6/dhcp6_srv.h b/src/bin/dhcp6/dhcp6_srv.h index 248e4c3eb8..80e42a47b1 100644 --- a/src/bin/dhcp6/dhcp6_srv.h +++ b/src/bin/dhcp6/dhcp6_srv.h @@ -999,10 +999,10 @@ private: /// @return true if option has been requested in the ORO. bool requestedInORO(const Pkt6Ptr& query, const uint16_t code) const; +protected: /// UDP port number on which server listens. uint16_t server_port_; -protected: /// UDP port number to which server sends all responses. uint16_t client_port_; diff --git a/src/bin/dhcp6/kea-dhcp6.rst b/src/bin/dhcp6/kea-dhcp6.rst index f8d1430b18..5086315f8c 100644 --- a/src/bin/dhcp6/kea-dhcp6.rst +++ b/src/bin/dhcp6/kea-dhcp6.rst @@ -3,7 +3,7 @@ kea-dhcp6 Kea kea-dhcp6 DHCPv6 server in Kea -2011-2018 +2011-2019 Internet Systems Consortium, Inc. ("ISC") kea-dhcp6 -v @@ -16,6 +16,8 @@ config-file config-file -p server-port-number +-P +client-port-number DESCRIPTION =========== @@ -52,6 +54,10 @@ The arguments are as follows: Server port number (1-65535) on which the server listens. This is useful for testing purposes only. +``-P`` + Client port number (1-65535) to which the server responds. This is + useful for testing purposes only. + DOCUMENTATION ============= diff --git a/src/bin/dhcp6/kea-dhcp6.xml b/src/bin/dhcp6/kea-dhcp6.xml index da10304760..eae41f2745 100644 --- a/src/bin/dhcp6/kea-dhcp6.xml +++ b/src/bin/dhcp6/kea-dhcp6.xml @@ -9,8 +9,8 @@ ISC Kea - Aug 28, 2019 - 1.6.0 + Oct 11, 2019 + 1.7.1 The Kea software has been written by a number of engineers working for ISC: Tomek Mrugalski, Stephen Morris, Marcin @@ -49,6 +49,7 @@ + @@ -123,6 +124,14 @@ + + + + Client port number (1-65535) to which the server responds. + This is useful for testing purposes only. + + + diff --git a/src/bin/dhcp6/tests/dhcp6_srv_unittest.cc b/src/bin/dhcp6/tests/dhcp6_srv_unittest.cc index e8062b43ac..4e6cbb1fec 100644 --- a/src/bin/dhcp6/tests/dhcp6_srv_unittest.cc +++ b/src/bin/dhcp6/tests/dhcp6_srv_unittest.cc @@ -1967,6 +1967,36 @@ TEST_F(Dhcpv6SrvTest, portsClientPort) { EXPECT_EQ(srv.client_port_, adv->getRemotePort()); } +// Checks if server responses are sent to the proper port. +TEST_F(Dhcpv6SrvTest, portsServerPort) { + + // Create the test server in test mode. + NakedDhcpv6Srv srv(0); + + // Enforce a specific server port value. + EXPECT_EQ(0, srv.server_port_); + srv.server_port_ = 1234; + + // Let's create a simple SOLICIT + Pkt6Ptr sol = PktCaptures::captureSimpleSolicit(); + + // Simulate that we have received that traffic + srv.fakeReceive(sol); + + // Server will now process to run its normal loop, but instead of calling + // IfaceMgr::receive6(), it will read all packets from the list set by + // fakeReceive() + srv.run(); + + // Get Advertise... + ASSERT_FALSE(srv.fake_sent_.empty()); + Pkt6Ptr adv = srv.fake_sent_.front(); + ASSERT_TRUE(adv); + + // Verify the local port: it must be the server port. + EXPECT_EQ(srv.server_port_, adv->getLocalPort()); +} + // Checks if server responses are sent to the proper port. TEST_F(Dhcpv6SrvTest, portsDirectTraffic) { diff --git a/src/bin/dhcp6/tests/dhcp6_test_utils.h b/src/bin/dhcp6/tests/dhcp6_test_utils.h index 8ae6b80029..d295bb7234 100644 --- a/src/bin/dhcp6/tests/dhcp6_test_utils.h +++ b/src/bin/dhcp6/tests/dhcp6_test_utils.h @@ -272,6 +272,7 @@ public: using Dhcpv6Srv::name_change_reqs_; using Dhcpv6Srv::VENDOR_CLASS_PREFIX; using Dhcpv6Srv::initContext; + using Dhcpv6Srv::server_port_; using Dhcpv6Srv::client_port_; /// @brief packets we pretend to receive