Made a pass on the code in p3disc. Corrected some bugs:

- suppressed an unwanted return in packet treatment
	- prevented sending info to a peer about itself
	- changed askInfoToAllPeers() such as not to discard info from peers with
	  NODISC flag (because we especially need info for these)
	- enabled receiving p3disc info even if p3discovery is disabled. Indeed,
	  disabeling p3disc is a measure of protection, so it should limit the
	  export of p3disc info, not the import.
	- removed test discarding info about dummy friends, because it is useless
	- don't discard info about peers that have the NODISC flag (meaning that we
	  especially need info for them)
	- added safety check about received GPG keys. Before we relied on this test
	  being performed by p3ConnMgr::addFriend()
	- added some debug info



git-svn-id: http://svn.code.sf.net/p/retroshare/code/branches/v0.5.0@3102 b45a01b8-16f6-495d-af2f-9b41ad6348cc
This commit is contained in:
csoler 2010-06-10 13:14:09 +00:00
parent 22f07fcc66
commit 9004aa79f0

View file

@ -146,7 +146,6 @@ int p3disc::handleIncoming()
{ {
RsDiscAskInfo *inf = NULL; RsDiscAskInfo *inf = NULL;
RsDiscReply *dri = NULL; RsDiscReply *dri = NULL;
//RsDiscIssuer *dii = NULL;
RsDiscVersion *dvi = NULL; RsDiscVersion *dvi = NULL;
RsDiscHeartbeat *dta = NULL; RsDiscHeartbeat *dta = NULL;
@ -173,9 +172,9 @@ int p3disc::handleIncoming()
} }
else if (NULL != (dta = dynamic_cast<RsDiscHeartbeat *> (item))) { else if (NULL != (dta = dynamic_cast<RsDiscHeartbeat *> (item))) {
recvHeartbeatMsg(dta); recvHeartbeatMsg(dta);
delete item; nhandled++ ;
return 1;
} }
delete item; delete item;
} }
#ifdef P3DISC_DEBUG #ifdef P3DISC_DEBUG
@ -200,13 +199,13 @@ void p3disc::statusChange(const std::list<pqipeer> &plist)
sendOwnVersion(pit->id); sendOwnVersion(pit->id);
sendAllInfoToJustConnectedPeer(pit->id); sendAllInfoToJustConnectedPeer(pit->id);
sendJustConnectedPeerInfoToAllPeer(pit->id); sendJustConnectedPeerInfoToAllPeer(pit->id);
} else if (!(pit->state & RS_PEER_S_FRIEND) && (pit->actions & RS_PEER_MOVED)) {
this->removeFriend(pit->id);
} else if ((pit->state & RS_PEER_S_FRIEND) && (pit->actions & RS_PEER_NEW)) {
this->askInfoToAllPeers(pit->id);
} }
else if (!(pit->state & RS_PEER_S_FRIEND) && (pit->actions & RS_PEER_MOVED))
removeFriend(pit->id);
else if ((pit->state & RS_PEER_S_FRIEND) && (pit->actions & RS_PEER_NEW))
askInfoToAllPeers(pit->id);
} }
#ifdef P3DISC_DEBUG #ifdef P3DISC_DEBUG
std::cerr << "p3disc::statusChange() finished." << std::endl; std::cerr << "p3disc::statusChange() finished." << std::endl;
#endif #endif
return; return;
@ -333,7 +332,10 @@ void p3disc::sendPeerDetails(std::string to, std::string about) {
std::list<std::string> sslChilds; std::list<std::string> sslChilds;
rsPeers->getSSLChildListOfGPGId(about, sslChilds); rsPeers->getSSLChildListOfGPGId(about, sslChilds);
bool shouldWeSendGPGKey = false;//the GPG key is send only if we've got a valid friend with DISC enabled bool shouldWeSendGPGKey = false;//the GPG key is send only if we've got a valid friend with DISC enabled
for (std::list<std::string>::iterator sslChildIt = sslChilds.begin(); sslChildIt != sslChilds.end(); sslChildIt++) {
for (std::list<std::string>::iterator sslChildIt = sslChilds.begin(); sslChildIt != sslChilds.end(); sslChildIt++)
if(to != *sslChildIt) // We don't send info to a peer about itself, but we allow sending info
{ // about peers with the same GPG id.
peerConnectState detail; peerConnectState detail;
if (!mConnMgr->getFriendNetStatus(*sslChildIt, detail) || detail.visState & RS_VIS_STATE_NODISC) { if (!mConnMgr->getFriendNetStatus(*sslChildIt, detail) || detail.visState & RS_VIS_STATE_NODISC) {
continue; continue;
@ -356,9 +358,11 @@ void p3disc::sendPeerDetails(std::string to, std::string about) {
} }
//send own details //send own details
if (about == rsPeers->getGPGOwnId()) { if (about == rsPeers->getGPGOwnId())
{
peerConnectState detail; peerConnectState detail;
if (mConnMgr->getOwnNetStatus(detail)) { if (mConnMgr->getOwnNetStatus(detail))
{
shouldWeSendGPGKey = true; shouldWeSendGPGKey = true;
RsPeerNetItem *rsPeerNetItem = new RsPeerNetItem(); RsPeerNetItem *rsPeerNetItem = new RsPeerNetItem();
rsPeerNetItem->clear(); rsPeerNetItem->clear();
@ -451,7 +455,8 @@ void p3disc::askInfoToAllPeers(std::string about)
peerConnectState connectState; peerConnectState connectState;
if (!mConnMgr->getFriendNetStatus(about, connectState) || (connectState.visState & RS_VIS_STATE_NODISC)) { if (!mConnMgr->getFriendNetStatus(about, connectState)) // || (connectState.visState & RS_VIS_STATE_NODISC)) {
{
#ifdef P3DISC_DEBUG #ifdef P3DISC_DEBUG
std::cerr << "p3disc::askInfoToAllPeers() friend disc is off, don't send the request." << std::endl; std::cerr << "p3disc::askInfoToAllPeers() friend disc is off, don't send the request." << std::endl;
#endif #endif
@ -500,11 +505,12 @@ void p3disc::askInfoToAllPeers(std::string about)
void p3disc::recvPeerDetails(RsDiscReply *item) void p3disc::recvPeerDetails(RsDiscReply *item)
{ {
// if off discard item. // discovery is only disabled for sending, not for receiving.
peerConnectState detail; // // if off discard item.
if (!mConnMgr->getOwnNetStatus(detail) || (detail.visState & RS_VIS_STATE_NODISC)) { // peerConnectState detail;
return; // if (!mConnMgr->getOwnNetStatus(detail) || (detail.visState & RS_VIS_STATE_NODISC)) {
} // return;
// }
#ifdef P3DISC_DEBUG #ifdef P3DISC_DEBUG
std::cerr << "p3disc::recvPeerFriendMsg() From: " << item->PeerId() << " About " << item->aboutId << std::endl; std::cerr << "p3disc::recvPeerFriendMsg() From: " << item->PeerId() << " About " << item->aboutId << std::endl;
@ -523,11 +529,13 @@ void p3disc::recvPeerDetails(RsDiscReply *item)
return; return;
} }
for (std::list<RsPeerNetItem>::iterator pitem = item->rsPeerList.begin(); pitem != item->rsPeerList.end(); pitem++) { for (std::list<RsPeerNetItem>::iterator pitem = item->rsPeerList.begin(); pitem != item->rsPeerList.end(); pitem++)
{
// there's no way we can receive from a peer some info about our own dummy friend
//don't update dummy friends //don't update dummy friends
if (rsPeers->isDummyFriend(pitem->pid)) { //if (rsPeers->isDummyFriend(pitem->pid)) {
continue; // continue;
} //}
addDiscoveryData(item->PeerId(), pitem->pid, pitem->currentlocaladdr, pitem->currentremoteaddr, 0, time(NULL)); addDiscoveryData(item->PeerId(), pitem->pid, pitem->currentlocaladdr, pitem->currentremoteaddr, 0, time(NULL));
@ -535,52 +543,75 @@ void p3disc::recvPeerDetails(RsDiscReply *item)
std::cerr << "p3disc::recvPeerFriendMsg() Peer Config Item:" << std::endl; std::cerr << "p3disc::recvPeerFriendMsg() Peer Config Item:" << std::endl;
pitem->print(std::cerr, 10); pitem->print(std::cerr, 10);
std::cerr << std::endl; std::cerr << std::endl;
#endif #endif
if (pitem->pid != rsPeers->getOwnId()) { if (pitem->pid != rsPeers->getOwnId())
//check that the friend is not a deleted one {
if (deletedSSLFriendsIds.find(pitem->pid) == deletedSSLFriendsIds.end()) { // Apparently, the connect manager won't add a friend if the gpg id is not
//|| { // trusted. However, this should be tested here for consistency and security
mConnMgr->addFriend(pitem->pid, pitem->gpg_id, pitem->netMode, RS_VIS_STATE_NODISC, 0); //add with no disc by default. If friend already exist, it will do nothing // in case of modifications in mConnMgr.
} else if (pitem->lastContact > ((uint32_t)deletedSSLFriendsIds[pitem->pid] + 3600*24)) { // the friend was seen up and running 24 hours after we deleted it, we will readd it //
mConnMgr->addFriend(pitem->pid, pitem->gpg_id, pitem->netMode, RS_VIS_STATE_NODISC, 0); //add with no disc by default. If friend already exist, it will do nothing if(AuthGPG::getAuthGPG()->isGPGAccepted(pitem->gpg_id) || pitem->gpg_id == AuthGPG::getAuthGPG()->getGPGOwnId())
} {
// Add with no disc by default. If friend already exists, it will do nothing
//
#ifdef P3DISC_DEBUG
std::cerr << "--> Adding to friends list " << pitem->pid << " - " << pitem->gpg_id << std::endl;
#endif
mConnMgr->addFriend(pitem->pid, pitem->gpg_id, pitem->netMode, 0, 0);
RsPeerDetails storedDetails; RsPeerDetails storedDetails;
if (rsPeers->getPeerDetails(pitem->pid, storedDetails) //update only if we got a detail
&& ((!(storedDetails.state & RS_PEER_CONNECTED) && storedDetails.lastConnect < (pitem->lastContact - 10000)) // Update if know this peer, and if it's not already connected.
|| item->PeerId() == pitem->pid)) { //update if it's fresh info or if it's from the peer itself //
//their info is fresher than ours (there is a 10000 seconds margin), update ours if(rsPeers->getPeerDetails(pitem->pid, storedDetails) && !(storedDetails.state & RS_PEER_CONNECTED))
{
#ifdef P3DISC_DEBUG
std::cerr << "Friend is not connected -> updating info" << std::endl;
std::cerr << " -> network mode: " << pitem->netMode << std::endl;
std::cerr << " -> location: " << pitem->location << std::endl;
#endif
// Update if it's fresh info or if it's from the peer itself
// their info is fresher than ours, update ours
//
mConnMgr->setNetworkMode(pitem->pid, pitem->netMode);
mConnMgr->setLocation(pitem->pid, pitem->location);
// The info from the peer itself is ultimately trustable, so we can override some info,
// such as:
// - local and global addresses
//
if (item->PeerId() == pitem->pid)
{
#ifdef P3DISC_DEBUG
std::cerr << "Info sent by the peer itself -> updating self info:" << std::endl;
std::cerr << " -> current local addr = " << pitem->currentlocaladdr << std::endl;
std::cerr << " -> current remote addr = " << pitem->currentremoteaddr << std::endl;
std::cerr << " -> clearing NODISC flag " << std::endl;
#endif
mConnMgr->setLocalAddress(pitem->pid, pitem->currentlocaladdr); mConnMgr->setLocalAddress(pitem->pid, pitem->currentlocaladdr);
mConnMgr->setExtAddress(pitem->pid, pitem->currentremoteaddr); mConnMgr->setExtAddress(pitem->pid, pitem->currentremoteaddr);
mConnMgr->setNetworkMode(pitem->pid, pitem->netMode); pitem->visState &= ~RS_VIS_STATE_NODISC ;
if (item->PeerId() == pitem->pid) { mConnMgr->setVisState(pitem->pid, pitem->visState);
mConnMgr->setVisState(pitem->pid, pitem->visState); //update vistate only if it's from the peer itself
}
if (storedDetails.location == "") {
mConnMgr->setLocation(pitem->pid, pitem->location);
} }
} }
#ifdef P3DISC_DEBUG
std::cerr << "Friend is already connected -> not updating" << std::endl;
#endif
//useless, we will exploit the list in the connect manager // allways update address list, except if it's ours
// } else {
// if (pitem->currentremoteaddr.sin_addr.s_addr != 0 && pitem->currentremoteaddr.sin_port != 0 &&
// pitem->currentremoteaddr.sin_addr.s_addr != 1 && pitem->currentremoteaddr.sin_port != 1 &&
// std::string(inet_ntoa(pitem->currentremoteaddr.sin_addr)) != "1.1.1.1" &&
// (!isLoopbackNet(&pitem->currentremoteaddr.sin_addr)) &&
// (!isPrivateNet(&pitem->currentremoteaddr.sin_addr))
// ) {
// //the current server address given by the peer looks nice, let's use it for our own ext address if needed
// sockaddr_in tempAddr;
// if (!mConnMgr->getExtFinderExtAddress(tempAddr) && !mConnMgr->getUpnpExtAddress(tempAddr)) {
// //don't change the port, just the ip
// pitem->currentremoteaddr.sin_port = mConnMgr->ownState.currentserveraddr.sin_port;
// mConnMgr->setExtAddress(mConnMgr->getOwnId(), pitem->currentremoteaddr);
// }
// }
}
//allways update address list
mConnMgr->setAddressList(pitem->pid, pitem->ipAddressList); mConnMgr->setAddressList(pitem->pid, pitem->ipAddressList);
}
#ifdef P3DISC_DEBUG
else
std::cerr << " skipping unknown gpg id " << pitem->gpg_id << std::endl ;
#endif
}
#ifdef P3DISC_DEBUG
else
std::cerr << "Skipping info about own id " << pitem->pid << std::endl ;
#endif
} }
rsicontrol->getNotify().notifyListChange(NOTIFY_LIST_NEIGHBOURS, NOTIFY_TYPE_MOD); rsicontrol->getNotify().notifyListChange(NOTIFY_LIST_NEIGHBOURS, NOTIFY_TYPE_MOD);
/* cleanup (handled by caller) */ /* cleanup (handled by caller) */