From: Thomas Markwalder Date: Fri, 27 Oct 2023 15:24:34 +0000 (-0400) Subject: [#3084] Server declines leases after ping-check X-Git-Tag: Kea-2.5.4~22 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=fabba7011c07790fd32fa59ca8acb733b9cdc967;p=thirdparty%2Fkea.git [#3084] Server declines leases after ping-check kea-dhcp4 declines leases in the lease store Needs additional UTs src/bin/dhcp4/dhcp4_messages.mes New messages: DHCP4_SERVER_INITIATED_DECLINE_FAILED DHCP4_SERVER_INITIATED_DECLINE src/bin/dhcp4/dhcp4_srv.* Dhcpv4Srv::serverDecline() Dhcpv4Srv::serverDeclineNoThrow() - new functions to render a lease declined after ping-check in-use outcome Dhcpv4Srv::processDhcp4Query() - modified unpark lambda to invoke serverDecline() following lease-offer completion if status is DROP --- diff --git a/src/bin/dhcp4/dhcp4_messages.cc b/src/bin/dhcp4/dhcp4_messages.cc index da47f7221a..9e9cbf45da 100644 --- a/src/bin/dhcp4/dhcp4_messages.cc +++ b/src/bin/dhcp4/dhcp4_messages.cc @@ -162,6 +162,8 @@ extern const isc::log::MessageID DHCP4_RESPONSE_FQDN_DATA = "DHCP4_RESPONSE_FQDN extern const isc::log::MessageID DHCP4_RESPONSE_HOSTNAME_DATA = "DHCP4_RESPONSE_HOSTNAME_DATA"; extern const isc::log::MessageID DHCP4_RESPONSE_HOSTNAME_GENERATE = "DHCP4_RESPONSE_HOSTNAME_GENERATE"; extern const isc::log::MessageID DHCP4_SERVER_FAILED = "DHCP4_SERVER_FAILED"; +extern const isc::log::MessageID DHCP4_SERVER_INITIATED_DECLINE = "DHCP4_SERVER_INITIATED_DECLINE"; +extern const isc::log::MessageID DHCP4_SERVER_INITIATED_DECLINE_FAILED = "DHCP4_SERVER_INITIATED_DECLINE_FAILED"; extern const isc::log::MessageID DHCP4_SHUTDOWN = "DHCP4_SHUTDOWN"; extern const isc::log::MessageID DHCP4_SHUTDOWN_REQUEST = "DHCP4_SHUTDOWN_REQUEST"; extern const isc::log::MessageID DHCP4_SRV_CONSTRUCT_ERROR = "DHCP4_SRV_CONSTRUCT_ERROR"; @@ -340,6 +342,8 @@ const char* values[] = { "DHCP4_RESPONSE_HOSTNAME_DATA", "%1: including Hostname option in the server's response: %2", "DHCP4_RESPONSE_HOSTNAME_GENERATE", "%1: server has generated hostname %2 for the client", "DHCP4_SERVER_FAILED", "server failed: %1", + "DHCP4_SERVER_INITIATED_DECLINE", "Lease for addr %1 has been found to be already be in use. The lease will be unavailable for %3 seconds.", + "DHCP4_SERVER_INITIATED_DECLINE_FAILED", "%1: error on server-initiated decline lease for address %2: %3", "DHCP4_SHUTDOWN", "server shutdown", "DHCP4_SHUTDOWN_REQUEST", "shutdown of server requested", "DHCP4_SRV_CONSTRUCT_ERROR", "error creating Dhcpv4Srv object, reason: %1", diff --git a/src/bin/dhcp4/dhcp4_messages.h b/src/bin/dhcp4/dhcp4_messages.h index f60dbbc68c..ec00771297 100644 --- a/src/bin/dhcp4/dhcp4_messages.h +++ b/src/bin/dhcp4/dhcp4_messages.h @@ -163,6 +163,8 @@ extern const isc::log::MessageID DHCP4_RESPONSE_FQDN_DATA; extern const isc::log::MessageID DHCP4_RESPONSE_HOSTNAME_DATA; extern const isc::log::MessageID DHCP4_RESPONSE_HOSTNAME_GENERATE; extern const isc::log::MessageID DHCP4_SERVER_FAILED; +extern const isc::log::MessageID DHCP4_SERVER_INITIATED_DECLINE; +extern const isc::log::MessageID DHCP4_SERVER_INITIATED_DECLINE_FAILED; extern const isc::log::MessageID DHCP4_SHUTDOWN; extern const isc::log::MessageID DHCP4_SHUTDOWN_REQUEST; extern const isc::log::MessageID DHCP4_SRV_CONSTRUCT_ERROR; diff --git a/src/bin/dhcp4/dhcp4_messages.mes b/src/bin/dhcp4/dhcp4_messages.mes index 2e1b3a92a1..3aee9656f8 100644 --- a/src/bin/dhcp4/dhcp4_messages.mes +++ b/src/bin/dhcp4/dhcp4_messages.mes @@ -1056,3 +1056,20 @@ option (if present). % DHCP6_DHCP4O6_PACKET_RECEIVED received DHCPv4o6 packet from DHCPv6 server (type %1) for %2 port %3 on interface %4 This debug message is printed when the server is receiving a DHCPv4o6 from the DHCPv6 server over inter-process communication. + +% DHCP4_SERVER_INITIATED_DECLINE_FAILED %1: error on server-initiated decline lease for address %2: %3 +This error message indicates that the software failed to decline a +lease from the lease database due to an error during a database +operation. The first argument includes the client and the transaction +identification information. The second argument holds the IPv4 address +which decline was attempted. The last one contains the reason for +failure. + +% DHCP4_SERVER_INITIATED_DECLINE Lease for addr %1 has been found to be already be in use. The lease will be unavailable for %3 seconds. +This informational message is printed when the server has detected via +ICMP ECHO (i.e. ping check) or other means that a lease which should be +free to offer is actually in use. This message may indicate a misconfiguration +in a network or more likely a device that is using an address that it is not +supposed to use. The server will fully recover from this situation, but if +the underlying problem of a misconfigured or rogue device is not solved, this +address may be declined again in the future. diff --git a/src/bin/dhcp4/dhcp4_srv.cc b/src/bin/dhcp4/dhcp4_srv.cc index e2a4cc1a59..6898d60c9c 100644 --- a/src/bin/dhcp4/dhcp4_srv.cc +++ b/src/bin/dhcp4/dhcp4_srv.cc @@ -1511,7 +1511,33 @@ Dhcpv4Srv::processDhcp4Query(Pkt4Ptr& query, Pkt4Ptr& rsp, // executed when the hook library unparks the packet. HooksManager::park( hook_label, query, - [this, callout_handle, query, rsp, callout_handle_state]() mutable { + [this, callout_handle, query, rsp, callout_handle_state, hook_idx, ctx]() mutable { + if (hook_idx == Hooks.hook_index_lease4_offer_) { + auto status = callout_handle->getStatus(); + if (status == CalloutHandle::NEXT_STEP_DROP) { + Lease4Ptr lease = ctx->new_lease_; + bool lease_exists = (ctx->offer_lft_ > 0); + if (MultiThreadingMgr::instance().getMode()) { + typedef function CallBack; + // We need to pass in the lease and flag as the callback handle state + // gets reset prior to the invocation of the on_completion_ callback. + boost::shared_ptr call_back = boost::make_shared( + std::bind(&Dhcpv4Srv::serverDeclineNoThrow, this, + callout_handle, query, lease, lease_exists)); + callout_handle_state->on_completion_ = [call_back]() { + MultiThreadingMgr::instance().getThreadPool().add(call_back); + }; + } else { + serverDecline(callout_handle, query, lease, lease_exists); + } + + // Discard the response and return. + rsp.reset(); + return; + } + } + + // Send the response to the client. if (MultiThreadingMgr::instance().getMode()) { typedef function CallBack; boost::shared_ptr call_back = boost::make_shared( @@ -3913,6 +3939,125 @@ Dhcpv4Srv::declineLease(const Lease4Ptr& lease, const Pkt4Ptr& decline, .arg(decline->getLabel()).arg(lease->valid_lft_); } +void +Dhcpv4Srv::serverDecline(hooks::CalloutHandlePtr& callout_handle, Pkt4Ptr& query, + Lease4Ptr lease, bool lease_exists) { + // We need to disassociate the lease from the client. Once we move a lease + // to declined state, it is no longer associated with the client in any + // way. + lease->decline(CfgMgr::instance().getCurrentCfg()->getDeclinePeriod()); + + // Add or update the lease in the database. + try { + if (lease_exists) { + LeaseMgrFactory::instance().updateLease4(lease); + } else { + LeaseMgrFactory::instance().addLease(lease); + } + } catch (const Exception& ex) { + // Update failed. + LOG_ERROR(lease4_logger, DHCP4_SERVER_INITIATED_DECLINE_FAILED) + .arg(query->getLabel()) + .arg(lease->addr_.toText()) + .arg(ex.what()); + return; + } + + // Bump up the statistics. If the lease does not exist (i.e. offer-lifetime == 0) we + // need to increment assigned address stats, otherwise the accounting wll be off. + // This saves us from having to determine later, when declined leases are reclaimed, + // whether or not we need to decrement assigned stats. In other words, this keeps + // a declined lease always counted also as an assigned lease, regardless of how + // it was declined, until it is reclaimed at which point both groups of stats + // are decremented. + + // Per subnet declined addresses counter. + StatsMgr::instance().addValue( + StatsMgr::generateName("subnet", lease->subnet_id_, "declined-addresses"), + static_cast(1)); + + if (!lease_exists) { + StatsMgr::instance().addValue( + StatsMgr::generateName("subnet", lease->subnet_id_, "assigned-addresses"), + static_cast(1)); + } + + const auto& subnet = CfgMgr::instance().getCurrentCfg()->getCfgSubnets4()->getBySubnetId(lease->subnet_id_); + if (subnet) { + const auto& pool = subnet->getPool(Lease::TYPE_V4, lease->addr_, false); + if (pool) { + StatsMgr::instance().addValue( + StatsMgr::generateName("subnet", subnet->getID(), + StatsMgr::generateName("pool", pool->getID(), "declined-addresses")), + static_cast(1)); + if (!lease_exists) { + StatsMgr::instance().addValue( + StatsMgr::generateName("subnet", subnet->getID(), + StatsMgr::generateName("pool", pool->getID(), "assigned-addresses")), + static_cast(1)); + } + } + } + + // Global declined addresses counter. + StatsMgr::instance().addValue("declined-addresses", static_cast(1)); + if (!lease_exists) { + StatsMgr::instance().addValue("assigned-addresses", static_cast(1)); + } + + + /* (comment it out so picky tools don't flag this as dead code + // + /// @todo #3110, HA will implement a handler for this hook point to + /// propagate an update of the lease to peers. + // + // Let's check if there are hooks installed for server decline hook point. + // If they are, let's pass the lease and client's packet. + if (HooksManager::calloutsPresent(Hooks.hook_index_lease4_server_decline)) { + CalloutHandlePtr callout_handle = getCalloutHandle(decline); + + // Use the RAII wrapper to make sure that the callout handle state is + // reset when this object goes out of scope. All hook points must do + // it to prevent possible circular dependency between the callout + // handle and its arguments. + ScopedCalloutHandleState callout_handle_state(callout_handle); + + // Pass the original packet + callout_handle->setArgument("query4", query); + + // Pass the lease to be updated + callout_handle->setArgument("lease4", lease); + + // Call callouts + HooksManager::callCallouts(Hooks.hook_index_lease4_server_decline_, + *callout_handle); + + // Check if callouts decided to skip the next processing step. + // If any of them did, we will drop the packet. + if ((callout_handle->getStatus() == CalloutHandle::NEXT_STEP_SKIP) || + (callout_handle->getStatus() == CalloutHandle::NEXT_STEP_DROP)) { + LOG_DEBUG(hooks_logger, DBGLVL_PKT_HANDLING, DHCP4_HOOK_SERVER_DECLINE_SKIP) + .arg(query->getLabel()).arg(lease->addr_.toText()); + return; + } + } + */ + + LOG_INFO(lease4_logger, DHCP4_SERVER_INITIATED_DECLINE).arg(lease->addr_.toText()) + .arg(query->getLabel()).arg(lease->valid_lft_); +} + +void +Dhcpv4Srv::serverDeclineNoThrow(hooks::CalloutHandlePtr& callout_handle, Pkt4Ptr& query, + Lease4Ptr lease, bool lease_exists) { + try { + serverDecline(callout_handle, query, lease, lease_exists); + } catch (...) { + LOG_ERROR(packet4_logger, DHCP4_PACKET_PROCESS_EXCEPTION); + } +} + + Pkt4Ptr Dhcpv4Srv::processInform(Pkt4Ptr& inform, AllocEngine::ClientContext4Ptr& context) { // server-id is supposed to be forbidden (as is requested address) diff --git a/src/bin/dhcp4/dhcp4_srv.h b/src/bin/dhcp4/dhcp4_srv.h index ba7152bf9c..72ead22724 100644 --- a/src/bin/dhcp4/dhcp4_srv.h +++ b/src/bin/dhcp4/dhcp4_srv.h @@ -1,4 +1,4 @@ -// Copyright (C) 2011-2022 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2011-2023 Internet Systems Consortium, Inc. ("ISC") // // This Source Code Form is subject to the terms of the Mozilla Public // License, v. 2.0. If a copy of the MPL was not distributed with this @@ -811,6 +811,36 @@ protected: test_send_responses_to_source_ = value; } + /// @brief Renders a lease declined after the server has detected, via ping-check + /// or other means, that its address is already in-use. + /// + /// This function is invoked during the unpark callback for the lease4-offer + /// hook point, if a hook callout has set the handle status to NEXT_STEP_DROP. + /// It will create/update the lease to DECLINED state in the lease database, + /// update the appropriate stats, and @todo implement a new hook point, + /// lease4-server-declined-lease (name subject to change). + /// + /// @param callout_handle - current callout handle + /// @param query - DHCPDISCOVER which instigated the declination. + /// @param lease - lease to decline (i.e lease that would have offered) + /// @param lease_exists - true if the lease already exists in the database + /// (as is the case when offer-lifetime is > 0) + void serverDecline(hooks::CalloutHandlePtr& callout_handle, Pkt4Ptr& query, + Lease4Ptr lease, bool lease_exists); + + /// @brief Exception safe wrapper around serverDecline() + /// + /// In MT mode this wrapper is used to safely invoke serverDecline() as a + /// DHCP worker thread task. + /// + /// @param callout_handle - current callout handle + /// @param query - DHCPDISCOVER which instigated the declination. + /// @param lease - lease to decline (i.e lease that would have offered) + /// @param lease_exists - true if the lease already exists in the database + /// (as is the case when offer-lifetime is > 0) + void serverDeclineNoThrow(hooks::CalloutHandlePtr& callout_handle, Pkt4Ptr& query, + Lease4Ptr lease, bool lease_exists); + public: /// @brief this is a prefix added to the content of vendor-class option