From: Tomek Mrugalski Date: Tue, 24 Jul 2018 18:41:07 +0000 (+0200) Subject: [5682] Consistency checks implemented in libdhcpsrv X-Git-Tag: ha_phase2~25 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=5bf6881bbf1013fecaee446066433ec10e8d2b9b;p=thirdparty%2Fkea.git [5682] Consistency checks implemented in libdhcpsrv --- diff --git a/src/lib/dhcpsrv/Makefile.am b/src/lib/dhcpsrv/Makefile.am index f8ef361b51..35ad7cf1df 100644 --- a/src/lib/dhcpsrv/Makefile.am +++ b/src/lib/dhcpsrv/Makefile.am @@ -43,6 +43,8 @@ EXTRA_DIST += parsers/host_reservations_list_parser.h EXTRA_DIST += parsers/ifaces_config_parser.cc EXTRA_DIST += parsers/ifaces_config_parser.h EXTRA_DIST += parsers/option_data_parser.h +EXTRA_DIST += parsers/sanity_checks_parser.cc +EXTRA_DIST += parsers/sanity_checks_parser.h EXTRA_DIST += parsers/simple_parser4.cc EXTRA_DIST += parsers/simple_parser4.h EXTRA_DIST += parsers/simple_parser6.cc @@ -92,6 +94,7 @@ libkea_dhcpsrv_la_SOURCES += base_host_data_source.h libkea_dhcpsrv_la_SOURCES += cache_host_data_source.h libkea_dhcpsrv_la_SOURCES += callout_handle_store.h libkea_dhcpsrv_la_SOURCES += cfg_4o6.cc cfg_4o6.h +libkea_dhcpsrv_la_SOURCES += cfg_consistency.cc fg_consistency.h libkea_dhcpsrv_la_SOURCES += cfg_db_access.cc cfg_db_access.h libkea_dhcpsrv_la_SOURCES += cfg_duid.cc cfg_duid.h libkea_dhcpsrv_la_SOURCES += cfg_hosts.cc cfg_hosts.h @@ -162,6 +165,7 @@ libkea_dhcpsrv_la_SOURCES += cql_lease_mgr.cc cql_lease_mgr.h endif libkea_dhcpsrv_la_SOURCES += pool.cc pool.h +libkea_dhcpsrv_la_SOURCES += sanity_checker.cc sanity_checker.h libkea_dhcpsrv_la_SOURCES += shared_network.cc shared_network.h libkea_dhcpsrv_la_SOURCES += srv_config.cc srv_config.h libkea_dhcpsrv_la_SOURCES += subnet.cc subnet.h @@ -190,6 +194,8 @@ libkea_dhcpsrv_la_SOURCES += parsers/ifaces_config_parser.cc libkea_dhcpsrv_la_SOURCES += parsers/ifaces_config_parser.h libkea_dhcpsrv_la_SOURCES += parsers/option_data_parser.cc libkea_dhcpsrv_la_SOURCES += parsers/option_data_parser.h +libkea_dhcpsrv_la_SOURCES += parsers/sanity_checks_parser.cc +libkea_dhcpsrv_la_SOURCES += parsers/sanity_checks_parser.h libkea_dhcpsrv_la_SOURCES += parsers/shared_network_parser.cc libkea_dhcpsrv_la_SOURCES += parsers/shared_network_parser.h libkea_dhcpsrv_la_SOURCES += parsers/shared_networks_list_parser.h diff --git a/src/lib/dhcpsrv/cfg_consistency.cc b/src/lib/dhcpsrv/cfg_consistency.cc new file mode 100644 index 0000000000..3ea5fb7086 --- /dev/null +++ b/src/lib/dhcpsrv/cfg_consistency.cc @@ -0,0 +1,43 @@ +// Copyright (C) 2018 Internet Systems Consortium, Inc. ("ISC") +// +// This Source Code Form is subject to the terms of the Mozilla Public +// License, v. 2.0. If a copy of the MPL was not distributed with this +// file, You can obtain one at http://mozilla.org/MPL/2.0/. + +#include +#include + +using namespace isc::data; + +namespace isc { +namespace dhcp { + +isc::data::ElementPtr CfgConsistency::toElement() const { + ElementPtr m(new MapElement()); + ElementPtr l(new StringElement(sanityCheckToText(getLeaseSanityCheck()))); + m->set("lease-checks", l); + + return (m); +} + +std::string CfgConsistency::sanityCheckToText(LeaseSanity check_type) { + switch (check_type) { + case LEASE_CHECK_NONE: + return ("none"); + case LEASE_CHECK_WARN: + return ("warn"); + case LEASE_CHECK_FIX: + return ("fix"); + case LEASE_CHECK_FIX_DEL: + return ("fix-del"); + case LEASE_CHECK_DEL: + return ("del"); + default: + return ("unknown"); + } +} + +}; +}; + + diff --git a/src/lib/dhcpsrv/cfg_consistency.h b/src/lib/dhcpsrv/cfg_consistency.h new file mode 100644 index 0000000000..d379b9ffec --- /dev/null +++ b/src/lib/dhcpsrv/cfg_consistency.h @@ -0,0 +1,77 @@ +// Copyright (C) 2018 Internet Systems Consortium, Inc. ("ISC") +// +// This Source Code Form is subject to the terms of the Mozilla Public +// License, v. 2.0. If a copy of the MPL was not distributed with this +// file, You can obtain one at http://mozilla.org/MPL/2.0/. + +#ifndef CFG_CONSISTENCY_H +#define CFG_CONSISTENCY_H + +#include +#include + +namespace isc { +namespace dhcp { + + +/// @brief Parameters for various consistency checks. +/// +class CfgConsistency : public isc::dhcp::UserContext, public isc::data::CfgToElement { + + public: + + /// @brief Values for subnet-id sanity checks done for leases. + enum LeaseSanity { + LEASE_CHECK_NONE, // Skip sanity checks + LEASE_CHECK_WARN, // Print a warning if subnet-id is incorrect. + LEASE_CHECK_FIX, // If subnet-id is incorrect, try to fix it (try to pick + // appropriate subnet, but if it fails, keep the lease, + // despite its broken subnet-id. + LEASE_CHECK_FIX_DEL, // If subnet-id is incorrect, try to fix it (try to pick + // appropriate subnet. If it fails, delete broken lease. + LEASE_CHECK_DEL // Delete leases with invalid subnet-id. + }; + + /// @brief Constructor + CfgConsistency() + : lease_sanity_check_(LEASE_CHECK_WARN) { + + } + + /// @brief Returns JSON representation + /// + /// @return Element pointer + virtual isc::data::ElementPtr toElement() const; + + /// @brief Sets specific sanity checks mode for leases. + /// + /// @param l sanity checks mode + void setLeaseSanityCheck(LeaseSanity l) { + lease_sanity_check_ = l; + } + + /// @brief Returns specific sanity checks mode for leases. + /// + /// @return sanity checks mode + LeaseSanity getLeaseSanityCheck() const { + return (lease_sanity_check_); + } + + /// @brief Converts sanity check value to printable text + /// + /// @param check_type sanity mode to be converted + static std::string sanityCheckToText(LeaseSanity check_type); + + private: + + /// @brief sanity checks mode + LeaseSanity lease_sanity_check_; +}; + +/// @brief Type used to for pointing to CfgConsistency structure +typedef boost::shared_ptr CfgConsistencyPtr; + +}; // namespace isc::dhcp +}; // namespace isc + +#endif /* CFG_CONSISTENCY_H */ diff --git a/src/lib/dhcpsrv/dhcpsrv_messages.mes b/src/lib/dhcpsrv/dhcpsrv_messages.mes index edf22403c7..39614ed5bc 100644 --- a/src/lib/dhcpsrv/dhcpsrv_messages.mes +++ b/src/lib/dhcpsrv/dhcpsrv_messages.mes @@ -430,6 +430,12 @@ and the attempt ended in error. The access string in question - which should be of the form 'keyword=value keyword=value...' is included in the message. +% DHCPSRV_LEASE_SANITY_FAIL The lease %1 with subnet-id %2 failed subnet-id checks. + +% DHCPSRV_LEASE_SANITY_FIXED The lease %2 with subnet-id %2 failed subnet-id checks, but was correced to subnet-id %3. + +% DHCPSRV_LEASE_SANITY_FAIL_DISCARD The lease %2 with subnet-id %2 failed subnet-id checks and was dropped. + % DHCPSRV_MEMFILE_ADD_ADDR4 adding IPv4 lease with address %1 A debug message issued when the server is about to add an IPv4 lease with the specified address to the memory file backend database. diff --git a/src/lib/dhcpsrv/lease_file_loader.h b/src/lib/dhcpsrv/lease_file_loader.h index cc9385c06a..0e98ebeedb 100644 --- a/src/lib/dhcpsrv/lease_file_loader.h +++ b/src/lib/dhcpsrv/lease_file_loader.h @@ -10,6 +10,7 @@ #include #include #include +#include #include @@ -85,6 +86,8 @@ public: lease_file.close(); lease_file.open(); + SanityChecker lease_checker; + boost::shared_ptr lease; // Track the number of corrupted leases. uint32_t errcnt = 0; @@ -122,6 +125,12 @@ public: DHCPSRV_MEMFILE_LEASE_LOAD) .arg(lease->toText()); + // Now see if we need to sanitize this lease. + lease_checker.checkLease(lease); + if (!lease) { + continue; + } + // Check if this lease exists. typename StorageType::iterator lease_it = storage.find(lease->addr_); diff --git a/src/lib/dhcpsrv/lease_mgr.h b/src/lib/dhcpsrv/lease_mgr.h index 852e5b2da8..5b3e98d4b8 100644 --- a/src/lib/dhcpsrv/lease_mgr.h +++ b/src/lib/dhcpsrv/lease_mgr.h @@ -261,19 +261,29 @@ public: /// @brief Adds an IPv4 lease. /// + /// The lease may be modified due to sanity checks setting (see + /// LeaseSanityChecks in CfgConsistency) before being inserted. For + /// performance reasons, the sanity checks do not make a copy, but rather + /// modify lease in place if needed. + /// /// @param lease lease to be added /// /// @result true if the lease was added, false if not (because a lease - /// with the same address was already there). - virtual bool addLease(const Lease4Ptr& lease) = 0; + /// with the same address was already there or failed sanity checks) + virtual bool addLease(Lease4Ptr& lease) = 0; /// @brief Adds an IPv6 lease. /// + /// The lease may be modified due to sanity checks setting (see + /// LeaseSanityChecks in CfgConsistency) before being inserted. For + /// performance reasons, the sanity checks do not make a copy, but rather + /// modify lease in place if needed. + /// /// @param lease lease to be added /// /// @result true if the lease was added, false if not (because a lease - /// with the same address was already there). - virtual bool addLease(const Lease6Ptr& lease) = 0; + /// with the same address was already there or failed sanity checks) + virtual bool addLease(Lease6Ptr& lease) = 0; /// @brief Returns an IPv4 lease for specified IPv4 address /// @@ -749,9 +759,6 @@ public: /// support transactions, this is a no-op. virtual void rollback() = 0; - /// @todo: Add host management here - /// As host reservation is outside of scope for 2012, support for hosts - /// is currently postponed. }; } // namespace dhcp diff --git a/src/lib/dhcpsrv/memfile_lease_mgr.cc b/src/lib/dhcpsrv/memfile_lease_mgr.cc index a4b4a1114c..f21dcc1103 100644 --- a/src/lib/dhcpsrv/memfile_lease_mgr.cc +++ b/src/lib/dhcpsrv/memfile_lease_mgr.cc @@ -10,6 +10,7 @@ #include #include #include +#include #include #include #include @@ -690,7 +691,7 @@ Memfile_LeaseMgr::getDBVersion() { } bool -Memfile_LeaseMgr::addLease(const Lease4Ptr& lease) { +Memfile_LeaseMgr::addLease(Lease4Ptr& lease) { LOG_DEBUG(dhcpsrv_logger, DHCPSRV_DBG_TRACE_DETAIL, DHCPSRV_MEMFILE_ADD_ADDR4).arg(lease->addr_.toText()); @@ -699,6 +700,13 @@ Memfile_LeaseMgr::addLease(const Lease4Ptr& lease) { return (false); } + // Run the lease through a checker. + static SanityChecker checker; + checker.checkLease(lease); + if (!lease) { + return (false); + } + // Try to write a lease to disk first. If this fails, the lease will // not be inserted to the memory and the disk and in-memory data will // remain consistent. @@ -711,7 +719,7 @@ Memfile_LeaseMgr::addLease(const Lease4Ptr& lease) { } bool -Memfile_LeaseMgr::addLease(const Lease6Ptr& lease) { +Memfile_LeaseMgr::addLease(Lease6Ptr& lease) { LOG_DEBUG(dhcpsrv_logger, DHCPSRV_DBG_TRACE_DETAIL, DHCPSRV_MEMFILE_ADD_ADDR6).arg(lease->addr_.toText()); @@ -720,6 +728,13 @@ Memfile_LeaseMgr::addLease(const Lease6Ptr& lease) { return (false); } + // Run the lease through a checker. + static SanityChecker checker; + checker.checkLease(lease); + if (!lease) { + return (false); + } + // Try to write a lease to disk first. If this fails, the lease will // not be inserted to the memory and the disk and in-memory data will // remain consistent. diff --git a/src/lib/dhcpsrv/memfile_lease_mgr.h b/src/lib/dhcpsrv/memfile_lease_mgr.h index 66dedccb0d..196ebb3ad3 100644 --- a/src/lib/dhcpsrv/memfile_lease_mgr.h +++ b/src/lib/dhcpsrv/memfile_lease_mgr.h @@ -141,12 +141,12 @@ public: /// @brief Adds an IPv4 lease. /// /// @param lease lease to be added - virtual bool addLease(const Lease4Ptr& lease); + virtual bool addLease(Lease4Ptr& lease); /// @brief Adds an IPv6 lease. /// /// @param lease lease to be added - virtual bool addLease(const Lease6Ptr& lease); + virtual bool addLease(Lease6Ptr& lease); /// @brief Returns existing IPv4 lease for specified IPv4 address. /// diff --git a/src/lib/dhcpsrv/parsers/sanity_checks_parser.cc b/src/lib/dhcpsrv/parsers/sanity_checks_parser.cc new file mode 100644 index 0000000000..6210857a91 --- /dev/null +++ b/src/lib/dhcpsrv/parsers/sanity_checks_parser.cc @@ -0,0 +1,55 @@ +// Copyright (C) 2018 Internet Systems Consortium, Inc. ("ISC") +// +// This Source Code Form is subject to the terms of the Mozilla Public +// License, v. 2.0. If a copy of the MPL was not distributed with this +// file, You can obtain one at http://mozilla.org/MPL/2.0/. + +#include +#include +#include +#include + +using namespace isc::data; + +namespace isc { +namespace dhcp { + +void +SanityChecksParser::parse(SrvConfig& cfg, const ConstElementPtr& sanity_checks) { + + if (!sanity_checks) { + return; + } + if (sanity_checks->getType() != Element::map) { + isc_throw(DhcpConfigError, "sanity-checks is supposed to be a map"); + } + + ConstElementPtr lease_checks = sanity_checks->get("lease-checks"); + if (lease_checks) { + if (lease_checks->getType() != Element::string) { + isc_throw(DhcpConfigError, "lease-checks must be a string"); + } + std::string lc = lease_checks->stringValue(); + CfgConsistency::LeaseSanity check; + if (lc == "none") { + check = CfgConsistency::LEASE_CHECK_NONE; + } else if (lc == "warn") { + check = CfgConsistency::LEASE_CHECK_WARN; + } else if (lc == "fix") { + check = CfgConsistency::LEASE_CHECK_FIX; + } else if (lc == "fix-del") { + check = CfgConsistency::LEASE_CHECK_FIX_DEL; + } else if (lc == "del") { + check = CfgConsistency::LEASE_CHECK_DEL; + } else { + isc_throw(DhcpConfigError, "Unsupported lease-checks value: " << lc + << ", supported values are: none, warn, fix, fix-del, del"); + } + cfg.getConsistency()->setLeaseSanityCheck(check); + } + + // Additional sanity check fields will come in later here. +} + +}; +}; diff --git a/src/lib/dhcpsrv/parsers/sanity_checks_parser.h b/src/lib/dhcpsrv/parsers/sanity_checks_parser.h new file mode 100644 index 0000000000..a9dab5ecc4 --- /dev/null +++ b/src/lib/dhcpsrv/parsers/sanity_checks_parser.h @@ -0,0 +1,32 @@ +// Copyright (C) 2018 Internet Systems Consortium, Inc. ("ISC") +// +// This Source Code Form is subject to the terms of the Mozilla Public +// License, v. 2.0. If a copy of the MPL was not distributed with this +// file, You can obtain one at http://mozilla.org/MPL/2.0/. + +#ifndef SANITY_CHECKS_PARSER_H +#define SANITY_CHECKS_PARSER_H + +#include +#include + +namespace isc { +namespace dhcp { + +/// @brief Simple parser for sanity-checks structure +/// +/// Currently parses only one parameter: +/// - lease-checks. Allowed values: none, warn, fix, fix-del, del +class SanityChecksParser : public isc::data::SimpleParser { + public: + /// @brief parses JSON structure + /// + /// @param srv_cfg parsed value will be stored here + /// @param value a JSON map that contains lease-checks parameter. + void parse(SrvConfig& srv_cfg, const isc::data::ConstElementPtr& value); +}; + +}; +}; + +#endif /* SANITY_CHECKS_PARSER_H */ diff --git a/src/lib/dhcpsrv/sanity_checker.cc b/src/lib/dhcpsrv/sanity_checker.cc new file mode 100644 index 0000000000..6328bf22d4 --- /dev/null +++ b/src/lib/dhcpsrv/sanity_checker.cc @@ -0,0 +1,119 @@ +// Copyright (C) 2018 Internet Systems Consortium, Inc. ("ISC") +// +// This Source Code Form is subject to the terms of the Mozilla Public +// License, v. 2.0. If a copy of the MPL was not distributed with this +// file, You can obtain one at http://mozilla.org/MPL/2.0/. +#include +#include +#include +#include +#include +#include + +namespace isc { +namespace dhcp { + +void SanityChecker::checkLease(Lease4Ptr& lease) { + CfgConsistencyPtr sanity = CfgMgr::instance().getCurrentCfg()->getConsistency(); + CfgSubnets4Ptr subnets = CfgMgr::instance().getCurrentCfg()->getCfgSubnets4(); + checkLeaseInternal(lease, sanity, subnets); +} + +void SanityChecker::checkLease(Lease6Ptr& lease) { + CfgConsistencyPtr sanity = CfgMgr::instance().getCurrentCfg()->getConsistency(); + CfgSubnets6Ptr subnets = CfgMgr::instance().getCurrentCfg()->getCfgSubnets6(); + checkLeaseInternal(lease, sanity, subnets); +} + +template +void SanityChecker::checkLeaseInternal(LeasePtrType lease, const CfgConsistencyPtr& checks, + const SubnetsType& subnets) { + + if (checks->getLeaseSanityCheck() == CfgConsistency::LEASE_CHECK_NONE) { + return; + } + + auto subnet = subnets->getBySubnetId(lease->subnet_id_); + + if (subnet && subnet->inRange(lease->addr_)) { + + // If the subnet is defined and the address is in range, we're good. + + return; + } + + // Ok, if we got here, that means that either we did not find a subnet + // of found it, but it wasn't the right subnet. + SubnetID id = findSubnetId(lease, subnets); + + switch (checks->getLeaseSanityCheck()) { + case CfgConsistency::LEASE_CHECK_NONE: + // No checks whatsoever, just return the lease as-is. + break; + case CfgConsistency::LEASE_CHECK_WARN: + if (lease->subnet_id_ != id) { + // Print a warning, but return the lease as is. + LOG_WARN(dhcpsrv_logger, DHCPSRV_LEASE_SANITY_FAIL) + .arg(lease->addr_.toText()).arg(lease->subnet_id_); + } + break; + + case CfgConsistency::LEASE_CHECK_FIX: + if (lease->subnet_id_ != id) { + + // If there is a better subnet, use it. + if (id != 0) { + LOG_INFO(dhcpsrv_logger, DHCPSRV_LEASE_SANITY_FIXED) + .arg(lease->addr_.toText()).arg(lease->subnet_id_).arg(id); + lease->subnet_id_ = id; + } + + // If not, return the lease as is. + } + break; + + case CfgConsistency::LEASE_CHECK_FIX_DEL: + if (lease->subnet_id_ != id) { + + // If there is a better subnet, use it. + if (id != 0) { + LOG_INFO(dhcpsrv_logger, DHCPSRV_LEASE_SANITY_FAIL_DISCARD) + .arg(lease->addr_.toText()).arg(lease->subnet_id_); + lease->subnet_id_ = id; + break; + } else { + // If not, delete the lease. + lease.reset(); + } + + } + break; + case CfgConsistency::LEASE_CHECK_DEL: + if (lease->subnet_id_ != id) { + LOG_INFO(dhcpsrv_logger, DHCPSRV_LEASE_SANITY_FAIL_DISCARD) + .arg(lease->addr_.toText()).arg(lease->subnet_id_); + lease.reset(); + } + break; + } + + // Additional checks may be implemented in the future here. + + /// @todo: add a check if the address is within specified dynamic pool + /// if not, check if the address is reserved. +} + +template +SubnetID SanityChecker::findSubnetId(const LeaseType& lease, const SubnetsType& subnets) { + //CfgSubnets4Ptr subnets = CfgMgr::instance().getCurrentCfg()->getCfgSubnets4(); + + auto subnet = subnets->selectSubnet(lease->addr_); + if (!subnet) { + return (0); + } + + return (subnet->getID()); +} + +}; +}; diff --git a/src/lib/dhcpsrv/sanity_checker.h b/src/lib/dhcpsrv/sanity_checker.h new file mode 100644 index 0000000000..6cc46bc59d --- /dev/null +++ b/src/lib/dhcpsrv/sanity_checker.h @@ -0,0 +1,73 @@ +// Copyright (C) 2018 Internet Systems Consortium, Inc. ("ISC") +// +// This Source Code Form is subject to the terms of the Mozilla Public +// License, v. 2.0. If a copy of the MPL was not distributed with this +// file, You can obtain one at http://mozilla.org/MPL/2.0/. + +#ifndef SANITY_CHECKER_H +#define SANITY_CHECKER_H + +#include +#include + +namespace isc { +namespace dhcp { + +/// @brief Code used to conduct various sanity checks. Currently used for leases. +/// +/// This class is expected to be used as a simple interface sanity checker for +/// various run-time and configuration elements. Currently is provides sanity +/// checking and correction for subnet-id parameter in leases. +class SanityChecker { + public: + + /// @brief Sanity checks and possibly corrects an IPv4 lease + /// + /// Depending on the sanity-checks/lease-checks parameter value (see + /// @ref CfgConsistency for details), this code may print a warning, + /// correct subnet-id or discard the lease. + /// + /// @param lease Lease to be sanity-checked + void checkLease(Lease4Ptr& lease); + + /// @brief Sanity checks and possibly corrects an IPv4 lease + /// + /// Depending on the sanity-checks/lease-checks parameter value (see + /// @ref CfgConsistency for details), this code may print a warning, + /// correct subnet-id or discard the lease. + /// + /// @param lease Lease to be sanity-checked + void checkLease(Lease6Ptr& lease); + + private: + + /// @brief Internal implementation for checkLease command + /// + /// @tparam LeaseType type of the lease (Lease4Ptr or Lease6Ptr) + /// @tparam SubnetsType type of the subnets container (CfgSubnets4Ptr or + /// CfgSubnets6Ptr) + /// @param lease a lease to be checked/corrected + /// @param checks a pointer to CfgConsistency structure (type of checks + /// specified here) + /// @param subnets configuration structure with subnets + template + void checkLeaseInternal(LeaseType lease, const CfgConsistencyPtr& checks, + const SubnetsType& subnets); + + /// @brief Internal method for finding appropriate subnet-id + /// + /// @tparam LeaseType type of the lease (Lease4Ptr or Lease6Ptr) + /// @tparam SubnetsType type of the subnets container (CfgSubnets4Ptr or + /// CfgSubnets6Ptr) + /// @param lease a lease to be checked/corrected + /// @param subnets configuration structure with subnets + template + SubnetID findSubnetId(const LeaseType& lease, const SubnetsType& subnets); +}; + + +}; +}; + + +#endif /* SANITY_CHECKER_H */ diff --git a/src/lib/dhcpsrv/srv_config.cc b/src/lib/dhcpsrv/srv_config.cc index c2e53b073c..a75bc2f02a 100644 --- a/src/lib/dhcpsrv/srv_config.cc +++ b/src/lib/dhcpsrv/srv_config.cc @@ -35,7 +35,8 @@ SrvConfig::SrvConfig() class_dictionary_(new ClientClassDictionary()), decline_timer_(0), echo_v4_client_id_(true), dhcp4o6_port_(0), d2_client_config_(new D2ClientConfig()), - configured_globals_(Element::createMap()) { + configured_globals_(Element::createMap()), + cfg_consist_(new CfgConsistency()) { } SrvConfig::SrvConfig(const uint32_t sequence) @@ -52,7 +53,8 @@ SrvConfig::SrvConfig(const uint32_t sequence) class_dictionary_(new ClientClassDictionary()), decline_timer_(0), echo_v4_client_id_(true), dhcp4o6_port_(0), d2_client_config_(new D2ClientConfig()), - configured_globals_(Element::createMap()) { + configured_globals_(Element::createMap()), + cfg_consist_(new CfgConsistency()) { } std::string @@ -413,6 +415,9 @@ SrvConfig::toElement() const { result->set("Logging", logging); } + ConstElementPtr cfg_consist = cfg_consist_->toElement(); + dhcp->set("sanity-checks", cfg_consist); + return (result); } diff --git a/src/lib/dhcpsrv/srv_config.h b/src/lib/dhcpsrv/srv_config.h index 98e5580f11..02af78e1c9 100644 --- a/src/lib/dhcpsrv/srv_config.h +++ b/src/lib/dhcpsrv/srv_config.h @@ -21,6 +21,7 @@ #include #include #include +#include #include #include #include @@ -341,6 +342,11 @@ public: return (cfg_host_operations6_); } + /// @brief Returns const pointer to object holding sanity checks flags + CfgConsistencyPtr getConsistency() { + return (cfg_consist_); + } + //@} /// @brief Returns non-const reference to an array that stores @@ -679,6 +685,9 @@ private: /// @brief Stores the global parameters specified via configuration isc::data::ElementPtr configured_globals_; + + /// @brief Pointer to the configuration consistency settings + CfgConsistencyPtr cfg_consist_; }; /// @name Pointers to the @c SrvConfig object. diff --git a/src/lib/dhcpsrv/tests/Makefile.am b/src/lib/dhcpsrv/tests/Makefile.am index 4c1f99a177..72afb9f705 100644 --- a/src/lib/dhcpsrv/tests/Makefile.am +++ b/src/lib/dhcpsrv/tests/Makefile.am @@ -123,6 +123,7 @@ libdhcpsrv_unittests_SOURCES += cql_lease_mgr_unittest.cc libdhcpsrv_unittests_SOURCES += cql_host_data_source_unittest.cc endif libdhcpsrv_unittests_SOURCES += pool_unittest.cc +libdhcpsrv_unittests_SOURCES += sanity_checks_unittest.cc libdhcpsrv_unittests_SOURCES += shared_network_parser_unittest.cc libdhcpsrv_unittests_SOURCES += shared_network_unittest.cc libdhcpsrv_unittests_SOURCES += shared_networks_list_parser_unittest.cc diff --git a/src/lib/dhcpsrv/tests/alloc_engine_expiration_unittest.cc b/src/lib/dhcpsrv/tests/alloc_engine_expiration_unittest.cc index 997fe9549b..ba416fcdbd 100644 --- a/src/lib/dhcpsrv/tests/alloc_engine_expiration_unittest.cc +++ b/src/lib/dhcpsrv/tests/alloc_engine_expiration_unittest.cc @@ -1,4 +1,4 @@ -// Copyright (C) 2015-2017 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2015-2018 Internet Systems Consortium, Inc. ("ISC") // // This Source Code Form is subject to the terms of the Mozilla Public // License, v. 2.0. If a copy of the MPL was not distributed with this @@ -1277,7 +1277,8 @@ ExpirationAllocEngine6Test::createLeases() { // Copy the lease before adding it to the lease manager. We want to // make sure that modifications to the leases held in the leases_ // container doesn't affect the leases in the lease manager. - LeaseMgrFactory::instance().addLease(Lease6Ptr(new Lease6(*lease))); + Lease6Ptr tmp(new Lease6(*lease)); + LeaseMgrFactory::instance().addLease(tmp); // Note in the statistics that this lease has been added. StatsMgr& stats_mgr = StatsMgr::instance(); @@ -1838,7 +1839,8 @@ ExpirationAllocEngine4Test::createLeases() { // Copy the lease before adding it to the lease manager. We want to // make sure that modifications to the leases held in the leases_ // container doesn't affect the leases in the lease manager. - LeaseMgrFactory::instance().addLease(Lease4Ptr(new Lease4(*lease))); + Lease4Ptr tmp(new Lease4(*lease)); + LeaseMgrFactory::instance().addLease(tmp); // Note in the statistics that this lease has been added. StatsMgr& stats_mgr = StatsMgr::instance(); diff --git a/src/lib/dhcpsrv/tests/lease_mgr_unittest.cc b/src/lib/dhcpsrv/tests/lease_mgr_unittest.cc index d32379fe8d..66a6c8ddc2 100644 --- a/src/lib/dhcpsrv/tests/lease_mgr_unittest.cc +++ b/src/lib/dhcpsrv/tests/lease_mgr_unittest.cc @@ -46,14 +46,14 @@ public: /// @brief Adds an IPv4 lease. /// /// @param lease lease to be added - virtual bool addLease(const Lease4Ptr&) { + virtual bool addLease(Lease4Ptr&) { return (false); } /// @brief Adds an IPv6 lease. /// /// @param lease lease to be added - virtual bool addLease(const Lease6Ptr&) { + virtual bool addLease(Lease6Ptr&) { return (false); } diff --git a/src/lib/dhcpsrv/tests/sanity_checks_unittest.cc b/src/lib/dhcpsrv/tests/sanity_checks_unittest.cc new file mode 100644 index 0000000000..8f76c155d3 --- /dev/null +++ b/src/lib/dhcpsrv/tests/sanity_checks_unittest.cc @@ -0,0 +1,269 @@ +// Copyright (C) 2018 Internet Systems Consortium, Inc. ("ISC") +// +// This Source Code Form is subject to the terms of the Mozilla Public +// License, v. 2.0. If a copy of the MPL was not distributed with this +// file, You can obtain one at http://mozilla.org/MPL/2.0/. + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +using namespace std; +using namespace isc::data; +using namespace isc::asiolink; +using namespace isc::dhcp; +using namespace isc::dhcp::test; + +class SanityChecksTest : public ::testing::Test { +public: + + SanityChecksTest() { + LeaseMgrFactory::destroy(); + } + + void startLeaseBackend(bool v6) { + std::ostringstream s; + s << "type=memfile " << (v6 ? "universe=6 " : "universe=4 ") + << "persist=false lfc-interval=0"; + LeaseMgrFactory::create(s.str()); + } + + void setLeaseCheck(CfgConsistency::LeaseSanity sanity) { + CfgMgr::instance().getCurrentCfg()->getConsistency()->setLeaseSanityCheck(sanity); + } + + ~SanityChecksTest() { + CfgMgr::instance().clear(); + LeaseMgrFactory::destroy(); + } + + /// @brief Generates a simple IPv4 lease. + /// + /// The HW address is randomly generated, subnet_id is specified. + /// + /// @param address Lease address. + /// @param subnet_id ID of the subnet to use. + /// + /// @return new lease with random content + Lease4Ptr newLease4(const IOAddress& address, SubnetID subnet_id) { + + // Randomize HW address. + vector mac(6); + isc::util::fillRandom(mac.begin(), mac.end()); + HWAddrPtr hwaddr(new HWAddr(mac, HTYPE_ETHER)); + + vector clientid(1); + + time_t timestamp = time(NULL) - 86400 + random()%86400; + + // Return created lease. + return (Lease4Ptr(new Lease4(address, hwaddr, + &clientid[0], 0, // no client-id + 1200, 600, 900, // valid, t1, t2 + timestamp, subnet_id, false, false, ""))); + } + + /// @brief Generates a simple IPv6 lease. + /// + /// The DUID and IAID are randomly generated, subnet_id is specified. + /// + /// @param address Lease address. + /// @param subnet_id ID of the subnet to use. + /// + /// @return new lease with random content + Lease6Ptr newLease6(const IOAddress& address, SubnetID subnet_id) { + // Let's generate DUID of random length. + std::vector duid_vec(8 + random()%20); + // And then fill it with random value. + isc::util::fillRandom(duid_vec.begin(), duid_vec.end()); + DuidPtr duid(new DUID(duid_vec)); + + Lease::Type lease_type = Lease::TYPE_NA; + uint32_t iaid = 1 + random()%100; + uint32_t valid_lft = 1200; + uint32_t preferred_lft = 1000; + uint32_t t1 = 600; + uint32_t t2 = 900; + + std::ostringstream hostname; + hostname << "hostname" << (random() % 2048); + + // Return created lease. + Lease6Ptr lease(new Lease6(lease_type, address, duid, iaid, + 1000, 1200, 600, 900, // pref, valid, t1, t2 + subnet_id, + false, false, "")); // fqdn fwd, rev, hostname + return (lease); + } + + Subnet4Ptr createSubnet4(string subnet_txt, SubnetID id) { + size_t pos = subnet_txt.find("/"); + isc::asiolink::IOAddress addr(subnet_txt.substr(0, pos)); + size_t len = boost::lexical_cast(subnet_txt.substr(pos + 1)); + + return (Subnet4Ptr(new Subnet4(addr, len, 1000, 2000, 3000, id))); + } + + void + parserCheck(SrvConfig& cfg, const string& txt, bool exp_throw, + CfgConsistency::LeaseSanity exp_sanity) { + + SanityChecksParser parser; + + ElementPtr json; + EXPECT_NO_THROW(json = Element::fromJSON(txt)); + + if (exp_throw) { + EXPECT_THROW(parser.parse(cfg, json), DhcpConfigError); + + return; + } + + // Should not throw. + EXPECT_NO_THROW(parser.parse(cfg, json)); + + EXPECT_EQ(cfg.getConsistency()->getLeaseSanityCheck(), exp_sanity); + } + + /// @brief Creates subnet configuration, inserts a lease and checks the result. + /// + /// This is a bit complicated test. It creates a Subnet configuration first + /// (defined by subnet/subnet_id), adds it to the current configuration, + /// then sets the lease checks to specified value, then creates a lease + /// (addr/lease_id) and tries to add it to the lease backend (memfile is used). + /// Afterwards it checks whether the lease is added or not (exp_lease_present) + /// and if it is present, whether it has expected subnet-id (expected_id). + /// + /// @param addr Address of the lease to be created + /// @param lease_id Subnet-id of the lease to be created + /// @param subnet_txt Text representation of the subnet, e.g. 192.0.2.0/24 + /// @param subnet_id Subnet-id of the subnet to be created + /// @param sanity Sanity checks set in the configuration + /// @param exp_lease_present whether the lease is expected to be inserted + /// @param expected_id if exp_lease_present is true, lease's subnet id should + /// match this value. + /// @return lease from the backend (may return null) + void + leaseAddCheck4(string addr, SubnetID lease_id, string subnet_txt, SubnetID subnet_id, + CfgConsistency::LeaseSanity sanity, bool exp_lease_present, + SubnetID expected_id) { + + // Let's start a backend in proper universe. + IOAddress a(addr); + startLeaseBackend(a.isV6()); + + // Now set the sanity checks to a proper value. + ASSERT_NO_THROW(CfgMgr::instance().getCurrentCfg()->getConsistency() + ->setLeaseSanityCheck(sanity)); + + // Create the subnet and add it to configuration. + Subnet4Ptr subnet = createSubnet4(subnet_txt, subnet_id); + ASSERT_NO_THROW(CfgMgr::instance().getCurrentCfg()->getCfgSubnets4()->add(subnet)); + + // Now try to insert the lease. + Lease4Ptr l = newLease4(addr, lease_id); + bool result = false; + EXPECT_NO_THROW(result = LeaseMgrFactory::instance().addLease(l)); + EXPECT_EQ(result, exp_lease_present); + + Lease4Ptr from_backend; + if (result && exp_lease_present) { + + ASSERT_TRUE(from_backend = LeaseMgrFactory::instance().getLease4(IOAddress(addr))); + + EXPECT_EQ(l->addr_, from_backend->addr_); + EXPECT_EQ(from_backend->subnet_id_, expected_id); + } + } +}; + +// Verify whether configuration parser is able to understand the values +// that are valid and reject those that are not. +TEST_F(SanityChecksTest, leaseCheck) { + + // These are valid and should be accepted. + string valid1 = "{ \"lease-checks\": \"none\" }"; + string valid2 = "{ \"lease-checks\": \"warn\" }"; + string valid3 = "{ \"lease-checks\": \"fix\" }"; + string valid4 = "{ \"lease-checks\": \"fix-del\" }"; + string valid5 = "{ \"lease-checks\": \"del\" }"; + + // These are not valid values or types. + string bogus1 = "{ \"lease-checks\": \"sanitize\" }"; + string bogus2 = "{ \"lease-checks\": \"ignore\" }"; + string bogus3 = "{ \"lease-checks\": true }"; + string bogus4 = "{ \"lease-checks\": 42 }"; + + SrvConfig cfg; + + // The default should be to warn about inconsistency. + EXPECT_EQ(cfg.getConsistency()->getLeaseSanityCheck(), + CfgConsistency::LEASE_CHECK_WARN); + + parserCheck(cfg, valid1, false, CfgConsistency::LEASE_CHECK_NONE); + parserCheck(cfg, valid2, false, CfgConsistency::LEASE_CHECK_WARN); + parserCheck(cfg, valid3, false, CfgConsistency::LEASE_CHECK_FIX); + parserCheck(cfg, valid4, false, CfgConsistency::LEASE_CHECK_FIX_DEL); + parserCheck(cfg, valid5, false, CfgConsistency::LEASE_CHECK_DEL); + + parserCheck(cfg, bogus1, true, CfgConsistency::LEASE_CHECK_NONE); + parserCheck(cfg, bogus2, true, CfgConsistency::LEASE_CHECK_NONE); + parserCheck(cfg, bogus3, true, CfgConsistency::LEASE_CHECK_NONE); + parserCheck(cfg, bogus4, true, CfgConsistency::LEASE_CHECK_NONE); +} + +// This test checks how the code behaves when there is no subnet matching +// the lease added. +TEST_F(SanityChecksTest, memfileAdd4NoSubnet) { + + startLeaseBackend(false); // false = V4 + + string addr("192.0.2.1"); + SubnetID lease_id = 1; + SubnetID subnet_id = 1; + + Lease4Ptr l = newLease4(addr, lease_id); + + EXPECT_TRUE(LeaseMgrFactory::instance().addLease(l)); + + Lease4Ptr from_backend; + ASSERT_TRUE(from_backend = LeaseMgrFactory::instance().getLease4(IOAddress(addr))); + + // The lease should be returned exactly as is. + detailCompareLease(l, from_backend); + + LeaseMgrFactory::instance().deleteLease(IOAddress(addr)); +} + +TEST_F(SanityChecksTest, memfileAdd4_checksNone) { + leaseAddCheck4("192.0.2.1", 1, "192.0.2.0/24", 2, CfgConsistency::LEASE_CHECK_NONE, true, 1); +} + +TEST_F(SanityChecksTest, memfileAdd4_checksWarn) { + leaseAddCheck4("192.0.2.1", 1, "192.0.2.0/24", 2, CfgConsistency::LEASE_CHECK_WARN, true, 1); +} + +TEST_F(SanityChecksTest, memfileAdd4_checksFix) { + leaseAddCheck4("192.0.2.1", 1, "192.0.2.0/24", 2, CfgConsistency::LEASE_CHECK_FIX, true, 2); +} + +TEST_F(SanityChecksTest, memfileAdd4_checksFixdel1) { + leaseAddCheck4("192.0.2.1", 1, "192.0.2.0/24", 2, CfgConsistency::LEASE_CHECK_FIX_DEL, true, 1); +} + +TEST_F(SanityChecksTest, memfileAdd4_checksFixdel2) { + leaseAddCheck4("192.0.2.1", 1, "192.0.3.0/24", 2, CfgConsistency::LEASE_CHECK_FIX_DEL, false, 0); +} + +TEST_F(SanityChecksTest, memfileAdd4_checksDel) { + leaseAddCheck4("192.0.2.1", 1, "192.0.2.0/24", 2, CfgConsistency::LEASE_CHECK_DEL, false, 0); +}