From: Francis Dupont Date: Tue, 10 Sep 2019 22:49:16 +0000 (+0200) Subject: [897-add-infinite-valid-lifetime] Checkpoint (todo: only mysql has the 2038 issue) X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=9a06e53154a2e18817139d68f0a626c31affeff9;p=thirdparty%2Fkea.git [897-add-infinite-valid-lifetime] Checkpoint (todo: only mysql has the 2038 issue) --- diff --git a/src/bin/dhcp4/dhcp4_srv.cc b/src/bin/dhcp4/dhcp4_srv.cc index f658489a56..0557ba4061 100644 --- a/src/bin/dhcp4/dhcp4_srv.cc +++ b/src/bin/dhcp4/dhcp4_srv.cc @@ -2178,11 +2178,16 @@ Dhcpv4Srv::assignLease(Dhcpv4Exchange& ex) { LOG_INFO(lease4_logger, DHCP4_LEASE_ADVERT) .arg(query->getLabel()) .arg(lease->addr_.toText()); - } else { + } else if (lease->valid_lft_ != Lease::INFINITY_LFT) { LOG_INFO(lease4_logger, DHCP4_LEASE_ALLOC) .arg(query->getLabel()) .arg(lease->addr_.toText()) .arg(lease->valid_lft_); + } else { + LOG_INFO(lease4_logger, DHCP4_LEASE_ALLOC) + .arg(query->getLabel()) + .arg(lease->addr_.toText()) + .arg("infinity"); } // We're logging this here, because this is the place where we know diff --git a/src/bin/dhcp6/dhcp6_srv.cc b/src/bin/dhcp6/dhcp6_srv.cc index 453f65d7e2..470e34e456 100644 --- a/src/bin/dhcp6/dhcp6_srv.cc +++ b/src/bin/dhcp6/dhcp6_srv.cc @@ -1897,12 +1897,18 @@ Dhcpv6Srv::assignIA_NA(const Pkt6Ptr& query, const Pkt6Ptr& answer, .arg(query->getLabel()) .arg(lease->addr_.toText()) .arg(ia->getIAID()); - } else { + } else if (lease->valid_lft_ != Lease::INFINITY_LFT) { LOG_INFO(lease6_logger, DHCP6_LEASE_ALLOC) .arg(query->getLabel()) .arg(lease->addr_.toText()) .arg(ia->getIAID()) .arg(lease->valid_lft_); + } else { + LOG_INFO(lease6_logger, DHCP6_LEASE_ALLOC) + .arg(query->getLabel()) + .arg(lease->addr_.toText()) + .arg(ia->getIAID()) + .arg("infinity"); } LOG_DEBUG(lease6_logger, DBG_DHCP6_DETAIL_DATA, DHCP6_LEASE_DATA) .arg(query->getLabel()) @@ -2019,13 +2025,20 @@ Dhcpv6Srv::assignIA_PD(const Pkt6Ptr& query, const Pkt6Ptr& /*answer*/, .arg((*l)->addr_.toText()) .arg(static_cast((*l)->prefixlen_)) .arg(ia->getIAID()); - } else { + } else if ((*l)->valid_lft_ != Lease::INFINITY_LFT) { LOG_INFO(lease6_logger, DHCP6_PD_LEASE_ALLOC) .arg(query->getLabel()) .arg((*l)->addr_.toText()) .arg(static_cast((*l)->prefixlen_)) .arg(ia->getIAID()) .arg((*l)->valid_lft_); + } else { + LOG_INFO(lease6_logger, DHCP6_PD_LEASE_ALLOC) + .arg(query->getLabel()) + .arg((*l)->addr_.toText()) + .arg(static_cast((*l)->prefixlen_)) + .arg(ia->getIAID()) + .arg("infinity"); } boost::shared_ptr diff --git a/src/lib/dhcpsrv/cql_lease_mgr.cc b/src/lib/dhcpsrv/cql_lease_mgr.cc index 3bea54355b..9cddf8f25d 100644 --- a/src/lib/dhcpsrv/cql_lease_mgr.cc +++ b/src/lib/dhcpsrv/cql_lease_mgr.cc @@ -302,6 +302,7 @@ StatementMap CqlLease4Exchange::tagged_statements_{ "FROM lease4 " "WHERE state = ? " "AND expire < ? " + "AND valid_lifetime < 4294967295 " "LIMIT ? " "ALLOW FILTERING "}}, @@ -448,8 +449,12 @@ CqlLease4Exchange::createBindForInsert(const Lease4Ptr &lease, AnyArray &data) { // For convenience for external tools, this is converted to lease // expiry time (expire). The relationship is given by: // expire = cltt_ + valid_lft_ - CqlExchange::convertToDatabaseTime(lease_->cltt_, lease_->valid_lft_, - expire_); + // Avoid overflow + uint32_t valid_lft = lease_->valid_lft_; + if (valid_lft == Lease::INFINITY_LFT) { + valid_lft = Lease::FIVEHUNDREDDAYS; + } + CqlExchange::convertToDatabaseTime(lease_->cltt_, valid_lft, expire_); // subnet_id: int subnet_id_ = static_cast(lease_->subnet_id_); @@ -551,8 +556,12 @@ CqlLease4Exchange::createBindForUpdate(const Lease4Ptr &lease, AnyArray &data, // For convenience for external tools, this is converted to lease // expiry time (expire). The relationship is given by: // expire = cltt_ + valid_lft_ - CqlExchange::convertToDatabaseTime(lease_->cltt_, lease_->valid_lft_, - expire_); + // Avoid overflow + uint32_t valid_lft = lease_->valid_lft_; + if (valid_lft == Lease::INFINITY_LFT) { + valid_lft = Lease::FIVEHUNDREDDAYS; + } + CqlExchange::convertToDatabaseTime(lease_->cltt_, valid_lft, expire_); // subnet_id: int subnet_id_ = static_cast(lease_->subnet_id_); @@ -696,7 +705,12 @@ CqlLease4Exchange::retrieve() { } time_t cltt = 0; - CqlExchange::convertFromDatabaseTime(expire_, valid_lifetime_, cltt); + // Recover from overflow + uint32_t valid_lft = valid_lifetime_; + if (valid_lft == Lease::INFINITY_LFT) { + valid_lft = Lease::FIVEHUNDREDDAYS; + } + CqlExchange::convertFromDatabaseTime(expire_, valid_lft, cltt); HWAddrPtr hwaddr(new HWAddr(hwaddr_, HTYPE_ETHER)); @@ -1004,6 +1018,7 @@ StatementMap CqlLease6Exchange::tagged_statements_ = { "FROM lease6 " "WHERE state = ? " "AND expire < ? " + "AND valid_lifetime < 4294967295 " "LIMIT ? " "ALLOW FILTERING "}}, @@ -1112,7 +1127,12 @@ CqlLease6Exchange::createBindForInsert(const Lease6Ptr &lease, AnyArray &data) { // For convenience for external tools, this is converted to lease // expiry time (expire). The relationship is given by: // expire = cltt_ + valid_lft_ - CqlExchange::convertToDatabaseTime(lease_->cltt_, lease_->valid_lft_, expire_); + // Avoid overflow + uint32_t valid_lft = lease_->valid_lft_; + if (valid_lft == Lease::INFINITY_LFT) { + valid_lft = Lease::FIVEHUNDREDDAYS; + } + CqlExchange::convertToDatabaseTime(lease_->cltt_, valid_lft, expire_); // subnet_id: int subnet_id_ = static_cast(lease_->subnet_id_); @@ -1247,8 +1267,12 @@ CqlLease6Exchange::createBindForUpdate(const Lease6Ptr &lease, AnyArray &data, // For convenience for external tools, this is converted to lease // expiry time (expire). The relationship is given by: // expire = cltt_ + valid_lft_ - CqlExchange::convertToDatabaseTime(lease_->cltt_, lease_->valid_lft_, - expire_); + // Avoid overflow + uint32_t valid_lft = lease_->valid_lft_; + if (valid_lft == Lease::INFINITY_LFT) { + valid_lft = Lease::FIVEHUNDREDDAYS; + } + CqlExchange::convertToDatabaseTime(lease_->cltt_, valid_lft, expire_); // subnet_id: int subnet_id_ = static_cast(lease_->subnet_id_); @@ -1505,7 +1529,12 @@ CqlLease6Exchange::retrieve() { fqdn_fwd_, fqdn_rev_, hostname_, hwaddr, prefix_len_)); time_t cltt = 0; - CqlExchange::convertFromDatabaseTime(expire_, valid_lifetime_, cltt); + // Recover from overflow + uint32_t valid_lft = valid_lifetime_; + if (valid_lft == Lease::INFINITY_LFT) { + valid_lft = Lease::FIVEHUNDREDDAYS; + } + CqlExchange::convertFromDatabaseTime(expire_, valid_lft, cltt); result->cltt_ = cltt; result->state_ = state_; @@ -2311,7 +2340,6 @@ CqlLeaseMgr::getLeases6(const DUID& duid) const { // Set up the WHERE clause value AnyArray data; - CassBlob duid_data(duid.getDuid()); data.add(&duid_data); diff --git a/src/lib/dhcpsrv/cql_lease_mgr.h b/src/lib/dhcpsrv/cql_lease_mgr.h index ae042b3c46..f5f1a8e658 100644 --- a/src/lib/dhcpsrv/cql_lease_mgr.h +++ b/src/lib/dhcpsrv/cql_lease_mgr.h @@ -320,7 +320,7 @@ public: /// /// @return Lease collection (may be empty if no IPv6 lease found). virtual Lease6Collection getLeases6(const DUID& duid) const override; - + /// @brief Returns range of IPv6 leases using paging. /// /// This method implements paged browsing of the lease database. The first diff --git a/src/lib/dhcpsrv/csv_lease_file4.cc b/src/lib/dhcpsrv/csv_lease_file4.cc index a696377c99..fc3cf1886a 100644 --- a/src/lib/dhcpsrv/csv_lease_file4.cc +++ b/src/lib/dhcpsrv/csv_lease_file4.cc @@ -60,7 +60,7 @@ CSVLeaseFile4::append(const Lease4& lease) { } row.writeAt(getColumnIndex("valid_lifetime"), lease.valid_lft_); - row.writeAt(getColumnIndex("expire"), static_cast(lease.cltt_ + lease.valid_lft_)); + row.writeAt(getColumnIndex("expire"), static_cast(lease.cltt_) + lease.valid_lft_); row.writeAt(getColumnIndex("subnet_id"), lease.subnet_id_); row.writeAt(getColumnIndex("fqdn_fwd"), lease.fqdn_fwd_); row.writeAt(getColumnIndex("fqdn_rev"), lease.fqdn_rev_); diff --git a/src/lib/dhcpsrv/csv_lease_file6.cc b/src/lib/dhcpsrv/csv_lease_file6.cc index a1119a6177..c73b7d9aa6 100644 --- a/src/lib/dhcpsrv/csv_lease_file6.cc +++ b/src/lib/dhcpsrv/csv_lease_file6.cc @@ -46,7 +46,7 @@ CSVLeaseFile6::append(const Lease6& lease) { row.writeAt(getColumnIndex("address"), lease.addr_.toText()); row.writeAt(getColumnIndex("duid"), lease.duid_->toText()); row.writeAt(getColumnIndex("valid_lifetime"), lease.valid_lft_); - row.writeAt(getColumnIndex("expire"), static_cast(lease.cltt_ + lease.valid_lft_)); + row.writeAt(getColumnIndex("expire"), static_cast(lease.cltt_) + lease.valid_lft_); row.writeAt(getColumnIndex("subnet_id"), lease.subnet_id_); row.writeAt(getColumnIndex("pref_lifetime"), lease.preferred_lft_); row.writeAt(getColumnIndex("lease_type"), lease.type_); diff --git a/src/lib/dhcpsrv/lease.cc b/src/lib/dhcpsrv/lease.cc index 4b1f0bd6b5..789a78a946 100644 --- a/src/lib/dhcpsrv/lease.cc +++ b/src/lib/dhcpsrv/lease.cc @@ -93,7 +93,7 @@ Lease::basicStatesToText(const uint32_t state) { bool Lease::expired() const { - return (getExpirationTime() < time(NULL)); + return ((valid_lft_ != INFINITY_LFT) && (getExpirationTime() < time(NULL))); } bool @@ -527,9 +527,13 @@ Lease6::toText() const { << "Address: " << addr_ << "\n" << "Prefix length: " << static_cast(prefixlen_) << "\n" << "IAID: " << iaid_ << "\n" - << "Pref life: " << preferred_lft_ << "\n" - << "Valid life: " << valid_lft_ << "\n" - << "Cltt: " << cltt_ << "\n" + << "Pref life: " << preferred_lft_ << "\n"; + if (valid_lft_ != INFINITY_LFT) { + stream << "Valid life: " << valid_lft_ << "\n"; + } else { + stream << "Valid life: " << "infinity" << "\n"; + } + stream << "Cltt: " << cltt_ << "\n" << "DUID: " << (duid_?duid_->toText():"(none)") << "\n" << "Hardware addr: " << (hwaddr_?hwaddr_->toText(false):"(none)") << "\n" << "Subnet ID: " << subnet_id_ << "\n" @@ -546,9 +550,13 @@ std::string Lease4::toText() const { ostringstream stream; - stream << "Address: " << addr_ << "\n" - << "Valid life: " << valid_lft_ << "\n" - << "Cltt: " << cltt_ << "\n" + stream << "Address: " << addr_ << "\n"; + if (valid_lft_ != INFINITY_LFT) { + stream << "Valid life: " << valid_lft_ << "\n"; + } else { + stream << "Valid life: " << "infinity" << "\n"; + } + stream << "Cltt: " << cltt_ << "\n" << "Hardware addr: " << (hwaddr_ ? hwaddr_->toText(false) : "(none)") << "\n" << "Client id: " << (client_id_ ? client_id_->toText() : "(none)") << "\n" << "Subnet ID: " << subnet_id_ << "\n" diff --git a/src/lib/dhcpsrv/lease.h b/src/lib/dhcpsrv/lease.h index 2f661daa5b..ff77ce87df 100644 --- a/src/lib/dhcpsrv/lease.h +++ b/src/lib/dhcpsrv/lease.h @@ -34,6 +34,12 @@ typedef boost::shared_ptr LeasePtr; /// leases. struct Lease : public isc::data::UserContext, public isc::data::CfgToElement { + /// @brief Infinity (means static, i.e. never expire) + static const uint32_t INFINITY_LFT = 0xffffffff; + + /// @brief Five hundred days (infinity overflows in 32 bits) + static const uint32_t FIVEHUNDREDDAYS = 500 * 24 * 60 * 60; + /// @brief Type of lease or pool typedef enum { TYPE_NA = 0, ///< the lease contains non-temporary IPv6 address diff --git a/src/lib/dhcpsrv/lease_mgr.h b/src/lib/dhcpsrv/lease_mgr.h index 233f3539c8..5026ee0b4e 100644 --- a/src/lib/dhcpsrv/lease_mgr.h +++ b/src/lib/dhcpsrv/lease_mgr.h @@ -459,9 +459,9 @@ public: /// @brief Returns collection of leases for matching DUID /// - /// @return Lease collection + /// @return Lease collection /// (may be empty if no IPv6 lease found for the DUID). - virtual Lease6Collection getLeases6(const DUID& duid) const = 0; + virtual Lease6Collection getLeases6(const DUID& duid) const = 0; /// @brief Returns range of IPv6 leases using paging. /// diff --git a/src/lib/dhcpsrv/memfile_lease_mgr.h b/src/lib/dhcpsrv/memfile_lease_mgr.h index 49dbf3e0bf..69ccd8c0b6 100644 --- a/src/lib/dhcpsrv/memfile_lease_mgr.h +++ b/src/lib/dhcpsrv/memfile_lease_mgr.h @@ -316,10 +316,10 @@ public: /// @brief Returns IPv6 leases for the DUID. /// /// @todo: implement an optimised of the query using index. - /// @return Lease collection (may be empty if no IPv6 lease found) + /// @return Lease collection (may be empty if no IPv6 lease found) /// for the DUID. virtual Lease6Collection getLeases6(const DUID& duid) const; - + /// @brief Returns range of IPv6 leases using paging. /// /// This method implements paged browsing of the lease database. The first diff --git a/src/lib/dhcpsrv/memfile_lease_storage.h b/src/lib/dhcpsrv/memfile_lease_storage.h index 3254eefe3a..4863633b09 100644 --- a/src/lib/dhcpsrv/memfile_lease_storage.h +++ b/src/lib/dhcpsrv/memfile_lease_storage.h @@ -46,7 +46,7 @@ struct ClientIdHWAddressSubnetIdIndexTag { }; struct SubnetIdIndexTag { }; /// @brief Tag for index using DUID. -struct DuidIndexTag { }; +struct DuidIndexTag { }; /// @name Multi index containers holding DHCPv4 and DHCPv6 leases. /// //@{ diff --git a/src/lib/dhcpsrv/mysql_lease_mgr.cc b/src/lib/dhcpsrv/mysql_lease_mgr.cc index 8d741fa085..900654f6fe 100644 --- a/src/lib/dhcpsrv/mysql_lease_mgr.cc +++ b/src/lib/dhcpsrv/mysql_lease_mgr.cc @@ -165,7 +165,8 @@ tagged_statements = { { "fqdn_fwd, fqdn_rev, hostname, " "state, user_context " "FROM lease4 " - "WHERE state != ? AND expire < ? " + "WHERE state != ? AND expire < ?" + " AND valid_lifetime != 4294967295 " "ORDER BY expire ASC " "LIMIT ?"}, {MySqlLeaseMgr::GET_LEASE6, @@ -241,7 +242,8 @@ tagged_statements = { { "hwaddr, hwtype, hwaddr_source, " "state, user_context " "FROM lease6 " - "WHERE state != ? AND expire < ? " + "WHERE state != ? AND expire < ?" + " AND valid_lifetime != 4294967295 " "ORDER BY expire ASC " "LIMIT ?"}, {MySqlLeaseMgr::INSERT_LEASE4, @@ -539,7 +541,12 @@ public: // expiry time (expire). The relationship is given by: // // expire = cltt_ + valid_lft_ - MySqlConnection::convertToDatabaseTime(lease_->cltt_, lease_->valid_lft_, + // Avoid overflow + uint32_t valid_lft = lease_->valid_lft_; + if (valid_lft == Lease::INFINITY_LFT) { + valid_lft = Lease::FIVEHUNDREDDAYS; + } + MySqlConnection::convertToDatabaseTime(lease_->cltt_, valid_lft, expire_); bind_[4].buffer_type = MYSQL_TYPE_TIMESTAMP; bind_[4].buffer = reinterpret_cast(&expire_); @@ -744,7 +751,12 @@ public: // Convert times received from the database to times for the lease // structure time_t cltt = 0; - MySqlConnection::convertFromDatabaseTime(expire_, valid_lifetime_, cltt); + // Recover from overflow + uint32_t valid_lft = valid_lifetime_; + if (valid_lft == Lease::INFINITY_LFT) { + valid_lft = Lease::FIVEHUNDREDDAYS; + } + MySqlConnection::convertFromDatabaseTime(expire_, valid_lft, cltt); if (client_id_null_ == MLM_TRUE) { // There's no client-id, so we pass client-id_length_ set to 0 @@ -971,7 +983,12 @@ public: /// expiry time (expire). The relationship is given by: // // expire = cltt_ + valid_lft_ - MySqlConnection::convertToDatabaseTime(lease_->cltt_, lease_->valid_lft_, + // Avoid overflow + uint32_t valid_lft = lease_->valid_lft_; + if (valid_lft == Lease::INFINITY_LFT) { + valid_lft = Lease::FIVEHUNDREDDAYS; + } + MySqlConnection::convertToDatabaseTime(lease_->cltt_, valid_lft, expire_); bind_[3].buffer_type = MYSQL_TYPE_TIMESTAMP; bind_[3].buffer = reinterpret_cast(&expire_); @@ -1384,7 +1401,12 @@ public: subnet_id_, fqdn_fwd_, fqdn_rev_, hostname, hwaddr, prefixlen_)); time_t cltt = 0; - MySqlConnection::convertFromDatabaseTime(expire_, valid_lifetime_, cltt); + // Recover from overflow + uint32_t valid_lft = valid_lifetime_; + if (valid_lft == Lease::INFINITY_LFT) { + valid_lft = Lease::FIVEHUNDREDDAYS; + } + MySqlConnection::convertFromDatabaseTime(expire_, valid_lft, cltt); result->cltt_ = cltt; // Set state. @@ -2312,7 +2334,7 @@ Lease6Collection MySqlLeaseMgr::getLeases6(const DUID& duid) const { LOG_DEBUG(dhcpsrv_logger, DHCPSRV_DBG_TRACE_DETAIL, DHCPSRV_MYSQL_GET_DUID) .arg(duid.toText()); - + // Set up the WHERE clause value MYSQL_BIND inbind[1]; memset(inbind, 0, sizeof(inbind)); @@ -2325,9 +2347,8 @@ MySqlLeaseMgr::getLeases6(const DUID& duid) const { const_cast(&duid_vector[0])); inbind[0].buffer_length = duid_length; inbind[0].length = &duid_length; - + Lease6Collection result; - getLeaseCollection(GET_LEASE6_DUID, inbind, result); return result; diff --git a/src/lib/dhcpsrv/mysql_lease_mgr.h b/src/lib/dhcpsrv/mysql_lease_mgr.h index a11009c874..19919c191f 100644 --- a/src/lib/dhcpsrv/mysql_lease_mgr.h +++ b/src/lib/dhcpsrv/mysql_lease_mgr.h @@ -316,7 +316,7 @@ public: /// @return Lease collection (may be empty if no IPv6 lease found) /// for the DUID. virtual Lease6Collection getLeases6(const DUID& duid) const; - + /// @brief Returns range of IPv6 leases using paging. /// /// This method implements paged browsing of the lease database. The first diff --git a/src/lib/dhcpsrv/ncr_generator.cc b/src/lib/dhcpsrv/ncr_generator.cc index 7b0e9f29d2..8dd5a7aca7 100644 --- a/src/lib/dhcpsrv/ncr_generator.cc +++ b/src/lib/dhcpsrv/ncr_generator.cc @@ -1,4 +1,4 @@ -// Copyright (C) 2015 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2015-2019 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 @@ -44,7 +44,7 @@ void queueNCRCommon(const NameChangeType& chg_type, const LeasePtrType& lease, DHCPSRV_QUEUE_NCR_SKIP) .arg(label) .arg(lease->addr_.toText()); - + return; } @@ -55,11 +55,15 @@ void queueNCRCommon(const NameChangeType& chg_type, const LeasePtrType& lease, D2Dhcid dhcid = D2Dhcid(identifier, hostname_wire); // Create name change request. + uint32_t valid_lft = lease->valid_lft_; + if (valid_lft == Lease::INFINITY_LFT) { + valid_lft = Lease::FIVEHUNDREDDAYS; + } NameChangeRequestPtr ncr (new NameChangeRequest(chg_type, lease->fqdn_fwd_, lease->fqdn_rev_, lease->hostname_, lease->addr_.toText(), - dhcid, lease->cltt_ + lease->valid_lft_, - lease->valid_lft_)); + dhcid, lease->cltt_ + valid_lft, + valid_lft)); LOG_DEBUG(dhcpsrv_logger, DHCPSRV_DBG_TRACE_DETAIL_DATA, DHCPSRV_QUEUE_NCR) .arg(label) diff --git a/src/lib/dhcpsrv/pgsql_lease_mgr.cc b/src/lib/dhcpsrv/pgsql_lease_mgr.cc index 26ff0c1c7b..c5d1ed9e26 100644 --- a/src/lib/dhcpsrv/pgsql_lease_mgr.cc +++ b/src/lib/dhcpsrv/pgsql_lease_mgr.cc @@ -145,7 +145,7 @@ PgSqlTaggedStatement tagged_statements[] = { "fqdn_fwd, fqdn_rev, hostname, " "state, user_context " "FROM lease4 " - "WHERE state != $1 AND expire < $2 " + "WHERE state != $1 AND expire < $2 AND valid_lifetime != 4294967295 " "ORDER BY expire " "LIMIT $3"}, @@ -238,7 +238,7 @@ PgSqlTaggedStatement tagged_statements[] = { "hwaddr, hwtype, hwaddr_source, " "state, user_context " "FROM lease6 " - "WHERE state != $1 AND expire < $2 " + "WHERE state != $1 AND expire < $2 AND valid_lifetime != 4294967295 " "ORDER BY expire " "LIMIT $3"}, @@ -469,7 +469,12 @@ public: valid_lifetime_str_ = boost::lexical_cast(lease->valid_lft_); bind_array.add(valid_lifetime_str_); - expire_str_ = convertToDatabaseTime(lease->cltt_, lease_->valid_lft_); + // Avoid overflow + uint32_t valid_lft = lease_->valid_lft_; + if (valid_lft == Lease::INFINITY_LFT) { + valid_lft = Lease::FIVEHUNDREDDAYS; + } + expire_str_ = convertToDatabaseTime(lease->cltt_, valid_lft); bind_array.add(expire_str_); subnet_id_str_ = boost::lexical_cast(lease->subnet_id_); @@ -524,7 +529,12 @@ public: getColumnValue(r, row , SUBNET_ID_COL, subnet_id_); - cltt_ = expire_ - valid_lifetime_; + // Recover from overflow + uint32_t valid_lft = valid_lifetime_; + if (valid_lft == Lease::INFINITY_LFT) { + valid_lft = Lease::FIVEHUNDREDDAYS; + } + cltt_ = expire_ - valid_lft; getColumnValue(r, row, FQDN_FWD_COL, fqdn_fwd_); @@ -700,7 +710,12 @@ public: valid_lifetime_str_ = boost::lexical_cast(lease->valid_lft_); bind_array.add(valid_lifetime_str_); - expire_str_ = convertToDatabaseTime(lease->cltt_, lease_->valid_lft_); + // Avoid overflow + uint32_t valid_lft = lease_->valid_lft_; + if (valid_lft == Lease::INFINITY_LFT) { + valid_lft = Lease::FIVEHUNDREDDAYS; + } + expire_str_ = convertToDatabaseTime(lease->cltt_, valid_lft); bind_array.add(expire_str_); subnet_id_str_ = boost::lexical_cast(lease->subnet_id_); @@ -806,7 +821,12 @@ public: expire_ = convertFromDatabaseTime(getRawColumnValue(r, row, EXPIRE_COL)); - cltt_ = expire_ - valid_lifetime_; + // Recover from overflow + uint32_t valid_lft = valid_lifetime_; + if (valid_lft == Lease::INFINITY_LFT) { + valid_lft = Lease::FIVEHUNDREDDAYS; + } + cltt_ = expire_ - valid_lft; getColumnValue(r, row , SUBNET_ID_COL, subnet_id_); diff --git a/src/lib/dhcpsrv/pgsql_lease_mgr.h b/src/lib/dhcpsrv/pgsql_lease_mgr.h index c779fafaa6..29650c0796 100644 --- a/src/lib/dhcpsrv/pgsql_lease_mgr.h +++ b/src/lib/dhcpsrv/pgsql_lease_mgr.h @@ -288,7 +288,7 @@ public: /// @return Lease collection (may be empty if no IPv6 lease found) /// for the DUID virtual Lease6Collection getLeases6(const DUID& duid) const; - + /// @brief Returns range of IPv6 leases using paging. /// /// This method implements paged browsing of the lease database. The first diff --git a/src/lib/dhcpsrv/tests/cql_lease_mgr_unittest.cc b/src/lib/dhcpsrv/tests/cql_lease_mgr_unittest.cc index 8c31f811d0..fa1b2267dc 100644 --- a/src/lib/dhcpsrv/tests/cql_lease_mgr_unittest.cc +++ b/src/lib/dhcpsrv/tests/cql_lease_mgr_unittest.cc @@ -508,6 +508,11 @@ TEST_F(CqlLeaseMgrTest, maxDate4) { testMaxDate4(); } +/// @brief checks that infinite lifetimes do not overflow. +TEST_F(CqlLeaseMgrTest, infiniteLifeTime4) { + testInfiniteLifeTime4(); +} + /// @brief Lease4 update tests /// /// Checks that we are able to update a lease in the database. @@ -620,6 +625,12 @@ TEST_F(CqlLeaseMgrTest, getExpiredLeases4) { testCqlGetExpiredLeases4(); } +/// @brief Checks that the static (i.e. with infinite valid lifetime) DHCPv4 +/// leases will never expire. +TEST_F(CqlLeaseMgrTest, staticLeases4) { + testStaticLeases4(); +} + /// @brief Check that expired reclaimed DHCPv4 leases are removed. TEST_F(CqlLeaseMgrTest, deleteExpiredReclaimedLeases4) { testDeleteExpiredReclaimedLeases4(); @@ -648,6 +659,11 @@ TEST_F(CqlLeaseMgrTest, maxDate6) { testMaxDate6(); } +/// @brief checks that infinite lifetimes do not overflow. +TEST_F(CqlLeaseMgrTest, infiniteLifeTime6) { + testInfiniteLifeTime6(); +} + /// @brief Verify that too long hostname for Lease6 is not accepted. /// /// Checks that the it is not possible to create a lease when the hostname @@ -764,6 +780,12 @@ TEST_F(CqlLeaseMgrTest, getExpiredLeases6) { testCqlGetExpiredLeases6(); } +/// @brief Checks that the static (i.e. with infinite valid lifetime) DHCPv6 +/// leases will never expire. +TEST_F(CqlLeaseMgrTest, staticLeases6) { + testStaticLeases6(); +} + /// @brief Check that expired reclaimed DHCPv6 leases are removed. TEST_F(CqlLeaseMgrTest, deleteExpiredReclaimedLeases6) { testDeleteExpiredReclaimedLeases6(); diff --git a/src/lib/dhcpsrv/tests/generic_lease_mgr_unittest.cc b/src/lib/dhcpsrv/tests/generic_lease_mgr_unittest.cc index eceec558d6..e40e9de2db 100644 --- a/src/lib/dhcpsrv/tests/generic_lease_mgr_unittest.cc +++ b/src/lib/dhcpsrv/tests/generic_lease_mgr_unittest.cc @@ -127,7 +127,7 @@ GenericLeaseMgrTest::initializeLease4(std::string address) { // The times used in the next tests are deliberately restricted - we // should be able to cope with valid lifetimes up to 0xffffffff. // However, this will lead to overflows. - // @TODO: test overflow conditions when code has been fixed + // @TODO: test overflow conditions as now the code was fixed. lease->valid_lft_ = 7000; lease->cltt_ = 234567; lease->subnet_id_ = 37; @@ -254,7 +254,7 @@ GenericLeaseMgrTest::initializeLease6(std::string address) { // The times used in the next tests are deliberately restricted - we // should be able to cope with valid lifetimes up to 0xffffffff. // However, this will lead to overflows. - // @TODO: test overflow conditions when code has been fixed + // @TODO: test overflow conditions as now the code was fixed. lease->preferred_lft_ = 7200; lease->valid_lft_ = 7000; lease->cltt_ = 234567; @@ -712,6 +712,23 @@ GenericLeaseMgrTest::testMaxDate4() { detailCompareLease(leases[1], l_returned); } +void +GenericLeaseMgrTest::testInfiniteLifeTime4() { + // Get the leases to be used for the test. + vector leases = createLeases4(); + Lease4Ptr lease = leases[1]; + + // Set valid_lft_ to infinite, cllt_ to now. + lease->valid_lft_ = Lease::INFINITY_LFT; + lease->cltt_ = time(NULL); + + // Insert should not throw. + EXPECT_TRUE(lmptr_->addLease(leases[1])); + Lease4Ptr l_returned = lmptr_->getLease4(ioaddress4_[1]); + ASSERT_TRUE(l_returned); + detailCompareLease(leases[1], l_returned); +} + void GenericLeaseMgrTest::testBasicLease4() { // Get the leases to be used for the test. @@ -883,6 +900,23 @@ GenericLeaseMgrTest::testMaxDate6() { detailCompareLease(leases[1], l_returned); } +void +GenericLeaseMgrTest::testInfiniteLifeTime6() { + // Get the leases to be used for the test. + vector leases = createLeases6(); + Lease6Ptr lease = leases[1]; + + // Set valid_lft_ to infinite, cllt_ to now. + lease->valid_lft_ = Lease::INFINITY_LFT; + lease->cltt_ = time(NULL); + + // Insert should not throw. + EXPECT_TRUE(lmptr_->addLease(leases[1])); + Lease6Ptr l_returned = lmptr_->getLease6(leasetype6_[1], ioaddress6_[1]); + ASSERT_TRUE(l_returned); + detailCompareLease(leases[1], l_returned); +} + // Checks whether a MAC address can be stored and retrieved together with // a lease. void @@ -2167,6 +2201,48 @@ GenericLeaseMgrTest::testGetExpiredLeases6() { } } +void +GenericLeaseMgrTest::testStaticLeases4() { + // Get the leases to be used for the test. + vector leases = createLeases4(); + Lease4Ptr lease = leases[1]; + + // Set valid_lft_ to infinite. Leave the small cltt even it won't happen + // in the real world to catch more possible issues. + lease->valid_lft_ = Lease::INFINITY_LFT; + + // Add it to the database. + ASSERT_TRUE(lmptr_->addLease(leases[1])); + + // Retrieve at most 10 expired leases. + Lease4Collection expired_leases; + ASSERT_NO_THROW(lmptr_->getExpiredLeases4(expired_leases, 10)); + + // No lease should be returned. + EXPECT_EQ(0, expired_leases.size()); +} + +void +GenericLeaseMgrTest::testStaticLeases6() { + // Get the leases to be used for the test. + vector leases = createLeases6(); + Lease6Ptr lease = leases[1]; + + // Set valid_lft_ to infinite. Leave the small cltt even it won't happen + // in the real world to catch more possible issues. + lease->valid_lft_ = Lease::INFINITY_LFT; + + // Add it to the database. + ASSERT_TRUE(lmptr_->addLease(leases[1])); + + // Retrieve at most 10 expired leases. + Lease6Collection expired_leases; + ASSERT_NO_THROW(lmptr_->getExpiredLeases6(expired_leases, 10)); + + // No lease should be returned. + EXPECT_EQ(0, expired_leases.size()); +} + void GenericLeaseMgrTest::testDeleteExpiredReclaimedLeases4() { // Get the leases to be used for the test. diff --git a/src/lib/dhcpsrv/tests/generic_lease_mgr_unittest.h b/src/lib/dhcpsrv/tests/generic_lease_mgr_unittest.h index a86632b440..67c2eab75a 100644 --- a/src/lib/dhcpsrv/tests/generic_lease_mgr_unittest.h +++ b/src/lib/dhcpsrv/tests/generic_lease_mgr_unittest.h @@ -68,8 +68,7 @@ public: /// @param address Address to use for the initialization /// /// @return Lease6Ptr. This will not point to anything if the - /// initialization - /// failed (e.g. unknown address). + /// initialization failed (e.g. unknown address). Lease6Ptr initializeLease6(std::string address); /// @brief Check Leases present and different @@ -145,6 +144,9 @@ public: /// @brief checks that invalid dates are safely handled. void testMaxDate4(); + /// @brief checks that infinite lifetimes do not overflow. + void testInfiniteLifeTime4(); + /// @brief Test lease retrieval using client id. void testGetLease4ClientId(); @@ -244,6 +246,9 @@ public: /// @brief Checks that invalid dates are safely handled. void testMaxDate6(); + /// @brief checks that infinite lifetimes do not overflow. + void testInfiniteLifeTime6(); + /// @brief Checks that Lease6 can be stored with and without a hardware address. void testLease6MAC(); @@ -342,6 +347,14 @@ public: /// - reclaimed leases are not returned. void testGetExpiredLeases6(); + /// @brief Checks that the static (i.e. with infinite valid lifetime) + /// DHCPv4 leases will never expire. + void testStaticLeases4(); + + /// @brief Checks that the static (i.e. with infinite valid lifetime) + /// DHCPv4 leases will never expire. + void testStaticLeases6(); + /// @brief Checks that declined IPv4 leases that have expired can be retrieved. /// /// This test checks that the following: diff --git a/src/lib/dhcpsrv/tests/lease_unittest.cc b/src/lib/dhcpsrv/tests/lease_unittest.cc index 0763c0ce78..0f5b2be287 100644 --- a/src/lib/dhcpsrv/tests/lease_unittest.cc +++ b/src/lib/dhcpsrv/tests/lease_unittest.cc @@ -926,6 +926,11 @@ TEST(Lease6Test, Lease6Expired) { // Case 3: the lease is expired lease.cltt_ = time(NULL) - 102; EXPECT_TRUE(lease.expired()); + + // Case 4: the lease is static + lease.cltt_ = 1; + lease.valid_lft_ = Lease::INFINITY_LFT; + EXPECT_FALSE(lease.expired()); } // Verify that the DUID can be returned as a vector object and if DUID is NULL diff --git a/src/lib/dhcpsrv/tests/mysql_lease_mgr_unittest.cc b/src/lib/dhcpsrv/tests/mysql_lease_mgr_unittest.cc index 72a8ba8ef7..35f3ca2a17 100644 --- a/src/lib/dhcpsrv/tests/mysql_lease_mgr_unittest.cc +++ b/src/lib/dhcpsrv/tests/mysql_lease_mgr_unittest.cc @@ -277,6 +277,11 @@ TEST_F(MySqlLeaseMgrTest, maxDate4) { testMaxDate4(); } +/// @brief checks that infinite lifetimes do not overflow. +TEST_F(MySqlLeaseMgrTest, infiniteLifeTime4) { + testInfiniteLifeTime4(); +} + /// @brief Lease4 update tests /// /// Checks that we are able to update a lease in the database. @@ -404,6 +409,12 @@ TEST_F(MySqlLeaseMgrTest, getExpiredLeases4) { testGetExpiredLeases4(); } +/// @brief Checks that the static (i.e. with infinite valid lifetime) DHCPv4 +/// leases will never expire. +TEST_F(MySqlLeaseMgrTest, staticLeases4) { + testStaticLeases4(); +} + /// @brief Check that expired reclaimed DHCPv4 leases are removed. TEST_F(MySqlLeaseMgrTest, deleteExpiredReclaimedLeases4) { testDeleteExpiredReclaimedLeases4(); @@ -432,6 +443,11 @@ TEST_F(MySqlLeaseMgrTest, maxDate6) { testMaxDate6(); } +/// @brief checks that infinite lifetimes do not overflow. +TEST_F(MySqlLeaseMgrTest, infiniteLifeTime6) { + testInfiniteLifeTime6(); +} + /// @brief Verify that too long hostname for Lease6 is not accepted. /// /// Checks that the it is not possible to create a lease when the hostname @@ -543,6 +559,12 @@ TEST_F(MySqlLeaseMgrTest, getExpiredLeases6) { testGetExpiredLeases6(); } +/// @brief Checks that the static (i.e. with infinite valid lifetime) DHCPv6 +/// leases will never expire. +TEST_F(MySqlLeaseMgrTest, staticLeases6) { + testStaticLeases6(); +} + /// @brief Check that expired reclaimed DHCPv6 leases are removed. TEST_F(MySqlLeaseMgrTest, deleteExpiredReclaimedLeases6) { testDeleteExpiredReclaimedLeases6(); diff --git a/src/lib/dhcpsrv/tests/pgsql_lease_mgr_unittest.cc b/src/lib/dhcpsrv/tests/pgsql_lease_mgr_unittest.cc index ec783594b8..6e162715bd 100644 --- a/src/lib/dhcpsrv/tests/pgsql_lease_mgr_unittest.cc +++ b/src/lib/dhcpsrv/tests/pgsql_lease_mgr_unittest.cc @@ -267,6 +267,11 @@ TEST_F(PgSqlLeaseMgrTest, maxDate4) { testMaxDate4(); } +/// @brief checks that infinite lifetimes do not overflow. +TEST_F(PgSqlLeaseMgrTest, infiniteLifeTime4) { + testInfiniteLifeTime4(); +} + /// @brief Lease4 update tests /// /// Checks that we are able to update a lease in the database. @@ -394,6 +399,12 @@ TEST_F(PgSqlLeaseMgrTest, getExpiredLeases4) { testGetExpiredLeases4(); } +/// @brief Checks that the static (i.e. with infinite valid lifetime) DHCPv4 +/// leases will never expire. +TEST_F(PgSqlLeaseMgrTest, staticLeases4) { + testStaticLeases4(); +} + /// @brief Check that expired reclaimed DHCPv4 leases are removed. TEST_F(PgSqlLeaseMgrTest, deleteExpiredReclaimedLeases4) { testDeleteExpiredReclaimedLeases4(); @@ -422,6 +433,11 @@ TEST_F(PgSqlLeaseMgrTest, maxDate6) { testMaxDate6(); } +/// @brief checks that infinite lifetimes do not overflow. +TEST_F(PgSqlLeaseMgrTest, infiniteLifeTime6) { + testInfiniteLifeTime6(); +} + /// @brief Verify that too long hostname for Lease6 is not accepted. /// /// Checks that the it is not possible to create a lease when the hostname @@ -533,6 +549,12 @@ TEST_F(PgSqlLeaseMgrTest, getExpiredLeases6) { testGetExpiredLeases6(); } +/// @brief Checks that the static (i.e. with infinite valid lifetime) DHCPv6 +/// leases will never expire. +TEST_F(PgSqlLeaseMgrTest, staticLeases6) { + testStaticLeases6(); +} + /// @brief Check that expired reclaimed DHCPv6 leases are removed. TEST_F(PgSqlLeaseMgrTest, deleteExpiredReclaimedLeases6) { testDeleteExpiredReclaimedLeases6();