From 209ac10ebe457bcefbcc180c937d4f97222d510e Mon Sep 17 00:00:00 2001 From: Thomas Markwalder Date: Fri, 14 Jun 2019 15:47:33 -0400 Subject: [PATCH] [#651,!384] Fix input IAID in postgresql lease6 fetchers src/lib/dhcpsrv/pgsql_lease_mgr.cc PgSqlLease6Exchange::Uiaid - made this public and added function to return the int32 bit value as a string PgSqlLeaseMgr::getLeases6() - uses Uiaid.dbInputString() now src/lib/dhcpsrv/tests/generic_lease_mgr_unittest.* GenericLeaseMgrTest::testLease6LargeIaidCheck() - new test to ensure large value iaids can be stored. src/lib/dhcpsrv/tests/pgsql_lease_mgr_unittest.cc TEST_F(PgSqlLeaseMgrTest, leases6LargeIaidCheck) - new test src/lib/dhcpsrv/tests/cql_lease_mgr_unittest.cc TEST_F(CqlLeaseMgrTest, lease6InvalidHostname) - new test src/lib/dhcpsrv/tests/mysql_lease_mgr_unittest.cc TEST_F(MySqlLeaseMgrTest, leases6LargeIaidCheck) - new test --- src/lib/dhcpsrv/pgsql_lease_mgr.cc | 46 ++++++++++++------- .../dhcpsrv/tests/cql_lease_mgr_unittest.cc | 8 ++++ .../tests/generic_lease_mgr_unittest.cc | 33 +++++++++++++ .../tests/generic_lease_mgr_unittest.h | 5 +- .../dhcpsrv/tests/mysql_lease_mgr_unittest.cc | 8 ++++ .../dhcpsrv/tests/pgsql_lease_mgr_unittest.cc | 8 ++++ 6 files changed, 90 insertions(+), 18 deletions(-) diff --git a/src/lib/dhcpsrv/pgsql_lease_mgr.cc b/src/lib/dhcpsrv/pgsql_lease_mgr.cc index f49f6f4360..26ff0c1c7b 100644 --- a/src/lib/dhcpsrv/pgsql_lease_mgr.cc +++ b/src/lib/dhcpsrv/pgsql_lease_mgr.cc @@ -614,6 +614,31 @@ private: static const size_t LEASE_COLUMNS = 17; public: + + /// @brief Union for marshalling IAID into and out of the database + /// IAID is defined in the RFC as 4 octets, which Kea code handles as + /// a uint32_t. Postgresql however, offers only signed integer types + /// of sizes 2, 4, and 8 bytes (SMALLINT, INT, and BIGINT respectively). + /// IAID is used in several indexes so rather than use the BIGINT, we + /// use this union to safely move the value into and out of an INT column. + union Uiaid { + /// @brief Constructor + /// @param val unsigned 32 bit value for the IAID. + Uiaid(uint32_t val) : uval_(val){}; + + /// @brief Constructor + /// @param val signed 32 bit value for the IAID. + Uiaid(int32_t val) : ival_(val){}; + + /// @brief Return a string representing the signed 32-bit value. + std::string dbInputString() { + return (boost::lexical_cast(ival_)); + }; + + uint32_t uval_; + int32_t ival_; + }; + PgSqlLease6Exchange() : lease_(), duid_length_(0), duid_(), iaid_u_(0), iaid_str_(""), lease_type_(Lease6::TYPE_NA), lease_type_str_(""), prefix_len_(0), @@ -691,7 +716,7 @@ public: // lexically cast from an integer version to avoid out of range // exception failure upon insert. iaid_u_.uval_ = lease_->iaid_; - iaid_str_ = boost::lexical_cast(iaid_u_.ival_); + iaid_str_ = iaid_u_.dbInputString(); bind_array.add(iaid_str_); prefix_len_str_ = boost::lexical_cast @@ -889,20 +914,7 @@ private: size_t duid_length_; vector duid_; uint8_t duid_buffer_[DUID::MAX_DUID_LEN]; - - /// @brief Union for marshalling IAID into and out of the database - /// IAID is defined in the RFC as 4 octets, which Kea code handles as - /// a uint32_t. Postgresql however, offers only signed integer types - /// of sizes 2, 4, and 8 bytes (SMALLINT, INT, and BIGINT respectively). - /// IAID is used in several indexes so rather than use the BIGINT, we - /// use this union to safely move the value into and out of an INT column. - union Uiaid { - Uiaid(uint32_t val) : uval_(val){}; - Uiaid(int32_t val) : ival_(val){}; - uint32_t uval_; - int32_t ival_; - } iaid_u_; - + union Uiaid iaid_u_; std::string iaid_str_; Lease6::Type lease_type_; std::string lease_type_str_; @@ -1454,7 +1466,7 @@ PgSqlLeaseMgr::getLeases6(Lease::Type lease_type, const DUID& duid, bind_array.add(duid.getDuid()); // IAID - std::string iaid_str = boost::lexical_cast(iaid); + std::string iaid_str = PgSqlLease6Exchange::Uiaid(iaid).dbInputString(); bind_array.add(iaid_str); // LEASE_TYPE @@ -1486,7 +1498,7 @@ PgSqlLeaseMgr::getLeases6(Lease::Type lease_type, const DUID& duid, bind_array.add(duid.getDuid()); // IAID - std::string iaid_str = boost::lexical_cast(iaid); + std::string iaid_str = PgSqlLease6Exchange::Uiaid(iaid).dbInputString(); bind_array.add(iaid_str); // SUBNET ID diff --git a/src/lib/dhcpsrv/tests/cql_lease_mgr_unittest.cc b/src/lib/dhcpsrv/tests/cql_lease_mgr_unittest.cc index b3ee6623d4..8c31f811d0 100644 --- a/src/lib/dhcpsrv/tests/cql_lease_mgr_unittest.cc +++ b/src/lib/dhcpsrv/tests/cql_lease_mgr_unittest.cc @@ -656,6 +656,14 @@ TEST_F(CqlLeaseMgrTest, lease6InvalidHostname) { testLease6InvalidHostname(); } +/// @brief Verify that large IAID values work correctly. +/// +/// Adds lease with a large IAID to the database and verifies it can +/// fetched correclty. +TEST_F(CqlLeaseMgrTest, leases6LargeIaidCheck) { + testLease6LargeIaidCheck(); +} + /// @brief Check GetLease6 methods - access by DUID/IAID /// /// Adds leases to the database and checks that they can be accessed via diff --git a/src/lib/dhcpsrv/tests/generic_lease_mgr_unittest.cc b/src/lib/dhcpsrv/tests/generic_lease_mgr_unittest.cc index 30c0c6a3dc..bc01cbb30b 100644 --- a/src/lib/dhcpsrv/tests/generic_lease_mgr_unittest.cc +++ b/src/lib/dhcpsrv/tests/generic_lease_mgr_unittest.cc @@ -1567,6 +1567,39 @@ GenericLeaseMgrTest::testLease6LeaseTypeCheck() { } } +void +GenericLeaseMgrTest::testLease6LargeIaidCheck() { + + DuidPtr duid(new DUID(vector(8, 0x77))); + IOAddress addr(std::string("2001:db8:1::111")); + SubnetID subnet_id = 8; // radom number + + // Use a value we know is larger than 32-bit signed max. + uint32_t large_iaid = 0xFFFFFFFE; + + // We should be able to add with no problems. + Lease6Ptr lease(new Lease6(Lease::TYPE_NA, addr, duid, large_iaid, + 100, 200, 0, 0, subnet_id)); + ASSERT_TRUE(lmptr_->addLease(lease)); + + // Sanity check that we added it. + Lease6Ptr found_lease = lmptr_->getLease6(Lease::TYPE_NA, addr); + ASSERT_TRUE(found_lease); + EXPECT_TRUE(*found_lease == *lease); + + // Verify getLease6() duid/iaid finds it. + found_lease = lmptr_->getLease6(Lease::TYPE_NA, *duid, large_iaid, subnet_id); + ASSERT_TRUE(found_lease); + EXPECT_TRUE(*found_lease == *lease); + + // Verify getLeases6() duid/iaid finds it. + Lease6Collection found_leases = lmptr_->getLeases6(Lease::TYPE_NA, + *duid, large_iaid); + // We should match the lease. + ASSERT_EQ(1, found_leases.size()); + EXPECT_TRUE(*(found_leases[0]) == *lease); +} + void GenericLeaseMgrTest::testGetLease6DuidIaidSubnetId() { // Get the leases to be used for the test and add them to the database. diff --git a/src/lib/dhcpsrv/tests/generic_lease_mgr_unittest.h b/src/lib/dhcpsrv/tests/generic_lease_mgr_unittest.h index d9228c367b..a86632b440 100644 --- a/src/lib/dhcpsrv/tests/generic_lease_mgr_unittest.h +++ b/src/lib/dhcpsrv/tests/generic_lease_mgr_unittest.h @@ -210,6 +210,9 @@ public: /// @brief Test method which returns all IPv6 leases for Subnet ID. void testGetLeases6SubnetId(); + /// @brief Test making/fetching leases with IAIDs > signed 32-bit max. + void testLease6LargeIaidCheck(); + /// @brief Test method which returns all IPv6 leases. void testGetLeases6(); @@ -272,7 +275,7 @@ public: /// Adds leases to the database and checks that they can be accessed via /// a combination of DIUID and IAID. void testGetLease6DuidIaidSubnetId(); - + /// @brief verifies getLeases6 method by DUID /// /// Adds 3 leases to backend and retrieves, verifes empty diff --git a/src/lib/dhcpsrv/tests/mysql_lease_mgr_unittest.cc b/src/lib/dhcpsrv/tests/mysql_lease_mgr_unittest.cc index e0b2125b0d..72a8ba8ef7 100644 --- a/src/lib/dhcpsrv/tests/mysql_lease_mgr_unittest.cc +++ b/src/lib/dhcpsrv/tests/mysql_lease_mgr_unittest.cc @@ -440,6 +440,14 @@ TEST_F(MySqlLeaseMgrTest, lease6InvalidHostname) { testLease6InvalidHostname(); } +/// @brief Verify that large IAID values work correctly. +/// +/// Adds lease with a large IAID to the database and verifies it can +/// fetched correclty. +TEST_F(MySqlLeaseMgrTest, leases6LargeIaidCheck) { + testLease6LargeIaidCheck(); +} + /// @brief Check GetLease6 methods - access by DUID/IAID /// /// Adds leases to the database and checks that they can be accessed via diff --git a/src/lib/dhcpsrv/tests/pgsql_lease_mgr_unittest.cc b/src/lib/dhcpsrv/tests/pgsql_lease_mgr_unittest.cc index 6298e9dbd2..ec783594b8 100644 --- a/src/lib/dhcpsrv/tests/pgsql_lease_mgr_unittest.cc +++ b/src/lib/dhcpsrv/tests/pgsql_lease_mgr_unittest.cc @@ -430,6 +430,14 @@ TEST_F(PgSqlLeaseMgrTest, lease6InvalidHostname) { testLease6InvalidHostname(); } +/// @brief Verify that large IAID values work correctly. +/// +/// Adds lease with a large IAID to the database and verifies it can +/// fetched correclty. +TEST_F(PgSqlLeaseMgrTest, leases6LargeIaidCheck) { + testLease6LargeIaidCheck(); +} + /// @brief Check GetLease6 methods - access by DUID/IAID /// /// Adds leases to the database and checks that they can be accessed via -- GitLab