BIND 10 trac3035, updated. ee888857ee0e9d9d571f8087ff5edbd1ddbd637b [3035] Process Hostname option sent by a client.
BIND 10 source code commits
bind10-changes at lists.isc.org
Mon Sep 9 12:19:48 UTC 2013
The branch, trac3035 has been updated
via ee888857ee0e9d9d571f8087ff5edbd1ddbd637b (commit)
via fc2b2f8fa482e56c2c6f7e1e49db4129e9c5e7e6 (commit)
via 1d58f76d64c79770956797c41f91e261633f812e (commit)
via c8a1da135c0a6e9e82bbbec0fb5ed062a88ee37b (commit)
via 4924ba4190e5a2df3d7f5fb99c448869ae9133ee (commit)
from df1550729f9ea49d6b0f90e677726fc0dec1f804 (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 ee888857ee0e9d9d571f8087ff5edbd1ddbd637b
Author: Marcin Siodelski <marcin at isc.org>
Date: Mon Sep 9 14:19:12 2013 +0200
[3035] Process Hostname option sent by a client.
commit fc2b2f8fa482e56c2c6f7e1e49db4129e9c5e7e6
Author: Marcin Siodelski <marcin at isc.org>
Date: Mon Sep 9 12:49:51 2013 +0200
[3035] Added a function to return the number of labels in the FQDN.
commit 1d58f76d64c79770956797c41f91e261633f812e
Author: Marcin Siodelski <marcin at isc.org>
Date: Mon Sep 9 12:28:06 2013 +0200
[3035] Basic implementation to process DHCP4 Hostname option.
commit c8a1da135c0a6e9e82bbbec0fb5ed062a88ee37b
Author: Marcin Siodelski <marcin at isc.org>
Date: Mon Sep 9 10:24:54 2013 +0200
[3035] Create NameChangeRequests for incoming Request and Release.
Also, use Ciaddr instead of Yiaddr to get the released lease from the
lease database.
commit 4924ba4190e5a2df3d7f5fb99c448869ae9133ee
Author: Marcin Siodelski <marcin at isc.org>
Date: Thu Sep 5 15:11:26 2013 +0200
[3035] Implemented function to enqueue new NameChangeRequests.
-----------------------------------------------------------------------
Summary of changes:
src/bin/dhcp4/dhcp4_messages.mes | 22 +++
src/bin/dhcp4/dhcp4_srv.cc | 181 ++++++++++++++----
src/bin/dhcp4/dhcp4_srv.h | 20 +-
src/bin/dhcp4/tests/dhcp4_srv_unittest.cc | 6 +-
src/bin/dhcp4/tests/fqdn_unittest.cc | 296 +++++++++++++++++++++++++++--
src/lib/dhcp/option_custom.h | 1 -
src/lib/dhcp/option_data_types.cc | 10 +
src/lib/dhcp/option_data_types.h | 8 +
8 files changed, 488 insertions(+), 56 deletions(-)
-----------------------------------------------------------------------
diff --git a/src/bin/dhcp4/dhcp4_messages.mes b/src/bin/dhcp4/dhcp4_messages.mes
index fca75bf..2fb4136 100644
--- a/src/bin/dhcp4/dhcp4_messages.mes
+++ b/src/bin/dhcp4/dhcp4_messages.mes
@@ -70,6 +70,11 @@ This message is printed when DHCPv4 server disables an interface from being
used to receive DHCPv4 traffic. Sockets on this interface will not be opened
by the Interface Manager until interface is enabled.
+% DHCP4_DHCID_COMPUTE_ERROR failed to compute the DHCID for lease: %1, reason: %2
+This error message is logged when the attempt to compute DHCID for a specified
+lease has failed. The lease details and reason for failure is logged in the
+message.
+
% DHCP4_HOOK_BUFFER_RCVD_SKIP received DHCPv4 buffer was dropped because a callout set the skip flag.
This debug message is printed when a callout installed on buffer4_receive
hook point set the skip flag. For this particular hook point, the
@@ -134,6 +139,13 @@ specified client after receiving a REQUEST message from it. There are many
possible reasons for such a failure. Additional messages will indicate the
reason.
+% DHCP4_NCR_CREATION_FAILED failed to generate name change requests for DNS: %1
+This message indicates that server was unable to generate so called
+NameChangeRequests which should be sent to the b10-dhcp_ddns module to create
+new DNS records for the lease being acquired or to update existing records
+for the renewed lease. The reason for the failure is printed in the logged
+message.
+
% DHCP4_NO_SOCKETS_OPEN no interface configured to listen to DHCP traffic
This warning message is issued when current server configuration specifies
no interfaces that server should listen on, or specified interfaces are not
@@ -211,6 +223,16 @@ failure is given in the message.
% DHCP4_QUERY_DATA received packet type %1, data is <%2>
A debug message listing the data received from the client.
+% DHCP4_QUEUE_ADDITION_NCR name change request for adding new DNS entry queued: %1
+A debug message which is logged when the NameChangeRequest to add new DNS
+entries for the particular lease has been queued. The parameter of this log
+carries the details of the NameChangeRequest.
+
+% DHCP4_QUEUE_REMOVAL_NCR name change request for removing a DNS entry queued: %1
+A debug message which is logged when the NameChangeRequest to remove existing
+DNS entry for the particular lease has been queued. The parameter of this log
+carries the details of the NameChangeRequest.
+
% DHCP4_RELEASE address %1 belonging to client-id %2, hwaddr %3 was released properly.
This debug message indicates that an address was released properly. It
is a normal operation during client shutdown.
diff --git a/src/bin/dhcp4/dhcp4_srv.cc b/src/bin/dhcp4/dhcp4_srv.cc
index 5cc0e5f..9c1fc03 100644
--- a/src/bin/dhcp4/dhcp4_srv.cc
+++ b/src/bin/dhcp4/dhcp4_srv.cc
@@ -18,7 +18,6 @@
#include <dhcp/hwaddr.h>
#include <dhcp/iface_mgr.h>
#include <dhcp/option4_addrlst.h>
-#include <dhcp/option_custom.h>
#include <dhcp/option_int.h>
#include <dhcp/option_int_array.h>
#include <dhcp/pkt4.h>
@@ -752,7 +751,7 @@ Dhcpv4Srv::processClientName(const Pkt4Ptr& query, Pkt4Ptr& answer) {
OptionCustomPtr hostname = boost::dynamic_pointer_cast<OptionCustom>
(query->getOption(DHO_HOST_NAME));
if (hostname) {
- processHostnameOption(query, answer);
+ processHostnameOption(hostname, answer);
}
}
@@ -810,7 +809,7 @@ Dhcpv4Srv::processClientFqdnOption(const Option4ClientFqdnPtr& fqdn,
// one.
if (fqdn->getDomainNameType() == Option4ClientFqdn::PARTIAL) {
std::ostringstream name;
- if (fqdn->getDomainName().empty()) {
+ if (fqdn->getDomainName().empty() || FQDN_REPLACE_CLIENT_NAME) {
name << FQDN_GENERATED_PARTIAL_NAME;
} else {
name << fqdn->getDomainName();
@@ -842,7 +841,45 @@ Dhcpv4Srv::processClientFqdnOption(const Option4ClientFqdnPtr& fqdn,
}
void
-Dhcpv4Srv::processHostnameOption(const Pkt4Ptr&, Pkt4Ptr&) {
+Dhcpv4Srv::processHostnameOption(const OptionCustomPtr& opt_hostname,
+ Pkt4Ptr& answer) {
+ // Do nothing if the DNS updates are disabled.
+ if (!FQDN_ENABLE_UPDATE) {
+ return;
+ }
+
+ std::string hostname = opt_hostname->readString();
+ unsigned int label_count = OptionDataTypeUtil::getLabelCount(hostname);
+ // Copy construct the hostname provided by the client. It is entirely
+ // possible that we will use the hostname option provided by the client
+ // to perform the DNS update and we will send the same option to him to
+ // indicate that we accepted this hostname.
+ OptionCustomPtr opt_hostname_resp(new OptionCustom(*opt_hostname));
+
+ // The hostname option may be unqualified or fully qualified. The lab_count
+ // holds the number of labels for the name. The number of 1 means that
+ // there is only root label "." (even for unqualified names, as the
+ // getLabelCount function treats each name as a fully qualified one).
+ // By checking the number of labels present in the hostname we may infer
+ // whether client has sent the fully qualified or unqualified hostname.
+
+ // If there is only one label, it is a root. We will have to generate
+ // the whole domain name for the client.
+ if (FQDN_REPLACE_CLIENT_NAME || (label_count < 2)) {
+ std::ostringstream resp_hostname;
+ resp_hostname << FQDN_GENERATED_PARTIAL_NAME << "."
+ << FQDN_PARTIAL_SUFFIX << ".";
+ opt_hostname_resp->writeString(resp_hostname.str());
+ // If there are two labels, it means that the client has specified
+ // the unqualified name. We have to concatenate the unqalified name
+ // with the domain name.
+ } else if (label_count == 2) {
+ std::ostringstream resp_hostname;
+ resp_hostname << hostname << "." << FQDN_PARTIAL_SUFFIX << ".";
+ opt_hostname_resp->writeString(resp_hostname.str());
+ }
+
+ answer->addOption(opt_hostname_resp);
}
void
@@ -872,19 +909,11 @@ Dhcpv4Srv::createNameChangeRequests(const Lease4Ptr& lease,
// removal request for non-existent hostname.
// - A server has performed reverse, forward or both updates.
// - FQDN data between the new and old lease do not match.
- if ((!old_lease->hostname_.empty() &&
- (old_lease->fqdn_fwd_ || old_lease->fqdn_rev_)) &&
- ((lease->hostname_ != old_lease->hostname_) ||
+ if ((lease->hostname_ != old_lease->hostname_) ||
(lease->fqdn_fwd_ != old_lease->fqdn_fwd_) ||
- (lease->fqdn_rev_ != old_lease->fqdn_rev_))) {
- D2Dhcid dhcid = computeDhcid(old_lease);
- NameChangeRequest ncr(isc::dhcp_ddns::CHG_REMOVE,
- old_lease->fqdn_fwd_,
- old_lease->fqdn_rev_,
- old_lease->hostname_,
- old_lease->addr_.toText(),
- dhcid, 0, old_lease->valid_lft_);
- name_change_reqs_.push(ncr);
+ (lease->fqdn_rev_ != old_lease->fqdn_rev_)) {
+ queueNameChangeRequest(isc::dhcp_ddns::CHG_REMOVE,
+ old_lease);
// If FQDN data from both leases match, there is no need to update.
} else if ((lease->hostname_ == old_lease->hostname_) &&
@@ -896,19 +925,51 @@ Dhcpv4Srv::createNameChangeRequests(const Lease4Ptr& lease,
}
}
- // We may need to generate the NameChangeRequest for the new lease. But,
- // this is only when hostname is set and if forward or reverse update has
- // been requested.
- if (!lease->hostname_.empty() && (lease->fqdn_fwd_ || lease->fqdn_rev_)) {
- D2Dhcid dhcid = computeDhcid(lease);
- NameChangeRequest ncr(isc::dhcp_ddns::CHG_ADD,
- lease->fqdn_fwd_, lease->fqdn_rev_,
- lease->hostname_, lease->addr_.toText(),
- dhcid, 0, lease->valid_lft_);
- name_change_reqs_.push(ncr);
+ // We may need to generate the NameChangeRequest for the new lease. It
+ // will be generated only if hostname is set and if forward or reverse
+ // update has been requested.
+ queueNameChangeRequest(isc::dhcp_ddns::CHG_ADD, lease);
+}
+
+void
+Dhcpv4Srv::
+queueNameChangeRequest(const isc::dhcp_ddns::NameChangeType chg_type,
+ const Lease4Ptr& lease) {
+ // The hostname must not be empty, and at least one type of update
+ // should be requested.
+ if (!lease || lease->hostname_.empty() ||
+ (!lease->fqdn_rev_ && !lease->fqdn_fwd_)) {
+ return;
+ }
+ D2Dhcid dhcid;
+ try {
+ dhcid = computeDhcid(lease);
+
+ } catch (const DhcidComputeError& ex) {
+ LOG_ERROR(dhcp4_logger, DHCP4_DHCID_COMPUTE_ERROR)
+ .arg(lease->toText())
+ .arg(ex.what());
+ return;
+
+ }
+ NameChangeRequest ncr(chg_type, lease->fqdn_fwd_, lease->fqdn_rev_,
+ lease->hostname_, lease->addr_.toText(),
+ dhcid, 0, lease->valid_lft_);
+ if (chg_type == isc::dhcp_ddns::CHG_ADD) {
+ LOG_DEBUG(dhcp4_logger, DBG_DHCP4_DETAIL_DATA,
+ DHCP4_QUEUE_ADDITION_NCR)
+ .arg(ncr.toText());
+
+ } else {
+ LOG_DEBUG(dhcp4_logger, DBG_DHCP4_DETAIL_DATA,
+ DHCP4_QUEUE_REMOVAL_NCR)
+ .arg(ncr.toText());
+
}
+ name_change_reqs_.push(ncr);
}
+
void
Dhcpv4Srv::assignLease(const Pkt4Ptr& question, Pkt4Ptr& answer) {
@@ -958,6 +1019,27 @@ Dhcpv4Srv::assignLease(const Pkt4Ptr& question, Pkt4Ptr& answer) {
CalloutHandlePtr callout_handle = getCalloutHandle(question);
+ std::string hostname;
+ bool fqdn_fwd = false;
+ bool fqdn_rev = false;
+ Option4ClientFqdnPtr fqdn = boost::dynamic_pointer_cast<
+ Option4ClientFqdn>(answer->getOption(DHO_FQDN));
+ if (fqdn) {
+ hostname = fqdn->getDomainName();
+ fqdn_fwd = fqdn->getFlag(Option4ClientFqdn::FLAG_S);
+ fqdn_rev = !fqdn->getFlag(Option4ClientFqdn::FLAG_N);
+ } else {
+ OptionCustomPtr opt_hostname = boost::dynamic_pointer_cast<
+ OptionCustom>(answer->getOption(DHO_HOST_NAME));
+ if (opt_hostname) {
+ hostname = opt_hostname->readString();
+ // @todo It could be configurable what sort of updates the server
+ // is doing when Hostname option was sent.
+ fqdn_fwd = true;
+ fqdn_rev = true;
+ }
+ }
+
// Use allocation engine to pick a lease for this client. Allocation engine
// will try to honour the hint, but it is just a hint - some other address
// may be used instead. If fake_allocation is set to false, the lease will
@@ -965,7 +1047,8 @@ Dhcpv4Srv::assignLease(const Pkt4Ptr& question, Pkt4Ptr& answer) {
// @todo pass the actual FQDN data.
Lease4Ptr old_lease;
Lease4Ptr lease = alloc_engine_->allocateAddress4(subnet, client_id, hwaddr,
- hint, false, false, "",
+ hint, fqdn_fwd, fqdn_rev,
+ hostname,
fake_allocation,
callout_handle,
old_lease);
@@ -999,6 +1082,21 @@ Dhcpv4Srv::assignLease(const Pkt4Ptr& question, Pkt4Ptr& answer) {
// @todo: send renew timer option (T1, option 58)
// @todo: send rebind timer option (T2, option 59)
+ // @todo Currently the NameChangeRequests are always generated if
+ // real (not fake) allocation is being performed. Should we have
+ // control switch to enable/disable NameChangeRequest creation?
+ // Perhaps we need a way to detect whether the b10-dhcp-ddns module
+ // is up an running?
+ if (!fake_allocation) {
+ try {
+ createNameChangeRequests(lease, old_lease);
+ } catch (const Exception& ex) {
+ LOG_ERROR(dhcp4_logger, DHCP4_NCR_CREATION_FAILED)
+ .arg(ex.what());
+ }
+
+ }
+
} else {
// Allocation engine did not allocate a lease. The engine logged
// cause of that failure. The only thing left is to insert
@@ -1012,6 +1110,9 @@ Dhcpv4Srv::assignLease(const Pkt4Ptr& question, Pkt4Ptr& answer) {
answer->setType(DHCPNAK);
answer->setYiaddr(IOAddress("0.0.0.0"));
+
+ answer->delOption(DHO_FQDN);
+ answer->delOption(DHO_HOST_NAME);
}
}
@@ -1091,6 +1192,12 @@ Dhcpv4Srv::processDiscover(Pkt4Ptr& discover) {
appendDefaultOptions(offer, DHCPOFFER);
appendRequestedOptions(discover, offer);
+ // If DISCOVER message contains the FQDN or Hostname option, server
+ // may respond to the client with the appropriate FQDN or Hostname
+ // option to indicate that whether it will take responsibility for
+ // updating DNS when the client sends REQUEST message.
+ processClientName(discover, offer);
+
assignLease(discover, offer);
// There are a few basic options that we always want to
@@ -1114,6 +1221,12 @@ Dhcpv4Srv::processRequest(Pkt4Ptr& request) {
appendDefaultOptions(ack, DHCPACK);
appendRequestedOptions(request, ack);
+ // If REQUEST message contains the FQDN or Hostname option, server
+ // should respond to the client with the appropriate FQDN or Hostname
+ // option to indicate if it takes responsibility for the DNS updates.
+ // This is performed by the function below.
+ processClientName(request, ack);
+
// Note that we treat REQUEST message uniformly, regardless if this is a
// first request (requesting for new address), renewing existing address
// or even rebinding.
@@ -1142,12 +1255,12 @@ Dhcpv4Srv::processRelease(Pkt4Ptr& release) {
try {
// Do we have a lease for that particular address?
- Lease4Ptr lease = LeaseMgrFactory::instance().getLease4(release->getYiaddr());
+ Lease4Ptr lease = LeaseMgrFactory::instance().getLease4(release->getCiaddr());
if (!lease) {
// No such lease - bogus release
LOG_DEBUG(dhcp4_logger, DBG_DHCP4_DETAIL, DHCP4_RELEASE_FAIL_NO_LEASE)
- .arg(release->getYiaddr().toText())
+ .arg(release->getCiaddr().toText())
.arg(release->getHWAddr()->toText())
.arg(client_id ? client_id->toText() : "(no client-id)");
return;
@@ -1158,7 +1271,7 @@ Dhcpv4Srv::processRelease(Pkt4Ptr& release) {
if (lease->hwaddr_ != release->getHWAddr()->hwaddr_) {
// @todo: Print hwaddr from lease as part of ticket #2589
LOG_DEBUG(dhcp4_logger, DBG_DHCP4_DETAIL, DHCP4_RELEASE_FAIL_WRONG_HWADDR)
- .arg(release->getYiaddr().toText())
+ .arg(release->getCiaddr().toText())
.arg(client_id ? client_id->toText() : "(no client-id)")
.arg(release->getHWAddr()->toText());
return;
@@ -1168,7 +1281,7 @@ Dhcpv4Srv::processRelease(Pkt4Ptr& release) {
// the client sent us.
if (lease->client_id_ && client_id && *lease->client_id_ != *client_id) {
LOG_DEBUG(dhcp4_logger, DBG_DHCP4_DETAIL, DHCP4_RELEASE_FAIL_WRONG_CLIENT_ID)
- .arg(release->getYiaddr().toText())
+ .arg(release->getCiaddr().toText())
.arg(client_id->toText())
.arg(lease->client_id_->toText());
return;
@@ -1209,11 +1322,15 @@ Dhcpv4Srv::processRelease(Pkt4Ptr& release) {
bool success = LeaseMgrFactory::instance().deleteLease(lease->addr_);
if (success) {
- // Release successful - we're done here
+ // Release successful
LOG_DEBUG(dhcp4_logger, DBG_DHCP4_DETAIL, DHCP4_RELEASE)
.arg(lease->addr_.toText())
.arg(client_id ? client_id->toText() : "(no client-id)")
.arg(release->getHWAddr()->toText());
+
+ // Remove existing DNS entries for the lease, if any.
+ queueNameChangeRequest(isc::dhcp_ddns::CHG_REMOVE, lease);
+
} else {
// Release failed -
LOG_ERROR(dhcp4_logger, DHCP4_RELEASE_FAIL)
diff --git a/src/bin/dhcp4/dhcp4_srv.h b/src/bin/dhcp4/dhcp4_srv.h
index bbc0912..ea77b39 100644
--- a/src/bin/dhcp4/dhcp4_srv.h
+++ b/src/bin/dhcp4/dhcp4_srv.h
@@ -19,6 +19,7 @@
#include <dhcp/pkt4.h>
#include <dhcp/option.h>
#include <dhcp/option4_client_fqdn.h>
+#include <dhcp/option_custom.h>
#include <dhcp_ddns/ncr_msg.h>
#include <dhcpsrv/subnet.h>
#include <dhcpsrv/alloc_engine.h>
@@ -306,9 +307,11 @@ private:
/// @brief Process Hostname %Option sent by a client.
///
- /// @param query A DISCOVER or REQUEST message from a cient.
+ /// @param opt_hostname An @c OptionCustom object encapsulating the Hostname
+ /// %Option.
/// @param [out] answer A response message to be sent to a client.
- void processHostnameOption(const Pkt4Ptr& query, Pkt4Ptr& answer);
+ void processHostnameOption(const OptionCustomPtr& opt_hostname,
+ Pkt4Ptr& answer);
protected:
@@ -329,6 +332,19 @@ protected:
void createNameChangeRequests(const Lease4Ptr& lease,
const Lease4Ptr& old_lease);
+ /// @brief Creates the NameChangeRequest and adds to the queue for
+ /// processing.
+ ///
+ /// This function adds the @c isc::dhcp_ddns::NameChangeRequest to the
+ /// queue and emits the debug message which indicates whether the request
+ /// being added is to remove DNS entry or add a new entry. This function
+ /// is exception free.
+ ///
+ /// @param chg_type A type of the NameChangeRequest (ADD or REMOVE).
+ /// @param ncr An isc::dhcp_ddns::NameChangeRequest object being added.
+ void queueNameChangeRequest(const isc::dhcp_ddns::NameChangeType chg_type,
+ const Lease4Ptr& lease);
+
/// @brief Attempts to renew received addresses
///
/// Attempts to renew existing lease. This typically includes finding a lease that
diff --git a/src/bin/dhcp4/tests/dhcp4_srv_unittest.cc b/src/bin/dhcp4/tests/dhcp4_srv_unittest.cc
index 3718a1e..0bba54a 100644
--- a/src/bin/dhcp4/tests/dhcp4_srv_unittest.cc
+++ b/src/bin/dhcp4/tests/dhcp4_srv_unittest.cc
@@ -860,7 +860,7 @@ TEST_F(Dhcpv4SrvTest, ReleaseBasic) {
// Generate client-id also duid_
Pkt4Ptr rel = Pkt4Ptr(new Pkt4(DHCPRELEASE, 1234));
rel->setRemoteAddr(addr);
- rel->setYiaddr(addr);
+ rel->setCiaddr(addr);
rel->addOption(clientid);
rel->addOption(srv->getServerID());
rel->setHWAddr(hw);
@@ -925,7 +925,7 @@ TEST_F(Dhcpv4SrvTest, ReleaseReject) {
// Generate client-id also duid_
Pkt4Ptr rel = Pkt4Ptr(new Pkt4(DHCPRELEASE, 1234));
rel->setRemoteAddr(addr);
- rel->setYiaddr(addr);
+ rel->setCiaddr(addr);
rel->addOption(clientid);
rel->addOption(srv->getServerID());
rel->setHWAddr(bogus_hw);
@@ -2284,7 +2284,7 @@ TEST_F(HooksDhcpv4SrvTest, lease4ReleaseSimple) {
// Generate client-id also duid_
Pkt4Ptr rel = Pkt4Ptr(new Pkt4(DHCPRELEASE, 1234));
rel->setRemoteAddr(addr);
- rel->setYiaddr(addr);
+ rel->setCiaddr(addr);
rel->addOption(clientid);
rel->addOption(srv_->getServerID());
rel->setHWAddr(hw);
diff --git a/src/bin/dhcp4/tests/fqdn_unittest.cc b/src/bin/dhcp4/tests/fqdn_unittest.cc
index 173c7bf..3001b6c 100644
--- a/src/bin/dhcp4/tests/fqdn_unittest.cc
+++ b/src/bin/dhcp4/tests/fqdn_unittest.cc
@@ -28,13 +28,12 @@ using namespace isc::dhcp_ddns;
using namespace isc::test;
namespace {
-class FqdnDhcpv4SrvTest : public Dhcpv4SrvTest {
+class NameDhcpv4SrvTest : public Dhcpv4SrvTest {
public:
- FqdnDhcpv4SrvTest() : Dhcpv4SrvTest() {
+ NameDhcpv4SrvTest() : Dhcpv4SrvTest() {
srv_ = new NakedDhcpv4Srv(0);
}
-
- virtual ~FqdnDhcpv4SrvTest() {
+ virtual ~NameDhcpv4SrvTest() {
delete srv_;
}
@@ -66,13 +65,29 @@ public:
RCODE_CLIENT(),
fqdn_name,
fqdn_type)));
+ }
+
+ // Create an instance of the Hostname option.
+ OptionCustomPtr
+ createHostname(const std::string& hostname) {
+ OptionDefinition def("hostname", DHO_HOST_NAME, "string");
+ OptionCustomPtr opt_hostname(new OptionCustom(def, Option::V4));
+ opt_hostname->writeString(hostname);
+ return (opt_hostname);
}
+ // Get the Client FQDN Option from the given message.
Option4ClientFqdnPtr getClientFqdnOption(const Pkt4Ptr& pkt) {
return (boost::dynamic_pointer_cast<
Option4ClientFqdn>(pkt->getOption(DHO_FQDN)));
}
+ // get the Hostname option from the given message.
+ OptionCustomPtr getHostnameOption(const Pkt4Ptr& pkt) {
+ return (boost::dynamic_pointer_cast<
+ OptionCustom>(pkt->getOption(DHO_HOST_NAME)));
+ }
+
// Create a message holding DHCPv4 Client FQDN Option.
Pkt4Ptr generatePktWithFqdn(const uint8_t msg_type,
const uint8_t fqdn_flags,
@@ -110,6 +125,31 @@ public:
return (pkt);
}
+ // Create a message holding a Hostname option.
+ Pkt4Ptr generatePktWithHostname(const uint8_t msg_type,
+ const std::string& hostname) {
+
+ Pkt4Ptr pkt = Pkt4Ptr(new Pkt4(msg_type, 1234));
+ pkt->setRemoteAddr(IOAddress("192.0.2.3"));
+ // For DISCOVER we don't include server id, because client broadcasts
+ // the message to all servers.
+ if (msg_type != DHCPDISCOVER) {
+ pkt->addOption(srv_->getServerID());
+ }
+
+ pkt->addOption(generateClientId());
+
+
+ // Create Client FQDN Option with the specified flags and
+ // domain-name.
+ pkt->addOption(createHostname(hostname));
+
+ return (pkt);
+
+ }
+
+
+
// Test that server generates the appropriate FQDN option in response to
// client's FQDN option.
void testProcessFqdn(const Pkt4Ptr& query, const uint8_t exp_flags,
@@ -144,6 +184,61 @@ public:
}
+ // Test that the Hostname option is returned in the server's response
+ // to the message holding Hostname option sent by a client.
+ void testProcessHostname(const Pkt4Ptr& query,
+ const std::string& exp_hostname) {
+ ASSERT_TRUE(getHostnameOption(query));
+
+ Pkt4Ptr answer;
+ if (query->getType() == DHCPDISCOVER) {
+ answer.reset(new Pkt4(DHCPOFFER, 1234));
+
+ } else {
+ answer.reset(new Pkt4(DHCPACK, 1234));
+
+ }
+ ASSERT_NO_THROW(srv_->processClientName(query, answer));
+
+ OptionCustomPtr hostname = getHostnameOption(answer);
+ ASSERT_TRUE(hostname);
+
+ EXPECT_EQ(exp_hostname, hostname->readString());
+
+ }
+
+
+ // Test that the client message holding an FQDN is processed and the
+ // NameChangeRequests are generated.
+ void testProcessMessageWithFqdn(const uint8_t msg_type,
+ const std::string& hostname) {
+ Pkt4Ptr req = generatePktWithFqdn(msg_type, Option4ClientFqdn::FLAG_S |
+ Option4ClientFqdn::FLAG_E, hostname,
+ Option4ClientFqdn::FULL, true);
+ Pkt4Ptr reply;
+ if (msg_type == DHCPDISCOVER) {
+ ASSERT_NO_THROW(reply = srv_->processDiscover(req));
+
+ } else if (msg_type == DHCPREQUEST) {
+ ASSERT_NO_THROW(reply = srv_->processRequest(req));
+
+ } else if (msg_type == DHCPRELEASE) {
+ ASSERT_NO_THROW(srv_->processRelease(req));
+ return;
+
+ } else {
+ return;
+ }
+
+ if (msg_type == DHCPDISCOVER) {
+ checkResponse(reply, DHCPOFFER, 1234);
+
+ } else {
+ checkResponse(reply, DHCPACK, 1234);
+ }
+
+ }
+
// Verify that NameChangeRequest holds valid values.
void verifyNameChangeRequest(const isc::dhcp_ddns::NameChangeType type,
const bool reverse, const bool forward,
@@ -171,7 +266,7 @@ public:
// Test that the exception is thrown if lease pointer specified as the argument
// of computeDhcid function is NULL.
-TEST_F(FqdnDhcpv4SrvTest, dhcidNullLease) {
+TEST_F(NameDhcpv4SrvTest, dhcidNullLease) {
Lease4Ptr lease;
EXPECT_THROW(srv_->computeDhcid(lease), isc::dhcp::DhcidComputeError);
@@ -179,7 +274,7 @@ TEST_F(FqdnDhcpv4SrvTest, dhcidNullLease) {
// Test that the appropriate exception is thrown if the lease object used
// to compute DHCID comprises wrong hostname.
-TEST_F(FqdnDhcpv4SrvTest, dhcidWrongHostname) {
+TEST_F(NameDhcpv4SrvTest, dhcidWrongHostname) {
// First, make sure that the lease with the correct hostname is accepted.
Lease4Ptr lease = createLease(IOAddress("192.0.2.3"),
"myhost.example.com.", true, true);
@@ -196,7 +291,7 @@ TEST_F(FqdnDhcpv4SrvTest, dhcidWrongHostname) {
// Test that the DHCID is computed correctly, when the lease holds
// correct hostname and non-NULL client id.
-TEST_F(FqdnDhcpv4SrvTest, dhcidComputeFromClientId) {
+TEST_F(NameDhcpv4SrvTest, dhcidComputeFromClientId) {
Lease4Ptr lease = createLease(IOAddress("192.0.2.3"),
"myhost.example.com.",
true, true);
@@ -211,7 +306,7 @@ TEST_F(FqdnDhcpv4SrvTest, dhcidComputeFromClientId) {
// Test that the DHCID is computed correctly, when the lease holds correct
// hostname and NULL client id.
-TEST_F(FqdnDhcpv4SrvTest, dhcidComputeFromHWAddr) {
+TEST_F(NameDhcpv4SrvTest, dhcidComputeFromHWAddr) {
Lease4Ptr lease = createLease(IOAddress("192.0.2.3"),
"myhost.example.com.",
true, true);
@@ -229,7 +324,7 @@ TEST_F(FqdnDhcpv4SrvTest, dhcidComputeFromHWAddr) {
// Test that server confirms to perform the forward and reverse DNS update,
// when client asks for it.
-TEST_F(FqdnDhcpv4SrvTest, serverUpdateForward) {
+TEST_F(NameDhcpv4SrvTest, serverUpdateForwardFqdn) {
Pkt4Ptr query = generatePktWithFqdn(DHCPREQUEST,
Option4ClientFqdn::FLAG_E |
Option4ClientFqdn::FLAG_S,
@@ -243,9 +338,17 @@ TEST_F(FqdnDhcpv4SrvTest, serverUpdateForward) {
}
+// Test that server processes the Hostname option sent by a client and
+// responds with the Hostname option to confirm that the
+TEST_F(NameDhcpv4SrvTest, serverUpdateHostname) {
+ Pkt4Ptr query = generatePktWithHostname(DHCPREQUEST,
+ "myhost.example.com.");
+ testProcessHostname(query, "myhost.example.com.");
+}
+
// Test that server generates the fully qualified domain name for the client
// if client supplies the partial name.
-TEST_F(FqdnDhcpv4SrvTest, serverUpdateForwardPartialName) {
+TEST_F(NameDhcpv4SrvTest, serverUpdateForwardPartialNameFqdn) {
Pkt4Ptr query = generatePktWithFqdn(DHCPREQUEST,
Option4ClientFqdn::FLAG_E |
Option4ClientFqdn::FLAG_S,
@@ -260,8 +363,15 @@ TEST_F(FqdnDhcpv4SrvTest, serverUpdateForwardPartialName) {
}
// Test that server generates the fully qualified domain name for the client
+// if client supplies the unqualified name in the Hostname option.
+TEST_F(NameDhcpv4SrvTest, serverUpdateUnqualifiedHostname) {
+ Pkt4Ptr query = generatePktWithHostname(DHCPREQUEST, "myhost");
+ testProcessHostname(query, "myhost.example.com.");
+}
+
+// Test that server generates the fully qualified domain name for the client
// if clietn supplies empty domain name.
-TEST_F(FqdnDhcpv4SrvTest, serverUpdateForwardNoName) {
+TEST_F(NameDhcpv4SrvTest, serverUpdateForwardNoNameFqdn) {
Pkt4Ptr query = generatePktWithFqdn(DHCPREQUEST,
Option4ClientFqdn::FLAG_E |
Option4ClientFqdn::FLAG_S,
@@ -276,7 +386,7 @@ TEST_F(FqdnDhcpv4SrvTest, serverUpdateForwardNoName) {
}
// Test server's response when client requests no DNS update.
-TEST_F(FqdnDhcpv4SrvTest, noUpdate) {
+TEST_F(NameDhcpv4SrvTest, noUpdateFqdn) {
Pkt4Ptr query = generatePktWithFqdn(DHCPREQUEST,
Option4ClientFqdn::FLAG_E |
Option4ClientFqdn::FLAG_N,
@@ -290,7 +400,7 @@ TEST_F(FqdnDhcpv4SrvTest, noUpdate) {
// Test that server does not accept delegation of the forward DNS update
// to a client.
-TEST_F(FqdnDhcpv4SrvTest, clientUpdateNotAllowed) {
+TEST_F(NameDhcpv4SrvTest, clientUpdateNotAllowedFqdn) {
Pkt4Ptr query = generatePktWithFqdn(DHCPREQUEST,
Option4ClientFqdn::FLAG_E,
"myhost.example.com.",
@@ -305,7 +415,7 @@ TEST_F(FqdnDhcpv4SrvTest, clientUpdateNotAllowed) {
// Test that exactly one NameChangeRequest is generated when the new lease
// has been acquired (old lease is NULL).
-TEST_F(FqdnDhcpv4SrvTest, createNameChangeRequestsNewLease) {
+TEST_F(NameDhcpv4SrvTest, createNameChangeRequestsNewLease) {
Lease4Ptr lease = createLease(IOAddress("192.0.2.3"), "myhost.example.com.",
true, true);
Lease4Ptr old_lease;
@@ -322,7 +432,7 @@ TEST_F(FqdnDhcpv4SrvTest, createNameChangeRequestsNewLease) {
// Test that no NameChangeRequest is generated when a lease is renewed and
// the FQDN data hasn't changed.
-TEST_F(FqdnDhcpv4SrvTest, createNameChangeRequestsRenewNoChange) {
+TEST_F(NameDhcpv4SrvTest, createNameChangeRequestsRenewNoChange) {
Lease4Ptr lease = createLease(IOAddress("192.0.2.3"), "myhost.example.com.",
true, true);
Lease4Ptr old_lease = createLease(IOAddress("192.0.2.3"),
@@ -335,7 +445,7 @@ TEST_F(FqdnDhcpv4SrvTest, createNameChangeRequestsRenewNoChange) {
// Test that no NameChangeRequest is generated when forward and reverse
// DNS update flags are not set in the lease.
-TEST_F(FqdnDhcpv4SrvTest, createNameChangeRequestsNoUpdate) {
+TEST_F(NameDhcpv4SrvTest, createNameChangeRequestsNoUpdate) {
Lease4Ptr lease1 = createLease(IOAddress("192.0.2.3"),
"lease1.example.com.",
true, true);
@@ -361,7 +471,7 @@ TEST_F(FqdnDhcpv4SrvTest, createNameChangeRequestsNoUpdate) {
// Test that two NameChangeRequests are generated when the lease is being
// renewed and the new lease has updated FQDN data.
-TEST_F(FqdnDhcpv4SrvTest, createNameChangeRequestsRenew) {
+TEST_F(NameDhcpv4SrvTest, createNameChangeRequestsRenew) {
Lease4Ptr lease1 = createLease(IOAddress("192.0.2.3"),
"lease1.example.com.",
true, true);
@@ -388,7 +498,7 @@ TEST_F(FqdnDhcpv4SrvTest, createNameChangeRequestsRenew) {
// This test verifies that exception is thrown when leases passed to the
// createNameChangeRequests function do not match, i.e. they comprise
// different IP addresses, client ids etc.
-TEST_F(FqdnDhcpv4SrvTest, createNameChangeRequestsLeaseMismatch) {
+TEST_F(NameDhcpv4SrvTest, createNameChangeRequestsLeaseMismatch) {
Lease4Ptr lease1 = createLease(IOAddress("192.0.2.3"),
"lease1.example.com.",
true, true);
@@ -399,4 +509,154 @@ TEST_F(FqdnDhcpv4SrvTest, createNameChangeRequestsLeaseMismatch) {
isc::Unexpected);
}
+// Test that the OFFER message generated as a result of the DISCOVER message
+// processing will not result in generation of the NameChangeRequests.
+TEST_F(NameDhcpv4SrvTest, processDiscover) {
+ Pkt4Ptr req = generatePktWithFqdn(DHCPDISCOVER, Option4ClientFqdn::FLAG_S |
+ Option4ClientFqdn::FLAG_E,
+ "myhost.example.com.",
+ Option4ClientFqdn::FULL, true);
+
+ Pkt4Ptr reply;
+ ASSERT_NO_THROW(reply = srv_->processDiscover(req));
+
+ checkResponse(reply, DHCPOFFER, 1234);
+
+ EXPECT_TRUE(srv_->name_change_reqs_.empty());
+}
+
+// Test that client may send two requests, each carrying FQDN option with
+// a different domain-name. Server should use existing lease for the second
+// request but modify the DNS entries for the lease according to the contents
+// of the FQDN sent in the second request.
+TEST_F(NameDhcpv4SrvTest, processTwoRequestsFqdn) {
+ Pkt4Ptr req1 = generatePktWithFqdn(DHCPREQUEST, Option4ClientFqdn::FLAG_S |
+ Option4ClientFqdn::FLAG_E,
+ "myhost.example.com.",
+ Option4ClientFqdn::FULL, true);
+
+ Pkt4Ptr reply;
+ ASSERT_NO_THROW(reply = srv_->processRequest(req1));
+
+ checkResponse(reply, DHCPACK, 1234);
+
+ // Verify that there is one NameChangeRequest generated.
+ ASSERT_EQ(1, srv_->name_change_reqs_.size());
+ verifyNameChangeRequest(isc::dhcp_ddns::CHG_ADD, true, true,
+ reply->getYiaddr().toText(), "myhost.example.com.",
+ "00010132E91AA355CFBB753C0F0497A5A940436"
+ "965B68B6D438D98E680BF10B09F3BCF",
+ 0, subnet_->getValid());
+
+ // Create another Request message but with a different FQDN. Server
+ // should generate two NameChangeRequests: one to remove existing entry,
+ // another one to add new entry with updated domain-name.
+ Pkt4Ptr req2 = generatePktWithFqdn(DHCPREQUEST, Option4ClientFqdn::FLAG_S |
+ Option4ClientFqdn::FLAG_E,
+ "otherhost.example.com.",
+ Option4ClientFqdn::FULL, true);
+
+ ASSERT_NO_THROW(reply = srv_->processRequest(req2));
+
+ checkResponse(reply, DHCPACK, 1234);
+
+ // There should be two NameChangeRequests. Verify that they are valid.
+ ASSERT_EQ(2, srv_->name_change_reqs_.size());
+ verifyNameChangeRequest(isc::dhcp_ddns::CHG_REMOVE, true, true,
+ reply->getYiaddr().toText(),
+ "myhost.example.com.",
+ "00010132E91AA355CFBB753C0F0497A5A940436"
+ "965B68B6D438D98E680BF10B09F3BCF",
+ 0, subnet_->getValid());
+
+ verifyNameChangeRequest(isc::dhcp_ddns::CHG_ADD, true, true,
+ reply->getYiaddr().toText(),
+ "otherhost.example.com.",
+ "000101A5AEEA7498BD5AD9D3BF600E49FF39A7E3"
+ "AFDCE8C3D0E53F35CC584DD63C89CA",
+ 0, subnet_->getValid());
+}
+
+// Test that client may send two requests, each carrying Hostname option with
+// a different name. Server should use existing lease for the second request
+// but modify the DNS entries for the lease according to the contents of the
+// Hostname sent in the second request.
+TEST_F(NameDhcpv4SrvTest, processTwoRequestsHostname) {
+ Pkt4Ptr req1 = generatePktWithHostname(DHCPREQUEST, "myhost.example.com.");
+
+ Pkt4Ptr reply;
+ ASSERT_NO_THROW(reply = srv_->processRequest(req1));
+
+ checkResponse(reply, DHCPACK, 1234);
+
+ // Verify that there is one NameChangeRequest generated.
+ ASSERT_EQ(1, srv_->name_change_reqs_.size());
+ verifyNameChangeRequest(isc::dhcp_ddns::CHG_ADD, true, true,
+ reply->getYiaddr().toText(), "myhost.example.com.",
+ "00010132E91AA355CFBB753C0F0497A5A940436"
+ "965B68B6D438D98E680BF10B09F3BCF",
+ 0, subnet_->getValid());
+
+ // Create another Request message but with a different Hostname. Server
+ // should generate two NameChangeRequests: one to remove existing entry,
+ // another one to add new entry with updated domain-name.
+ Pkt4Ptr req2 = generatePktWithHostname(DHCPREQUEST, "otherhost");
+
+ ASSERT_NO_THROW(reply = srv_->processRequest(req2));
+
+ checkResponse(reply, DHCPACK, 1234);
+
+ // There should be two NameChangeRequests. Verify that they are valid.
+ ASSERT_EQ(2, srv_->name_change_reqs_.size());
+ verifyNameChangeRequest(isc::dhcp_ddns::CHG_REMOVE, true, true,
+ reply->getYiaddr().toText(),
+ "myhost.example.com.",
+ "00010132E91AA355CFBB753C0F0497A5A940436"
+ "965B68B6D438D98E680BF10B09F3BCF",
+ 0, subnet_->getValid());
+
+ verifyNameChangeRequest(isc::dhcp_ddns::CHG_ADD, true, true,
+ reply->getYiaddr().toText(),
+ "otherhost.example.com.",
+ "000101A5AEEA7498BD5AD9D3BF600E49FF39A7E3"
+ "AFDCE8C3D0E53F35CC584DD63C89CA",
+ 0, subnet_->getValid());
+}
+
+// Test that when the Release message is sent for the previously acquired
+// lease, then server genenerates a NameChangeRequest to remove the entries
+// corresponding to the lease being released.
+TEST_F(NameDhcpv4SrvTest, processRequestRelease) {
+ Pkt4Ptr req = generatePktWithFqdn(DHCPREQUEST, Option4ClientFqdn::FLAG_S |
+ Option4ClientFqdn::FLAG_E,
+ "myhost.example.com.",
+ Option4ClientFqdn::FULL, true);
+
+ Pkt4Ptr reply;
+ ASSERT_NO_THROW(reply = srv_->processRequest(req));
+
+ checkResponse(reply, DHCPACK, 1234);
+
+ // Verify that there is one NameChangeRequest generated.
+ ASSERT_EQ(1, srv_->name_change_reqs_.size());
+ verifyNameChangeRequest(isc::dhcp_ddns::CHG_ADD, true, true,
+ reply->getYiaddr().toText(), "myhost.example.com.",
+ "00010132E91AA355CFBB753C0F0497A5A940436"
+ "965B68B6D438D98E680BF10B09F3BCF",
+ 0, subnet_->getValid());
+
+ // Create a Release message.
+ Pkt4Ptr rel = Pkt4Ptr(new Pkt4(DHCPRELEASE, 1234));
+ rel->setCiaddr(reply->getYiaddr());
+ rel->setRemoteAddr(IOAddress("192.0.2.3"));
+ rel->addOption(generateClientId());
+ rel->addOption(srv_->getServerID());
+
+ ASSERT_NO_THROW(srv_->processRelease(rel));
+
+ // The lease has been removed, so there should be a NameChangeRequest to
+ // remove corresponding DNS entries.
+ ASSERT_EQ(1, srv_->name_change_reqs_.size());
+}
+
} // end of anonymous namespace
diff --git a/src/lib/dhcp/option_custom.h b/src/lib/dhcp/option_custom.h
index a7d2b95..6ae0b18 100644
--- a/src/lib/dhcp/option_custom.h
+++ b/src/lib/dhcp/option_custom.h
@@ -105,7 +105,6 @@ public:
template<typename T>
void addArrayDataField(const T value) {
checkArrayType();
-
OptionDataType data_type = definition_.getType();
if (OptionDataTypeTraits<T>::type != data_type) {
isc_throw(isc::dhcp::InvalidDataType,
diff --git a/src/lib/dhcp/option_data_types.cc b/src/lib/dhcp/option_data_types.cc
index 3c55ada..b2e84a3 100644
--- a/src/lib/dhcp/option_data_types.cc
+++ b/src/lib/dhcp/option_data_types.cc
@@ -227,6 +227,16 @@ OptionDataTypeUtil::writeFqdn(const std::string& fqdn,
}
}
+unsigned int
+OptionDataTypeUtil::getLabelCount(const std::string& text_name) {
+ try {
+ isc::dns::Name name(text_name);
+ return (name.getLabelCount());
+ } catch (const isc::Exception& ex) {
+ isc_throw(BadDataTypeCast, ex.what());
+ }
+}
+
std::string
OptionDataTypeUtil::readString(const std::vector<uint8_t>& buf) {
std::string value;
diff --git a/src/lib/dhcp/option_data_types.h b/src/lib/dhcp/option_data_types.h
index 35d6a1f..253776f 100644
--- a/src/lib/dhcp/option_data_types.h
+++ b/src/lib/dhcp/option_data_types.h
@@ -375,6 +375,14 @@ public:
std::vector<uint8_t>& buf,
const bool downcase = false);
+ /// @brief Return the number of labels in the Name.
+ ///
+ /// @param text_name A text representation of the name.
+ ///
+ /// @return A number of labels in the provided name.
+ /// @throw isc::BadCast if provided name is malformed.
+ static unsigned int getLabelCount(const std::string& text_name);
+
/// @brief Read string value from a buffer.
///
/// @param buf input buffer.
More information about the bind10-changes
mailing list