added a flag in peerState specific to short invites, and several checks for consistency

This commit is contained in:
csoler 2019-05-22 21:46:11 +02:00
parent dac76439bd
commit 56e591f728
No known key found for this signature in database
GPG Key ID: 7BCA522266C0804C
4 changed files with 91 additions and 13 deletions

View File

@ -89,7 +89,7 @@ static const std::string kConfigKeyProxyServerPortI2P = "PROXY_SERVER_PORT_I2P";
void printConnectState(std::ostream &out, peerState &peer); void printConnectState(std::ostream &out, peerState &peer);
peerState::peerState() peerState::peerState()
:netMode(RS_NET_MODE_UNKNOWN), vs_disc(RS_VS_DISC_FULL), vs_dht(RS_VS_DHT_FULL), lastcontact(0), :skip_pgp_signature_validation(false),netMode(RS_NET_MODE_UNKNOWN), vs_disc(RS_VS_DISC_FULL), vs_dht(RS_VS_DHT_FULL), lastcontact(0),
hiddenNode(false), hiddenPort(0), hiddenType(RS_HIDDEN_TYPE_NONE) hiddenNode(false), hiddenPort(0), hiddenType(RS_HIDDEN_TYPE_NONE)
{ {
sockaddr_storage_clear(localaddr); sockaddr_storage_clear(localaddr);
@ -338,17 +338,31 @@ bool p3PeerMgrIMPL::isFriend(const RsPeerId& id)
#ifdef PEER_DEBUG_COMMON #ifdef PEER_DEBUG_COMMON
std::cerr << "p3PeerMgrIMPL::isFriend(" << id << ") called" << std::endl; std::cerr << "p3PeerMgrIMPL::isFriend(" << id << ") called" << std::endl;
#endif #endif
RsStackMutex stack(mPeerMtx); /****** STACK LOCK MUTEX *******/ RS_STACK_MUTEX(mPeerMtx);
bool ret = (mFriendList.end() != mFriendList.find(id)); bool ret = (mFriendList.end() != mFriendList.find(id));
#ifdef PEER_DEBUG_COMMON #ifdef PEER_DEBUG_COMMON
std::cerr << "p3PeerMgrIMPL::isFriend(" << id << ") returning : " << ret << std::endl; std::cerr << "p3PeerMgrIMPL::isFriend(" << id << ") returning : " << ret << std::endl;
#endif #endif
return ret; return ret;
} }
bool p3PeerMgrIMPL::isSslOnlyFriend(const RsPeerId& id)
{
#ifdef PEER_DEBUG_COMMON
std::cerr << "p3PeerMgrIMPL::isFriend(" << id << ") called" << std::endl;
#endif
RS_STACK_MUTEX(mPeerMtx);
auto it = mFriendList.find(id);
bool ret = it != mFriendList.end() && it->second.skip_pgp_signature_validation ;
#ifdef PEER_DEBUG_COMMON
std::cerr << "p3PeerMgrIMPL::isFriend(" << id << ") returning : " << ret << std::endl;
#endif
return ret;
}
bool p3PeerMgrIMPL::getPeerName(const RsPeerId &ssl_id, std::string &name) bool p3PeerMgrIMPL::getPeerName(const RsPeerId &ssl_id, std::string &name)
{ {
RsStackMutex stack(mPeerMtx); /****** STACK LOCK MUTEX *******/ RS_STACK_MUTEX(mPeerMtx);
/* check for existing */ /* check for existing */
std::map<RsPeerId, peerState>::iterator it; std::map<RsPeerId, peerState>::iterator it;
@ -915,9 +929,7 @@ bool p3PeerMgrIMPL::addFriend(const RsPeerId& input_id, const RsPgpId& input_gpg
if (id == AuthSSL::getAuthSSL()->OwnId()) if (id == AuthSSL::getAuthSSL()->OwnId())
{ {
#ifdef PEER_DEBUG RsErr() << "p3PeerMgrIMPL::addFriend() cannot add own id as a friend. That's a bug!" << std::endl;
std::cerr << "p3PeerMgrIMPL::addFriend() cannot add own id as a friend." << std::endl;
#endif
/* (1) already exists */ /* (1) already exists */
return false; return false;
} }
@ -937,9 +949,20 @@ bool p3PeerMgrIMPL::addFriend(const RsPeerId& input_id, const RsPgpId& input_gpg
#ifdef PEER_DEBUG #ifdef PEER_DEBUG
std::cerr << "p3PeerMgrIMPL::addFriend() Already Exists" << std::endl; std::cerr << "p3PeerMgrIMPL::addFriend() Already Exists" << std::endl;
#endif #endif
/* (1) already exists */ if(it->second.gpg_id.isNull()) // already exists as a SSL-only friend
{
it->second.gpg_id = input_gpg_id;
it->second.skip_pgp_signature_validation = false;
return true; return true;
} }
else if(it->second.gpg_id != input_gpg_id)// already exists as a friend with a different PGP id!!
{
RsErr() << "Trying to add SSL id (" << id << ") that is already a friend with existing PGP key (" << it->second.gpg_id << ") but using a different PGP key (" << input_gpg_id << "). This is a bug!" << std::endl;
return false;
}
else
return true; /* (1) already exists */
}
//Authentication is now tested at connection time, we don't store the ssl cert anymore //Authentication is now tested at connection time, we don't store the ssl cert anymore
// //
@ -973,6 +996,15 @@ bool p3PeerMgrIMPL::addFriend(const RsPeerId& input_id, const RsPgpId& input_gpg
it->second.netMode = netMode; it->second.netMode = netMode;
it->second.lastcontact = lastContact; it->second.lastcontact = lastContact;
if(!it->second.gpg_id.isNull() && it->second.gpg_id != input_gpg_id)// already exists as a friend with a different PGP id!!
{
RsErr() << "Trying to add SSL id (" << id << ") that is already known (but not friend) with existing PGP key (" << it->second.gpg_id << ") but using a different PGP key (" << input_gpg_id << "). This is a bug!" << std::endl;
return false;
}
it->second.gpg_id = input_gpg_id;
it->second.skip_pgp_signature_validation = false;
mStatusChanged = true; mStatusChanged = true;
notifyLinkMgr = true; notifyLinkMgr = true;
@ -997,6 +1029,9 @@ bool p3PeerMgrIMPL::addFriend(const RsPeerId& input_id, const RsPgpId& input_gpg
pstate.netMode = netMode; pstate.netMode = netMode;
pstate.lastcontact = lastContact; pstate.lastcontact = lastContact;
it->second.gpg_id = input_gpg_id;
it->second.skip_pgp_signature_validation = false;
/* addr & timestamps -> auto cleared */ /* addr & timestamps -> auto cleared */
mFriendList[id] = pstate; mFriendList[id] = pstate;
@ -1030,14 +1065,18 @@ bool p3PeerMgrIMPL::addFriend(const RsPeerId& input_id, const RsPgpId& input_gpg
} }
bool p3PeerMgrIMPL::addSslOnlyFriend( bool p3PeerMgrIMPL::addSslOnlyFriend( const RsPeerId& sslId, const RsPeerDetails& dt )
const RsPeerId& sslId, const RsPeerDetails& dt )
{ {
if(sslId.isNull() || sslId == getOwnId()) return false; if(sslId.isNull() || sslId == getOwnId())
{
RsErr() <<"Attempt to add yourself or a null ID as SSL-only friend (id=" << sslId << ")" << std::endl;
return false;
}
peerState pstate; peerState pstate;
{ RS_STACK_MUTEX(mPeerMtx); {
RS_STACK_MUTEX(mPeerMtx);
/* If in mOthersList -> move over */ /* If in mOthersList -> move over */
auto it = mOthersList.find(sslId); auto it = mOthersList.find(sslId);
@ -1047,8 +1086,16 @@ bool p3PeerMgrIMPL::addSslOnlyFriend(
mOthersList.erase(it); mOthersList.erase(it);
} }
} // RS_STACK_MUTEX(mPeerMtx); } // RS_STACK_MUTEX(mPeerMtx);
if(!pstate.gpg_id.isNull() && AuthGPG::getAuthGPG()->isGPGAccepted(pstate.gpg_id))
{
RsErr() << "Trying to add as SSL-only friend a peer which PGP id is already a friend. This means the code is inconsistent. Not doing this!" << std::endl;
return false;
}
pstate.gpg_id.clear();
pstate.id = sslId; pstate.id = sslId;
if(!dt.name.empty()) pstate.name = dt.name; if(!dt.name.empty()) pstate.name = dt.name;
@ -1060,6 +1107,8 @@ bool p3PeerMgrIMPL::addSslOnlyFriend(
if(dt.hiddenType) pstate.hiddenType = dt.hiddenType; if(dt.hiddenType) pstate.hiddenType = dt.hiddenType;
if(!dt.location.empty()) pstate.location = dt.location; if(!dt.location.empty()) pstate.location = dt.location;
pstate.skip_pgp_signature_validation = true;
{ RS_STACK_MUTEX(mPeerMtx); { RS_STACK_MUTEX(mPeerMtx);
mFriendList[sslId] = pstate; mFriendList[sslId] = pstate;

View File

@ -76,6 +76,14 @@ class peerState
RsPeerId id; RsPeerId id;
RsPgpId gpg_id; RsPgpId gpg_id;
// This flag is used when adding a single SSL cert as friend without adding its PGP key in the friend list. This allows to
// have short invites. However, because this represent a significant security risk, we perform multiple consistency checks
// whenever we use this flag, in particular:
// flat is true <==> friend SSL cert is in the friend list, but PGP id is not in the friend list
// PGP id is undefined and therefore set to null
bool skip_pgp_signature_validation;
uint32_t netMode; /* EXT / UPNP / UDP / HIDDEN / INVALID */ uint32_t netMode; /* EXT / UPNP / UDP / HIDDEN / INVALID */
/* visState */ /* visState */
uint16_t vs_disc; uint16_t vs_disc;
@ -133,6 +141,7 @@ public:
virtual bool removeFriend(const RsPeerId &ssl_id, bool removePgpId) = 0; virtual bool removeFriend(const RsPeerId &ssl_id, bool removePgpId) = 0;
virtual bool isFriend(const RsPeerId& ssl_id) = 0; virtual bool isFriend(const RsPeerId& ssl_id) = 0;
virtual bool isSslOnlyFriend(const RsPeerId &ssl_id)=0;
virtual bool getAssociatedPeers(const RsPgpId &gpg_id, std::list<RsPeerId> &ids) = 0; virtual bool getAssociatedPeers(const RsPgpId &gpg_id, std::list<RsPeerId> &ids) = 0;
virtual bool removeAllFriendLocations(const RsPgpId &gpgid) = 0; virtual bool removeAllFriendLocations(const RsPgpId &gpgid) = 0;
@ -255,6 +264,7 @@ public:
virtual bool removeFriend(const RsPgpId &pgp_id); virtual bool removeFriend(const RsPgpId &pgp_id);
virtual bool isFriend(const RsPeerId &ssl_id); virtual bool isFriend(const RsPeerId &ssl_id);
virtual bool isSslOnlyFriend(const RsPeerId &ssl_id);
virtual bool getAssociatedPeers(const RsPgpId &gpg_id, std::list<RsPeerId> &ids); virtual bool getAssociatedPeers(const RsPgpId &gpg_id, std::list<RsPeerId> &ids);
virtual bool removeAllFriendLocations(const RsPgpId &gpgid); virtual bool removeAllFriendLocations(const RsPgpId &gpgid);

View File

@ -571,7 +571,20 @@ bool p3Peers::isPgpFriend(const RsPgpId& pgpId)
{ return AuthGPG::getAuthGPG()->isGPGAccepted(pgpId); } { return AuthGPG::getAuthGPG()->isGPGAccepted(pgpId); }
bool p3Peers::isSslOnlyFriend(const RsPeerId& sslId) bool p3Peers::isSslOnlyFriend(const RsPeerId& sslId)
{ return isFriend(sslId) && !isPgpFriend(getGPGId(sslId)); } {
bool has_ssl_only_flag = mPeerMgr->isSslOnlyFriend(sslId) ;
if(has_ssl_only_flag)
{
if(isPgpFriend(getGPGId(sslId)))
{
RsErr() << "Peer " << sslId << " has SSL-friend-only flag but his PGP id is in the list of friends. This is inconsistent (Bug in the code). Returning false for security reasons." << std::endl;
return false;
}
return true;
}
return false;
}
bool p3Peers::isGPGAccepted(const RsPgpId &gpg_id_is_friend) bool p3Peers::isGPGAccepted(const RsPgpId &gpg_id_is_friend)
{ return isPgpFriend(gpg_id_is_friend); } { return isPgpFriend(gpg_id_is_friend); }

View File

@ -110,6 +110,12 @@ RsThread::~RsThread()
{ {
RsErr() << "Deleting a thread that is still running! Something is very wrong here and Retroshare is likely to crash because of this." << std::endl; RsErr() << "Deleting a thread that is still running! Something is very wrong here and Retroshare is likely to crash because of this." << std::endl;
print_stacktrace(); print_stacktrace();
while(isRunning())
{
std::cerr << "." << std::endl;
rstime::rs_usleep(1000*1000);
}
} }
} }