From 209ac10ebe457bcefbcc180c937d4f97222d510e Mon Sep 17 00:00:00 2001 From: Thomas Markwalder Date: Fri, 14 Jun 2019 15:47:33 -0400 Subject: [PATCH 1/5] [#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 From b58541c344708e56d93032c5f09a2b4a058970af Mon Sep 17 00:00:00 2001 From: Thomas Markwalder Date: Mon, 17 Jun 2019 15:18:05 -0400 Subject: [PATCH 2/5] [#651,!384] Added ChangeLog entry --- ChangeLog | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/ChangeLog b/ChangeLog index e4a9f4d4f1..9bf661f6c0 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,9 @@ +1606. [bug] tmark + Corrected an error with retrieving DHCPv6 leases, whose IAID + values are larger than int32_t max, from Postgresql lease + databases. + (Gitlab #651,!384, git TBD) + 1605. [func] marcin Extended mysql_cb hooks library to support new API calls for managing the DHCP servers in the database. In addition, added -- GitLab From 586839c1f1d69a8524b4cea9f7caea9f9b64161f Mon Sep 17 00:00:00 2001 From: Thomas Markwalder Date: Fri, 28 Jun 2019 12:18:59 -0400 Subject: [PATCH 3/5] [#651,!384] Updated ChangeLog entry --- ChangeLog | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ChangeLog b/ChangeLog index 9bf661f6c0..37cea71fd0 100644 --- a/ChangeLog +++ b/ChangeLog @@ -2,7 +2,7 @@ Corrected an error with retrieving DHCPv6 leases, whose IAID values are larger than int32_t max, from Postgresql lease databases. - (Gitlab #651,!384, git TBD) + (Gitlab #651,!384, git b58541c344708e56d93032c5f09a2b4a058970af) 1605. [func] marcin Extended mysql_cb hooks library to support new API calls for -- GitLab From 67e047df61d56558d474514a21ed0db96152557a Mon Sep 17 00:00:00 2001 From: Thomas Markwalder Date: Fri, 28 Jun 2019 13:55:39 -0400 Subject: [PATCH 4/5] [#651,!384] Fixed unit test --- src/lib/dhcpsrv/tests/generic_lease_mgr_unittest.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/lib/dhcpsrv/tests/generic_lease_mgr_unittest.cc b/src/lib/dhcpsrv/tests/generic_lease_mgr_unittest.cc index bc01cbb30b..765a86948e 100644 --- a/src/lib/dhcpsrv/tests/generic_lease_mgr_unittest.cc +++ b/src/lib/dhcpsrv/tests/generic_lease_mgr_unittest.cc @@ -1572,14 +1572,14 @@ GenericLeaseMgrTest::testLease6LargeIaidCheck() { DuidPtr duid(new DUID(vector(8, 0x77))); IOAddress addr(std::string("2001:db8:1::111")); - SubnetID subnet_id = 8; // radom number + SubnetID subnet_id = 8; // random 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)); + 100, 200, subnet_id)); ASSERT_TRUE(lmptr_->addLease(lease)); // Sanity check that we added it. -- GitLab From 29eef3fb17c1dfd9765d971ad2721e6bb09cf552 Mon Sep 17 00:00:00 2001 From: Thomas Markwalder Date: Fri, 28 Jun 2019 13:57:19 -0400 Subject: [PATCH 5/5] [#651,!384] Updated ChangeLog entry --- ChangeLog | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ChangeLog b/ChangeLog index 37cea71fd0..7fda87db0e 100644 --- a/ChangeLog +++ b/ChangeLog @@ -2,7 +2,7 @@ Corrected an error with retrieving DHCPv6 leases, whose IAID values are larger than int32_t max, from Postgresql lease databases. - (Gitlab #651,!384, git b58541c344708e56d93032c5f09a2b4a058970af) + (Gitlab #651,!384, git 67e047df61d56558d474514a21ed0db96152557a) 1605. [func] marcin Extended mysql_cb hooks library to support new API calls for -- GitLab