From: Francis Dupont Date: Wed, 7 Feb 2018 14:36:44 +0000 (+0100) Subject: [5390] Simplified the logic (addressed comment) X-Git-Tag: trac5502_base~3^2~1 X-Git-Url: http://git.ipfire.org/gitweb/gitweb.cgi?a=commitdiff_plain;h=8af68d76eec727db973bee95bec5eeb361eaea22;p=thirdparty%2Fkea.git [5390] Simplified the logic (addressed comment) --- diff --git a/doc/guide/dhcp4-srv.xml b/doc/guide/dhcp4-srv.xml index 9df4dd62a8..66a39e6cb2 100644 --- a/doc/guide/dhcp4-srv.xml +++ b/doc/guide/dhcp4-srv.xml @@ -773,8 +773,8 @@ temporarily override a list of interface names and listen on all interfaces. Usually loopback interfaces (e.g. the "lo" or "lo0" interface) - may not configured but if only one interface is configured and - IP/UDP sockets are specified a loopback interface is accepted. + may not configured but if a loopback interface is explicitely configured + and IP/UDP sockets are specified the loopback interface is accepted. It can be used for instance to run Kea in a FreeBSD jail having diff --git a/doc/guide/dhcp6-srv.xml b/doc/guide/dhcp6-srv.xml index 3ff642e1d6..c26b7ab205 100644 --- a/doc/guide/dhcp6-srv.xml +++ b/doc/guide/dhcp6-srv.xml @@ -655,8 +655,8 @@ temporarily override a list of interface names and listen on all interfaces. Usually loopback interfaces (e.g. the "lo" or "lo0" interface) - may not configured but if only one interface is configured a loopback - interface is accepted. Note Kea requires link-local address which does + may not configured but if a loopback interface is explicitely configured + it is accepted. Note Kea requires a link-local address which does not exist on all systems, or a specified unicast address as in: diff --git a/src/lib/dhcpsrv/cfg_iface.cc b/src/lib/dhcpsrv/cfg_iface.cc index 540ccac0bf..e6ba36f44e 100644 --- a/src/lib/dhcpsrv/cfg_iface.cc +++ b/src/lib/dhcpsrv/cfg_iface.cc @@ -57,38 +57,22 @@ CfgIface::openSockets(const uint16_t family, const uint16_t port, // of the IfaceMgr. Those modifications require that sockets are closed. closeSockets(); // The loopback interface can be used only when: - // - wildcard is not used // - UDP socket will be used, i.e. not IPv4 and RAW socket - // - there is one interface name only in the interface set - // and this interface is a loopback interface. - // - or the interface set is empty and all interfaces in the address - // map are the same and a loopback interface. + // - the loopback interface is in the interface set or the address map. bool loopback_used_ = false; - if (!wildcard_used_ && - ((family == AF_INET6) || (socket_type_ == SOCKET_UDP)) && - (iface_set_.size() == 1) && - (address_map_.empty())) { - // Get the first and only interface. - IfacePtr iface = IfaceMgr::instance().getIface(*iface_set_.begin()); - if (iface && iface->flag_loopback_) { - loopback_used_ = true; + if ((family == AF_INET6) || (socket_type_ == SOCKET_UDP)) { + // Check interface set + for (IfaceSet::const_iterator iface_name = iface_set_.begin(); + iface_name != iface_set_.end(); ++iface_name) { + IfacePtr iface = IfaceMgr::instance().getIface(*iface_name); + if (iface && iface->flag_loopback_) { + loopback_used_ = true; + } } - } else if (!wildcard_used_ && - ((family == AF_INET6) || (socket_type_ == SOCKET_UDP)) && - iface_set_.empty() && - !address_map_.empty()) { - // Get the first interface - const std::string& name = address_map_.begin()->first; - bool same = true; + // Check address map for (ExplicitAddressMap::const_iterator unicast = address_map_.begin(); unicast != address_map_.end(); ++unicast) { - if (unicast->first != name) { - same = false; - break; - } - } - if (same) { - IfacePtr iface = IfaceMgr::instance().getIface(name); + IfacePtr iface = IfaceMgr::instance().getIface(unicast->first); if (iface && iface->flag_loopback_) { loopback_used_ = true; } @@ -99,7 +83,7 @@ CfgIface::openSockets(const uint16_t family, const uint16_t port, // interface names specified by the user. If wildcard interface was // specified, mark all interfaces active. Mark loopback inactive when // not explicitely allowed. - setState(family, !wildcard_used_, loopback_used_); + setState(family, !wildcard_used_, !loopback_used_); IfaceMgr& iface_mgr = IfaceMgr::instance(); // Remove selection of unicast addresses from all interfaces. iface_mgr.clearUnicasts(); diff --git a/src/lib/dhcpsrv/tests/cfg_iface_unittest.cc b/src/lib/dhcpsrv/tests/cfg_iface_unittest.cc index 4246fc735e..25c36056b2 100644 --- a/src/lib/dhcpsrv/tests/cfg_iface_unittest.cc +++ b/src/lib/dhcpsrv/tests/cfg_iface_unittest.cc @@ -209,8 +209,10 @@ TEST_F(CfgIfaceTest, explicitLoopbackV4) { ASSERT_NO_THROW(cfg.use(AF_INET, "lo")); ASSERT_NO_THROW(cfg.useSocketType(AF_INET, CfgIface::SOCKET_UDP)); cfg.openSockets(AF_INET, DHCP4_SERVER_PORT); - // No loopback socket - EXPECT_FALSE(socketOpen("lo", "127.0.0.1")); + // No wildcard is no longer a constraint + EXPECT_TRUE(socketOpen("lo", "127.0.0.1")); + cfg.closeSockets(); + ASSERT_FALSE(socketOpen("lo", "127.0.0.1")); // Retry without UDP sockets cfg.reset(); @@ -225,31 +227,21 @@ TEST_F(CfgIfaceTest, explicitLoopbackV4) { ASSERT_NO_THROW(cfg.use(AF_INET, "lo")); ASSERT_NO_THROW(cfg.useSocketType(AF_INET, CfgIface::SOCKET_UDP)); cfg.openSockets(AF_INET, DHCP4_SERVER_PORT); - // No loopback socket + // Only loopback is no longer a constraint + EXPECT_TRUE(socketOpen("lo", "127.0.0.1")); + cfg.closeSockets(); EXPECT_FALSE(socketOpen("lo", "127.0.0.1")); - // Finally with a second interface and address + // Finally with interfaces and addresses cfg.reset(); ASSERT_NO_THROW(cfg.use(AF_INET, "eth0/10.0.0.1")); - ASSERT_NO_THROW(cfg.use(AF_INET, "lo")); + ASSERT_NO_THROW(cfg.use(AF_INET, "lo/127.0.0.1")); ASSERT_NO_THROW(cfg.useSocketType(AF_INET, CfgIface::SOCKET_UDP)); cfg.openSockets(AF_INET, DHCP4_SERVER_PORT); - // No loopback socket - EXPECT_FALSE(socketOpen("lo", "127.0.0.1")); -} - -// Tests that the loopback socket is not opened in raw mode. -TEST_F(CfgIfaceTest, explicitLoopbackV4Negative) { - CfgIface cfg; - ASSERT_NO_THROW(cfg.use(AF_INET, "lo")); - - // Use UDP sockets - ASSERT_NO_THROW(cfg.useSocketType(AF_INET, CfgIface::SOCKET_RAW)); - - // Open sockets on specified interfaces and addresses. - cfg.openSockets(AF_INET, DHCP4_SERVER_PORT); + // Only loopback is no longer a constraint + EXPECT_TRUE(socketOpen("lo", "127.0.0.1")); + cfg.closeSockets(); EXPECT_FALSE(socketOpen("lo", "127.0.0.1")); - EXPECT_FALSE(socketOpen("lo", AF_INET)); } // This test checks that the interface names can be explicitly selected @@ -291,20 +283,6 @@ TEST_F(CfgIfaceTest, explicitNamesV6) { EXPECT_FALSE(socketOpen("lo", AF_INET6)); } -// This test checks that the loopback socket can be opened. -TEST_F(CfgIfaceTest, loopback6) { - CfgIface cfg; - // Specify valid interface names. There should be no error. - ASSERT_NO_THROW(cfg.use(AF_INET6, "lo")); - - // Open sockets on specified interfaces. - cfg.openSockets(AF_INET6, DHCP6_SERVER_PORT); - - // There should be a socket open. - EXPECT_FALSE(socketOpen("lo", AF_INET6)); - EXPECT_FALSE(socketOpen("lo", "::1")); -} - // This test checks that the wildcard interface name can be specified to // select all interfaces to open IPv4 sockets. TEST_F(CfgIfaceTest, wildcardV4) { @@ -403,24 +381,30 @@ TEST_F(CfgIfaceTest, explicitLoopbackV6) { ASSERT_NO_THROW(cfg.use(AF_INET6, "*")); ASSERT_NO_THROW(cfg.use(AF_INET6, "lo/::1")); cfg.openSockets(AF_INET6, DHCP6_SERVER_PORT); - // No loopback socket - EXPECT_FALSE(socketOpen("lo", AF_INET6)); + // No wildcard is no longer a constraint + EXPECT_TRUE(socketOpen("lo", AF_INET6)); + cfg.closeSockets(); + ASSERT_FALSE(socketOpen("lo", AF_INET6)); // Retry with a second interface cfg.reset(); ASSERT_NO_THROW(cfg.use(AF_INET6, "eth0")); ASSERT_NO_THROW(cfg.use(AF_INET6, "lo/::1")); cfg.openSockets(AF_INET6, DHCP6_SERVER_PORT); - // No loopback socket - EXPECT_FALSE(socketOpen("lo", AF_INET6)); + // Only loopback is no longer a constraint + EXPECT_TRUE(socketOpen("lo", AF_INET6)); + cfg.closeSockets(); + ASSERT_FALSE(socketOpen("lo", AF_INET6)); - // Finally with a second interface and address + // Finally with interfaces and addresses cfg.reset(); ASSERT_NO_THROW(cfg.use(AF_INET6, "eth0/2001:db8:1::1")); ASSERT_NO_THROW(cfg.use(AF_INET6, "lo/::1")); cfg.openSockets(AF_INET6, DHCP6_SERVER_PORT); - // No loopback socket - EXPECT_FALSE(socketOpen("lo", AF_INET6)); + // Only loopback is no longer a constraint + EXPECT_TRUE(socketOpen("lo", AF_INET6)); + cfg.closeSockets(); + ASSERT_FALSE(socketOpen("lo", AF_INET6)); } // Test that the equality and inequality operators work fine for CfgIface.