From afc1fe7f558db142fa5321a3535ab19aea499004 Mon Sep 17 00:00:00 2001 From: Tomek Mrugalski Date: Thu, 12 Feb 2015 17:43:18 +0100 Subject: [PATCH] [3565] The v6 server is now able to allocate new leases during Renew --- src/bin/dhcp6/dhcp6_srv.cc | 4 + src/bin/dhcp6/tests/dhcp6_srv_unittest.cc | 44 +++-- src/bin/dhcp6/tests/dhcp6_test_utils.cc | 210 ++++++++++++++++++---- src/bin/dhcp6/tests/dhcp6_test_utils.h | 47 +++-- 4 files changed, 251 insertions(+), 54 deletions(-) diff --git a/src/bin/dhcp6/dhcp6_srv.cc b/src/bin/dhcp6/dhcp6_srv.cc index 37ac9a0f56..13ac19dd3e 100644 --- a/src/bin/dhcp6/dhcp6_srv.cc +++ b/src/bin/dhcp6/dhcp6_srv.cc @@ -1587,6 +1587,10 @@ Dhcpv6Srv::extendIA_NA(const Subnet6Ptr& subnet, const DuidPtr& duid, hints_count++; } + // If this is a Renew, it's ok to allocate new leases + if (query->getType() == DHCPV6_RENEW) { + ctx.allow_new_leases_in_renewals_ = true; + } Lease6Collection leases = alloc_engine_->renewLeases6(ctx); // Ok, now we have the leases extended. We have: diff --git a/src/bin/dhcp6/tests/dhcp6_srv_unittest.cc b/src/bin/dhcp6/tests/dhcp6_srv_unittest.cc index 8efc923dc0..bdb36ddda9 100644 --- a/src/bin/dhcp6/tests/dhcp6_srv_unittest.cc +++ b/src/bin/dhcp6/tests/dhcp6_srv_unittest.cc @@ -903,20 +903,38 @@ TEST_F(Dhcpv6SrvTest, pdRenewBasic) { } // This test verifies that incoming (invalid) RENEW with an address -// can be handled properly. -// -// This test checks 3 scenarios: -// 1. there is no such lease at all -// 2. there is such a lease, but it is assigned to a different IAID -// 3. there is such a lease, but it belongs to a different client +// can be handled properly. This has changed with #3565. The server +// is now able to allocate a lease in Renew if it's available. +// Previous testRenewReject is now split into 3 tests. // -// expected: -// - returned REPLY message has copy of client-id -// - returned REPLY message has server-id -// - returned REPLY message has IA_NA that includes STATUS-CODE -// - No lease in LeaseMgr -TEST_F(Dhcpv6SrvTest, RenewReject) { - testRenewReject(Lease::TYPE_NA, IOAddress("2001:db8:1:1::dead")); +// This test checks the first scenario: There is no lease at all. +// The server will try to assign it. Since it is not used by anyone else, +// the server will assign it. This is convenient for various types +// of recoveries, e.g. when the server lost its database. +TEST_F(Dhcpv6SrvTest, RenewUnknown) { + // False means that the lease should not be created before renewal attempt + testRenewBasic(Lease::TYPE_NA, "2001:db8:1:1::abc", "2001:db8:1:1::abc", + 128, false); +} + +// This test checks that a client that renews existing lease, but uses +// a wrong IAID, will be processed correctly. As there is no lease for +// this (duid, type, iaid) tuple, this is treated as a new IA, regardless +// if the client inserted an address that is used in a different IA. +// After #3565 was implemented, the server will attempt to assign a lease. +// The one that client requested is already used with different IAID, so +// it will just pick a different lease. This is the second out of three +// scenarios tests by old RenewReject test. +TEST_F(Dhcpv6SrvTest, RenewWrongIAID) { + testRenewWrongIAID(Lease::TYPE_NA, IOAddress("2001:db8:1:1::abc")); +} + +// This test checks whether client A can renew an address that is currently +// leased by client B. The server should detect that the lease belong to +// someone else and assign a different lease. This is the third out of three +// scenarios tests by old RenewReject test. +TEST_F(Dhcpv6SrvTest, RenewSomeoneElesesLease) { + testRenewSomeoneElsesLease(Lease::TYPE_NA, IOAddress("2001:db8::1")); } // This test verifies that incoming (invalid) RENEW with a prefix diff --git a/src/bin/dhcp6/tests/dhcp6_test_utils.cc b/src/bin/dhcp6/tests/dhcp6_test_utils.cc index eec0886e37..98f043a76f 100644 --- a/src/bin/dhcp6/tests/dhcp6_test_utils.cc +++ b/src/bin/dhcp6/tests/dhcp6_test_utils.cc @@ -200,7 +200,7 @@ Dhcpv6SrvTest::createIA(isc::dhcp::Lease::Type lease_type, void Dhcpv6SrvTest::testRenewBasic(Lease::Type type, const std::string& existing_addr, const std::string& renew_addr, - const uint8_t prefix_len) { + const uint8_t prefix_len, bool insert_before_renew) { NakedDhcpv6Srv srv(0); const IOAddress existing(existing_addr); @@ -213,24 +213,27 @@ Dhcpv6SrvTest::testRenewBasic(Lease::Type type, const std::string& existing_addr // Check that the address we are about to use is indeed in pool ASSERT_TRUE(subnet_->inPool(type, existing)); - // Note that preferred, valid, T1 and T2 timers and CLTT are set to invalid - // value on purpose. They should be updated during RENEW. - Lease6Ptr lease(new Lease6(type, existing, duid_, iaid, 501, 502, 503, 504, - subnet_->getID(), HWAddrPtr(), prefix_len)); - lease->cltt_ = 1234; - ASSERT_TRUE(LeaseMgrFactory::instance().addLease(lease)); + Lease6Ptr l; + if (insert_before_renew) { + // Note that preferred, valid, T1 and T2 timers and CLTT are set to invalid + // value on purpose. They should be updated during RENEW. + Lease6Ptr lease(new Lease6(type, existing, duid_, iaid, 501, 502, 503, 504, + subnet_->getID(), HWAddrPtr(), prefix_len)); + lease->cltt_ = 1234; + ASSERT_TRUE(LeaseMgrFactory::instance().addLease(lease)); - // Check that the lease is really in the database - Lease6Ptr l = LeaseMgrFactory::instance().getLease6(type, existing); - ASSERT_TRUE(l); + // Check that the lease is really in the database + l = LeaseMgrFactory::instance().getLease6(type, existing); + ASSERT_TRUE(l); - // Check that T1, T2, preferred, valid and cltt really set and not using - // previous (500, 501, etc.) values - EXPECT_NE(l->t1_, subnet_->getT1()); - EXPECT_NE(l->t2_, subnet_->getT2()); - EXPECT_NE(l->preferred_lft_, subnet_->getPreferred()); - EXPECT_NE(l->valid_lft_, subnet_->getValid()); - EXPECT_NE(l->cltt_, time(NULL)); + // Check that T1, T2, preferred, valid and cltt really set and not using + // previous (500, 501, etc.) values + EXPECT_NE(l->t1_, subnet_->getT1()); + EXPECT_NE(l->t2_, subnet_->getT2()); + EXPECT_NE(l->preferred_lft_, subnet_->getPreferred()); + EXPECT_NE(l->valid_lft_, subnet_->getValid()); + EXPECT_NE(l->cltt_, time(NULL)); + } Pkt6Ptr req = createMessage(DHCPV6_RENEW, type, IOAddress(renew_addr), prefix_len, iaid); @@ -299,6 +302,114 @@ Dhcpv6SrvTest::testRenewBasic(Lease::Type type, const std::string& existing_addr EXPECT_TRUE(LeaseMgrFactory::instance().deleteLease(renew_addr)); } +void +Dhcpv6SrvTest::testRenewWrongIAID(Lease::Type type, const IOAddress& addr) { + + NakedDhcpv6Srv srv(0); + + const uint32_t transid = 1234; + const uint32_t valid_iaid = 234; + const uint32_t bogus_iaid = 456; + + uint8_t prefix_len = (type == Lease::TYPE_PD) ? 128 : pd_pool_->getLength(); + + // Quick sanity check that the address we're about to use is ok + ASSERT_TRUE(subnet_->inPool(type, addr)); + + // Check that the lease is NOT in the database + Lease6Ptr l = LeaseMgrFactory::instance().getLease6(type, addr); + ASSERT_FALSE(l); + + // GenerateClientId() also sets duid_ + OptionPtr clientid = generateClientId(); + + // Note that preferred, valid, T1 and T2 timers and CLTT are set to invalid + // value on purpose. They should be updated during RENEW. + Lease6Ptr lease(new Lease6(type, addr, duid_, valid_iaid, + 501, 502, 503, 504, subnet_->getID(), + HWAddrPtr(), prefix_len)); + ASSERT_TRUE(LeaseMgrFactory::instance().addLease(lease)); + + // Pass it to the server and hope for a REPLY + // Let's create a RENEW + Pkt6Ptr renew = createMessage(DHCPV6_RENEW, type, IOAddress(addr), prefix_len, + bogus_iaid); + renew->addOption(clientid); + renew->addOption(srv.getServerID()); + + // The duid and address matches, but the iaid is different. The server could + // respond with NoBinding. However, according to + // draft-ietf-dhc-dhcpv6-stateful-issues-10, the server can also assign a + // new address. And that's what we expect here. + Pkt6Ptr reply = srv.processRenew(renew); + checkResponse(reply, DHCPV6_REPLY, transid); + + // Check that IA_NA was returned and that there's an address included + boost::shared_ptr + addr_opt = checkIA_NA(reply, bogus_iaid, subnet_->getT1(), subnet_->getT2()); + + ASSERT_TRUE(addr_opt); + + // Check that we've got the an address + checkIAAddr(addr_opt, addr_opt->getAddress(), Lease::TYPE_NA); + + // Check that we got a different address than was in the database. + EXPECT_NE(addr_opt->getAddress().toText(), addr.toText()); + + // Check that the lease is really in the database + l = checkLease(duid_, reply->getOption(D6O_IA_NA), addr_opt); + ASSERT_TRUE(l); +} + +void +Dhcpv6SrvTest::testRenewSomeoneElsesLease(Lease::Type type, const IOAddress& addr) { + + NakedDhcpv6Srv srv(0); + const uint32_t valid_iaid = 234; + const uint32_t transid = 1234; + + uint8_t prefix_len = (type == Lease::TYPE_PD) ? 128 : pd_pool_->getLength(); + + // GenerateClientId() also sets duid_ + OptionPtr clientid = generateClientId(); + + // Let's create a lease. + Lease6Ptr lease(new Lease6(type, addr, duid_, valid_iaid, + 501, 502, 503, 504, subnet_->getID(), + HWAddrPtr(), prefix_len)); + ASSERT_TRUE(LeaseMgrFactory::instance().addLease(lease)); + + // CASE 3: Lease belongs to a client with different client-id + Pkt6Ptr renew = createMessage(DHCPV6_RENEW, type, IOAddress(addr), prefix_len, + valid_iaid); + renew->addOption(generateClientId(13)); // generate different DUID (length 13) + renew->addOption(srv.getServerID()); + + // The iaid and address matches, but the duid is different. + // The server should not renew it, but assign something else. + Pkt6Ptr reply = srv.processRenew(renew); + checkResponse(reply, DHCPV6_REPLY, transid); + OptionPtr tmp = reply->getOption(D6O_IA_NA); + ASSERT_TRUE(tmp); + + // Check that IA_?? was returned and that there's proper status code + // Check that IA_NA was returned and that there's an address included + boost::shared_ptr + addr_opt = checkIA_NA(reply, valid_iaid, subnet_->getT1(), subnet_->getT2()); + + ASSERT_TRUE(addr_opt); + + // Check that we've got the an address + checkIAAddr(addr_opt, addr_opt->getAddress(), Lease::TYPE_NA); + + // Check that we got a different address than was in the database. + EXPECT_NE(addr_opt->getAddress().toText(), addr.toText()); + + // Check that the lease is really in the database + Lease6Ptr l = checkLease(duid_, reply->getOption(D6O_IA_NA), addr_opt); + ASSERT_TRUE(l); +} + void Dhcpv6SrvTest::testRenewReject(Lease::Type type, const IOAddress& addr) { @@ -349,7 +460,41 @@ Dhcpv6SrvTest::testRenewReject(Lease::Type type, const IOAddress& addr) { // Check that IA_?? was returned and that there's proper status code boost::shared_ptr ia = boost::dynamic_pointer_cast(tmp); ASSERT_TRUE(ia); - checkIA_NAStatusCode(ia, STATUS_NoBinding, subnet_->getT1(), subnet_->getT2()); + + if (type == Lease::TYPE_PD) { + // For PD, the check is easy. NoBinding and no prefixes + checkIA_NAStatusCode(ia, STATUS_NoBinding, subnet_->getT1(), subnet_->getT2()); + } else { + // For IA, it's more involved, as the server will reject the address + // (and send it with 0 lifetimes), but will also assign a new address. + + // First, check that the requested address is rejected. + bool found = false; + + dhcp::OptionCollection options = ia->getOptions(); + for (isc::dhcp::OptionCollection::iterator opt = options.begin(); + opt != options.end(); ++opt) { + if (opt->second->getType() != D6O_IAADDR) { + continue; + } + + dhcp::Option6IAAddrPtr opt_addr = + boost::dynamic_pointer_cast(opt->second); + ASSERT_TRUE(opt_addr); + + if (opt_addr->getAddress() != addr) { + // There may be other addresses, e.g. the newly assigned one + continue; + } + + found = true; + EXPECT_NE(0, opt_addr->getPreferred()); + EXPECT_NE(0, opt_addr->getValid()); + } + + EXPECT_TRUE(found) << "Expected address " << addr.toText() + << " with zero lifetimes not found."; + } // Check that there is no lease added l = LeaseMgrFactory::instance().getLease6(type, addr); @@ -654,24 +799,27 @@ Dhcpv6SrvTest::compareOptions(const isc::dhcp::OptionPtr& option1, void NakedDhcpv6SrvTest::checkIA_NAStatusCode( const boost::shared_ptr& ia, - uint16_t expected_status_code, uint32_t expected_t1, uint32_t expected_t2) + uint16_t expected_status_code, uint32_t expected_t1, uint32_t expected_t2, + bool check_addr) { // Make sure there is no address assigned. Depending on the situation, // the server will either not return the address at all and sometimes // it will return it with zeroed lifetimes. - dhcp::OptionCollection options = ia->getOptions(); - for (isc::dhcp::OptionCollection::iterator opt = options.begin(); - opt != options.end(); ++opt) { - if (opt->second->getType() != D6O_IAADDR) { - continue; + if (check_addr) { + dhcp::OptionCollection options = ia->getOptions(); + for (isc::dhcp::OptionCollection::iterator opt = options.begin(); + opt != options.end(); ++opt) { + if (opt->second->getType() != D6O_IAADDR) { + continue; + } + + dhcp::Option6IAAddrPtr addr = + boost::dynamic_pointer_cast(opt->second); + ASSERT_TRUE(addr); + + EXPECT_EQ(0, addr->getPreferred()); + EXPECT_EQ(0, addr->getValid()); } - - dhcp::Option6IAAddrPtr addr = - boost::dynamic_pointer_cast(opt->second); - ASSERT_TRUE(addr); - - EXPECT_EQ(0, addr->getPreferred()); - EXPECT_EQ(0, addr->getValid()); } // T1, T2 should NOT be zeroed. draft-ietf-dhc-dhcpv6-stateful-issues-10, diff --git a/src/bin/dhcp6/tests/dhcp6_test_utils.h b/src/bin/dhcp6/tests/dhcp6_test_utils.h index 6d758383d1..79839fc615 100644 --- a/src/bin/dhcp6/tests/dhcp6_test_utils.h +++ b/src/bin/dhcp6/tests/dhcp6_test_utils.h @@ -231,18 +231,26 @@ public: expected_t2); } - // Checks that server rejected IA_NA, i.e. that it has no addresses and - // that expected status code really appears there. In some limited cases - // (reply to RELEASE) it may be used to verify positive case, where - // IA_NA response is expected to not include address. - // - // Status code indicates type of error encountered (in theory it can also - // indicate success, but servers typically don't send success status - // as this is the default result and it saves bandwidth) + /// @brief Checks that the server inserted expected status code in IA_NA + /// + /// Check that the server used status code as expected, i.e. that it has + /// no addresses (if status code is non-zero) and that expected status + /// code really appears there. In some limited cases (reply to RELEASE) + /// it may be used to verify positive case, where IA_NA response is + /// expected to not include address. + /// + /// Status code indicates type of error encountered (in theory it can also + /// indicate success, but servers typically don't send success status + /// as this is the default result and it saves bandwidth) + /// @param ia IA_NA container to be checked + /// @param expected_status_code expected value in status-code option + /// @param expected_t1 expected T1 in IA_NA option + /// @param expected_t2 expected T2 in IA_NA option + /// @param check_addr whether to check for address with 0 lifetimes void checkIA_NAStatusCode (const boost::shared_ptr& ia, uint16_t expected_status_code, uint32_t expected_t1, - uint32_t expected_t2); + uint32_t expected_t2, bool check_addr = true); void checkMsgStatusCode(const isc::dhcp::Pkt6Ptr& msg, uint16_t expected_status) @@ -464,10 +472,29 @@ public: /// @param existing_addr address to be preinserted into the database /// @param renew_addr address being sent in RENEW /// @param prefix_len length of the prefix (128 for addresses) + /// @param insert_before_renew should the lease be inserted into the database + /// before we try renewal? void testRenewBasic(isc::dhcp::Lease::Type type, const std::string& existing_addr, - const std::string& renew_addr, const uint8_t prefix_len); + const std::string& renew_addr, const uint8_t prefix_len, + bool insert_before_renew = true); + + /// @brief Checks if RENEW with invalid IAID is processed correctly. + /// + /// @param type lease type (currently only IA_NA is supported) + /// @param addr address to be renewed + void + testRenewWrongIAID(isc::dhcp::Lease::Type type, + const asiolink::IOAddress& addr); + + /// @brief Checks if client A can renew address used by client B + /// + /// @param type lease type (currently only IA_NA is supported) + /// @param addr address to be renewed + void + testRenewSomeoneElsesLease(isc::dhcp::Lease::Type type, + const asiolink::IOAddress& addr); /// @brief Performs negative RENEW test /// -- 2.47.3