From: Tomek Mrugalski Date: Mon, 30 Jul 2018 12:20:30 +0000 (+0200) Subject: [5682] Revert "[5682] Corrective sanity checks implemented for leaseX-add" X-Git-Tag: ha_phase2~12 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=dde35b6f4d5b2d896caf0171bde87d36d5a4f1da;p=thirdparty%2Fkea.git [5682] Revert "[5682] Corrective sanity checks implemented for leaseX-add" This reverts commit 4d1462582ff4aa1ec663dba5d2dadedb7c0984ed. --- diff --git a/src/hooks/dhcp/lease_cmds/lease_cmds.cc b/src/hooks/dhcp/lease_cmds/lease_cmds.cc index 27045e81f4..59face144b 100644 --- a/src/hooks/dhcp/lease_cmds/lease_cmds.cc +++ b/src/hooks/dhcp/lease_cmds/lease_cmds.cc @@ -14,7 +14,6 @@ #include #include #include -#include #include #include #include @@ -253,8 +252,6 @@ 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. @@ -264,12 +261,8 @@ 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."); - } + if (lease4) { LeaseMgrFactory::instance().addLease(lease4); } @@ -280,10 +273,6 @@ 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 9c0c90d450..3956741fae 100644 --- a/src/hooks/dhcp/lease_cmds/lease_parser.cc +++ b/src/hooks/dhcp/lease_cmds/lease_parser.cc @@ -8,8 +8,6 @@ #include #include #include -#include -#include #include #include @@ -44,21 +42,6 @@ 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."); @@ -192,6 +175,13 @@ 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")) { @@ -210,29 +200,6 @@ 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 9181209212..1a09a5b2a1 100644 --- a/src/hooks/dhcp/lease_cmds/tests/lease_cmds_unittest.cc +++ b/src/hooks/dhcp/lease_cmds/tests/lease_cmds_unittest.cc @@ -276,29 +276,15 @@ public: if (insert_lease) { if (v6) { - 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); + 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)); } else { - 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); + 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)); } } } @@ -704,100 +690,6 @@ 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) { @@ -1089,104 +981,6 @@ 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) {