From 2900bc26a5056705c50273f5c0bd17d38bfb4054 Mon Sep 17 00:00:00 2001 From: kote Date: Fri, 6 Sep 2019 02:58:28 +0800 Subject: [PATCH 1/2] fixed #1388 : took code from https://github.com/PurpleI2P/i2pd/commit/736c95a870814f00031c75b2d4ad7af64a65918d and fixed it as https://github.com/PurpleI2P/i2pd/issues/1388#issuecomment-528495918 tells --- daemon/UPnP.cpp | 132 ++++++++++++++++++++++++++++++++---------------- daemon/UPnP.h | 58 ++++++++++++--------- 2 files changed, 123 insertions(+), 67 deletions(-) diff --git a/daemon/UPnP.cpp b/daemon/UPnP.cpp index 3aad19c6..d112c7d2 100644 --- a/daemon/UPnP.cpp +++ b/daemon/UPnP.cpp @@ -79,43 +79,58 @@ namespace transport void UPnP::Discover () { -#if MINIUPNPC_API_VERSION >= 14 - int nerror = 0; - m_Devlist = upnpDiscover (2000, m_MulticastIf, m_Minissdpdpath, 0, 0, 2, &nerror); -#elif ( MINIUPNPC_API_VERSION >= 8 || defined(UPNPDISCOVER_SUCCESS) ) - int nerror = 0; - m_Devlist = upnpDiscover (2000, m_MulticastIf, m_Minissdpdpath, 0, 0, &nerror); + bool isError; + int err; + +#if ((MINIUPNPC_API_VERSION >= 8) || defined (UPNPDISCOVER_SUCCESS)) + err = UPNPDISCOVER_SUCCESS; + +#if (MINIUPNPC_API_VERSION >= 14) + m_Devlist = upnpDiscover (UPNP_RESPONSE_TIMEOUT, NULL, NULL, 0, 0, 2, &err); #else - m_Devlist = upnpDiscover (2000, m_MulticastIf, m_Minissdpdpath, 0); + m_Devlist = upnpDiscover (UPNP_RESPONSE_TIMEOUT, NULL, NULL, 0, 0, &err); #endif + + isError = err != UPNPDISCOVER_SUCCESS; +#else // MINIUPNPC_API_VERSION >= 8 + err = 0; + m_Devlist = upnpDiscover (UPNP_RESPONSE_TIMEOUT, NULL, NULL, 0); + isError = m_Devlist == NULL; +#endif // MINIUPNPC_API_VERSION >= 8 { - // notify satrting thread + // notify starting thread std::unique_lock l(m_StartedMutex); m_Started.notify_all (); } - int r; - r = UPNP_GetValidIGD (m_Devlist, &m_upnpUrls, &m_upnpData, m_NetworkAddr, sizeof (m_NetworkAddr)); - if (r == 1) + if (isError) + { + LogPrint (eLogError, "UPnP: unable to discover Internet Gateway Devices: error ", err); + return; + } + + err = UPNP_GetValidIGD (m_Devlist, &m_upnpUrls, &m_upnpData, m_NetworkAddr, sizeof (m_NetworkAddr)); + if (err == UPNP_IGD_VALID_CONNECTED) { - r = UPNP_GetExternalIPAddress (m_upnpUrls.controlURL, m_upnpData.first.servicetype, m_externalIPAddress); - if(r != UPNPCOMMAND_SUCCESS) + err = UPNP_GetExternalIPAddress (m_upnpUrls.controlURL, m_upnpData.first.servicetype, m_externalIPAddress); + if(err != UPNPCOMMAND_SUCCESS) { - LogPrint (eLogError, "UPnP: UPNP_GetExternalIPAddress() returned ", r); + LogPrint (eLogError, "UPnP: unable to get external address: error ", err); return; } else { + LogPrint (eLogError, "UPnP: found Internet Gateway Device ", m_upnpUrls.controlURL); if (!m_externalIPAddress[0]) { - LogPrint (eLogError, "UPnP: GetExternalIPAddress() failed."); + LogPrint (eLogError, "UPnP: found Internet Gateway Device doesn't know our external address"); return; } } } else { - LogPrint (eLogError, "UPnP: GetValidIGD() failed."); + LogPrint (eLogError, "UPnP: unable to find valid Internet Gateway Device: error ", err); return; } @@ -126,6 +141,20 @@ namespace transport PortMapping (); } + int UPnP::CheckMapping (const char* port, const char* type) + { + int err = UPNPCOMMAND_SUCCESS; + +#if (MINIUPNPC_API_VERSION >= 10) + err = UPNP_GetSpecificPortMappingEntry(m_upnpUrls.controlURL, m_upnpData.first.servicetype, port, type, NULL, NULL, NULL, NULL, NULL, NULL); +#elif ((MINIUPNPC_API_VERSION >= 8) || defined (UPNPDISCOVER_SUCCESS)) + err = UPNP_GetSpecificPortMappingEntry(m_upnpUrls.controlURL, m_upnpData.first.servicetype, port, type, NULL, NULL, NULL, NULL, NULL); +#else + err = UPNP_GetSpecificPortMappingEntry(m_upnpUrls.controlURL, m_upnpData.first.servicetype, port, type, NULL, NULL); +#endif + return err; + } + void UPnP::PortMapping () { const auto& a = context.GetRouterInfo().GetAddresses(); @@ -134,53 +163,70 @@ namespace transport if (!address->host.is_v6 () && address->port) TryPortMapping (address); } - m_Timer.expires_from_now (boost::posix_time::minutes(20)); // every 20 minutes + m_Timer.expires_from_now (boost::posix_time::minutes(20)); // every 20 minutes m_Timer.async_wait ([this](const boost::system::error_code& ecode) { if (ecode != boost::asio::error::operation_aborted) PortMapping (); }); - - } - - void UPnP::CloseMapping () - { - const auto& a = context.GetRouterInfo().GetAddresses(); - for (const auto& address : a) - { - if (!address->host.is_v6 () && address->port) - CloseMapping (address); - } } void UPnP::TryPortMapping (std::shared_ptr address) { std::string strType (GetProto (address)), strPort (std::to_string (address->port)); - int r; std::string strDesc; i2p::config::GetOption("upnp.name", strDesc); -#ifdef UPNPDISCOVER_SUCCESS - r = UPNP_AddPortMapping (m_upnpUrls.controlURL, m_upnpData.first.servicetype, strPort.c_str (), strPort.c_str (), m_NetworkAddr, strDesc.c_str (), strType.c_str (), 0, "0"); + int err = UPNPCOMMAND_SUCCESS; + + // check for existing mapping + err = CheckMapping (strPort.c_str (), strType.c_str ()); + if (err != UPNPCOMMAND_SUCCESS) // if mapping not found + { + LogPrint (eLogDebug, "UPnP: possibly port ", strPort, " is not forwarded: return code ", err); + +#if ((MINIUPNPC_API_VERSION >= 8) || defined (UPNPDISCOVER_SUCCESS)) + err = UPNP_AddPortMapping (m_upnpUrls.controlURL, m_upnpData.first.servicetype, strPort.c_str (), strPort.c_str (), m_NetworkAddr, strDesc.c_str (), strType.c_str (), NULL, NULL); #else - r = UPNP_AddPortMapping (m_upnpUrls.controlURL, m_upnpData.first.servicetype, strPort.c_str (), strPort.c_str (), m_NetworkAddr, strDesc.c_str (), strType.c_str (), 0); + err = UPNP_AddPortMapping (m_upnpUrls.controlURL, m_upnpData.first.servicetype, strPort.c_str (), strPort.c_str (), m_NetworkAddr, strDesc.c_str (), strType.c_str (), NULL); #endif - if (r!=UPNPCOMMAND_SUCCESS) - { - LogPrint (eLogError, "UPnP: AddPortMapping (", m_NetworkAddr, ":", strPort, ") failed with code ", r); - return; + if (err != UPNPCOMMAND_SUCCESS) + { + LogPrint (eLogError, "UPnP: port forwarding to ", m_NetworkAddr, ":", strPort, " failed: return code ", err); + return; + } + else + { + LogPrint (eLogInfo, "UPnP: port successfully forwarded (", m_externalIPAddress ,":", strPort, " type ", strType, " -> ", m_NetworkAddr ,":", strPort ,")"); + return; + } } else { - LogPrint (eLogDebug, "UPnP: Port Mapping successful. (", m_NetworkAddr ,":", strPort, " type ", strType, " -> ", m_externalIPAddress ,":", strPort ,")"); + LogPrint (eLogDebug, "UPnP: external forward from ", m_NetworkAddr, ":", strPort, " exists on current Internet Gateway Device"); return; } } + void UPnP::CloseMapping () + { + const auto& a = context.GetRouterInfo().GetAddresses(); + for (const auto& address : a) + { + if (!address->host.is_v6 () && address->port) + CloseMapping (address); + } + } + void UPnP::CloseMapping (std::shared_ptr address) { std::string strType (GetProto (address)), strPort (std::to_string (address->port)); - int r = 0; - r = UPNP_DeletePortMapping (m_upnpUrls.controlURL, m_upnpData.first.servicetype, strPort.c_str (), strType.c_str (), 0); - LogPrint (eLogError, "UPnP: DeletePortMapping() returned : ", r); + int err = UPNPCOMMAND_SUCCESS; + + err = CheckMapping (strPort.c_str (), strType.c_str ()); + if (err == UPNPCOMMAND_SUCCESS) + { + err = UPNP_DeletePortMapping (m_upnpUrls.controlURL, m_upnpData.first.servicetype, strPort.c_str (), strType.c_str (), NULL); + LogPrint (eLogError, "UPnP: DeletePortMapping() returned : ", err); + } } void UPnP::Close () @@ -195,11 +241,11 @@ namespace transport switch (address->transportStyle) { case i2p::data::RouterInfo::eTransportNTCP: - return "TCP"; - break; + return "TCP"; + break; case i2p::data::RouterInfo::eTransportSSU: default: - return "UDP"; + return "UDP"; } } } diff --git a/daemon/UPnP.h b/daemon/UPnP.h index 5313a1c4..48855f0c 100644 --- a/daemon/UPnP.h +++ b/daemon/UPnP.h @@ -19,20 +19,31 @@ namespace i2p { namespace transport { + const int UPNP_RESPONSE_TIMEOUT = 2000; // in milliseconds + + enum + { + UPNP_IGD_NONE = 0, + UPNP_IGD_VALID_CONNECTED = 1, + UPNP_IGD_VALID_NOT_CONNECTED = 2, + UPNP_IGD_INVALID = 3 + }; + class UPnP { - public: + public: UPnP (); ~UPnP (); - void Close (); + void Close (); - void Start (); - void Stop (); + void Start (); + void Stop (); - private: + private: void Discover (); + int CheckMapping (const char* port, const char* type); void PortMapping (); void TryPortMapping (std::shared_ptr address); void CloseMapping (); @@ -41,23 +52,21 @@ namespace transport void Run (); std::string GetProto (std::shared_ptr address); - private: + private: bool m_IsRunning; - std::unique_ptr m_Thread; + std::unique_ptr m_Thread; std::condition_variable m_Started; std::mutex m_StartedMutex; boost::asio::io_service m_Service; boost::asio::deadline_timer m_Timer; - struct UPNPUrls m_upnpUrls; - struct IGDdatas m_upnpData; - - // For miniupnpc - char * m_MulticastIf = 0; - char * m_Minissdpdpath = 0; - struct UPNPDev * m_Devlist = 0; - char m_NetworkAddr[64]; - char m_externalIPAddress[40]; + struct UPNPUrls m_upnpUrls; + struct IGDdatas m_upnpData; + + // For miniupnpc + struct UPNPDev * m_Devlist = 0; + char m_NetworkAddr[64]; + char m_externalIPAddress[40]; }; } } @@ -65,14 +74,15 @@ namespace transport #else // USE_UPNP namespace i2p { namespace transport { - /* class stub */ - class UPnP { - public: - UPnP () {}; - ~UPnP () {}; - void Start () { LogPrint(eLogWarning, "UPnP: this module was disabled at compile-time"); } - void Stop () {}; - }; + /* class stub */ + class UPnP { + public: + + UPnP () {}; + ~UPnP () {}; + void Start () { LogPrint(eLogWarning, "UPnP: this module was disabled at compile-time"); } + void Stop () {}; + }; } } #endif // USE_UPNP From f7a084969a5d501fcd489a5bcde6236f67273d8e Mon Sep 17 00:00:00 2001 From: kote Date: Fri, 6 Sep 2019 03:21:26 +0800 Subject: [PATCH 2/2] fixed #1387 --- daemon/UPnP.cpp | 15 +++++++++++---- daemon/UPnP.h | 1 + qt/i2pd_qt/i2pd_qt.pro | 4 +--- 3 files changed, 13 insertions(+), 7 deletions(-) diff --git a/daemon/UPnP.cpp b/daemon/UPnP.cpp index d112c7d2..b2ebb005 100644 --- a/daemon/UPnP.cpp +++ b/daemon/UPnP.cpp @@ -110,9 +110,10 @@ namespace transport } err = UPNP_GetValidIGD (m_Devlist, &m_upnpUrls, &m_upnpData, m_NetworkAddr, sizeof (m_NetworkAddr)); + m_upnpUrlsInitialized=err!=0; if (err == UPNP_IGD_VALID_CONNECTED) { - err = UPNP_GetExternalIPAddress (m_upnpUrls.controlURL, m_upnpData.first.servicetype, m_externalIPAddress); + err = UPNP_GetExternalIPAddress (m_upnpUrls.controlURL, m_upnpData.first.servicetype, m_externalIPAddress); if(err != UPNPCOMMAND_SUCCESS) { LogPrint (eLogError, "UPnP: unable to get external address: error ", err); @@ -218,11 +219,14 @@ namespace transport void UPnP::CloseMapping (std::shared_ptr address) { + if(!m_upnpUrlsInitialized) { + return; + } std::string strType (GetProto (address)), strPort (std::to_string (address->port)); int err = UPNPCOMMAND_SUCCESS; err = CheckMapping (strPort.c_str (), strType.c_str ()); - if (err == UPNPCOMMAND_SUCCESS) + if (err == UPNPCOMMAND_SUCCESS) { err = UPNP_DeletePortMapping (m_upnpUrls.controlURL, m_upnpData.first.servicetype, strPort.c_str (), strType.c_str (), NULL); LogPrint (eLogError, "UPnP: DeletePortMapping() returned : ", err); @@ -233,8 +237,11 @@ namespace transport { freeUPNPDevlist (m_Devlist); m_Devlist = 0; - FreeUPNPUrls (&m_upnpUrls); - } + if(m_upnpUrlsInitialized){ + FreeUPNPUrls (&m_upnpUrls); + m_upnpUrlsInitialized=false; + } + } std::string UPnP::GetProto (std::shared_ptr address) { diff --git a/daemon/UPnP.h b/daemon/UPnP.h index 48855f0c..e66f0aff 100644 --- a/daemon/UPnP.h +++ b/daemon/UPnP.h @@ -60,6 +60,7 @@ namespace transport std::mutex m_StartedMutex; boost::asio::io_service m_Service; boost::asio::deadline_timer m_Timer; + bool m_upnpUrlsInitialized=false; struct UPNPUrls m_upnpUrls; struct IGDdatas m_upnpData; diff --git a/qt/i2pd_qt/i2pd_qt.pro b/qt/i2pd_qt/i2pd_qt.pro index e6349855..15565c5c 100644 --- a/qt/i2pd_qt/i2pd_qt.pro +++ b/qt/i2pd_qt/i2pd_qt.pro @@ -6,9 +6,7 @@ TARGET = i2pd_qt TEMPLATE = app QMAKE_CXXFLAGS *= -std=c++11 -Wno-unused-parameter -Wno-maybe-uninitialized -# For now, disable UPnP which currently crashes in UPnP::Stop() -- https://github.com/PurpleI2P/i2pd/issues/1387 -#DEFINES += USE_UPNP -DEFINES -= USE_UPNP +DEFINES += USE_UPNP CONFIG(debug, debug|release) { message(Debug build)