From 96cccbbc75723043753bf9011973fd45dc069d08 Mon Sep 17 00:00:00 2001 From: csoler Date: Mon, 1 Jun 2015 22:03:07 +0000 Subject: [PATCH] added hability to disconnect friends which report our own IP as something different than what we know. Removed a test in discovery2 that prevented sending info to a peer about himself. Not active yet since it needs some testing git-svn-id: http://svn.code.sf.net/p/retroshare/code/trunk@8340 b45a01b8-16f6-495d-af2f-9b41ad6348cc --- libretroshare/src/pqi/p3linkmgr.cc | 33 +++++++++++++++ libretroshare/src/pqi/p3linkmgr.h | 2 + libretroshare/src/pqi/p3netmgr.cc | 49 ++++++++++++++++++---- libretroshare/src/pqi/p3netmgr.h | 2 +- libretroshare/src/pqi/p3peermgr.cc | 19 +++++++++ libretroshare/src/pqi/p3peermgr.h | 2 + libretroshare/src/pqi/pqimonitor.cc | 7 +++- libretroshare/src/pqi/pqimonitor.h | 4 ++ libretroshare/src/pqi/pqisslpersongrp.cc | 12 +++++- libretroshare/src/pqi/pqisslpersongrp.h | 4 +- libretroshare/src/services/p3discovery2.cc | 11 +++-- 11 files changed, 129 insertions(+), 16 deletions(-) diff --git a/libretroshare/src/pqi/p3linkmgr.cc b/libretroshare/src/pqi/p3linkmgr.cc index b453470c0..ae71d8e87 100644 --- a/libretroshare/src/pqi/p3linkmgr.cc +++ b/libretroshare/src/pqi/p3linkmgr.cc @@ -2218,6 +2218,39 @@ int p3LinkMgrIMPL::removeFriend(const RsPeerId &id) return 1; } +void p3LinkMgrIMPL::disconnectFriend(const RsPeerId& id) +{ + std::list disconnect_clients ; + + { + RsStackMutex stack(mLinkMtx); /****** STACK LOCK MUTEX *******/ + + disconnect_clients = clients ; + + std::cerr << "Disconnecting friend " << id << std::endl; + + std::map::iterator it; + it = mFriendList.find(id); + + if (it == mFriendList.end()) + { + std::cerr << "p3LinkMgrIMPL::removeFriend() ERROR, friend not there : " << id; + std::cerr << std::endl; + return ; + } + + /* Move to OthersList (so remove can be handled via the action) */ + peerConnectState peer = it->second; + + peer.state &= (~RS_PEER_S_CONNECTED); + peer.state &= (~RS_PEER_S_ONLINE); + peer.actions = RS_PEER_DISCONNECTED; + peer.inConnAttempt = false; + } + + for(std::list::const_iterator it(disconnect_clients.begin());it!=disconnect_clients.end();++it) + (*it)->disconnectPeer(id) ; +} void p3LinkMgrIMPL::printPeerLists(std::ostream &out) { diff --git a/libretroshare/src/pqi/p3linkmgr.h b/libretroshare/src/pqi/p3linkmgr.h index 640e8396d..1931bb10e 100644 --- a/libretroshare/src/pqi/p3linkmgr.h +++ b/libretroshare/src/pqi/p3linkmgr.h @@ -264,6 +264,8 @@ void tick(); /* THIS COULD BE ADDED TO INTERFACE */ void setFriendVisibility(const RsPeerId &id, bool isVisible); + void disconnectFriend(const RsPeerId& id) ; + /* add/remove friends */ virtual int addFriend(const RsPeerId &ssl_id, bool isVisible); int removeFriend(const RsPeerId &ssl_id); diff --git a/libretroshare/src/pqi/p3netmgr.cc b/libretroshare/src/pqi/p3netmgr.cc index 099ebe9c2..bb1805a93 100644 --- a/libretroshare/src/pqi/p3netmgr.cc +++ b/libretroshare/src/pqi/p3netmgr.cc @@ -677,6 +677,12 @@ void p3NetMgrIMPL::netUpnpCheck() } +class ZeroInt +{ + public: + ZeroInt() { n=0; } + uint32_t n ; +}; void p3NetMgrIMPL::netExtCheck() { @@ -690,10 +696,11 @@ void p3NetMgrIMPL::netExtCheck() bool isStable = false; struct sockaddr_storage tmpip ; - /* check for External Address */ + std::map address_votes ; + + /* check for External Address */ /* in order of importance */ /* (1) UPnP -> which handles itself */ - if (!mNetFlags.mExtAddrOk) { #if defined(NETMGR_DEBUG_TICK) || defined(NETMGR_DEBUG_RESET) std::cerr << "p3NetMgrIMPL::netExtCheck() Ext Not Ok" << std::endl; @@ -711,7 +718,9 @@ void p3NetMgrIMPL::netExtCheck() isStable = true; mNetFlags.mExtAddr = tmpip; mNetFlags.mExtAddrOk = true; - mNetFlags.mExtAddrStableOk = isStable; + mNetFlags.mExtAddrStableOk = isStable; + + address_votes[tmpip].n++ ; } else { @@ -724,7 +733,6 @@ void p3NetMgrIMPL::netExtCheck() } /* Next ask the DhtStunner */ - if (!mNetFlags.mExtAddrOk) { #if defined(NETMGR_DEBUG_TICK) || defined(NETMGR_DEBUG_RESET) std::cerr << "p3NetMgrIMPL::netExtCheck() Ext Not Ok, Checking DhtStunner" << std::endl; @@ -744,6 +752,7 @@ void p3NetMgrIMPL::netExtCheck() mNetFlags.mExtAddrOk = true; mNetFlags.mExtAddrStableOk = isStable; + address_votes[tmpaddr].n++ ; #ifdef NETMGR_DEBUG_STATEBOX std::cerr << "p3NetMgrIMPL::netExtCheck() From DhtStunner: "; std::cerr << sockaddr_storage_tostring(tmpaddr); @@ -755,7 +764,6 @@ void p3NetMgrIMPL::netExtCheck() } /* otherwise ask ExtAddrFinder */ - if (!mNetFlags.mExtAddrOk) { /* ExtAddrFinder */ if (mUseExtAddrFinder) @@ -782,6 +790,8 @@ void p3NetMgrIMPL::netExtCheck() mNetFlags.mExtAddrOk = true; mNetFlags.mExtAddrStableOk = isStable; + address_votes[tmpip].n++ ; + /* XXX HACK TO FIX */ #warning "ALLOWING ExtAddrFinder -> ExtAddrStableOk = true (which it is not normally)" mNetFlags.mExtAddrStableOk = true; @@ -794,7 +804,18 @@ void p3NetMgrIMPL::netExtCheck() /* finalise address */ if (mNetFlags.mExtAddrOk) - { + { + // look at votes. + + std::cerr << "Figuring out ext addr from voting:" << std::endl; + uint32_t max = 0 ; + for(std::map::const_iterator it(address_votes.begin());it!=address_votes.end();++it) + { + std::cerr << " Vote 1: " << sockaddr_storage_iptostring(it->first) << " : " << it->second.n << " votes." << std::endl; + + if(it->second.n > max) + mNetFlags.mExtAddr = it->first ; + } #if defined(NETMGR_DEBUG_TICK) || defined(NETMGR_DEBUG_RESET) std::cerr << "p3NetMgrIMPL::netExtCheck() "; @@ -802,7 +823,7 @@ void p3NetMgrIMPL::netExtCheck() std::cerr << std::endl; #endif //update ip address list - mExtAddr = mNetFlags.mExtAddr; + mExtAddr = mNetFlags.mExtAddr; mNetStatus = RS_NET_DONE; netSetupDone = true; @@ -818,7 +839,7 @@ void p3NetMgrIMPL::netExtCheck() #if defined(NETMGR_DEBUG_TICK) || defined(NETMGR_DEBUG_RESET) std::cerr << "p3NetMgrIMPL::netUdpCheck() UDP Unstable :( "; std::cerr << std::endl; - std::cerr << "p3NetMgrIMPL::netUdpCheck() We are unreachable"; + std::cerr << "p3NetMgrIMPL::netUdpCheck() We are unreachable"; std::cerr << std::endl; std::cerr << "netMode => RS_NET_MODE_UNREACHABLE"; std::cerr << std::endl; @@ -1096,6 +1117,18 @@ bool p3NetMgrIMPL::setLocalAddress(const struct sockaddr_storage &addr) } return true; } +bool p3NetMgrIMPL::getExtAddress(struct sockaddr_storage& addr) +{ + RsStackMutex stack(mNetMtx); /****** STACK LOCK MUTEX *******/ + + if(mNetFlags.mExtAddrOk) + { + addr = mExtAddr ; + return true ; + } + else + return false ; +} bool p3NetMgrIMPL::setExtAddress(const struct sockaddr_storage &addr) { diff --git a/libretroshare/src/pqi/p3netmgr.h b/libretroshare/src/pqi/p3netmgr.h index ff2bc0f4d..eb1802328 100644 --- a/libretroshare/src/pqi/p3netmgr.h +++ b/libretroshare/src/pqi/p3netmgr.h @@ -216,6 +216,7 @@ void tick(); // THESE MIGHT BE ADDED TO INTERFACE. bool setLocalAddress(const struct sockaddr_storage &addr); bool setExtAddress(const struct sockaddr_storage &addr); +bool getExtAddress(sockaddr_storage &addr); /*************** Setup ***************************/ void addNetAssistConnect(uint32_t type, pqiNetAssistConnect *); @@ -226,7 +227,6 @@ void addNetListener(pqiNetListener *listener); // SHOULD MAKE THIS PROTECTED. bool checkNetAddress(); /* check our address is sensible */ - protected: void slowTick(); diff --git a/libretroshare/src/pqi/p3peermgr.cc b/libretroshare/src/pqi/p3peermgr.cc index b311e3084..8ede822b2 100644 --- a/libretroshare/src/pqi/p3peermgr.cc +++ b/libretroshare/src/pqi/p3peermgr.cc @@ -1225,6 +1225,25 @@ bool p3PeerMgrIMPL::setDynDNS(const RsPeerId &id, const std::string &dyndns) return changed; } +bool p3PeerMgrIMPL::addCandidateForOwnExternalAddress(const RsPeerId &from, const sockaddr_storage &addr) +{ + //#ifdef PEER_DEBUG + std::cerr << "Own external address is " << sockaddr_storage_iptostring(addr) << ", as reported by friend " << from << std::endl; + //#endif + + // disconnect every friend that has reported a wrong external address + + sockaddr_storage own_addr ; + + if(mNetMgr->getExtAddress(own_addr) && !sockaddr_storage_sameip(own_addr,addr)) + { + std::cerr << "(WW) peer reports an address that is not our current external address. This is weird." << std::endl; + + //mLinkMgr->disconnectFriend(from) ; + } + return true ; +} + bool p3PeerMgrIMPL::updateAddressList(const RsPeerId& id, const pqiIpAddrSet &addrs) { #ifdef PEER_DEBUG diff --git a/libretroshare/src/pqi/p3peermgr.h b/libretroshare/src/pqi/p3peermgr.h index 658b6561b..4ec7c9673 100644 --- a/libretroshare/src/pqi/p3peermgr.h +++ b/libretroshare/src/pqi/p3peermgr.h @@ -152,6 +152,7 @@ virtual bool assignPeersToGroup(const std::string &groupId, const std::list &plist) = 0; + // This is used to force disconnection of a peer, if e.g. something suspicious happenned. + + virtual void disconnectPeer(const RsPeerId& peer) ; + #ifdef WINDOWS_SYS /////////////////////////////////////////////////////////// // hack for too many connections diff --git a/libretroshare/src/pqi/pqisslpersongrp.cc b/libretroshare/src/pqi/pqisslpersongrp.cc index 5319a6c6b..8988d8872 100644 --- a/libretroshare/src/pqi/pqisslpersongrp.cc +++ b/libretroshare/src/pqi/pqisslpersongrp.cc @@ -132,7 +132,17 @@ pqiperson * pqisslpersongrp::locked_createPerson(const RsPeerId& id, pqilistener #endif } - return pqip; + return pqip; +} + +void pqisslpersongrp::disconnectPeer(const RsPeerId &peer) +{ + std::map::iterator it = ssl_tunnels.find(peer) ; + + if(it != ssl_tunnels.end()) + it->second->disconnect() ; + else + std::cerr << "pqisslpersongrp::cannot find peer " << peer << ". cannot disconnect!" << std::endl; } diff --git a/libretroshare/src/pqi/pqisslpersongrp.h b/libretroshare/src/pqi/pqisslpersongrp.h index ec90d3036..f9544a93a 100644 --- a/libretroshare/src/pqi/pqisslpersongrp.h +++ b/libretroshare/src/pqi/pqisslpersongrp.h @@ -45,7 +45,9 @@ class pqisslpersongrp: public pqipersongrp /********* FUNCTIONS to OVERLOAD for specialisation ********/ virtual pqilistener *locked_createListener(const struct sockaddr_storage &laddr); virtual pqiperson *locked_createPerson(const RsPeerId& id, pqilistener *listener); - /********* FUNCTIONS to OVERLOAD for specialisation ********/ + /********* FUNCTIONS to OVERLOAD for specialisation ********/ + + virtual void disconnectPeer(const RsPeerId& peer) ; private: diff --git a/libretroshare/src/services/p3discovery2.cc b/libretroshare/src/services/p3discovery2.cc index 1b94ce2f7..687e23bc4 100644 --- a/libretroshare/src/services/p3discovery2.cc +++ b/libretroshare/src/services/p3discovery2.cc @@ -427,7 +427,7 @@ void p3discovery2::updatePeerAddresses(const RsDiscContactItem *item) } else { - mPeerMgr->setDynDNS(item->sslId, item->dyndns); + mPeerMgr->setDynDNS(item->sslId, item->dyndns); updatePeerAddressList(item); } @@ -797,10 +797,12 @@ void p3discovery2::sendContactInfo_locked(const PGPID &aboutId, const SSLID &toI std::cerr << std::endl; #endif - if ((sit->first == rsPeers->getOwnId()) || (sit->first == toId)) - { + if (sit->first == rsPeers->getOwnId()) + { + // sending info of toId to himself will be used by toId to check that the IP it is connected as is the same + // as its external IP. #ifdef P3DISC_DEBUG - std::cerr << "p3discovery2::processContactInfo() not sending info on self or theirself"; + std::cerr << "p3discovery2::processContactInfo() not sending info on self"; std::cerr << std::endl; #endif continue; @@ -848,6 +850,7 @@ void p3discovery2::processContactInfo(const SSLID &fromId, const RsDiscContactIt if (item->sslId == rsPeers->getOwnId()) { + mPeerMgr->addCandidateForOwnExternalAddress(item->PeerId(), item->extAddrV4.addr) ; #ifdef P3DISC_DEBUG std::cerr << "p3discovery2::processContactInfo(" << fromId << ") PGPID: "; std::cerr << item->pgpId << " Ignoring Info on self";