From f9f7e0df18aeb735263bcdd30a9bd019c342bfcd Mon Sep 17 00:00:00 2001 From: Gioacchino Mazzurco Date: Tue, 7 May 2019 12:11:38 +0200 Subject: [PATCH] pqi free x509 cert even before exit Avoid risk of introducing memory leak if we change the behaviour code from exit to return in the future --- libretroshare/src/pqi/pqissl.cc | 21 +++++++++++++-------- libretroshare/src/pqi/pqissllistener.cc | 18 +++++++++++------- 2 files changed, 24 insertions(+), 15 deletions(-) diff --git a/libretroshare/src/pqi/pqissl.cc b/libretroshare/src/pqi/pqissl.cc index 85c2548b6..ca21e70ed 100644 --- a/libretroshare/src/pqi/pqissl.cc +++ b/libretroshare/src/pqi/pqissl.cc @@ -1151,6 +1151,7 @@ int pqissl::Authorise_SSL_Connection() // reset switch. waiting = WAITING_NOT; +#ifdef RS_PQISSL_AUTH_REDUNDANT_CHECK X509* peercert = SSL_get_peer_certificate(ssl_connection); if (!peercert) { @@ -1172,11 +1173,11 @@ int pqissl::Authorise_SSL_Connection() * If the cert is from a friend anyway we should find a way to make good * use of this connection instead of throwing it away... */ + X509_free(peercert); reset_locked(); return failure; } -#ifdef RS_PQISSL_AUTH_REDUNDANT_CHECK /* At this point the actual connection authentication has already been * performed in AuthSSL::VerifyX509Callback, any furter authentication check * like the following two are redundant. */ @@ -1188,20 +1189,24 @@ int pqissl::Authorise_SSL_Connection() << "certificate signature. This should never happen at this " << "point!" << std::endl; print_stacktrace(); + + X509_free(peercert); // not needed but just in case we change to return exit(failure); } RsPgpId pgpId = RsX509Cert::getCertIssuer(*peercert); if( !AuthGPG::getAuthGPG()->isGPGAccepted(pgpId) ) { - RsErr() << __PRETTY_FUNCTION__ << " pgpId: " << pgpId - << " is not friend. It is very unlikely to happen at this " - << "point! Either the user must have been so fast to deny " - << "friendship just after VerifyX509Callback have returned " - << "success and just before this code being executed, or " - << "something really fishy is happening! Share the full log " - << "with developers." << std::endl; + RsFatal() << __PRETTY_FUNCTION__ << " pgpId: " << pgpId + << " is not friend. It is very unlikely to happen at this " + << "point! Either the user must have been so fast to deny " + << "friendship just after VerifyX509Callback have returned " + << "success and just before this code being executed, or " + << "something really fishy is happening! Share the full log " + << "with developers." << std::endl; print_stacktrace(); + + X509_free(peercert); // not needed but just in case we change to return exit(failure); } #endif // def RS_PQISSL_AUTH_REDUNDANT_CHECK diff --git a/libretroshare/src/pqi/pqissllistener.cc b/libretroshare/src/pqi/pqissllistener.cc index c54837f7d..ed36417a2 100644 --- a/libretroshare/src/pqi/pqissllistener.cc +++ b/libretroshare/src/pqi/pqissllistener.cc @@ -793,19 +793,23 @@ int pqissllistener::completeConnection(int fd, IncomingSSLInfo& info) << "certificate signature. This should never happen at this " << "point!" << std::endl; print_stacktrace(); + + X509_free(peercert); // not needed but just in case we change to return exit(failure); } if( !AuthGPG::getAuthGPG()->isGPGAccepted(pgpId) ) { - RsErr() << __PRETTY_FUNCTION__ << " pgpId: " << pgpId - << " is not friend. It is very unlikely to happen at this " - << "point! Either the user must have been so fast to deny " - << "friendship just after VerifyX509Callback have returned " - << "success and just before this code being executed, or " - << "something really fishy is happening! Share the full log " - << "with developers." << std::endl; + RsFatal() << __PRETTY_FUNCTION__ << " pgpId: " << pgpId + << " is not friend. It is very unlikely to happen at this " + << "point! Either the user must have been so fast to deny " + << "friendship just after VerifyX509Callback have returned " + << "success and just before this code being executed, or " + << "something really fishy is happening! Share the full log " + << "with developers." << std::endl; print_stacktrace(); + + X509_free(peercert); // not needed but just in case we change to return exit(failure); } #endif //def RS_PQISSL_AUTH_REDUNDANT_CHECK