From: Thomas Markwalder Date: Mon, 20 Jan 2014 15:32:32 +0000 (-0500) Subject: [3282] Use DDNS configuration values in b10-dhcp4 X-Git-Tag: bind10-1.2.0beta1-release~91^2~3 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=c75896605bb0224250ff5a460d4bb7de55e18751;p=thirdparty%2Fkea.git [3282] Use DDNS configuration values in b10-dhcp4 Replaced hard-coded constants in dhcp4_srv.cc with methods and logic in D2ClientMgr to implement behavior changes based on dhcp-ddns configuration values. --- diff --git a/src/bin/dhcp4/dhcp4_srv.cc b/src/bin/dhcp4/dhcp4_srv.cc index 22ab6536bb..4c061fa889 100644 --- a/src/bin/dhcp4/dhcp4_srv.cc +++ b/src/bin/dhcp4/dhcp4_srv.cc @@ -79,36 +79,6 @@ Dhcp4Hooks Hooks; namespace isc { namespace dhcp { -namespace { - -// @todo The following constants describe server's behavior with respect to the -// DHCPv4 Client FQDN Option sent by a client. They will be removed -// when DDNS parameters for DHCPv4 are implemented with the ticket #3033. - -// @todo Additional configuration parameter which we may consider is the one -// that controls whether the DHCP server sends the removal NameChangeRequest -// if it discovers that the entry for the particular client exists or that -// it always updates the DNS. - -// Should server always include the FQDN option in its response, regardless -// if it has been requested in Parameter Request List Option (Disabled). -const bool FQDN_ALWAYS_INCLUDE = false; -// Enable A RR update delegation to the client (Disabled). -const bool FQDN_ALLOW_CLIENT_UPDATE = false; -// Globally enable updates (Enabled). -const bool FQDN_ENABLE_UPDATE = true; -// Do update, even if client requested no updates with N flag (Disabled). -const bool FQDN_OVERRIDE_NO_UPDATE = false; -// Server performs an update when client requested delegation (Enabled). -const bool FQDN_OVERRIDE_CLIENT_UPDATE = true; -// The fully qualified domain-name suffix if partial name provided by -// a client. -const char* FQDN_PARTIAL_SUFFIX = "example.com"; -// Should server replace the domain-name supplied by the client (Disabled). -const bool FQDN_REPLACE_CLIENT_NAME = false; - -} - Dhcpv4Srv::Dhcpv4Srv(uint16_t port, const char* dbconfig, const bool use_bcast, const bool direct_response_desired) : shutdown_(true), alloc_engine_(), port_(port), @@ -580,8 +550,8 @@ Dhcpv4Srv::appendServerID(const Pkt4Ptr& response) { // The source address for the outbound message should have been set already. // This is the address that to the best of the server's knowledge will be // available from the client. - // @todo: perhaps we should consider some more sophisticated server id - // generation, but for the current use cases, it should be ok. + /// @todo: perhaps we should consider some more sophisticated server id + /// generation, but for the current use cases, it should be ok. response->addOption(OptionPtr(new Option4AddrLst(DHO_DHCP_SERVER_IDENTIFIER, response->getLocalAddr())) ); @@ -650,8 +620,8 @@ Dhcpv4Srv::appendRequestedVendorOptions(const Pkt4Ptr& question, Pkt4Ptr& answer uint32_t vendor_id = vendor_req->getVendorId(); // Let's try to get ORO within that vendor-option - /// @todo This is very specific to vendor-id=4491 (Cable Labs). Other vendors - /// may have different policies. + /// @todo This is very specific to vendor-id=4491 (Cable Labs). Other + /// vendors may have different policies. OptionUint8ArrayPtr oro = boost::dynamic_pointer_cast(vendor_req->getOption(DOCSIS3_V4_ORO)); @@ -757,68 +727,19 @@ Dhcpv4Srv::processClientFqdnOption(const Option4ClientFqdnPtr& fqdn, // response to a client. Option4ClientFqdnPtr fqdn_resp(new Option4ClientFqdn(*fqdn)); - // RFC4702, section 4 - set 'NOS' flags to 0. - fqdn_resp->setFlag(Option4ClientFqdn::FLAG_S, 0); - fqdn_resp->setFlag(Option4ClientFqdn::FLAG_O, 0); - fqdn_resp->setFlag(Option4ClientFqdn::FLAG_N, 0); - - // Conditions when N flag has to be set to indicate that server will not - // perform DNS updates: - // 1. Updates are globally disabled, - // 2. Client requested no update and server respects it, - // 3. Client requested that the forward DNS update is delegated to the - // client but server neither respects requests for forward update - // delegation nor it is configured to send update on its own when - // client requested delegation. - if (!FQDN_ENABLE_UPDATE || - (fqdn->getFlag(Option4ClientFqdn::FLAG_N) && - !FQDN_OVERRIDE_NO_UPDATE) || - (!fqdn->getFlag(Option4ClientFqdn::FLAG_S) && - !FQDN_ALLOW_CLIENT_UPDATE && !FQDN_OVERRIDE_CLIENT_UPDATE)) { - fqdn_resp->setFlag(Option4ClientFqdn::FLAG_N, true); - - // Conditions when S flag is set to indicate that server will perform DNS - // update on its own: - // 1. Client requested that server performs DNS update and DNS updates are - // globally enabled. - // 2. Client requested that server delegates forward update to the client - // but server doesn't respect requests for delegation and it is - // configured to perform an update on its own when client requested the - // delegation. - } else if (fqdn->getFlag(Option4ClientFqdn::FLAG_S) || - (!fqdn->getFlag(Option4ClientFqdn::FLAG_S) && - !FQDN_ALLOW_CLIENT_UPDATE && FQDN_OVERRIDE_CLIENT_UPDATE)) { - fqdn_resp->setFlag(Option4ClientFqdn::FLAG_S, true); - } - - // Server MUST set the O flag if it has overriden the client's setting - // of S flag. - if (fqdn->getFlag(Option4ClientFqdn::FLAG_S) != - fqdn_resp->getFlag(Option4ClientFqdn::FLAG_S)) { - fqdn_resp->setFlag(Option4ClientFqdn::FLAG_O, true); - } + // Set the server S, N, and O flags based on client's flags and + // current configuration. + D2ClientMgr& d2_mgr = CfgMgr::instance().getD2ClientMgr(); + d2_mgr.adjustFqdnFlags(*fqdn, *fqdn_resp); - // If client suppled partial or empty domain-name, server should generate - // one. - if (fqdn->getDomainNameType() == Option4ClientFqdn::PARTIAL) { - std::ostringstream name; - if (fqdn->getDomainName().empty() || FQDN_REPLACE_CLIENT_NAME) { - fqdn_resp->setDomainName("", Option4ClientFqdn::PARTIAL); + // Carry over the client's E flag. + fqdn_resp->setFlag(Option4ClientFqdn::FLAG_E, + fqdn->getFlag(Option4ClientFqdn::FLAG_E)); - } else { - name << fqdn->getDomainName(); - name << "." << FQDN_PARTIAL_SUFFIX; - fqdn_resp->setDomainName(name.str(), Option4ClientFqdn::FULL); - } - - // Server may be configured to replace a name supplied by a client, even if - // client supplied fully qualified domain-name. The empty domain-name is - // is set to indicate that the name must be generated when the new lease - // is acquired. - } else if(FQDN_REPLACE_CLIENT_NAME) { - fqdn_resp->setDomainName("", Option4ClientFqdn::PARTIAL); - } + // Adjust the domain name based on domain name value and type sent by the + // client and current configuration. + d2_mgr.adjustDomainName(*fqdn, *fqdn_resp); // Add FQDN option to the response message. Note that, there may be some // cases when server may choose not to include the FQDN option in a @@ -838,15 +759,20 @@ Dhcpv4Srv::processClientFqdnOption(const Option4ClientFqdnPtr& fqdn, void Dhcpv4Srv::processHostnameOption(const OptionCustomPtr& opt_hostname, Pkt4Ptr& answer) { + // Fetch D2 configuration. + D2ClientMgr& d2_mgr = CfgMgr::instance().getD2ClientMgr(); + // Do nothing if the DNS updates are disabled. - if (!FQDN_ENABLE_UPDATE) { + if (!d2_mgr.ddnsEnabled()) { return; } std::string hostname = isc::util::str::trim(opt_hostname->readString()); unsigned int label_count = OptionDataTypeUtil::getLabelCount(hostname); // The hostname option sent by the client should be at least 1 octet long. - // If it isn't we ignore this option. + // If it isn't we ignore this option. (Per RFC 2131, section 3.14) + /// @todo It would be more liberal to accept this and let it fall into + /// the case of replace or less than two below. if (label_count == 0) { LOG_DEBUG(dhcp4_logger, DBG_DHCP4_DETAIL_DATA, DHCP4_EMPTY_HOSTNAME); return; @@ -864,21 +790,20 @@ Dhcpv4Srv::processHostnameOption(const OptionCustomPtr& opt_hostname, // By checking the number of labels present in the hostname we may infer // whether client has sent the fully qualified or unqualified hostname. - // @todo We may want to reconsider whether it is appropriate for the - // client to send a root domain name as a Hostname. There are - // also extensions to the auto generation of the client's name, - // e.g. conversion to the puny code which may be considered at some point. - // For now, we just remain liberal and expect that the DNS will handle - // conversion if needed and possible. - if (FQDN_REPLACE_CLIENT_NAME || (label_count < 2)) { + /// @todo We may want to reconsider whether it is appropriate for the + /// client to send a root domain name as a Hostname. There are + /// also extensions to the auto generation of the client's name, + /// e.g. conversion to the puny code which may be considered at some point. + /// For now, we just remain liberal and expect that the DNS will handle + /// conversion if needed and possible. + if ((d2_mgr.getD2ClientConfig()->getReplaceClientName()) || + (label_count < 2)) { opt_hostname_resp->writeString(""); - // 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()); + // 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. + opt_hostname_resp->writeString(d2_mgr.qualifyName(hostname)); } answer->addOption(opt_hostname_resp); @@ -969,9 +894,9 @@ queueNameChangeRequest(const isc::dhcp_ddns::NameChangeType chg_type, void Dhcpv4Srv::sendNameChangeRequests() { while (!name_change_reqs_.empty()) { - // @todo Once next NameChangeRequest is picked from the queue - // we should send it to the b10-dhcp_ddns module. Currently we - // just drop it. + /// @todo Once next NameChangeRequest is picked from the queue + /// we should send it to the b10-dhcp_ddns module. Currently we + /// just drop it. name_change_reqs_.pop(); } } @@ -990,8 +915,8 @@ Dhcpv4Srv::assignLease(const Pkt4Ptr& question, Pkt4Ptr& answer) { // thing this client can get is some global information (like DNS // servers). - // perhaps this should be logged on some higher level? This is most likely - // configuration bug. + // perhaps this should be logged on some higher level? This is most + // likely configuration bug. LOG_ERROR(dhcp4_logger, DHCP4_SUBNET_SELECTION_FAILED) .arg(question->getRemoteAddr().toText()) .arg(serverReceivedPacketName(question->getType())); @@ -1004,7 +929,7 @@ Dhcpv4Srv::assignLease(const Pkt4Ptr& question, Pkt4Ptr& answer) { // as siaddr has nothing to do with a lease, but otherwise we would have // to select subnet twice (performance hit) or update too many functions // at once. - // @todo: move subnet selection to a common code + /// @todo: move subnet selection to a common code answer->setSiaddr(subnet->getSiaddr()); LOG_DEBUG(dhcp4_logger, DBG_DHCP4_DETAIL_DATA, DHCP4_SUBNET_SELECTED) @@ -1047,8 +972,8 @@ Dhcpv4Srv::assignLease(const Pkt4Ptr& question, Pkt4Ptr& answer) { (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. + /// @todo It could be configurable what sort of updates the + /// server is doing when Hostname option was sent. fqdn_fwd = true; fqdn_rev = true; } @@ -1058,7 +983,7 @@ Dhcpv4Srv::assignLease(const Pkt4Ptr& question, Pkt4Ptr& answer) { // 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 // be inserted into the LeaseMgr as well. - // @todo pass the actual FQDN data. + /// @todo pass the actual FQDN data. Lease4Ptr old_lease; Lease4Ptr lease = alloc_engine_->allocateLease4(subnet, client_id, hwaddr, hint, fqdn_fwd, fqdn_rev, @@ -1083,14 +1008,9 @@ Dhcpv4Srv::assignLease(const Pkt4Ptr& question, Pkt4Ptr& answer) { // generating the entire hostname for the client. The example of the // client's name, generated from the IP address is: host-192-0-2-3. if ((fqdn || opt_hostname) && lease->hostname_.empty()) { - hostname = lease->addr_.toText(); - // Replace dots with hyphens. - std::replace(hostname.begin(), hostname.end(), '.', '-'); - ostringstream stream; - // The partial suffix will need to be replaced with the actual - // domain-name for the client when configuration is implemented. - stream << "host-" << hostname << "." << FQDN_PARTIAL_SUFFIX << "."; - lease->hostname_ = stream.str(); + lease->hostname_ = CfgMgr::instance() + .getD2ClientMgr().generateFqdn(lease->addr_); + // The operations below are rather safe, but we want to catch // any potential exceptions (e.g. invalid lease database backend // implementation) and log an error. @@ -1122,22 +1042,18 @@ Dhcpv4Srv::assignLease(const Pkt4Ptr& question, Pkt4Ptr& answer) { // Subnet mask (type 1) answer->addOption(getNetmaskOption(subnet)); - // @todo: send renew timer option (T1, option 58) - // @todo: send rebind timer option (T2, option 59) + /// @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) { + // Create NameChangeRequests if DDNS is enabled and this is a + // real allocation. + if (!fake_allocation && CfgMgr::instance().ddnsEnabled()) { try { createNameChangeRequests(lease, old_lease); } catch (const Exception& ex) { LOG_ERROR(dhcp4_logger, DHCP4_NCR_CREATION_FAILED) .arg(ex.what()); } - } } else { @@ -1178,8 +1094,8 @@ Dhcpv4Srv::adjustIfaceData(const Pkt4Ptr& query, const Pkt4Ptr& response) { // address for the response. Instead, we have to check what address our // socket is bound to and use it as a source address. This operation // may throw if for some reason the socket is closed. - // @todo Consider an optimization that we use local address from - // the query if this address is not broadcast. + /// @todo Consider an optimization that we use local address from + /// the query if this address is not broadcast. SocketInfo sock_info = IfaceMgr::instance().getSocket(*query); // Set local adddress, port and interface. response->setLocalAddr(sock_info.addr_); @@ -1294,7 +1210,7 @@ Pkt4Ptr Dhcpv4Srv::processRequest(Pkt4Ptr& request) { /// @todo Uncomment this (see ticket #3116) - // sanityCheck(request, MANDATORY); + /// sanityCheck(request, MANDATORY); Pkt4Ptr ack = Pkt4Ptr (new Pkt4(DHCPACK, request->getTransid())); @@ -1336,7 +1252,7 @@ void Dhcpv4Srv::processRelease(Pkt4Ptr& release) { /// @todo Uncomment this (see ticket #3116) - // sanityCheck(release, MANDATORY); + /// sanityCheck(release, MANDATORY); // Try to find client-id ClientIdPtr client_id; @@ -1361,7 +1277,7 @@ Dhcpv4Srv::processRelease(Pkt4Ptr& release) { // Does the hardware address match? We don't want one client releasing // second client's leases. if (lease->hwaddr_ != release->getHWAddr()->hwaddr_) { - // @todo: Print hwaddr from lease as part of ticket #2589 + /// @todo: Print hwaddr from lease as part of ticket #2589 LOG_DEBUG(dhcp4_logger, DBG_DHCP4_DETAIL, DHCP4_RELEASE_FAIL_WRONG_HWADDR) .arg(release->getCiaddr().toText()) .arg(client_id ? client_id->toText() : "(no client-id)") @@ -1419,9 +1335,11 @@ Dhcpv4Srv::processRelease(Pkt4Ptr& release) { .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); - + if (CfgMgr::instance().ddnsEnabled() && + CfgMgr::instance().getD2ClientConfig()->getRemoveOnRenew()) { + // 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) @@ -1662,8 +1580,8 @@ Dhcpv4Srv::openActiveSockets(const uint16_t port, } // Let's reopen active sockets. openSockets4 will check internally whether // sockets are marked active or inactive. - // @todo Optimization: we should not reopen all sockets but rather select - // those that have been affected by the new configuration. + /// @todo Optimization: we should not reopen all sockets but rather select + /// those that have been affected by the new configuration. isc::dhcp::IfaceMgrErrorMsgCallback error_handler = boost::bind(&Dhcpv4Srv::ifaceMgrSocket4ErrorHandler, _1); if (!IfaceMgr::instance().openSockets4(port, use_bcast, error_handler)) { diff --git a/src/bin/dhcp4/tests/fqdn_unittest.cc b/src/bin/dhcp4/tests/fqdn_unittest.cc index d3bf9ae2c1..73f15a760d 100644 --- a/src/bin/dhcp4/tests/fqdn_unittest.cc +++ b/src/bin/dhcp4/tests/fqdn_unittest.cc @@ -18,6 +18,7 @@ #include #include #include +#include #include #include @@ -32,13 +33,55 @@ namespace { class NameDhcpv4SrvTest : public Dhcpv4SrvFakeIfaceTest { public: + + // Bit Constants for turning on and off DDNS configuration options. + static const uint16_t REMOVE_ON_RENEW = 1; + static const uint16_t ALWAYS_INCLUDE_FQDN = 2; + static const uint16_t OVERRIDE_NO_UPDATE = 4; + static const uint16_t OVERRIDE_CLIENT_UPDATE = 8; + static const uint16_t REPLACE_CLIENT_NAME = 16; + NameDhcpv4SrvTest() : Dhcpv4SrvFakeIfaceTest() { srv_ = new NakedDhcpv4Srv(0); + // Config DDNS to be enabled, all controls off + enableD2(); } + virtual ~NameDhcpv4SrvTest() { delete srv_; } + /// @brief Sets the server's DDNS configuration to ddns updates disabled. + void disableD2() { + // Default constructor creates a config with DHCP-DDNS updates + // disabled. + D2ClientConfigPtr cfg(new D2ClientConfig()); + CfgMgr::instance().setD2ClientConfig(cfg); + } + + /// @brief Enables DHCP-DDNS updates with the given options enabled. + /// + /// Replaces the current D2ClientConfiguration with a configuration + /// which as updates enabled and the control options set based upon + /// the bit mask of options. + /// + /// @param mask Bit mask of configuration options that should be enabled. + void enableD2(const uint16_t mask = 0) { + D2ClientConfigPtr cfg; + + ASSERT_NO_THROW(cfg.reset(new D2ClientConfig(true, + isc::asiolink::IOAddress("192.0.2.1"), 477, + dhcp_ddns::NCR_UDP, dhcp_ddns::FMT_JSON, + (mask & REMOVE_ON_RENEW), + (mask & ALWAYS_INCLUDE_FQDN), + (mask & OVERRIDE_NO_UPDATE), + (mask & OVERRIDE_CLIENT_UPDATE), + (mask & REPLACE_CLIENT_NAME), + "myhost", "example.com"))); + + CfgMgr::instance().setD2ClientConfig(cfg); + } + // Create a lease to be used by various tests. Lease4Ptr createLease(const isc::asiolink::IOAddress& addr, const std::string& hostname, @@ -78,15 +121,18 @@ public: return (opt_hostname); } - // Generates partial hostname from the address. The format of the - // generated address is: host-A-B-C-D, where A.B.C.D is an IP - // address. + /// @brief Convenience method for generating an FQDN from an IP address. + /// + /// This is just a wrapper method around the D2ClientMgr's method for + /// generating domain names from the configured prefix, suffix, and a + /// given IP address. This is useful for verifying that fully generated + /// names are correct. + /// + /// @param addr IP address used in the lease. + /// + /// @return An std::string contained the generated FQDN. std::string generatedNameFromAddress(const IOAddress& addr) { - std::string gen_name = addr.toText(); - std::replace(gen_name.begin(), gen_name.end(), '.', '-'); - std::ostringstream hostname; - hostname << "host-" << gen_name; - return (hostname.str()); + return(CfgMgr::instance().getD2ClientMgr().generateFqdn(addr)); } // Get the Client FQDN Option from the given message. @@ -182,6 +228,21 @@ public: Option4ClientFqdnPtr fqdn = getClientFqdnOption(answer); ASSERT_TRUE(fqdn); + checkFqdnFlags(answer, exp_flags); + + EXPECT_EQ(exp_domain_name, fqdn->getDomainName()); + EXPECT_EQ(exp_domain_type, fqdn->getDomainNameType()); + + } + + /// @brief Checks the packet's FQDN option flags against a given mask + /// + /// @param pkt IPv4 packet whose FQDN flags should be checked. + /// @param exp_flags Bit mask of flags that are expected to be true. + void checkFqdnFlags(const Pkt4Ptr& pkt, const uint8_t exp_flags) { + Option4ClientFqdnPtr fqdn = getClientFqdnOption(pkt); + ASSERT_TRUE(fqdn); + const bool flag_n = (exp_flags & Option4ClientFqdn::FLAG_N) != 0; const bool flag_s = (exp_flags & Option4ClientFqdn::FLAG_S) != 0; const bool flag_o = (exp_flags & Option4ClientFqdn::FLAG_O) != 0; @@ -191,12 +252,9 @@ public: EXPECT_EQ(flag_s, fqdn->getFlag(Option4ClientFqdn::FLAG_S)); EXPECT_EQ(flag_o, fqdn->getFlag(Option4ClientFqdn::FLAG_O)); EXPECT_EQ(flag_e, fqdn->getFlag(Option4ClientFqdn::FLAG_E)); - - EXPECT_EQ(exp_domain_name, fqdn->getDomainName()); - EXPECT_EQ(exp_domain_type, fqdn->getDomainNameType()); - } + // Processes the Hostname option in the client's message and returns // the hostname option which would be sent to the client. It will // throw NULL pointer if the hostname option is not to be included @@ -221,8 +279,8 @@ public: } - // Test that the client message holding an FQDN is processed and the - // NameChangeRequests are generated. + // Test that the client message holding an FQDN is processed and + // that the response packet is as expected. void testProcessMessageWithFqdn(const uint8_t msg_type, const std::string& hostname) { Pkt4Ptr req = generatePktWithFqdn(msg_type, Option4ClientFqdn::FLAG_S | @@ -287,6 +345,50 @@ public: srv_->name_change_reqs_.pop(); } + + /// @brief Tests processing a request with the given client flags + /// + /// This method creates a request with its FQDN flags set to the given + /// value and submits it to the server for processing. It then checks + /// the following: + /// 1. Did the server generate an ACK with the correct FQDN flags + /// 2. If the server should have generated an NCR, did it? and If + /// so was it correct? + /// + /// @param client_flags Mask of client FQDN flags which are true + /// @param response_flags Mask of expected FQDN flags in the response + void flagVsConfigScenario(const uint8_t client_flags, + const uint8_t response_flags) { + Pkt4Ptr req = generatePktWithFqdn(DHCPREQUEST, client_flags, + "myhost.example.com.", + Option4ClientFqdn::FULL, true); + + // Process the request. + Pkt4Ptr reply; + ASSERT_NO_THROW(reply = srv_->processRequest(req)); + + // Verify the response and flags. + checkResponse(reply, DHCPACK, 1234); + checkFqdnFlags(reply, response_flags); + + // There should be an NCR only if response S flag is 1. + /// @todo This logic will need to change if forward and reverse + /// updates are ever controlled independently. + if ((response_flags & Option4ClientFqdn::FLAG_S) == 0) { + ASSERT_EQ(0, srv_->name_change_reqs_.size()); + } else { + // Verify that there is one NameChangeRequest generated as expected. + ASSERT_EQ(1, srv_->name_change_reqs_.size()); + verifyNameChangeRequest(isc::dhcp_ddns::CHG_ADD, true, true, + reply->getYiaddr().toText(), + "myhost.example.com.", + "", // empty DHCID means don't check it + time(NULL) + subnet_->getValid(), + subnet_->getValid(), true); + } + } + + NakedDhcpv4Srv* srv_; }; @@ -348,21 +450,101 @@ TEST_F(NameDhcpv4SrvTest, dhcidComputeFromHWAddr) { EXPECT_EQ(dhcid_ref, dhcid.toStr()); } +// Tests the following scenario: +// - Updates are enabled +// - All overrides are off +// - Client requests forward update (N = 0, S = 1) +// +// Server should perform the update: +// - Reponse flags should N = 0, S = 1, O = 0 +// - Should queue an NCR +TEST_F(NameDhcpv4SrvTest, updatesEnabled) { + flagVsConfigScenario((Option4ClientFqdn::FLAG_E | + Option4ClientFqdn::FLAG_S), + (Option4ClientFqdn::FLAG_E | + Option4ClientFqdn::FLAG_S)); +} -// Test that server confirms to perform the forward and reverse DNS update, -// when client asks for it. -TEST_F(NameDhcpv4SrvTest, serverUpdateForwardFqdn) { - Pkt4Ptr query = generatePktWithFqdn(DHCPREQUEST, - Option4ClientFqdn::FLAG_E | - Option4ClientFqdn::FLAG_S, - "myhost.example.com.", - Option4ClientFqdn::FULL, - true); +// Tests the following scenario +// - Updates are disabled +// - Client requests forward update (N = 0, S = 1) +// +// Server should NOT perform updates: +// - Response flags should N = 1, S = 0, O = 1 +// - Should not queue any NCRs +TEST_F(NameDhcpv4SrvTest, updatesDisabled) { + disableD2(); + flagVsConfigScenario((Option4ClientFqdn::FLAG_E | + Option4ClientFqdn::FLAG_S), + (Option4ClientFqdn::FLAG_E | + Option4ClientFqdn::FLAG_N | + Option4ClientFqdn::FLAG_O)); +} - testProcessFqdn(query, - Option4ClientFqdn::FLAG_E | Option4ClientFqdn::FLAG_S, - "myhost.example.com."); +// Tests the following scenario: +// - Updates are enabled +// - All overrides are off. +// - Client requests no updates (N = 1, S = 0) +// +// Server should NOT perform updates: +// - Response flags should N = 1, S = 0, O = 0 +// - Should not queue any NCRs +TEST_F(NameDhcpv4SrvTest, respectNoUpdate) { + flagVsConfigScenario((Option4ClientFqdn::FLAG_E | + Option4ClientFqdn::FLAG_N), + (Option4ClientFqdn::FLAG_E | + Option4ClientFqdn::FLAG_N)); +} +// Tests the following scenario: +// - Updates are enabled +// - override-no-update is on +// - Client requests no updates (N = 1, S = 0) +// +// Server should override "no update" request and perform updates: +// - Response flags should be N = 0, S = 1, O = 1 +// - Should queue an NCR +TEST_F(NameDhcpv4SrvTest, overrideNoUpdate) { + enableD2(OVERRIDE_NO_UPDATE); + flagVsConfigScenario((Option4ClientFqdn::FLAG_E | + Option4ClientFqdn::FLAG_N), + (Option4ClientFqdn::FLAG_E | + Option4ClientFqdn::FLAG_S | + Option4ClientFqdn::FLAG_O)); +} + +// Tests the following scenario: +// - Updates are enabled +// - All overrides are off. +// - Client requests delegation (N = 0, S = 0) +// +// Server should respect client's delegation request and NOT do updates: + +// - Response flags should be N = 1, S = 0, O = 0 +// - Should not queue any NCRs +TEST_F(NameDhcpv4SrvTest, respectClientDelegation) { + + flagVsConfigScenario(Option4ClientFqdn::FLAG_E, + (Option4ClientFqdn::FLAG_E | + Option4ClientFqdn::FLAG_N)); +} + +// Tests the following scenario: +// - Updates are enabled +// - override-client-update is on. +// - Client requests delegation (N = 0, S = 0) +// +// Server should override client's delegation request and do updates: +// - Response flags should be N = 0, S = 1, O = 1 +// - Should queue an NCR +TEST_F(NameDhcpv4SrvTest, overrideClientDelegation) { + // Turn on override-client-update. + enableD2(OVERRIDE_CLIENT_UPDATE); + + flagVsConfigScenario(Option4ClientFqdn::FLAG_E, + (Option4ClientFqdn::FLAG_E | + Option4ClientFqdn::FLAG_S | + Option4ClientFqdn::FLAG_O)); } // Test that server processes the Hostname option sent by a client and @@ -447,34 +629,6 @@ TEST_F(NameDhcpv4SrvTest, serverUpdateForwardNoNameFqdn) { } -// Test server's response when client requests no DNS update. -TEST_F(NameDhcpv4SrvTest, noUpdateFqdn) { - Pkt4Ptr query = generatePktWithFqdn(DHCPREQUEST, - Option4ClientFqdn::FLAG_E | - Option4ClientFqdn::FLAG_N, - "myhost.example.com.", - Option4ClientFqdn::FULL, - true); - testProcessFqdn(query, Option4ClientFqdn::FLAG_E | - Option4ClientFqdn::FLAG_N, - "myhost.example.com."); -} - -// Test that server does not accept delegation of the forward DNS update -// to a client. -TEST_F(NameDhcpv4SrvTest, clientUpdateNotAllowedFqdn) { - Pkt4Ptr query = generatePktWithFqdn(DHCPREQUEST, - Option4ClientFqdn::FLAG_E, - "myhost.example.com.", - Option4ClientFqdn::FULL, - true); - - testProcessFqdn(query, Option4ClientFqdn::FLAG_E | - Option4ClientFqdn::FLAG_S | Option4ClientFqdn::FLAG_O, - "myhost.example.com."); - -} - // Test that exactly one NameChangeRequest is generated when the new lease // has been acquired (old lease is NULL). TEST_F(NameDhcpv4SrvTest, createNameChangeRequestsNewLease) { @@ -600,17 +754,41 @@ TEST_F(NameDhcpv4SrvTest, processRequestFqdnEmptyDomainName) { // Verify that there is one NameChangeRequest generated. ASSERT_EQ(1, srv_->name_change_reqs_.size()); + // The hostname is generated from the IP address acquired (yiaddr). - std::ostringstream hostname; - hostname << generatedNameFromAddress(reply->getYiaddr()) - << ".example.com."; + std::string hostname = generatedNameFromAddress(reply->getYiaddr()); + verifyNameChangeRequest(isc::dhcp_ddns::CHG_ADD, true, true, - reply->getYiaddr().toText(), hostname.str(), + reply->getYiaddr().toText(), hostname, "", // empty DHCID forces that it is not checked time(NULL) + subnet_->getValid(), subnet_->getValid(), true); } +// Test that server generates client's hostname from the IP address assigned +// to it when DHCPv4 Client FQDN option specifies an empty domain-name AND +// ddns updates are disabled. +TEST_F(NameDhcpv4SrvTest, processRequestEmptyDomainNameDisabled) { + disableD2(); + Pkt4Ptr req = generatePktWithFqdn(DHCPREQUEST, Option4ClientFqdn::FLAG_S | + Option4ClientFqdn::FLAG_E, + "", Option4ClientFqdn::PARTIAL, true); + Pkt4Ptr reply; + ASSERT_NO_THROW(reply = srv_->processRequest(req)); + + checkResponse(reply, DHCPACK, 1234); + + Option4ClientFqdnPtr fqdn = getClientFqdnOption(reply); + ASSERT_TRUE(fqdn); + + // The hostname is generated from the IP address acquired (yiaddr). + std::string hostname = generatedNameFromAddress(reply->getYiaddr()); + + EXPECT_EQ(hostname, fqdn->getDomainName()); + EXPECT_EQ(Option4ClientFqdn::FULL, fqdn->getDomainNameType()); +} + + // Test that server generates client's hostname from the IP address assigned // to it when Hostname option carries the top level domain-name. TEST_F(NameDhcpv4SrvTest, processRequestEmptyHostname) { @@ -626,11 +804,12 @@ TEST_F(NameDhcpv4SrvTest, processRequestEmptyHostname) { // Verify that there is one NameChangeRequest generated. ASSERT_EQ(1, srv_->name_change_reqs_.size()); + // The hostname is generated from the IP address acquired (yiaddr). - std::ostringstream hostname; - hostname << generatedNameFromAddress(reply->getYiaddr()) << ".example.com."; + std::string hostname = generatedNameFromAddress(reply->getYiaddr()); + verifyNameChangeRequest(isc::dhcp_ddns::CHG_ADD, true, true, - reply->getYiaddr().toText(), hostname.str(), + reply->getYiaddr().toText(), hostname, "", // empty DHCID forces that it is not checked time(NULL), subnet_->getValid(), true); } @@ -742,21 +921,26 @@ TEST_F(NameDhcpv4SrvTest, processTwoRequestsHostname) { time(NULL), subnet_->getValid(), true); } -// 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) { +// Test that when a release message is sent for a previously acquired lease, +// DDNS updates are enabled and remove-on-renew is true that the server +// genenerates a NameChangeRequest to remove entries corresponding to the +// released lease. +TEST_F(NameDhcpv4SrvTest, processRequestReleaseRemoveOn) { + // Enable ddns updates and remove-on-renew . + enableD2(REMOVE_ON_RENEW); + ASSERT_TRUE(CfgMgr::instance().ddnsEnabled()); + ASSERT_TRUE(CfgMgr::instance().getD2ClientConfig()->getRemoveOnRenew()); + + // Create and process a lease request so we have a lease to release. 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. + // Verify that there is one NameChangeRequest generated for lease. ASSERT_EQ(1, srv_->name_change_reqs_.size()); verifyNameChangeRequest(isc::dhcp_ddns::CHG_ADD, true, true, reply->getYiaddr().toText(), "myhost.example.com.", @@ -764,18 +948,96 @@ TEST_F(NameDhcpv4SrvTest, processRequestRelease) { "965B68B6D438D98E680BF10B09F3BCF", time(NULL), subnet_->getValid(), true); - // Create a Release message. + // Create and process the 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()); + verifyNameChangeRequest(isc::dhcp_ddns::CHG_REMOVE, true, true, + reply->getYiaddr().toText(), "myhost.example.com.", + "00010132E91AA355CFBB753C0F0497A5A940436" + "965B68B6D438D98E680BF10B09F3BCF", + time(NULL), subnet_->getValid(), true); } +// Test that when a Release message is sent for a previously acquired +// lease and DDNS updates are enabled but remove-on-renew is off that server +// does NOT generate NameChangeRequest to remove entries corresponding to +// the released lease. +TEST_F(NameDhcpv4SrvTest, processRequestReleaseRemoveOff) { + // Verify the config is as expected. + ASSERT_TRUE(CfgMgr::instance().ddnsEnabled()); + ASSERT_FALSE(CfgMgr::instance().getD2ClientConfig()->getRemoveOnRenew()); + + // Create and process a lease request so we have a lease to release. + 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 for the new lease. + ASSERT_EQ(1, srv_->name_change_reqs_.size()); + verifyNameChangeRequest(isc::dhcp_ddns::CHG_ADD, true, true, + reply->getYiaddr().toText(), "myhost.example.com.", + "00010132E91AA355CFBB753C0F0497A5A940436" + "965B68B6D438D98E680BF10B09F3BCF", + time(NULL), subnet_->getValid(), true); + + // Create and process the 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)); + + // With remove-on-renew off, there should be not be a NameChangeRequest + // for the remove. + ASSERT_EQ(0, srv_->name_change_reqs_.size()); +} + +// Test that when the Release message is sent for a previously acquired lease +// and DDNS updates are disabled that server does NOT generate a +// NameChangeRequest to remove entries corresponding to the released lease. +TEST_F(NameDhcpv4SrvTest, processRequestReleaseUpdatesDisabled) { + // Disable DDNS. + disableD2(); + ASSERT_FALSE(CfgMgr::instance().ddnsEnabled()); + + // Create and process a lease request so we have a lease to release. + 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); + + // With DDNS updates disabled, there should be not be a NameChangeRequest + // for the add. + ASSERT_EQ(0, srv_->name_change_reqs_.size()); + + // Create and process the 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)); + + // With DDNS updates disabled, there should be not be a NameChangeRequest + // for the remove. + ASSERT_EQ(0, srv_->name_change_reqs_.size()); +} + + } // end of anonymous namespace