From b6dbdf9396ec3445471d67651783bac3f709a0d9 Mon Sep 17 00:00:00 2001 From: drbob Date: Thu, 8 Dec 2011 20:00:20 +0000 Subject: [PATCH 1/2] Improvements to BadPeer Filter. * Enabling Local BadPeer Filter. - This will remove any peer you detect is spoofing yourself or your friends. - This list is also shared with you friends. (in Test Mode). * added Cleanup of BadPeer Filter. - Instead of permanent ban, peers are be banned for 6 hours. - bdManager periodically calls this - which prints out ban list too. * added #define to disable the Filter - for testing purposes. NOTES: This Ip Filter should probably be moved from DHT level to UdpLayer level. This will enable it to filter STUN / UDP Connection Packets too. git-svn-id: http://svn.code.sf.net/p/retroshare/code/branches/v0.5-dhtmods@4716 b45a01b8-16f6-495d-af2f-9b41ad6348cc --- libbitdht/src/bitdht/bdfilter.cc | 55 +++++++++++++++++++++++++++++++ libbitdht/src/bitdht/bdfilter.h | 4 ++- libbitdht/src/bitdht/bdmanager.cc | 9 +++++ libbitdht/src/bitdht/bdnode.cc | 55 ++++++++++--------------------- 4 files changed, 84 insertions(+), 39 deletions(-) diff --git a/libbitdht/src/bitdht/bdfilter.cc b/libbitdht/src/bitdht/bdfilter.cc index 769d1a6c2..49fccf2c5 100644 --- a/libbitdht/src/bitdht/bdfilter.cc +++ b/libbitdht/src/bitdht/bdfilter.cc @@ -36,6 +36,8 @@ * #define DEBUG_FILTER 1 **/ +#define BDFILTER_ENTRY_DROP_PERIOD (6 * 3600) + bdFilter::bdFilter(const bdNodeId *ownId, std::list &startList, uint32_t filterFlags, bdDhtFunctions *fns) @@ -129,6 +131,7 @@ int bdFilter::addPeerToFilter(const bdId *id, uint32_t flags) return true; } + return false; } @@ -171,4 +174,56 @@ bool bdFilter::isOwnIdWithoutBitDhtFlags(const bdId *id, uint32_t peerFlags) } +/* periodically we want to cleanup the filter.... + * if we haven't had an IP address reported as filtered for several hours. + * remove it from the list. + */ + +bool bdFilter::cleanupFilter() +{ + std::cerr << "bdFilter::cleanupFilter() Current BanList" << std::endl; + struct in_addr inaddr; + + std::set::iterator sit; + for(sit = mIpsBanned.begin(); sit != mIpsBanned.end(); sit++) + { + inaddr.s_addr = *sit; + std::cerr << "\tBanned: " << inet_ntoa(inaddr) << std::endl; + } + + mIpsBanned.clear(); + + std::cerr << "Filter List:" << std::endl; + + time_t now = time(NULL); + time_t dropTime = now - BDFILTER_ENTRY_DROP_PERIOD; + + std::list::iterator it; + for(it = mFiltered.begin(); it != mFiltered.end();) + { + std::cerr << "\t" << inet_ntoa(it->mAddr.sin_addr); + std::cerr << " Flags: " << it->mFilterFlags; + std::cerr << " FilterTS: " << now - it->mFilterTS; + std::cerr << " LastSeen: " << now - it->mLastSeen; + + if (it->mLastSeen < dropTime) + { + /* remove from filter */ + std::cerr << " OLD DROPPING" << std::endl; + it = mFiltered.erase(it); + } + else + { + std::cerr << " OK" << std::endl; + uint32_t saddr = it->mAddr.sin_addr.s_addr; + mIpsBanned.insert(saddr); + + it++; + } + } + + return true; +} + + diff --git a/libbitdht/src/bitdht/bdfilter.h b/libbitdht/src/bitdht/bdfilter.h index b18f64ad7..f222c6d22 100644 --- a/libbitdht/src/bitdht/bdfilter.h +++ b/libbitdht/src/bitdht/bdfilter.h @@ -60,10 +60,12 @@ bool filteredIPs(std::list &answer); int checkPeer(const bdId *id, uint32_t peerFlags); int addrOkay(struct sockaddr_in *addr); +int addPeerToFilter(const bdId *id, uint32_t flags); + +bool cleanupFilter(); private: -int addPeerToFilter(const bdId *id, uint32_t flags); bool isOwnIdWithoutBitDhtFlags(const bdId *id, uint32_t peerFlags); // searching for diff --git a/libbitdht/src/bitdht/bdmanager.cc b/libbitdht/src/bitdht/bdmanager.cc index 0833468f0..a7ee7f256 100644 --- a/libbitdht/src/bitdht/bdmanager.cc +++ b/libbitdht/src/bitdht/bdmanager.cc @@ -43,6 +43,7 @@ #include "bitdht/bdmsgs.h" #include "bitdht/bencode.h" #include "bitdht/bdquerymgr.h" +#include "bitdht/bdfilter.h" #include #include @@ -386,6 +387,14 @@ void bdNodeManager::iteration() updateStore(); +#ifdef DEBUG_MGR + std::cerr << "bdNodeManager::iteration(): Cleaning up Filter (should do less frequently)"; + std::cerr << std::endl; +#endif + + mFilterPeers->cleanupFilter(); + + #ifdef DEBUG_MGR std::cerr << "bdNodeManager::iteration(): Do App Search"; std::cerr << std::endl; diff --git a/libbitdht/src/bitdht/bdnode.cc b/libbitdht/src/bitdht/bdnode.cc index 4ebcd8676..8b88cdd2e 100644 --- a/libbitdht/src/bitdht/bdnode.cc +++ b/libbitdht/src/bitdht/bdnode.cc @@ -3,7 +3,7 @@ * * BitDHT: An Flexible DHT library. * - * Copyright 2010 by Robert Fernie + * Copyright 2010-2011 by Robert Fernie * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Library General Public @@ -428,8 +428,7 @@ void bdNode::send_connect_msg(bdId *id, int msgtype, bdId *srcAddr, bdId *destAd - -#define TEST_BAD_PEER 1 +//#define DISABLE_BAD_PEER_FILTER 1 void bdNode::checkPotentialPeer(bdId *id, bdId *src) { @@ -443,12 +442,7 @@ void bdNode::checkPotentialPeer(bdId *id, bdId *src) std::cerr << ") BAD ADDRESS!!!! SHOULD DISCARD POTENTIAL PEER"; std::cerr << std::endl; -#ifdef TEST_BAD_PEER - std::cerr << "IN TEST MODE... so letting it through."; - std::cerr << std::endl; -#else return; -#endif } /* is it masquarading? */ @@ -465,17 +459,12 @@ void bdNode::checkPotentialPeer(bdId *id, bdId *src) std::cerr << ") MASQARADING AS KNOWN PEER - FLAGGING AS BAD"; std::cerr << std::endl; -#ifdef TEST_BAD_PEER - std::cerr << "IN TEST MODE... Notifying, but letting it through."; - std::cerr << std::endl; - - mBadPeerQueue.queuePeer(id, 0); -#else - - mFilterPeers->addBadPeer(id, 0); // Stores in queue for later callback and desemination around the network. mBadPeerQueue.queuePeer(id, 0); +#ifndef DISABLE_BAD_PEER_FILTER + mFilterPeers->addPeerToFilter(id, 0); + std::list filteredIPs; mFilterPeers->filteredIPs(filteredIPs); mStore.filterIpList(filteredIPs); @@ -533,17 +522,14 @@ void bdNode::addPeer(const bdId *id, uint32_t peerflags) mFilterPeers->filteredIPs(filteredIPs); mStore.filterIpList(filteredIPs); + mBadPeerQueue.queuePeer(id, peerflags); + return; } -// NB: TODO CLEANUP THIS CODE - ONCE LOGIC IS TESTED! - /* next we check if it is a friend, whitelist etc, and adjust flags */ bdFriendEntry entry; -#ifdef TEST_BAD_PEER - bool peerBad = false; -#endif if (mFriendList.findPeerEntry(&(id->id), entry)) { /* found! */ @@ -560,21 +546,26 @@ void bdNode::addPeer(const bdId *id, uint32_t peerflags) std::cerr << ") MASQARADING AS KNOWN PEER - FLAGGING AS BAD"; std::cerr << std::endl; -#ifdef TEST_BAD_PEER - peerBad = true; -#else - mFilterPeers->addBadPeer(id, peerflags); + // Stores in queue for later callback and desemination around the network. - mBadPeerList->queuePeer(id, peerflags); + mBadPeerQueue.queuePeer(id, peerflags); + +#ifndef DISABLE_BAD_PEER_FILTER + mFilterPeers->addPeerToFilter(id, peerflags); std::list filteredIPs; mFilterPeers->filteredIPs(filteredIPs); mStore.filterIpList(filteredIPs); +#endif // DO WE EXPLICITLY NEED TO DO THIS, OR WILL THEY JUST BE DROPPED? //mNodeSpace.remove_badpeer(id); //mQueryMgr->remove_badpeer(id); + + // FLAG in NodeSpace (Should be dropped very quickly anyway) + mNodeSpace.flagpeer(id, 0, BITDHT_PEER_EXFLAG_BADPEER); +#ifndef DISABLE_BAD_PEER_FILTER return; #endif } @@ -584,18 +575,6 @@ void bdNode::addPeer(const bdId *id, uint32_t peerflags) mQueryMgr->addPeer(id, peerflags); mNodeSpace.add_peer(id, peerflags); -#ifdef TEST_BAD_PEER - // NOTE: We will push bad peers to Query in the testing case. - // This allows us to test the multiple solutions... as well. - // In normal behaviour - they will just get stripped and never added. - if (peerBad) - { - mNodeSpace.flagpeer(id, 0, BITDHT_PEER_EXFLAG_BADPEER); - //mQueryMgr->flag_badpeer(id); - } -#endif - - bdPeer peer; peer.mPeerId = *id; peer.mPeerFlags = peerflags; From 2048bb5e47cadbb6493eb9e17eca52f10fc56ee3 Mon Sep 17 00:00:00 2001 From: drbob Date: Thu, 8 Dec 2011 20:15:08 +0000 Subject: [PATCH 2/2] 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 | 11 ++ libretroshare/src/dht/p3bitdht_peers.cc | 10 ++ libretroshare/src/serialiser/rsserviceids.h | 4 +- libretroshare/src/services/p3banlist.cc | 14 ++- libretroshare/src/tcponudp/udpstunner.cc | 116 ++++++++++++++++++-- libretroshare/src/tcponudp/udpstunner.h | 2 + 6 files changed, 139 insertions(+), 18 deletions(-) diff --git a/libretroshare/src/dht/p3bitdht_peernet.cc b/libretroshare/src/dht/p3bitdht_peernet.cc index 1f79e33e0..1787104a7 100644 --- a/libretroshare/src/dht/p3bitdht_peernet.cc +++ b/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; } diff --git a/libretroshare/src/dht/p3bitdht_peers.cc b/libretroshare/src/dht/p3bitdht_peers.cc index 21f942cbe..046f02cdd 100644 --- a/libretroshare/src/dht/p3bitdht_peers.cc +++ b/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; } diff --git a/libretroshare/src/serialiser/rsserviceids.h b/libretroshare/src/serialiser/rsserviceids.h index 5bbd10942..14394abd4 100644 --- a/libretroshare/src/serialiser/rsserviceids.h +++ b/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; diff --git a/libretroshare/src/services/p3banlist.cc b/libretroshare/src/services/p3banlist.cc index 32d1fa117..96803687e 100644 --- a/libretroshare/src/services/p3banlist.cc +++ b/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); diff --git a/libretroshare/src/tcponudp/udpstunner.cc b/libretroshare/src/tcponudp/udpstunner.cc index 0bf030874..eeb386a53 100644 --- a/libretroshare/src/tcponudp/udpstunner.cc +++ b/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::iterator it; - for(it = mStunList.begin(); it != mStunList.end(); it++) - { - out << "\t" << it->id << std::endl; - } - out << std::endl; + + locked_printStunList(); 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::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::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::reverse_iterator it; std::list::reverse_iterator p1; std::list::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 diff --git a/libretroshare/src/tcponudp/udpstunner.h b/libretroshare/src/tcponudp/udpstunner.h index 9054ad721..9d9f7b40a 100644 --- a/libretroshare/src/tcponudp/udpstunner.h +++ b/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);