fixed comments from review of PR

This commit is contained in:
csoler 2019-09-19 20:59:35 +02:00
parent 44d6cb7be2
commit 654d760d84
No known key found for this signature in database
GPG key ID: 7BCA522266C0804C
6 changed files with 23 additions and 23 deletions

View file

@ -40,7 +40,7 @@ RsItem *RsDiscSerialiser::create_item(
case RsGossipDiscoveryItemType::CONTACT: return new RsDiscContactItem(); case RsGossipDiscoveryItemType::CONTACT: return new RsDiscContactItem();
case RsGossipDiscoveryItemType::IDENTITY_LIST: return new RsDiscIdentityListItem(); case RsGossipDiscoveryItemType::IDENTITY_LIST: return new RsDiscIdentityListItem();
default: default:
return NULL; return nullptr;
} }
return nullptr; return nullptr;
@ -65,13 +65,17 @@ void RsDiscPgpListItem::serial_process(
void RsDiscPgpKeyItem::serial_process(RsGenericSerializer::SerializeJob j,RsGenericSerializer::SerializeContext& ctx) void RsDiscPgpKeyItem::serial_process(RsGenericSerializer::SerializeJob j,RsGenericSerializer::SerializeContext& ctx)
{ {
RsTypeSerializer::serial_process(j,ctx,pgpKeyId,"pgpKeyId") ; 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() void RsDiscPgpKeyItem::clear()
{ {
pgpKeyId.clear(); pgpKeyId.clear();
pgpKeyData.TlvClear(); free(bin_data);
bin_data = nullptr;
bin_len=0;
} }
void RsDiscContactItem::clear() void RsDiscContactItem::clear()

View file

@ -38,8 +38,6 @@ enum class RsGossipDiscoveryItemType : uint8_t
PGP_CERT = 0x2, // deprecated PGP_CERT = 0x2, // deprecated
CONTACT = 0x5, CONTACT = 0x5,
IDENTITY_LIST = 0x6, IDENTITY_LIST = 0x6,
INVITE = 0x7, // deprecated
INVITE_REQUEST = 0x8, // deprecated
PGP_CERT_BINARY = 0x9, PGP_CERT_BINARY = 0x9,
}; };
@ -89,11 +87,14 @@ public:
RsDiscPgpKeyItem() : RsDiscItem(RsGossipDiscoveryItemType::PGP_CERT_BINARY) RsDiscPgpKeyItem() : RsDiscItem(RsGossipDiscoveryItemType::PGP_CERT_BINARY)
{ setPriorityLevel(QOS_PRIORITY_RS_DISC_PGP_CERT); } { setPriorityLevel(QOS_PRIORITY_RS_DISC_PGP_CERT); }
virtual ~RsDiscPgpKeyItem() { delete[](bin_data);bin_data=nullptr;bin_len=0;}
void clear() override; void clear() override;
void serial_process( RsGenericSerializer::SerializeJob j, RsGenericSerializer::SerializeContext& ctx) override; void serial_process( RsGenericSerializer::SerializeJob j, RsGenericSerializer::SerializeContext& ctx) override;
RsPgpId pgpKeyId; // duplicate information for practical reasons 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 class RsDiscContactItem: public RsDiscItem

View file

@ -374,12 +374,12 @@ void p3discovery2::recvOwnContactInfo(const RsPeerId &fromId, const RsDiscContac
RsPeerDetails det ; RsPeerDetails det ;
if(!rsPeers->getPeerDetails(fromId,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; return;
} }
if(det.gpg_id != item->pgpId) 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; return;
} }
@ -748,7 +748,6 @@ void p3discovery2::updatePeers_locked(const RsPeerId &aboutId)
std::set<RsPgpId> mutualFriends; std::set<RsPgpId> mutualFriends;
std::set<RsPeerId> onlineFriends; std::set<RsPeerId> onlineFriends;
std::set<RsPeerId>::const_iterator sit;
const std::set<RsPgpId> &friendSet = ait->second.mFriendSet; const std::set<RsPgpId> &friendSet = ait->second.mFriendSet;
@ -803,17 +802,15 @@ void p3discovery2::updatePeers_locked(const RsPeerId &aboutId)
std::cerr << std::endl; std::cerr << std::endl;
#endif #endif
// update aboutId about Other Peers. // 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); sendContactInfo_locked(*fit, aboutId);
}
#ifdef P3DISC_DEBUG #ifdef P3DISC_DEBUG
std::cerr << "p3discovery2::updatePeer_locked() Updating Online Peers about " << aboutId; std::cerr << "p3discovery2::updatePeer_locked() Updating Online Peers about " << aboutId;
std::cerr << std::endl; std::cerr << std::endl;
#endif #endif
// update Other Peers about aboutPgpId. // 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. // This could be more efficient, and only be specific about aboutId.
// but we'll leave it like this for the moment. // but we'll leave it like this for the moment.
@ -956,7 +953,7 @@ void p3discovery2::processContactInfo(const RsPeerId &fromId, const RsDiscContac
bool should_notify_discovery = false; bool should_notify_discovery = false;
auto sit= it->second.mSslIds.find(item->sslId); 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)) if (!mPeerMgr->isFriend(item->sslId))
{ {
@ -1064,8 +1061,8 @@ void p3discovery2::sendPGPCertificate(const RsPgpId &aboutId, const RsPeerId &to
return ; return ;
} }
pgp_key_item->pgpKeyData.bin_data = bin_data; pgp_key_item->bin_data = bin_data;
pgp_key_item->pgpKeyData.bin_len = bin_len; pgp_key_item->bin_len = bin_len;
sendItem(pgp_key_item); sendItem(pgp_key_item);
} }
@ -1078,7 +1075,7 @@ void p3discovery2::recvPGPCertificate(const RsPeerId& fromId, RsDiscPgpKeyItem*
std::string cert_name; std::string cert_name;
std::list<RsPgpId> cert_signers; std::list<RsPgpId> 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; std::cerr << "(EE) cannot parse own PGP key sent by " << fromId << std::endl;
return; 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; std::cerr << __PRETTY_FUNCTION__ << "Received PGP key " << cert_pgp_id << " from from friend " << fromId << ". Adding to keyring." << std::endl;
#endif #endif
// now that will add the key *and* set the skip_signature_validation flag at once // 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; 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 // 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

View file

@ -1272,8 +1272,6 @@ int AuthSSLimpl::VerifyX509Callback(int /*preverify_ok*/, X509_STORE_CTX* ctx)
return verificationFailed; return verificationFailed;
} }
if(isSslOnlyFriend && pgpId !=
if ( !isSslOnlyFriend && pgpId != AuthGPG::getAuthGPG()->getGPGOwnId() && !AuthGPG::getAuthGPG()->isGPGAccepted(pgpId) ) if ( !isSslOnlyFriend && pgpId != AuthGPG::getAuthGPG()->getGPGOwnId() && !AuthGPG::getAuthGPG()->isGPGAccepted(pgpId) )
{ {
std::string errMsg = "Connection attempt signed by PGP key id: " + std::string errMsg = "Connection attempt signed by PGP key id: " +

View file

@ -64,7 +64,7 @@ struct RsGossipDiscoveryFriendInviteReceivedEvent : RsEvent
class RsGossipDiscovery class RsGossipDiscovery
{ {
public: public:
virtual ~RsGossipDiscovery() {} virtual ~RsGossipDiscovery() = default;
/** /**
* @brief getDiscFriends get a list of all friends of a given friend * @brief getDiscFriends get a list of all friends of a given friend

View file

@ -582,7 +582,7 @@ bool p3Peers::isSslOnlyFriend(const RsPeerId& sslId)
{ {
if(isPgpFriend(getGPGId(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 false;
} }
return true; return true;