diff --git a/src/bin/dhcp4/json_config_parser.cc b/src/bin/dhcp4/json_config_parser.cc index 5601d5497f..4ce6a19214 100644 --- a/src/bin/dhcp4/json_config_parser.cc +++ b/src/bin/dhcp4/json_config_parser.cc @@ -182,6 +182,9 @@ public: void sanityChecks(const SrvConfigPtr& cfg, const ConstElementPtr& global) { + /// Global lifetime sanity checks + cfg->sanityChecksLifetime("valid-lifetime"); + /// Shared network sanity checks const SharedNetwork4Collection* networks = cfg->getCfgSharedNetworks4()->getAll(); if (networks) { diff --git a/src/bin/dhcp6/json_config_parser.cc b/src/bin/dhcp6/json_config_parser.cc index 6cb44e7b52..943042ca37 100644 --- a/src/bin/dhcp6/json_config_parser.cc +++ b/src/bin/dhcp6/json_config_parser.cc @@ -269,6 +269,10 @@ public: void sanityChecks(const SrvConfigPtr& cfg, const ConstElementPtr& global) { + /// Global lifetime sanity checks + cfg->sanityChecksLifetime("preferred-lifetime"); + cfg->sanityChecksLifetime("valid-lifetime"); + /// Shared network sanity checks const SharedNetwork6Collection* networks = cfg->getCfgSharedNetworks6()->getAll(); if (networks) { diff --git a/src/bin/dhcp6/tests/config_parser_unittest.cc b/src/bin/dhcp6/tests/config_parser_unittest.cc index 538e5a2be5..1e0106fc33 100644 --- a/src/bin/dhcp6/tests/config_parser_unittest.cc +++ b/src/bin/dhcp6/tests/config_parser_unittest.cc @@ -737,6 +737,7 @@ public: "\"dhcp-ddns\": { \"enable-updates\" : false }, " "\"option-def\": [ ], " "\"option-data\": [ ] }"; + CfgMgr::instance().rollback(); static_cast(executeConfiguration(config, "reset configuration database")); // The default setting is to listen on all interfaces. In order to diff --git a/src/lib/dhcpsrv/cb_ctl_dhcp4.cc b/src/lib/dhcpsrv/cb_ctl_dhcp4.cc index d7ba1e8a68..3f07a6a1e7 100644 --- a/src/lib/dhcpsrv/cb_ctl_dhcp4.cc +++ b/src/lib/dhcpsrv/cb_ctl_dhcp4.cc @@ -79,6 +79,9 @@ CBControlDHCPv4::databaseConfigApply(const BackendSelector& backend_selector, // Add defaults. external_cfg->applyDefaultsConfiguredGlobals(SimpleParser4::GLOBAL4_DEFAULTS); + // Sanity check it. + external_cfg->sanityChecksLifetime("valid-lifetime"); + // Now that we successfully fetched the new global parameters, let's // remove existing ones and merge them into the current configuration. cfg->clearConfiguredGlobals(); @@ -234,6 +237,8 @@ CBControlDHCPv4::databaseConfigApply(const BackendSelector& backend_selector, // If we're configuring the server after startup, we do not apply the // ip-reservations-unique setting here. It will be applied when the // configuration is committed. + auto const& cfg = CfgMgr::instance().getStagingCfg(); + external_cfg->sanityChecksLifetime(*cfg, "valid-lifetime"); CfgMgr::instance().mergeIntoStagingCfg(external_cfg->getSequence()); } else { if (globals_fetched) { @@ -253,6 +258,8 @@ CBControlDHCPv4::databaseConfigApply(const BackendSelector& backend_selector, external_cfg->addConfiguredGlobal("ip-reservations-unique", Element::create(true)); } } + auto const& cfg = CfgMgr::instance().getCurrentCfg(); + external_cfg->sanityChecksLifetime(*cfg, "valid-lifetime"); CfgMgr::instance().mergeIntoCurrentCfg(external_cfg->getSequence()); } LOG_INFO(dhcpsrv_logger, DHCPSRV_CFGMGR_CONFIG4_MERGED); diff --git a/src/lib/dhcpsrv/cb_ctl_dhcp6.cc b/src/lib/dhcpsrv/cb_ctl_dhcp6.cc index 2882bcbc25..bb4db53670 100644 --- a/src/lib/dhcpsrv/cb_ctl_dhcp6.cc +++ b/src/lib/dhcpsrv/cb_ctl_dhcp6.cc @@ -78,6 +78,10 @@ CBControlDHCPv6::databaseConfigApply(const db::BackendSelector& backend_selector // Add defaults. external_cfg->applyDefaultsConfiguredGlobals(SimpleParser6::GLOBAL6_DEFAULTS); + // Sanity check it. + external_cfg->sanityChecksLifetime("preferred-lifetime"); + external_cfg->sanityChecksLifetime("valid-lifetime"); + // Now that we successfully fetched the new global parameters, let's // remove existing ones and merge them into the current configuration. cfg->clearConfiguredGlobals(); @@ -233,6 +237,9 @@ CBControlDHCPv6::databaseConfigApply(const db::BackendSelector& backend_selector // If we're configuring the server after startup, we do not apply the // ip-reservations-unique setting here. It will be applied when the // configuration is committed. + auto const& cfg = CfgMgr::instance().getStagingCfg(); + external_cfg->sanityChecksLifetime(*cfg, "preferred-lifetime"); + external_cfg->sanityChecksLifetime(*cfg, "valid-lifetime"); CfgMgr::instance().mergeIntoStagingCfg(external_cfg->getSequence()); } else { if (globals_fetched) { @@ -252,6 +259,9 @@ CBControlDHCPv6::databaseConfigApply(const db::BackendSelector& backend_selector external_cfg->addConfiguredGlobal("ip-reservations-unique", Element::create(true)); } } + auto const& cfg = CfgMgr::instance().getCurrentCfg(); + external_cfg->sanityChecksLifetime(*cfg, "preferred-lifetime"); + external_cfg->sanityChecksLifetime(*cfg, "valid-lifetime"); CfgMgr::instance().mergeIntoCurrentCfg(external_cfg->getSequence()); } LOG_INFO(dhcpsrv_logger, DHCPSRV_CFGMGR_CONFIG6_MERGED); diff --git a/src/lib/dhcpsrv/network.cc b/src/lib/dhcpsrv/network.cc index c964fc8ae4..a5f84320c9 100644 --- a/src/lib/dhcpsrv/network.cc +++ b/src/lib/dhcpsrv/network.cc @@ -101,7 +101,8 @@ Network::getRequiredClasses() const { Optional Network::getGlobalProperty(Optional property, - const std::string& global_name) const { + const std::string& global_name, + bool /*triplet*/) const { if (!global_name.empty() && fetch_globals_fn_) { ConstElementPtr globals = fetch_globals_fn_(); if (globals && (globals->getType() == Element::map)) { diff --git a/src/lib/dhcpsrv/network.h b/src/lib/dhcpsrv/network.h index 4fdd85c306..dd1ee9a67b 100644 --- a/src/lib/dhcpsrv/network.h +++ b/src/lib/dhcpsrv/network.h @@ -333,7 +333,7 @@ public: /// @param inheritance inheritance mode to be used. Triplet getValid(const Inheritance& inheritance = Inheritance::ALL) const { return (getProperty(&Network::getValid, valid_, inheritance, - "valid-lifetime")); + "valid-lifetime", true)); } /// @brief Sets new valid lifetime for a network. @@ -556,7 +556,7 @@ public: (inheritance != Inheritance::PARENT_NETWORK)) { // Get global mode. util::Optional mode_label; - mode_label = getGlobalProperty(mode_label, "ddns-replace-client-name"); + mode_label = getGlobalProperty(mode_label, "ddns-replace-client-name", false); if (!mode_label.unspecified()) { try { // If the mode is globally configured, convert it to an enum. @@ -769,7 +769,8 @@ protected: /// of @c property. template ReturnType getGlobalProperty(ReturnType property, - const std::string& global_name) const { + const std::string& global_name, + bool /*triplet*/) const { if (!global_name.empty() && fetch_globals_fn_) { data::ConstElementPtr globals = fetch_globals_fn_(); if (globals && (globals->getType() == data::Element::map)) { @@ -784,6 +785,59 @@ protected: return (property); } + /// @brief The @c getGlobalProperty specialization for Triplet. + /// + /// The behavior depends on the triplet argument: + /// - if true it tries to get min and max values too + /// - if false the property is implemented as an Optional + /// + /// @note: use overloading vs specialization because full specialization + /// is not allowed in this scope. + /// + /// @tparam IntType Type of the encapsulated value(s). + /// + /// @param property Value to be returned when it is specified or when + /// no global value is found. + /// @param global_name Name of the global parameter which value should + /// be returned + /// @param triplet False (default) when has no min and max value so + /// is in fact only an @c OptionalValue, true otherwise. + /// + /// @return Optional value fetched from the global level or the value + /// of @c property. + template + Triplet getGlobalProperty(Triplet property, + const std::string& global_name, + bool triplet) const { + if (!global_name.empty() && fetch_globals_fn_) { + data::ConstElementPtr globals = fetch_globals_fn_(); + if (globals && (globals->getType() == data::Element::map)) { + data::ConstElementPtr param = globals->get(global_name); + if (param) { + IntType def_value = static_cast(param->intValue()); + if (!triplet) { + return (def_value); + } else { + IntType min_value = def_value; + IntType max_value = def_value; + const std::string& min_name = "min-" + global_name; + data::ConstElementPtr min_param = globals->get(min_name); + if (min_param) { + min_value = static_cast(min_param->intValue()); + } + const std::string& max_name = "max-" + global_name; + data::ConstElementPtr max_param = globals->get(max_name); + if (min_param) { + min_value = static_cast(min_param->intValue()); + } + return (Triplet(min_value, def_value, max_value)); + } + } + } + } + return (property); + } + /// @brief The @c getGlobalProperty specialization for Optional. /// /// This does two things: @@ -802,7 +856,8 @@ protected: /// of @c property. util::Optional getGlobalProperty(util::Optional property, - const std::string& global_name) const; + const std::string& global_name, + bool /*triplet*/) const; /// @brief Returns a value associated with a network using inheritance. /// @@ -827,6 +882,8 @@ protected: /// level. This value is empty by default, which indicates that the /// global value for the given parameter is not supported and shouldn't /// be fetched. + /// @param triplet False (default) when has no min and max value so + /// is in fact only an @c OptionalValue, true otherwise. /// /// @return Optional value fetched from this instance level, parent /// network level or global level @@ -834,7 +891,8 @@ protected: ReturnType getProperty(ReturnType(BaseType::*MethodPointer)(const Inheritance&) const, ReturnType property, const Inheritance& inheritance, - const std::string& global_name = "") const { + const std::string& global_name = "", + bool triplet = false) const { // If no inheritance is to be used, return the value for this // network regardless if it is specified or not. @@ -853,7 +911,7 @@ protected: // If global value requested, return it. } else if (inheritance == Inheritance::GLOBAL) { - return (getGlobalProperty(ReturnType(), global_name)); + return (getGlobalProperty(ReturnType(), global_name, triplet)); } // We use inheritance and the value is not specified on the network level. @@ -876,7 +934,7 @@ protected: // can be specified on global level and there is a callback // that returns the global values, try to find this parameter // at the global scope. - return (getGlobalProperty(property, global_name)); + return (getGlobalProperty(property, global_name, triplet)); } // We haven't found the value at any level, so return the unspecified. diff --git a/src/lib/dhcpsrv/srv_config.cc b/src/lib/dhcpsrv/srv_config.cc index d0c2646a7c..7cb6ec5909 100644 --- a/src/lib/dhcpsrv/srv_config.cc +++ b/src/lib/dhcpsrv/srv_config.cc @@ -248,7 +248,7 @@ SrvConfig::mergeGlobals(SrvConfig& other) { } } catch(const std::exception& ex) { isc_throw (BadValue, "Invalid value:" << element->str() - << " explict global:" << name); + << " explicit global:" << name); } } } @@ -400,6 +400,179 @@ SrvConfig::extractConfiguredGlobals(isc::data::ConstElementPtr config) { } } +void +SrvConfig::sanityChecksLifetime(const std::string& name) const { + // Initialize as some compilers complain otherwise. + uint32_t value = 0; + ConstElementPtr has_value = getConfiguredGlobal(name); + if (has_value) { + value = has_value->intValue(); + } + + uint32_t min_value = 0; + ConstElementPtr has_min = getConfiguredGlobal("min-" + name); + if (has_min) { + min_value = has_min->intValue(); + } + + uint32_t max_value = 0; + ConstElementPtr has_max = getConfiguredGlobal("max-" + name); + if (has_max) { + max_value = has_max->intValue(); + } + + if (!has_value && !has_min && !has_max) { + return; + } + if (has_value) { + if (!has_min && !has_max) { + // default only. + min_value = value; + max_value = value; + } else if (!has_min) { + // default and max. + min_value = value; + } else if (!has_max) { + // default and min. + max_value = value; + } + } else if (has_min) { + // min only. + if (!has_max) { + value = min_value; + max_value = min_value; + } else { + // min and max. + isc_throw(BadValue, "have min-" << name << " and max-" + << name << " but no " << name << " (default)"); + } + } else { + // max only. + min_value = max_value; + value = max_value; + } + + // Check that min <= max. + if (min_value > max_value) { + if (has_min && has_max) { + isc_throw(BadValue, "the value of min-" << name << " (" + << min_value << ") is not less than max-" << name << " (" + << max_value << ")"); + } else if (has_min) { + // Only min and default so min > default. + isc_throw(BadValue, "the value of min-" << name << " (" + << min_value << ") is not less than (default) " << name + << " (" << value << ")"); + } else { + // Only default and max so default > max. + isc_throw(BadValue, "the value of (default) " << name + << " (" << value << ") is not less than max-" << name + << " (" << max_value << ")"); + } + } + + // Check that value is between min and max. + if ((value < min_value) || (value > max_value)) { + isc_throw(BadValue, "the value of (default) " << name << " (" + << value << ") is not between min-" << name << " (" + << min_value << ") and max-" << name << " (" + << max_value << ")"); + } +} + +void +SrvConfig::sanityChecksLifetime(const SrvConfig& target_config, + const std::string& name) const { + // Three cases: + // - the external/source config has the parameter: use it. + // - only the target config has the parameter: use this one. + // - no config has the parameter. + uint32_t value = 0; + ConstElementPtr has_value = getConfiguredGlobal(name); + if (!has_value) { + has_value = target_config.getConfiguredGlobal(name); + } + if (has_value) { + value = has_value->intValue(); + } + + uint32_t min_value = 0; + ConstElementPtr has_min = getConfiguredGlobal("min-" + name); + if (!has_min) { + has_min = target_config.getConfiguredGlobal("min-" + name); + } + if (has_min) { + min_value = has_min->intValue(); + } + + uint32_t max_value = 0; + ConstElementPtr has_max = getConfiguredGlobal("max-" + name); + if (!has_max) { + has_max = target_config.getConfiguredGlobal("max-" + name); + } + if (has_max) { + max_value = has_max->intValue(); + } + + if (!has_value && !has_min && !has_max) { + return; + } + if (has_value) { + if (!has_min && !has_max) { + // default only. + min_value = value; + max_value = value; + } else if (!has_min) { + // default and max. + min_value = value; + } else if (!has_max) { + // default and min. + max_value = value; + } + } else if (has_min) { + // min only. + if (!has_max) { + value = min_value; + max_value = min_value; + } else { + // min and max. + isc_throw(BadValue, "have min-" << name << " and max-" + << name << " but no " << name << " (default)"); + } + } else { + // max only. + min_value = max_value; + value = max_value; + } + + // Check that min <= max. + if (min_value > max_value) { + if (has_min && has_max) { + isc_throw(BadValue, "the value of min-" << name << " (" + << min_value << ") is not less than max-" << name << " (" + << max_value << ")"); + } else if (has_min) { + // Only min and default so min > default. + isc_throw(BadValue, "the value of min-" << name << " (" + << min_value << ") is not less than (default) " << name + << " (" << value << ")"); + } else { + // Only default and max so default > max. + isc_throw(BadValue, "the value of (default) " << name + << " (" << value << ") is not less than max-" << name + << " (" << max_value << ")"); + } + } + + // Check that value is between min and max. + if ((value < min_value) || (value > max_value)) { + isc_throw(BadValue, "the value of (default) " << name << " (" + << value << ") is not between min-" << name << " (" + << min_value << ") and max-" << name << " (" + << max_value << ")"); + } +} + ElementPtr SrvConfig::toElement() const { // Toplevel map diff --git a/src/lib/dhcpsrv/srv_config.h b/src/lib/dhcpsrv/srv_config.h index 981d2977b1..63256c284d 100644 --- a/src/lib/dhcpsrv/srv_config.h +++ b/src/lib/dhcpsrv/srv_config.h @@ -124,7 +124,7 @@ public: /// @brief Returns whether or not DNS should be updated when leases renew. /// - /// If this is true, DNS should always be updated when leases are + /// If this is true, DNS should always be updated when leases are /// extended (i.e. renewed/rebound) even if the DNS information /// has not changed. /// @@ -793,6 +793,19 @@ public: configured_globals_->set(name, value); } + /// @brief Conducts sanity checks on global lifetime parameters. + /// + /// @param name Base name of the lifetime parameter. + void sanityChecksLifetime(const std::string& name) const; + + /// @brief Conducts sanity checks on global lifetime parameters + /// before merge from the external configuration to the target one. + /// + /// @param target_config Target configuration. + /// @param name Base name of the lifetime parameter. + void sanityChecksLifetime(const SrvConfig& target_config, + const std::string& name) const; + /// @brief Moves deprecated parameters from dhcp-ddns element to global element /// /// Given a server configuration element map, the following parameters are moved