BIND 10 master, updated. 805d2b269c6bf3e7be68c13f1da1709d8150a666 [master] Merge branch 'trac3279'
BIND 10 source code commits
bind10-changes at lists.isc.org
Wed Jan 15 10:51:31 UTC 2014
The branch, master has been updated
via 805d2b269c6bf3e7be68c13f1da1709d8150a666 (commit)
via aad427b882473bdfe968db775858f47b6829d61b (commit)
via 60ee84646638336b46c253ba1a91589094eac28a (commit)
via 6d5522801eb392e991a4ecfc67be59e0ea7763b6 (commit)
via c716f4335d644744d16fa624e2b14dcf7bf217a4 (commit)
via 0d08bdc79a71a209b13a793d95a8ff4b4bea9d51 (commit)
via eb73d9223bbb17aed86eabaf29e62f9e18ac089d (commit)
via 779a230985d1d16af1f30f1948421bbdacb59521 (commit)
from cb0080e355e2a8848e275b206354a4e516b9d661 (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 805d2b269c6bf3e7be68c13f1da1709d8150a666
Merge: cb0080e aad427b
Author: Marcin Siodelski <marcin at isc.org>
Date: Wed Jan 15 11:36:07 2014 +0100
[master] Merge branch 'trac3279'
Conflicts:
src/lib/dhcp/iface_mgr.cc
src/lib/dhcp/iface_mgr.h
src/lib/dhcp/tests/iface_mgr_unittest.cc
-----------------------------------------------------------------------
Summary of changes:
src/bin/dhcp4/dhcp4_messages.mes | 9 ++-
src/bin/dhcp4/dhcp4_srv.cc | 62 +++++++++++++++++-
src/bin/dhcp4/dhcp4_srv.h | 15 +++++
src/bin/dhcp4/tests/dhcp4_srv_unittest.cc | 52 ++++++++++++++-
src/bin/dhcp4/tests/dhcp4_test_utils.h | 1 +
src/lib/dhcp/iface_mgr.cc | 20 +++++-
src/lib/dhcp/iface_mgr.h | 27 +++++---
src/lib/dhcp/tests/iface_mgr_unittest.cc | 99 +++++++++++++++++++++--------
8 files changed, 247 insertions(+), 38 deletions(-)
-----------------------------------------------------------------------
diff --git a/src/bin/dhcp4/dhcp4_messages.mes b/src/bin/dhcp4/dhcp4_messages.mes
index f561695..4fba57a 100644
--- a/src/bin/dhcp4/dhcp4_messages.mes
+++ b/src/bin/dhcp4/dhcp4_messages.mes
@@ -1,4 +1,4 @@
-# Copyright (C) 2012-2013 Internet Systems Consortium, Inc. ("ISC")
+# Copyright (C) 2012-2014 Internet Systems Consortium, Inc. ("ISC")
#
# Permission to use, copy, modify, and/or distribute this software for any
# purpose with or without fee is hereby granted, provided that the above
@@ -175,6 +175,13 @@ IPv4 DHCP server but it is not running.
A debug message issued during startup, this indicates that the IPv4 DHCP
server is about to open sockets on the specified port.
+% DHCP4_PACKET_NOT_FOR_US received DHCPv4 message (transid=%1, iface=%2) dropped because it contains foreign server identifier
+This debug message is issued when received DHCPv4 message is dropped because
+it is addressed to a different server, i.e. a server identifier held by
+this message doesn't match the identifier used by our server. The arguments
+of this message hold the name of the transaction id and interface on which
+the message has been received.
+
% DHCP4_OPEN_SOCKET_FAIL failed to create socket: %1
A warning message issued when IfaceMgr fails to open and bind a socket. The reason
for the failure is appended as an argument of the log message.
diff --git a/src/bin/dhcp4/dhcp4_srv.cc b/src/bin/dhcp4/dhcp4_srv.cc
index 39dde03..22ab653 100644
--- a/src/bin/dhcp4/dhcp4_srv.cc
+++ b/src/bin/dhcp4/dhcp4_srv.cc
@@ -1,4 +1,4 @@
-// Copyright (C) 2011-2013 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2011-2014 Internet Systems Consortium, Inc. ("ISC")
//
// Permission to use, copy, modify, and/or distribute this software for any
// purpose with or without fee is hereby granted, provided that the above
@@ -259,6 +259,15 @@ Dhcpv4Srv::run() {
}
}
+ // Check if the DHCPv4 packet has been sent to us or to someone else.
+ // If it hasn't been sent to us, drop it!
+ if (!acceptServerId(query)) {
+ LOG_DEBUG(dhcp4_logger, DBG_DHCP4_DETAIL, DHCP4_PACKET_NOT_FOR_US)
+ .arg(query->getTransid())
+ .arg(query->getIface());
+ continue;
+ }
+
// When receiving a packet without message type option, getType() will
// throw. Let's set type to -1 as default error indicator.
int type = -1;
@@ -1525,6 +1534,57 @@ Dhcpv4Srv::selectSubnet(const Pkt4Ptr& question) {
return (subnet);
}
+bool
+Dhcpv4Srv::acceptServerId(const Pkt4Ptr& pkt) const {
+ // This function is meant to be called internally by the server class, so
+ // we rely on the caller to sanity check the pointer and we don't check
+ // it here.
+
+ // Check if server identifier option is present. If it is not present
+ // we accept the message because it is targetted to all servers.
+ // Note that we don't check cases that server identifier is mandatory
+ // but not present. This is meant to be sanity checked in other
+ // functions.
+ OptionPtr option = pkt->getOption(DHO_DHCP_SERVER_IDENTIFIER);
+ if (!option) {
+ return (true);
+ }
+ // Server identifier is present. Let's convert it to 4-byte address
+ // and try to match with server identifiers used by the server.
+ OptionCustomPtr option_custom =
+ boost::dynamic_pointer_cast<OptionCustom>(option);
+ // Unable to convert the option to the option type which encapsulates it.
+ // We treat this as non-matching server id.
+ if (!option_custom) {
+ return (false);
+ }
+ // The server identifier option should carry exactly one IPv4 address.
+ // If the option definition for the server identifier doesn't change,
+ // the OptionCustom object should have exactly one IPv4 address and
+ // this check is somewhat redundant. On the other hand, if someone
+ // breaks option it may be better to check that here.
+ if (option_custom->getDataFieldsNum() != 1) {
+ return (false);
+ }
+
+ // The server identifier MUST be an IPv4 address. If given address is
+ // v6, it is wrong.
+ IOAddress server_id = option_custom->readAddress();
+ if (!server_id.isV4()) {
+ return (false);
+ }
+
+ // This function iterates over all interfaces on which the
+ // server is listening to find the one which has a socket bound
+ // to the address carried in the server identifier option.
+ // This has some performance implications. However, given that
+ // typically there will be just a few active interfaces the
+ // performance hit should be acceptable. If it turns out to
+ // be significant, we will have to cache server identifiers
+ // when sockets are opened.
+ return (IfaceMgr::instance().hasOpenSocket(server_id));
+}
+
void
Dhcpv4Srv::sanityCheck(const Pkt4Ptr& pkt, RequirementLevel serverid) {
OptionPtr server_id = pkt->getOption(DHO_DHCP_SERVER_IDENTIFIER);
diff --git a/src/bin/dhcp4/dhcp4_srv.h b/src/bin/dhcp4/dhcp4_srv.h
index 324ba4f..04e622b 100644
--- a/src/bin/dhcp4/dhcp4_srv.h
+++ b/src/bin/dhcp4/dhcp4_srv.h
@@ -167,6 +167,21 @@ public:
protected:
+ /// @brief Verifies if the server id belongs to our server.
+ ///
+ /// This function checks if the server identifier carried in the specified
+ /// DHCPv4 message belongs to this server. If the server identifier option
+ /// is absent or the value carried by this option is equal to one of the
+ /// server identifiers used by the server, the true is returned. If the
+ /// server identifier option is present, but it doesn't match any server
+ /// identifier used by this server, the false value is returned.
+ ///
+ /// @param pkt DHCPv4 message which server identifier is to be checked.
+ ///
+ /// @return true, if the server identifier is absent or matches one of the
+ /// server identifiers that the server is using; false otherwise.
+ bool acceptServerId(const Pkt4Ptr& pkt) const;
+
/// @brief verifies if specified packet meets RFC requirements
///
/// Checks if mandatory option is really there, that forbidden option
diff --git a/src/bin/dhcp4/tests/dhcp4_srv_unittest.cc b/src/bin/dhcp4/tests/dhcp4_srv_unittest.cc
index 5142c70..057847c 100644
--- a/src/bin/dhcp4/tests/dhcp4_srv_unittest.cc
+++ b/src/bin/dhcp4/tests/dhcp4_srv_unittest.cc
@@ -1,4 +1,4 @@
-// Copyright (C) 2011-2013 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2011-2014 Internet Systems Consortium, Inc. ("ISC")
//
// Permission to use, copy, modify, and/or distribute this software for any
// purpose with or without fee is hereby granted, provided that the above
@@ -1031,6 +1031,56 @@ TEST_F(Dhcpv4SrvFakeIfaceTest, RenewBasic) {
EXPECT_TRUE(LeaseMgrFactory::instance().deleteLease(addr));
}
+// This test verifies that the logic which matches server identifier in the
+// received message with server identifiers used by a server works correctly:
+// - a message with no server identifier is accepted,
+// - a message with a server identifier which matches one of the server
+// identifiers used by a server is accepted,
+// - a message with a server identifier which doesn't match any server
+// identifier used by a server, is not accepted.
+TEST_F(Dhcpv4SrvFakeIfaceTest, acceptServerId) {
+ NakedDhcpv4Srv srv(0);
+
+ Pkt4Ptr pkt(new Pkt4(DHCPREQUEST, 1234));
+ // If no server identifier option is present, the message is always
+ // accepted.
+ EXPECT_TRUE(srv.acceptServerId(pkt));
+
+ // Create definition of the server identifier option.
+ OptionDefinition def("server-identifier", DHO_DHCP_SERVER_IDENTIFIER,
+ "ipv4-address", false);
+
+ // Add a server identifier option which doesn't match server ids being
+ // used by the server. The accepted server ids are the IPv4 addresses
+ // configured on the interfaces. The 10.1.2.3 is not configured on
+ // any interfaces.
+ OptionCustomPtr other_serverid(new OptionCustom(def, Option::V6));
+ other_serverid->writeAddress(IOAddress("10.1.2.3"));
+ pkt->addOption(other_serverid);
+ EXPECT_FALSE(srv.acceptServerId(pkt));
+
+ // Remove the server identifier.
+ ASSERT_NO_THROW(pkt->delOption(DHO_DHCP_SERVER_IDENTIFIER));
+
+ // Add a server id being an IPv4 address configured on eth0 interface.
+ // A DHCPv4 message holding this server identifier should be accepted.
+ OptionCustomPtr eth0_serverid(new OptionCustom(def, Option::V6));
+ eth0_serverid->writeAddress(IOAddress("192.0.3.1"));
+ ASSERT_NO_THROW(pkt->addOption(eth0_serverid));
+ EXPECT_TRUE(srv.acceptServerId(pkt));
+
+ // Remove the server identifier.
+ ASSERT_NO_THROW(pkt->delOption(DHO_DHCP_SERVER_IDENTIFIER));
+
+ // Add a server id being an IPv4 address configured on eth1 interface.
+ // A DHCPv4 message holding this server identifier should be accepted.
+ OptionCustomPtr eth1_serverid(new OptionCustom(def, Option::V6));
+ eth1_serverid->writeAddress(IOAddress("10.0.0.1"));
+ ASSERT_NO_THROW(pkt->addOption(eth1_serverid));
+ EXPECT_TRUE(srv.acceptServerId(pkt));
+
+}
+
// @todo: Implement tests for rejecting renewals
// This test verifies if the sanityCheck() really checks options presence.
diff --git a/src/bin/dhcp4/tests/dhcp4_test_utils.h b/src/bin/dhcp4/tests/dhcp4_test_utils.h
index ebb6300..65ae312 100644
--- a/src/bin/dhcp4/tests/dhcp4_test_utils.h
+++ b/src/bin/dhcp4/tests/dhcp4_test_utils.h
@@ -438,6 +438,7 @@ public:
using Dhcpv4Srv::processClientName;
using Dhcpv4Srv::computeDhcid;
using Dhcpv4Srv::createNameChangeRequests;
+ using Dhcpv4Srv::acceptServerId;
using Dhcpv4Srv::sanityCheck;
using Dhcpv4Srv::srvidToString;
using Dhcpv4Srv::unpackOptions;
diff --git a/src/lib/dhcp/iface_mgr.cc b/src/lib/dhcp/iface_mgr.cc
index d97614b..8624c69 100644
--- a/src/lib/dhcp/iface_mgr.cc
+++ b/src/lib/dhcp/iface_mgr.cc
@@ -1,4 +1,4 @@
-// Copyright (C) 2011-2014 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2011-2014 Internet Systems Consortium, Inc. ("ISC")
//
// Permission to use, copy, modify, and/or distribute this software for any
// purpose with or without fee is hereby granted, provided that the above
@@ -321,6 +321,24 @@ IfaceMgr::hasOpenSocket(const uint16_t family) const {
return (false);
}
+bool
+IfaceMgr::hasOpenSocket(const IOAddress& addr) const {
+ // Iterate over all interfaces and search for open sockets.
+ for (IfaceCollection::const_iterator iface = ifaces_.begin();
+ iface != ifaces_.end(); ++iface) {
+ const Iface::SocketCollection& sockets = iface->getSockets();
+ for (Iface::SocketCollection::const_iterator sock = sockets.begin();
+ sock != sockets.end(); ++sock) {
+ // Check if the socket address matches the specified address.
+ if (sock->addr_ == addr) {
+ return (true);
+ }
+ }
+ }
+ // There are no open sockets found for the specified family.
+ return (false);
+}
+
void IfaceMgr::stubDetectIfaces() {
string ifaceName;
const string v4addr("127.0.0.1"), v6addr("::1");
diff --git a/src/lib/dhcp/iface_mgr.h b/src/lib/dhcp/iface_mgr.h
index 83b19dc..3fde867 100644
--- a/src/lib/dhcp/iface_mgr.h
+++ b/src/lib/dhcp/iface_mgr.h
@@ -1,4 +1,4 @@
-// Copyright (C) 2011-2014 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2011-2014 Internet Systems Consortium, Inc. ("ISC")
//
// Permission to use, copy, modify, and/or distribute this software for any
// purpose with or without fee is hereby granted, provided that the above
@@ -862,6 +862,24 @@ public:
ifaces_.push_back(iface);
}
+ /// @brief Checks if there is at least one socket of the specified family
+ /// open.
+ ///
+ /// @param family A socket family.
+ ///
+ /// @return true if there is at least one socket open, false otherwise.
+ bool hasOpenSocket(const uint16_t family) const;
+
+ /// @brief Checks if there is a socket open and bound to an address.
+ ///
+ /// This function checks if one of the sockets opened by the IfaceMgr is
+ /// bound to the IP address specified as the method parameter.
+ ///
+ /// @param addr Address of the socket being searched.
+ ///
+ /// @return true if there is a socket bound to the specified address.
+ bool hasOpenSocket(const isc::asiolink::IOAddress& addr) const;
+
/// A value of socket descriptor representing "not specified" state.
static const int INVALID_SOCKET = -1;
@@ -983,13 +1001,6 @@ private:
getLocalAddress(const isc::asiolink::IOAddress& remote_addr,
const uint16_t port);
- /// @brief Checks if there is at least one socket of the specified family
- /// open.
- ///
- /// @param family A socket family.
- ///
- /// @return true if there is at least one socket open, false otherwise.
- bool hasOpenSocket(const uint16_t family) const;
/// Holds instance of a class derived from PktFilter, used by the
/// IfaceMgr to open sockets and send/receive packets through these
diff --git a/src/lib/dhcp/tests/iface_mgr_unittest.cc b/src/lib/dhcp/tests/iface_mgr_unittest.cc
index 5d05542..853cc42 100644
--- a/src/lib/dhcp/tests/iface_mgr_unittest.cc
+++ b/src/lib/dhcp/tests/iface_mgr_unittest.cc
@@ -269,6 +269,7 @@ public:
}
}
}
+
};
/// @brief A test fixture class for IfaceMgr.
@@ -1503,6 +1504,43 @@ TEST_F(IfaceMgrTest, openSocket4ErrorHandler) {
}
+// This test verifies that the function correctly checks that the v4 socket is
+// open and bound to a specific address.
+TEST_F(IfaceMgrTest, hasOpenSocketForAddress4) {
+ NakedIfaceMgr ifacemgr;
+
+ // Remove all real interfaces and create a set of dummy interfaces.
+ ifacemgr.createIfaces();
+
+ // Use the custom packet filter object. This object mimics the socket
+ // opening operation - the real socket is not open.
+ boost::shared_ptr<TestPktFilter> custom_packet_filter(new TestPktFilter());
+ ASSERT_TRUE(custom_packet_filter);
+ ASSERT_NO_THROW(ifacemgr.setPacketFilter(custom_packet_filter));
+
+ // Simulate opening sockets using the dummy packet filter.
+ ASSERT_NO_THROW(ifacemgr.openSockets4(DHCP4_SERVER_PORT, true, NULL));
+
+ // Expect that the sockets are open on both eth0 and eth1.
+ ASSERT_EQ(1, ifacemgr.getIface("eth0")->getSockets().size());
+ ASSERT_EQ(1, ifacemgr.getIface("eth1")->getSockets().size());
+ // Socket shouldn't have been opened on loopback.
+ ASSERT_TRUE(ifacemgr.getIface("lo")->getSockets().empty());
+
+ // Check that there are sockets bound to addresses that we have
+ // set for interfaces.
+ EXPECT_TRUE(ifacemgr.hasOpenSocket(IOAddress("192.0.2.3")));
+ EXPECT_TRUE(ifacemgr.hasOpenSocket(IOAddress("10.0.0.1")));
+ // Check that there is no socket for the address which is not
+ // configured on any interface.
+ EXPECT_FALSE(ifacemgr.hasOpenSocket(IOAddress("10.1.1.1")));
+
+ // Check that v4 sockets are open, but no v6 socket is open.
+ EXPECT_TRUE(ifacemgr.hasOpenSocket(AF_INET));
+ EXPECT_FALSE(ifacemgr.hasOpenSocket(AF_INET6));
+
+}
+
// This test checks that the sockets are open and bound to link local addresses
// only, if unicast addresses are not specified.
TEST_F(IfaceMgrTest, openSockets6LinkLocal) {
@@ -1830,32 +1868,6 @@ TEST_F(IfaceMgrTest, openSockets6NoIfaces) {
EXPECT_FALSE(socket_open);
}
-// Test that exception is thrown when trying to bind a new socket to the port
-// and address which is already in use by another socket.
-TEST_F(IfaceMgrTest, openSockets6NoErrorHandler) {
- NakedIfaceMgr ifacemgr;
-
- // Remove all real interfaces and create a set of dummy interfaces.
- ifacemgr.createIfaces();
-
- boost::shared_ptr<PktFilter6Stub> filter(new PktFilter6Stub());
- ASSERT_TRUE(filter);
- ASSERT_NO_THROW(ifacemgr.setPacketFilter(filter));
-
- // Open socket on eth0. The openSockets6 should detect that this
- // socket has been already open and an attempt to open another socket
- // and bind to this address and port should fail.
- ASSERT_NO_THROW(ifacemgr.openSocket("eth0",
- IOAddress("fe80::3a60:77ff:fed5:cdef"),
- DHCP6_SERVER_PORT));
-
- // The function throws an exception when it tries to open a socket
- // and bind it to the address in use.
- EXPECT_THROW(ifacemgr.openSockets6(DHCP6_SERVER_PORT),
- isc::dhcp::SocketConfigError);
-
-}
-
// Test that the external error handler is called when trying to bind a new
// socket to the address and port being in use. The sockets on the other
// interfaces should open just fine.
@@ -1897,6 +1909,41 @@ TEST_F(IfaceMgrTest, openSocket6ErrorHandler) {
}
+// This test verifies that the function correctly checks that the v6 socket is
+// open and bound to a specific address.
+TEST_F(IfaceMgrTest, hasOpenSocketForAddress6) {
+ NakedIfaceMgr ifacemgr;
+
+ // Remove all real interfaces and create a set of dummy interfaces.
+ ifacemgr.createIfaces();
+
+ boost::shared_ptr<PktFilter6Stub> filter(new PktFilter6Stub());
+ ASSERT_TRUE(filter);
+ ASSERT_NO_THROW(ifacemgr.setPacketFilter(filter));
+
+ // Simulate opening sockets using the dummy packet filter.
+ bool success = false;
+ ASSERT_NO_THROW(success = ifacemgr.openSockets6(DHCP6_SERVER_PORT));
+ EXPECT_TRUE(success);
+
+ // Make sure that the sockets are bound as expected.
+ ASSERT_TRUE(ifacemgr.isBound("eth0", "fe80::3a60:77ff:fed5:cdef"));
+ EXPECT_TRUE(ifacemgr.isBound("eth1", "fe80::3a60:77ff:fed5:abcd"));
+
+ // There should be v6 sockets only, no v4 sockets.
+ EXPECT_TRUE(ifacemgr.hasOpenSocket(AF_INET6));
+ EXPECT_FALSE(ifacemgr.hasOpenSocket(AF_INET));
+
+ // Check that there are sockets bound to the addresses we have configured
+ // for interfaces.
+ EXPECT_TRUE(ifacemgr.hasOpenSocket(IOAddress("fe80::3a60:77ff:fed5:cdef")));
+ EXPECT_TRUE(ifacemgr.hasOpenSocket(IOAddress("fe80::3a60:77ff:fed5:abcd")));
+ // Check that there is no socket bound to the address which hasn't been
+ // configured on any interface.
+ EXPECT_FALSE(ifacemgr.hasOpenSocket(IOAddress("fe80::3a60:77ff:feed:1")));
+
+}
+
// Test the Iface structure itself
TEST_F(IfaceMgrTest, iface) {
boost::scoped_ptr<Iface> iface;
More information about the bind10-changes
mailing list