diff --git a/contrib/src/pjproject/0007-pj_ice_sess.patch b/contrib/src/pjproject/0007-pj_ice_sess.patch deleted file mode 100644 index 6f6263582358d6cf31a2b3b202da62ba5c83b2bb..0000000000000000000000000000000000000000 --- a/contrib/src/pjproject/0007-pj_ice_sess.patch +++ /dev/null @@ -1,42 +0,0 @@ -From 02c53ba6beb9e9faeb59059bf466e444dfdb14a6 Mon Sep 17 00:00:00 2001 -From: jrun <darwinskernel@gmail.com> -Date: Thu, 20 Feb 2020 12:33:47 -0500 -Subject: [PATCH 7/9] pj_ice_sess - ---- - pjnath/include/pjnath/ice_strans.h | 2 ++ - pjnath/src/pjnath/ice_strans.c | 5 +++++ - 2 files changed, 7 insertions(+) - -diff --git a/pjnath/include/pjnath/ice_strans.h b/pjnath/include/pjnath/ice_strans.h -index 1269442cd..41f3b6297 100644 ---- a/pjnath/include/pjnath/ice_strans.h -+++ b/pjnath/include/pjnath/ice_strans.h -@@ -1054,6 +1054,8 @@ PJ_DECL(pj_status_t) pj_ice_strans_sendto2(pj_ice_strans *ice_st, - const pj_sockaddr_t *dst_addr, - int dst_addr_len); - -+PJ_DECL(pj_ice_sess *) pj_ice_strans_get_ice_sess(pj_ice_strans *ice_st); -+ - - /** - * @} -diff --git a/pjnath/src/pjnath/ice_strans.c b/pjnath/src/pjnath/ice_strans.c -index 643bbbac9..603b75121 100644 ---- a/pjnath/src/pjnath/ice_strans.c -+++ b/pjnath/src/pjnath/ice_strans.c -@@ -1949,6 +1949,11 @@ static void on_ice_destroy(pj_ice_sess *ice) - } - } - -+PJ_DECL(pj_ice_sess *) pj_ice_strans_get_ice_sess( pj_ice_strans *ice_st ) -+{ -+ return ice_st->ice; -+} -+ - /* - * Callback called by ICE session when ICE processing is complete, either - * successfully or with failure. --- -2.24.1 - diff --git a/contrib/src/pjproject/package.json b/contrib/src/pjproject/package.json index 898e2c44e1f7b4ff95ad4ff02d3c265e92f68f6a..ab5b30fd09e8d661885285e37d314ba70665f07b 100644 --- a/contrib/src/pjproject/package.json +++ b/contrib/src/pjproject/package.json @@ -10,7 +10,6 @@ "0004-multiple_listeners.patch", "0005-fix_ebusy_turn.patch", "0006-ignore_ipv6_on_transport_check.patch", - "0007-pj_ice_sess.patch", "0007-upnp-srflx-nat-assisted-cand.patch", "0008-fix_ioqueue_ipv6_sendto.patch", "0009-add-config-site.patch", diff --git a/contrib/src/pjproject/rules.mak b/contrib/src/pjproject/rules.mak index edab0e0e28461b7ecef7b520192e90ff4b931c14..7b7544aee020ad6fe20a4dc6ad7aef4941a3cca1 100644 --- a/contrib/src/pjproject/rules.mak +++ b/contrib/src/pjproject/rules.mak @@ -51,7 +51,6 @@ pjproject: pjproject-$(PJPROJECT_VERSION).tar.gz .sum-pjproject $(APPLY) $(SRC)/pjproject/0004-multiple_listeners.patch $(APPLY) $(SRC)/pjproject/0005-fix_ebusy_turn.patch $(APPLY) $(SRC)/pjproject/0006-ignore_ipv6_on_transport_check.patch - $(APPLY) $(SRC)/pjproject/0007-pj_ice_sess.patch $(APPLY) $(SRC)/pjproject/0007-upnp-srflx-nat-assisted-cand.patch $(APPLY) $(SRC)/pjproject/0008-fix_ioqueue_ipv6_sendto.patch $(APPLY) $(SRC)/pjproject/0009-add-config-site.patch diff --git a/src/account.h b/src/account.h index 844d87c9d56a03ae6589cb6f0af7729cb4769255..aa54b6a20d9af5939264273f3bd0bac065030681 100644 --- a/src/account.h +++ b/src/account.h @@ -510,7 +510,7 @@ protected: */ bool upnpEnabled_; mutable std::mutex upnp_mtx {}; - std::unique_ptr<jami::upnp::Controller> upnp_; + std::shared_ptr<jami::upnp::Controller> upnp_; std::set<std::string> defaultModerators_ {}; bool localModeratorsEnabled_; diff --git a/src/client/configurationmanager.cpp b/src/client/configurationmanager.cpp index e85d06505a889d1ce1b00d7175f4695e6f237e61..291db2af7b7b92710a868d85ffcc63f406e2c486 100644 --- a/src/client/configurationmanager.cpp +++ b/src/client/configurationmanager.cpp @@ -966,7 +966,7 @@ connectivityChanged() // reset the UPnP context #if !(defined(TARGET_OS_IOS) && TARGET_OS_IOS) try { - jami::upnp::getUPnPContext()->connectivityChanged(); + jami::upnp::UPnPContext::getUPnPContext()->connectivityChanged(); } catch (std::runtime_error& e) { JAMI_ERR("UPnP context error: %s", e.what()); } diff --git a/src/ice_transport.cpp b/src/ice_transport.cpp index 55aeced158512b8c80c4689e8f49b2738b85276d..5ade17b140ab00608680d75dcff1fc0ae060ad88 100644 --- a/src/ice_transport.cpp +++ b/src/ice_transport.cpp @@ -63,6 +63,7 @@ static constexpr int MAX_DESTRUCTION_TIMEOUT {3}; using MutexGuard = std::lock_guard<std::mutex>; using MutexLock = std::unique_lock<std::mutex>; +using namespace upnp; namespace { @@ -117,6 +118,7 @@ public: IpAddr getLocalAddress(unsigned comp_id) const; IpAddr getRemoteAddress(unsigned comp_id) const; static const char* getCandidateType(const pj_ice_sess_cand* cand); + bool isTcpEnabled() const { return config_.protocol == PJ_ICE_TP_TCP; } std::unique_ptr<pj_pool_t, std::function<void(pj_pool_t*)>> pool_ {}; IceTransportCompleteCb on_initdone_cb_ {}; @@ -165,27 +167,17 @@ public: IpAddr addr; pj_ice_cand_transport transport; }; - std::vector<LocalCandidate> getLocalICECandidates(unsigned comp_id) const; - /** - * Adds a reflective candidate to ICE session - * Must be called before negotiation - */ - void addReflectiveCandidate(int comp_id, - const IpAddr& base, - const IpAddr& addr, - const pj_ice_cand_transport& transport); - - /** - * Creates UPnP port mappings and adds ICE candidates based on those mappings - */ - void selectUPnPIceCandidates(); - - void setDefaultRemoteAddress(int comp_id, const IpAddr& addr); - const IpAddr& getDefaultRemoteAddress(int comp_id) const; + bool addStunConfig(int af); + void requestUpnpMappings(); + void setupHostAndUpnpCandidates(); + void setDefaultRemoteAddress(unsigned comp_id, const IpAddr& addr); + const IpAddr& getDefaultRemoteAddress(unsigned comp_id) const; - std::unique_ptr<upnp::Controller> upnp_ {}; + std::shared_ptr<upnp::Controller> upnp_ {}; std::mutex upnpMutex_ {}; + std::vector<std::unique_ptr<Mapping>> upnpMappings_; + std::mutex upnpMappingsMutex_ {}; bool onlyIPv4Private_ {true}; @@ -201,35 +193,21 @@ public: std::atomic_bool destroying_ {false}; onShutdownCb scb {}; - // Default remote adresses - std::vector<IpAddr> iceDefaultRemoteAddr_ {}; + // Default remote addresses + std::vector<IpAddr> iceDefaultRemoteAddr_; }; //============================================================================== /** - * Add stun/turn servers or default host as candidates + * Add stun/turn configuration or default host as candidates */ -static void -add_stun_server(pj_ice_strans_cfg& cfg, int af) -{ - if (cfg.stun_tp_cnt >= PJ_ICE_MAX_STUN) - throw std::runtime_error("Too many STUN servers"); - auto& stun = cfg.stun_tp[cfg.stun_tp_cnt++]; - - pj_ice_strans_stun_cfg_default(&stun); - stun.cfg.max_pkt_size = STUN_MAX_PACKET_SIZE; - stun.af = af; - stun.conn_type = cfg.stun.conn_type; - - JAMI_DBG("[ice (%s)] added host stun server", (cfg.protocol == PJ_ICE_TP_TCP ? "TCP" : "UDP")); -} static void add_stun_server(pj_pool_t& pool, pj_ice_strans_cfg& cfg, const StunServerInfo& info) { if (cfg.stun_tp_cnt >= PJ_ICE_MAX_STUN) - throw std::runtime_error("Too many STUN servers"); + throw std::runtime_error("Too many STUN configurations"); IpAddr ip {info.uri}; @@ -322,6 +300,9 @@ IceTransport::Impl::Impl(const char* name, , thread_() , iceDefaultRemoteAddr_(component_count) { + JAMI_DBG("[ice:%p] Creating IceTransport session for \"%s\" - comp count %u - as a %s", + this, name, component_count, master ? "master" : "slave"); + sip_utils::register_thread(); if (options.upnpEnable) upnp_.reset(new upnp::Controller(false)); @@ -346,9 +327,8 @@ IceTransport::Impl::Impl(const char* name, peerChannels_.resize(component_count_ + 1); - // Add local hosts (IPv4, IPv6) as stun candidates - add_stun_server(config_, pj_AF_INET6()); - add_stun_server(config_, pj_AF_INET()); + requestUpnpMappings(); + setupHostAndUpnpCandidates(); pool_.reset( pj_pool_create(iceTransportFactory.getPoolFactory(), "IceTransport.pool", 512, 512, NULL)); @@ -438,7 +418,7 @@ IceTransport::Impl::Impl(const char* name, } }); - // Init to invalid adresses + // Init to invalid addresses iceDefaultRemoteAddr_.reserve(component_count); } @@ -580,7 +560,6 @@ IceTransport::Impl::onComplete(pj_ice_strans* ice_st, pj_ice_strans_op op, pj_st setInitiatorSession(); else setSlaveSession(); - selectUPnPIceCandidates(); } if (op == PJ_ICE_STRANS_OP_INIT and on_initdone_cb_) @@ -729,206 +708,148 @@ IceTransport::Impl::createIceSession(pj_ice_sess_role role) return true; } -std::vector<IceTransport::Impl::LocalCandidate> -IceTransport::Impl::getLocalICECandidates(unsigned comp_id) const +bool +IceTransport::Impl::addStunConfig(int af) { - std::vector<LocalCandidate> cand_addrs; - pj_ice_sess_cand cand[PJ_ARRAY_SIZE(cand_)]; - unsigned cand_cnt = PJ_ARRAY_SIZE(cand); - - if (pj_ice_strans_enum_cands(icest_.get(), comp_id, &cand_cnt, cand) != PJ_SUCCESS) { - JAMI_ERR("[ice:%p] pj_ice_strans_enum_cands() failed", this); - return cand_addrs; + if (config_.stun_tp_cnt >= PJ_ICE_MAX_STUN) { + JAMI_ERR("Max number of STUN configurations reached (%i)", PJ_ICE_MAX_STUN); + return false; } - for (unsigned i = 0; i < cand_cnt; ++i) { - cand_addrs.push_back({cand[i].addr, cand[i].transport}); + if (af != pj_AF_INET() and af != pj_AF_INET6()) { + JAMI_ERR("Invalid address familly (%i)", af); + return false; } - return cand_addrs; + auto& stun = config_.stun_tp[config_.stun_tp_cnt++]; + + pj_ice_strans_stun_cfg_default(&stun); + stun.cfg.max_pkt_size = STUN_MAX_PACKET_SIZE; + stun.af = af; + stun.conn_type = config_.stun.conn_type; + + JAMI_DBG("[ice:%p)] added host stun server for %s transport", + this, config_.protocol == PJ_ICE_TP_TCP ? "TCP" : "UDP"); + + return true; } void -IceTransport::Impl::addReflectiveCandidate(int comp_id, - const IpAddr& base, - const IpAddr& addr, - const pj_ice_cand_transport& transport) -{ - // HACK-HACK-HACK-HACK-HACK-HACK-HACK-HACK-HACK-HACK-HACK-HACK-HACK-HACK-HACK-HACK-HACK-HACK - // WARNING: following implementation is a HACK of PJNATH !! - // ice_strans doesn't have any API that permit to inject ICE any kind of - // candidates. So, the hack consists in accessing hidden ICE session using a - // patched PJPNATH library with a new API exposing this session - // (pj_ice_strans_get_ice_sess). Then call pj_ice_sess_add_cand() with a - // carfully forged candidate: the transport_id field uses an index in ICE - // transport STUN servers array corresponding to a STUN server with the same - // address familly. This implies we hope they'll not be modification of - // transport_id meaning in future and no conflics with the borrowed STUN - // config. - // HACK-HACK-HACK-HACK-HACK-HACK-HACK-HACK-HACK-HACK-HACK-HACK-HACK-HACK-HACK-HACK-HACK-HACK - sip_utils::register_thread(); +IceTransport::Impl::requestUpnpMappings() +{ + // Must be called once ! - // borrowed from pjproject/pjnath/ice_strans.c, modified to be C++11'ized. - static auto CREATE_TP_ID = [](pj_uint8_t type, pj_uint8_t idx) { - return (pj_uint8_t)((type << 6) | idx); - }; - static constexpr int SRFLX_PREF = 65535; - static constexpr int TP_STUN = 1; - - // find a compatible STUN host with same address familly, normally all system - // enabled host addresses are represented, so we expect to always found this - // host - int idx = -1; - auto af = addr.getFamily(); - if (af == AF_UNSPEC) { - JAMI_ERR("[ice:%p] Unable to add reflective IP %s: unknown address familly", - this, - addr.toString().c_str()); + IpAddr localIp {}; + IpAddr publicIp {}; + + std::lock_guard<std::mutex> lock(upnpMutex_); + + if (upnp_ == nullptr) return; - } - for (unsigned i = 0; i < config_.stun_tp_cnt; ++i) { - if (config_.stun_tp[i].af == af) { - idx = i; - break; + auto transport = isTcpEnabled() ? PJ_CAND_TCP_PASSIVE : PJ_CAND_UDP; + auto portType = transport == PJ_CAND_UDP ? PortType::UDP : PortType::TCP; + localIp = upnp_->getLocalIP(); + publicIp = upnp_->getExternalIP(); + + // Request upnp mapping for each component. + for (unsigned compId = 1; compId <= component_count_; compId++) { + // Set port number to 0 to get any available port. + Mapping map {0, 0, portType}; + // Request the mapping + uint16_t port = upnp_->requestMappingAdd( + [this, map](uint16_t allocatedPort, bool success) { + if (success) { + JAMI_DBG("[ice:%p]: Successfully allocated port %u [%s]", + this, allocatedPort, map.getTypeStr().c_str()); + } else { + JAMI_WARN("[ice:%p]: Could not allocate for %s transport", + this, map.getTypeStr().c_str()); + } + }, + map); + + // Note that even if the returned port number is valid, the mapping + // might not be readily available. This is can happen if there is no + // port already provisioned. In this case a new mapping is requested + // and may take time for the IGD to grant the mapping. + // Hopefully, the mapping will be ready when the ICE connectivity + // checks are performed. + if (port > 0) { + std::lock_guard<std::mutex> lock(upnpMappingsMutex_); + upnpMappings_.emplace_back(std::make_unique<Mapping>(port, port, portType)); } } - if (idx < 0) { - JAMI_ERR("[ice:%p] Unable to add reflective IP %s: no suitable local STUN " - "host found", - this, - addr.toString().c_str()); +} + +void +IceTransport::Impl::setupHostAndUpnpCandidates() +{ + + JAMI_DBG("[ice:%p]: Setup host and UPNP candidates", this); + + // STUN configs layout: + // - index 0 : host IPv4 + // - index 1 : host IPv6 + // - index 2 : upnp/srflx IPv4. + + config_.stun_tp_cnt = 0; + if (not addStunConfig(pj_AF_INET())) + return; + if (not addStunConfig(pj_AF_INET6())) return; + + IpAddr localIp {}; + IpAddr publicIp {}; + + { + std::lock_guard<std::mutex> lock(upnpMutex_); + if (upnp_ == nullptr) + return; + localIp = upnp_->getLocalIP(); + publicIp = upnp_->getExternalIP(); } - pj_ice_sess_cand cand; - - cand.type = PJ_ICE_CAND_TYPE_SRFLX; - cand.status = PJ_EPENDING; // not used - cand.comp_id = comp_id; - cand.transport_id = CREATE_TP_ID(TP_STUN, idx); // HACK!! - cand.local_pref = SRFLX_PREF; // reflective - cand.transport = transport; - /* cand.foundation = ? */ - /* cand.prio = calculated by ice session */ - /* make base and addr the same since we're not going through a server */ - pj_sockaddr_cp(&cand.base_addr, base.pjPtr()); - pj_sockaddr_cp(&cand.addr, addr.pjPtr()); - pj_sockaddr_cp(&cand.rel_addr, &cand.base_addr); - pj_ice_calc_foundation(pool_.get(), &cand.foundation, cand.type, &cand.base_addr); - - std::lock_guard<std::mutex> lk(iceMutex_); - if (!icest_) + // Add config for UPNP candidates. + if (not addStunConfig(pj_AF_INET())) return; - auto ret = pj_ice_sess_add_cand(pj_ice_strans_get_ice_sess(icest_.get()), - cand.comp_id, - cand.transport_id, - cand.type, - cand.local_pref, - &cand.foundation, - &cand.addr, - &cand.base_addr, - &cand.rel_addr, - pj_sockaddr_get_len(&cand.addr), - NULL, - cand.transport); - - if (ret != PJ_SUCCESS) { - last_errmsg_ = sip_utils::sip_strerror(ret); - JAMI_ERR("[ice:%p] pj_ice_sess_add_cand failed with error %d: %s", - this, - ret, - last_errmsg_.c_str()); - JAMI_ERR("[ice:%p] failed to add candidate for comp_id=%d : %s : %s", - this, - comp_id, - base.toString().c_str(), - addr.toString().c_str()); - } else { - JAMI_DBG("[ice:%p] succeed to add candidate for comp_id=%d : %s : %s", - this, - comp_id, - base.toString().c_str(), - addr.toString().c_str()); - } -} -void -IceTransport::Impl::selectUPnPIceCandidates() -{ - // For every component, get the candidate(s) - // Create a port mapping either with that port, or with an available port - // Add candidate with that port and public IP - std::lock_guard<std::mutex> lk(upnpMutex_); - if (upnp_) { - auto publicIp = upnp_->getExternalIP(); - if (not publicIp) { - JAMI_WARN("[ice:%p] Could not determine public IP for ICE candidates", this); - return; - } - auto localIp = upnp_->getLocalIP(); - if (not localIp) { - JAMI_WARN("[ice:%p] Could not determine local IP for ICE candidates", this); - return; - } - // Use local list to store needed ports with their corresponding port type. - auto upnpIceCntr = 0; - for (unsigned comp_id = 1; comp_id <= component_count_; ++comp_id) { - auto candidates = getLocalICECandidates(comp_id); - for (const auto& candidate : candidates) { - if (candidate.transport == PJ_CAND_TCP_ACTIVE) - continue; // We don't need to map port 9. - localIp.setPort(candidate.addr.getPort()); - if (candidate.addr != localIp) - continue; - uint16_t port = candidate.addr.getPort(); - auto portType = candidate.transport == PJ_CAND_UDP ? upnp::PortType::UDP - : upnp::PortType::TCP; - // Request port - upnpIceCntr++; - JAMI_DBG("[ice:%p] UPnP: Trying to open port %d for ICE comp %d/%d and adding " - "candidate with public IP", - this, - port, - upnpIceCntr, - component_count_); - upnp_->requestMappingAdd( - [this, candidate, comp_id](uint16_t port_used, bool success) { - if (upnp_) { - std::lock_guard<std::mutex> lk(upnpMutex_); - auto publicIp = upnp_->getExternalIP(); - if (not publicIp) { - JAMI_WARN( - "[ice:%p] Could not determine public IP for ICE candidates", - this); - return; - } - auto localIp = upnp_->getLocalIP(); - if (not localIp) { - JAMI_WARN( - "[ice:%p] Could not determine local IP for ICE candidates", - this); - return; - } - if (success and port_used == candidate.addr.getPort()) { - publicIp.setPort(port_used); - addReflectiveCandidate(comp_id, - candidate.addr, - publicIp, - candidate.transport); - } - } - }, - port, - portType, - true); - } + assert(config_.stun_tp_cnt > 0 && config_.stun_tp_cnt < PJ_ICE_MAX_STUN); + auto& stun = config_.stun_tp[config_.stun_tp_cnt - 1]; + + std::lock_guard<std::mutex> lock(upnpMappingsMutex_); + + // Abort if not enough mapped ports. + if (upnpMappings_.size() < component_count_) + return; + + // Reset user mapping counter. + stun.cfg.user_mapping_cnt = 0; + + // Add allocated mappings + for (unsigned compIdx = 0; compIdx < component_count_; compIdx++) { + if (stun.cfg.user_mapping_cnt < PJ_ICE_MAX_COMP) { + localIp.setPort(upnpMappings_[compIdx]->getPortInternal()); + publicIp.setPort(upnpMappings_[compIdx]->getPortExternal()); + + JAMI_DBG("[ice:%p]: Set upnp mapping %s (%s) -> %s (%s) for comp %u", + this, + localIp.toString(true).c_str(), + localIp.getFamily() == pj_AF_INET() ? "IPv4" : localIp.getFamily() == pj_AF_INET6() ? "IPv6" : "unknown", + publicIp.toString(true).c_str(), + publicIp.getFamily() == pj_AF_INET() ? "IPv4" : localIp.getFamily() == pj_AF_INET6() ? "IPv6" : "unknown", + compIdx+1); + + pj_sockaddr_cp(&stun.cfg.user_mapping[compIdx].mapped_addr, publicIp.pjPtr()); + pj_sockaddr_cp(&stun.cfg.user_mapping[compIdx].local_addr, localIp.pjPtr()); + + stun.cfg.user_mapping_cnt++; } } } void -IceTransport::Impl::setDefaultRemoteAddress(int comp_id, const IpAddr& addr) +IceTransport::Impl::setDefaultRemoteAddress(unsigned comp_id, const IpAddr& addr) { // Component ID must be valid. assert(comp_id < component_count_); @@ -939,7 +860,7 @@ IceTransport::Impl::setDefaultRemoteAddress(int comp_id, const IpAddr& addr) } const IpAddr& -IceTransport::Impl::getDefaultRemoteAddress(int comp_id) const +IceTransport::Impl::getDefaultRemoteAddress(unsigned comp_id) const { // Component ID must be valid. assert(comp_id < component_count_); @@ -1051,20 +972,20 @@ bool IceTransport::start(const Attribute& rem_attrs, const std::vector<IceCandidate>& rem_candidates) { if (not isInitialized()) { - JAMI_ERR("[ice:%p] not initialized transport", this); + JAMI_ERR("[ice:%p] not initialized transport", pimpl_.get()); pimpl_->is_stopped_ = true; return false; } // pj_ice_strans_start_ice crashes if remote candidates array is empty if (rem_candidates.empty()) { - JAMI_ERR("[ice:%p] start failed: no remote candidates", this); + JAMI_ERR("[ice:%p] start failed: no remote candidates", pimpl_.get()); pimpl_->is_stopped_ = true; return false; } pj_str_t ufrag, pwd; - JAMI_DBG("[ice:%p] negotiation starting (%zu remote candidates)", this, rem_candidates.size()); + JAMI_DBG("[ice:%p] negotiation starting (%zu remote candidates)", pimpl_.get(), rem_candidates.size()); auto status = pj_ice_strans_start_ice(pimpl_->icest_.get(), pj_strset(&ufrag, (char*) rem_attrs.ufrag.c_str(), @@ -1076,7 +997,7 @@ IceTransport::start(const Attribute& rem_attrs, const std::vector<IceCandidate>& rem_candidates.data()); if (status != PJ_SUCCESS) { pimpl_->last_errmsg_ = sip_utils::sip_strerror(status); - JAMI_ERR("[ice:%p] start failed: %s", this, pimpl_->last_errmsg_.c_str()); + JAMI_ERR("[ice:%p] start failed: %s", pimpl_.get(), pimpl_->last_errmsg_.c_str()); pimpl_->is_stopped_ = true; return false; } @@ -1088,12 +1009,12 @@ bool IceTransport::start(const SDP& sdp) { if (not isInitialized()) { - JAMI_ERR("[ice:%p] not initialized transport", this); + JAMI_ERR("[ice:%p] not initialized transport", pimpl_.get()); pimpl_->is_stopped_ = true; return false; } - JAMI_DBG("[ice:%p] negotiation starting (%zu remote candidates)", this, sdp.candidates.size()); + JAMI_DBG("[ice:%p] negotiation starting (%zu remote candidates)", pimpl_.get(), sdp.candidates.size()); pj_str_t ufrag, pwd; std::vector<IceCandidate> rem_candidates; @@ -1115,7 +1036,7 @@ IceTransport::start(const SDP& sdp) rem_candidates.data()); if (status != PJ_SUCCESS) { pimpl_->last_errmsg_ = sip_utils::sip_strerror(status); - JAMI_ERR("[ice:%p] start failed: %s", this, pimpl_->last_errmsg_.c_str()); + JAMI_ERR("[ice:%p] start failed: %s", pimpl_.get(), pimpl_->last_errmsg_.c_str()); pimpl_->is_stopped_ = true; return false; } @@ -1134,7 +1055,7 @@ IceTransport::stop() auto status = pj_ice_strans_stop_ice(pimpl_->icest_.get()); if (status != PJ_SUCCESS) { pimpl_->last_errmsg_ = sip_utils::sip_strerror(status); - JAMI_ERR("ICE stop failed: %s", pimpl_->last_errmsg_.c_str()); + JAMI_ERR("[ice:%p] ICE stop failed: %s", pimpl_.get(), pimpl_->last_errmsg_.c_str()); return false; } } @@ -1189,7 +1110,7 @@ IceTransport::getLocalCandidates(unsigned comp_id) const return res; if (pj_ice_strans_enum_cands(pimpl_->icest_.get(), comp_id + 1, &cand_cnt, cand) != PJ_SUCCESS) { - JAMI_ERR("[ice:%p] pj_ice_strans_enum_cands() failed", this); + JAMI_ERR("[ice:%p] pj_ice_strans_enum_cands() failed", pimpl_.get()); return res; } } @@ -1238,34 +1159,6 @@ IceTransport::getLocalCandidates(unsigned comp_id) const return res; } -bool -IceTransport::registerPublicIP(unsigned compId, const IpAddr& publicIP) -{ - if (not isInitialized()) { - JAMI_ERR("[ice:%p] registerPublicIP() called on non initialized transport", this); - return false; - } - - // Find the local candidate corresponding to local host, - // then register a rflx candidate using given public address - // and this local address as base. It's port is used for both address - // even if on the public side it have strong probabilities to not exist. - // But as this candidate is made after initialization, it's not used during - // negotiation, only to exchanged candidates between peers. - auto localIP = ip_utils::getLocalAddr(publicIP.getFamily()); - auto pubIP = publicIP; - for (const auto& cand : pimpl_->getLocalICECandidates(compId)) { - auto port = cand.addr.getPort(); - localIP.setPort(port); - if (cand.addr != localIP) - continue; - pubIP.setPort(port); - pimpl_->addReflectiveCandidate(compId, cand.addr, pubIP, cand.transport); - return true; - } - return false; -} - std::vector<uint8_t> IceTransport::packIceMsg(uint8_t version) const { @@ -1330,7 +1223,7 @@ IceTransport::getCandidateFromSDP(const std::string& line, IceCandidate& cand) c type, tcp_type); if (cnt != 7 && cnt != 8) { - JAMI_ERR("[ice:%p] Invalid ICE candidate line", this); + JAMI_ERR("[ice:%p] Invalid ICE candidate line", pimpl_.get()); return false; } @@ -1349,7 +1242,7 @@ IceTransport::getCandidateFromSDP(const std::string& line, IceCandidate& cand) c else if (strcmp(type, "relay") == 0) cand.type = PJ_ICE_CAND_TYPE_RELAYED; else { - JAMI_WARN("[ice:%p] invalid remote candidate type '%s'", this, type); + JAMI_WARN("[ice:%p] invalid remote candidate type '%s'", pimpl_.get(), type); return false; } @@ -1361,7 +1254,7 @@ IceTransport::getCandidateFromSDP(const std::string& line, IceCandidate& cand) c else if (strcmp(tcp_type, "so") == 0) cand.transport = PJ_CAND_TCP_SO; else { - JAMI_WARN("[ice:%p] invalid transport type type '%s'", this, tcp_type); + JAMI_WARN("[ice:%p] invalid transport type type '%s'", pimpl_.get(), tcp_type); return false; } } else { @@ -1382,7 +1275,7 @@ IceTransport::getCandidateFromSDP(const std::string& line, IceCandidate& cand) c pj_sockaddr_init(af, &cand.addr, NULL, 0); status = pj_sockaddr_set_str_addr(af, &cand.addr, &tmpaddr); if (status != PJ_SUCCESS) { - JAMI_WARN("[ice:%p] invalid IP address '%s'", this, ipaddr); + JAMI_WARN("[ice:%p] invalid IP address '%s'", pimpl_.get(), ipaddr); return false; } @@ -1450,7 +1343,7 @@ IceTransport::send(int comp_id, const unsigned char* buf, size_t len) auto remote = getRemoteAddress(comp_id); if (!remote) { - JAMI_ERR("[ice:%p] can't find remote address for component %d", this, comp_id); + JAMI_ERR("[ice:%p] can't find remote address for component %d", pimpl_.get(), comp_id); errno = EINVAL; return -1; } @@ -1473,7 +1366,7 @@ IceTransport::send(int comp_id, const unsigned char* buf, size_t len) errno = EAGAIN; } else { pimpl_->last_errmsg_ = sip_utils::sip_strerror(status); - JAMI_ERR("[ice:%p] ice send failed: %s", this, pimpl_->last_errmsg_.c_str()); + JAMI_ERR("[ice:%p] ice send failed: %s", pimpl_.get(), pimpl_->last_errmsg_.c_str()); errno = EIO; } return -1; @@ -1489,7 +1382,7 @@ IceTransport::waitForInitialization(std::chrono::milliseconds timeout) if (!pimpl_->iceCV_.wait_for(lk, timeout, [this] { return pimpl_->threadTerminateFlags_ or pimpl_->_isInitialized() or pimpl_->_isFailed(); })) { - JAMI_WARN("[ice:%p] waitForInitialization: timeout", this); + JAMI_WARN("[ice:%p] waitForInitialization: timeout", pimpl_.get()); return -1; } return not(pimpl_->threadTerminateFlags_ or pimpl_->_isFailed()); @@ -1502,7 +1395,7 @@ IceTransport::waitForNegotiation(std::chrono::milliseconds timeout) if (!pimpl_->iceCV_.wait_for(lk, timeout, [this] { return pimpl_->threadTerminateFlags_ or pimpl_->_isRunning() or pimpl_->_isFailed(); })) { - JAMI_WARN("[ice:%p] waitForIceNegotiation: timeout", this); + JAMI_WARN("[ice:%p] waitForIceNegotiation: timeout", pimpl_.get()); return -1; } return not(pimpl_->threadTerminateFlags_ or pimpl_->_isFailed()); @@ -1560,7 +1453,7 @@ IceTransport::parseSDPList(const std::vector<uint8_t>& msg) bool IceTransport::isTCPEnabled() { - return pimpl_->config_.protocol == PJ_ICE_TP_TCP; + return pimpl_->isTcpEnabled(); } ICESDP diff --git a/src/ice_transport.h b/src/ice_transport.h index e5db678f6da737ee60fe500c675ac243184bc75c..6427b5a17079753daab681b84d9105e082983958 100644 --- a/src/ice_transport.h +++ b/src/ice_transport.h @@ -195,8 +195,6 @@ public: IpAddr getDefaultLocalAddress() const { return getLocalAddress(0); } - bool registerPublicIP(unsigned compId, const IpAddr& publicIP); - /** * Return ICE session attributes */ diff --git a/src/jamidht/jamiaccount.cpp b/src/jamidht/jamiaccount.cpp index d9f51ec17fdccadab094ed9a6d58f2d8b6f8cc10..676d8c6ba18beef800ad356c8f857b6c3e772f2c 100644 --- a/src/jamidht/jamiaccount.cpp +++ b/src/jamidht/jamiaccount.cpp @@ -1799,10 +1799,22 @@ JamiAccount::registerAsyncOps() loadCachedProxyServer([onLoad](const std::string&) { onLoad(); }); - if (upnp_) { + if (upnp_!= nullptr) { + // Release current mapping if any. + if (dhtPortMapping_.getPortExternal()) { + upnp_->requestMappingRemove(dhtPortMapping_); + dhtPortMapping_ = upnp::Mapping {}; + } + upnp::Mapping map {dhtPort_, dhtPort_, upnp::PortType::UDP, "JAMI-DHT"}; upnp_->requestMappingAdd( [this, onLoad, update = std::make_shared<bool>(false)](uint16_t port_used, bool success) { + if (success) { + dhtPortMapping_ = upnp::Mapping { port_used, port_used, upnp::PortType::UDP, "DHT Port" }; + } else { + dhtPortMapping_ = upnp::Mapping {}; + } + auto oldPort = static_cast<in_port_t>(dhtPortUsed_); auto newPort = success ? port_used : dhtPort_; if (*update) { @@ -1815,21 +1827,19 @@ JamiAccount::registerAsyncOps() } else { *update = true; if (success) - JAMI_WARN("[Account %s] Starting DHT on port %u", + JAMI_DBG("[Account %s] Starting DHT on port %u", getAccountID().c_str(), newPort); else - JAMI_WARN("[Account %s] Failed to open port %u: starting DHT anyways", + JAMI_WARN("[Account %s] Failed to open port %u: starting DHT anyway", getAccountID().c_str(), oldPort); - onLoad(); } }, - dhtPort_, - upnp::PortType::UDP, - false); - } else - onLoad(); + map); + } + // DHT will be started regardless if UPNP is available and/or successful. + onLoad(); } void @@ -2500,8 +2510,11 @@ JamiAccount::doUnregister(std::function<void(bool)> released_cb) dht_->join(); - if (upnp_) - upnp_->requestMappingRemove(static_cast<in_port_t>(dhtPortUsed_), upnp::PortType::UDP); + // Release current upnp mapping if any. + if (upnp_ and dhtPortMapping_.getPortExternal()) { + upnp_->requestMappingRemove(dhtPortMapping_); + dhtPortMapping_ = upnp::Mapping {}; + } lock.unlock(); @@ -3262,8 +3275,6 @@ JamiAccount::registerDhtAddress(IceTransport& ice) { const auto reg_addr = [&](IceTransport& ice, const IpAddr& ip) { JAMI_DBG("[Account %s] using public IP: %s", getAccountID().c_str(), ip.toString().c_str()); - for (unsigned compId = 1; compId <= ice.getComponentCount(); ++compId) - ice.registerPublicIP(compId, ip); return ip; }; diff --git a/src/jamidht/jamiaccount.h b/src/jamidht/jamiaccount.h index 163a25393e839126156b1fe807e9b6d1253ce0ab..e1148c47825cc82b1ae252354bec71640f7f86ed 100644 --- a/src/jamidht/jamiaccount.h +++ b/src/jamidht/jamiaccount.h @@ -41,6 +41,8 @@ #include <opendht/dhtrunner.h> #include <opendht/default_types.h> +#include "upnp/protocol/mapping.h" + #include <pjsip/sip_types.h> #include <vector> @@ -633,6 +635,8 @@ private: * DHT port preference */ in_port_t dhtPort_ {}; + /* Current port mapping */ + upnp::Mapping dhtPortMapping_ {}; bool dhtPeerDiscovery_ {false}; diff --git a/src/sip/sipaccount.cpp b/src/sip/sipaccount.cpp index 6affb9688fd2c8469910b582d8ef3d0b988216ad..56ec6abb84570f58449bcd96799c923d471cd30a 100644 --- a/src/sip/sipaccount.cpp +++ b/src/sip/sipaccount.cpp @@ -739,12 +739,14 @@ SIPAccount::getVolatileAccountDetails() const void SIPAccount::mapPortUPnP() { + upnp::Mapping map {publishedPort_, localPort_, upnp::PortType::UDP, "SIP Account Port"}; + upnp_->requestMappingAdd( [this](uint16_t port_used, bool success) { auto oldPort = static_cast<in_port_t>(publishedPortUsed_); auto newPort = success ? port_used : publishedPort_; if (not success and not isRegistered()) { - JAMI_WARN("[Account %s] Failed to open port %u: registering SIP account anyways", + JAMI_WARN("[Account %s] Failed to open port %u: registering SIP account anyway", getAccountID().c_str(), oldPort); doRegister1_(); @@ -756,7 +758,7 @@ SIPAccount::mapPortUPnP() getAccountID().c_str(), newPort); else - JAMI_WARN("[Account %s] SIP port changed to %u: reregistering SIP account", + JAMI_WARN("[Account %s] SIP port changed to %u: re-registering SIP account", getAccountID().c_str(), newPort); publishedPortUsed_ = newPort; @@ -765,10 +767,7 @@ SIPAccount::mapPortUPnP() } doRegister1_(); }, - publishedPort_, - jami::upnp::PortType::UDP, - false, - localPort_); + map); } void diff --git a/src/sip/sipcall.cpp b/src/sip/sipcall.cpp index 602cf9a0903a8075d2cbb8541dcb04cd995f10bb..df7ea0ea43c72dc34e23e763a6135642d32c13ff 100644 --- a/src/sip/sipcall.cpp +++ b/src/sip/sipcall.cpp @@ -1062,10 +1062,6 @@ SIPCall::setupLocalSDPFromIce() JAMI_ERR("No account detected"); return; } - if (const auto& ip = account->getPublishedIpAddress()) { - for (unsigned compId = 1; compId <= media_tr->getComponentCount(); ++compId) - media_tr->registerPublicIP(compId, ip); - } JAMI_WARN("[call:%s] fill SDP with ICE transport %p", getCallId().c_str(), media_tr); sdp_->addIceAttributes(media_tr->getLocalAttributes()); @@ -1462,10 +1458,14 @@ SIPCall::openPortsUPnP() * and marked as used, the old port should be "released" */ JAMI_DBG("[call:%s] opening ports via UPNP for SDP session", getCallId().c_str()); + upnp::Mapping mapAudioRtp {sdp_->getLocalAudioPort(), sdp_->getLocalAudioPort(), + upnp::PortType::UDP, "SIP Audio RTP port"}; upnp_->requestMappingAdd( [this](uint16_t, bool success) { if (!success) return; + upnp::Mapping mapAudioRtcp {sdp_->getLocalAudioControlPort(), sdp_->getLocalAudioControlPort(), + upnp::PortType::UDP, "SIP Audio RTCP port"}; upnp_->requestMappingAdd( [this](uint16_t, bool success) { if (!success) @@ -1473,18 +1473,18 @@ SIPCall::openPortsUPnP() sdp_->setLocalPublishedAudioPorts(sdp_->getLocalAudioPort(), sdp_->getLocalAudioControlPort()); }, - sdp_->getLocalAudioControlPort(), - upnp::PortType::UDP, - true); + mapAudioRtcp); }, - sdp_->getLocalAudioPort(), - upnp::PortType::UDP, - true); + mapAudioRtp); #ifdef ENABLE_VIDEO + upnp::Mapping mapVideoRtp {sdp_->getLocalVideoPort(), sdp_->getLocalVideoPort(), + upnp::PortType::UDP, "SIP Video RTP port"}; upnp_->requestMappingAdd( [this](uint16_t, bool success) { if (!success) return; + upnp::Mapping mapVideoRtcp {sdp_->getLocalVideoControlPort(), sdp_->getLocalVideoControlPort(), + upnp::PortType::UDP, "SIP Video RTCP port"}; upnp_->requestMappingAdd( [this](uint16_t, bool success) { if (!success) @@ -1492,13 +1492,9 @@ SIPCall::openPortsUPnP() sdp_->setLocalPublishedVideoPorts(sdp_->getLocalVideoPort(), sdp_->getLocalVideoControlPort()); }, - sdp_->getLocalVideoControlPort(), - upnp::PortType::UDP, - true); + mapVideoRtcp); }, - sdp_->getLocalVideoPort(), - upnp::PortType::UDP, - true); + mapVideoRtp); #endif } } @@ -1686,6 +1682,11 @@ SIPCall::initIceMediaTransport(bool master, dht::ThreadPool::io().run([ice = std::make_shared<decltype(tmpMediaTransport_)>( std::move(tmpMediaTransport_))] {}); tmpMediaTransport_ = std::move(transport); + if (tmpMediaTransport_ != nullptr) { + JAMI_DBG("[call:%s] Successfully created media ICE transport", getCallId().c_str()); + } else { + JAMI_ERR("[call:%s] Failed to create media ICE transport", getCallId().c_str()); + } return static_cast<bool>(tmpMediaTransport_); } diff --git a/src/sip/sipcall.h b/src/sip/sipcall.h index 9e64534ab5c7e4948c09abae66efc786aa3d6d96..6e032a47f667e275a83a61896f309cc10be880d1 100644 --- a/src/sip/sipcall.h +++ b/src/sip/sipcall.h @@ -362,7 +362,7 @@ private: char contactBuffer_[PJSIP_MAX_URL_SIZE] {}; pj_str_t contactHeader_ {contactBuffer_, 0}; - std::unique_ptr<jami::upnp::Controller> upnp_; + std::shared_ptr<jami::upnp::Controller> upnp_; /** Local audio port, as seen by me. */ unsigned int localAudioPort_ {0}; diff --git a/src/upnp/protocol/global_mapping.h b/src/upnp/protocol/global_mapping.h index b29b70f0e5d37a5f7f7cb7f39fd67ac19db28206..d950e2506a6f7f205e59ae08e599119a8a75a6d2 100644 --- a/src/upnp/protocol/global_mapping.h +++ b/src/upnp/protocol/global_mapping.h @@ -46,7 +46,6 @@ public: : Mapping(mapping.getPortExternal(), mapping.getPortInternal(), mapping.getType(), - mapping.isUnique(), mapping.getDescription()) , users(users) {} diff --git a/src/upnp/protocol/igd.cpp b/src/upnp/protocol/igd.cpp index 3be55c33a29ab0f58b6ab2b629a36c13e123a062..ee752857163ca259894f8cbb60417c32463e2d88 100644 --- a/src/upnp/protocol/igd.cpp +++ b/src/upnp/protocol/igd.cpp @@ -67,8 +67,7 @@ IGD::getMapping(in_port_t externalPort, upnp::PortType type) const if (it->first == externalPort) { return Mapping(it->second.getPortExternal(), it->second.getPortInternal(), - it->second.getType(), - it->second.isUnique()); + it->second.getType()); } } return Mapping(0, 0); diff --git a/src/upnp/protocol/igd.h b/src/upnp/protocol/igd.h index 22ae20249b7f1b7e88155e521e5a45065753ec46..d999a8d646fa520cdb6b027f8b4fee7346174140 100644 --- a/src/upnp/protocol/igd.h +++ b/src/upnp/protocol/igd.h @@ -89,4 +89,4 @@ private: }; } // namespace upnp -} // namespace jami \ No newline at end of file +} // namespace jami diff --git a/src/upnp/protocol/mapping.cpp b/src/upnp/protocol/mapping.cpp index a172a4eb234fb468b933ae9522a0f60c373aeed6..39574263977e5e2a0f27068e383cddd41325a4be 100644 --- a/src/upnp/protocol/mapping.cpp +++ b/src/upnp/protocol/mapping.cpp @@ -20,6 +20,7 @@ */ #include "mapping.h" +#include "logger.h" namespace jami { namespace upnp { @@ -27,13 +28,15 @@ namespace upnp { Mapping::Mapping(uint16_t portExternal, uint16_t portInternal, PortType type, - bool unique, - const std::string& description) + const std::string& description, + bool available) : portExternal_(portExternal) , portInternal_(portInternal) , type_(type) , description_(description) - , unique_(unique) {}; + , available_(available) + , open_(false) + {}; Mapping::Mapping(Mapping&& other) noexcept : @@ -45,7 +48,8 @@ Mapping::Mapping(Mapping&& other) noexcept , portInternal_(other.portInternal_) , type_(other.type_) , description_(std::move(other.description_)) - , unique_(other.unique_) + , available_(other.available_) + , open_(other.open_) { other.portExternal_ = 0; other.portInternal_ = 0; @@ -61,7 +65,8 @@ Mapping::Mapping(const Mapping& other) , portInternal_(other.portInternal_) , type_(other.type_) , description_(std::move(other.description_)) - , unique_(other.unique_) + , available_ (other.available_) + , open_ (other.open_) {} Mapping& @@ -74,6 +79,11 @@ Mapping::operator=(Mapping&& other) noexcept other.portInternal_ = 0; type_ = other.type_; description_ = std::move(other.description_); + available_ = other.available_; + other.available_ = false; + open_ = other.open_; + other.open_ = false; + #if HAVE_LIBNATPMP renewal_ = other.renewal_; #endif @@ -148,10 +158,45 @@ Mapping::operator>=(const Mapping& other) const noexcept return false; } +void +Mapping::setAvailable(bool val) +{ + JAMI_DBG("UPnP: Changing mapping %s state from %s to %s", + toString().c_str(), + available_ ? "AVAILABLE":"UNAVAILABLE", val ? "AVAILABLE":"UNAVAILABLE"); + + available_ = val; +} + +void +Mapping::setOpen(bool val) +{ + if (open_ == val) { + // This condition can occur if the NAT supports and responds + // to more than one protocol (i.e. UPNP-IGD and NAT-PMP). + JAMI_DBG("UPnP: Mapped port %s already %s", + toString().c_str(), open_ ? "OPEN":"CLOSED"); + return; + } + + JAMI_DBG("UPnP: Changing mapped port %s state from %s to %s", + toString().c_str(), + open_ ? "OPEN":"CLOSED", val ? "OPEN":"CLOSED"); + + open_ = val; +} + +void +Mapping::setDescription(const std::string& descr) +{ + description_ = descr; +} + std::string Mapping::toString() const { - return getPortExternalStr() + ":" + getPortInternalStr() + " " + getTypeStr(); + return getPortExternalStr() + ":" + getPortInternalStr() + " [" + getTypeStr() + "]" + + " \"" + description_.c_str() + "\""; } bool diff --git a/src/upnp/protocol/mapping.h b/src/upnp/protocol/mapping.h index 871a0eed8ee034ecc17a20c5ab6ab27e784598b6..10cccc5e23d59a99d570bfb5745e0214dea149ea 100644 --- a/src/upnp/protocol/mapping.h +++ b/src/upnp/protocol/mapping.h @@ -41,20 +41,20 @@ enum class PortType { UDP, TCP }; class Mapping { public: - constexpr static const char* UPNP_DEFAULT_MAPPING_DESCRIPTION = "RING"; + constexpr static const char* UPNP_DEFAULT_MAPPING_DESCRIPTION = "JAMI"; constexpr static uint16_t UPNP_PORT_MIN = 1024; constexpr static uint16_t UPNP_PORT_MAX = 65535; Mapping(uint16_t portExternal = 0, uint16_t portInternal = 0, PortType type = PortType::UDP, - bool unique = true, - const std::string& description = UPNP_DEFAULT_MAPPING_DESCRIPTION); + const std::string& description = UPNP_DEFAULT_MAPPING_DESCRIPTION, + bool available = false); Mapping(Mapping&& other) noexcept; Mapping(const Mapping& other); ~Mapping() = default; - Mapping& operator=(Mapping&&) noexcept; + Mapping& operator=(Mapping&& other) noexcept; bool operator==(const Mapping& other) const noexcept; bool operator!=(const Mapping& other) const noexcept; bool operator<(const Mapping& other) const noexcept; @@ -62,15 +62,21 @@ public: bool operator<=(const Mapping& other) const noexcept; bool operator>=(const Mapping& other) const noexcept; + void setPortExternal(uint16_t port) { portExternal_ = port; } uint16_t getPortExternal() const { return portExternal_; } std::string getPortExternalStr() const { return std::to_string(portExternal_); } + void setPortInternal(uint16_t port) { portInternal_ = port; } uint16_t getPortInternal() const { return portInternal_; } std::string getPortInternalStr() const { return std::to_string(portInternal_); } PortType getType() const { return type_; } std::string getTypeStr() const { return type_ == PortType::UDP ? "UDP" : "TCP"; } - bool isUnique() const { return unique_; } + static const std::string getTypeStr(PortType type) { return type == PortType::UDP ? "UDP" : "TCP"; } + bool isAvailable() const { return available_; } + void setAvailable(bool val); + bool isOpen() const {return open_;}; + void setOpen(bool val); std::string getDescription() const { return description_; } - + void setDescription(const std::string& descr); std::string toString() const; bool isValid() const; @@ -88,8 +94,12 @@ protected: uint16_t portInternal_; PortType type_; std::string description_; - bool unique_; + // Track if the mapping is available to use. + bool available_; + // This flag is set to true when a positive response is + // received from the IGD. + bool open_; }; } // namespace upnp -} // namespace jami \ No newline at end of file +} // namespace jami diff --git a/src/upnp/protocol/natpmp/nat_pmp.cpp b/src/upnp/protocol/natpmp/nat_pmp.cpp index 60a7df18e639cb0704d39b150a6128d0335a0107..9fd964b670432501cce0fe34081c8e4a93694f64 100644 --- a/src/upnp/protocol/natpmp/nat_pmp.cpp +++ b/src/upnp/protocol/natpmp/nat_pmp.cpp @@ -25,6 +25,7 @@ namespace upnp { constexpr static unsigned int ADD_MAP_LIFETIME {3600}; constexpr static unsigned int MAX_RESTART_SEARCH_RETRY {5}; +constexpr static auto TIMEOUT_BEFORE_READ_RETRY {std::chrono::milliseconds(300)}; NatPmp::NatPmp() { @@ -195,10 +196,9 @@ NatPmp::searchForIgd() } void -NatPmp::requestMappingAdd(IGD* igd, uint16_t port_external, uint16_t port_internal, PortType type) +NatPmp::requestMappingAdd(IGD* igd, const Mapping& mapping) { std::unique_lock<std::mutex> lk(validIgdMutex_); - Mapping mapping {port_external, port_internal, type}; if (pmpIGD_) { if (not igd->isMapInUse(mapping)) { if (pmpIGD_->publicIp_ == igd->publicIp_) { @@ -239,9 +239,12 @@ NatPmp::addPortMapping(Mapping& mapping, bool renew) } while (pmpRun_) { natpmpresp_t response; - std::this_thread::sleep_for(std::chrono::milliseconds(2)); + auto r = readnatpmpresponseorretry(&natpmpHdl_, &response); - if (r != NATPMP_TRYAGAIN) { + + if (r == NATPMP_TRYAGAIN) { + std::this_thread::sleep_for(std::chrono::milliseconds(TIMEOUT_BEFORE_READ_RETRY)); + } else { if (r < 0) { JAMI_ERR("NAT-PMP: Can't register port mapping %s: %s", mapping.toString().c_str(), @@ -251,11 +254,11 @@ NatPmp::addPortMapping(Mapping& mapping, bool renew) + std::chrono::seconds(response.pnu.newportmapping.lifetime / 2); if (pmpIGD_) { if (not renew) { - JAMI_WARN("NAT-PMP: Opened port %s", mapping.toString().c_str()); + JAMI_DBG("NAT-PMP: Opened port %s", mapping.toString().c_str()); lk2.unlock(); notifyContextPortOpenCb_(pmpIGD_->publicIp_, std::move(mapToAdd), true); } else { - JAMI_WARN("NAT-PMP: Renewed port %s", mapping.toString().c_str()); + JAMI_DBG("NAT-PMP: Renewed port %s", mapping.toString().c_str()); } pmpIGD_->addMapToRenew(std::move(mapping)); } @@ -308,12 +311,15 @@ NatPmp::removePortMapping(Mapping& mapping) } while (pmpRun_) { natpmpresp_t response; - std::this_thread::sleep_for(std::chrono::milliseconds(2)); + auto r = readnatpmpresponseorretry(&natpmpHdl_, &response); - if (r < 0 && r != NATPMP_TRYAGAIN) { + + if (r == NATPMP_TRYAGAIN) { + std::this_thread::sleep_for(std::chrono::milliseconds(TIMEOUT_BEFORE_READ_RETRY)); + } else if (r < 0) { JAMI_ERR("NAT-PMP: Can't unregister port mapping %s", mapping.toString().c_str()); break; - } else if (r != NATPMP_TRYAGAIN) { + } else { mapping.renewal_ = clock::now() + std::chrono::seconds(response.pnu.newportmapping.lifetime / 2); if (pmpIGD_) { @@ -364,13 +370,17 @@ NatPmp::searchForPmpIgd() } while (pmpRun_) { natpmpresp_t response; - std::this_thread::sleep_for(std::chrono::milliseconds(2)); + auto r = readnatpmpresponseorretry(&natpmpHdl_, &response); - std::unique_lock<std::mutex> lk2(validIgdMutex_); - if (r < 0 && r != NATPMP_TRYAGAIN) { + + if (r == NATPMP_TRYAGAIN) { + std::this_thread::sleep_for(std::chrono::milliseconds(TIMEOUT_BEFORE_READ_RETRY)); + } else if (r < 0) { + std::unique_lock<std::mutex> lk2(validIgdMutex_); pmpIGD_->renewal_ = clock::now() + std::chrono::minutes(5); break; - } else if (r != NATPMP_TRYAGAIN) { + } else { + std::unique_lock<std::mutex> lk2(validIgdMutex_); restartSearchRetry_ = 0; pmpIGD_->localIp_ = ip_utils::getLocalAddr(AF_INET); pmpIGD_->publicIp_ = IpAddr(response.pnu.publicaddress.addr); @@ -406,9 +416,12 @@ NatPmp::deleteAllPortMappings(int proto) while (pmpRun_) { natpmpresp_t response; - std::this_thread::sleep_for(std::chrono::milliseconds(2)); + auto r = readnatpmpresponseorretry(&natpmpHdl_, &response); - if (r != NATPMP_TRYAGAIN) { + + if (r == NATPMP_TRYAGAIN) { + std::this_thread::sleep_for(std::chrono::milliseconds(TIMEOUT_BEFORE_READ_RETRY)); + } else { if (r < 0) JAMI_ERR("NAT-PMP: Can't remove all port mappings: %s", getNatPmpErrorStr(r)); break; diff --git a/src/upnp/protocol/natpmp/nat_pmp.h b/src/upnp/protocol/natpmp/nat_pmp.h index 4cdafc812e7956b1b4da04981f1354a18499f5ac..7aaaba05c393df11d6be772afa827139ef5e4f57 100644 --- a/src/upnp/protocol/natpmp/nat_pmp.h +++ b/src/upnp/protocol/natpmp/nat_pmp.h @@ -55,7 +55,10 @@ public: ~NatPmp(); // Returns the protocol type. - Type getType() const override { return Type::NAT_PMP; } + Type getProtocol() const override { return Type::NAT_PMP; } + + // Get protocol type as string. + virtual std::string getProtocolName() const override { return {"NAT_PMP"};} // Notifies a change in network. void clearIgds() override; @@ -64,10 +67,7 @@ public: void searchForIgd() override; // Tries to add mapping. - void requestMappingAdd(IGD* igd, - uint16_t port_external, - uint16_t port_internal, - PortType type) override; + void requestMappingAdd(IGD* igd, const Mapping& mapping) override; // Removes a mapping. void requestMappingRemove(const Mapping& igdMapping) override; diff --git a/src/upnp/protocol/pupnp/pupnp.cpp b/src/upnp/protocol/pupnp/pupnp.cpp index 8b1406aa33245d7391212fd497c53ccdaaf2dd77..a87f3803e902398815ab3612b9d0a4d9ea2c7bd2 100644 --- a/src/upnp/protocol/pupnp/pupnp.cpp +++ b/src/upnp/protocol/pupnp/pupnp.cpp @@ -113,9 +113,9 @@ PUPnP::PUPnP() port6 = UpnpGetServerPort6(); #endif if (ip_address6 and port6) - JAMI_DBG("PUPnP: Initialiazed on %s:%u | %s:%u", ip_address, port, ip_address6, port6); + JAMI_DBG("PUPnP: Initialized on %s:%u | %s:%u", ip_address, port, ip_address6, port6); else - JAMI_DBG("PUPnP: Initialiazed on %s:%u", ip_address, port); + JAMI_DBG("PUPnP: Initialized on %s:%u", ip_address, port); // Relax the parser to allow malformed XML text. ixmlRelaxParser(1); @@ -137,8 +137,10 @@ PUPnP::PUPnP() JAMI_ERR("PUPnP: Can't register client: %s", UpnpGetErrorMessage(upnp_err)); pupnpRun_ = false; break; - } else + } else { + JAMI_DBG("PUPnP: Successfully registered client"); clientRegistered_ = true; + } } if (not pupnpRun_) @@ -313,9 +315,14 @@ PUPnP::validateIgd(const IGDInfo& info) if (updateIgdListCb_(this, std::move(igd_candidate.get()), std::move(igd_candidate.get()->publicIp_), - true)) + true)) { + JAMI_DBG("PUPnP: IGD with public IP %s was added to the list", - igd_candidate->publicIp_.toString().c_str()); + igd_candidate->publicIp_.toString().c_str()); + } else { + JAMI_DBG("PUPnP: IGD with public IP %s is already in the list", + igd_candidate->publicIp_.toString().c_str()); + } // Keep local IGD list internally. validIgdList_.emplace(igd_candidate->getUDN(), std::move(igd_candidate)); @@ -335,9 +342,8 @@ PUPnP::validateIgd(const IGDInfo& info) } void -PUPnP::requestMappingAdd(IGD* igd, uint16_t port_external, uint16_t port_internal, PortType type) +PUPnP::requestMappingAdd(IGD* igd, const Mapping& mapping) { - Mapping mapping {port_external, port_internal, type}; if (auto pupnp_igd = dynamic_cast<const UPnPIGD*>(igd)) { JAMI_DBG("PUPnP: Attempting to open port %s", mapping.toString().c_str()); actionAddPortMappingAsync(*pupnp_igd, mapping); @@ -506,9 +512,36 @@ PUPnP::removeAllLocalMappings(IGD* igd) actionDeletePortMappingsByDesc(*igd_del_map, Mapping::UPNP_DEFAULT_MAPPING_DESCRIPTION); } +const char* +PUPnP::eventTypeToString(Upnp_EventType eventType) +{ + switch(eventType){ + case UPNP_CONTROL_ACTION_REQUEST: return "UPNP_CONTROL_ACTION_REQUEST"; + case UPNP_CONTROL_ACTION_COMPLETE: return "UPNP_CONTROL_ACTION_COMPLETE"; + case UPNP_CONTROL_GET_VAR_REQUEST: return "UPNP_CONTROL_GET_VAR_REQUEST"; + case UPNP_CONTROL_GET_VAR_COMPLETE: return "UPNP_CONTROL_GET_VAR_COMPLETE"; + case UPNP_DISCOVERY_ADVERTISEMENT_ALIVE: return "UPNP_DISCOVERY_ADVERTISEMENT_ALIVE"; + case UPNP_DISCOVERY_ADVERTISEMENT_BYEBYE: return "UPNP_DISCOVERY_ADVERTISEMENT_BYEBYE"; + case UPNP_DISCOVERY_SEARCH_RESULT: return "UPNP_DISCOVERY_SEARCH_RESULT"; + case UPNP_DISCOVERY_SEARCH_TIMEOUT: return "UPNP_DISCOVERY_SEARCH_TIMEOUT"; + case UPNP_EVENT_SUBSCRIPTION_REQUEST: return "UPNP_EVENT_SUBSCRIPTION_REQUEST"; + case UPNP_EVENT_RECEIVED: return "UPNP_EVENT_RECEIVED"; + case UPNP_EVENT_RENEWAL_COMPLETE: return "UPNP_EVENT_RENEWAL_COMPLETE"; + case UPNP_EVENT_SUBSCRIBE_COMPLETE: return "UPNP_EVENT_SUBSCRIBE_COMPLETE"; + case UPNP_EVENT_UNSUBSCRIBE_COMPLETE: return "UPNP_EVENT_UNSUBSCRIBE_COMPLETE"; + case UPNP_EVENT_AUTORENEWAL_FAILED: return "UPNP_EVENT_AUTORENEWAL_FAILED"; + case UPNP_EVENT_SUBSCRIPTION_EXPIRED: return "UPNP_EVENT_SUBSCRIPTION_EXPIRED"; + default : return "Unknown UPNP Event !"; + } +} + int PUPnP::ctrlPtCallback(Upnp_EventType event_type, const void* event, void* user_data) { + // Filter out less relevant logs. + if (event_type != UPNP_DISCOVERY_ADVERTISEMENT_ALIVE) + JAMI_DBG("PUPnP: Control point callback event_type %s", PUPnP::eventTypeToString(event_type)); + if (auto pupnp = static_cast<PUPnP*>(user_data)) return pupnp->handleCtrlPtUPnPEvents(event_type, event); @@ -549,8 +582,11 @@ PUPnP::handleCtrlPtUPnPEvents(Upnp_EventType event_type, const void* event) const UpnpDiscovery* d_event = (const UpnpDiscovery*) event; // First check the error code. - if (UpnpDiscovery_get_ErrCode(d_event) != UPNP_E_SUCCESS) + auto upnp_status = UpnpDiscovery_get_ErrCode(d_event); + if (upnp_status != UPNP_E_SUCCESS) { + JAMI_ERR("PUPnP: UPNP discovery is in erroneous state: %s", UpnpGetErrorMessage(upnp_status)); break; + } // Check if this device ID is already in the list. std::string cpDeviceId = UpnpDiscovery_get_DeviceID_cstr(d_event); @@ -567,9 +603,18 @@ PUPnP::handleCtrlPtUPnPEvents(Upnp_EventType event_type, const void* event) // NOTE: here, we check if the location given is related to the source address. // If it's not the case, it's certainly a router plugged in the network, but not // related to this network. So the given location will be unreachable and this - // will cause some timeout - if (IpAddr(url.host) != ipSrc) + // will cause some timeout. + + // Only check the IP address (ignore the port number). + if (IpAddr(url.host).toString(false) != ipSrc.toString(false)) { + JAMI_WARN("PUPnP: Returned location %s does not match the source address %s", + IpAddr(url.host).toString(true, true).c_str(), ipSrc.toString(true, true).c_str()); + break; + } else { + JAMI_WARN("PUPnP: Returned IGD location %s", IpAddr(url.host).toString(true, true).c_str()); + } + dwnldlXmlList_.emplace_back( dht::ThreadPool::io().get<pIGDInfo>([this, location = std::move(igdLocationUrl)] { @@ -579,15 +624,18 @@ PUPnP::handleCtrlPtUPnPEvents(Upnp_EventType event_type, const void* event) if (doc_container_ptr) doc_desc_ptr.reset(doc_container_ptr); pupnpCv_.notify_all(); - if (upnp_err != UPNP_E_SUCCESS or not doc_desc_ptr) + if (upnp_err != UPNP_E_SUCCESS or not doc_desc_ptr) { JAMI_WARN("PUPnP: Error downloading device XML document from %s -> %s", - location.c_str(), - UpnpGetErrorMessage(upnp_err)); - else + location.c_str(), UpnpGetErrorMessage(upnp_err)); + return std::make_unique<IGDInfo>( + IGDInfo {std::move(location), XMLDocument(nullptr, ixmlDocument_free)}); + } + else { + JAMI_WARN("PUPnP: Succeeded to download device XML document from %s", + location.c_str()); return std::make_unique<IGDInfo>( IGDInfo {std::move(location), std::move(doc_desc_ptr)}); - return std::make_unique<IGDInfo>( - IGDInfo {std::move(location), XMLDocument(nullptr, ixmlDocument_free)}); + } })); break; diff --git a/src/upnp/protocol/pupnp/pupnp.h b/src/upnp/protocol/pupnp/pupnp.h index 31ce740e28b51a97945ea6547b08ed413fc24b6c..17f3a2b1f4d5e355779b1833eff9d7ff01411c0a 100644 --- a/src/upnp/protocol/pupnp/pupnp.h +++ b/src/upnp/protocol/pupnp/pupnp.h @@ -95,7 +95,10 @@ public: ~PUPnP(); // Returns the protocol type. - Type getType() const override { return Type::PUPNP; } + Type getProtocol() const override { return Type::PUPNP; } + + // Get protocol type as a string. + virtual std::string getProtocolName() const override { return {"PUPNP"};} // Notifies a change in network. void clearIgds() override; @@ -104,10 +107,8 @@ public: void searchForIgd() override; // Tries to add mapping. Assumes mutex is already locked. - void requestMappingAdd(IGD* igd, - uint16_t port_external, - uint16_t port_internal, - PortType type) override; + void requestMappingAdd(IGD* igd, const Mapping& mapping) override; + // Treats the reception of an add mapping action answer. void processAddMapAction(const std::string_view& ctrlURL, IXML_Document* actionRequest); @@ -168,6 +169,8 @@ private: bool actionDeletePortMappingAsync(const UPnPIGD& igd, const std::string& port_external, const std::string& protocol); + // Event type to string + static const char* eventTypeToString(Upnp_EventType eventType); private: NON_COPYABLE(PUPnP); @@ -192,4 +195,4 @@ private: }; } // namespace upnp -} // namespace jami \ No newline at end of file +} // namespace jami diff --git a/src/upnp/protocol/upnp_protocol.h b/src/upnp/protocol/upnp_protocol.h index 56c09467dd3fddc1dd1f8ace4a30034ea6ff1707..f668cb5b75d72145289c0cb4fd5d4375d68fcd33 100644 --- a/src/upnp/protocol/upnp_protocol.h +++ b/src/upnp/protocol/upnp_protocol.h @@ -66,8 +66,11 @@ public: UPnPProtocol() {}; virtual ~UPnPProtocol() {}; - // Allows each protocol to return it's type. - virtual Type getType() const = 0; + // Get protocol type. + virtual Type getProtocol() const = 0; + + // Get protocol type as string. + virtual std::string getProtocolName() const = 0; // Clear all known IGDs. virtual void clearIgds() = 0; @@ -76,12 +79,7 @@ public: virtual void searchForIgd() = 0; // Sends a request to add a mapping. - virtual void requestMappingAdd(IGD* igd, - uint16_t port_external, - uint16_t port_internal, - PortType type) - = 0; - + virtual void requestMappingAdd(IGD* igd, const Mapping& map) = 0; // Sends a request to remove a mapping. virtual void requestMappingRemove(const Mapping& igdMapping) = 0; @@ -110,4 +108,4 @@ protected: }; } // namespace upnp -} // namespace jami \ No newline at end of file +} // namespace jami diff --git a/src/upnp/upnp_context.cpp b/src/upnp/upnp_context.cpp index 7527f1b06a4f02407efd2097e76bc260ffdecc1a..8bc62e0d696a79716ecd254a82bf068d7ed38357 100644 --- a/src/upnp/upnp_context.cpp +++ b/src/upnp/upnp_context.cpp @@ -24,19 +24,10 @@ namespace jami { namespace upnp { -static uint16_t -generateRandomPort(uint16_t min, uint16_t max) -{ - // Seed the generator. - static std::mt19937 gen(dht::crypto::getSeededRandomEngine()); - // Define the range. - std::uniform_int_distribution<uint16_t> dist(min, max); - return dist(gen); -} - std::shared_ptr<UPnPContext> -getUPnPContext() +UPnPContext::getUPnPContext() { + // This is the unique shared instance (singleton) of UPnPContext class. static auto context = std::make_shared<UPnPContext>(); return context; } @@ -52,6 +43,7 @@ UPnPContext::UPnPContext() natPmp->searchForIgd(); protocolList_.push_back(std::move(natPmp)); #endif + #if HAVE_LIBUPNP auto pupnp = std::make_unique<PUPnP>(); pupnp->setOnPortMapAdd(std::bind(&UPnPContext::onMappingAdded, this, _1, _2, _3)); @@ -60,11 +52,51 @@ UPnPContext::UPnPContext() pupnp->searchForIgd(); protocolList_.push_back(std::move(pupnp)); #endif + + // Set port ranges + portRange_[0] = { 20000, 20500 }; // UDP. + portRange_[1] = { 1024, 9999 }; // TCP. + + // Provision now. + preAllocateProvisionedPorts(PortType::TCP, 5); + preAllocateProvisionedPorts(PortType::UDP, 8); } UPnPContext::~UPnPContext() { - igdList_.clear(); + requestAllMappingRemove(PortType::UDP); + requestAllMappingRemove(PortType::TCP); + { + std::lock_guard<std::mutex> lock(igdListMutex_); + igdList_.clear(); + } + { + std::lock_guard<std::mutex> lock(mapProvisionListMutex_); + for (auto& it : mapProvisionList_) + it.clear(); + } +} + +uint16_t +UPnPContext::generateRandomPort(uint16_t min, uint16_t max, bool mustBeEven) +{ + if (min >= max){ + JAMI_ERR("Max port number (%i) must be greater than min port number (%i) !", + max, min); + return 0; + } + + int fact = mustBeEven ? 2 : 1; + if (mustBeEven) { + min /= fact; + max /= fact; + } + + // Seed the generator. + static std::mt19937 gen(dht::crypto::getSeededRandomEngine()); + // Define the range. + std::uniform_int_distribution<uint16_t> dist(min, max); + return dist(gen) * fact; } void @@ -74,7 +106,7 @@ UPnPContext::connectivityChanged() // Trigger controllers onConnectionChanged { - std::lock_guard<std::mutex> lk(cbListMutex_); + std::lock_guard<std::mutex> lock(mapCbListMutex_); for (auto const& ctrl : mapCbList_) ctrl.second.onConnectionChanged(); } @@ -84,11 +116,18 @@ UPnPContext::connectivityChanged() protocol->clearIgds(); { - std::lock_guard<std::mutex> lk(igdListMutex_); + std::lock_guard<std::mutex> lock(igdListMutex_); if (not igdList_.empty()) igdList_.clear(); } + { + std::lock_guard<std::mutex> lk(mapProvisionListMutex_); + // CLear provisioned mappings. + for (auto& it : mapProvisionList_) + it.clear(); + } + // Restart search for UPnP // When IGD ready, this should re-open ports (cf registerCallback) for (auto const& protocol : protocolList_) @@ -123,7 +162,7 @@ IpAddr UPnPContext::getLocalIP() const { // Lock mutex on the igd list. - std::lock_guard<std::mutex> igdListLock(igdListMutex_); + std::lock_guard<std::mutex> lock(igdListMutex_); // Return first valid local Ip. if (not igdList_.empty()) { @@ -142,7 +181,7 @@ IpAddr UPnPContext::getExternalIP() const { // Lock mutex on the igd list. - std::lock_guard<std::mutex> igdListLock(igdListMutex_); + std::lock_guard<std::mutex> lock(igdListMutex_); // Return first valid external Ip. if (not igdList_.empty()) { @@ -157,38 +196,167 @@ UPnPContext::getExternalIP() const return {}; } +const Mapping +UPnPContext::selectProvisionedMapping(const Mapping& requestedMap) +{ + auto desiredPort = requestedMap.getPortExternal(); + + if (desiredPort == 0) { + JAMI_DBG("UPnP: Desired port is not set, will provide the first available port for [%s]", + requestedMap.getTypeStr().c_str()); + } else { + JAMI_DBG("UPnP: Try to find mapping for port %i [%s]", + desiredPort, requestedMap.getTypeStr().c_str()); + } + + std::lock_guard<std::mutex> lk(mapProvisionListMutex_); + auto& provisionList = getMappingList(requestedMap.getType()); + + for (auto& [_,map] : provisionList) { + // If the desired port is null, we pick the first available port. + if ( (desiredPort == 0 or map.getPortExternal() == desiredPort) and map.isAvailable()) { + map.setAvailable(false); + map.setDescription(requestedMap.getDescription()); + return map; + } + } + + JAMI_WARN("UPnP: Did not find provisioned mapping for port %i [%s]", + desiredPort, requestedMap.getTypeStr().c_str()); + + return {}; +} + +void +UPnPContext::unselectProvisionedPort(const Mapping& map) +{ + std::lock_guard<std::mutex> lk(mapProvisionListMutex_); + + auto& provisionList = getMappingList(map.getType()); + auto it = provisionList.find(map.getPortExternal()); + + if (it != provisionList.end()) { + if(not it->second.isAvailable()) { + // Make the mapping available for future use. + it->second.setAvailable(true); + } else { + JAMI_WARN("UPnP: Trying to release an unused mapping %s", + it->second.toString().c_str()); + } + } else { + JAMI_WARN("UPnP: Trying to release an un-existing mapping for port %i", map.getPortExternal()); + } +} + +uint16_t UPnPContext::getAvailablePortNumber(PortType type, uint16_t minPort, uint16_t maxPort) +{ + // Only return the an availalable random port. No actual + // reservation is made here. + + + if (minPort > maxPort) { + JAMI_ERR("UPnP: Min port %u can not be greater than max port %u", + minPort, maxPort); + return 0; + } + + unsigned typeIdx = type == PortType::UDP ? 0 : 1; + if (minPort == 0) + minPort = portRange_[typeIdx].first; + if (maxPort == 0) + maxPort = portRange_[typeIdx].second; + + + std::lock_guard<std::mutex> lk(mapProvisionListMutex_); + auto& provisionList = getMappingList(type); + unsigned int tryCount = 0; + + while (tryCount++ < MAX_REQUEST_RETRIES) { + uint16_t port = generateRandomPort(minPort, maxPort); + if (provisionList.find(port) == provisionList.end()) + return port; + } + + // Very unlikely to get here. + JAMI_ERR("UPnP: Could not find an available port after %i trials", MAX_REQUEST_RETRIES); + return 0; +} + bool -UPnPContext::isIgdInList(const IpAddr& publicIpAddr) +UPnPContext::provisionPort(IGD* requestedIgd, const Mapping& requestedMap) { - for (auto const& item : igdList_) { - if (item.second->publicIp_) { - if (item.second->publicIp_ == publicIpAddr) { - return true; - } + assert(requestedIgd != nullptr); + + JAMI_DBG("UPnPContext: Provision for mapping %s on IGD %s", + requestedMap.toString().c_str(), requestedIgd->publicIp_.toString(true).c_str()); + + for (auto const& [protocol, igd] : igdList_) { + if (igd == requestedIgd) { + protocol->requestMappingAdd(igd, requestedMap); + return true; } } + + JAMI_ERR("UPnPContext: IGD %p not found", requestedIgd); return false; } -UPnPProtocol::Type -UPnPContext::getIgdProtocol(IGD* igd) +bool +UPnPContext::preAllocateProvisionedPorts(PortType type, unsigned portCount, uint16_t minPort, uint16_t maxPort) { - for (auto const& item : igdList_) { - if (item.second->publicIp_ == igd->publicIp_) { - return item.first->getType(); + JAMI_DBG("UPnPContext: Try provision %i [%s] ports", + portCount, Mapping::getTypeStr(type).c_str()); + + unsigned int provPortsFound = 0; + + while (provPortsFound < portCount) { + auto port = getAvailablePortNumber(type, minPort, maxPort); + if (port > 0) { + // Found an available port number + provPortsFound++; + Mapping map {port, port, type, "JAMI", true}; + // Build the control data structure. + ControllerData ctrlData {}; + ctrlData.id = 0; + ctrlData.keepCb = true; + ctrlData.onMapAdded = [](const Mapping& map, bool success) {}; + ctrlData.onMapRemoved = [this](const Mapping& map, bool) {}; + ctrlData.onConnectionChanged = [this]() {}; + requestMappingAdd(std::move(ctrlData), map); + } else { + // Very unlikely to get here ! + JAMI_ERR("UPnPContext: Can not find any available port to provision !"); + return false; } } - return UPnPProtocol::Type::UNKNOWN; + return true; +} + +bool +UPnPContext::isIgdInList(const UPnPProtocol* protocol, const IpAddr& publicIpAddr) +{ + std::lock_guard<std::mutex> lock(igdListMutex_); + for (auto const& item : igdList_) { + if (item.second->publicIp_) { + if (item.first == protocol && item.second->publicIp_ == publicIpAddr) { + return true; + } + } + } + return false; } bool UPnPContext::igdListChanged(UPnPProtocol* protocol, IGD* igd, IpAddr publicIpAddr, bool added) { - std::lock_guard<std::mutex> igdListLock(igdListMutex_); if (added) { + JAMI_DBG("UPnPContext: IGD %p [%s] added for public address %s", + igd, protocol->getProtocolName().c_str(), publicIpAddr.toString(true, true).c_str()); return addIgdToList(protocol, igd); } else { + JAMI_WARN("UPnPContext: Failed to add IGD %p [%s] for public address %s", + igd, protocol->getProtocolName().c_str(), publicIpAddr.toString(true, true).c_str()); if (publicIpAddr) { return removeIgdFromList(publicIpAddr); } else { @@ -200,34 +368,37 @@ UPnPContext::igdListChanged(UPnPProtocol* protocol, IGD* igd, IpAddr publicIpAdd bool UPnPContext::addIgdToList(UPnPProtocol* protocol, IGD* igd) { - // igdListMutex_ already locked // Check if IGD has a valid public IP. if (not igd->publicIp_) { JAMI_WARN("UPnPContext: IGD trying to be added has invalid public IpAddress"); return false; } + + // Check if the IGD is in the list. + if (isIgdInList(protocol, igd->publicIp_)) { + JAMI_DBG("UPnPContext: IGD with protocol %s and public IP %s is already in the list", + protocol->getProtocolName().c_str(), + igd->publicIp_.toString().c_str()); + return false; + } + { - // Check if the IGD is in the list. - if (isIgdInList(igd->publicIp_)) { - JAMI_DBG("UPnPContext: IGD with public IP %s is already in the list", - igd->publicIp_.toString().c_str()); - return false; - } + // Add the new IGD to the list. + std::lock_guard<std::mutex> lock(igdListMutex_); igdList_.emplace_back(protocol, igd); - JAMI_DBG("UPnP: IGD with public IP %s was added to the list", - igd->publicIp_.toString().c_str()); + JAMI_DBG("UPnPContext: IGD with protocol %s public IP %s was added to the list", + protocol->getProtocolName().c_str(), + igd->publicIp_.toString().c_str()); } + // Iterate over callback list and dispatch any pending mapping requests - std::lock_guard<std::mutex> lk2(cbListMutex_); + std::lock_guard<std::mutex> lock(mapCbListMutex_); for (auto const& cbAdd : mapCbList_) { JAMI_DBG("[upnp:controller@%ld] sending out request in cb queue for mapping %s", (long) cbAdd.second.id, cbAdd.first.toString().c_str()); registerAddMappingTimeout(cbAdd.first); - protocol->requestMappingAdd(igd, - cbAdd.first.getPortExternal(), - cbAdd.first.getPortInternal(), - cbAdd.first.getType()); + provisionPort(igd, cbAdd.first); } return true; } @@ -235,7 +406,8 @@ UPnPContext::addIgdToList(UPnPProtocol* protocol, IGD* igd) bool UPnPContext::removeIgdFromList(IGD* igd) { - // igdListMutex_ already locked + std::lock_guard<std::mutex> lock(igdListMutex_); + auto it = igdList_.begin(); while (it != igdList_.end()) { if (it->second->publicIp_ == igd->publicIp_) { @@ -254,7 +426,7 @@ UPnPContext::removeIgdFromList(IGD* igd) bool UPnPContext::removeIgdFromList(IpAddr publicIpAddr) { - // igdListMutex_ already locked + std::lock_guard<std::mutex> lock(igdListMutex_); auto it = igdList_.begin(); while (it != igdList_.end()) { if (it->second->publicIp_ == publicIpAddr) { @@ -273,7 +445,7 @@ UPnPContext::removeIgdFromList(IpAddr publicIpAddr) bool UPnPContext::isMappingInUse(const unsigned int portDesired, PortType type) { - std::lock_guard<std::mutex> lk(igdListMutex_); + std::lock_guard<std::mutex> lock(igdListMutex_); for (auto const& igd : igdList_) { if (igd.second->isMapInUse(portDesired, type)) { return true; @@ -284,98 +456,103 @@ UPnPContext::isMappingInUse(const unsigned int portDesired, PortType type) void UPnPContext::incrementNbOfUsers(const unsigned int portDesired, PortType type) { - std::lock_guard<std::mutex> lk(igdListMutex_); + std::lock_guard<std::mutex> lock(igdListMutex_); for (auto const& igd : igdList_) { igd.second->incrementNbOfUsers(portDesired, type); } } -void -UPnPContext::requestMappingAdd( - ControllerData&& ctrlData, uint16_t portDesired, uint16_t portLocal, PortType type, bool unique) +uint16_t +UPnPContext::requestMappingAdd(ControllerData&& ctrlData, const Mapping& requestedMap) { - std::lock_guard<std::mutex> lk1(igdListMutex_); - std::lock_guard<std::mutex> lk2(pendindRequestMutex_); - // If no IGD is found yet, register the callback and exit. - if (igdList_.empty()) { - JAMI_WARN("UPnP: Trying to add mapping %u:%u %s with no Internet Gateway Device available", - portDesired, - portLocal, - type == upnp::PortType::UDP ? "UDP" : "TCP"); - Mapping map {portDesired, portLocal, type, unique}; - registerAddMappingTimeout(map); - registerCallback(map, std::move(ctrlData)); - return; + std::lock_guard<std::mutex> lock(igdListMutex_); + + auto map { requestedMap }; + if (map.getPortExternal() == 0) { + JAMI_DBG("UPnPContext: Port number not set. Will request a random port number"); + auto port = getAvailablePortNumber(map.getType()); + map.setPortExternal(port); + map.setPortInternal(port); } - // If the mapping requested is unique, find a mapping that isn't already used. - if (unique) { - // Make a list of all currently opened mappings across all IGDs. - std::vector<uint16_t> currentMappings; - for (auto const& igd : igdList_) { - auto globalMappings = igd.second->getCurrentMappingList(type); - for (auto const& map : *globalMappings) - currentMappings.emplace_back(map.second.getPortExternal()); - } - // Also take in consideration the pending map requests. - for (auto const& pendingMap : pendingAddMapList_) { - currentMappings.emplace_back(pendingMap.map.getPortExternal()); - } - if (std::find(currentMappings.begin(), currentMappings.end(), portDesired) - != currentMappings.end()) { - // Keep searching until you find a unique port. - bool unique_found = false; - portDesired = generateRandomPort(upnp::Mapping::UPNP_PORT_MIN, - upnp::Mapping::UPNP_PORT_MAX); - while (not unique_found) { - if (std::find(currentMappings.begin(), currentMappings.end(), portDesired) - != currentMappings.end()) - portDesired = generateRandomPort(upnp::Mapping::UPNP_PORT_MIN, - upnp::Mapping::UPNP_PORT_MAX); - else - unique_found = true; - } + + { + std::lock_guard<std::mutex> lk(mapProvisionListMutex_); + auto& provisionList = getMappingList(requestedMap.getType()); + auto ret = provisionList.emplace(requestedMap.getPortExternal(), requestedMap); + if (not ret.second) { + JAMI_WARN("UPnPContext: Mapping request for %s already performed !", + requestedMap.toString().c_str()); + return 0; } } + + // No available IGD. Register the callback and exit. The mapping + // requests will be performed in when a IGD becomes available (in addIgdToList() method). + if (igdList_.empty()) { + JAMI_WARN("UPnPContext: No IGD available. Mapping will be requested when an IGD becomes available"); + registerCallback(map, std::move(ctrlData)); + return 0; + } + // Register the callback and the pending timeout. - Mapping map {portDesired, portDesired, type, unique}; registerAddMappingTimeout(map); registerCallback(map, std::move(ctrlData)); - // Send out request to open the port to all IGDs. + // Send out request to to all available IGDs. for (auto const& igd : igdList_) - requestMappingAdd(igd.second, portDesired, portDesired, type); + provisionPort(igd.second, map); + + return map.getPortExternal(); } void -UPnPContext::requestMappingAdd(IGD* igd, uint16_t portExternal, uint16_t portInternal, PortType type) +UPnPContext::onMappingAdded(IpAddr igdIp, const Mapping& map, bool success) { - // Iterate over the IGD list and call add the mapping with the corresponding protocol. - if (not igdList_.empty()) { - for (auto const& item : igdList_) { - if (item.second == igd) { - item.first->requestMappingAdd(item.second, portExternal, portInternal, type); - return; - } + if (not map.isValid()) { + JAMI_ERR("PUPnP: Mapping %s is invalid !", map.toString().c_str()); + return; + } + + Mapping provMap {}; + + unregisterAddMappingTimeout(map); + + { + std::lock_guard<std::mutex> lk(mapProvisionListMutex_); + auto& provisionList = getMappingList(map.getType()); + auto it = provisionList.find(map.getPortExternal()); + if (it == provisionList.end()) { + JAMI_ERR("PUPnP: Received an response for an unknown mapping %s (on IGD %s)", + map.toString().c_str(), igdIp.toString().c_str()); + return; } + + it->second.setOpen((success)); + + // Make a copy to prevent calling callback while holding + // the mutex lock. + provMap = Mapping(it->second); } -} -void -UPnPContext::onMappingAdded(IpAddr igdIp, const Mapping& map, bool success) -{ - if (map.isValid()) { - unregisterAddMappingTimeout(map); - if (success) - addMappingToIgd(igdIp, map); - dispatchOnAddCallback(map, success); - if (success) - unregisterCallback(map); + if (success) { + addMappingToIgd(igdIp, map); + JAMI_DBG("PUPnP: Mapping %s (on IGD %s) successfully performed", + provMap.toString().c_str(), igdIp.toString().c_str()); + } else { + JAMI_ERR("PUPnP: Failed to perform mapping %s (on IGD %s)", + provMap.toString().c_str(), igdIp.toString().c_str()); + unregisterProvisionedMapping(provMap); } + + dispatchOnAddCallback(provMap, success); + // update when the state of the mapping changes. + if (success) + unregisterCallback(provMap); } void UPnPContext::addMappingToIgd(IpAddr igdIp, const Mapping& map) { - std::lock_guard<std::mutex> lk(igdListMutex_); + std::lock_guard<std::mutex> lock(igdListMutex_); for (auto const& igd : igdList_) { if (igd.second->publicIp_ == igdIp) { igd.second->incrementNbOfUsers(map); @@ -389,7 +566,7 @@ UPnPContext::dispatchOnAddCallback(const Mapping& map, bool success) { std::vector<MapCb> cbList; { - std::lock_guard<std::mutex> lk(cbListMutex_); + std::lock_guard<std::mutex> lock(mapCbListMutex_); auto mmIt = mapCbList_.equal_range(map); for (auto it = mmIt.first; it != mmIt.second; it++) cbList.emplace_back(it->second.onMapAdded); @@ -401,15 +578,15 @@ UPnPContext::dispatchOnAddCallback(const Mapping& map, bool success) bool UPnPContext::requestMappingRemove(const Mapping& map) { + // Only decrement the number of users. The actual removing of + // the mapping will be done when the context is destroyed. auto removed = false; - std::lock_guard<std::mutex> lk(igdListMutex_); + std::lock_guard<std::mutex> lock(igdListMutex_); if (not igdList_.empty()) { for (auto const& igd : igdList_) { if (igd.second->isMapInUse(map)) { if (igd.second->getNbOfUsers(map) > 1) { igd.second->decrementNbOfUsers(map); - } else { - igd.first->requestMappingRemove(map); } removed = true; } @@ -418,12 +595,27 @@ UPnPContext::requestMappingRemove(const Mapping& map) return removed; } +void +UPnPContext::requestAllMappingRemove(PortType type) +{ + std::lock_guard<std::mutex> lk(mapProvisionListMutex_); + auto& provisionList = getMappingList(type); + + for (auto& map : provisionList) { + for (auto& proto : protocolList_) { + proto->requestMappingRemove(map.second); + } + } +} + void UPnPContext::onMappingRemoved(IpAddr igdIp, const Mapping& map, bool success) { if (map.isValid()) { - if (success) + if (success) { removeMappingFromIgd(igdIp, map); + unregisterProvisionedMapping(map); // Will only unregister if the mapping that was removed was a provisioned port. + } dispatchOnRmCallback(map, success); if (success) unregisterCallback(map); @@ -433,7 +625,7 @@ UPnPContext::onMappingRemoved(IpAddr igdIp, const Mapping& map, bool success) void UPnPContext::removeMappingFromIgd(IpAddr igdIp, const Mapping& map) { - std::lock_guard<std::mutex> lk(igdListMutex_); + std::lock_guard<std::mutex> lock(igdListMutex_); for (auto const& igd : igdList_) { if (igd.second->publicIp_ == igdIp) { igd.second->removeMapInUse(map); @@ -447,7 +639,7 @@ UPnPContext::dispatchOnRmCallback(const Mapping& map, bool success) { std::vector<MapCb> cbList; { - std::lock_guard<std::mutex> lk(cbListMutex_); + std::lock_guard<std::mutex> lk(mapCbListMutex_); for (auto it = mapCbList_.cbegin(); it != mapCbList_.cend(); it++) { if (it->first == map) { cbList.emplace_back(it->second.onMapRemoved); @@ -458,10 +650,30 @@ UPnPContext::dispatchOnRmCallback(const Mapping& map, bool success) cb(map, success); } +bool +UPnPContext::unregisterProvisionedMapping(const Mapping& map) +{ + std::lock_guard<std::mutex> lk(mapProvisionListMutex_); + auto& provisionList = getMappingList(map.getType()); + if (provisionList.erase(map.getPortExternal()) != 1) { + JAMI_WARN("UPnPContext: Trying to unregister an unknown mapping %s ", + map.toString().c_str()); + return false; + } + return true; +} + +std::map<uint16_t, Mapping>& +UPnPContext::getMappingList(PortType type) +{ + unsigned typeIdx = type == PortType::UDP ? 0 : 1; + return mapProvisionList_[typeIdx]; +} + void UPnPContext::registerAddMappingTimeout(const Mapping& map) { - // Mutex is already locked. + std::lock_guard<std::mutex> lock(pendindRequestMutex_); for (auto it = pendingAddMapList_.cbegin(); it != pendingAddMapList_.cend(); it++) { if (it->map == map) { return; @@ -475,7 +687,7 @@ UPnPContext::registerAddMappingTimeout(const Mapping& map) map.toString().c_str()); std::vector<MapCb> cbList; { - std::lock_guard<std::mutex> lk(cbListMutex_); + std::lock_guard<std::mutex> lk(mapCbListMutex_); auto mmIt = mapCbList_.equal_range(map); for (auto it = mmIt.first; it != mmIt.second; it++) cbList.emplace_back(it->second.onMapAdded); @@ -503,7 +715,7 @@ UPnPContext::unregisterAddMappingTimeout(const Mapping& map) void UPnPContext::registerCallback(const Mapping& map, ControllerData&& ctrlData) { - // Mutex is already locked. + std::lock_guard<std::mutex> lock(mapCbListMutex_); auto mmIt = mapCbList_.equal_range(map); for (auto it = mmIt.first; it != mmIt.second; it++) if (it->second.id == ctrlData.id) @@ -518,7 +730,7 @@ UPnPContext::registerCallback(const Mapping& map, ControllerData&& ctrlData) void UPnPContext::unregisterCallback(const Mapping& map) { - std::lock_guard<std::mutex> lk(cbListMutex_); + std::lock_guard<std::mutex> lk(mapCbListMutex_); auto mmIt = mapCbList_.equal_range(map); for (auto it = mmIt.first; it != mmIt.second; it++) { if (not it->second.keepCb) { @@ -533,7 +745,7 @@ UPnPContext::unregisterCallback(const Mapping& map) void UPnPContext::unregisterAllCallbacks(uint64_t ctrlId) { - std::lock_guard<std::mutex> lk(cbListMutex_); + std::lock_guard<std::mutex> lk(mapCbListMutex_); for (auto it = mapCbList_.begin(); it != mapCbList_.end(); ++it) { if (it->second.id == ctrlId) { JAMI_DBG("[upnp:controller@%ld] unregistering cb", (long) ctrlId); diff --git a/src/upnp/upnp_context.h b/src/upnp/upnp_context.h index 584e25d0c244e8a5ceaaff1c33765bfb32e52754..625592f906875dfeafd8cbd142ab53659cf2b1a2 100644 --- a/src/upnp/upnp_context.h +++ b/src/upnp/upnp_context.h @@ -93,6 +93,11 @@ public: UPnPContext(); ~UPnPContext(); + // Retrieve the UPnPContext singleton + static std::shared_ptr<UPnPContext> getUPnPContext(); + + static uint16_t generateRandomPort(uint16_t min, uint16_t max, bool mustBeEven = false); + // Check if there is a valid IGD in the IGD list. bool hasValidIGD(); @@ -111,16 +116,11 @@ public: // Increments the number of users for a given port. void incrementNbOfUsers(const unsigned int portDesired, PortType type); - // Sends out a request to a protocol to add a mapping. - void requestMappingAdd(ControllerData&& ctrlData, - uint16_t portDesired, - uint16_t portLocal, - PortType type, - bool unique); + uint16_t requestMappingAdd(ControllerData&& ctrlData, + const Mapping& map); + // Adds mapping to corresponding IGD. void addMappingToIgd(IpAddr igdIp, const Mapping& map); - // Tries to add a mapping to a specific IGD. - void requestMappingAdd(IGD* igd, uint16_t portExternal, uint16_t portInternal, PortType type); // Callback function for when mapping is added. void onMappingAdded(IpAddr igdIp, const Mapping& map, bool success); // Calls corresponding callback. @@ -131,8 +131,10 @@ public: // Unregisters a timeout for a given pending add map request. void unregisterAddMappingTimeout(const Mapping& map); - // Sends out a request to protocol to remove a mapping. + // Remove a mapping. bool requestMappingRemove(const Mapping& map); + // Remove all mapping of a given type. + void requestAllMappingRemove(PortType type); // Removes mapping from corresponding IGD. void removeMappingFromIgd(IpAddr igdIp, const Mapping& map); // Callback function for when mapping is removed. @@ -147,12 +149,21 @@ public: // Removes all callback with a specific controller Id. void unregisterAllCallbacks(uint64_t ctrlId); -private: - // Checks if the IGD is in the list by checking the IGD's public Ip. - bool isIgdInList(const IpAddr& publicIpAddr); + // Returns a selected provisioned port depending on the type of port that is being requested. + const Mapping selectProvisionedMapping(const Mapping& requestedMap); + // Releases a previously provisioned port. + void unselectProvisionedPort(const Mapping& map); - // Returns the protocol of the IGD. - UPnPProtocol::Type getIgdProtocol(IGD* igd); + // Provision ports. + uint16_t getAvailablePortNumber(PortType type, uint16_t minPort = 0, uint16_t maxPort = 0); + bool provisionPort(IGD* igd, const Mapping& map); + bool preAllocateProvisionedPorts(PortType type, unsigned portCount, uint16_t minPort = 0, + uint16_t maxPort = 0); + +private: + // Checks if the IGD is in the list by checking the IGD's + // protocol and public Ip. + bool isIgdInList(const UPnPProtocol* protocol, const IpAddr& publicIpAddr); // Returns a random port that is not yet used by the daemon for UPnP. uint16_t chooseRandomPort(IGD& igd, PortType type); @@ -163,14 +174,20 @@ private: // Tries to add IGD to the list by getting it's public Ip address internally. bool addIgdToList(UPnPProtocol* protocol, IGD* igd); - // Removes IGD from list by specifiying the IGD itself. + // Removes IGD from list by specifying the IGD itself. bool removeIgdFromList(IGD* igd); - // Removes IGD from list by specifiying the IGD's public Ip address. + // Removes IGD from list by specifying the IGD's public Ip address. bool removeIgdFromList(IpAddr publicIpAddr); + // Removes the corresponding mapping from the provision list. + bool unregisterProvisionedMapping(const Mapping& map); + + // Get the mapping list + std::map<uint16_t, Mapping>& getMappingList(PortType type); + public: - constexpr static unsigned MAX_RETRIES = 20; + constexpr static unsigned MAX_REQUEST_RETRIES = 20; private: NON_COPYABLE(UPnPContext); @@ -179,16 +196,22 @@ private: mutable std::mutex igdListMutex_; // Mutex used to access these lists and IGDs in a thread-safe manner. std::list<std::pair<UPnPProtocol*, IGD*>> - igdList_; // List of IGDs with their corresponding public IPs. + igdList_ {}; // List of IGDs with their corresponding public IPs. + + // Mutex that protects the provisioned mappings list. + std::mutex mapProvisionListMutex_; + // List of provisioned mappings. + std::map<uint16_t, Mapping> mapProvisionList_[2] {}; std::mutex pendindRequestMutex_; // Mutex that protects the pending map request lists. std::vector<PendingMapRequest> pendingAddMapList_ {}; // Vector of pending add mapping requests. - std::mutex cbListMutex_; // Mutex that protects the callback list. + std::mutex mapCbListMutex_; // Mutex that protects the callback list. std::multimap<Mapping, ControllerData> - mapCbList_; // List of mappings with their corresponding callbacks. + mapCbList_ {}; // List of mappings with their corresponding callbacks. + // Port ranges for UPD and TCP (in that order). + std::pair<uint16_t, uint16_t> portRange_[2] {}; }; -std::shared_ptr<UPnPContext> getUPnPContext(); } // namespace upnp } // namespace jami diff --git a/src/upnp/upnp_control.cpp b/src/upnp/upnp_control.cpp index e7ccb255a7d2f30c3a3eb8ee12903cbd714a6552..21cacc6e87d3eeb9907ac4086b79e85108157fab 100644 --- a/src/upnp/upnp_control.cpp +++ b/src/upnp/upnp_control.cpp @@ -29,15 +29,17 @@ Controller::Controller(bool keepCb) { id_ = (uint64_t) this; try { - upnpContext_ = getUPnPContext(); + upnpContext_ = UPnPContext::getUPnPContext(); } catch (std::runtime_error& e) { JAMI_ERR("UPnP context error: %s", e.what()); } + + JAMI_DBG("Controller@%lu: Created UPnP Controller session", id_); } Controller::~Controller() { - JAMI_DBG("Destroying UPnP Controller %p", this); + JAMI_DBG("Controller@%ld: Destroying UPnP Controller session", id_); // Avoid to call removeLocalMap on destroy if (upnpContext_) { upnpContext_->unregisterAllCallbacks(id_); @@ -71,78 +73,77 @@ Controller::getExternalIP() const return {}; } -void -Controller::requestMappingAdd(NotifyServiceCallback&& cb, - uint16_t portDesired, - PortType type, - bool unique, - uint16_t portLocal) +uint16_t Controller::requestMappingAdd(ControllerData&& ctrlData, const Mapping& map) { - if (portLocal == 0) - portLocal = portDesired; + return upnpContext_->requestMappingAdd(std::move(ctrlData), map); +} - // Check if the mapping requested isn't already in the controllers internal list. - if (isLocalMapPresent(portDesired, type)) { - cb(portDesired, true); - return; +uint16_t +Controller::requestMappingAdd(NotifyServiceCallback&& cb, const Mapping& requestedMap) +{ + + if (not upnpContext_) + return 0; + + JAMI_DBG("Controller@%lu: Trying to find a provisioned mapping %s", + id_, requestedMap.toString().c_str()); + + // Try to get a provisioned port + const auto map = upnpContext_->selectProvisionedMapping(requestedMap); + if (map.isValid()) { + JAMI_DBG("Controller@%lu: Found provisioned mapping %s", + id_, map.toString().c_str()); + // Found an available port. + cb(map.getPortExternal(), true); + addLocalMap(map); + return map.getPortExternal(); } - // If the port mapping requested not unique, check if it's already being used. - if (not unique) { - if (upnpContext_->isMappingInUse(portDesired, type)) { - upnpContext_->incrementNbOfUsers(portDesired, type); - Mapping map(portDesired, portLocal, type, unique); - addLocalMap(map); - cb(portDesired, true); - return; + // No port available. + // Should not get here if the ports were properly provisioned. + + JAMI_WARN("Controller@%lu: No available UPNP mapping for %s. Trying to request one now.", + id_, requestedMap.toString().c_str()); + + // Build the control data structure. + ControllerData ctrlData {}; + ctrlData.id = id_; + ctrlData.keepCb = keepCb_; + + std::weak_ptr<Controller> thisWkPtr = std::static_pointer_cast<Controller>(shared_from_this()); + ctrlData.onMapAdded = [cb, requestedMap, thisWkPtr](const Mapping& map, bool success) { + if (auto controller = thisWkPtr.lock()) { + cb(map.getPortExternal(), success); + if (success and map.isValid()) + controller->addLocalMap(map); } - } + }; + ctrlData.onMapRemoved = [thisWkPtr](const Mapping& map, bool) { + if (auto controller = thisWkPtr.lock()) { + if (map.isValid()) + controller->removeLocalMap(map); + } + }; + ctrlData.onConnectionChanged = [thisWkPtr]() { + if (auto controller = thisWkPtr.lock()) { + // Clear local mappings in case of a connectivity change + std::lock_guard<std::mutex> lk(controller->mapListMutex_); + controller->udpMappings_.clear(); + controller->tcpMappings_.clear(); + } + }; - // Send out request. - upnpContext_->requestMappingAdd(ControllerData {id_, - keepCb_, - [cb, portDesired, this](const Mapping& map, - bool success) { - cb(portDesired, success); - if (map.isValid()) - addLocalMap(map); - }, - [this](const Mapping& map, bool) { - if (map.isValid()) - removeLocalMap(map); - }, - [this]() { - // Clear local mappings in case of a - // connectivity changed - std::lock_guard<std::mutex> lk( - mapListMutex_); - udpMappings_.clear(); - tcpMappings_.clear(); - }}, - portDesired, - portLocal, - type, - unique); + // Send the request. + return requestMappingAdd(std::move(ctrlData), requestedMap); } bool -Controller::requestMappingRemove(uint16_t portExternal, PortType type) +Controller::requestMappingRemove( const Mapping& map) { if (not upnpContext_) return false; - std::lock_guard<std::mutex> lk(mapListMutex_); - auto& instanceMappings = type == PortType::UDP ? udpMappings_ : tcpMappings_; - auto mapIt = instanceMappings.find(portExternal); - if (mapIt != instanceMappings.end()) { - if (!upnpContext_->requestMappingRemove(mapIt->second)) { - // No port mapped, so the callback will not be removed, - // remove it anyway - upnpContext_->unregisterCallback(mapIt->second); - } - instanceMappings.erase(mapIt); - return true; - } - return false; + + return upnpContext_->requestMappingRemove(map); } bool @@ -161,18 +162,21 @@ Controller::requestAllMappingRemove(PortType type) return; std::lock_guard<std::mutex> lk(mapListMutex_); auto& instanceMappings = type == PortType::UDP ? udpMappings_ : tcpMappings_; - for (const auto& map : instanceMappings) { - if (!upnpContext_->requestMappingRemove(map.second)) { + for (const auto& [_, map] : instanceMappings) { + if (!upnpContext_->requestMappingRemove(map)) { // No port mapped, so the callback will not be removed, // remove it anyway - upnpContext_->unregisterCallback(map.second); + upnpContext_->unregisterCallback(map); } + upnpContext_->unselectProvisionedPort(map); } } void Controller::addLocalMap(const Mapping& map) { + JAMI_DBG("Controller@%ld: Added map %s", id_, map.toString().c_str()); + std::lock_guard<std::mutex> lk(mapListMutex_); auto& instanceMappings = map.getType() == PortType::UDP ? udpMappings_ : tcpMappings_; instanceMappings.emplace(map.getPortExternal(), Mapping(map)); @@ -183,6 +187,9 @@ Controller::removeLocalMap(const Mapping& map) { if (not upnpContext_) return false; + + JAMI_DBG("Controller@%ld: Remove map %s", id_, map.toString().c_str()); + std::lock_guard<std::mutex> lk(mapListMutex_); auto& instanceMappings = map.getType() == PortType::UDP ? udpMappings_ : tcpMappings_; auto it = instanceMappings.find(map.getPortExternal()); @@ -193,5 +200,21 @@ Controller::removeLocalMap(const Mapping& map) return false; } +bool +Controller::preAllocateProvisionedPorts(upnp::PortType type, unsigned portCount, + uint16_t minPort, uint16_t maxPort) +{ + if (upnpContext_) + return upnpContext_->preAllocateProvisionedPorts(type, portCount); + + return false; +} + +uint16_t +Controller::generateRandomPort(uint16_t min, uint16_t max, bool mustBeEven) +{ + return UPnPContext::generateRandomPort(min, max, mustBeEven); +} + } // namespace upnp } // namespace jami diff --git a/src/upnp/upnp_control.h b/src/upnp/upnp_control.h index 2966b35d1254d7ea0e28ed49db084bc0892be43b..665374d7e01d509842810b7de0f8b24e82f4392e 100644 --- a/src/upnp/upnp_control.h +++ b/src/upnp/upnp_control.h @@ -42,12 +42,14 @@ class IpAddr; namespace jami { namespace upnp { + class UPnPContext; -class Controller +class Controller : public std::enable_shared_from_this<Controller> { public: using NotifyServiceCallback = std::function<void(uint16_t, bool)>; + Controller(bool keepCb = false); ~Controller(); @@ -60,16 +62,24 @@ public: // Gets the local ip that interface with the first valid IGD in the list. IpAddr getLocalIP() const; - void requestMappingAdd(NotifyServiceCallback&& cb, - uint16_t portDesired, - PortType type, - bool unique, - uint16_t portLocal = 0); - bool requestMappingRemove(uint16_t portExternal, PortType type); + // Request port mapping. + // Returns the port number for which a mapping is being requested. The + // mapping might not be ready when the method. The final result is + // returned through the provided callback. + uint16_t requestMappingAdd(NotifyServiceCallback&& cb, const Mapping& map); + + // Remove port mapping. + bool requestMappingRemove( const Mapping& map); // Checks if the map is present locally given a port and type. bool isLocalMapPresent(uint16_t portExternal, PortType type) const; + // Generates provision ports to be used throughout the application's lifetime. + bool preAllocateProvisionedPorts(upnp::PortType type, unsigned portCount, uint16_t minPort, uint16_t maxPort); + + // Generates random port number. + static uint16_t generateRandomPort(uint16_t min, uint16_t max, bool mustBeEven = false); + private: std::shared_ptr<UPnPContext> upnpContext_; // Context from which the controller executes the wanted commands. @@ -83,6 +93,9 @@ private: bool keepCb_ {false}; // Variable that indicates if the controller wants to keep it's callbacks // in the list after a connectivity change. + // Try to provision a UPNP port. + uint16_t requestMappingAdd(ControllerData&& ctrlData, const Mapping& map); +private: // Adds a mapping locally to the list. void addLocalMap(const Mapping& map); // Removes a mapping locally from the list.