BIND 10 trac2837, updated. ab624f466e084edecb4a6299a56f8d33282a1ba0 [2837] Changed mysql to enforce STRICT mode Changes: - hwaddr.h - added MAX_HWADDR_LEN - hwaddr.cc - modified constructor to throw InvalidParameter if input address is too big - hwaddr_unittest.cc - added tests for new constructor throws
BIND 10 source code commits
bind10-changes at lists.isc.org
Fri Mar 15 18:21:26 UTC 2013
The branch, trac2837 has been updated
via ab624f466e084edecb4a6299a56f8d33282a1ba0 (commit)
from b47eafefecf38d32013bc7377aa4eebf531c49f6 (commit)
Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.
- Log -----------------------------------------------------------------
commit ab624f466e084edecb4a6299a56f8d33282a1ba0
Author: Thomas Markwalder <tmark at isc.org>
Date: Fri Mar 15 14:19:42 2013 -0400
[2837] Changed mysql to enforce STRICT mode
Changes:
- hwaddr.h - added MAX_HWADDR_LEN
- hwaddr.cc - modified constructor to throw InvalidParameter
if input address is too big
- hwaddr_unittest.cc - added tests for new constructor throws
- lease_mgr.h - removed HWADDR_MAX
- mysql_lease_mgr.cc - added logic to set SQL mode to STRICT
- tests/mysql_lease_mgr_unittest.cc - modified tests that were
failing on STRICT mode, to expect exceptions.
-----------------------------------------------------------------------
Summary of changes:
src/lib/dhcp/hwaddr.cc | 18 ++++++-
src/lib/dhcp/hwaddr.h | 2 +
src/lib/dhcp/tests/hwaddr_unittest.cc | 12 ++++-
src/lib/dhcpsrv/lease_mgr.h | 2 -
src/lib/dhcpsrv/mysql_lease_mgr.cc | 16 ++++--
src/lib/dhcpsrv/tests/mysql_lease_mgr_unittest.cc | 55 +++++++++------------
6 files changed, 63 insertions(+), 42 deletions(-)
-----------------------------------------------------------------------
diff --git a/src/lib/dhcp/hwaddr.cc b/src/lib/dhcp/hwaddr.cc
index d19f2ad..9e6eaf4 100644
--- a/src/lib/dhcp/hwaddr.cc
+++ b/src/lib/dhcp/hwaddr.cc
@@ -14,6 +14,7 @@
#include <dhcp/hwaddr.h>
#include <dhcp/dhcp4.h>
+#include <exceptions/exceptions.h>
#include <iomanip>
#include <sstream>
#include <vector>
@@ -26,11 +27,24 @@ HWAddr::HWAddr()
}
HWAddr::HWAddr(const uint8_t* hwaddr, size_t len, uint8_t htype)
- :hwaddr_(hwaddr, hwaddr + len), htype_(htype) {
+ :htype_(htype) {
+
+ if (len > MAX_HWADDR_LEN)
+ isc_throw(InvalidParameter, "hwaddr length exceeds MAX_HWADDR_LEN");
+
+ hwaddr_.resize(len);
+ memcpy(&hwaddr_[0], hwaddr, len);
}
HWAddr::HWAddr(const std::vector<uint8_t>& hwaddr, uint8_t htype)
- :hwaddr_(hwaddr), htype_(htype) {
+ :htype_(htype) {
+
+ if (hwaddr.size() > MAX_HWADDR_LEN)
+ isc_throw(InvalidParameter,
+ "address vector size exceeds MAX_HWADDR_LEN");
+
+ hwaddr_ = hwaddr;
+
}
std::string HWAddr::toText() const {
diff --git a/src/lib/dhcp/hwaddr.h b/src/lib/dhcp/hwaddr.h
index 93b06a1..13a16bf 100644
--- a/src/lib/dhcp/hwaddr.h
+++ b/src/lib/dhcp/hwaddr.h
@@ -27,6 +27,8 @@ namespace dhcp {
/// @brief Hardware type that represents information from DHCPv4 packet
struct HWAddr {
public:
+ /// @brief Maximum size of a hardware address.
+ static const size_t MAX_HWADDR_LEN = 20;
/// @brief default constructor
HWAddr();
diff --git a/src/lib/dhcp/tests/hwaddr_unittest.cc b/src/lib/dhcp/tests/hwaddr_unittest.cc
index 144d62d..8a6fc4a 100644
--- a/src/lib/dhcp/tests/hwaddr_unittest.cc
+++ b/src/lib/dhcp/tests/hwaddr_unittest.cc
@@ -40,9 +40,12 @@ TEST(HWAddrTest, constructor) {
const uint8_t data1[] = {0, 1, 2, 3, 4, 5, 6};
const uint8_t htype = HTYPE_ETHER;
-
vector<uint8_t> data2(data1, data1 + sizeof(data1));
+ // over the limit data
+ uint8_t big_data[HWAddr::MAX_HWADDR_LEN+1]={0};
+ vector<uint8_t> big_data_vector(big_data, big_data + sizeof(big_data));
+
scoped_ptr<HWAddr> hwaddr1(new HWAddr(data1, sizeof(data1), htype));
scoped_ptr<HWAddr> hwaddr2(new HWAddr(data2, htype));
scoped_ptr<HWAddr> hwaddr3(new HWAddr());
@@ -55,6 +58,13 @@ TEST(HWAddrTest, constructor) {
EXPECT_EQ(0, hwaddr3->hwaddr_.size());
EXPECT_EQ(htype, hwaddr3->htype_);
+
+ // check that over the limit data length throws exception
+ EXPECT_THROW(HWAddr(big_data, sizeof(big_data), HTYPE_ETHER),
+ InvalidParameter);
+
+ // check that over the limit vector throws exception
+ EXPECT_THROW(HWAddr(big_data_vector, HTYPE_ETHER), InvalidParameter);
}
// This test checks if the comparison operators are sane.
diff --git a/src/lib/dhcpsrv/lease_mgr.h b/src/lib/dhcpsrv/lease_mgr.h
index b6f7001..69c5317 100644
--- a/src/lib/dhcpsrv/lease_mgr.h
+++ b/src/lib/dhcpsrv/lease_mgr.h
@@ -211,8 +211,6 @@ struct Lease {
/// would be required. As this is a critical part of the code that will be used
/// extensively, direct access is warranted.
struct Lease4 : public Lease {
- /// @brief Maximum size of a hardware address
- static const size_t HWADDR_MAX = 20;
/// @brief Address extension
///
diff --git a/src/lib/dhcpsrv/mysql_lease_mgr.cc b/src/lib/dhcpsrv/mysql_lease_mgr.cc
index 01ce170..0b35f37 100644
--- a/src/lib/dhcpsrv/mysql_lease_mgr.cc
+++ b/src/lib/dhcpsrv/mysql_lease_mgr.cc
@@ -94,9 +94,6 @@ namespace {
/// colon separators.
const size_t ADDRESS6_TEXT_MAX_LEN = 39;
-/// @brief Maximum size of a hardware address.
-const size_t HWADDR_MAX_LEN = 20;
-
/// @brief MySQL True/False constants
///
/// Declare typed values so as to avoid problems of data conversion. These
@@ -495,7 +492,7 @@ private:
std::string columns_[LEASE_COLUMNS];///< Column names
my_bool error_[LEASE_COLUMNS]; ///< Error array
std::vector<uint8_t> hwaddr_; ///< Hardware address
- uint8_t hwaddr_buffer_[HWADDR_MAX_LEN];
+ uint8_t hwaddr_buffer_[HWAddr::MAX_HWADDR_LEN];
///< Hardware address buffer
unsigned long hwaddr_length_; ///< Hardware address length
std::vector<uint8_t> client_id_; ///< Client identification
@@ -1025,6 +1022,17 @@ MySqlLeaseMgr::openDatabase() {
mysql_error(mysql_));
}
+ // Set SQL mode options for the connection: SQL mode governs how what
+ // constitutes insertable data for a given column, and how to handle
+ // invalid data. We want to ensure we get the strictest behavior and
+ // to reject invalid data with an error.
+ const char *sql_mode = "set SESSION sql_mode ='STRICT_ALL_TABLES'";
+ result = mysql_options(mysql_, MYSQL_INIT_COMMAND, sql_mode);
+ if (result != 0) {
+ isc_throw(DbOpenError, "unable to set SQL mode options: " <<
+ mysql_error(mysql_));
+ }
+
// Open the database.
//
// The option CLIENT_FOUND_ROWS is specified so that in an UPDATE,
diff --git a/src/lib/dhcpsrv/tests/mysql_lease_mgr_unittest.cc b/src/lib/dhcpsrv/tests/mysql_lease_mgr_unittest.cc
index 2ae6a49..f4ae69c 100644
--- a/src/lib/dhcpsrv/tests/mysql_lease_mgr_unittest.cc
+++ b/src/lib/dhcpsrv/tests/mysql_lease_mgr_unittest.cc
@@ -18,6 +18,7 @@
#include <dhcpsrv/lease_mgr_factory.h>
#include <dhcpsrv/mysql_lease_mgr.h>
#include <dhcpsrv/tests/test_utils.h>
+#include <exceptions/exceptions.h>
#include <gtest/gtest.h>
@@ -807,28 +808,23 @@ TEST_F(MySqlLeaseMgrTest, getLease4HwaddrSize) {
vector<Lease4Ptr> leases = createLeases4();
// Now add leases with increasing hardware address size.
- for (uint8_t i = 0; i <= Lease4::HWADDR_MAX; ++i) {
+ for (uint8_t i = 0; i <= HWAddr::MAX_HWADDR_LEN; ++i) {
leases[1]->hwaddr_.resize(i, i);
EXPECT_TRUE(lmptr_->addLease(leases[1]));
// @todo: Simply use HWAddr directly once 2589 is implemented
- Lease4Collection returned = lmptr_->getLease4(HWAddr(leases[1]->hwaddr_, HTYPE_ETHER));
+ Lease4Collection returned =
+ lmptr_->getLease4(HWAddr(leases[1]->hwaddr_, HTYPE_ETHER));
+
ASSERT_EQ(1, returned.size());
detailCompareLease(leases[1], *returned.begin());
(void) lmptr_->deleteLease(leases[1]->addr_);
}
- // Expect some problem when accessing a lease that had too long a hardware
- // address. (The 42 is a random value put in each byte of the address.)
- // In fact the address is stored in a truncated form, so we won't find it
- // when we look.
- // @todo Check if there is some way of detecting that data added
- // to the database is truncated. There does not appear to
- // be any indication in the C API.
- leases[1]->hwaddr_.resize(Lease4::HWADDR_MAX + 100, 42);
- EXPECT_TRUE(lmptr_->addLease(leases[1]));
- // @todo: Simply use HWAddr directly once 2589 is implemented
- Lease4Collection returned = lmptr_->getLease4(HWAddr(leases[1]->hwaddr_, HTYPE_ETHER));
- EXPECT_EQ(0, returned.size());
+ // Database should not let us add one that is too big
+ // (The 42 is a random value put in each byte of the address.)
+ // @todo: 2589 will make this test impossible
+ leases[1]->hwaddr_.resize(HWAddr::MAX_HWADDR_LEN + 100, 42);
+ EXPECT_THROW(lmptr_->addLease(leases[1]), isc::dhcp::DbOperationError);
}
/// @brief Check GetLease4 methods - access by Hardware Address & Subnet ID
@@ -845,8 +841,9 @@ TEST_F(MySqlLeaseMgrTest, getLease4HwaddrSubnetId) {
// Get the leases matching the hardware address of lease 1 and
// subnet ID of lease 1. Result should be a single lease - lease 1.
// @todo: Simply use HWAddr directly once 2589 is implemented
- Lease4Ptr returned = lmptr_->getLease4(HWAddr(leases[1]->hwaddr_, HTYPE_ETHER),
- leases[1]->subnet_id_);
+ Lease4Ptr returned = lmptr_->getLease4(HWAddr(leases[1]->hwaddr_,
+ HTYPE_ETHER), leases[1]->subnet_id_);
+
ASSERT_TRUE(returned);
detailCompareLease(leases[1], returned);
@@ -881,9 +878,8 @@ TEST_F(MySqlLeaseMgrTest, getLease4HwaddrSubnetId) {
leases[1]->addr_ = leases[2]->addr_;
EXPECT_TRUE(lmptr_->addLease(leases[1]));
// @todo: Simply use HWAddr directly once 2589 is implemented
- EXPECT_THROW(returned = lmptr_->getLease4(HWAddr(leases[1]->hwaddr_, HTYPE_ETHER),
- leases[1]->subnet_id_),
- isc::dhcp::MultipleRecords);
+ EXPECT_THROW(returned = lmptr_->getLease4(HWAddr(leases[1]->hwaddr_,
+ HTYPE_ETHER), leases[1]->subnet_id_), isc::dhcp::MultipleRecords);
// Delete all leases in the database
for (int i = 0; ADDRESS4[i] != NULL; ++i) {
@@ -903,28 +899,21 @@ TEST_F(MySqlLeaseMgrTest, getLease4HwaddrSubnetIdSize) {
// Now add leases with increasing hardware address size and check
// that they can be retrieved.
- for (uint8_t i = 0; i <= Lease4::HWADDR_MAX; ++i) {
+ for (uint8_t i = 0; i <= HWAddr::MAX_HWADDR_LEN; ++i) {
leases[1]->hwaddr_.resize(i, i);
EXPECT_TRUE(lmptr_->addLease(leases[1]));
// @todo: Simply use HWAddr directly once 2589 is implemented
- Lease4Ptr returned = lmptr_->getLease4(HWAddr(leases[1]->hwaddr_, HTYPE_ETHER),
- leases[1]->subnet_id_);
+ Lease4Ptr returned = lmptr_->getLease4(HWAddr(leases[1]->hwaddr_,
+ HTYPE_ETHER), leases[1]->subnet_id_);
ASSERT_TRUE(returned);
detailCompareLease(leases[1], returned);
(void) lmptr_->deleteLease(leases[1]->addr_);
}
- // Expect some error when getting a lease with too long a hardware
- // address. Set the contents of each byte to 42, a random value.
- // @todo Check if there is some way of detecting that data added
- // to the database is truncated. There does not appear to
- // be any indication in the C API.
- leases[1]->hwaddr_.resize(Lease4::HWADDR_MAX + 100, 42);
- EXPECT_TRUE(lmptr_->addLease(leases[1]));
- // @todo: Simply use HWAddr directly once 2589 is implemented
- Lease4Ptr returned = lmptr_->getLease4(HWAddr(leases[1]->hwaddr_, HTYPE_ETHER),
- leases[1]->subnet_id_);
- EXPECT_FALSE(returned);
+ // Database should not let us add one that is too big
+ // (The 42 is a random value put in each byte of the address.)
+ leases[1]->hwaddr_.resize(HWAddr::MAX_HWADDR_LEN + 100, 42);
+ EXPECT_THROW(lmptr_->addLease(leases[1]), isc::dhcp::DbOperationError);
}
/// @brief Check GetLease4 methods - access by Client ID
More information about the bind10-changes
mailing list