From bf4de7854c8bb52e86d5f7ee582a64da5c77d0ca Mon Sep 17 00:00:00 2001 From: Tomek Mrugalski Date: Thu, 26 Jul 2018 01:35:44 +0200 Subject: [PATCH] [5682] Corrective sanity checks implemented for leaseX-add --- src/hooks/dhcp/lease_cmds/lease_cmds.cc | 13 +- src/hooks/dhcp/lease_cmds/lease_parser.cc | 47 +++- .../lease_cmds/tests/lease_cmds_unittest.cc | 222 +++++++++++++++++- 3 files changed, 266 insertions(+), 16 deletions(-) diff --git a/src/hooks/dhcp/lease_cmds/lease_cmds.cc b/src/hooks/dhcp/lease_cmds/lease_cmds.cc index 59face144b..27045e81f4 100644 --- a/src/hooks/dhcp/lease_cmds/lease_cmds.cc +++ b/src/hooks/dhcp/lease_cmds/lease_cmds.cc @@ -14,6 +14,7 @@ #include #include #include +#include #include #include #include @@ -252,6 +253,8 @@ LeaseCmdsImpl::leaseAddHandler(CalloutHandle& handle) { ConstSrvConfigPtr config = CfgMgr::instance().getCurrentCfg(); + SanityChecker checks; + Lease4Ptr lease4; Lease6Ptr lease6; // This parameter is ignored for the commands adding the lease. @@ -261,8 +264,12 @@ LeaseCmdsImpl::leaseAddHandler(CalloutHandle& handle) { lease4 = parser.parse(config, cmd_args_, force_create); // checkLeaseIntegrity(config, lease4); - if (lease4) { + checks.checkLease(lease4); + if (!lease4) { + isc_throw(BadValue, "Lease failed sanity checks."); + } + LeaseMgrFactory::instance().addLease(lease4); } @@ -273,6 +280,10 @@ LeaseCmdsImpl::leaseAddHandler(CalloutHandle& handle) { // checkLeaseIntegrity(config, lease6); if (lease6) { + checks.checkLease(lease6); + if (!lease6) { + isc_throw(BadValue, "Lease failed sanity checks."); + } LeaseMgrFactory::instance().addLease(lease6); } } diff --git a/src/hooks/dhcp/lease_cmds/lease_parser.cc b/src/hooks/dhcp/lease_cmds/lease_parser.cc index 3956741fae..9c0c90d450 100644 --- a/src/hooks/dhcp/lease_cmds/lease_parser.cc +++ b/src/hooks/dhcp/lease_cmds/lease_parser.cc @@ -8,6 +8,8 @@ #include #include #include +#include +#include #include #include @@ -42,6 +44,21 @@ Lease4Parser::parse(ConstSrvConfigPtr& cfg, HWAddrPtr hwaddr_ptr = HWAddrPtr(new HWAddr(hwaddr)); Subnet4Ptr subnet = cfg->getCfgSubnets4()->getSubnet(subnet_id); + if (!subnet) { + + CfgConsistency::LeaseSanity sanity = + CfgMgr::instance().getCurrentCfg()->getConsistency() + ->getLeaseSanityCheck(); + + if (sanity == CfgConsistency::LEASE_CHECK_FIX || + sanity == CfgConsistency::LEASE_CHECK_FIX_DEL) { + // Ok, we got an invalid subnet-id, but the sanity checks + // are telling us to try to save the day. Let's try to + // find a better subnets. + subnet = cfg->getCfgSubnets4()->selectSubnet(addr); + } + } + if (!subnet) { isc_throw(BadValue, "Invalid subnet-id: No IPv4 subnet with subnet-id=" << subnet_id << " currently configured."); @@ -175,13 +192,6 @@ Lease6Parser::parse(ConstSrvConfigPtr& cfg, DUID duid = DUID::fromText(duid_txt); DuidPtr duid_ptr = DuidPtr(new DUID(duid)); - // Check if the subnet-id specified is sane. - Subnet6Ptr subnet = cfg->getCfgSubnets6()->getSubnet(subnet_id); - if (!subnet) { - isc_throw(BadValue, "Invalid subnet-id: No IPv6 subnet with subnet-id=" - << subnet_id << " currently configured."); - } - Lease::Type type = Lease::TYPE_NA; uint8_t prefix_len = 128; if (lease_info->contains("type")) { @@ -200,6 +210,29 @@ Lease6Parser::parse(ConstSrvConfigPtr& cfg, } } + // Check if the subnet-id specified is sane. + Subnet6Ptr subnet = cfg->getCfgSubnets6()->getSubnet(subnet_id); + if (!subnet && (type == Lease::TYPE_NA)) { + + CfgConsistency::LeaseSanity sanity = + CfgMgr::instance().getCurrentCfg()->getConsistency() + ->getLeaseSanityCheck(); + + if (sanity == CfgConsistency::LEASE_CHECK_FIX || + sanity == CfgConsistency::LEASE_CHECK_FIX_DEL) { + // Ok, we got an invalid subnet-id, but the sanity checks + // are telling us to try to save the day. Let's try to + // find a better subnets. + subnet = cfg->getCfgSubnets6()->selectSubnet(addr); + } + } + + if (!subnet) { + isc_throw(BadValue, "Invalid subnet-id: No IPv6 subnet with subnet-id=" + << subnet_id << " currently configured."); + } + + // Check if the address specified really belongs to the subnet. if ((type == Lease::TYPE_NA) && !subnet->inRange(addr)) { isc_throw(BadValue, "The address " << addr.toText() << " does not belong " diff --git a/src/hooks/dhcp/lease_cmds/tests/lease_cmds_unittest.cc b/src/hooks/dhcp/lease_cmds/tests/lease_cmds_unittest.cc index 4a18e39fa4..5192aa4383 100644 --- a/src/hooks/dhcp/lease_cmds/tests/lease_cmds_unittest.cc +++ b/src/hooks/dhcp/lease_cmds/tests/lease_cmds_unittest.cc @@ -276,15 +276,29 @@ public: if (insert_lease) { if (v6) { - lmptr_->addLease(createLease6("2001:db8:1::1", 66, 0x42)); - lmptr_->addLease(createLease6("2001:db8:1::2", 66, 0x56)); - lmptr_->addLease(createLease6("2001:db8:2::1", 99, 0x42)); - lmptr_->addLease(createLease6("2001:db8:2::2", 99, 0x56)); + Lease6Ptr l = createLease6("2001:db8:1::1", 66, 0x42); + lmptr_->addLease(l); + + l = createLease6("2001:db8:1::2", 66, 0x56); + lmptr_->addLease(l); + + l = createLease6("2001:db8:2::1", 99, 0x42); + lmptr_->addLease(l); + + l = createLease6("2001:db8:2::2", 99, 0x56); + lmptr_->addLease(l); } else { - lmptr_->addLease(createLease4("192.0.2.1", 44, 0x08, 0x42)); - lmptr_->addLease(createLease4("192.0.2.2", 44, 0x09, 0x56)); - lmptr_->addLease(createLease4("192.0.3.1", 88, 0x08, 0x42)); - lmptr_->addLease(createLease4("192.0.3.2", 88, 0x09, 0x56)); + Lease4Ptr l = createLease4("192.0.2.1", 44, 0x08, 0x42); + lmptr_->addLease(l); + + l = createLease4("192.0.2.2", 44, 0x09, 0x56); + lmptr_->addLease(l); + + l = createLease4("192.0.3.1", 88, 0x08, 0x42); + lmptr_->addLease(l); + + l = createLease4("192.0.3.2", 88, 0x09, 0x56); + lmptr_->addLease(l); } } } @@ -690,6 +704,100 @@ TEST_F(LeaseCmdsTest, Lease4Add) { } +// Check that a well formed lease but with invalid subnet-id is +// rejected when sanity-checks are set to DEL. +TEST_F(LeaseCmdsTest, Lease4AddSanityCheckDel) { + + // Initialize lease manager (false = v4, false = don't add leases) + initLeaseMgr(false, false); + + // Set the sanity checks level. + CfgMgr::instance().getCurrentCfg()->getConsistency() + ->setLeaseSanityCheck(CfgConsistency::LEASE_CHECK_DEL); + + // Check that the lease manager pointer is there. + ASSERT_TRUE(lmptr_); + + // Now send the command. + string txt = + "{\n" + " \"command\": \"lease4-add\",\n" + " \"arguments\": {" + " \"subnet-id\": 444,\n" + " \"ip-address\": \"192.0.2.202\",\n" + " \"hw-address\": \"1a:1b:1c:1d:1e:1f\"\n" + " }\n" + "}"; + string exp_rsp = "Invalid subnet-id: No IPv4 subnet with " + "subnet-id=444 currently configured."; + testCommand(txt, CONTROL_RESULT_ERROR, exp_rsp); +} + +// Check that a well formed lease but with invalid subnet-id is +// corrected when sanity-checks is set to FIX. +TEST_F(LeaseCmdsTest, Lease4AddSanityCheckFixOk) { + + // Initialize lease manager (false = v4, false = don't add leases) + initLeaseMgr(false, false); + + // Set the sanity checks level. + CfgMgr::instance().getCurrentCfg()->getConsistency() + ->setLeaseSanityCheck(CfgConsistency::LEASE_CHECK_FIX); + + // Check that the lease manager pointer is there. + ASSERT_TRUE(lmptr_); + + // Now send the command. + string txt = + "{\n" + " \"command\": \"lease4-add\",\n" + " \"arguments\": {" + " \"subnet-id\": 444,\n" + " \"ip-address\": \"192.0.2.202\",\n" + " \"hw-address\": \"1a:1b:1c:1d:1e:1f\"\n" + " }\n" + "}"; + string exp_rsp = "Lease added."; + testCommand(txt, CONTROL_RESULT_SUCCESS, exp_rsp); + + // Now check that the lease is really there. + Lease4Ptr l = lmptr_->getLease4(IOAddress("192.0.2.202")); + ASSERT_TRUE(l); + + // Make sure the lease has subnet-id corrected. + EXPECT_EQ(44, l->subnet_id_); +} + +// Check that a well formed lease but with invalid subnet-id is +// still rejected, if there are no appropriate subnets. +// (there is subnet with subnet-id 444 and there is no subnet +// that 192.0.222.202 belongs to. +TEST_F(LeaseCmdsTest, Lease4AddSanityCheckFixDel) { + + // Initialize lease manager (false = v4, false = don't add leases) + initLeaseMgr(false, false); + + CfgMgr::instance().getCurrentCfg()->getConsistency() + ->setLeaseSanityCheck(CfgConsistency::LEASE_CHECK_FIX); + + // Check that the lease manager pointer is there. + ASSERT_TRUE(lmptr_); + + // Now send the command. + string txt = + "{\n" + " \"command\": \"lease4-add\",\n" + " \"arguments\": {" + " \"subnet-id\": 444,\n" + " \"ip-address\": \"192.0.222.202\",\n" + " \"hw-address\": \"1a:1b:1c:1d:1e:1f\"\n" + " }\n" + "}"; + string exp_rsp = "Invalid subnet-id: No IPv4 subnet with " + "subnet-id=444 currently configured."; + testCommand(txt, CONTROL_RESULT_ERROR, exp_rsp); +} + // Check that a well formed lease4 with tons of parameters can be added. TEST_F(LeaseCmdsTest, Lease4AddFull) { @@ -981,6 +1089,104 @@ TEST_F(LeaseCmdsTest, Lease6Add) { EXPECT_FALSE(l->getContext()); } +// Check that a well formed lease but with invalid subnet-id is +// rejected when sanity-checks are set to DEL. +TEST_F(LeaseCmdsTest, Lease6AddSanityCheckDel) { + + // Initialize lease manager (true = v6, false = don't add leases) + initLeaseMgr(true, false); + + // Set the sanity checks level. + CfgMgr::instance().getCurrentCfg()->getConsistency() + ->setLeaseSanityCheck(CfgConsistency::LEASE_CHECK_DEL); + + // Check that the lease manager pointer is there. + ASSERT_TRUE(lmptr_); + + // Now send the command. + string txt = + "{\n" + " \"command\": \"lease6-add\",\n" + " \"arguments\": {" + " \"subnet-id\": 666,\n" + " \"ip-address\": \"2001:db8:1::3\",\n" + " \"duid\": \"1a:1b:1c:1d:1e:1f\",\n" + " \"iaid\": 1234\n" + " }\n" + "}"; + string exp_rsp = "Invalid subnet-id: No IPv6 subnet with " + "subnet-id=666 currently configured."; + testCommand(txt, CONTROL_RESULT_ERROR, exp_rsp); +} + +// Check that a simple, well formed lease6 can be added. +TEST_F(LeaseCmdsTest, Lease6AddSanityCheckFixOk) { + + // Initialize lease manager (true = v6, false = don't add leases) + initLeaseMgr(true, false); + + // Set the sanity checks level. + CfgMgr::instance().getCurrentCfg()->getConsistency() + ->setLeaseSanityCheck(CfgConsistency::LEASE_CHECK_FIX); + + // Check that the lease manager pointer is there. + ASSERT_TRUE(lmptr_); + + // Now send the command. + string txt = + "{\n" + " \"command\": \"lease6-add\",\n" + " \"arguments\": {" + " \"subnet-id\": 666,\n" + " \"ip-address\": \"2001:db8:1::3\",\n" + " \"duid\": \"1a:1b:1c:1d:1e:1f\",\n" + " \"iaid\": 1234\n" + " }\n" + "}"; + string exp_rsp = "Lease added."; + testCommand(txt, CONTROL_RESULT_SUCCESS, exp_rsp); + + // Now check that the lease is really there. + Lease6Ptr l = lmptr_->getLease6(Lease::TYPE_NA, IOAddress("2001:db8:1::3")); + ASSERT_TRUE(l); + + // Make sure the lease has subnet-id corrected. + EXPECT_EQ(66, l->subnet_id_); +} + +// Check that a well formed lease but with invalid subnet-id is +// still rejected, if there are no appropriate subnets. +// (there is subnet with subnet-id 666 and there is no subnet +// that 2001:fff:1::3 belongs to. +TEST_F(LeaseCmdsTest, Lease6AddSanityCheckFixDel) { + + // Initialize lease manager (true = v6, false = don't add leases) + initLeaseMgr(true, false); + + // Set the sanity checks level. + CfgMgr::instance().getCurrentCfg()->getConsistency() + ->setLeaseSanityCheck(CfgConsistency::LEASE_CHECK_FIX); + + // Check that the lease manager pointer is there. + ASSERT_TRUE(lmptr_); + + // Now send the command. + string txt = + "{\n" + " \"command\": \"lease6-add\",\n" + " \"arguments\": {" + " \"subnet-id\": 666,\n" + " \"ip-address\": \"2001:fff:1::3\",\n" + " \"duid\": \"1a:1b:1c:1d:1e:1f\",\n" + " \"iaid\": 1234\n" + " }\n" + "}"; + string exp_rsp = "Invalid subnet-id: No IPv6 subnet with " + "subnet-id=666 currently configured."; + testCommand(txt, CONTROL_RESULT_ERROR, exp_rsp); +} + + // Check that a simple, well formed prefix lease can be added. TEST_F(LeaseCmdsTest, Lease6AddPrefix) { -- 2.47.2