Improvements to BanPeer Code, mainly from a UdpStunner perspective.

* Added UdpStunner::dropStunPeer() code to remove bad peer from stun list.
 * added calls to UdpStunner::dropStunPeer for BanLists going to/from DHT.
 * added  DEBUG_BANLIST_CONDENSE to remove unnecessary debug.
 * Improved UdpStunner::status() print out of stunlist.
 * Added extra check to throw away stun reports where remote_addr == reported ext_addr.
	- This was causing peers to get the wrong IP addresses.
 * Modified UdpStunner ExtAddr checks to make sure the IP addresses match.
 * Changed BANLIST service Id to from Test ID to real ID.

NOTE: These are stop-gap methods to avoid the wrong Stun reports.
A more robust scheme must be implemented. I'd expect that it would
involve a strict initial check to establish your IP address...
(e.g. require 4 peers to confirm it, allowing for some fake entries)
After this we can have a weaker check ensuring IP address matches.
If we detect a likely REAL change of IP address - drop back to requiring
a strick check again.



git-svn-id: http://svn.code.sf.net/p/retroshare/code/branches/v0.5-dhtmods@4717 b45a01b8-16f6-495d-af2f-9b41ad6348cc

libretroshare/src/dht/p3bitdht_peernet.cc

@@ -77,6 +77,17 @@ int p3BitDht::InfoCallback(const bdId *id, uint32_t type, uint32_t flags, std::s
 		mPeerSharer->updatePeer(rsid, addr, outtype, outreason, outage);
 	}
 
+	/* call to the Stunners to drop the address as well */
+	/* IDEALLY these addresses should all be filtered at UdpLayer level instead! */
+	if (mDhtStunner)
+	{
+		mDhtStunner->dropStunPeer(addr);
+	}
+	if (mProxyStunner)
+	{
+		mProxyStunner->dropStunPeer(addr);
+	}
+
 	return 1;
 }
 

libretroshare/src/dht/p3bitdht_peers.cc

@@ -186,6 +186,16 @@ bool 	p3BitDht::dropPeer(std::string pid)
 int p3BitDht::addBadPeer(const struct sockaddr_in &addr, uint32_t reason, uint32_t flags, uint32_t age)
 {
 	//mUdpBitDht->updateKnownPeer(&id, 0, bdflags);
+
+	if (mDhtStunner)
+	{
+		mDhtStunner->dropStunPeer(addr);
+	}
+	if (mProxyStunner)
+	{
+		mProxyStunner->dropStunPeer(addr);
+	}
+
 	return 1;
 }
 

libretroshare/src/serialiser/rsserviceids.h

@@ -47,6 +47,8 @@ const uint16_t RS_SERVICE_TYPE_MSG         = 0x0013;
 const uint16_t RS_SERVICE_TYPE_TURTLE      = 0x0014;
 const uint16_t RS_SERVICE_TYPE_TUNNEL      = 0x0015;
 
+/* BanList Still Testing at the moment - Service Only */
+const uint16_t RS_SERVICE_TYPE_BANLIST     = 0x0101;
 
 /* Caches based on p3distrib (Cache Only)
  * Unfortunately, noone changed the DUMMY IDS... so we are stuck with them!
@@ -95,8 +97,6 @@ const uint16_t RS_SERVICE_TYPE_PHOTO       = 0xf040;
 /* DSDV Testing at the moment - Service Only */
 const uint16_t RS_SERVICE_TYPE_DSDV        = 0xf050;
 
-/* BanList Testing at the moment - Service Only */
-const uint16_t RS_SERVICE_TYPE_BANLIST     = 0xf060;
 
 /* Games/External Apps - Service Only */
 const uint16_t RS_SERVICE_TYPE_GAME_LAUNCHER = 0xf200;

libretroshare/src/services/p3banlist.cc

@@ -234,6 +234,10 @@ bool p3BanList::addBanEntry(const std::string &peerId, const struct sockaddr_in
 	return updated;
 }
 
