From: Marcin Siodelski Date: Tue, 22 Jul 2014 14:00:46 +0000 (+0200) Subject: [3320] Covered the case where the client is in INIT-REBOOT state. X-Git-Tag: trac3482_base~73^2~2 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=4d522d63ddaf9ba23780841545472fdb24f4afec;p=thirdparty%2Fkea.git [3320] Covered the case where the client is in INIT-REBOOT state. --- diff --git a/src/bin/dhcp4/dhcp4_messages.mes b/src/bin/dhcp4/dhcp4_messages.mes index 131297426f..cd36b2a6f5 100644 --- a/src/bin/dhcp4/dhcp4_messages.mes +++ b/src/bin/dhcp4/dhcp4_messages.mes @@ -147,6 +147,11 @@ point, the setting of the flag instructs the server not to choose a subnet, an action that severely limits further processing; the server will be only able to offer global options - no addresses will be assigned. +% DHCP4_INVALID_ADDRESS_INIT_REBOOT client having client-id %1, hwaddr %2 and being in the INIT-REBOOT state requested invalid address %3 +This debug message is issued when the client being in the INIT-REBOOT state +requested an address which is not assigned to him. The server will respond +to this client with DHCPNAK. + % DHCP4_LEASE_ADVERT lease %1 advertised (client client-id %2, hwaddr %3) This debug message indicates that the server successfully advertised a lease. It is up to the client to choose one server out of othe advertised diff --git a/src/bin/dhcp4/dhcp4_srv.cc b/src/bin/dhcp4/dhcp4_srv.cc index e8f756044d..122de028ff 100644 --- a/src/bin/dhcp4/dhcp4_srv.cc +++ b/src/bin/dhcp4/dhcp4_srv.cc @@ -940,8 +940,14 @@ Dhcpv4Srv::assignLease(const Pkt4Ptr& question, Pkt4Ptr& answer) { if (opt) { client_id = ClientIdPtr(new ClientId(opt->getData())); } + // client-id is not mandatory in DHCPv4 + // Get the server identifier. It will be used to determine the state + // of the client. + OptionCustomPtr opt_serverid = boost::dynamic_pointer_cast< + OptionCustom>(question->getOption(DHO_DHCP_SERVER_IDENTIFIER)); + // Try to get the Requested IP Address option and use the address as a hint // for the allocation engine. If the server doesn't already have a lease // for this client it will try to allocate the one requested. @@ -962,6 +968,25 @@ Dhcpv4Srv::assignLease(const Pkt4Ptr& question, Pkt4Ptr& answer) { // allocation. bool fake_allocation = (question->getType() == DHCPDISCOVER); + // If there is no server id and there is a Requested IP Address option + // the client is in the INIT-REBOOT state in which the server has to + // determine whether the client's notion of the address has to be verified. + if (!fake_allocation && !opt_serverid && opt_requested_address) { + Lease4Ptr lease = LeaseMgrFactory::instance().getLease4(hint); + if (!lease) { + LOG_DEBUG(dhcp4_logger, DBG_DHCP4_DETAIL, + DHCP4_INVALID_ADDRESS_INIT_REBOOT) + .arg(client_id ? client_id->toText():"(no client-id)") + .arg(hwaddr ? hwaddr->toText():"(no hwaddr info)") + .arg(hint.toText()); + + answer->setType(DHCPNAK); + answer->setYiaddr(IOAddress("0.0.0.0")); + return; + } + } + + CalloutHandlePtr callout_handle = getCalloutHandle(question); std::string hostname; @@ -1001,8 +1026,8 @@ Dhcpv4Srv::assignLease(const Pkt4Ptr& question, Pkt4Ptr& answer) { /// @todo pass the actual FQDN data. Lease4Ptr old_lease; Lease4Ptr lease = alloc_engine_->allocateLease4(subnet, client_id, hwaddr, - hint, fqdn_fwd, fqdn_rev, - hostname, + hint, fqdn_fwd, fqdn_rev, + hostname, fake_allocation, callout_handle, old_lease); diff --git a/src/bin/dhcp4/tests/dhcp4_client.cc b/src/bin/dhcp4/tests/dhcp4_client.cc index 2042a2ad23..0d36ba4f55 100644 --- a/src/bin/dhcp4/tests/dhcp4_client.cc +++ b/src/bin/dhcp4/tests/dhcp4_client.cc @@ -70,6 +70,15 @@ Dhcp4Client::Dhcp4Client(boost::shared_ptr& srv, use_relay_(false) { } +void +Dhcp4Client::addRequestedAddress(const asiolink::IOAddress& addr) { + if (context_.query_) { + Option4AddrLstPtr opt(new Option4AddrLst(DHO_DHCP_REQUESTED_ADDRESS, + addr)); + context_.query_->addOption(opt); + } +} + void Dhcp4Client::applyConfiguration() { Pkt4Ptr resp = context_.response_; @@ -140,10 +149,7 @@ Dhcp4Client::doDiscover(const boost::shared_ptr& requested_addr) { // Request options if any. includePRL(); if (requested_addr) { - Option4AddrLstPtr - opt_requested_addr(new Option4AddrLst(DHO_DHCP_REQUESTED_ADDRESS, - IOAddress(*requested_addr))); - context_.query_->addOption(opt_requested_addr); + addRequestedAddress(*requested_addr); } // Send the message to the server. sendMsg(context_.query_); @@ -155,11 +161,10 @@ void Dhcp4Client::doDORA(const boost::shared_ptr& requested_addr) { doDiscover(requested_addr); if (context_.response_ && (context_.response_->getType() == DHCPOFFER)) { - doRequest(requested_addr); + doRequest(); } } - void Dhcp4Client::doInform(const bool set_ciaddr) { context_.query_ = createMsg(DHCPINFORM); @@ -187,7 +192,7 @@ Dhcp4Client::doInform(const bool set_ciaddr) { } void -Dhcp4Client::doRequest(const boost::shared_ptr& requested_addr) { +Dhcp4Client::doRequest() { context_.query_ = createMsg(DHCPREQUEST); // Set ciaddr. @@ -198,18 +203,17 @@ Dhcp4Client::doRequest(const boost::shared_ptr& requested_addr) { } // Requested IP address. - if ((state_ == SELECTING) || (state_ == INIT_REBOOT)) { + if (state_ == SELECTING) { if (context_.response_ && (context_.response_->getType() == DHCPOFFER) && (context_.response_->getYiaddr() != IOAddress("0.0.0.0"))) { - Option4AddrLstPtr - opt_requested_addr(new Option4AddrLst(DHO_DHCP_REQUESTED_ADDRESS, - IOAddress(context_.response_->getYiaddr()))); - context_.query_->addOption(opt_requested_addr); + addRequestedAddress(context_.response_->getYiaddr()); } else { isc_throw(Dhcp4ClientError, "error sending the DHCPREQUEST because" " the received DHCPOFFER message was invalid"); } + } else if (state_ == INIT_REBOOT) { + addRequestedAddress(config_.lease_.addr_); } // Server identifier. diff --git a/src/bin/dhcp4/tests/dhcp4_client.h b/src/bin/dhcp4/tests/dhcp4_client.h index 5d826fff36..0ba2762075 100644 --- a/src/bin/dhcp4/tests/dhcp4_client.h +++ b/src/bin/dhcp4/tests/dhcp4_client.h @@ -122,6 +122,13 @@ public: /// @brief Sends DHCPDISCOVER message to the server and receives response. /// + /// The message being sent to the server includes Parameter Request List + /// option if any options to be requested have been specified using the + /// @c requestOptions or @c requestOption methods. + /// + /// The configuration returned by the server in the DHCPOFFER message is + /// NOT stored in the client configuration: @c config_. + /// /// @param requested_addr A pointer to the IP Address to be sent in the /// Requested IP Address option or NULL if the option should not be /// included. @@ -130,6 +137,9 @@ public: /// @brief Perform 4-way exchange with a server. /// + /// This method calls @c doDiscover and @c doRequest to perform the 4-way + /// exchange with the server. + /// /// @param requested_addr A pointer to the address to be requested using the /// Requested IP Address option. void doDORA(const boost::shared_ptr& @@ -157,11 +167,29 @@ public: /// @brief Sends DHCPREQUEST Message to the server and receives a response. /// - /// @param requested_addr A pointer to the IP Address to be sent in the - /// Requested IP Address option or NULL if the option should not be - /// included. - void doRequest(const boost::shared_ptr& - requested_addr = boost::shared_ptr()); + /// This method simulates sending the DHCPREQUEST message to the server and + /// receiving a response. The DHCPREQUEST message can be used by the client + /// being in various states: + /// - SELECTING - client is trying to obtain a new lease and it has selected + /// the server using the DHCPDISCOVER. + /// - INIT-REBOOT - client cached an address it was previously using and is + /// now trying to verify if this addres is still valid. + /// - RENEW - client's renewal timer has passed and the client is trying to + /// extend the lifetime of the lease. + /// - REBIND - client's rebind timer has passed and the client is trying to + /// extend the lifetime of the lease from any server. + /// + /// Depending on the state that the client is in, different combinations of + /// - ciaddr + /// - Requested IP Address option + /// - server identifier + /// are used (as per RFC2131, section 4.3.2). Therefore, the unit tests + /// must setthe appropriate state of the client prior to calling this + /// method using the @c setState function. + /// + /// When the server returns the DHCPACK the configuration carried in the + /// DHCPACK message is applied and can be obtained from the @c config_. + void doRequest(); /// @brief Generates a hardware address used by the client. /// @@ -268,6 +296,12 @@ public: private: + /// @brief Creates and addds Requested IP Address option to the client's + /// query. + /// + /// @param addr Address to be added in the Requested IP Address option. + void addRequestedAddress(const asiolink::IOAddress& addr); + /// @brief Stores configuration received from the server. /// /// This methods stores the configuration obtained from the DHCP server diff --git a/src/bin/dhcp4/tests/dhcp4_srv_unittest.cc b/src/bin/dhcp4/tests/dhcp4_srv_unittest.cc index f134a694ce..9100596243 100644 --- a/src/bin/dhcp4/tests/dhcp4_srv_unittest.cc +++ b/src/bin/dhcp4/tests/dhcp4_srv_unittest.cc @@ -590,92 +590,6 @@ TEST_F(Dhcpv4SrvTest, DiscoverNoTimers) { checkClientId(offer, clientid); } -// This test verifies that incoming DISCOVER can be handled properly, that an -// OFFER is generated, that the response has an address and that address -// really belongs to the configured pool. -// -// constructed very simple DISCOVER message with: -// - client-id option -// - address set to specific value as hint -// -// expected returned OFFER message: -// - copy of client-id -// - server-id -// - offered address -TEST_F(Dhcpv4SrvTest, DiscoverHint) { - IfaceMgrTestConfig test_config(true); - IfaceMgr::instance().openSockets4(); - - boost::scoped_ptr srv; - ASSERT_NO_THROW(srv.reset(new NakedDhcpv4Srv(0))); - IOAddress hint("192.0.2.107"); - - Pkt4Ptr dis = Pkt4Ptr(new Pkt4(DHCPDISCOVER, 1234)); - dis->setRemoteAddr(IOAddress("192.0.2.1")); - OptionPtr clientid = generateClientId(); - dis->addOption(clientid); - dis->setYiaddr(hint); - dis->setIface("eth1"); - - // Pass it to the server and get an offer - Pkt4Ptr offer = srv->processDiscover(dis); - - // Check if we get response at all - checkResponse(offer, DHCPOFFER, 1234); - - // Check that address was returned from proper range, that its lease - // lifetime is correct, that T1 and T2 are returned properly - checkAddressParams(offer, subnet_, true, true); - - EXPECT_EQ(offer->getYiaddr(), hint); - - // Check identifiers - checkServerId(offer, srv->getServerID()); - checkClientId(offer, clientid); -} - - -// This test verifies that incoming DISCOVER can be handled properly, that an -// OFFER is generated, that the response has an address and that address -// really belongs to the configured pool. -// -// constructed very simple DISCOVER message with: -// - address set to specific value as hint -// -// expected returned OFFER message: -// - copy of client-id -// - server-id -// - offered address -TEST_F(Dhcpv4SrvTest, DiscoverNoClientId) { - IfaceMgrTestConfig test_config(true); - IfaceMgr::instance().openSockets4(); - - boost::scoped_ptr srv; - ASSERT_NO_THROW(srv.reset(new NakedDhcpv4Srv(0))); - IOAddress hint("192.0.2.107"); - - Pkt4Ptr dis = Pkt4Ptr(new Pkt4(DHCPDISCOVER, 1234)); - dis->setRemoteAddr(IOAddress("192.0.2.1")); - dis->setYiaddr(hint); - dis->setHWAddr(generateHWAddr(6)); - dis->setIface("eth1"); - - // Pass it to the server and get an offer - Pkt4Ptr offer = srv->processDiscover(dis); - - // Check if we get response at all - checkResponse(offer, DHCPOFFER, 1234); - - // Check that address was returned from proper range, that its lease - // lifetime is correct, that T1 and T2 are returned properly - checkAddressParams(offer, subnet_, true, true); - - EXPECT_EQ(offer->getYiaddr(), hint); - - // Check identifiers - checkServerId(offer, srv->getServerID()); -} - // This test verifies that incoming DISCOVER can be handled properly, that an // OFFER is generated, that the response has an address and that address // really belongs to the configured pool. @@ -825,58 +739,6 @@ TEST_F(Dhcpv4SrvTest, discoverEchoClientId) { checkClientId(offer, clientid); } -// This test verifies that incoming REQUEST can be handled properly, that an -// ACK is generated, that the response has an address and that address -// really belongs to the configured pool. -// -// constructed a single REQUEST message with: -// - client-id option -// - hwaddr information -// - requested address (that the client received in DISCOVER/OFFER exchange) -// -// expected returned ACK message: -// - copy of client-id -// - server-id -// - assigned address -// -// Test verifies that the lease is actually in the database. -TEST_F(Dhcpv4SrvTest, RequestBasic) { - IfaceMgrTestConfig test_config(true); - IfaceMgr::instance().openSockets4(); - - boost::scoped_ptr srv; - ASSERT_NO_THROW(srv.reset(new NakedDhcpv4Srv(0))); - - IOAddress hint("192.0.2.107"); - Pkt4Ptr req = Pkt4Ptr(new Pkt4(DHCPREQUEST, 1234)); - req->setRemoteAddr(IOAddress("192.0.2.1")); - OptionPtr clientid = generateClientId(); - req->addOption(clientid); - req->setYiaddr(hint); - req->setIface("eth1"); - - // Pass it to the server and get an advertise - Pkt4Ptr ack = srv->processRequest(req); - - // Check if we get response at all - checkResponse(ack, DHCPACK, 1234); - EXPECT_EQ(hint, ack->getYiaddr()); - - // Check that address was returned from proper range, that its lease - // lifetime is correct, that T1 and T2 are returned properly - checkAddressParams(ack, subnet_, true, true); - - // Check identifiers - checkServerId(ack, srv->getServerID()); - checkClientId(ack, clientid); - - // Check that the lease is really in the database - Lease4Ptr l = checkLease(ack, clientid, req->getHWAddr(), hint); - - ASSERT_TRUE(l); - LeaseMgrFactory::instance().deleteLease(l->addr_); -} - // Check that option 58 and 59 are not included if they are not specified. TEST_F(Dhcpv4SrvTest, RequestNoTimers) { IfaceMgrTestConfig test_config(true); diff --git a/src/bin/dhcp4/tests/dora_unittest.cc b/src/bin/dhcp4/tests/dora_unittest.cc index 0d7b5fb22b..fa88dca669 100644 --- a/src/bin/dhcp4/tests/dora_unittest.cc +++ b/src/bin/dhcp4/tests/dora_unittest.cc @@ -140,16 +140,86 @@ public: }; +/// This test verifies that the client in the SELECTING state can get +/// an address when it doesn't request any specific address in the +/// DHCPDISCOVER message. +TEST_F(DORATest, selectingDoNotRequestAddress) { + Dhcp4Client client(Dhcp4Client::SELECTING); + // Configure DHCP server. + configure(DORA_CONFIGS[0], *client.getServer()); + + // Perform 4-way exchange with the server but to not request any + // specific address in the DHCPDISCOVER message. + ASSERT_NO_THROW(client.doDORA()); + + // Make sure that the server responded. + ASSERT_TRUE(client.getContext().response_); + Pkt4Ptr resp = client.getContext().response_; + // Make sure that the server has responded with DHCPACK. + ASSERT_EQ(DHCPACK, static_cast(resp->getType())); + // Response must not be relayed. + EXPECT_FALSE(resp->isRelayed()); + // Make sure that the server id is present. + EXPECT_EQ("10.0.0.1", client.config_.serverid_.toText()); + // Make sure that the client has got the lease with the requested address. + ASSERT_NE(client.config_.lease_.addr_.toText(), "0.0.0.0"); +} + // This test verifies that the client in a SELECTING state can request // a specific address and that this address will be assigned when -// available. -TEST_F(DORATest, selectingRequestAvailableAddress) { +// available. It also tests that if the client requests an address which +// is in use the client will get a different address. +TEST_F(DORATest, selectingRequestAddress) { + Dhcp4Client client(Dhcp4Client::SELECTING); + // Configure DHCP server. + configure(DORA_CONFIGS[0], *client.getServer()); + + // Perform 4-way exchange with the server. + ASSERT_NO_THROW(client.doDORA(boost::shared_ptr< + IOAddress>(new IOAddress("10.0.0.50")))); + + // Make sure that the server responded. + ASSERT_TRUE(client.getContext().response_); + Pkt4Ptr resp = client.getContext().response_; + // Make sure that the server has responded with DHCPACK. + ASSERT_EQ(DHCPACK, static_cast(resp->getType())); + // Response must not be relayed. + EXPECT_FALSE(resp->isRelayed()); + // Make sure that the server id is present. + EXPECT_EQ("10.0.0.1", client.config_.serverid_.toText()); + // Make sure that the client has got the lease with the requested address. + ASSERT_EQ("10.0.0.50", client.config_.lease_.addr_.toText()); + + // Simulate different client requesting the same address. + client.modifyHWAddr(); + ASSERT_NO_THROW(client.doDORA(boost::shared_ptr< + IOAddress>(new IOAddress("10.0.0.50")))); + resp = client.getContext().response_; + // Make sure that the server responded. + ASSERT_TRUE(resp); + // Make sure that the server has responded with DHCPACK. + ASSERT_EQ(DHCPACK, static_cast(resp->getType())); + // Response must not be relayed. + EXPECT_FALSE(resp->isRelayed()); + // Make sure that the server id is present. + EXPECT_EQ("10.0.0.1", client.config_.serverid_.toText()); + // Make sure that the client has got some address. + EXPECT_NE(client.config_.lease_.addr_.toText(), "0.0.0.0"); + // Make sure that the client has got a different address than requested + // as the requested one is already in use. + EXPECT_NE(client.config_.lease_.addr_.toText(), "10.0.0.50"); +} + +// This test verifies that the client will get the address that it has +// been allocated when the client requests a different address. +TEST_F(DORATest, selectingRequestNonMatchingAddress) { Dhcp4Client client(Dhcp4Client::SELECTING); // Configure DHCP server. configure(DORA_CONFIGS[0], *client.getServer()); // Perform 4-way exchange with the server. - ASSERT_NO_THROW(client.doDORA(boost::shared_ptr(new IOAddress("10.0.0.50")))); + ASSERT_NO_THROW(client.doDORA(boost::shared_ptr< + IOAddress>(new IOAddress("10.0.0.50")))); // Make sure that the server responded. ASSERT_TRUE(client.getContext().response_); @@ -161,7 +231,76 @@ TEST_F(DORATest, selectingRequestAvailableAddress) { // Make sure that the server id is present. EXPECT_EQ("10.0.0.1", client.config_.serverid_.toText()); // Make sure that the client has got the lease with the requested address. + ASSERT_EQ("10.0.0.50", client.config_.lease_.addr_.toText()); + + // Let's request a different address. The server should respond with + // the one that the client already has allocated. + ASSERT_NO_THROW(client.doDORA(boost::shared_ptr< + IOAddress>(new IOAddress("10.0.0.80")))); + // Make sure that the server responded. + ASSERT_TRUE(client.getContext().response_); + resp = client.getContext().response_; + // Make sure that the server has responded with DHCPACK. + ASSERT_EQ(DHCPACK, static_cast(resp->getType())); + // Response must not be relayed. + EXPECT_FALSE(resp->isRelayed()); + // Make sure that the server id is present. + EXPECT_EQ("10.0.0.1", client.config_.serverid_.toText()); + // Make sure that the client has got the lease with the address that + // the client has recorded in the lease database. EXPECT_EQ("10.0.0.50", client.config_.lease_.addr_.toText()); } +// Test that the client in the INIT-REBOOT state can request the IP +// address it has and the address is returned. Also, check that if +// if the client requests in valid address the server sends a DHCPNAK. +TEST_F(DORATest, InitRebootRequest) { + Dhcp4Client client(Dhcp4Client::SELECTING); + // Configure DHCP server. + configure(DORA_CONFIGS[0], *client.getServer()); + // Obtain a lease from the server using the 4-way exchange. + ASSERT_NO_THROW(client.doDORA(boost::shared_ptr< + IOAddress>(new IOAddress("10.0.0.50")))); + // Make sure that the server responded. + ASSERT_TRUE(client.getContext().response_); + Pkt4Ptr resp = client.getContext().response_; + // Make sure that the server has responded with DHCPACK. + ASSERT_EQ(DHCPACK, static_cast(resp->getType())); + // Response must not be relayed. + EXPECT_FALSE(resp->isRelayed()); + // Make sure that the server id is present. + EXPECT_EQ("10.0.0.1", client.config_.serverid_.toText()); + // Make sure that the client has got the lease with the requested address. + ASSERT_EQ("10.0.0.50", client.config_.lease_.addr_.toText()); + + // Client has a lease in the database. Let's transition the client + // to the INIT_REBOOT state so as the client can request the cached + // lease using the DHCPREQUEST message. + client.setState(Dhcp4Client::INIT_REBOOT); + ASSERT_NO_THROW(client.doRequest(boost::shared_ptr< + IOAddress>(new IOAddress("10.0.0.50")))); + + // Make sure that the server responded. + ASSERT_TRUE(client.getContext().response_); + resp = client.getContext().response_; + // Make sure that the server has responded with DHCPACK. + ASSERT_EQ(DHCPACK, static_cast(resp->getType())); + // Response must not be relayed. + EXPECT_FALSE(resp->isRelayed()); + // Make sure that the server id is present. + EXPECT_EQ("10.0.0.1", client.config_.serverid_.toText()); + // Make sure that the client has got the lease with the requested address. + ASSERT_EQ("10.0.0.50", client.config_.lease_.addr_.toText()); + + // Try to request a different address than the client has. The server + // should respond with DHCPNAK. + client.config_.lease_.addr_ = IOAddress("10.0.0.30"); + ASSERT_NO_THROW(client.doRequest(boost::shared_ptr< + IOAddress>(new IOAddress("10.0.0.50")))); + // Make sure that the server responded. + ASSERT_TRUE(client.getContext().response_); + resp = client.getContext().response_; + EXPECT_EQ(DHCPNAK, static_cast(resp->getType())); +} + } // end of anonymous namespace