From 654d760d8426e67f4bad5558224b2dc1c2a56248 Mon Sep 17 00:00:00 2001 From: csoler Date: Thu, 19 Sep 2019 20:59:35 +0200 Subject: [PATCH] fixed comments from review of PR --- .../gossipdiscovery/gossipdiscoveryitems.cc | 10 +++++--- .../gossipdiscovery/gossipdiscoveryitems.h | 7 +++--- .../src/gossipdiscovery/p3gossipdiscovery.cc | 23 ++++++++----------- libretroshare/src/pqi/authssl.cc | 2 -- .../src/retroshare/rsgossipdiscovery.h | 2 +- libretroshare/src/rsserver/p3peers.cc | 2 +- 6 files changed, 23 insertions(+), 23 deletions(-) diff --git a/libretroshare/src/gossipdiscovery/gossipdiscoveryitems.cc b/libretroshare/src/gossipdiscovery/gossipdiscoveryitems.cc index 3d56e3bc9..bee4c9b59 100644 --- a/libretroshare/src/gossipdiscovery/gossipdiscoveryitems.cc +++ b/libretroshare/src/gossipdiscovery/gossipdiscoveryitems.cc @@ -40,7 +40,7 @@ RsItem *RsDiscSerialiser::create_item( case RsGossipDiscoveryItemType::CONTACT: return new RsDiscContactItem(); case RsGossipDiscoveryItemType::IDENTITY_LIST: return new RsDiscIdentityListItem(); default: - return NULL; + return nullptr; } return nullptr; @@ -65,13 +65,17 @@ void RsDiscPgpListItem::serial_process( void RsDiscPgpKeyItem::serial_process(RsGenericSerializer::SerializeJob j,RsGenericSerializer::SerializeContext& ctx) { RsTypeSerializer::serial_process(j,ctx,pgpKeyId,"pgpKeyId") ; - RsTypeSerializer::serial_process(j,ctx,pgpKeyData,"pgpKeyData") ; + + RsTypeSerializer::TlvMemBlock_proxy prox(bin_data,bin_len) ; + RsTypeSerializer::serial_process(j,ctx,prox,"keyData") ; } void RsDiscPgpKeyItem::clear() { pgpKeyId.clear(); - pgpKeyData.TlvClear(); + free(bin_data); + bin_data = nullptr; + bin_len=0; } void RsDiscContactItem::clear() diff --git a/libretroshare/src/gossipdiscovery/gossipdiscoveryitems.h b/libretroshare/src/gossipdiscovery/gossipdiscoveryitems.h index 5e147d2ba..f5e219b95 100644 --- a/libretroshare/src/gossipdiscovery/gossipdiscoveryitems.h +++ b/libretroshare/src/gossipdiscovery/gossipdiscoveryitems.h @@ -38,8 +38,6 @@ enum class RsGossipDiscoveryItemType : uint8_t PGP_CERT = 0x2, // deprecated CONTACT = 0x5, IDENTITY_LIST = 0x6, - INVITE = 0x7, // deprecated - INVITE_REQUEST = 0x8, // deprecated PGP_CERT_BINARY = 0x9, }; @@ -89,11 +87,14 @@ public: RsDiscPgpKeyItem() : RsDiscItem(RsGossipDiscoveryItemType::PGP_CERT_BINARY) { setPriorityLevel(QOS_PRIORITY_RS_DISC_PGP_CERT); } + virtual ~RsDiscPgpKeyItem() { delete[](bin_data);bin_data=nullptr;bin_len=0;} + void clear() override; void serial_process( RsGenericSerializer::SerializeJob j, RsGenericSerializer::SerializeContext& ctx) override; RsPgpId pgpKeyId; // duplicate information for practical reasons - RsTlvBinaryData pgpKeyData; + unsigned char *bin_data; // binry key data allocated with new unsigned char[] + uint32_t bin_len; }; class RsDiscContactItem: public RsDiscItem diff --git a/libretroshare/src/gossipdiscovery/p3gossipdiscovery.cc b/libretroshare/src/gossipdiscovery/p3gossipdiscovery.cc index 588d593ad..2001667f1 100644 --- a/libretroshare/src/gossipdiscovery/p3gossipdiscovery.cc +++ b/libretroshare/src/gossipdiscovery/p3gossipdiscovery.cc @@ -374,12 +374,12 @@ void p3discovery2::recvOwnContactInfo(const RsPeerId &fromId, const RsDiscContac RsPeerDetails det ; if(!rsPeers->getPeerDetails(fromId,det)) { - std::cerr << "(EE) Cannot obtain details from " << fromId << " who is supposed to be a friend! Dropping the info." << std::endl; + RsErr() << "(EE) Cannot obtain details from " << fromId << " who is supposed to be a friend! Dropping the info." << std::endl; return; } if(det.gpg_id != item->pgpId) { - std::cerr << "(EE) peer " << fromId << " sent own details with PGP key ID " << item->pgpId << " which does not match the known key id " << det.gpg_id << ". Dropping the info." << std::endl; + RsErr() << "(EE) peer " << fromId << " sent own details with PGP key ID " << item->pgpId << " which does not match the known key id " << det.gpg_id << ". Dropping the info." << std::endl; return; } @@ -748,7 +748,6 @@ void p3discovery2::updatePeers_locked(const RsPeerId &aboutId) std::set mutualFriends; std::set onlineFriends; - std::set::const_iterator sit; const std::set &friendSet = ait->second.mFriendSet; @@ -803,17 +802,15 @@ void p3discovery2::updatePeers_locked(const RsPeerId &aboutId) std::cerr << std::endl; #endif // update aboutId about Other Peers. - for(fit = mutualFriends.begin(); fit != mutualFriends.end(); ++fit) - { + for(auto fit = mutualFriends.begin(); fit != mutualFriends.end(); ++fit) sendContactInfo_locked(*fit, aboutId); - } #ifdef P3DISC_DEBUG std::cerr << "p3discovery2::updatePeer_locked() Updating Online Peers about " << aboutId; std::cerr << std::endl; #endif // update Other Peers about aboutPgpId. - for(sit = onlineFriends.begin(); sit != onlineFriends.end(); ++sit) + for(auto sit = onlineFriends.begin(); sit != onlineFriends.end(); ++sit) { // This could be more efficient, and only be specific about aboutId. // but we'll leave it like this for the moment. @@ -951,12 +948,12 @@ void p3discovery2::processContactInfo(const RsPeerId &fromId, const RsDiscContac // The peer the discovery info is about is a friend. We gather the nodes for that profile into the local structure and notify p3peerMgr. if(!rsPeers->isGPGAccepted(item->pgpId)) // this is an additional check, because the friendship previously depends on the local cache. We need - return ; // fresh information here. + return ; // fresh information here. bool should_notify_discovery = false; auto sit= it->second.mSslIds.find(item->sslId); - DiscSslInfo sslInfo& (it->second.mSslIds[item->sslId]); // This line inserts the entry while not removing already existing data + DiscSslInfo& sslInfo(it->second.mSslIds[item->sslId]); // This line inserts the entry while not removing already existing data if (!mPeerMgr->isFriend(item->sslId)) { @@ -1064,8 +1061,8 @@ void p3discovery2::sendPGPCertificate(const RsPgpId &aboutId, const RsPeerId &to return ; } - pgp_key_item->pgpKeyData.bin_data = bin_data; - pgp_key_item->pgpKeyData.bin_len = bin_len; + pgp_key_item->bin_data = bin_data; + pgp_key_item->bin_len = bin_len; sendItem(pgp_key_item); } @@ -1078,7 +1075,7 @@ void p3discovery2::recvPGPCertificate(const RsPeerId& fromId, RsDiscPgpKeyItem* std::string cert_name; std::list cert_signers; - if(!AuthGPG::getAuthGPG()->getGPGDetailsFromBinaryBlock( (unsigned char*)item->pgpKeyData.bin_data,item->pgpKeyData.bin_len, cert_pgp_id, cert_name, cert_signers )) + if(!AuthGPG::getAuthGPG()->getGPGDetailsFromBinaryBlock( (unsigned char*)item->bin_data,item->bin_len, cert_pgp_id, cert_name, cert_signers )) { std::cerr << "(EE) cannot parse own PGP key sent by " << fromId << std::endl; return; @@ -1121,7 +1118,7 @@ void p3discovery2::recvPGPCertificate(const RsPeerId& fromId, RsDiscPgpKeyItem* std::cerr << __PRETTY_FUNCTION__ << "Received PGP key " << cert_pgp_id << " from from friend " << fromId << ". Adding to keyring." << std::endl; #endif // now that will add the key *and* set the skip_signature_validation flag at once - rsPeers->loadPgpKeyFromBinaryData((unsigned char*)item->pgpKeyData.bin_data,item->pgpKeyData.bin_len, tmp_pgp_id,error_string); // no error should occur at this point because we called loadDetailsFromStringCert() already + rsPeers->loadPgpKeyFromBinaryData((unsigned char*)item->bin_data,item->bin_len, tmp_pgp_id,error_string); // no error should occur at this point because we called loadDetailsFromStringCert() already delete item; // Make sure we allow connections after the key is added. This is not the case otherwise. We only do that if the peer is non validated peer, since diff --git a/libretroshare/src/pqi/authssl.cc b/libretroshare/src/pqi/authssl.cc index f07bbef47..6700a3fde 100644 --- a/libretroshare/src/pqi/authssl.cc +++ b/libretroshare/src/pqi/authssl.cc @@ -1272,8 +1272,6 @@ int AuthSSLimpl::VerifyX509Callback(int /*preverify_ok*/, X509_STORE_CTX* ctx) return verificationFailed; } - if(isSslOnlyFriend && pgpId != - if ( !isSslOnlyFriend && pgpId != AuthGPG::getAuthGPG()->getGPGOwnId() && !AuthGPG::getAuthGPG()->isGPGAccepted(pgpId) ) { std::string errMsg = "Connection attempt signed by PGP key id: " + diff --git a/libretroshare/src/retroshare/rsgossipdiscovery.h b/libretroshare/src/retroshare/rsgossipdiscovery.h index 89839995a..c9dc9924a 100644 --- a/libretroshare/src/retroshare/rsgossipdiscovery.h +++ b/libretroshare/src/retroshare/rsgossipdiscovery.h @@ -64,7 +64,7 @@ struct RsGossipDiscoveryFriendInviteReceivedEvent : RsEvent class RsGossipDiscovery { public: - virtual ~RsGossipDiscovery() {} + virtual ~RsGossipDiscovery() = default; /** * @brief getDiscFriends get a list of all friends of a given friend diff --git a/libretroshare/src/rsserver/p3peers.cc b/libretroshare/src/rsserver/p3peers.cc index 393ae816e..4197260cd 100644 --- a/libretroshare/src/rsserver/p3peers.cc +++ b/libretroshare/src/rsserver/p3peers.cc @@ -582,7 +582,7 @@ bool p3Peers::isSslOnlyFriend(const RsPeerId& sslId) { 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; + RsErr() << __PRETTY_FUNCTION__ << ": 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;