+/***
+ * EXTRA DEBUGGING.
+ * #define DEBUG_BANLIST_CONDENSE		1
+ ***/
 
 int p3BanList::condenseBanSources_locked()
 {
@@ -250,7 +254,7 @@ int p3BanList::condenseBanSources_locked()
 	{
 		if (now - it->second.mLastUpdate > RSBANLIST_ENTRY_MAX_AGE)
 		{
-#ifdef DEBUG_BANLIST
+#ifdef DEBUG_BANLIST_CONDENSE
 			std::cerr << "p3BanList::condenseBanSources_locked()";
 			std::cerr << " Ignoring Out-Of-Date peer: " << it->first;
 			std::cerr << std::endl;
@@ -258,7 +262,7 @@ int p3BanList::condenseBanSources_locked()
 			continue;
 		}
 
-#ifdef DEBUG_BANLIST
+#ifdef DEBUG_BANLIST_CONDENSE
 		std::cerr << "p3BanList::condenseBanSources_locked()";
 		std::cerr << " Condensing Info from peer: " << it->first;
 		std::cerr << std::endl;
@@ -271,7 +275,7 @@ int p3BanList::condenseBanSources_locked()
 			/* check timestamp */
 			if (now - lit->second.mTs > RSBANLIST_ENTRY_MAX_AGE)
 			{
-#ifdef DEBUG_BANLIST
+#ifdef DEBUG_BANLIST_CONDENSE
 				std::cerr << "p3BanList::condenseBanSources_locked()";
 				std::cerr << " Ignoring Out-Of-Date Entry for: ";
 				std::cerr << rs_inet_ntoa(lit->second.addr.sin_addr);
@@ -296,7 +300,7 @@ int p3BanList::condenseBanSources_locked()
 				bp.level = lvl;
 				bp.addr.sin_port = 0;
 				mBanSet[lit->second.addr.sin_addr.s_addr] = bp;
-#ifdef DEBUG_BANLIST
+#ifdef DEBUG_BANLIST_CONDENSE
 				std::cerr << "p3BanList::condenseBanSources_locked()";
 				std::cerr << " Added New Entry for: ";
 				std::cerr << rs_inet_ntoa(lit->second.addr.sin_addr);
@@ -305,7 +309,7 @@ int p3BanList::condenseBanSources_locked()
 			}
 			else
 			{
-#ifdef DEBUG_BANLIST
+#ifdef DEBUG_BANLIST_CONDENSE
 				std::cerr << "p3BanList::condenseBanSources_locked()";
 				std::cerr << " Merging Info for: ";
 				std::cerr << rs_inet_ntoa(lit->second.addr.sin_addr);

libretroshare/src/tcponudp/udpstunner.cc

@@ -290,13 +290,8 @@ int     UdpStunner::status(std::ostream &out)
 	out << std::endl;
 
 	out << "UdpStunner::status()" << std::endl;
-	out << "UdpStunner::potentialpeers:" << std::endl;
+
-        std::list<TouStunPeer>::iterator it;
+	locked_printStunList();
-	for(it = mStunList.begin(); it != mStunList.end(); it++)
-	{
-		out << "\t" << it->id << std::endl;
-	}
-	out << std::endl;
 
 	return 1;
 }
@@ -693,6 +688,55 @@ bool    UdpStunner::storeStunPeer(const struct sockaddr_in &remote, const char *
 }
 
 
+bool    UdpStunner::dropStunPeer(const struct sockaddr_in &remote)
+{
+
+#ifdef DEBUG_UDP_STUNNER
+	std::cerr << "UdpStunner::dropStunPeer() : ";
+	std::cerr << std::endl;
+#endif
+
+        RsStackMutex stack(stunMtx);   /********** LOCK MUTEX *********/
+
+	std::list<TouStunPeer>::iterator it;
+	int count = 0;
+	for(it = mStunList.begin(); it != mStunList.end();)
+	{
+		if ((remote.sin_addr.s_addr == it->remote.sin_addr.s_addr) &&
+		    (remote.sin_port == it->remote.sin_port))
+		{
+#ifdef DEBUG_UDP_STUNNER
+			std::cerr << "UdpStunner::dropStunPeer() Found Entry";
+			std::cerr << std::endl;
+#endif
+
+			it = mStunList.erase(it);
+			count++;
+		}
+		else
+		{
+			it++;
+		}
+	}
+
+	if (count)
+	{
+#ifdef DEBUG_UDP_STUNNER
+		std::cerr << "UdpStunner::dropStunPeer() Dropped " << count << " Instances";
+		std::cerr << std::endl;
+#endif
+		return true;
+	}
+
+#ifdef DEBUG_UDP_STUNNER
+	std::cerr << "UdpStunner::dropStunPeer() Peer Not Here";
+	std::cerr << std::endl;
+#endif
+
+	return false;
+}
+
+
 bool    UdpStunner::checkStunDesired()
 {
 
@@ -891,6 +935,23 @@ bool    UdpStunner::locked_recvdStun(const struct sockaddr_in &remote, const str
 	}
 #endif
 
+	/* sanoty checks on the address 
+	 * have nasty peer that is returning its own address....
+	 */
+
+#ifndef UDPSTUN_ALLOW_LOCALNET // CANNOT HAVE THIS CHECK IN TESTING MODE!
+
+	if (remote.sin_addr.s_addr == extaddr.sin_addr.s_addr)
+	{
+#ifdef DEBUG_UDP_STUNNER
+#endif
+		std::cerr << "UdpStunner::locked_recvdStun() WARNING, BAD PEER: ";
+		std::cerr << "Stun Peer Returned its own address: " << rs_inet_ntoa(remote.sin_addr);
+		std::cerr << std::endl;
+		return false;
+	}
+#endif
+
 	bool found = true;
 	std::list<TouStunPeer>::iterator it;
 	for(it = mStunList.begin(); it != mStunList.end(); it++)
@@ -964,6 +1025,15 @@ bool    UdpStunner::locked_checkExternalAddress()
 	bool found2 = false;
 	time_t now = time(NULL);
 	/* iterator backwards - as these are the most recent */
+
+	/********
+	 *  DUE TO PEERS SENDING BACK FAKE STUN PACKETS... we are increasing.
+	 *  requirements to three peers...they all need matching IP addresses to have a known ExtAddr
+	 *
+	 * Wanted to compare 3 peer addresses... but this will mean that the UDP connections
+	 * will take much longer... have to think of a better solution.
+	 *
+	 */
 	std::list<TouStunPeer>::reverse_iterator it;
 	std::list<TouStunPeer>::reverse_iterator p1;
 	std::list<TouStunPeer>::reverse_iterator p2;
@@ -983,7 +1053,7 @@ bool    UdpStunner::locked_checkExternalAddress()
 #else
 			(isExternalNet(&(it->eaddr.sin_addr))) &&
 #endif
-			(it->failCount == 0) && (age < (mTargetStunPeriod * 2)))
+			(it->failCount == 0) && (age < (mTargetStunPeriod * 2))) 
 		{
 			if (!found1)
 			{
@@ -1001,7 +1071,30 @@ bool    UdpStunner::locked_checkExternalAddress()
 
 	if (found1 && found2)
 	{
-		if ((p1->eaddr.sin_addr.s_addr == p2->eaddr.sin_addr.s_addr) &&
+		/* If any of the addresses are different - two possibilities...
+		 * 1) We have changed IP address.
+		 * 2) Someone has sent us a fake STUN Packet. (Wrong Address).
+		 *
+		 */
+		if (p1->eaddr.sin_addr.s_addr == p2->eaddr.sin_addr.s_addr)
+		{
+			eaddrKnown = true;
+		}
+		else
+		{
+#ifdef DEBUG_UDP_STUNNER
+			std::cerr << "UdpStunner::locked_checkExternalAddress() Found Address mismatch:";
+			std::cerr << std::endl;
+			std::cerr << "  " << inet_ntoa(p1->eaddr.sin_addr);
+			std::cerr << "  " << inet_ntoa(p2->eaddr.sin_addr);
+			std::cerr << std::endl;
+			std::cerr << "UdpStunner::locked_checkExternalAddress() Flagging Ext Addr as Unknown";
+			std::cerr << std::endl;
+#endif
+			eaddrKnown = false;
+		}
+
+		if ((eaddrKnown) &&	
 		    (p1->eaddr.sin_port == p2->eaddr.sin_port))
 		{
 			eaddrStable = true;
@@ -1010,7 +1103,7 @@ bool    UdpStunner::locked_checkExternalAddress()
 		{
 			eaddrStable = false;
 		}
-		eaddrKnown = true;
+
 		eaddr = p1->eaddr;
 		eaddrTime = now;
 
@@ -1019,7 +1112,8 @@ bool    UdpStunner::locked_checkExternalAddress()
 		if (eaddrStable)
 			std::cerr << " Stable NAT translation (GOOD!) ";
 		else
-			std::cerr << " unStable (symmetric NAT translation (BAD!) ";
+			std::cerr << " unStable (symmetric NAT translation (BAD!) or Address Unknown";
+
 		std::cerr << std::endl;
 #endif
 

libretroshare/src/tcponudp/udpstunner.h

@@ -94,6 +94,8 @@ int     releaseExclusiveMode(std::string holder, bool forceStun);
 
 void 	setTargetStunPeriod(int32_t sec_per_stun);
 bool    addStunPeer(const struct sockaddr_in &remote, const char *peerid);
+bool    dropStunPeer(const struct sockaddr_in &remote);
+
 bool    getStunPeer(int idx, std::string &id,
                 struct sockaddr_in &remote, struct sockaddr_in &eaddr,
                 uint32_t &failCount, time_t &lastSend);