diff --git a/src/lib/dhcpsrv/database_backends.dox b/src/lib/dhcpsrv/database_backends.dox index 11ee9f7203dcd368cb49dbfb67e96dae53c72955..556b6a83bfcd41ffa3f557ddd83fb2d5582640b4 100644 --- a/src/lib/dhcpsrv/database_backends.dox +++ b/src/lib/dhcpsrv/database_backends.dox @@ -22,16 +22,22 @@ the abstract isc::dhcp::LeaseMgr class. This provides methods to create, retrieve, modify and delete leases in the database. - There are currently two available Lease Managers, MySQL and Memfile: + There are currently three available Lease Managers, Memfile, MySQL and + PostgreSQL: + + - Memfile is an in-memory lease database which can be configured to persist + its content to disk in a flat-file. Support for the Memfile database + backend is built into Kea DHCP. - The MySQL lease manager uses the freely available MySQL as its backend - database. This is not included in BIND 10 DHCP by default: + database. This is not included in Kea DHCP by default: the \--with-dhcp-mysql switch must be supplied to "configure" for support to be compiled into the software. - - Memfile is an in-memory lease database, with (currently) nothing being - written to persistent storage. The long-term plans for the backend do - include the ability to store this on disk, but it is currently a - low-priority item. + + - The PostgreSQL lease manager uses the freely available PostgreSQL as its + backend database. This is not included in Kea DHCP by default: + the \--with-dhcp-pgsql switch must be supplied to "configure" for + support to be compiled into the software. @section dhcpdb-instantiation Instantiation of Lease Managers @@ -74,6 +80,16 @@ - user - database user ID under which the database is accessed. If not specified, no user ID is used - the database is assumed to be open. + @subsection dhcpdb-keywords-pgsql PostgreSQL connection string keywords + + - host - host on which the selected database is running. If not + supplied, "localhost" is assumed. + - name - name of the PostgreSQL database to access. There is no + default - this must always be supplied. + - password - password for the selected user ID (see below). If not + specified, no password is used. + - user - database user ID under which the database is accessed. If not + specified, no user ID is used - the database is assumed to be open. @section dhcp-backend-unittest Running Unit Tests @@ -81,7 +97,7 @@ certain database-specific pre-requisites for successfully running the unit tests. These are listed in the following sections. - @subsection dhcp-mysql-unittest MySQL + @subsection dhcp-mysql-unittest MySQL Unit Tests A database called keatest must be created. A database user, also called keatest (and with a password keatest) must also be created and @@ -122,7 +138,7 @@ that BIND 10 has been build with the \--with-dhcp-mysql switch (see the installation section in the BIND 10 Guide). - @subsection dhcp-pgsql-unittest PostgreSQL unit-tests + @subsection dhcp-pgsql-unittest PostgreSQL Unit Tests Conceptually, the steps required to run PostgreSQL unit-tests are the same as in MySQL. First, a database called keatest must be created. A database diff --git a/src/lib/dhcpsrv/lease_mgr.cc b/src/lib/dhcpsrv/lease_mgr.cc index e4fa07f2b9beb878e6c475b1bb65a9e46bf24a24..6a8f358358579f9ff073aa53a8f6516818a23162 100644 --- a/src/lib/dhcpsrv/lease_mgr.cc +++ b/src/lib/dhcpsrv/lease_mgr.cc @@ -32,6 +32,8 @@ using namespace std; namespace isc { namespace dhcp { +const time_t LeaseMgr::MAX_DB_TIME = 2147483647; + std::string LeaseMgr::getParameter(const std::string& name) const { ParameterMap::const_iterator param = parameters_.find(name); if (param == parameters_.end()) { diff --git a/src/lib/dhcpsrv/lease_mgr.h b/src/lib/dhcpsrv/lease_mgr.h index aab7b9ed0546cd240ab0692b80ff131a8bbdf18c..001fef34dabacfb7731d891677cfe4f3568bdcaf 100644 --- a/src/lib/dhcpsrv/lease_mgr.h +++ b/src/lib/dhcpsrv/lease_mgr.h @@ -122,6 +122,10 @@ public: /// see the documentation of those classes for details. class LeaseMgr { public: + /// @brief Defines maxiumum value for time that can be reliably stored. + // If I'm still alive I'll be too old to care. You fix it. + static const time_t MAX_DB_TIME; + /// Database configuration parameter map typedef std::map ParameterMap; diff --git a/src/lib/dhcpsrv/pgsql_lease_mgr.cc b/src/lib/dhcpsrv/pgsql_lease_mgr.cc index f84f517dbf4b749bf460695e21f81308f6313e85..eef68d5a87b971446d577b0dd2dce71b0e1d6613 100644 --- a/src/lib/dhcpsrv/pgsql_lease_mgr.cc +++ b/src/lib/dhcpsrv/pgsql_lease_mgr.cc @@ -268,6 +268,7 @@ std::string PsqlBindArray::toText() { class PgSqlLeaseExchange { public: + PgSqlLeaseExchange() : addr_str_(""), valid_lifetime_(0), valid_lft_str_(""), expire_(0), expire_str_(""), subnet_id_(0), subnet_id_str_(""), @@ -289,6 +290,14 @@ public: /// @return std::string containing the stringified time std::string convertToDatabaseTime(const time_t& time_val) { + // PostgreSQL does funny things with time if you get past Y2038. It + // will accept the values (unlike MySQL which throws) but it + // stops correctly adjusting to local time when reading them back + // out. So lets disallow it here. + if (time_val > LeaseMgr::MAX_DB_TIME) { + isc_throw(BadValue, "Time value is too large: " << time_val); + } + struct tm tinfo; char buffer[20]; localtime_r(&time_val, &tinfo); @@ -326,10 +335,11 @@ public: /// /// @return a const char* pointer to the column's raw data /// @throw DbOperationError if the value cannot be fetched. - const char* getColumnValue(PGresult*& r, const int row, const size_t col) { + const char* getRawColumnValue(PGresult*& r, const int row, + const size_t col) { const char* value = PQgetvalue(r, row, col); if (!value) { - isc_throw(DbOperationError, "getColumnValue no data for :" + isc_throw(DbOperationError, "getRawColumnValue no data for :" << getColumnLabel(col) << " row:" << row); } @@ -347,7 +357,7 @@ public: /// invalid. void getColumnValue(PGresult*& r, const int row, const size_t col, bool &value) { - const char* data = getColumnValue(r, row, col); + const char* data = getRawColumnValue(r, row, col); if (!strlen(data) || *data == 'f') { value = false; } else if (*data == 't') { @@ -370,7 +380,7 @@ public: /// invalid. void getColumnValue(PGresult*& r, const int row, const size_t col, uint32_t &value) { - const char* data = getColumnValue(r, row, col); + const char* data = getRawColumnValue(r, row, col); try { value = boost::lexical_cast(data); } catch (const std::exception& ex) { @@ -391,7 +401,7 @@ public: /// invalid. void getColumnValue(PGresult*& r, const int row, const size_t col, uint8_t &value) { - const char* data = getColumnValue(r, row, col); + const char* data = getRawColumnValue(r, row, col); try { // lexically casting as uint8_t doesn't convert from char // so we use uint16_t and implicitly convert. @@ -458,7 +468,7 @@ public: // Returns converted bytes in a dynamically allocated buffer, and // sets bytes_converted. unsigned char* bytes = PQunescapeBytea((const unsigned char*) - (getColumnValue(r, row, col)), + (getRawColumnValue(r, row, col)), &bytes_converted); // Unlikely it couldn't allocate it but you never know. @@ -649,8 +659,8 @@ public: getColumnValue(r, row, VALID_LIFETIME_COL, valid_lifetime_); - expire_ = convertFromDatabaseTime(getColumnValue(r, row, - EXPIRE_COL)); + expire_ = convertFromDatabaseTime(getRawColumnValue(r, row, + EXPIRE_COL)); getColumnValue(r, row , SUBNET_ID_COL, subnet_id_); @@ -659,7 +669,7 @@ public: getColumnValue(r, row, FQDN_FWD_COL, fqdn_fwd_); getColumnValue(r, row, FQDN_REV_COL, fqdn_rev_); - hostname_ = getColumnValue(r, row, HOSTNAME_COL); + hostname_ = getRawColumnValue(r, row, HOSTNAME_COL); return (Lease4Ptr(new Lease4(addr4_, hwaddr_buffer_, hwaddr_length_, client_id_buffer_, client_id_length_, @@ -761,7 +771,11 @@ public: addr_str_ = lease_->addr_.toText(); bind_array.add(addr_str_); - bind_array.add(lease_->duid_->getDuid()); + if (lease_->duid_) { + bind_array.add(lease_->duid_->getDuid()); + } else { + isc_throw (BadValue, "IPv6 Lease cannot have a null DUID"); + } valid_lft_str_ = boost::lexical_cast (lease->valid_lft_); @@ -819,8 +833,8 @@ public: getColumnValue(r, row, VALID_LIFETIME_COL, valid_lifetime_); - expire_ = convertFromDatabaseTime(getColumnValue(r, row, - EXPIRE_COL)); + expire_ = convertFromDatabaseTime(getRawColumnValue(r, row, + EXPIRE_COL)); cltt_ = expire_ - valid_lifetime_; @@ -837,7 +851,7 @@ public: getColumnValue(r, row, FQDN_FWD_COL, fqdn_fwd_); getColumnValue(r, row, FQDN_REV_COL, fqdn_rev_); - hostname_ = getColumnValue(r, row, HOSTNAME_COL); + hostname_ = getRawColumnValue(r, row, HOSTNAME_COL); Lease6Ptr result(new Lease6(lease_type_, addr, duid_ptr, iaid_, pref_lifetime_, valid_lifetime_, 0, 0, @@ -863,7 +877,7 @@ public: /// invalid. isc::asiolink::IOAddress getIPv6Value(PGresult*& r, const int row, const size_t col) { - const char* data = getColumnValue(r, row, col); + const char* data = getRawColumnValue(r, row, col); try { return (isc::asiolink::IOAddress(data)); } catch (const std::exception& ex) { diff --git a/src/lib/dhcpsrv/pgsql_lease_mgr.h b/src/lib/dhcpsrv/pgsql_lease_mgr.h index db46b844612ad1174c7ea7ce1808430ecb93e0bb..86c1e9b6a5d0e56b67d25170cb780cc616d953ce 100644 --- a/src/lib/dhcpsrv/pgsql_lease_mgr.h +++ b/src/lib/dhcpsrv/pgsql_lease_mgr.h @@ -28,7 +28,7 @@ namespace isc { namespace dhcp { /// @brief Structure used to bind C++ input values to dynamic SQL parameters -/// The structure contains three vectors which storing the input values, +/// The structure contains three vectors which store the input values, /// data lengths, and formats. These vectors are passed directly into the /// PostgreSQL execute call. /// @@ -69,19 +69,37 @@ struct PsqlBindArray { return (values_.empty()); } - /// @brief Adds a char value to the array. + /// @brief Adds a char array to bind array based + /// + /// Adds a TEXT_FMT value to the end of the bind array, using the given + /// char* as the data source. Note that value is expected to be NULL + /// terminated. + /// + /// @param value char array containing the null-terminated text to add. void add(const char* value); - /// @brief Adds a string value to the array. + /// @brief Adds an string value to the bind array + /// + /// Adds a TEXT formatted value to the end of the bind array using the + /// given string as the data source. + /// + /// @param value std::string containing the value to add. void add(const std::string& value); - /// @brief Adds a vector to the array. + /// @brief Adds a binary value to the bind array. + /// + /// Adds a BINARY_FMT value to the end of the bind array using the + /// given vector as the data source. + /// + /// @param value vector of binary bytes. void add(const std::vector& data); - /// @brief Adds a uint32_t value to the array. - void add(const uint32_t& value); - - /// @brief Adds a bool value to the array. + /// @brief Adds a boolean value to the bind array. + /// + /// Converts the given boolean value to its corresponding to PostgreSQL + /// string value and adds it as a TEXT_FMT value to the bind array. + /// + /// @param value std::string containing the value to add. void add(const bool& value); /// @brief Dumps the contents of the array to a string. @@ -370,16 +388,14 @@ public: /// @brief Commit Transactions /// - /// Commits all pending database operations. On databases that don't - /// support transactions, this is a no-op. + /// Commits all pending database operations. /// /// @throw DbOperationError Iif the commit failed. virtual void commit(); /// @brief Rollback Transactions /// - /// Rolls back all pending database operations. On databases that don't - /// support transactions, this is a no-op. + /// Rolls back all pending database operations. /// /// @throw DbOperationError If the rollback failed. virtual void rollback(); diff --git a/src/lib/dhcpsrv/tests/generic_lease_mgr_unittest.cc b/src/lib/dhcpsrv/tests/generic_lease_mgr_unittest.cc index 70fb327867dbca1b602a06f4f7ca89769c4e9661..e9bde2465c844922f69a8f011e14b67ef5d1c18a 100644 --- a/src/lib/dhcpsrv/tests/generic_lease_mgr_unittest.cc +++ b/src/lib/dhcpsrv/tests/generic_lease_mgr_unittest.cc @@ -682,6 +682,29 @@ GenericLeaseMgrTest::testAddGetDelete6(bool check_t1_t2) { EXPECT_FALSE(x); } +void +GenericLeaseMgrTest::testMaxDate4() { + // Get the leases to be used for the test. + vector leases = createLeases4(); + Lease4Ptr lease = leases[1]; + + // Set valid_lft_ to 1 day, cllt_ to max time. This should make expire + // time too large to store. + lease->valid_lft_ = 24*60*60; + lease->cltt_ = LeaseMgr::MAX_DB_TIME; + + // Insert should throw. + ASSERT_THROW(lmptr_->addLease(leases[1]), DbOperationError); + + // Set valid_lft_ to 0, which should make expire time small enough to + // store and try again. + lease->valid_lft_ = 0; + 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. @@ -710,7 +733,6 @@ GenericLeaseMgrTest::testBasicLease4() { detailCompareLease(leases[3], l_returned); // Check that we can't add a second lease with the same address -std::cout << "OK - Duplicate check here!!!!!!" << std::endl; EXPECT_FALSE(lmptr_->addLease(leases[1])); // Delete a lease, check that it's gone, and that we can't delete it @@ -829,6 +851,29 @@ GenericLeaseMgrTest::testBasicLease6() { detailCompareLease(leases[2], l_returned); } +void +GenericLeaseMgrTest::testMaxDate6() { + // Get the leases to be used for the test. + vector leases = createLeases6(); + Lease6Ptr lease = leases[1]; + + // Set valid_lft_ to 1 day, cllt_ to max time. This should make expire + // time too large to store. + lease->valid_lft_ = 24*60*60; + lease->cltt_ = LeaseMgr::MAX_DB_TIME; + + // Insert should throw. + ASSERT_THROW(lmptr_->addLease(leases[1]), DbOperationError); + + // Set valid_lft_ to 0, which should make expire time small enough to + // store and try again. + lease->valid_lft_ = 0; + EXPECT_TRUE(lmptr_->addLease(leases[1])); + Lease6Ptr l_returned = lmptr_->getLease6(leasetype6_[1], ioaddress6_[1]); + ASSERT_TRUE(l_returned); + detailCompareLease(leases[1], l_returned); +} + void GenericLeaseMgrTest::testLease4InvalidHostname() { // 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 c153f08237bd20a68eaa9a93bc34cac096a3fcd5..9058f54039298e6d9bb9447f12594558f6443339 100644 --- a/src/lib/dhcpsrv/tests/generic_lease_mgr_unittest.h +++ b/src/lib/dhcpsrv/tests/generic_lease_mgr_unittest.h @@ -105,6 +105,9 @@ public: /// @brief checks that addLease, getLease4(addr) and deleteLease() works void testBasicLease4(); + /// @brief checks that invalid dates are safely handled. + void testMaxDate4(); + /// @brief Test lease retrieval using client id. void testGetLease4ClientId(); @@ -180,6 +183,10 @@ public: /// IPv6 address) works. void testBasicLease6(); + /// @brief checks that invalid dates are safely handled. + void testMaxDate6(); + + /// @brief Test that IPv6 lease can be added, retrieved and deleted. /// /// This method checks basic IPv6 lease operations. There's check_t1_t2 diff --git a/src/lib/dhcpsrv/tests/mysql_lease_mgr_unittest.cc b/src/lib/dhcpsrv/tests/mysql_lease_mgr_unittest.cc index b9854518e049546dffa2eaa30e0b1a7df7e9b6cf..fecf32c33e2ccd8d58b8d8a85820f4039d7c837d 100644 --- a/src/lib/dhcpsrv/tests/mysql_lease_mgr_unittest.cc +++ b/src/lib/dhcpsrv/tests/mysql_lease_mgr_unittest.cc @@ -333,6 +333,11 @@ TEST_F(MySqlLeaseMgrTest, basicLease4) { testBasicLease4(); } +/// @brief Check that Lease4 code safely handles invalid dates. +TEST_F(MySqlLeaseMgrTest, maxDate4) { + testMaxDate4(); +} + /// @brief Lease4 update tests /// /// Checks that we are able to update a lease in the database. @@ -437,6 +442,11 @@ TEST_F(MySqlLeaseMgrTest, basicLease6) { testBasicLease6(); } +/// @brief Check that Lease6 code safely handles invalid dates. +TEST_F(MySqlLeaseMgrTest, maxDate6) { + testMaxDate6(); +} + /// @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 diff --git a/src/lib/dhcpsrv/tests/pgsql_lease_mgr_unittest.cc b/src/lib/dhcpsrv/tests/pgsql_lease_mgr_unittest.cc index 649ae9af9de9267a03fe38be1a91d3b73d07af6e..d3eb6ff2f43ca76aec3cec6a2f6146c6cb803fcd 100644 --- a/src/lib/dhcpsrv/tests/pgsql_lease_mgr_unittest.cc +++ b/src/lib/dhcpsrv/tests/pgsql_lease_mgr_unittest.cc @@ -299,6 +299,11 @@ TEST_F(PgSqlLeaseMgrTest, basicLease4) { testBasicLease4(); } +/// @brief Check that Lease4 code safely handles invalid dates. +TEST_F(PgSqlLeaseMgrTest, maxDate4) { + testMaxDate4(); +} + /// @brief Lease4 update tests /// /// Checks that we are able to update a lease in the database. @@ -403,6 +408,11 @@ TEST_F(PgSqlLeaseMgrTest, basicLease6) { testBasicLease6(); } +/// @brief Check that Lease6 code safely handles invalid dates. +TEST_F(PgSqlLeaseMgrTest, maxDate6) { + testMaxDate6(); +} + /// @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