From e5805e9047cade2da2cad7e0cb74f7b1c12a16ff Mon Sep 17 00:00:00 2001
From: Gioacchino Mazzurco <gio@eigenlab.org>
Date: Wed, 24 Oct 2018 05:32:21 +0200
Subject: [PATCH] Avoid external port being reset without necessity

Fix RetroSahre behaviour when manually configured external port is
  different to local port
---
 libretroshare/src/pqi/p3netmgr.cc | 39 +++++++++++++++++--------------
 libretroshare/src/pqi/p3netmgr.h  | 16 ++++++++++++-
 2 files changed, 37 insertions(+), 18 deletions(-)

diff --git a/libretroshare/src/pqi/p3netmgr.cc b/libretroshare/src/pqi/p3netmgr.cc
index 7776fb4bb..3887f1ea1 100644
--- a/libretroshare/src/pqi/p3netmgr.cc
+++ b/libretroshare/src/pqi/p3netmgr.cc
@@ -689,6 +689,7 @@ void p3NetMgrIMPL::netExtCheck()
 
 	{
 		RS_STACK_MUTEX(mNetMtx);
+
 		bool isStable = false;
 		sockaddr_storage tmpip;
 
@@ -782,8 +783,7 @@ void p3NetMgrIMPL::netExtCheck()
 #if defined(NETMGR_DEBUG_TICK) || defined(NETMGR_DEBUG_RESET)
 					std::cerr << "p3NetMgrIMPL::netExtCheck() Ext supplied by ExtAddrFinder" << std::endl;
 #endif
-					/* best guess at port */
-					sockaddr_storage_setport(tmpip, sockaddr_storage_port(mLocalAddr));
+					sockaddr_storage_setport(tmpip, guessNewExtPort());
 
 #if defined(NETMGR_DEBUG_TICK) || defined(NETMGR_DEBUG_RESET)
 					std::cerr << "p3NetMgrIMPL::netExtCheck() ";
@@ -823,9 +823,7 @@ void p3NetMgrIMPL::netExtCheck()
 #if defined(NETMGR_DEBUG_TICK) || defined(NETMGR_DEBUG_RESET)
 				std::cerr << "p3NetMgrIMPL::netExtCheck() Ext supplied by ExtAddrFinder" << std::endl;
 #endif
-				/* best guess at port */
-				sockaddr_storage_setport( tmpaddr,
-				                          sockaddr_storage_port(mLocalAddr) );
+				sockaddr_storage_setport(tmpaddr, guessNewExtPort());
 
 #if defined(NETMGR_DEBUG_TICK) || defined(NETMGR_DEBUG_RESET)
 				std::cerr << "p3NetMgrIMPL::netExtCheck() ";
@@ -1121,30 +1119,38 @@ bool p3NetMgrIMPL::checkNetAddress()
 			addrChanged = true;
 		}
 
-		/* if localaddr = serveraddr, then ensure that the ports
+#if DEAD_CODE
+		/* Enabling this piece of code breaks setup where an additional BOFH
+		 * overlooked port like 80 or 443 is manually forwarded to RetroShare to
+		 * avoid restrictive firewals.
+		 * In the case of a real mismatch, it is not really problematic, as our
+		 * peers would get and then attempt to connect also to the right port.
+		 */
+
+		/* if localaddr == serveraddr, then ensure that the ports
 		 * are the same (modify server)... this mismatch can
 		 * occur when the local port is changed....
 		 */
-        if (sockaddr_storage_sameip(mLocalAddr, mExtAddr) && sockaddr_storage_port(mLocalAddr) != sockaddr_storage_port(mExtAddr))
+		if ( sockaddr_storage_sameip(mLocalAddr, mExtAddr)
+		     && sockaddr_storage_port(mLocalAddr) != sockaddr_storage_port(mExtAddr) )
 		{
 #ifdef NETMGR_DEBUG_RESET
-        std::cerr << "p3NetMgrIMPL::checkNetAddress() local and external ports are not the same. Setting external port to " <<       sockaddr_storage_port(mLocalAddr) << std::endl;
+			std::cerr << __PRETTY_FUNCTION__ << " local and external ports are"
+			          << " not the same. Setting external port to "
+			          << sockaddr_storage_port(mLocalAddr) << std::endl;
 #endif
 			sockaddr_storage_setport(mExtAddr, sockaddr_storage_port(mLocalAddr));
 			addrChanged = true;
 		}
-
-		// ensure that address family is set, otherwise windows Barfs.
-		//mLocalAddr.sin_family = AF_INET;
-		//mExtAddr.sin_family = AF_INET;
+#endif // DEAD_CODE
 
 #ifdef NETMGR_DEBUG_TICK
-		std::cerr << "p3NetMgrIMPL::checkNetAddress() Final Local Address: " << sockaddr_storage_tostring(mLocalAddr);
-		std::cerr << std::endl;
+		std::cerr << __PRETTY_FUNCTION__ << " Final Local Address: "
+		          << sockaddr_storage_tostring(mLocalAddr) << std::endl;
 #endif
 		
 	}
-	
+
 	if (addrChanged)
 	{
 #ifdef NETMGR_DEBUG_RESET
@@ -1997,8 +2003,7 @@ void p3NetMgrIMPL::updateNetStateBox_startup()
 			bool extFinderOk = mExtAddrFinder->hasValidIP(tmpip);
 			if (extFinderOk)
 			{
-				/* best guess at port */
-				sockaddr_storage_setport(tmpip, sockaddr_storage_port(mNetFlags.mLocalAddr));
+				sockaddr_storage_setport(tmpip, guessNewExtPort());
 
 #ifdef	NETMGR_DEBUG_STATEBOX
 				std::cerr << "p3NetMgrIMPL::updateNetStateBox_startup() ";
diff --git a/libretroshare/src/pqi/p3netmgr.h b/libretroshare/src/pqi/p3netmgr.h
index e16b035b8..558b812ff 100644
--- a/libretroshare/src/pqi/p3netmgr.h
+++ b/libretroshare/src/pqi/p3netmgr.h
@@ -301,7 +301,21 @@ void    netUnreachableCheck();
 void 	updateNetStateBox_temporal();
 void 	updateNetStateBox_startup();
 void 	updateNetStateBox_reset();
-void    updateNatSetting();
+    void updateNatSetting();
+
+	/** Conservatively guess new external port, previous approach (aka always
+	 * reset it to local port) break setups where external manually
+	 * forwarded port is different then local port. A common case is having
+	 * SSLH listening on port 80 on the router with public IP forwanding
+	 * plain HTTP connections to a web server and --anyprot connections to
+	 * retroshare to make censor/BOFH/bad firewall life a little more
+	 * difficult */
+	uint16_t guessNewExtPort()
+	{
+		uint16_t newExtPort = sockaddr_storage_port(mExtAddr);
+		if(!newExtPort) newExtPort = sockaddr_storage_port(mLocalAddr);
+		return newExtPort;
+	}
 
 private:
 	// These should have there own Mutex Protection,