BIND 10 master, updated. 0b4b438f337361eeeb7f5e5dbaf9938c24353812 [master] Merge branch 'trac3210' (DHCPv4 client-id echo config parameter)
BIND 10 source code commits
bind10-changes at lists.isc.org
Mon Dec 16 17:51:28 UTC 2013
The branch, master has been updated
via 0b4b438f337361eeeb7f5e5dbaf9938c24353812 (commit)
via 88a4858db206dfcd53a227562198f308f7779a72 (commit)
via 689bd411d1c25b3bd798a24567cadf4f867475c8 (commit)
via 8f96d784205c46e35f368e39e9233711639918b9 (commit)
via 9d3cf130a0c92c715b8ff1b7ce8f1743ab9c2bc7 (commit)
via 4a08e3662e7720ac6c96b7d585084bbe4033c29b (commit)
via 96fdb92a291b0f8897027e3b1c8b79918ae32483 (commit)
via 9fbb30d633080d5e6a65cef793b1a990f4f1d5e8 (commit)
from 2133cf13c8ce75c146c1f66179f9c7d9a9228ef8 (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 0b4b438f337361eeeb7f5e5dbaf9938c24353812
Merge: 2133cf1 88a4858
Author: Tomek Mrugalski <tomasz at isc.org>
Date: Mon Dec 16 18:31:24 2013 +0100
[master] Merge branch 'trac3210' (DHCPv4 client-id echo config parameter)
Conflicts:
ChangeLog
doc/guide/bind10-guide.xml
-----------------------------------------------------------------------
Summary of changes:
ChangeLog | 9 ++++-
doc/guide/bind10-guide.xml | 45 ++++++++++++++--------
src/bin/dhcp4/config_parser.cc | 21 ++++++++++-
src/bin/dhcp4/dhcp4.spec | 6 +++
src/bin/dhcp4/dhcp4_srv.cc | 4 +-
src/bin/dhcp4/tests/config_parser_unittest.cc | 41 ++++++++++++++++++++
src/bin/dhcp4/tests/dhcp4_srv_unittest.cc | 50 +++++++++++++++++++++++++
src/bin/dhcp4/tests/dhcp4_test_utils.cc | 23 ++++++++++--
src/bin/dhcp4/tests/dhcp4_test_utils.h | 8 +++-
src/lib/dhcpsrv/cfgmgr.cc | 2 +-
src/lib/dhcpsrv/cfgmgr.h | 19 ++++++++++
src/lib/dhcpsrv/tests/cfgmgr_unittest.cc | 17 +++++++++
12 files changed, 218 insertions(+), 27 deletions(-)
-----------------------------------------------------------------------
diff --git a/ChangeLog b/ChangeLog
index 2e79148..8cebf12 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,4 +1,11 @@
-7XX. [func] dclink, tomek
+719. [func] tomek
+ b10-dhcp4: Support for sending back client-id (RFC6842) has been
+ added now. Also a configuration parameter (echo-client-id) has
+ been added, so it is possible to enable backward compatibility
+ ("echo-client-id false").
+ (Trac #3210, git 88a4858db206dfcd53a227562198f308f7779a72)
+
+718. [func] dclink, tomek
libdhcp++: Interface detection implemented for FreeBSD, NetBSD,
OpenBSD, Mac OS X and Solaris 11. Thanks to David Carlier for
contributing a patch.
diff --git a/doc/guide/bind10-guide.xml b/doc/guide/bind10-guide.xml
index 87463a4..d915e27 100644
--- a/doc/guide/bind10-guide.xml
+++ b/doc/guide/bind10-guide.xml
@@ -4419,6 +4419,29 @@ Dhcp4/subnet4 [] list (default)
</section>
+ <section id="dhcp4-echo-client-id">
+ <title>Echoing client-id (RFC6842)</title>
+ <para>Original DHCPv4 spec (RFC2131) states that the DHCPv4
+ server must not send back client-id options when responding to
+ clients. However, in some cases that confused clients that did
+ not have MAC address or client-id. See RFC6842 for details. That
+ behavior has changed with the publication of RFC6842 which
+ updated RFC2131. That update now states that the server must
+ send client-id if client sent it. That is the default behaviour
+ that Kea offers. However, in some cases older devices that do
+ not support RFC6842 may refuse to accept responses that include
+ client-id option. To enable backward compatibility, an optional
+ configuration parameter has been introduced. To configure it,
+ use the following commands:</para>
+
+<screen>
+> <userinput>config add Dhcp4/echo-client-id</userinput>
+> <userinput>config set Dhcp4/echo-client-id False</userinput>
+> <userinput>config commit</userinput>
+</screen>
+
+ </section>
+
<section id="dhcp4-std">
<title>Supported Standards</title>
<para>The following standards and draft standards are currently
@@ -4429,7 +4452,8 @@ Dhcp4/subnet4 [] list (default)
REQUEST, RELEASE, ACK, and NAK.</simpara>
</listitem>
<listitem>
- <simpara><ulink url="http://tools.ietf.org/html/rfc2132">RFC 2132</ulink>: Supported options are: PAD (0),
+ <simpara><ulink url="http://tools.ietf.org/html/rfc2132">RFC 2132</ulink>:
+ Supported options are: PAD (0),
END(255), Message Type(53), DHCP Server Identifier (54),
Domain Name (15), DNS Servers (6), IP Address Lease Time
(51), Subnet mask (1), and Routers (3).</simpara>
@@ -4437,6 +4461,10 @@ Dhcp4/subnet4 [] list (default)
<listitem>
<simpara><ulink url="http://tools.ietf.org/html/rfc3046">RFC 3046</ulink>:
Relay Agent Information option is supported.</simpara>
+ <simpara><ulink url="http://tools.ietf.org/html/rfc6842">RFC 6842</ulink>:
+ Server by default sends back client-id option. That capability may be
+ disabled. See <xref linkend="dhcp4-echo-client-id"/> for details.
+ </simpara>
</listitem>
</itemizedlist>
</section>
@@ -4460,21 +4488,6 @@ Dhcp4/renew-timer 1000 integer (default)
> <userinput>config commit</userinput></screen>
</para>
</listitem>
- <listitem>
- <simpara>During the initial IPv4 node configuration, the
- server is expected to send packets to a node that does not
- have IPv4 address assigned yet. The server requires
- certain tricks (or hacks) to transmit such packets. This
- is not implemented yet, therefore DHCPv4 server supports
- relayed traffic only (that is, normal point to point
- communication).</simpara>
- </listitem>
-
- <listitem>
- <simpara>Upon start, the server will open sockets on all
- interfaces that are not loopback, are up and running and
- have IPv4 address.</simpara>
- </listitem>
<listitem>
<simpara>The DHCPv4 server does not support
diff --git a/src/bin/dhcp4/config_parser.cc b/src/bin/dhcp4/config_parser.cc
index a5acac0..777cce5 100644
--- a/src/bin/dhcp4/config_parser.cc
+++ b/src/bin/dhcp4/config_parser.cc
@@ -216,7 +216,6 @@ protected:
return (parser);
}
-
/// @brief Determines if the given option space name and code describe
/// a standard option for the DCHP4 server.
///
@@ -395,6 +394,8 @@ DhcpConfigParser* createGlobalDhcp4ConfigParser(const std::string& config_id) {
parser = new DbAccessParser(config_id);
} else if (config_id.compare("hooks-libraries") == 0) {
parser = new HooksLibrariesParser(config_id);
+ } else if (config_id.compare("echo-client-id") == 0) {
+ parser = new BooleanParser(config_id, globalContext()->boolean_values_);
} else {
isc_throw(NotImplemented,
"Parser error: Global configuration parameter not supported: "
@@ -404,6 +405,20 @@ DhcpConfigParser* createGlobalDhcp4ConfigParser(const std::string& config_id) {
return (parser);
}
+void commitGlobalOptions() {
+ // Although the function is modest for now, it is certain that the number
+ // of global switches will increase over time, hence the name.
+
+ // Set whether v4 server is supposed to echo back client-id (yes = RFC6842
+ // compatible, no = backward compatibility)
+ try {
+ bool echo_client_id = globalContext()->boolean_values_->getParam("echo-client-id");
+ CfgMgr::instance().echoClientId(echo_client_id);
+ } catch (...) {
+ // Ignore errors. This flag is optional
+ }
+}
+
isc::data::ConstElementPtr
configureDhcp4Server(Dhcpv4Srv&, isc::data::ConstElementPtr config_set) {
if (!config_set) {
@@ -536,6 +551,9 @@ configureDhcp4Server(Dhcpv4Srv&, isc::data::ConstElementPtr config_set) {
iface_parser->commit();
}
+ // Apply global options
+ commitGlobalOptions();
+
// This occurs last as if it succeeds, there is no easy way
// revert it. As a result, the failure to commit a subsequent
// change causes problems when trying to roll back.
@@ -579,4 +597,3 @@ ParserContextPtr& globalContext() {
}; // end of isc::dhcp namespace
}; // end of isc namespace
-
diff --git a/src/bin/dhcp4/dhcp4.spec b/src/bin/dhcp4/dhcp4.spec
index b507159..9372dfd 100644
--- a/src/bin/dhcp4/dhcp4.spec
+++ b/src/bin/dhcp4/dhcp4.spec
@@ -54,6 +54,12 @@
"item_default": ""
},
+ { "item_name": "echo-client-id",
+ "item_type": "boolean",
+ "item_optional": true,
+ "item_default": true
+ },
+
{ "item_name": "option-def",
"item_type": "list",
"item_optional": false,
diff --git a/src/bin/dhcp4/dhcp4_srv.cc b/src/bin/dhcp4/dhcp4_srv.cc
index 329d604..5e5572b 100644
--- a/src/bin/dhcp4/dhcp4_srv.cc
+++ b/src/bin/dhcp4/dhcp4_srv.cc
@@ -653,8 +653,10 @@ Dhcpv4Srv::copyDefaultFields(const Pkt4Ptr& question, Pkt4Ptr& answer) {
answer->setGiaddr(question->getGiaddr());
// Let's copy client-id to response. See RFC6842.
+ // It is possible to disable RFC6842 to keep backward compatibility
+ bool echo = CfgMgr::instance().echoClientId();
OptionPtr client_id = question->getOption(DHO_DHCP_CLIENT_IDENTIFIER);
- if (client_id) {
+ if (client_id && echo) {
answer->addOption(client_id);
}
diff --git a/src/bin/dhcp4/tests/config_parser_unittest.cc b/src/bin/dhcp4/tests/config_parser_unittest.cc
index 6e53d4d..1d9ccb7 100644
--- a/src/bin/dhcp4/tests/config_parser_unittest.cc
+++ b/src/bin/dhcp4/tests/config_parser_unittest.cc
@@ -557,6 +557,47 @@ TEST_F(Dhcp4ParserTest, nextServerOverride) {
EXPECT_EQ("1.2.3.4", subnet->getSiaddr().toText());
}
+// Check whether it is possible to configure echo-client-id
+TEST_F(Dhcp4ParserTest, echoClientId) {
+
+ ConstElementPtr status;
+
+ string config_false = "{ \"interfaces\": [ \"*\" ],"
+ "\"rebind-timer\": 2000, "
+ "\"renew-timer\": 1000, "
+ "\"echo-client-id\": false,"
+ "\"subnet4\": [ { "
+ " \"pool\": [ \"192.0.2.1 - 192.0.2.100\" ],"
+ " \"subnet\": \"192.0.2.0/24\" } ],"
+ "\"valid-lifetime\": 4000 }";
+
+ string config_true = "{ \"interfaces\": [ \"*\" ],"
+ "\"rebind-timer\": 2000, "
+ "\"renew-timer\": 1000, "
+ "\"echo-client-id\": true,"
+ "\"subnet4\": [ { "
+ " \"pool\": [ \"192.0.2.1 - 192.0.2.100\" ],"
+ " \"subnet\": \"192.0.2.0/24\" } ],"
+ "\"valid-lifetime\": 4000 }";
+
+ ElementPtr json_false = Element::fromJSON(config_false);
+ ElementPtr json_true = Element::fromJSON(config_true);
+
+ // Let's check the default. It should be true
+ ASSERT_TRUE(CfgMgr::instance().echoClientId());
+
+ // Now check that "false" configuration is really applied.
+ EXPECT_NO_THROW(status = configureDhcp4Server(*srv_, json_false));
+ ASSERT_FALSE(CfgMgr::instance().echoClientId());
+
+ // Now check that "true" configuration is really applied.
+ EXPECT_NO_THROW(status = configureDhcp4Server(*srv_, json_true));
+ ASSERT_TRUE(CfgMgr::instance().echoClientId());
+
+ // In any case revert back to the default value (true)
+ CfgMgr::instance().echoClientId(true);
+}
+
// This test checks if it is possible to override global values
// on a per subnet basis.
TEST_F(Dhcp4ParserTest, subnetLocal) {
diff --git a/src/bin/dhcp4/tests/dhcp4_srv_unittest.cc b/src/bin/dhcp4/tests/dhcp4_srv_unittest.cc
index a15bbad..3787149 100644
--- a/src/bin/dhcp4/tests/dhcp4_srv_unittest.cc
+++ b/src/bin/dhcp4/tests/dhcp4_srv_unittest.cc
@@ -608,6 +608,32 @@ TEST_F(Dhcpv4SrvTest, ManyDiscovers) {
cout << "Offered address to client3=" << addr3.toText() << endl;
}
+// Checks whether echoing back client-id is controllable, i.e.
+// whether the server obeys echo-client-id and sends (or not)
+// client-id
+TEST_F(Dhcpv4SrvTest, discoverEchoClientId) {
+ NakedDhcpv4Srv srv(0);
+
+ Pkt4Ptr dis = Pkt4Ptr(new Pkt4(DHCPDISCOVER, 1234));
+ dis->setRemoteAddr(IOAddress("192.0.2.1"));
+ OptionPtr clientid = generateClientId();
+ dis->addOption(clientid);
+
+ // Pass it to the server and get an offer
+ Pkt4Ptr offer = srv.processDiscover(dis);
+
+ // Check if we get response at all
+ checkResponse(offer, DHCPOFFER, 1234);
+ checkClientId(offer, clientid);
+
+ CfgMgr::instance().echoClientId(false);
+ offer = srv.processDiscover(dis);
+
+ // Check if we get response at all
+ checkResponse(offer, DHCPOFFER, 1234);
+ checkClientId(offer, clientid);
+}
+
// This test verifies that incoming REQUEST can be handled properly, that an
// ACK is generated, that the response has an address and that address
// really belongs to the configured pool.
@@ -750,6 +776,30 @@ TEST_F(Dhcpv4SrvTest, ManyRequests) {
cout << "Offered address to client3=" << addr3.toText() << endl;
}
+// Checks whether echoing back client-id is controllable
+TEST_F(Dhcpv4SrvTest, requestEchoClientId) {
+ NakedDhcpv4Srv srv(0);
+
+ Pkt4Ptr dis = Pkt4Ptr(new Pkt4(DHCPREQUEST, 1234));
+ dis->setRemoteAddr(IOAddress("192.0.2.1"));
+ OptionPtr clientid = generateClientId();
+ dis->addOption(clientid);
+
+ // Pass it to the server and get ACK
+ Pkt4Ptr ack = srv.processRequest(dis);
+
+ // Check if we get response at all
+ checkResponse(ack, DHCPACK, 1234);
+ checkClientId(ack, clientid);
+
+ CfgMgr::instance().echoClientId(false);
+ ack = srv.processDiscover(dis);
+
+ // Check if we get response at all
+ checkResponse(ack, DHCPOFFER, 1234);
+ checkClientId(ack, clientid);
+}
+
// This test verifies that incoming (positive) REQUEST/Renewing can be handled properly, that a
// REPLY is generated, that the response has an address and that address
diff --git a/src/bin/dhcp4/tests/dhcp4_test_utils.cc b/src/bin/dhcp4/tests/dhcp4_test_utils.cc
index e29c31d..8590927 100644
--- a/src/bin/dhcp4/tests/dhcp4_test_utils.cc
+++ b/src/bin/dhcp4/tests/dhcp4_test_utils.cc
@@ -67,6 +67,12 @@ Dhcpv4SrvTest::Dhcpv4SrvTest()
valid_iface_ = ifaces.begin()->getName();
}
+Dhcpv4SrvTest::~Dhcpv4SrvTest() {
+
+ // Make sure that we revert to default value
+ CfgMgr::instance().echoClientId(true);
+}
+
void Dhcpv4SrvTest::addPrlOption(Pkt4Ptr& pkt) {
OptionUint8ArrayPtr option_prl =
@@ -332,12 +338,21 @@ void Dhcpv4SrvTest::checkServerId(const Pkt4Ptr& rsp, const OptionPtr& expected_
}
void Dhcpv4SrvTest::checkClientId(const Pkt4Ptr& rsp, const OptionPtr& expected_clientid) {
+
+ bool include_clientid = CfgMgr::instance().echoClientId();
+
// check that server included our own client-id
OptionPtr opt = rsp->getOption(DHO_DHCP_CLIENT_IDENTIFIER);
- ASSERT_TRUE(opt);
- EXPECT_EQ(expected_clientid->getType(), opt->getType());
- EXPECT_EQ(expected_clientid->len(), opt->len());
- EXPECT_TRUE(expected_clientid->getData() == opt->getData());
+ if (include_clientid) {
+ // Normal mode: echo back (see RFC6842)
+ ASSERT_TRUE(opt);
+ EXPECT_EQ(expected_clientid->getType(), opt->getType());
+ EXPECT_EQ(expected_clientid->len(), opt->len());
+ EXPECT_TRUE(expected_clientid->getData() == opt->getData());
+ } else {
+ // Backward compatibility mode for pre-RFC6842 devices
+ ASSERT_FALSE(opt);
+ }
}
::testing::AssertionResult
diff --git a/src/bin/dhcp4/tests/dhcp4_test_utils.h b/src/bin/dhcp4/tests/dhcp4_test_utils.h
index c448a2f..26ed87d 100644
--- a/src/bin/dhcp4/tests/dhcp4_test_utils.h
+++ b/src/bin/dhcp4/tests/dhcp4_test_utils.h
@@ -81,8 +81,7 @@ public:
Dhcpv4SrvTest();
/// @brief destructor
- virtual ~Dhcpv4SrvTest() {
- }
+ virtual ~Dhcpv4SrvTest();
/// @brief Add 'Parameter Request List' option to the packet.
///
@@ -189,6 +188,11 @@ public:
void checkServerId(const Pkt4Ptr& rsp, const OptionPtr& expected_srvid);
/// @brief Checks if server response (OFFER, ACK, NAK) includes proper client-id
+ ///
+ /// This method follows values reported by CfgMgr in echoClientId() method.
+ /// Depending on its configuration, the client-id is either mandatory or
+ /// forbidden to appear in the response.
+ ///
/// @param rsp response packet to be validated
/// @param expected_clientid expected value of client-id
void checkClientId(const Pkt4Ptr& rsp, const OptionPtr& expected_clientid);
diff --git a/src/lib/dhcpsrv/cfgmgr.cc b/src/lib/dhcpsrv/cfgmgr.cc
index b4fb11d..7789c74 100644
--- a/src/lib/dhcpsrv/cfgmgr.cc
+++ b/src/lib/dhcpsrv/cfgmgr.cc
@@ -350,7 +350,7 @@ CfgMgr::getUnicast(const std::string& iface) const {
CfgMgr::CfgMgr()
: datadir_(DHCP_DATA_DIR),
- all_ifaces_active_(false) {
+ all_ifaces_active_(false), echo_v4_client_id_(true) {
// DHCP_DATA_DIR must be set set with -DDHCP_DATA_DIR="..." in Makefile.am
// Note: the definition of DHCP_DATA_DIR needs to include quotation marks
// See AM_CPPFLAGS definition in Makefile.am
diff --git a/src/lib/dhcpsrv/cfgmgr.h b/src/lib/dhcpsrv/cfgmgr.h
index 7b5ead3..cda59f2 100644
--- a/src/lib/dhcpsrv/cfgmgr.h
+++ b/src/lib/dhcpsrv/cfgmgr.h
@@ -316,6 +316,22 @@ public:
const isc::asiolink::IOAddress*
getUnicast(const std::string& iface) const;
+ /// @brief Sets whether server should send back client-id in DHCPv4
+ ///
+ /// This is a compatibility flag. The default (true) is compliant with
+ /// RFC6842. False is for backward compatibility.
+ ///
+ /// @param echo should the client-id be sent or not
+ void echoClientId(const bool echo) {
+ echo_v4_client_id_ = echo;
+ }
+
+ /// @brief Returns whether server should send back client-id in DHCPv4.
+ /// @return true if client-id should be returned, false otherwise.
+ bool echoClientId() const {
+ return (echo_v4_client_id_);
+ }
+
protected:
/// @brief Protected constructor.
@@ -392,6 +408,9 @@ private:
/// A flag which indicates that server should listen on all available
/// interfaces.
bool all_ifaces_active_;
+
+ /// Indicates whether v4 server should send back client-id
+ bool echo_v4_client_id_;
};
} // namespace isc::dhcp
diff --git a/src/lib/dhcpsrv/tests/cfgmgr_unittest.cc b/src/lib/dhcpsrv/tests/cfgmgr_unittest.cc
index 94f78c3..08ce768 100644
--- a/src/lib/dhcpsrv/tests/cfgmgr_unittest.cc
+++ b/src/lib/dhcpsrv/tests/cfgmgr_unittest.cc
@@ -653,6 +653,23 @@ TEST_F(CfgMgrTest, activateAllIfaces) {
EXPECT_FALSE(cfg_mgr.isActiveIface("eth2"));
}
+// This test verifies that RFC6842 (echo client-id) compatibility may be
+// configured.
+TEST_F(CfgMgrTest, echoClientId) {
+ CfgMgr& cfg_mgr = CfgMgr::instance();
+
+ // Check that the default is true
+ EXPECT_TRUE(cfg_mgr.echoClientId());
+
+ // Check that it can be modified to false
+ cfg_mgr.echoClientId(false);
+ EXPECT_FALSE(cfg_mgr.echoClientId());
+
+ // Check that the default value can be restored
+ cfg_mgr.echoClientId(true);
+ EXPECT_TRUE(cfg_mgr.echoClientId());
+}
+
/// @todo Add unit-tests for testing:
/// - addActiveIface() with invalid interface name
/// - addActiveIface() with the same interface twice
More information about the bind10-changes
mailing list