diff --git a/include/upnp/mapping.h b/include/upnp/mapping.h index b9c2078eac1a48f0fe34fe69672f3c0f45caba7c..f4398b742c14e34bdb18165b9e5acbb5942cfe94 100644 --- a/include/upnp/mapping.h +++ b/include/upnp/mapping.h @@ -99,9 +99,8 @@ public: bool getAutoUpdate() const; key_t getMapKey() const; static PortType getTypeFromMapKey(key_t key); -#if HAVE_LIBNATPMP sys_clock::time_point getRenewalTime() const; -#endif + sys_clock::time_point getExpiryTime() const; private: NotifyCallback getNotifyCallback() const; @@ -112,10 +111,8 @@ private: void setIgd(const std::shared_ptr<IGD>& igd); void setAvailable(bool val); void setState(const MappingState& state); - void updateDescription(); -#if HAVE_LIBNATPMP void setRenewalTime(sys_clock::time_point time); -#endif + void setExpiryTime(sys_clock::time_point time); mutable std::mutex mutex_; PortType type_ {PortType::UDP}; @@ -129,12 +126,11 @@ private: // Track the state of the mapping MappingState state_; NotifyCallback notifyCb_; - // If true, a new mapping will be requested on behave of the mapping + // If true, a new mapping will be requested on behalf of the mapping // owner when the mapping state changes from "OPEN" to "FAILED". bool autoUpdate_; -#if HAVE_LIBNATPMP sys_clock::time_point renewalTime_; -#endif + sys_clock::time_point expiryTime_; }; struct MappingInfo diff --git a/include/upnp/upnp_context.h b/include/upnp/upnp_context.h index 095886472acc579509b358a5f1421b9f424e263f..fca38a530b7df0fc1795aade85db5cb6469186be 100644 --- a/include/upnp/upnp_context.h +++ b/include/upnp/upnp_context.h @@ -16,15 +16,6 @@ */ #pragma once -/*#include "upnp_protocol.h" -#if HAVE_LIBNATPMP -#include "protocol/natpmp/nat_pmp.h" -#endif -#if HAVE_LIBUPNP -#include "protocol/pupnp/pupnp.h" -#endif -#include "igd.h"*/ - #include "../ip_utils.h" #include "mapping.h" @@ -32,6 +23,7 @@ #include <opendht/rng.h> #include <opendht/logger.h> #include <asio/steady_timer.hpp> +#include <asio/system_timer.hpp> #include <set> #include <map> @@ -68,8 +60,8 @@ struct IGDInfo enum class UpnpIgdEvent { ADDED, REMOVED, INVALID_STATE }; // Interface used to report mapping event from the protocol implementations. -// This interface is meant to be implemented only by UPnPConext class. Sincce -// this class is a singleton, it's assumed that it out-lives the protocol +// This interface is meant to be implemented only by UPnPContext class. Since +// this class is a singleton, it's assumed that it outlives the protocol // implementations. In other words, the observer is always assumed to point to a // valid instance. class UpnpMappingObserver @@ -81,34 +73,12 @@ public: virtual void onIgdUpdated(const std::shared_ptr<IGD>& igd, UpnpIgdEvent event) = 0; virtual void onMappingAdded(const std::shared_ptr<IGD>& igd, const Mapping& map) = 0; virtual void onMappingRequestFailed(const Mapping& map) = 0; -#if HAVE_LIBNATPMP virtual void onMappingRenewed(const std::shared_ptr<IGD>& igd, const Mapping& map) = 0; -#endif virtual void onMappingRemoved(const std::shared_ptr<IGD>& igd, const Mapping& map) = 0; }; class UPnPContext : public UpnpMappingObserver { -private: - struct MappingStatus - { - int openCount_ {0}; - int readyCount_ {0}; - int pendingCount_ {0}; - int inProgressCount_ {0}; - int failedCount_ {0}; - - void reset() - { - openCount_ = 0; - readyCount_ = 0; - pendingCount_ = 0; - inProgressCount_ = 0; - failedCount_ = 0; - }; - int sum() { return openCount_ + pendingCount_ + inProgressCount_ + failedCount_; } - }; - public: UPnPContext(const std::shared_ptr<asio::io_context>& ctx, const std::shared_ptr<dht::log::Logger>& logger); ~UPnPContext(); @@ -127,13 +97,14 @@ public: // Get external Ip of a chosen IGD. IpAddr getExternalIP() const; - // Inform the UPnP context that the network status has changed. This clears the list of known + // Inform the UPnP context that the network status has changed. void connectivityChanged(); // Returns a shared pointer of the mapping. Mapping::sharedPtr_t reserveMapping(Mapping& requestedMap); - // Release an used mapping (make it available for future use). + // Release a used mapping (make it available for future use). + // TODO: The current implementation doesn't seem to do the "make it available for future use" part... fix this. void releaseMapping(const Mapping& map); // Register a controller @@ -142,7 +113,7 @@ public: void unregisterController(void* controller); // Generate random port numbers - static uint16_t generateRandomPort(PortType type, bool mustBeEven = false); + static uint16_t generateRandomPort(PortType type); // Return information about the UPnPContext's valid IGDs, including the list // of all existing port mappings (for IGDs which support a protocol that allows @@ -154,6 +125,14 @@ public: ctx->dispatch(std::move(f)); } + void restart() + { + ctx->dispatch([this]{ + stopUpnp(); + startUpnp(); + }); + } + private: // Initialization void init(); @@ -176,11 +155,15 @@ private: void shutdown(std::condition_variable& cv); - // Create and register a new mapping. + // Add a new mapping to the local list and + // send a request to the IGD to create it. Mapping::sharedPtr_t registerMapping(Mapping& map); - // Removes the mapping from the list. - void unregisterMapping(const Mapping::sharedPtr_t& map); + // Remove the given mapping from the local list. + // + // If the mapping has auto-update enabled, then a new mapping of the same + // type will be reserved unless ignoreAutoUpdate is true. + void unregisterMapping(const Mapping::sharedPtr_t& map, bool ignoreAutoUpdate = false); // Perform the request on the provided IGD. void requestMapping(const Mapping::sharedPtr_t& map); @@ -188,9 +171,6 @@ private: // Request a mapping remove from the IGD. void requestRemoveMapping(const Mapping::sharedPtr_t& map); - // Remove all mappings of the given type. - void deleteAllMappings(PortType type); - // Update the state and notify the listener void updateMappingState(const Mapping::sharedPtr_t& map, MappingState newState, @@ -199,44 +179,35 @@ private: // Provision ports. uint16_t getAvailablePortNumber(PortType type); - // Update preferred IGD - void updatePreferredIgd(); + // If the current IGD is still valid, do nothing. + // If not, then replace it with a valid one (if possible) or set it to null. + void updateCurrentIgd(); - // Get preferred IGD - std::shared_ptr<IGD> getPreferredIgd() const; + // Get the current IGD + std::shared_ptr<IGD> getCurrentIgd() const; - // Check and prune the mapping list. Called periodically. - void updateMappingList(bool async); + // Send a renewal request to the IGD for each mapping which is past its renewal time. + void renewMappings(); - // Provision (pre-allocate) the requested number of mappings. - bool provisionNewMappings(PortType type, int portCount); + // Set a timer so that renewMappings is called when needed + void scheduleMappingsRenewal(); + void _scheduleMappingsRenewal(); - // Close unused mappings. - bool deleteUnneededMappings(PortType type, int portCount); + // Add or remove mappings to maintain the number of available mappings + // within the limits set by minAvailableMappings_ and maxAvailableMappings_. + void enforceAvailableMappingsLimits(); - /** - * Prune the mapping list.To avoid competing with allocation - * requests, the pruning is performed only if there are no - * requests in progress. - */ - void pruneMappingList(); + // Provision (pre-allocate) the requested number of mappings. + void provisionNewMappings(PortType type, int portCount); - /** - * Check if there are allocated mappings from previous instances, - * and try to close them. - * Only done for UPNP protocol. NAT-PMP allocations will expire - * anyway if not renewed. - */ - void pruneUnMatchedMappings(const std::shared_ptr<IGD>& igd, - const std::map<Mapping::key_t, Mapping>& remoteMapList); + // Close unused mappings. + void deleteUnneededMappings(PortType type, int portCount); - /** - * Check the local mapping list against the list returned by the - * IGD and remove all mappings which do not have a match. - * Only done for UPNP protocol. - */ - void pruneUnTrackedMappings(const std::shared_ptr<IGD>& igd, - const std::map<Mapping::key_t, Mapping>& remoteMapList); + // Get information from the current IGD about the mappings it currently has + // and update the local list accordingly. (Only called if the current IGD + // uses the UPnP protocol -- NAT-PMP doesn't support doing this.) + void syncLocalMappingListWithIgd(); + void _syncLocalMappingListWithIgd(); void pruneMappingsWithInvalidIgds(const std::shared_ptr<IGD>& igd); @@ -252,19 +223,8 @@ private: // Get the mapping from the key. Mapping::sharedPtr_t getMappingWithKey(Mapping::key_t key); - // Get the number of mappings per state. - void getMappingStatus(PortType type, MappingStatus& status); - void getMappingStatus(MappingStatus& status); - -#if HAVE_LIBNATPMP - void renewAllocations(); -#endif - // Process requests with pending status. - void processPendingRequests(const std::shared_ptr<IGD>& igd); - - // Process mapping with auto-update flag enabled. - void processMappingWithAutoUpdate(); + void processPendingRequests(); // Implementation of UpnpMappingObserver interface. @@ -273,12 +233,12 @@ private: // Callback used to report add request status. void onMappingAdded(const std::shared_ptr<IGD>& igd, const Mapping& map) override; // Callback invoked when a request fails. Reported on failures for both - // new requests and renewal requests (if supported by the the protocol). + // new requests and renewal requests. void onMappingRequestFailed(const Mapping& map) override; -#if HAVE_LIBNATPMP + // Callback used to report renew request status. void onMappingRenewed(const std::shared_ptr<IGD>& igd, const Mapping& map) override; -#endif + // Callback used to report remove request status. void onMappingRemoved(const std::shared_ptr<IGD>& igd, const Mapping& map) override; @@ -298,10 +258,7 @@ private: // The known public address. The external addresses returned by // the IGDs will be checked against this address. IpAddr knownPublicAddress_ {}; - - // Set of registered controllers - std::mutex mutable controllerMutex_; - std::set<void*> controllerList_; + std::mutex publicAddressMutex_; // Map of available protocols. std::map<NatProtocolType, std::shared_ptr<UPnPProtocol>> protocolList_; @@ -309,25 +266,40 @@ private: // Port ranges for TCP and UDP (in that order). std::map<PortType, std::pair<uint16_t, uint16_t>> portRange_ {}; - // Min open ports limit - int minOpenPortLimit_[2] {4, 8}; - // Max open ports limit - int maxOpenPortLimit_[2] {8, 12}; + // Minimum and maximum limits on the number of available + // mappings to keep in the list at any given time + static constexpr unsigned minAvailableMappings_[2] {4, 8}; + static constexpr unsigned maxAvailableMappings_[2] {8, 12}; + unsigned getMinAvailableMappings(PortType type) { + unsigned index = (type == PortType::TCP) ? 0 : 1; + return minAvailableMappings_[index]; + } + unsigned getMaxAvailableMappings(PortType type) { + unsigned index = (type == PortType::TCP) ? 0 : 1; + return maxAvailableMappings_[index]; + } std::shared_ptr<asio::io_context> ctx; std::shared_ptr<dht::log::Logger> logger_; - asio::steady_timer mappingListUpdateTimer_; asio::steady_timer connectivityChangedTimer_; - - // Current preferred IGD. Can be null if there is no valid IGD. - std::shared_ptr<IGD> preferredIgd_; - - // This mutex must lock only these two members. All other - // members must be accessed only from the UPNP context thread. + asio::system_timer mappingRenewalTimer_; + asio::steady_timer renewalSchedulingTimer_; + asio::steady_timer syncTimer_; + std::mutex syncMutex_; + bool syncRequested_ {false}; + + // This mutex must lock only the members below. All other + // members must be accessed only from the UPnP context thread. std::mutex mutable mappingMutex_; + // List of mappings. std::map<Mapping::key_t, Mapping::sharedPtr_t> mappingList_[2] {}; - std::set<std::shared_ptr<IGD>> validIgdList_ {}; + + // Current IGD. Can be null if there is no valid IGD. + std::shared_ptr<IGD> currentIgd_; + + // Set of registered controllers + std::set<void*> controllerList_; // Shutdown synchronization bool shutdownComplete_ {false}; diff --git a/src/upnp/protocol/mapping.cpp b/src/upnp/protocol/mapping.cpp index 846553f4d0be3d048e118f9b575032a21955ef0e..fa9b6c73a2d6207158e0f910d58bdb20ca05a24b 100644 --- a/src/upnp/protocol/mapping.cpp +++ b/src/upnp/protocol/mapping.cpp @@ -33,9 +33,7 @@ Mapping::Mapping(PortType type, uint16_t portExternal, uint16_t portInternal, bo , state_(MappingState::PENDING) , notifyCb_(nullptr) , autoUpdate_(false) -#if HAVE_LIBNATPMP - , renewalTime_(sys_clock::now()) -#endif + , renewalTime_(sys_clock::time_point::max()) {} Mapping::Mapping(const Mapping& other) @@ -51,9 +49,8 @@ Mapping::Mapping(const Mapping& other) state_ = other.state_; notifyCb_ = other.notifyCb_; autoUpdate_ = other.autoUpdate_; -#if HAVE_LIBNATPMP renewalTime_ = other.renewalTime_; -#endif + expiryTime_ = other.expiryTime_; } void @@ -326,7 +323,6 @@ Mapping::getAutoUpdate() const return autoUpdate_; } -#if HAVE_LIBNATPMP sys_clock::time_point Mapping::getRenewalTime() const { @@ -340,7 +336,20 @@ Mapping::setRenewalTime(sys_clock::time_point time) std::lock_guard lock(mutex_); renewalTime_ = time; } -#endif + +sys_clock::time_point +Mapping::getExpiryTime() const +{ + std::lock_guard lock(mutex_); + return expiryTime_; +} + +void +Mapping::setExpiryTime(sys_clock::time_point time) +{ + std::lock_guard lock(mutex_); + expiryTime_ = time; +} } // namespace upnp } // namespace dhtnet diff --git a/src/upnp/protocol/natpmp/nat_pmp.cpp b/src/upnp/protocol/natpmp/nat_pmp.cpp index 9d306bf14cdab890ce1e094d84694e6ed21def67..3c17e09e07a22f0c45cd8b536a88c13a9fa92de0 100644 --- a/src/upnp/protocol/natpmp/nat_pmp.cpp +++ b/src/upnp/protocol/natpmp/nat_pmp.cpp @@ -262,16 +262,6 @@ NatPmp::incrementErrorsCounter(const std::shared_ptr<IGD>& igdIn) void NatPmp::requestMappingAdd(const Mapping& mapping) { - // Process on nat-pmp thread. - /*if (not isValidThread()) { - ioContext->post([w = weak(), mapping] { - if (auto pmpThis = w.lock()) { - pmpThis->requestMappingAdd(mapping); - } - }); - return; - }*/ - Mapping map(mapping); assert(map.getIgd()); auto err = addPortMapping(map); @@ -300,22 +290,12 @@ NatPmp::requestMappingAdd(const Mapping& mapping) void NatPmp::requestMappingRenew(const Mapping& mapping) { - // Process on nat-pmp thread. - /*if (not isValidThread()) { - ioContext->post([w = weak(), mapping] { - if (auto pmpThis = w.lock()) { - pmpThis->requestMappingRenew(mapping); - } - }); - return; - }*/ - Mapping map(mapping); auto err = addPortMapping(map); if (err < 0) { if (logger_) logger_->warn("NAT-PMP: Renewal request for mapping {} on {} failed with error {:d}: {}", - map.toString().c_str(), - igd_->toString().c_str(), + map.toString(), + igd_->toString(), err, getNatPmpErrorStr(err)); // Notify the listener. @@ -327,8 +307,8 @@ NatPmp::requestMappingRenew(const Mapping& mapping) } } else { if (logger_) logger_->debug("NAT-PMP: Renewal request for mapping {} on {} succeeded", - map.toString().c_str(), - igd_->toString().c_str()); + map.toString(), + igd_->toString()); // Notify the listener. processMappingRenewed(map); } @@ -376,10 +356,8 @@ NatPmp::readResponse(natpmp_t& handle, natpmpresp_t& response) } int -NatPmp::sendMappingRequest(const Mapping& mapping, uint32_t& lifetime) +NatPmp::sendMappingRequest(Mapping& mapping, uint32_t& lifetime) { - //CHECK_VALID_THREAD(); - int err = sendnewportmappingrequest(&natpmpHdl_, mapping.getType() == PortType::UDP ? NATPMP_PROTOCOL_UDP : NATPMP_PROTOCOL_TCP, @@ -415,9 +393,24 @@ NatPmp::sendMappingRequest(const Mapping& mapping, uint32_t& lifetime) continue; } - lifetime = response.pnu.newportmapping.lifetime; - // Done. - break; + uint16_t newExternalPort = response.pnu.newportmapping.mappedpublicport; + uint32_t newLifetime = response.pnu.newportmapping.lifetime; + if (lifetime > 0) { + // We requested the creation/renewal of a mapping and didn't get an error, so at this point + // newExternalPort and newLifetime should both be nonzero. However, that's not always the case + // in practice (presumably because some routers don't implement NAT-PMP correctly). + if (newExternalPort == 0 || newLifetime == 0) { + if (logger_) logger_->warn("NAT-PMP: mapping request returned without error but the response" + " contains invalid data (external port: {}, lifetime: {})", + newExternalPort, + newLifetime); + err = NATPMP_ERR_INVALIDARGS; + } else { + lifetime = newLifetime; + mapping.setExternalPort(newExternalPort); + } + } + break; } return err; @@ -446,7 +439,7 @@ NatPmp::addPortMapping(Mapping& mapping) } // Set the renewal time and update. - mapping.setRenewalTime(sys_clock::now() + std::chrono::seconds(lifetime * 4 / 5)); + mapping.setRenewalTime(sys_clock::now() + std::chrono::seconds(lifetime / 2)); mapping.setState(MappingState::OPEN); return 0; @@ -495,8 +488,6 @@ NatPmp::removePortMapping(Mapping& mapping) void NatPmp::getIgdPublicAddress() { - //CHECK_VALID_THREAD(); - // Set the public address for this IGD if it does not // have one already. if (igd_->getPublicIp()) { @@ -558,8 +549,6 @@ NatPmp::getIgdPublicAddress() void NatPmp::removeAllMappings() { - //CHECK_VALID_THREAD(); - if (logger_) logger_->warn("NAT-PMP: Send request to close all existing mappings to IGD {}", igd_->toString().c_str()); diff --git a/src/upnp/protocol/natpmp/nat_pmp.h b/src/upnp/protocol/natpmp/nat_pmp.h index fbc2fc0bebe7d00305c3257d7b3589beb307776f..510253301c776d064be655a67dab926e21a7ee8d 100644 --- a/src/upnp/protocol/natpmp/nat_pmp.h +++ b/src/upnp/protocol/natpmp/nat_pmp.h @@ -36,7 +36,7 @@ namespace dhtnet { namespace upnp { // Requested lifetime in seconds. The actual lifetime might be different. -constexpr static unsigned int MAPPING_ALLOCATION_LIFETIME {60 * 60}; +constexpr static unsigned int MAPPING_ALLOCATION_LIFETIME {7200}; // Max number of IGD search attempts before failure. constexpr static unsigned int MAX_RESTART_SEARCH_RETRIES {3}; // Time-out between two successive read response. @@ -101,7 +101,7 @@ private: void getIgdPublicAddress(); void removeAllMappings(); int readResponse(natpmp_t& handle, natpmpresp_t& response); - int sendMappingRequest(const Mapping& mapping, uint32_t& lifetime); + int sendMappingRequest(Mapping& mapping, uint32_t& lifetime); // Adds a port mapping. int addPortMapping(Mapping& mapping); diff --git a/src/upnp/protocol/pupnp/pupnp.cpp b/src/upnp/protocol/pupnp/pupnp.cpp index dbb05f1fe0ccdf8a694b2c621181c54d6e765bdc..1df82191294120864813e75f846c7764635196e0 100644 --- a/src/upnp/protocol/pupnp/pupnp.cpp +++ b/src/upnp/protocol/pupnp/pupnp.cpp @@ -442,7 +442,7 @@ PUPnP::validateIgd(const std::string& location, IXML_Document* doc_container_ptr " Service ID : {}\n" " Base URL : {}\n" " Location URL : {}\n" - " control URL : {}\n" + " Control URL : {}\n" " Event URL : {}", igd_candidate->getUID(), igd_candidate->getFriendlyName(), @@ -556,6 +556,9 @@ PUPnP::requestMappingAdd(const Mapping& mapping) return; Mapping mapRes(mapping); if (upnpThis->actionAddPortMapping(mapRes)) { + auto now = sys_clock::now(); + mapRes.setRenewalTime(now + std::chrono::seconds(MAPPING_LEASE_DURATION / 2)); + mapRes.setExpiryTime(now + std::chrono::seconds(MAPPING_LEASE_DURATION)); mapRes.setState(MappingState::OPEN); mapRes.setInternalAddress(upnpThis->getHostAddress().toString()); upnpThis->processAddMapAction(mapRes); @@ -568,6 +571,38 @@ PUPnP::requestMappingAdd(const Mapping& mapping) }); } +void +PUPnP::requestMappingRenew(const Mapping& mapping) +{ + ioContext->post([w = weak(), mapping] { + if (auto upnpThis = w.lock()) { + if (not upnpThis->isRunning()) + return; + Mapping mapRes(mapping); + if (upnpThis->actionAddPortMapping(mapRes)) { + if (upnpThis->logger_) + upnpThis->logger_->debug("PUPnP: Renewal request for mapping {} on {} succeeded", + mapRes.toString(), + mapRes.getIgd()->toString()); + auto now = sys_clock::now(); + mapRes.setRenewalTime(now + std::chrono::seconds(MAPPING_LEASE_DURATION / 2)); + mapRes.setExpiryTime(now + std::chrono::seconds(MAPPING_LEASE_DURATION)); + mapRes.setState(MappingState::OPEN); + mapRes.setInternalAddress(upnpThis->getHostAddress().toString()); + upnpThis->processMappingRenewed(mapRes); + } else { + if (upnpThis->logger_) + upnpThis->logger_->debug("PUPnP: Renewal request for mapping {} on {} failed", + mapRes.toString(), + mapRes.getIgd()->toString()); + upnpThis->incrementErrorsCounter(mapRes.getIgd()); + mapRes.setState(MappingState::FAILED); + upnpThis->processRequestMappingFailure(mapRes); + } + } + }); +} + void PUPnP::requestMappingRemove(const Mapping& mapping) { @@ -624,6 +659,20 @@ PUPnP::processAddMapAction(const Mapping& map) }); } +void +PUPnP::processMappingRenewed(const Mapping& map) +{ + if (observer_ == nullptr) + return; + + ioContext->post([w = weak(), map] { + if (auto upnpThis = w.lock()) { + if (upnpThis->observer_) + upnpThis->observer_->onMappingRenewed(map.getIgd(), std::move(map)); + } + }); +} + void PUPnP::processRequestMappingFailure(const Mapping& map) { @@ -632,8 +681,6 @@ PUPnP::processRequestMappingFailure(const Mapping& map) ioContext->post([w = weak(), map] { if (auto upnpThis = w.lock()) { - if (upnpThis->logger_) upnpThis->logger_->debug("PUPnP: Closed mapping {}", map.toString()); - // JAMI_DBG("PUPnP: Failed to request mapping %s", map.toString().c_str()); if (upnpThis->observer_) upnpThis->observer_->onMappingRequestFailed(map); } @@ -1278,6 +1325,7 @@ PUPnP::getMappingsListByDescr(const std::shared_ptr<IGD>& igd, const std::string break; } + auto timeIgdRequestSent = sys_clock::now(); int upnp_err = UpnpSendAction(ctrlptHandle_, upnpIgd->getControlURL().c_str(), upnpIgd->getServiceType().c_str(), @@ -1330,8 +1378,7 @@ PUPnP::getMappingsListByDescr(const std::shared_ptr<IGD>& igd, const std::string std::string transport(getFirstDocItem(response.get(), "NewProtocol")); if (port_internal.empty() || port_external.empty() || transport.empty()) { - // if (logger_) logger_->e("PUPnP: GetGenericPortMappingEntry returned an invalid entry at index %i", - // entry_idx); + // Invalid entry, ignore continue; } @@ -1340,8 +1387,14 @@ PUPnP::getMappingsListByDescr(const std::shared_ptr<IGD>& igd, const std::string auto ePort = to_int<uint16_t>(port_external); auto iPort = to_int<uint16_t>(port_internal); + auto leaseDurationStr = getFirstDocItem(response.get(), "NewLeaseDuration"); + auto leaseDuration = to_int<uint32_t>(leaseDurationStr); + auto expiryTime = (leaseDuration == 0) ? sys_clock::time_point::max() + : timeIgdRequestSent + std::chrono::seconds(leaseDuration); + Mapping map(type, ePort, iPort); map.setIgd(igd); + map.setExpiryTime(expiryTime); mapList.emplace(map.getMapKey(), std::move(map)); } @@ -1514,7 +1567,7 @@ PUPnP::actionAddPortMapping(const Mapping& mapping) ACTION_ADD_PORT_MAPPING, igd->getServiceType().c_str(), "NewLeaseDuration", - "0"); + std::to_string(MAPPING_LEASE_DURATION).c_str()); action.reset(action_container_ptr); diff --git a/src/upnp/protocol/pupnp/pupnp.h b/src/upnp/protocol/pupnp/pupnp.h index e29e01d969a9cb12450184697c62485dd1d4c9c4..32fe19a1ed45c9898f8d8c3d38c3164fd4585e04 100644 --- a/src/upnp/protocol/pupnp/pupnp.h +++ b/src/upnp/protocol/pupnp/pupnp.h @@ -51,6 +51,8 @@ class IpAddr; namespace dhtnet { namespace upnp { +constexpr static unsigned int MAPPING_LEASE_DURATION {604800}; + class PUPnP : public UPnPProtocol { public: @@ -100,8 +102,7 @@ public: void requestMappingAdd(const Mapping& mapping) override; // Renew an allocated mapping. - // Not implemented. Currently, UPNP allocations do not have expiration time. - void requestMappingRenew([[maybe_unused]] const Mapping& mapping) override { assert(false); }; + void requestMappingRenew(const Mapping& mapping) override; // Removes a mapping. void requestMappingRemove(const Mapping& igdMapping) override; @@ -153,6 +154,9 @@ private: // Process the reception of an add mapping action answer. void processAddMapAction(const Mapping& map); + // Called after a successful mapping renewal + void processMappingRenewed(const Mapping& map); + // Process the a mapping request failure. void processRequestMappingFailure(const Mapping& map); diff --git a/src/upnp/upnp_context.cpp b/src/upnp/upnp_context.cpp index e161a4394a67cf858250381c8bb244e1bc2017aa..42de87f54b05b1fec20668ad67323c3484446543 100644 --- a/src/upnp/upnp_context.cpp +++ b/src/upnp/upnp_context.cpp @@ -30,13 +30,14 @@ #else #include <fmt/ostream.h> #endif +#include <fmt/chrono.h> namespace dhtnet { namespace upnp { -constexpr static auto MAP_UPDATE_INTERVAL = std::chrono::seconds(30); +constexpr static auto MAPPING_RENEWAL_THROTTLING_DELAY = std::chrono::seconds(10); constexpr static int MAX_REQUEST_RETRIES = 20; -constexpr static int MAX_REQUEST_REMOVE_COUNT = 5; +constexpr static int MAX_REQUEST_REMOVE_COUNT = 10; // TODO: increase? constexpr static uint16_t UPNP_TCP_PORT_MIN {10000}; constexpr static uint16_t UPNP_TCP_PORT_MAX {UPNP_TCP_PORT_MIN + 5000}; @@ -46,7 +47,9 @@ constexpr static uint16_t UPNP_UDP_PORT_MAX {UPNP_UDP_PORT_MIN + 5000}; UPnPContext::UPnPContext(const std::shared_ptr<asio::io_context>& ioContext, const std::shared_ptr<dht::log::Logger>& logger) : ctx(createIoContext(ioContext, logger)) , logger_(logger) - , mappingListUpdateTimer_(*ctx) + , mappingRenewalTimer_(*ctx) + , renewalSchedulingTimer_(*ctx) + , syncTimer_(*ctx) , connectivityChangedTimer_(*ctx) { if (logger_) logger_->debug("Creating UPnPContext instance [{}]", fmt::ptr(this)); @@ -90,7 +93,6 @@ UPnPContext::shutdown(std::condition_variable& cv) std::lock_guard lock(mappingMutex_); mappingList_->clear(); - mappingListUpdateTimer_.cancel(); controllerList_.clear(); protocolList_.clear(); shutdownComplete_ = true; @@ -174,39 +176,38 @@ UPnPContext::stopUpnp(bool forceRelease) { if (logger_) logger_->debug("Stopping UPnP context"); - // Clear all current mappings if any. + connectivityChangedTimer_.cancel(); + mappingRenewalTimer_.cancel(); + renewalSchedulingTimer_.cancel(); + syncTimer_.cancel(); + syncRequested_ = false; - // Use a temporary list to avoid processing the mapping - // list while holding the lock. + // Clear all current mappings + + // Use a temporary list to avoid processing the mappings while holding the lock. std::list<Mapping::sharedPtr_t> toRemoveList; { std::lock_guard lock(mappingMutex_); PortType types[2] {PortType::TCP, PortType::UDP}; for (auto& type : types) { - auto& mappingList = getMappingList(type); - for (auto const& [_, map] : mappingList) { + const auto& mappingList = getMappingList(type); + for (const auto& [_, map] : mappingList) { toRemoveList.emplace_back(map); } } - // Invalidate the current IGDs. - preferredIgd_.reset(); - validIgdList_.clear(); + // Invalidate the current IGD. + currentIgd_.reset(); } for (auto const& map : toRemoveList) { requestRemoveMapping(map); - // Notify is not needed in updateState when - // shutting down (hence set it to false). NotifyCallback - // would trigger a new SIP registration and create a - // false registered state upon program close. - // It's handled by upper layers. - updateMappingState(map, MappingState::FAILED, false); - // We dont remove mappings with auto-update enabled, - // unless forceRelease is true. - if (not map->getAutoUpdate() or forceRelease) { - map->enableAutoUpdate(false); - unregisterMapping(map); + if (map->getAutoUpdate() && !forceRelease) { + // Set the mapping's state to PENDING so that it + // gets recreated if we restart UPnP later. + map->setState(MappingState::PENDING); + } else { + unregisterMapping(map, true); } } @@ -219,28 +220,16 @@ UPnPContext::stopUpnp(bool forceRelease) } uint16_t -UPnPContext::generateRandomPort(PortType type, bool mustBeEven) +UPnPContext::generateRandomPort(PortType type) { auto minPort = type == PortType::TCP ? UPNP_TCP_PORT_MIN : UPNP_UDP_PORT_MIN; auto maxPort = type == PortType::TCP ? UPNP_TCP_PORT_MAX : UPNP_UDP_PORT_MAX; - if (minPort >= maxPort) { - // if (logger_) logger_->error("Max port number ({}) must be greater than min port number ({})", maxPort, minPort); - // Must be called with valid range. - assert(false); - } - - int fact = mustBeEven ? 2 : 1; - if (mustBeEven) { - minPort /= fact; - maxPort /= fact; - } - // Seed the generator. static std::mt19937 gen(dht::crypto::getSeededRandomEngine()); // Define the range. std::uniform_int_distribution<uint16_t> dist(minPort, maxPort); - return dist(gen) * fact; + return dist(gen); } void @@ -298,9 +287,6 @@ UPnPContext::_connectivityChanged(const asio::error_code& ec) stopUpnp(); startUpnp(); - - // Mapping with auto update enabled must be processed first. - processMappingWithAutoUpdate(); } void @@ -309,7 +295,7 @@ UPnPContext::setPublicAddress(const IpAddr& addr) if (not addr) return; - std::lock_guard lock(mappingMutex_); + std::lock_guard lock(publicAddressMutex_); if (knownPublicAddress_ != addr) { knownPublicAddress_ = std::move(addr); if (logger_) logger_->debug("Setting the known public address to {}", addr.toString()); @@ -320,17 +306,15 @@ bool UPnPContext::isReady() const { std::lock_guard lock(mappingMutex_); - return not validIgdList_.empty(); + return currentIgd_ ? true : false; } IpAddr UPnPContext::getExternalIP() const { std::lock_guard lock(mappingMutex_); - // Return the first IGD Ip available. - if (not validIgdList_.empty()) { - return (*validIgdList_.begin())->getPublicIp(); - } + if (currentIgd_) + return currentIgd_->getPublicIp(); return {}; } @@ -350,7 +334,7 @@ UPnPContext::reserveMapping(Mapping& requestedMap) { std::lock_guard lock(mappingMutex_); - auto& mappingList = getMappingList(requestedMap.getType()); + const auto& mappingList = getMappingList(requestedMap.getType()); // We try to provide a mapping in "OPEN" state. If not found, // we provide any available mapping. In this case, it's up to @@ -375,7 +359,6 @@ UPnPContext::reserveMapping(Mapping& requestedMap) // Create a mapping if none was available. if (not mapRes) { - // JAMI_WARN("Did not find any available mapping. Will request one now"); mapRes = registerMapping(requestedMap); } @@ -390,11 +373,12 @@ UPnPContext::reserveMapping(Mapping& requestedMap) cb(mapRes); } - updateMappingList(true); + enforceAvailableMappingsLimits(); return mapRes; } +// TODO: double-check what the expected behavior is when the mapping has auto-update enabled. void UPnPContext::releaseMapping(const Mapping& map) { @@ -417,6 +401,7 @@ UPnPContext::releaseMapping(const Mapping& map) // Remove it. requestRemoveMapping(mapPtr); unregisterMapping(mapPtr); + enforceAvailableMappingsLimits(); }); } @@ -464,30 +449,30 @@ UPnPContext::getIgdsInfo() const { std::vector<IGDInfo> igdInfoList; - std::lock_guard lk(mappingMutex_); - for (auto& igd : validIgdList_) { - auto protocol = protocolList_.at(igd->getProtocol()); + for (const auto& [_, protocol] : protocolList_) { + for (auto& igd : protocol->getIgdList()) { + IGDInfo info; + info.uid = igd->getUID(); + info.localIp = igd->getLocalIp(); + info.publicIp = igd->getPublicIp(); + info.mappingInfoList = protocol->getMappingsInfo(igd); - IGDInfo info; - info.uid = igd->getUID(); - info.localIp = igd->getLocalIp(); - info.publicIp = igd->getPublicIp(); - info.mappingInfoList = protocol->getMappingsInfo(igd); - - igdInfoList.push_back(std::move(info)); + igdInfoList.push_back(std::move(info)); + } } return igdInfoList; } +// TODO: refactor this function so that it can never fail unless there are literally no ports available uint16_t UPnPContext::getAvailablePortNumber(PortType type) { - // Only return an availalable random port. No actual + // Only return an available random port. No actual // reservation is made here. std::lock_guard lock(mappingMutex_); - auto& mappingList = getMappingList(type); + const auto& mappingList = getMappingList(type); int tryCount = 0; while (tryCount++ < MAX_REQUEST_RETRIES) { uint16_t port = generateRandomPort(type); @@ -505,13 +490,14 @@ void UPnPContext::requestMapping(const Mapping::sharedPtr_t& map) { assert(map); - auto const& igd = getPreferredIgd(); + auto const& igd = getCurrentIgd(); // We must have at least a valid IGD pointer if we get here. - // Not this method is called only if there were a valid IGD, however, - // because the processing is asynchronous, it's possible that the IGD - // was invalidated when the this code executed. + // Note that this method is called only if there was a valid IGD, but + // because the processing is asynchronous, there may no longer + // be one by the time this code executes. if (not igd) { - if (logger_) logger_->debug("No valid IGDs available"); + if (logger_) logger_->debug("Unable to request mapping {}: no valid IGDs available", + map->toString()); return; } @@ -528,13 +514,11 @@ UPnPContext::requestMapping(const Mapping::sharedPtr_t& map) protocol->requestMappingAdd(*map); } -bool +void UPnPContext::provisionNewMappings(PortType type, int portCount) { if (logger_) logger_->debug("Provision {:d} new mappings of type [{}]", portCount, Mapping::getTypeStr(type)); - assert(portCount > 0); - while (portCount > 0) { auto port = getAvailablePortNumber(type); if (port > 0) { @@ -544,23 +528,16 @@ UPnPContext::provisionNewMappings(PortType type, int portCount) registerMapping(map); } else { // Very unlikely to get here! - if (logger_) logger_->error("Can not find any available port to provision!"); - return false; + if (logger_) logger_->error("Cannot provision port: no available port number"); } } - - return true; } -bool +void UPnPContext::deleteUnneededMappings(PortType type, int portCount) { if (logger_) logger_->debug("Remove {:d} unneeded mapping of type [{}]", portCount, Mapping::getTypeStr(type)); - assert(portCount > 0); - - //CHECK_VALID_THREAD(); - std::lock_guard lock(mappingMutex_); auto& mappingList = getMappingList(type); @@ -588,18 +565,19 @@ UPnPContext::deleteUnneededMappings(PortType type, int portCount) it++; } } - - return true; } void -UPnPContext::updatePreferredIgd() +UPnPContext::updateCurrentIgd() { - if (preferredIgd_ and preferredIgd_->isValid()) + std::lock_guard lock(mappingMutex_); + if (currentIgd_ and currentIgd_->isValid()) { + if (logger_) logger_->debug("Current IGD is still valid, no need to update"); return; + } // Reset and search for the best IGD. - preferredIgd_.reset(); + currentIgd_.reset(); for (auto const& [_, protocol] : protocolList_) { if (protocol->isReady()) { @@ -610,236 +588,309 @@ UPnPContext::updatePreferredIgd() continue; // Prefer NAT-PMP over PUPNP. - if (preferredIgd_ and igd->getProtocol() != NatProtocolType::NAT_PMP) + if (currentIgd_ and igd->getProtocol() != NatProtocolType::NAT_PMP) continue; // Update. - preferredIgd_ = igd; + currentIgd_ = igd; } } - if (preferredIgd_ and preferredIgd_->isValid()) { - if (logger_) logger_->debug("Preferred IGD updated to [{}] IGD [{} {}] ", - preferredIgd_->getProtocolName(), - preferredIgd_->getUID(), - preferredIgd_->toString()); + if (currentIgd_ and currentIgd_->isValid()) { + if (logger_) logger_->debug("Current IGD updated to [{}] IGD [{} {}] ", + currentIgd_->getProtocolName(), + currentIgd_->getUID(), + currentIgd_->toString()); + } else { + if (logger_) logger_->warn("Couldn't update current IGD: no valid IGD was found"); } } std::shared_ptr<IGD> -UPnPContext::getPreferredIgd() const +UPnPContext::getCurrentIgd() const { - //CHECK_VALID_THREAD(); - - return preferredIgd_; + return currentIgd_; } void -UPnPContext::updateMappingList(bool async) +UPnPContext::enforceAvailableMappingsLimits() { - // Run async if requested. - if (async) { - ctx->post([this] { updateMappingList(false); }); - return; - } + for (auto type : {PortType::TCP, PortType::UDP}) { + int pendingCount = 0; + int inProgressCount = 0; + int openCount = 0; + { + std::lock_guard lock(mappingMutex_); + const auto& mappingList = getMappingList(type); + for (const auto& [_, mapping] : mappingList) { + if (!mapping->isAvailable()) + continue; + switch (mapping->getState()) { + case MappingState::PENDING: + pendingCount++; + break; + case MappingState::IN_PROGRESS: + inProgressCount++; + break; + case MappingState::OPEN: + openCount++; + break; + default: + break; + } + } + } + int availableCount = openCount + pendingCount + inProgressCount; + if (logger_) logger_->debug("Number of 'available' {} mappings in the local list: {} ({} open + {} pending + {} in progress)", + Mapping::getTypeStr(type), + availableCount, + openCount, + pendingCount, + inProgressCount); + + int minAvailableMappings = getMinAvailableMappings(type); + if (minAvailableMappings > availableCount) { + provisionNewMappings(type, minAvailableMappings - availableCount); + continue; + } - mappingListUpdateTimer_.cancel(); + int maxAvailableMappings = getMaxAvailableMappings(type); + if (openCount > maxAvailableMappings) { + deleteUnneededMappings(type, openCount - maxAvailableMappings); + } + } +} - // Skip if no controller registered. - if (controllerList_.empty()) +void +UPnPContext::renewMappings() +{ + if (!started_) return; - // Cancel the current timer (if any) and re-schedule. - std::shared_ptr<IGD> prefIgd = getPreferredIgd(); - if (not prefIgd) { - if (logger_) logger_->debug("UPNP/NAT-PMP enabled, but no valid IGDs available"); - // No valid IGD. Nothing to do. + const auto& igd = getCurrentIgd(); + if (!igd) { + if (logger_) logger_->debug("Cannot renew mappings: no valid IGD available"); return; } - mappingListUpdateTimer_.expires_after(MAP_UPDATE_INTERVAL); - mappingListUpdateTimer_.async_wait([this](asio::error_code const& ec) { - if (ec != asio::error::operation_aborted) - updateMappingList(false); - }); - - // Process pending requests if any. - processPendingRequests(prefIgd); - - // Make new requests for mappings that failed and have - // the auto-update option enabled. - processMappingWithAutoUpdate(); - - PortType typeArray[2] = {PortType::TCP, PortType::UDP}; - - for (auto idx : {0, 1}) { - auto type = typeArray[idx]; + auto now = sys_clock::now(); + auto nextRenewalTime = sys_clock::time_point::max(); - MappingStatus status; - getMappingStatus(type, status); + std::vector<Mapping::sharedPtr_t> toRenew; + int toRenewLaterCount = 0; - if (logger_) logger_->debug("Mapping status [{}] - overall {:d}: {:d} open ({:d} ready + {:d} in use), {:d} pending, {:d} " - "in-progress, {:d} failed", - Mapping::getTypeStr(type), - status.sum(), - status.openCount_, - status.readyCount_, - status.openCount_ - status.readyCount_, - status.pendingCount_, - status.inProgressCount_, - status.failedCount_); + for (auto type : {PortType::TCP, PortType::UDP}) { + std::lock_guard lock(mappingMutex_); + const auto& mappingList = getMappingList(type); + for (const auto& [_, map] : mappingList) { + if (not map->isValid()) + continue; + if (map->getState() != MappingState::OPEN) + continue; - if (status.failedCount_ > 0) { - std::lock_guard lock(mappingMutex_); - auto const& mappingList = getMappingList(type); - for (auto const& [_, map] : mappingList) { - if (map->getState() == MappingState::FAILED) { - if (logger_) logger_->debug("Mapping status [{}] - Available [{}]", - map->toString(true), - map->isAvailable() ? "YES" : "NO"); - } + auto mapRenewalTime = map->getRenewalTime(); + if (now >= mapRenewalTime) { + toRenew.emplace_back(map); + } else if (mapRenewalTime < sys_clock::time_point::max()) { + toRenewLaterCount++; + if (mapRenewalTime < nextRenewalTime) + nextRenewalTime = map->getRenewalTime(); } - } - int toRequestCount = (int) minOpenPortLimit_[idx] - - (int) (status.readyCount_ + status.inProgressCount_ - + status.pendingCount_); - - // Provision/release mappings accordingly. - if (toRequestCount > 0) { - // Take into account the request in-progress when making - // requests for new mappings. - provisionNewMappings(type, toRequestCount); - } else if (status.readyCount_ > maxOpenPortLimit_[idx]) { - deleteUnneededMappings(type, status.readyCount_ - maxOpenPortLimit_[idx]); } } - // Prune the mapping list if needed - if (protocolList_.at(NatProtocolType::PUPNP)->isReady()) { -#if HAVE_LIBNATPMP - // Dont perform if NAT-PMP is valid. - if (not protocolList_.at(NatProtocolType::NAT_PMP)->isReady()) -#endif - { - pruneMappingList(); - } + if (!toRenew.empty()) { + if (logger_) logger_->debug("Sending renewal requests for {} mappings", toRenew.size()); + } + for (const auto& map : toRenew) { + const auto& protocol = protocolList_.at(map->getIgd()->getProtocol()); + protocol->requestMappingRenew(*map); + } + if (toRenewLaterCount > 0) { + nextRenewalTime += MAPPING_RENEWAL_THROTTLING_DELAY; + if (logger_) logger_->debug("{} mappings didn't need to be renewed (next renewal scheduled for {:%Y-%m-%d %H:%M:%S})", + toRenewLaterCount, + fmt::localtime(sys_clock::to_time_t(nextRenewalTime))); + mappingRenewalTimer_.expires_at(nextRenewalTime); + mappingRenewalTimer_.async_wait([this](asio::error_code const& ec) { + if (ec != asio::error::operation_aborted) + renewMappings(); + }); } - -#if HAVE_LIBNATPMP - // Renew nat-pmp allocations - if (protocolList_.at(NatProtocolType::NAT_PMP)->isReady()) - renewAllocations(); -#endif } void -UPnPContext::pruneMappingList() +UPnPContext::scheduleMappingsRenewal() { - //CHECK_VALID_THREAD(); - - MappingStatus status; - getMappingStatus(status); + // Debounce the scheduling function so that it doesn't get called multiple + // times when several mappings are added or renewed in rapid succession. + renewalSchedulingTimer_.expires_after(std::chrono::milliseconds(500)); + renewalSchedulingTimer_.async_wait([this](asio::error_code const& ec) { + if (ec != asio::error::operation_aborted) + _scheduleMappingsRenewal(); + }); +} - // Do not prune the list if there are pending/in-progress requests. - if (status.inProgressCount_ != 0 or status.pendingCount_ != 0) { +void +UPnPContext::_scheduleMappingsRenewal() +{ + if (!started_) return; + + sys_clock::time_point nextRenewalTime = sys_clock::time_point::max(); + for (auto type : {PortType::TCP, PortType::UDP}) { + std::lock_guard lock(mappingMutex_); + const auto& mappingList = getMappingList(type); + for (const auto& [_, map] : mappingList) { + if (map->getState() == MappingState::OPEN && + map->getRenewalTime() < nextRenewalTime) + nextRenewalTime = map->getRenewalTime(); + } } + if (nextRenewalTime == sys_clock::time_point::max()) + return; - auto const& igd = getPreferredIgd(); - if (not igd or igd->getProtocol() != NatProtocolType::PUPNP) { + // Add a small delay so that we don't have to call renewMappings multiple + // times in a row (and iterate over the whole list of mappings each time) + // when multiple mappings have almost the same renewal time. + nextRenewalTime += MAPPING_RENEWAL_THROTTLING_DELAY; + if (nextRenewalTime == mappingRenewalTimer_.expiry()) return; - } - auto protocol = protocolList_.at(NatProtocolType::PUPNP); - auto remoteMapList = protocol->getMappingsListByDescr(igd, - Mapping::UPNP_MAPPING_DESCRIPTION_PREFIX); - /*if (remoteMapList.empty()) { - std::lock_guard lock(mappingMutex_); - if (not getMappingList(PortType::TCP).empty() or getMappingList(PortType::TCP).empty()) { - // JAMI_WARN("We have provisionned mappings but the PUPNP IGD returned an empty list!"); - } - }*/ + if (logger_) logger_->debug("Scheduling next port mapping renewal for {:%Y-%m-%d %H:%M:%S}", + fmt::localtime(sys_clock::to_time_t(nextRenewalTime))); + mappingRenewalTimer_.expires_at(nextRenewalTime); + mappingRenewalTimer_.async_wait([this](asio::error_code const& ec) { + if (ec != asio::error::operation_aborted) + renewMappings(); + }); +} - pruneUnMatchedMappings(igd, remoteMapList); - pruneUnTrackedMappings(igd, remoteMapList); +void +UPnPContext::syncLocalMappingListWithIgd() +{ + std::lock_guard lock(syncMutex_); + if (syncRequested_) + return; + + syncRequested_ = true; + syncTimer_.expires_after(std::chrono::minutes(5)); + syncTimer_.async_wait([this](asio::error_code const& ec) { + if (ec != asio::error::operation_aborted) + _syncLocalMappingListWithIgd(); + }); } void -UPnPContext::pruneUnMatchedMappings(const std::shared_ptr<IGD>& igd, - const std::map<Mapping::key_t, Mapping>& remoteMapList) +UPnPContext::_syncLocalMappingListWithIgd() { - // Check/synchronize local mapping list with the list - // returned by the IGD. + { + std::lock_guard lock(syncMutex_); + syncRequested_ = false; + } + const auto& igd = getCurrentIgd(); + if (!started_ || !igd || igd->getProtocol() != NatProtocolType::PUPNP) { + return; + } + auto pupnp = protocolList_.at(NatProtocolType::PUPNP); + if (!pupnp->isReady()) + return; + if (logger_) logger_->debug("Synchronizing local mapping list with IGD [{}]", + igd->toString()); + auto remoteMapList = pupnp->getMappingsListByDescr(igd, + Mapping::UPNP_MAPPING_DESCRIPTION_PREFIX); + bool requestsInProgress = false; + // Use a temporary list to avoid processing mappings while holding the lock. + std::list<Mapping::sharedPtr_t> toRemoveFromLocalList; for (auto type: {PortType::TCP, PortType::UDP}) { - // Use a temporary list to avoid processing mappings while holding the lock. - std::list<Mapping::sharedPtr_t> toRemoveList; - { - std::lock_guard lock(mappingMutex_); - for (auto const& [_, map] : getMappingList(type)) { - // Only check mappings allocated by UPNP protocol. - if (map->getProtocol() != NatProtocolType::PUPNP) { - continue; - } - // Set mapping as failed if not found in the list - // returned by the IGD. - if (map->getState() == MappingState::OPEN - and remoteMapList.find(map->getMapKey()) == remoteMapList.end()) { - toRemoveList.emplace_back(map); - - if (logger_) logger_->warn("Mapping {} (IGD {}) marked as \"OPEN\" but not found in the " - "remote list. Mark as failed!", - map->toString(), - igd->toString()); + std::lock_guard lock(mappingMutex_); + for (auto& [_, map] : getMappingList(type)) { + if (map->getProtocol() != NatProtocolType::PUPNP) { + continue; + } + switch (map->getState()) { + case MappingState::PENDING: + case MappingState::IN_PROGRESS: + requestsInProgress = true; + break; + case MappingState::OPEN: { + auto it = remoteMapList.find(map->getMapKey()); + if (it == remoteMapList.end()) { + if (logger_) logger_->warn("Mapping {} (IGD {}) marked as \"OPEN\" but not found in the " + "remote list. Removing from local list.", + map->toString(), + igd->toString()); + toRemoveFromLocalList.emplace_back(map); + } else { + auto oldExpiryTime = map->getExpiryTime(); + auto newExpiryTime = it->second.getExpiryTime(); + // The value of newExpiryTime is based on the mapping's "lease duration" that we got from + // the IGD, which is supposed to be (according to the UPnP specification) the number of + // seconds remaining before the mapping expires. In practice, the duration values returned + // by some routers are only precise to the hour (i.e. they're always multiples of 3600). This + // means that newExpiryTime can exceed the real expiry time by up to an hour in the worst case. + // In order to avoid accidentally scheduling a mapping's renewal too late, we only allow ourselves to + // push back its renewal time if newExpiryTime is bigger than oldExpiryTime by a sufficient margin. + if (newExpiryTime < oldExpiryTime || + newExpiryTime > oldExpiryTime + std::chrono::seconds(2 * 3600)) { + auto newRenewalTime = map->getRenewalTime() + (newExpiryTime - oldExpiryTime) / 2; + map->setRenewalTime(newRenewalTime); + map->setExpiryTime(newExpiryTime); + } + } + break; } + default: + break; } } + } + scheduleMappingsRenewal(); - for (auto const& map : toRemoveList) { - updateMappingState(map, MappingState::FAILED); - unregisterMapping(map); - } + for (auto const& map : toRemoveFromLocalList) { + updateMappingState(map, MappingState::FAILED); + unregisterMapping(map); } -} + if (!toRemoveFromLocalList.empty()) + enforceAvailableMappingsLimits(); -void -UPnPContext::pruneUnTrackedMappings(const std::shared_ptr<IGD>& igd, - const std::map<Mapping::key_t, Mapping>& remoteMapList) -{ + if (requestsInProgress) { + // It's unlikely that there will be requests in progress when this function is + // called, but if there are, that suggests that we are dealing with a slow + // router, so we return early instead of sending additional deletion requests + // (which aren't essential and could end up "competing" with higher-priority + // creation/renewal requests). + return; + } // Use a temporary list to avoid processing mappings while holding the lock. - std::list<Mapping> toRemoveList; + std::list<Mapping> toRemoveFromIgd; { std::lock_guard lock(mappingMutex_); for (auto const& [_, map] : remoteMapList) { - // Must has valid IGD pointer and use UPNP protocol. - assert(map.getIgd()); - assert(map.getIgd()->getProtocol() == NatProtocolType::PUPNP); - auto& mappingList = getMappingList(map.getType()); + const auto& mappingList = getMappingList(map.getType()); auto it = mappingList.find(map.getMapKey()); if (it == mappingList.end()) { // Not present, request mapping remove. - toRemoveList.emplace_back(std::move(map)); + toRemoveFromIgd.emplace_back(std::move(map)); // Make only few remove requests at once. - if (toRemoveList.size() >= MAX_REQUEST_REMOVE_COUNT) + if (toRemoveFromIgd.size() >= MAX_REQUEST_REMOVE_COUNT) break; } } } - // Remove un-tracked mappings. - auto protocol = protocolList_.at(NatProtocolType::PUPNP); - for (auto const& map : toRemoveList) { - protocol->requestMappingRemove(map); + for (const auto& map : toRemoveFromIgd) { + pupnp->requestMappingRemove(map); } + } void UPnPContext::pruneMappingsWithInvalidIgds(const std::shared_ptr<IGD>& igd) { - //CHECK_VALID_THREAD(); - // Use temporary list to avoid holding the lock while // processing the mapping list. std::list<Mapping::sharedPtr_t> toRemoveList; @@ -848,7 +899,7 @@ UPnPContext::pruneMappingsWithInvalidIgds(const std::shared_ptr<IGD>& igd) PortType types[2] {PortType::TCP, PortType::UDP}; for (auto& type : types) { - auto& mappingList = getMappingList(type); + const auto& mappingList = getMappingList(type); for (auto const& [_, map] : mappingList) { if (map->getIgd() == igd) toRemoveList.emplace_back(map); @@ -867,7 +918,7 @@ UPnPContext::pruneMappingsWithInvalidIgds(const std::shared_ptr<IGD>& igd) } void -UPnPContext::processPendingRequests(const std::shared_ptr<IGD>& igd) +UPnPContext::processPendingRequests() { // This list holds the mappings to be requested. This is // needed to avoid performing the requests while holding @@ -880,12 +931,11 @@ UPnPContext::processPendingRequests(const std::shared_ptr<IGD>& igd) PortType typeArray[2] {PortType::TCP, PortType::UDP}; for (auto type : typeArray) { - auto& mappingList = getMappingList(type); - for (auto& [_, map] : mappingList) { + const auto& mappingList = getMappingList(type); + for (const auto& [_, map] : mappingList) { if (map->getState() == MappingState::PENDING) { - if (logger_) logger_->debug("Send pending request for mapping {} to IGD {}", - map->toString(), - igd->toString()); + if (logger_) logger_->debug("Will attempt to send a request for pending mapping {}", + map->toString()); requestsList.emplace_back(map); } } @@ -898,50 +948,6 @@ UPnPContext::processPendingRequests(const std::shared_ptr<IGD>& igd) } } -void -UPnPContext::processMappingWithAutoUpdate() -{ - // This list holds the mappings to be requested. This is - // needed to avoid performing the requests while holding - // the lock. - std::list<Mapping::sharedPtr_t> requestsList; - - // Populate the list of requests for mappings with auto-update enabled. - { - std::lock_guard lock(mappingMutex_); - PortType typeArray[2] {PortType::TCP, PortType::UDP}; - - for (auto type : typeArray) { - auto& mappingList = getMappingList(type); - for (auto const& [_, map] : mappingList) { - if (map->getState() == MappingState::FAILED and map->getAutoUpdate()) { - requestsList.emplace_back(map); - } - } - } - } - - for (auto const& oldMap : requestsList) { - // Request a new mapping if auto-update is enabled. - if (logger_) logger_->debug("Mapping {} has auto-update enabled, a new mapping will be requested", - oldMap->toString()); - - // Reserve a new mapping. - Mapping newMapping(oldMap->getType()); - newMapping.enableAutoUpdate(true); - newMapping.setNotifyCallback(oldMap->getNotifyCallback()); - - auto const& mapPtr = reserveMapping(newMapping); - assert(mapPtr); - - // Release the old one. - oldMap->setAvailable(true); - oldMap->enableAutoUpdate(false); - oldMap->setNotifyCallback(nullptr); - unregisterMapping(oldMap); - } -} - void UPnPContext::onIgdUpdated(const std::shared_ptr<IGD>& igd, UpnpIgdEvent event) { @@ -960,26 +966,33 @@ UPnPContext::onIgdUpdated(const std::shared_ptr<IGD>& igd, UpnpIgdEvent event) protocolName, IgdState); - // Check if the IGD has valid addresses. if (not igdLocalAddr) { - if (logger_) logger_->warn("[{}] IGD has an invalid local address", protocolName); + if (logger_) logger_->warn("[{}] IGD [{} {}] has an invalid local address, ignoring", + protocolName, + igd->getUID(), + igd->toString()); return; } if (not igd->getPublicIp()) { - if (logger_) logger_->warn("[{}] IGD has an invalid public address", protocolName); + if (logger_) logger_->warn("[{}] IGD [{} {}] has an invalid public address, ignoring", + protocolName, + igd->getUID(), + igd->toString()); return; } - if (knownPublicAddress_ and igd->getPublicIp() != knownPublicAddress_) { - if (logger_) logger_->warn("[{}] IGD external address [{}] does not match known public address [{}]." - " The mapped addresses might not be reachable", - protocolName, - igd->getPublicIp().toString(), - knownPublicAddress_.toString()); + { + std::lock_guard lock(publicAddressMutex_); + if (knownPublicAddress_ and igd->getPublicIp() != knownPublicAddress_) { + if (logger_) logger_->warn("[{}] IGD external address [{}] does not match known public address [{}]." + " The mapped addresses might not be reachable", + protocolName, + igd->getPublicIp().toString(), + knownPublicAddress_.toString()); + } } - // Update the IGD list. if (event == UpnpIgdEvent::REMOVED or event == UpnpIgdEvent::INVALID_STATE) { if (logger_) logger_->warn("State of IGD [{} {}] [{}] changed to [{}]. Pruning the mapping list", igd->getUID(), @@ -988,36 +1001,18 @@ UPnPContext::onIgdUpdated(const std::shared_ptr<IGD>& igd, UpnpIgdEvent event) IgdState); pruneMappingsWithInvalidIgds(igd); - - std::lock_guard lock(mappingMutex_); - validIgdList_.erase(igd); - } else { - std::lock_guard lock(mappingMutex_); - auto ret = validIgdList_.emplace(igd); - if (ret.second) { - if (logger_) logger_->debug("IGD [{}] on address {} was added. Will process any pending requests", - protocolName, - igdLocalAddr.toString(true, true)); - } else { - // Already in the list. - if (logger_) logger_->error("IGD [{}] on address {} already in the list", - protocolName, - igdLocalAddr.toString(true, true)); - return; - } } - updatePreferredIgd(); - - // Update the provisionned mappings. - updateMappingList(false); + updateCurrentIgd(); + if (isReady()) { + processPendingRequests(); + enforceAvailableMappingsLimits(); + } } void UPnPContext::onMappingAdded(const std::shared_ptr<IGD>& igd, const Mapping& mapRes) { - //CHECK_VALID_THREAD(); - // Check if we have a pending request for this response. auto map = getMappingWithKey(mapRes.getMapKey()); if (not map) { @@ -1033,9 +1028,11 @@ UPnPContext::onMappingAdded(const std::shared_ptr<IGD>& igd, const Mapping& mapR map->setIgd(igd); map->setInternalAddress(mapRes.getInternalAddress()); map->setExternalPort(mapRes.getExternalPort()); - + map->setRenewalTime(mapRes.getRenewalTime()); + map->setExpiryTime(mapRes.getExpiryTime()); // Update the state and report to the owner. updateMappingState(map, MappingState::OPEN); + scheduleMappingsRenewal(); if (logger_) logger_->debug("Mapping {} (on IGD {} [{}]) successfully performed", map->toString(), @@ -1045,24 +1042,23 @@ UPnPContext::onMappingAdded(const std::shared_ptr<IGD>& igd, const Mapping& mapR // Call setValid() to reset the errors counter. We need // to reset the counter on each successful response. igd->setValid(true); + if (igd->getProtocol() == NatProtocolType::PUPNP) + syncLocalMappingListWithIgd(); } -#if HAVE_LIBNATPMP void UPnPContext::onMappingRenewed(const std::shared_ptr<IGD>& igd, const Mapping& map) { auto mapPtr = getMappingWithKey(map.getMapKey()); if (not mapPtr) { - // We may receive a notification for a canceled request. Ignore it. if (logger_) logger_->warn("Renewed mapping {} from IGD {} [{}] does not have a match in local list", map.toString(), igd->toString(), map.getProtocolName()); return; } - if (mapPtr->getProtocol() != NatProtocolType::NAT_PMP or not mapPtr->isValid() - or mapPtr->getState() != MappingState::OPEN) { + if (!mapPtr->isValid() || mapPtr->getState() != MappingState::OPEN) { if (logger_) logger_->warn("Renewed mapping {} from IGD {} [{}] is in unexpected state", mapPtr->toString(), igd->toString(), @@ -1071,8 +1067,11 @@ UPnPContext::onMappingRenewed(const std::shared_ptr<IGD>& igd, const Mapping& ma } mapPtr->setRenewalTime(map.getRenewalTime()); + mapPtr->setExpiryTime(map.getExpiryTime()); + scheduleMappingsRenewal(); + if (igd->getProtocol() == NatProtocolType::PUPNP) + syncLocalMappingListWithIgd(); } -#endif void UPnPContext::requestRemoveMapping(const Mapping::sharedPtr_t& map) @@ -1085,17 +1084,6 @@ UPnPContext::requestRemoveMapping(const Mapping::sharedPtr_t& map) protocol->requestMappingRemove(*map); } -void -UPnPContext::deleteAllMappings(PortType type) -{ - std::lock_guard lock(mappingMutex_); - auto& mappingList = getMappingList(type); - - for (auto const& [_, map] : mappingList) { - requestRemoveMapping(map); - } -} - void UPnPContext::onMappingRemoved(const std::shared_ptr<IGD>& igd, const Mapping& mapRes) { @@ -1137,7 +1125,7 @@ UPnPContext::registerMapping(Mapping& map) } // No available IGD. The pending mapping requests will be processed - // when a IGD becomes available (in onIgdAdded() method). + // when an IGD becomes available if (not isReady()) { if (logger_) logger_->warn("No IGD available. Mapping will be requested when an IGD becomes available"); } else { @@ -1148,26 +1136,33 @@ UPnPContext::registerMapping(Mapping& map) } void -UPnPContext::unregisterMapping(const Mapping::sharedPtr_t& map) +UPnPContext::unregisterMapping(const Mapping::sharedPtr_t& map, bool ignoreAutoUpdate) { - //CHECK_VALID_THREAD(); - if (not map) { - // JAMI_ERR("Mapping pointer is null"); return; } - if (map->getAutoUpdate()) { - // Dont unregister mappings with auto-update enabled. - return; + if (map->getAutoUpdate() && !ignoreAutoUpdate) { + if (logger_) logger_->debug("Mapping {} has auto-update enabled, a new mapping will be requested", + map->toString()); + + Mapping newMapping(map->getType()); + newMapping.enableAutoUpdate(true); + newMapping.setNotifyCallback(map->getNotifyCallback()); + reserveMapping(newMapping); + + // TODO: figure out if this line is actually necessary + // (See https://review.jami.net/c/jami-daemon/+/16940) + map->setNotifyCallback(nullptr); } + std::lock_guard lock(mappingMutex_); auto& mappingList = getMappingList(map->getType()); if (mappingList.erase(map->getMapKey()) == 1) { if (logger_) logger_->debug("Unregistered mapping {}", map->toString()); } else { // The mapping may already be un-registered. Just ignore it. - if (logger_) logger_->debug("Mapping {} [{}] does not have a local match", + if (logger_) logger_->debug("Can't unregister mapping {} [{}] since it doesn't have a local match", map->toString(), map->getProtocolName()); } @@ -1191,85 +1186,36 @@ UPnPContext::getMappingWithKey(Mapping::key_t key) return it->second; } -void -UPnPContext::getMappingStatus(PortType type, MappingStatus& status) -{ - std::lock_guard lock(mappingMutex_); - auto& mappingList = getMappingList(type); - - for (auto const& [_, map] : mappingList) { - switch (map->getState()) { - case MappingState::PENDING: { - status.pendingCount_++; - break; - } - case MappingState::IN_PROGRESS: { - status.inProgressCount_++; - break; - } - case MappingState::FAILED: { - status.failedCount_++; - break; - } - case MappingState::OPEN: { - status.openCount_++; - if (map->isAvailable()) - status.readyCount_++; - break; - } - - default: - // Must not get here. - assert(false); - break; - } - } -} - -void -UPnPContext::getMappingStatus(MappingStatus& status) -{ - getMappingStatus(PortType::TCP, status); - getMappingStatus(PortType::UDP, status); -} - void UPnPContext::onMappingRequestFailed(const Mapping& mapRes) { + auto igd = mapRes.getIgd(); auto const& map = getMappingWithKey(mapRes.getMapKey()); if (not map) { // We may receive a response for a removed request. Just ignore it. - if (logger_) logger_->debug("Mapping {} [IGD {}] does not have a local match", - mapRes.toString(), - mapRes.getProtocolName()); - return; - } - - auto igd = map->getIgd(); - if (not igd) { - if (logger_) logger_->error("IGD pointer is null"); + if (logger_) logger_->debug("Ignoring failed request for mapping {} [IGD {}] since it doesn't have a local match", + mapRes.toString(), + igd->toString()); return; } updateMappingState(map, MappingState::FAILED); unregisterMapping(map); - if (logger_) logger_->warn("Mapping request for {} failed on IGD {} [{}]", + if (logger_) logger_->warn("Request for mapping {} on IGD {} failed", map->toString(), - igd->toString(), - igd->getProtocolName()); + igd->toString()); + + enforceAvailableMappingsLimits(); } void UPnPContext::updateMappingState(const Mapping::sharedPtr_t& map, MappingState newState, bool notify) { - // CHECK_VALID_THREAD(); - assert(map); // Ignore if the state did not change. if (newState == map->getState()) { - // JAMI_DBG("Mapping %s already in state %s", map->toString().c_str(), map->getStateStr()); return; } @@ -1281,44 +1227,5 @@ UPnPContext::updateMappingState(const Mapping::sharedPtr_t& map, MappingState ne map->getNotifyCallback()(map); } -#if HAVE_LIBNATPMP -void -UPnPContext::renewAllocations() -{ - //CHECK_VALID_THREAD(); - - // Check if the we have valid PMP IGD. - auto pmpProto = protocolList_.at(NatProtocolType::NAT_PMP); - - auto now = sys_clock::now(); - std::vector<Mapping::sharedPtr_t> toRenew; - - for (auto type : {PortType::TCP, PortType::UDP}) { - std::lock_guard lock(mappingMutex_); - auto mappingList = getMappingList(type); - for (auto const& [_, map] : mappingList) { - if (not map->isValid()) - continue; - if (map->getProtocol() != NatProtocolType::NAT_PMP) - continue; - if (map->getState() != MappingState::OPEN) - continue; - if (now < map->getRenewalTime()) - continue; - - toRenew.emplace_back(map); - } - } - - // Quit if there are no mapping to renew - if (toRenew.empty()) - return; - - for (auto const& map : toRenew) { - pmpProto->requestMappingRenew(*map); - } -} -#endif - } // namespace upnp } // namespace dhtnet diff --git a/tools/upnp/upnpctrl.cpp b/tools/upnp/upnpctrl.cpp index a5034a6be4ce323cac129a92a37bd72e53db0a79..7c4febff98144065cabcb43f00a2830ba70ddc06 100644 --- a/tools/upnp/upnpctrl.cpp +++ b/tools/upnp/upnpctrl.cpp @@ -115,6 +115,8 @@ main(int argc, char** argv) } } else if (command == "mappings") { print_mappings(upnpContext); + } else if (command == "restart") { + upnpContext->restart(); } else { fmt::print("Unknown command: {}\n", command); }