From: Razvan Becheriu <razvan@isc.org>
Date: Fri, 9 Aug 2019 15:23:47 +0000 (+0300)
Subject: [#805, !6-p] check for null or empty hwaddr before writing to memfile (v4)
X-Git-Tag: Kea-1.6.0~41^2~7
X-Git-Url: http://git.ipfire.org/gitweb/gitweb.cgi?a=commitdiff_plain;h=f551f62e49e6435d419e3e5566440fd2fe7e71e8;p=thirdparty%2Fkea.git

[#805, !6-p] check for null or empty hwaddr before writing to memfile (v4)
---

diff --git a/src/lib/dhcpsrv/csv_lease_file4.cc b/src/lib/dhcpsrv/csv_lease_file4.cc
index 646da9f25d..3d4c707802 100644
--- a/src/lib/dhcpsrv/csv_lease_file4.cc
+++ b/src/lib/dhcpsrv/csv_lease_file4.cc
@@ -36,13 +36,22 @@ CSVLeaseFile4::append(const Lease4& lease) {
 
     CSVRow row(getColumnCount());
     row.writeAt(getColumnIndex("address"), lease.addr_.toText());
+    std::string hwaddr_text;
     if (!lease.hwaddr_) {
         // Bump the error counter
         ++write_errs_;
 
         isc_throw(BadValue, "Lease4 must have hardware address specified.");
+    } else {
+        hwaddr_text = lease.hwaddr_->toText(false);
+        if (hwaddr_text.empty() && lease.state_ != Lease::STATE_DECLINED) {
+            // Bump the error counter
+            ++write_errs_;
+
+            isc_throw(BadValue, "Lease4 must have non empty hardware address.");
+        }
     }
-    row.writeAt(getColumnIndex("hwaddr"), lease.hwaddr_->toText(false));
+    row.writeAt(getColumnIndex("hwaddr"), hwaddr_text);
     // Client id may be unset (NULL).
     if (lease.client_id_) {
         row.writeAt(getColumnIndex("client_id"), lease.client_id_->toText());
diff --git a/src/lib/dhcpsrv/tests/csv_lease_file4_unittest.cc b/src/lib/dhcpsrv/tests/csv_lease_file4_unittest.cc
index 23bfce4ed4..f85efc5f6b 100644
--- a/src/lib/dhcpsrv/tests/csv_lease_file4_unittest.cc
+++ b/src/lib/dhcpsrv/tests/csv_lease_file4_unittest.cc
@@ -462,6 +462,70 @@ TEST_F(CSVLeaseFile4Test, highLeaseLifetime) {
     EXPECT_EQ(lease->cltt_, lease_read->cltt_);
 }
 
+// Verifies that it is not possible to output a lease with empty hwaddr in other
+// than the declined state
+TEST_F(CSVLeaseFile4Test, emptyHWAddrDefaultStateOnly) {
+    CSVLeaseFile4 lf(filename_);
+    ASSERT_NO_THROW(lf.recreate());
+    ASSERT_TRUE(io_.exists());
+
+    HWAddrPtr hwaddr;
+
+    // Create lease with null hwaddr and default state
+    Lease4Ptr lease_null_hwaddr(new Lease4(IOAddress("192.0.3.2"),
+                                hwaddr,
+                                NULL, 0,
+                                0xFFFFFFFF, time(0),
+                                8, true, true,
+                                "host.example.com"));
+    // Try to write this lease out to the lease file.
+    ASSERT_THROW(lf.append(*lease_null_hwaddr), BadValue);
+
+    hwaddr.reset(new HWAddr());
+
+    //Create lease with empty hdaddr and default state
+    Lease4Ptr lease_empty_hwaddr(new Lease4(IOAddress("192.0.3.2"),
+                                 hwaddr,
+                                 NULL, 0,
+                                 0xFFFFFFFF, time(0),
+                                 8, true, true,
+                                 "host.example.com"));
+    // Try to write this lease out to the lease file.
+    ASSERT_THROW(lf.append(*lease_empty_hwaddr), BadValue);
+
+    // Create lease with hwaddr and current time.
+    Lease4Ptr lease(new Lease4(IOAddress("192.0.3.2"),
+                               hwaddr0_,
+                               NULL, 0,
+                               0xFFFFFFFF, time(0),
+                               8, true, true,
+                               "host.example.com"));
+
+    // Decline the lease
+    lease->decline(1000);
+    ASSERT_TRUE(lease->hwaddr_);
+    EXPECT_EQ(lease->hwaddr_->toText(false), "");
+
+    // Write this lease out to the lease file.
+    ASSERT_NO_THROW(lf.append(*lease));
+
+    // Close the lease file.
+    lf.close();
+
+    Lease4Ptr lease_read;
+
+    // Re-open the file for reading.
+    ASSERT_NO_THROW(lf.open());
+
+    // Read the lease and make sure it is successful.
+    EXPECT_TRUE(lf.next(lease_read));
+    ASSERT_TRUE(lease_read);
+
+    // The valid lifetime and the cltt should match with the original lease.
+    EXPECT_EQ(lease->valid_lft_, lease_read->valid_lft_);
+    EXPECT_EQ(lease->cltt_, lease_read->cltt_);
+}
+
 /// @todo Currently we don't check invalid lease attributes, such as invalid
 /// lease type, invalid preferred lifetime vs valid lifetime etc. The Lease6
 /// should be extended with the function that validates lease attributes. Once