From 41d4599fe39fb0fd0f4b0af986326fe8ecf4ead5 Mon Sep 17 00:00:00 2001 From: Gioacchino Mazzurco Date: Fri, 3 May 2019 01:27:32 +0200 Subject: [PATCH 1/8] Cleanup huge amount of AuthSSL cruft Make reduntant auth check in pqi effective (even if redundant only siganture was checked but friendess wasn't) Evidence redundant auth check in pqi by putting it inside #ifdef this way the beaviior being the same with and without redundat check can be verified easier Solve lot of compiler warnings and made code more readable Remove dangerous sslcert wrapper Remove misleading messeges and notification about peer not giving cert, FailedCertificate logic is wrong since many years as authentication is fully handled inside VerifyX509Callback --- libretroshare/src/pqi/authssl.cc | 813 +++++++++--------------- libretroshare/src/pqi/authssl.h | 317 +++++---- libretroshare/src/pqi/p3linkmgr.cc | 41 -- libretroshare/src/pqi/p3linkmgr.h | 4 - libretroshare/src/pqi/pqissl.cc | 217 +++---- libretroshare/src/pqi/pqissl.h | 19 +- libretroshare/src/pqi/pqissllistener.cc | 213 ++----- libretroshare/src/pqi/pqissllistener.h | 18 +- libretroshare/src/pqi/sslfns.cc | 90 +-- libretroshare/src/pqi/sslfns.h | 28 +- libretroshare/src/rsserver/rsinit.cc | 5 +- 11 files changed, 682 insertions(+), 1083 deletions(-) diff --git a/libretroshare/src/pqi/authssl.cc b/libretroshare/src/pqi/authssl.cc index cec288c58..6581c2e97 100644 --- a/libretroshare/src/pqi/authssl.cc +++ b/libretroshare/src/pqi/authssl.cc @@ -3,7 +3,8 @@ * * * libretroshare: retroshare core library * * * - * Copyright 2004-2008 by Robert Fernie, Retroshare Team. * + * Copyright (C) 2004-2008 Robert Fernie * + * Copyright (C) 2019 Gioacchino Mazzurco * * * * This program is free software: you can redistribute it and/or modify * * it under the terms of the GNU Lesser General Public License as * @@ -62,9 +63,7 @@ const uint32_t RS_SSL_HANDSHAKE_DIAGNOSTIC_WRONG_SIGNATURE_VERSION = 0x0a ; * #define AUTHSSL_DEBUG 1 ***/ -// initialisation du pointeur de singleton -AuthSSL *AuthSSL::instance_ssl = NULL; -static pthread_mutex_t *mutex_buf = NULL; +static pthread_mutex_t* mutex_buf = nullptr; struct CRYPTO_dynlock_value { @@ -214,31 +213,17 @@ void tls_cleanup() } } -/* hidden function - for testing purposes() */ -void AuthSSL::setAuthSSL_debug(AuthSSL *newssl) +/*static*/ AuthSSL& AuthSSL::instance() { - instance_ssl = newssl; + static AuthSSLimpl mInstance; + return mInstance; } -void AuthSSL::AuthSSLInit() -{ - if (instance_ssl == NULL) - { - instance_ssl = new AuthSSLimpl(); - } -} - -AuthSSL *AuthSSL::getAuthSSL() -{ - return instance_ssl; -} +AuthSSL* AuthSSL::getAuthSSL() { return &instance(); } + +AuthSSL::~AuthSSL() = default; -AuthSSL::AuthSSL() -{ - return; -} - /********************************************************************************/ /********************************************************************************/ /********************* Cert Search / Add / Remove **************************/ @@ -247,26 +232,55 @@ AuthSSL::AuthSSL() static int verify_x509_callback(int preverify_ok, X509_STORE_CTX *ctx); - -sslcert::sslcert(X509 *x509, const RsPeerId& pid) +std::string RsX509Cert::getCertName(const X509& x509) { - certificate = x509; - id = pid; #if OPENSSL_VERSION_NUMBER < 0x10100000L || defined(LIBRESSL_VERSION_NUMBER) - // cppcheck-suppress useInitializationList - name = getX509CNString(x509->cert_info->subject); - org = getX509OrgString(x509->cert_info->subject); - location = getX509LocString(x509->cert_info->subject); - issuer = RsPgpId(std::string(getX509CNString(x509->cert_info->issuer))); + return getX509CNString(x509.cert_info->subject); #else - name = getX509CNString(X509_get_subject_name(x509)); - org = getX509OrgString(X509_get_subject_name(x509)); - location = getX509LocString(X509_get_subject_name(x509)); - issuer = RsPgpId(std::string(getX509CNString(X509_get_issuer_name(x509)))); + return getX509CNString(X509_get_subject_name(&x509)); #endif - email = ""; +} - authed = false; +std::string RsX509Cert::getCertLocation(const X509& x509) +{ +#if OPENSSL_VERSION_NUMBER < 0x10100000L || defined(LIBRESSL_VERSION_NUMBER) + return getX509LocString(x509.cert_info->subject); +#else + return getX509LocString(X509_get_subject_name(&x509)); +#endif +} + +std::string RsX509Cert::getCertOrg(const X509& x509) +{ +#if OPENSSL_VERSION_NUMBER < 0x10100000L || defined(LIBRESSL_VERSION_NUMBER) + return getX509OrgString(x509.cert_info->subject); +#else + return getX509OrgString(X509_get_subject_name(&x509)); +#endif +} + +/*static*/ RsPgpId RsX509Cert::getCertIssuer(const X509& x509) +{ +#if OPENSSL_VERSION_NUMBER < 0x10100000L || defined(LIBRESSL_VERSION_NUMBER) + return RsPgpId(getX509CNString(x509.cert_info->issuer)); +#else + return RsPgpId(getX509CNString(X509_get_issuer_name(&x509))); +#endif +} + +/*static*/ std::string RsX509Cert::getCertIssuerString(const X509& x509) +{ +#if OPENSSL_VERSION_NUMBER < 0x10100000L || defined(LIBRESSL_VERSION_NUMBER) + return getX509CNString(x509.cert_info->issuer); +#else + return getX509CNString(X509_get_issuer_name(&x509)); +#endif +} + +/*static*/ RsPeerId RsX509Cert::getCertSslId(const X509& x509) +{ + RsPeerId sslid; + return getX509id(const_cast(&x509), sslid) ? sslid : RsPeerId(); } /************************************************************************ @@ -289,28 +303,18 @@ sslcert::sslcert(X509 *x509, const RsPeerId& pid) /********************************************************************************/ -AuthSSLimpl::AuthSSLimpl() - : p3Config(), sslctx(NULL), - mOwnCert(NULL), sslMtx("AuthSSL"), mOwnPrivateKey(NULL), mOwnPublicKey(NULL), init(0) +AuthSSLimpl::AuthSSLimpl() : + p3Config(), sslctx(nullptr), mOwnCert(nullptr), sslMtx("AuthSSL"), + mOwnPrivateKey(nullptr), mOwnPublicKey(nullptr), init(0) {} + +bool AuthSSLimpl::active() { return init; } + +int AuthSSLimpl::InitAuth( + const char* cert_file, const char* priv_key_file, const char* passwd, + std::string /*alternative_location_name*/ ) { -} - -bool AuthSSLimpl::active() -{ - return init; -} - - -int AuthSSLimpl::InitAuth(const char *cert_file, const char *priv_key_file, - const char *passwd, std::string alternative_location_name) -{ -#ifdef AUTHSSL_DEBUG - std::cerr << "AuthSSLimpl::InitAuth()"; - std::cerr << std::endl; -#endif - /* single call here si don't need to invoke mutex yet */ -static int initLib = 0; + static int initLib = 0; if (!initLib) { initLib = 1; @@ -498,12 +502,7 @@ static int initLib = 0; std::cerr << "SSL Verification Set" << std::endl; - mOwnCert = new sslcert(x509, mOwnId); - - // New locations don't store the name in the cert. - // If empty, use the external supplied value. - if (mOwnCert->location == "") - mOwnCert->location = alternative_location_name ; + mOwnCert = x509; std::cerr << "Inited SSL context: " << std::endl; std::cerr << " Certificate: " << mOwnId << std::endl; @@ -576,20 +575,10 @@ const RsPeerId& AuthSSLimpl::OwnId() } std::string AuthSSLimpl::getOwnLocation() -{ -#ifdef AUTHSSL_DEBUG - std::cerr << "AuthSSLimpl::OwnId()" << std::endl; -#endif - return mOwnCert->location; -} +{ return RsX509Cert::getCertLocation(*mOwnCert); } std::string AuthSSLimpl::SaveOwnCertificateToString() -{ -#ifdef AUTHSSL_DEBUG - std::cerr << "AuthSSLimpl::SaveOwnCertificateToString() " << std::endl; -#endif - return saveX509ToPEM(mOwnCert->certificate); -} +{ return saveX509ToPEM(mOwnCert); } /********************************************************************************/ /********************************************************************************/ @@ -670,25 +659,20 @@ bool AuthSSLimpl::VerifySignBin(const void *data, const uint32_t len, RsStackMutex stack(sslMtx); /***** STACK LOCK MUTEX *****/ /* find the peer */ - sslcert *peer; - if (sslId == mOwnId) - { - peer = mOwnCert; - } - else if (!locked_FindCert(sslId, &peer)) + X509* peercert; + if (sslId == mOwnId) peercert = mOwnCert; + else if (!locked_FindCert(sslId, &peercert)) { std::cerr << "VerifySignBin() no peer" << std::endl; return false; } - return SSL_VerifySignBin(data, len, sign, signlen, peer->certificate); + return SSL_VerifySignBin(data, len, sign, signlen, peercert); } bool AuthSSLimpl::VerifyOwnSignBin(const void *data, const uint32_t len, unsigned char *sign, unsigned int signlen) -{ - return SSL_VerifySignBin(data, len, sign, signlen, mOwnCert->certificate); -} +{ return SSL_VerifySignBin(data, len, sign, signlen, mOwnCert); } /********************************************************************************/ @@ -984,152 +968,129 @@ X509 *AuthSSLimpl::SignX509ReqWithGPG(X509_REQ *req, long /*days*/) } -/* This function, checks that the X509 is signed by a known GPG key, - * NB: we do not have to have approved this person as a friend. - * this is important - as it allows non-friends messages to be validated. - */ - -bool AuthSSLimpl::AuthX509WithGPG(X509 *x509,uint32_t& diagnostic) +bool AuthSSLimpl::AuthX509WithGPG(X509 *x509, uint32_t& diagnostic) { -#ifdef AUTHSSL_DEBUG - fprintf(stderr, "AuthSSLimpl::AuthX509WithGPG() called\n"); -#endif - - if (!CheckX509Certificate(x509)) - { - std::cerr << "AuthSSLimpl::AuthX509() X509 NOT authenticated : Certificate failed basic checks" << std::endl; - diagnostic = RS_SSL_HANDSHAKE_DIAGNOSTIC_CERTIFICATE_NOT_VALID ; - return false; - } - - /* extract CN for peer Id */ -#if OPENSSL_VERSION_NUMBER < 0x10100000L || defined(LIBRESSL_VERSION_NUMBER) - RsPgpId issuer(std::string(getX509CNString(x509->cert_info->issuer))); -#else - RsPgpId issuer(std::string(getX509CNString(X509_get_issuer_name(x509)))); -#endif + RsPgpId issuer = RsX509Cert::getCertIssuer(*x509); RsPeerDetails pd; -#ifdef AUTHSSL_DEBUG - std::cerr << "Checking GPG issuer : " << issuer.toStdString() << std::endl ; -#endif - if (!AuthGPG::getAuthGPG()->getGPGDetails(issuer, pd)) { - std::cerr << "AuthSSLimpl::AuthX509() X509 NOT authenticated : AuthGPG::getAuthGPG()->getGPGDetails() returned false." << std::endl; - diagnostic = RS_SSL_HANDSHAKE_DIAGNOSTIC_ISSUER_UNKNOWN ; + if (!AuthGPG::getAuthGPG()->getGPGDetails(issuer, pd)) + { + RsErr() << __PRETTY_FUNCTION__ << " X509 NOT authenticated : " + << "AuthGPG::getAuthGPG()->getGPGDetails(" << issuer + << ",...) returned false." << std::endl; + diagnostic = RS_SSL_HANDSHAKE_DIAGNOSTIC_ISSUER_UNKNOWN; return false; } + else + Dbg3() << __PRETTY_FUNCTION__ << " issuer: " << issuer << " found" + << std::endl; /* verify GPG signature */ - /*** NOW The Manual signing bit (HACKED FROM asn1/a_sign.c) ***/ #if OPENSSL_VERSION_NUMBER < 0x10100000L || defined(LIBRESSL_VERSION_NUMBER) int (*i2d)(X509_CINF*, unsigned char**) = i2d_X509_CINF; - ASN1_BIT_STRING *signature = x509->signature; - X509_CINF *data = x509->cert_info; + ASN1_BIT_STRING* signature = x509->signature; + X509_CINF* data = x509->cert_info; #else - const ASN1_BIT_STRING *signature = NULL ; - const X509_ALGOR *algor2=NULL; - + const ASN1_BIT_STRING* signature = nullptr; + const X509_ALGOR* algor2 = nullptr; X509_get0_signature(&signature,&algor2,x509); #endif - uint32_t certificate_version = getX509RetroshareCertificateVersion(x509) ; + uint32_t certificate_version = getX509RetroshareCertificateVersion(x509); - EVP_MD_CTX *ctx = EVP_MD_CTX_create(); - int inl=0; + EVP_MD_CTX* ctx = EVP_MD_CTX_create(); + int inl = 0; - const unsigned char *signed_data = NULL ; - uint32_t signed_data_length =0; + const unsigned char* signed_data = nullptr; + uint32_t signed_data_length = 0; /* input buffer */ #if OPENSSL_VERSION_NUMBER < 0x10100000L || defined(LIBRESSL_VERSION_NUMBER) - inl=i2d(data,NULL); - unsigned char *buf_in=(unsigned char *)OPENSSL_malloc((unsigned int)inl); - unsigned char *p=NULL; + inl = i2d(data, nullptr); + unsigned char* buf_in = static_cast( + OPENSSL_malloc(static_cast(inl)) ); + unsigned char* p = nullptr; #else - unsigned char *buf_in=NULL; + unsigned char* buf_in = nullptr; inl=i2d_re_X509_tbs(x509,&buf_in) ; // this does the i2d over x509->cert_info #endif - if(buf_in == NULL) + if(buf_in == nullptr) { - fprintf(stderr, "AuthSSLimpl::AuthX509: ASN1err(ASN1_F_ASN1_SIGN,ERR_R_MALLOC_FAILURE)\n"); + RsErr() << __PRETTY_FUNCTION__ + << " ASN1err(ASN1_F_ASN1_SIGN,ERR_R_MALLOC_FAILURE)" << std::endl; diagnostic = RS_SSL_HANDSHAKE_DIAGNOSTIC_MALLOC_ERROR ; - - return false ; + return false; } -#ifdef AUTHSSL_DEBUG - std::cerr << "Buffers Allocated" << std::endl; -#endif - #if OPENSSL_VERSION_NUMBER < 0x10100000L || defined(LIBRESSL_VERSION_NUMBER) - p=buf_in; + p = buf_in; i2d(data,&p); #endif { // this scope is to avoid cross-initialization jumps to err. - const Sha1CheckSum sha1 = RsDirUtil::sha1sum(buf_in,inl) ; // olds the memory until destruction + const Sha1CheckSum sha1 = RsDirUtil::sha1sum( + buf_in, static_cast(inl) ); if(certificate_version < RS_CERTIFICATE_VERSION_NUMBER_07_0001) { - // If the certificate belongs to 0.6 version, we hash it here, and then re-hash the hash it in the PGP signature. - - signed_data = sha1.toByteArray() ; + /* If the certificate belongs to 0.6 version, we hash it here, and + * then re-hash the hash it in the PGP signature */ + signed_data = sha1.toByteArray(); signed_data_length = sha1.SIZE_IN_BYTES; } else { signed_data = buf_in ; - signed_data_length = inl ; + signed_data_length = static_cast(inl); } /* NOW check sign via GPG Functions */ - //get the fingerprint of the key that is supposed to sign -#ifdef AUTHSSL_DEBUG - std::cerr << "AuthSSLimpl::AuthX509() verifying the gpg sig with keyprint : " << pd.fpr << std::endl; - std::cerr << "Sigoutl = " << sigoutl << std::endl ; - std::cerr << "pd.fpr = " << pd.fpr << std::endl ; -#endif - // Take a early look at signature parameters. In particular we dont accept signatures with unsecure hash algorithms. + Dbg2() << __PRETTY_FUNCTION__ + << " verifying the PGP Key signature with finger print: " + << pd.fpr << std::endl; + + /* Take a early look at signature parameters. In particular we dont + * accept signatures with unsecure hash algorithms */ PGPSignatureInfo signature_info ; - PGPKeyManagement::parseSignature(signature->data,signature->length,signature_info) ; + PGPKeyManagement::parseSignature( + signature->data, static_cast(signature->length), + signature_info ); if(signature_info.signature_version != PGP_PACKET_TAG_SIGNATURE_VERSION_V4) { - diagnostic = RS_SSL_HANDSHAKE_DIAGNOSTIC_WRONG_SIGNATURE_VERSION ; - goto err ; + diagnostic = RS_SSL_HANDSHAKE_DIAGNOSTIC_WRONG_SIGNATURE_VERSION; + goto err; } - std::string sigtypestring ; + std::string sigtypestring; switch(signature_info.signature_type) { - case PGP_PACKET_TAG_SIGNATURE_TYPE_BINARY_DOCUMENT : - break ; - - case PGP_PACKET_TAG_SIGNATURE_TYPE_STANDALONE_SIG : - case PGP_PACKET_TAG_SIGNATURE_TYPE_CANONICAL_TEXT : - case PGP_PACKET_TAG_SIGNATURE_TYPE_UNKNOWN : + case PGP_PACKET_TAG_SIGNATURE_TYPE_BINARY_DOCUMENT: break; + case PGP_PACKET_TAG_SIGNATURE_TYPE_STANDALONE_SIG: /*fallthrough*/ + case PGP_PACKET_TAG_SIGNATURE_TYPE_CANONICAL_TEXT: /*fallthrough*/ + case PGP_PACKET_TAG_SIGNATURE_TYPE_UNKNOWN: /*fallthrough*/ default: - diagnostic = RS_SSL_HANDSHAKE_DIAGNOSTIC_WRONG_SIGNATURE_TYPE ; - goto err ; + diagnostic = RS_SSL_HANDSHAKE_DIAGNOSTIC_WRONG_SIGNATURE_TYPE; + goto err; } switch(signature_info.public_key_algorithm) { - case PGP_PACKET_TAG_PUBLIC_KEY_ALGORITHM_RSA_ES : - case PGP_PACKET_TAG_PUBLIC_KEY_ALGORITHM_RSA_S : sigtypestring = "RSA" ; + case PGP_PACKET_TAG_PUBLIC_KEY_ALGORITHM_RSA_ES: /*fallthrough*/ + case PGP_PACKET_TAG_PUBLIC_KEY_ALGORITHM_RSA_S: + sigtypestring = "RSA"; break ; - - case PGP_PACKET_TAG_PUBLIC_KEY_ALGORITHM_DSA : sigtypestring = "DSA" ; + case PGP_PACKET_TAG_PUBLIC_KEY_ALGORITHM_DSA: + sigtypestring = "DSA"; break ; - - case PGP_PACKET_TAG_PUBLIC_KEY_ALGORITHM_RSA_E : - case PGP_PACKET_TAG_PUBLIC_KEY_ALGORITHM_UNKNOWN: + case PGP_PACKET_TAG_PUBLIC_KEY_ALGORITHM_RSA_E: /*fallthrough*/ + case PGP_PACKET_TAG_PUBLIC_KEY_ALGORITHM_UNKNOWN: /*fallthrough*/ default: diagnostic = RS_SSL_HANDSHAKE_DIAGNOSTIC_HASH_ALGORITHM_NOT_ACCEPTED ; goto err ; @@ -1137,95 +1098,57 @@ bool AuthSSLimpl::AuthX509WithGPG(X509 *x509,uint32_t& diagnostic) switch(signature_info.hash_algorithm) { - case PGP_PACKET_TAG_HASH_ALGORITHM_SHA1 : sigtypestring += "+SHA1" ; break; - case PGP_PACKET_TAG_HASH_ALGORITHM_SHA256: sigtypestring += "+SHA256" ; break; - case PGP_PACKET_TAG_HASH_ALGORITHM_SHA512: sigtypestring += "+SHA512" ; break; - - // We dont accept signatures with unknown or week hash algorithms. - - case PGP_PACKET_TAG_HASH_ALGORITHM_MD5: - case PGP_PACKET_TAG_HASH_ALGORITHM_UNKNOWN: + case PGP_PACKET_TAG_HASH_ALGORITHM_SHA1: + sigtypestring += "+SHA1"; + break; + case PGP_PACKET_TAG_HASH_ALGORITHM_SHA256: + sigtypestring += "+SHA256"; + break; + case PGP_PACKET_TAG_HASH_ALGORITHM_SHA512: + sigtypestring += "+SHA512"; + break; + // We dont accept signatures with unknown or weak hash algorithms. + case PGP_PACKET_TAG_HASH_ALGORITHM_MD5: /*fallthrough*/ + case PGP_PACKET_TAG_HASH_ALGORITHM_UNKNOWN: /*fallthrough*/ default: - diagnostic = RS_SSL_HANDSHAKE_DIAGNOSTIC_HASH_ALGORITHM_NOT_ACCEPTED ; - goto err ; + diagnostic = RS_SSL_HANDSHAKE_DIAGNOSTIC_HASH_ALGORITHM_NOT_ACCEPTED; + goto err; } // passed, verify the signature itself - if (!AuthGPG::getAuthGPG()->VerifySignBin(signed_data, signed_data_length, signature->data, signature->length, pd.fpr)) + if (!AuthGPG::getAuthGPG()->VerifySignBin( + signed_data, signed_data_length, signature->data, + static_cast(signature->length), pd.fpr )) { - diagnostic = RS_SSL_HANDSHAKE_DIAGNOSTIC_WRONG_SIGNATURE ; + diagnostic = RS_SSL_HANDSHAKE_DIAGNOSTIC_WRONG_SIGNATURE; goto err; } - RsPeerId peerIdstr ; - getX509id(x509, peerIdstr) ; - - std::string fpr = pd.fpr.toStdString(); - std::cerr << "Verified " << sigtypestring << " signature of certificate " << peerIdstr << ", Version " << std::hex << certificate_version - << std::dec << " using PGP key " ; - for(uint32_t i=0;iVerifyX509Callback(preverify_ok, ctx); + /* According to OpenSSL documentation must return 0 if verification failed + * and 1 if succeded (aka can continue connection). + * About preverify_ok OpenSSL documentation doesn't tell which value is + * passed to the first callback in the authentication chain, it just says + * that the result of previous step is passed down, so I have tested it + * and we get passed 0 always so in our case as there is no other + * verifications step vefore we ignore it completely */ - X509 *x509 = X509_STORE_CTX_get_current_cert(ctx) ; + constexpr int verificationFailed = 0; + constexpr int verificationSuccess = 1; - if(x509 != NULL) + using Evt_t = RsAuthSslConnectionAutenticationEvent; + std::unique_ptr ev = std::unique_ptr(new Evt_t); + + X509* x509Cert = X509_STORE_CTX_get_current_cert(ctx); + if(!x509Cert) { -#if OPENSSL_VERSION_NUMBER < 0x10100000L || defined(LIBRESSL_VERSION_NUMBER) - RsPgpId gpgid (std::string(getX509CNString(x509->cert_info->issuer))); -#else - RsPgpId gpgid (std::string(getX509CNString(X509_get_issuer_name(x509)))); -#endif + std::string errMsg = "Cannot get certificate! OpenSSL error: " + + std::to_string(X509_STORE_CTX_get_error(ctx)) + " depth: " + + std::to_string(X509_STORE_CTX_get_error_depth(ctx)); - if(gpgid.isNull()) + RsErr() << __PRETTY_FUNCTION__ << " " << errMsg << std::endl; + + if(rsEvents) { -#if OPENSSL_VERSION_NUMBER < 0x10100000L || defined(LIBRESSL_VERSION_NUMBER) - std::cerr << "verify_x509_callback(): wrong PGP id \"" << std::string(getX509CNString(x509->cert_info->issuer)) << "\"" << std::endl; -#else - std::cerr << "verify_x509_callback(): wrong PGP id \"" << std::string(getX509CNString(X509_get_issuer_name(x509))) << "\"" << std::endl; -#endif - return false ; + ev->mErrorMsg = errMsg; + rsEvents->postEvent(std::move(ev)); } -#if OPENSSL_VERSION_NUMBER < 0x10100000L || defined(LIBRESSL_VERSION_NUMBER) - std::string sslcn = getX509CNString(x509->cert_info->subject); -#else - std::string sslcn = getX509CNString(X509_get_subject_name(x509)); -#endif - RsPeerId sslid ; + return verificationFailed; + } - getX509id(x509,sslid); + RsPeerId sslId = RsX509Cert::getCertSslId(*x509Cert); + std::string sslCn = RsX509Cert::getCertIssuerString(*x509Cert); + RsPgpId pgpId(sslCn); - if(sslid.isNull()) + if(sslId.isNull()) + { + std::string errMsg = "x509Cert has invalid sslId!"; + + RsInfo() << __PRETTY_FUNCTION__ << " " << errMsg << std::endl; + + if(rsEvents) { - std::cerr << "verify_x509_callback(): wrong PGP id \"" << sslcn << "\"" << std::endl; - return false ; + ev->mSslCn = sslCn; + ev->mPgpId = pgpId; + ev->mErrorMsg = errMsg; + rsEvents->postEvent(std::move(ev)); } - AuthSSL::getAuthSSL()->setCurrentConnectionAttemptInfo(gpgid,sslid,sslcn) ; - } + return verificationFailed; + } - return verify; -} + if(pgpId.isNull()) + { + std::string errMsg = "x509Cert has invalid pgpId! sslCn >>>" + sslCn + + "<<<"; -int AuthSSLimpl::VerifyX509Callback(int preverify_ok, X509_STORE_CTX *ctx) -{ - char buf[256]; - X509 *err_cert; + RsInfo() << __PRETTY_FUNCTION__ << " " << errMsg << std::endl; - err_cert = X509_STORE_CTX_get_current_cert(ctx); -#ifdef AUTHSSL_DEBUG - int err, depth; - err = X509_STORE_CTX_get_error(ctx); - depth = X509_STORE_CTX_get_error_depth(ctx); -#endif + if(rsEvents) + { + ev->mSslId = sslId; + ev->mSslCn = sslCn; + ev->mErrorMsg = errMsg; + rsEvents->postEvent(std::move(ev)); + } - if(err_cert == NULL) - { - std::cerr << "AuthSSLimpl::VerifyX509Callback(): Cannot get certificate. Error!" << std::endl; - return false ; - } -#ifdef AUTHSSL_DEBUG - std::cerr << "AuthSSLimpl::VerifyX509Callback(preverify_ok: " << preverify_ok - << " Err: " << err << " Depth: " << depth << std::endl; -#endif + return verificationFailed; + } - /* - * Retrieve the pointer to the SSL of the connection currently treated - * and the application specific data stored into the SSL object. - */ + uint32_t auth_diagnostic; + if (!AuthX509WithGPG(x509Cert, auth_diagnostic)) + { + std::string errMsg = "Certificate was rejected because PGP " + "signature verification failed with diagnostic: " + + std::to_string(auth_diagnostic) + " certName: " + + RsX509Cert::getCertName(*x509Cert) + " sslId: " + + RsX509Cert::getCertSslId(*x509Cert).toStdString() + + " issuerString: " + RsX509Cert::getCertIssuerString(*x509Cert); - X509_NAME_oneline(X509_get_subject_name(err_cert), buf, 256); + RsInfo() << __PRETTY_FUNCTION__ << " " << errMsg << std::endl; -#ifdef AUTHSSL_DEBUG - std::cerr << "AuthSSLimpl::VerifyX509Callback: depth: " << depth << ":" << buf << std::endl; -#endif + if(rsEvents) + { + ev->mSslId = sslId; + ev->mSslCn = sslCn; + ev->mPgpId = pgpId; + ev->mErrorMsg = errMsg; + rsEvents->postEvent(std::move(ev)); + } + return verificationFailed; + } - if (!preverify_ok) { -#ifdef AUTHSSL_DEBUG - fprintf(stderr, "Verify error:num=%d:%s:depth=%d:%s\n", err, - X509_verify_cert_error_string(err), depth, buf); -#endif - } + if ( pgpId != AuthGPG::getAuthGPG()->getGPGOwnId() && + !AuthGPG::getAuthGPG()->isGPGAccepted(pgpId) ) + { + std::string errMsg = "Connection attempt signed by PGP key id: " + + pgpId.toStdString() + " not accepted because it is not" + " a friend."; - /* - * At this point, err contains the last verification error. We can use - * it for something special - */ + Dbg1() << __PRETTY_FUNCTION__ << " " << errMsg << std::endl; - if (!preverify_ok) - { + if(rsEvents) + { + ev->mSslId = sslId; + ev->mSslCn = sslCn; + ev->mPgpId = pgpId; + ev->mErrorMsg = errMsg; + rsEvents->postEvent(std::move(ev)); + } - X509_NAME_oneline(X509_get_issuer_name(X509_STORE_CTX_get_current_cert(ctx)), buf, 256); -#ifdef AUTHSSL_DEBUG - printf("issuer= %s\n", buf); -#endif + return verificationFailed; + } -#ifdef AUTHSSL_DEBUG - fprintf(stderr, "Doing REAL PGP Certificates\n"); -#endif - uint32_t auth_diagnostic ; + AuthSSL::instance().setCurrentConnectionAttemptInfo(pgpId, sslId, sslCn); - /* do the REAL Authentication */ - if (!AuthX509WithGPG(X509_STORE_CTX_get_current_cert(ctx),auth_diagnostic)) - { -#ifdef AUTHSSL_DEBUG - fprintf(stderr, "AuthSSLimpl::VerifyX509Callback() X509 not authenticated.\n"); -#endif - std::cerr << "(WW) Certificate was rejected because authentication failed. Diagnostic = " << auth_diagnostic << std::endl; - return false; - } -#if OPENSSL_VERSION_NUMBER < 0x10100000L || defined(LIBRESSL_VERSION_NUMBER) - RsPgpId pgpid(std::string(getX509CNString(X509_STORE_CTX_get_current_cert(ctx)->cert_info->issuer))); -#else - RsPgpId pgpid(std::string(getX509CNString(X509_get_issuer_name(X509_STORE_CTX_get_current_cert(ctx))))); -#endif + LocalStoreCert(x509Cert); - if (pgpid != AuthGPG::getAuthGPG()->getGPGOwnId() && !AuthGPG::getAuthGPG()->isGPGAccepted(pgpid)) - { -#ifdef AUTHSSL_DEBUG - fprintf(stderr, "AuthSSLimpl::VerifyX509Callback() pgp key not accepted : \n"); - fprintf(stderr, "issuer pgpid : "); - fprintf(stderr, "%s\n",pgpid.c_str()); - fprintf(stderr, "\n AuthGPG::getAuthGPG()->getGPGOwnId() : "); - fprintf(stderr, "%s\n",AuthGPG::getAuthGPG()->getGPGOwnId().c_str()); - fprintf(stderr, "\n"); -#endif - return false; - } + Dbg1() << __PRETTY_FUNCTION__ << " authentication successfull!" << std::endl; - preverify_ok = true; + if(rsEvents) + { + ev->mSuccess = true; + ev->mSslId = sslId; + ev->mSslCn = sslCn; + ev->mPgpId = pgpId; + rsEvents->postEvent(std::move(ev)); + } - } else { -#ifdef AUTHSSL_DEBUG - fprintf(stderr, "A normal certificate is probably a security breach attempt. We sould fail it !!!\n"); -#endif - preverify_ok = false; - } - - if (preverify_ok) { - - //sslcert *cert = NULL; - RsPeerId certId; - getX509id(X509_STORE_CTX_get_current_cert(ctx), certId); - - } - -#ifdef AUTHSSL_DEBUG - if (preverify_ok) { - fprintf(stderr, "AuthSSLimpl::VerifyX509Callback returned true.\n"); - } else { - fprintf(stderr, "AuthSSLimpl::VerifyX509Callback returned false.\n"); - } -#endif - - return preverify_ok; + return verificationSuccess; } @@ -1406,33 +1304,21 @@ int AuthSSLimpl::VerifyX509Callback(int preverify_ok, X509_STORE_CTX *ctx) bool AuthSSLimpl::encrypt(void *&out, int &outlen, const void *in, int inlen, const RsPeerId& peerId) { - RsStackMutex stack(sslMtx); /******* LOCKED ******/ + RS_STACK_MUTEX(sslMtx); -#ifdef AUTHSSL_DEBUG - std::cerr << "AuthSSLimpl::encrypt() called for peerId : " << peerId << " with inlen : " << inlen << std::endl; -#endif - //TODO : use ssl to crypt the binary input buffer -// out = malloc(inlen); -// memcpy(out, in, inlen); -// outlen = inlen; - - EVP_PKEY *public_key; - if (peerId == mOwnId) { - public_key = mOwnPublicKey; - } else { - if (!mCerts[peerId]) { - #ifdef AUTHSSL_DEBUG - std::cerr << "AuthSSLimpl::encrypt() public key not found." << std::endl; - #endif - return false; - } else { -#if OPENSSL_VERSION_NUMBER < 0x10100000L || defined(LIBRESSL_VERSION_NUMBER) - public_key = mCerts[peerId]->certificate->cert_info->key->pkey; -#else - public_key = X509_get0_pubkey(mCerts[peerId]->certificate) ; -#endif - } - } + /*const*/ EVP_PKEY* public_key = nullptr; + if (peerId == mOwnId) { public_key = mOwnPublicKey; } + else + { + if (!mCerts[peerId]) + { + RsErr() << __PRETTY_FUNCTION__ << " public key not found." + << std::endl; + return false; + } + else public_key = const_cast( + RsX509Cert::getPubKey(*mCerts[peerId]) ); + } EVP_CIPHER_CTX *ctx = EVP_CIPHER_CTX_new(); int eklen, net_ekl; @@ -1640,104 +1526,10 @@ void AuthSSLimpl::getCurrentConnectionAttemptInfo(RsPgpId& gpg_id,RsPeerId& ssl_ ssl_cn = _last_sslcn_to_connect ; } -/* store for discovery */ -bool AuthSSLimpl::FailedCertificate(X509 *x509, const RsPgpId& gpgid, - const RsPeerId& sslid, - const std::string& sslcn, - const struct sockaddr_storage& addr, - bool incoming) -{ - std::string ip_address = sockaddr_storage_tostring(addr); - - uint32_t auth_diagnostic = 0 ; - bool authed ; - - if(x509 == NULL) - { - auth_diagnostic = RS_SSL_HANDSHAKE_DIAGNOSTIC_CERTIFICATE_MISSING ; - authed = false ; - } - else - authed = AuthX509WithGPG(x509,auth_diagnostic) ; - - if(authed) - LocalStoreCert(x509); - -#ifdef AUTHSSL_DEBUG - std::cerr << "AuthSSLimpl::FailedCertificate() "; -#endif - if (incoming) - { - RsServer::notify()->AddPopupMessage(RS_POPUP_CONNECT_ATTEMPT, gpgid.toStdString(), sslcn, sslid.toStdString()); - - switch(auth_diagnostic) - { - case RS_SSL_HANDSHAKE_DIAGNOSTIC_CERTIFICATE_MISSING: - RsServer::notify()->notifyConnectionWithoutCert(); - RsServer::notify()->AddFeedItem(RS_FEED_ITEM_SEC_MISSING_CERTIFICATE, gpgid.toStdString(), sslid.toStdString(), sslcn, ip_address); - break ; - case RS_SSL_HANDSHAKE_DIAGNOSTIC_CERTIFICATE_NOT_VALID: - RsServer::notify()->AddFeedItem(RS_FEED_ITEM_SEC_BAD_CERTIFICATE, gpgid.toStdString(), sslid.toStdString(), sslcn, ip_address); - break ; - case RS_SSL_HANDSHAKE_DIAGNOSTIC_ISSUER_UNKNOWN: - RsServer::notify()->AddFeedItem(RS_FEED_ITEM_SEC_UNKNOWN_IN , gpgid.toStdString(), sslid.toStdString(), sslcn, ip_address); - break ; - case RS_SSL_HANDSHAKE_DIAGNOSTIC_MALLOC_ERROR: - RsServer::notify()->AddFeedItem(RS_FEED_ITEM_SEC_INTERNAL_ERROR , gpgid.toStdString(), sslid.toStdString(), sslcn, ip_address); - break ; - case RS_SSL_HANDSHAKE_DIAGNOSTIC_WRONG_SIGNATURE: - RsServer::notify()->AddFeedItem(RS_FEED_ITEM_SEC_WRONG_SIGNATURE, gpgid.toStdString(), sslid.toStdString(), sslcn, ip_address); - break ; - case RS_SSL_HANDSHAKE_DIAGNOSTIC_OK: - case RS_SSL_HANDSHAKE_DIAGNOSTIC_UNKNOWN: - default: - RsServer::notify()->AddFeedItem(RS_FEED_ITEM_SEC_CONNECT_ATTEMPT, gpgid.toStdString(), sslid.toStdString(), sslcn, ip_address); - } - -#ifdef AUTHSSL_DEBUG - std::cerr << " Incoming from: "; -#endif - } - else - { - if(authed) - RsServer::notify()->AddFeedItem(RS_FEED_ITEM_SEC_AUTH_DENIED, gpgid.toStdString(), sslid.toStdString(), sslcn, ip_address); - else - RsServer::notify()->AddFeedItem(RS_FEED_ITEM_SEC_UNKNOWN_OUT, gpgid.toStdString(), sslid.toStdString(), sslcn, ip_address); - -#ifdef AUTHSSL_DEBUG - std::cerr << " Outgoing to: "; -#endif - } - -#ifdef AUTHSSL_DEBUG - std::cerr << "GpgId: " << gpgid << " SSLcn: " << sslcn << " peerId: " << sslid << ", ip address: " << ip_address; - std::cerr << std::endl; -#endif - - return false; -} - -bool AuthSSLimpl::CheckCertificate(const RsPeerId& id, X509 *x509) -{ - (void) id; /* remove unused parameter warning */ - - uint32_t diagnos ; - /* if auths -> store */ - if (AuthX509WithGPG(x509,diagnos)) - { - LocalStoreCert(x509); - return true; - } - return false; -} - - - /* Locked search -> internal help function */ -bool AuthSSLimpl::locked_FindCert(const RsPeerId& id, sslcert **cert) +bool AuthSSLimpl::locked_FindCert(const RsPeerId& id, X509** cert) { - std::map::iterator it; + std::map::iterator it; if (mCerts.end() != (it = mCerts.find(id))) { @@ -1752,21 +1544,15 @@ bool AuthSSLimpl::locked_FindCert(const RsPeerId& id, sslcert **cert) bool AuthSSLimpl::RemoveX509(RsPeerId id) { - std::map::iterator it; + std::map::iterator it; RsStackMutex stack(sslMtx); /******* LOCKED ******/ if (mCerts.end() != (it = mCerts.find(id))) { - sslcert *cert = it->second; - - /* clean up */ - X509_free(cert->certificate); - cert->certificate = NULL; - delete cert; - + X509* cert = it->second; + X509_free(cert); mCerts.erase(it); - return true; } return false; @@ -1803,15 +1589,15 @@ bool AuthSSLimpl::LocalStoreCert(X509* x509) } /* do a search */ - std::map::iterator it; + std::map::iterator it; if (mCerts.end() != (it = mCerts.find(peerId))) { - sslcert *cert = it->second; + X509* cert = it->second; /* found something */ /* check that they are exact */ - if (0 != X509_cmp(cert->certificate, x509)) + if (0 != X509_cmp(cert, x509)) { /* MAJOR ERROR */ std::cerr << "ERROR : AuthSSLimpl::LocalStoreCert() got two ssl certificates with identical ids -> dropping second"; @@ -1825,7 +1611,7 @@ bool AuthSSLimpl::LocalStoreCert(X509* x509) #ifdef AUTHSSL_DEBUG std::cerr << "AuthSSLimpl::LocalStoreCert() storing certificate for " << peerId << std::endl; #endif - mCerts[peerId] = new sslcert(X509_dup(x509), peerId); + mCerts[peerId] = X509_dup(x509); /* flag for saving config */ IndicateConfigChanged(); @@ -1859,7 +1645,7 @@ bool AuthSSLimpl::saveList(bool& cleanup, std::list& lst) // Now save config for network digging strategies RsConfigKeyValueSet *vitem = new RsConfigKeyValueSet ; - std::map::iterator mapIt; + std::map::iterator mapIt; for (mapIt = mCerts.begin(); mapIt != mCerts.end(); ++mapIt) { if (mapIt->first == mOwnId) { continue; @@ -1869,7 +1655,7 @@ bool AuthSSLimpl::saveList(bool& cleanup, std::list& lst) #ifdef AUTHSSL_DEBUG std::cerr << "AuthSSLimpl::saveList() called (mapIt->first) : " << (mapIt->first) << std::endl ; #endif - kv.value = saveX509ToPEM(mapIt->second->certificate); + kv.value = saveX509ToPEM(mapIt->second); vitem->tlvkvs.pairs.push_back(kv) ; } lst.push_back(vitem); @@ -1916,9 +1702,14 @@ bool AuthSSLimpl::loadList(std::list& load) return true; } -/********************************************************************************/ -/********************************************************************************/ -/********************************************************************************/ -/********************************************************************************/ -/********************************************************************************/ +RsAuthSslConnectionAutenticationEvent::RsAuthSslConnectionAutenticationEvent() : + RsEvent(RsEventType::AUTHSSL_CONNECTION_AUTENTICATION), mSuccess(false) {} +const EVP_PKEY*RsX509Cert::getPubKey(const X509& x509) +{ +#if OPENSSL_VERSION_NUMBER < 0x10100000L || defined(LIBRESSL_VERSION_NUMBER) + return x509.cert_info->key->pkey; +#else + return X509_get0_pubkey(&x509); +#endif +} diff --git a/libretroshare/src/pqi/authssl.h b/libretroshare/src/pqi/authssl.h index e6dd33078..0d523c189 100644 --- a/libretroshare/src/pqi/authssl.h +++ b/libretroshare/src/pqi/authssl.h @@ -3,7 +3,8 @@ * * * libretroshare: retroshare core library * * * - * Copyright 2004-2008 by Robert Fernie, Retroshare Team. * + * Copyright (C) 2004-2008 Robert Fernie * + * Copyright (C) 2019 Gioacchino Mazzurco * * * * This program is free software: you can redistribute it and/or modify * * it under the terms of the GNU Lesser General Public License as * @@ -19,21 +20,8 @@ * along with this program. If not, see . * * * *******************************************************************************/ -#ifndef MRK_AUTH_SSL_HEADER -#define MRK_AUTH_SSL_HEADER +#pragma once -/* - * This is an implementation of SSL certificate authentication, which is - * overloaded with pgp style signatures, and web-of-trust authentication. - * - * only the owner ssl cert is store, the rest is jeus callback verification - * - * To use as an SSL authentication system, you must use a common CA certificate. - * * The pqissl stuff doesn't need to differentiate between SSL, SSL + PGP, - * as its X509 certs. - * * The rsserver stuff has to distinguish between all three types ;( - * - */ #include #include @@ -42,197 +30,248 @@ #include #include "util/rsthreads.h" - #include "pqi/pqi_base.h" #include "pqi/pqinetwork.h" #include "pqi/p3cfgmgr.h" +#include "util/rsmemory.h" +#include "retroshare/rsevents.h" -/* This #define removes Connection Manager references in AuthSSL. - * They should not be here. What about Objects and orthogonality? - * This code is also stopping immediate reconnections from working. +/** + * Functions to interact elegantly with X509 certificates, using this functions + * you can avoid annoying #ifdef *SSL_VERSION_NUMBER all around the code. + * Function names should be self descriptive. */ - -class AuthSSL; - -class sslcert +namespace RsX509Cert { - public: - sslcert(X509* x509, const RsPeerId& id); - sslcert(); - - /* certificate parameters */ - RsPeerId id; - std::string name; - std::string location; - std::string org; - std::string email; - - RsPgpId issuer; - PGPFingerprintType fpr; - - /* Auth settings */ - bool authed; - - /* INTERNAL Parameters */ - X509* certificate; +std::string getCertName(const X509& x509); +std::string getCertLocation(const X509& x509); +std::string getCertOrg(const X509& x509); +RsPgpId getCertIssuer(const X509& x509); +std::string getCertIssuerString(const X509& x509); +RsPeerId getCertSslId(const X509& x509); +const EVP_PKEY* getPubKey(const X509& x509); }; -/* required to install instance */ +/** + * Event triggered by AuthSSL when authentication of a connection attempt either + * fail or success + */ +struct RsAuthSslConnectionAutenticationEvent : RsEvent +{ + RsAuthSslConnectionAutenticationEvent(); + bool mSuccess; + RsPeerId mSslId; + std::string mSslCn; + RsPgpId mPgpId; + std::string mErrorMsg; + + ///* @see RsEvent @see RsSerializable + void serial_process( RsGenericSerializer::SerializeJob j, + RsGenericSerializer::SerializeContext& ctx) override + { + RsEvent::serial_process(j, ctx); + RS_SERIAL_PROCESS(mSuccess); + RS_SERIAL_PROCESS(mSslId); + RS_SERIAL_PROCESS(mSslCn); + RS_SERIAL_PROCESS(mPgpId); + RS_SERIAL_PROCESS(mErrorMsg); + } +}; + + +/** + * This is an implementation of SSL certificate authentication with PGP + * signatures, instead of centralized certification authority. + */ class AuthSSL { - public: - AuthSSL(); +public: + static AuthSSL& instance(); -static AuthSSL *getAuthSSL(); -static void AuthSSLInit(); + RS_DEPRECATED_FOR(AuthSSL::instance()) + static AuthSSL* getAuthSSL(); - /* Initialisation Functions (Unique) */ -virtual bool validateOwnCertificate(X509 *x509, EVP_PKEY *pkey) = 0; + /* Initialisation Functions (Unique) */ + virtual bool validateOwnCertificate(X509 *x509, EVP_PKEY *pkey) = 0; -virtual bool active() = 0; -virtual int InitAuth(const char *srvr_cert, const char *priv_key, - const char *passwd, std::string alternative_location_name) = 0; -virtual bool CloseAuth() = 0; + virtual bool active() = 0; + virtual int InitAuth( + const char* srvr_cert, const char* priv_key, const char* passwd, + std::string alternative_location_name ) = 0; + virtual bool CloseAuth() = 0; /*********** Overloaded Functions from p3AuthMgr **********/ - - /* get Certificate Id */ -virtual const RsPeerId& OwnId() = 0; -virtual std::string getOwnLocation() = 0; + + /* get Certificate Id */ + virtual const RsPeerId& OwnId() = 0; + virtual std::string getOwnLocation() = 0; /* Load/Save certificates */ -virtual std::string SaveOwnCertificateToString() = 0; - + virtual std::string SaveOwnCertificateToString() = 0; + /* Sign / Encrypt / Verify Data */ -virtual bool SignData(std::string input, std::string &sign) = 0; -virtual bool SignData(const void *data, const uint32_t len, std::string &sign) = 0; + virtual bool SignData(std::string input, std::string &sign) = 0; + virtual bool SignData( + const void* data, const uint32_t len, std::string& sign ) = 0; -virtual bool SignDataBin(std::string, unsigned char*, unsigned int*) = 0; -virtual bool SignDataBin(const void*, uint32_t, unsigned char*, unsigned int*) = 0; -virtual bool VerifyOwnSignBin(const void*, uint32_t, unsigned char*, unsigned int) = 0; -virtual bool VerifySignBin(const void *data, const uint32_t len, - unsigned char *sign, unsigned int signlen, const RsPeerId& sslId) = 0; + virtual bool SignDataBin(std::string, unsigned char*, unsigned int*) = 0; + virtual bool SignDataBin( + const void*, uint32_t, unsigned char*, unsigned int* ) = 0; + virtual bool VerifyOwnSignBin( + const void*, uint32_t, unsigned char*, unsigned int ) = 0; + virtual bool VerifySignBin( + const void* data, const uint32_t len, unsigned char* sign, + unsigned int signlen, const RsPeerId& sslId ) = 0; -// return : false if encrypt failed -virtual bool encrypt(void *&out, int &outlen, const void *in, int inlen, const RsPeerId& peerId) = 0; -// return : false if decrypt fails -virtual bool decrypt(void *&out, int &outlen, const void *in, int inlen) = 0; + /// return false if failed + virtual bool encrypt( + void*& out, int& outlen, const void* in, int inlen, + const RsPeerId& peerId ) = 0; + /// return false if failed + virtual bool decrypt(void*& out, int& outlen, const void* in, int inlen) = 0; + virtual X509* SignX509ReqWithGPG(X509_REQ* req, long days) = 0; -virtual X509* SignX509ReqWithGPG(X509_REQ *req, long days) = 0; -virtual bool AuthX509WithGPG(X509 *x509,uint32_t& auth_diagnostic)=0; + /** + * @brief Verify PGP signature correcteness on given X509 certificate + * Beware this doesn't check if the PGP signer is friend or not, just if the + * signature is valid! + * @param[in] x509 pointer ti the X509 certificate to check + * @param[out] diagnostic one of RS_SSL_HANDSHAKE_DIAGNOSTIC_* diagnostic + * codes + * @return true if correctly signed, false otherwise + */ + virtual bool AuthX509WithGPG( + X509* x509, + uint32_t& diagnostic = RS_DEFAULT_STORAGE_PARAM(uint32_t) + ) = 0; + /** + * @brief Callback provided to OpenSSL to authenticate connections + * This is the ultimate place where connection attempts get accepted + * if authenticated or refused if not authenticated. + * Emits @see RsAuthSslConnectionAutenticationEvent. + * @param preverify_ok passed by OpenSSL ignored as this call is the first + * in the authentication callback chain + * @param ctx OpenSSL connection context + * @return 0 if authentication failed, 1 if success see OpenSSL + * documentation + */ + virtual int VerifyX509Callback(int preverify_ok, X509_STORE_CTX* ctx) = 0; -virtual int VerifyX509Callback(int preverify_ok, X509_STORE_CTX *ctx) = 0; -virtual bool ValidateCertificate(X509 *x509, RsPeerId& peerId) = 0; /* validate + get id */ + /// SSL specific functions used in pqissl/pqissllistener + virtual SSL_CTX* getCTX() = 0; - public: /* SSL specific functions used in pqissl/pqissllistener */ -virtual SSL_CTX *getCTX() = 0; + virtual void setCurrentConnectionAttemptInfo( + const RsPgpId& gpg_id, const RsPeerId& ssl_id, + const std::string& ssl_cn ) = 0; + virtual void getCurrentConnectionAttemptInfo( + RsPgpId& gpg_id, RsPeerId& ssl_id, std::string& ssl_cn ) = 0; -/* Restored these functions: */ -virtual void setCurrentConnectionAttemptInfo(const RsPgpId& gpg_id,const RsPeerId& ssl_id,const std::string& ssl_cn) = 0 ; -virtual void getCurrentConnectionAttemptInfo( RsPgpId& gpg_id, RsPeerId& ssl_id, std::string& ssl_cn) = 0 ; + virtual ~AuthSSL(); -virtual bool FailedCertificate(X509 *x509, const RsPgpId& gpgid,const RsPeerId& sslid,const std::string& sslcn,const struct sockaddr_storage &addr, bool incoming) = 0; /* store for discovery */ -virtual bool CheckCertificate(const RsPeerId& peerId, X509 *x509) = 0; /* check that they are exact match */ +protected: + AuthSSL() {} -static void setAuthSSL_debug(AuthSSL*) ; // used for debug only. The real function is InitSSL() -static AuthSSL *instance_ssl ; + RS_SET_CONTEXT_DEBUG_LEVEL(2) }; class AuthSSLimpl : public AuthSSL, public p3Config { - public: +public: - /* Initialisation Functions (Unique) */ + /** Initialisation Functions (Unique) */ AuthSSLimpl(); -bool validateOwnCertificate(X509 *x509, EVP_PKEY *pkey); + bool validateOwnCertificate(X509 *x509, EVP_PKEY *pkey) override; -virtual bool active(); -virtual int InitAuth(const char *srvr_cert, const char *priv_key, - const char *passwd, std::string alternative_location_name); -virtual bool CloseAuth(); + bool active() override; + int InitAuth( const char *srvr_cert, const char *priv_key, + const char *passwd, std::string alternative_location_name ) + override; + + bool CloseAuth() override; /*********** Overloaded Functions from p3AuthMgr **********/ - - /* get Certificate Id */ -virtual const RsPeerId& OwnId(); -virtual std::string getOwnLocation(); + + const RsPeerId& OwnId() override; + virtual std::string getOwnLocation() override; /* Load/Save certificates */ -virtual std::string SaveOwnCertificateToString(); - + virtual std::string SaveOwnCertificateToString() override; + /* Sign / Encrypt / Verify Data */ -virtual bool SignData(std::string input, std::string &sign); -virtual bool SignData(const void *data, const uint32_t len, std::string &sign); + bool SignData(std::string input, std::string &sign) override; + bool SignData( + const void *data, const uint32_t len, std::string &sign) override; -virtual bool SignDataBin(std::string, unsigned char*, unsigned int*); -virtual bool SignDataBin(const void*, uint32_t, unsigned char*, unsigned int*); -virtual bool VerifyOwnSignBin(const void*, uint32_t, unsigned char*, unsigned int); -virtual bool VerifySignBin(const void *data, const uint32_t len, - unsigned char *sign, unsigned int signlen, const RsPeerId& sslId); + bool SignDataBin(std::string, unsigned char*, unsigned int*) override; + virtual bool SignDataBin( + const void*, uint32_t, unsigned char*, unsigned int*) override; + bool VerifyOwnSignBin( + const void*, uint32_t, unsigned char*, unsigned int) override; + virtual bool VerifySignBin( + const void *data, const uint32_t len, unsigned char *sign, + unsigned int signlen, const RsPeerId& sslId) override; -// return : false if encrypt failed -virtual bool encrypt(void *&out, int &outlen, const void *in, int inlen, const RsPeerId& peerId); -// return : false if decrypt fails -virtual bool decrypt(void *&out, int &outlen, const void *in, int inlen); + bool encrypt( + void*& out, int& outlen, const void* in, int inlen, + const RsPeerId& peerId ) override; + bool decrypt(void *&out, int &outlen, const void *in, int inlen) override; + virtual X509* SignX509ReqWithGPG(X509_REQ *req, long days) override; -virtual X509* SignX509ReqWithGPG(X509_REQ *req, long days); -virtual bool AuthX509WithGPG(X509 *x509,uint32_t& auth_diagnostic); + /// @see AuthSSL + bool AuthX509WithGPG(X509 *x509,uint32_t& auth_diagnostic) override; - -virtual int VerifyX509Callback(int preverify_ok, X509_STORE_CTX *ctx); -virtual bool ValidateCertificate(X509 *x509, RsPeerId& peerId); /* validate + get id */ + /// @see AuthSSL + int VerifyX509Callback(int preverify_ok, X509_STORE_CTX *ctx) override; /*****************************************************************/ /*********************** p3config ******************************/ - /* Key Functions to be overloaded for Full Configuration */ - virtual RsSerialiser *setupSerialiser(); - virtual bool saveList(bool &cleanup, std::list& ); - virtual bool loadList(std::list& load); + /* Key Functions to be overloaded for Full Configuration */ + RsSerialiser* setupSerialiser() override; + bool saveList(bool &cleanup, std::list& ) override; + bool loadList(std::list& load) override; /*****************************************************************/ - public: /* SSL specific functions used in pqissl/pqissllistener */ -virtual SSL_CTX *getCTX(); +public: + /* SSL specific functions used in pqissl/pqissllistener */ + SSL_CTX* getCTX() override; -/* Restored these functions: */ -virtual void setCurrentConnectionAttemptInfo(const RsPgpId& gpg_id,const RsPeerId& ssl_id,const std::string& ssl_cn) ; -virtual void getCurrentConnectionAttemptInfo( RsPgpId& gpg_id, RsPeerId& ssl_id, std::string& ssl_cn) ; -virtual bool FailedCertificate(X509 *x509, const RsPgpId& gpgid,const RsPeerId& sslid,const std::string& sslcn,const struct sockaddr_storage &addr, bool incoming); /* store for discovery */ -virtual bool CheckCertificate(const RsPeerId& peerId, X509 *x509); /* check that they are exact match */ + /* Restored these functions: */ + void setCurrentConnectionAttemptInfo( + const RsPgpId& gpg_id, const RsPeerId& ssl_id, + const std::string& ssl_cn ) override; + void getCurrentConnectionAttemptInfo( + RsPgpId& gpg_id, RsPeerId& ssl_id, std::string& ssl_cn ) override; - private: +private: -bool LocalStoreCert(X509* x509); -bool RemoveX509(const RsPeerId id); + bool LocalStoreCert(X509* x509); + bool RemoveX509(const RsPeerId id); /*********** LOCKED Functions ******/ -bool locked_FindCert(const RsPeerId& id, sslcert **cert); + bool locked_FindCert(const RsPeerId& id, X509** cert); /* Data */ /* these variables are constants -> don't need to protect */ SSL_CTX *sslctx; RsPeerId mOwnId; - sslcert *mOwnCert; + X509* mOwnCert; - RsMutex sslMtx; /* protects all below */ + RsMutex sslMtx; /* protects all below */ - - EVP_PKEY *mOwnPrivateKey; - EVP_PKEY *mOwnPublicKey; + EVP_PKEY* mOwnPrivateKey; + EVP_PKEY* mOwnPublicKey; int init; + std::map mCerts; - std::map mCerts; - - RsPgpId _last_gpgid_to_connect ; - std::string _last_sslcn_to_connect ; - RsPeerId _last_sslid_to_connect ; + RsPgpId _last_gpgid_to_connect; + std::string _last_sslcn_to_connect; + RsPeerId _last_sslid_to_connect; }; - -#endif // MRK_AUTH_SSL_HEADER diff --git a/libretroshare/src/pqi/p3linkmgr.cc b/libretroshare/src/pqi/p3linkmgr.cc index 2110e23a8..860c916f7 100644 --- a/libretroshare/src/pqi/p3linkmgr.cc +++ b/libretroshare/src/pqi/p3linkmgr.cc @@ -1032,47 +1032,6 @@ bool p3LinkMgrIMPL::connectResult(const RsPeerId &id, bool success, bool isIncom * From various sources */ -// from pqissl, when a connection failed due to security -void p3LinkMgrIMPL::notifyDeniedConnection(const RsPgpId& gpgid,const RsPeerId& sslid,const std::string& sslcn,const struct sockaddr_storage &/*addr*/, bool incoming) -{ - std::cerr << "p3LinkMgrIMPL::notifyDeniedConnection()"; - std::cerr << " pgpid: " << gpgid; - std::cerr << " sslid: " << sslid; - std::cerr << " sslcn: " << sslcn; - std::cerr << std::endl; - - RsStackMutex stack(mLinkMtx); /****** STACK LOCK MUTEX *******/ - - std::map::iterator it; - it = mFriendList.find(sslid); - if (it == mFriendList.end()) - { - std::cerr << "p3LinkMgrIMPL::notifyDeniedConnection() of NON-FRIEND: " << sslid; - std::cerr << std::endl; - return; - } - - it->second.wasDeniedConnection = true; - it->second.deniedTS = time(NULL); - - if ((!incoming) && it->second.inConnAttempt) - { - it->second.deniedInConnAttempt = true; - it->second.deniedConnectionAttempt = it->second.currentConnAddrAttempt; - - std::cerr << "p3LinkMgrIMPL::notifyDeniedConnection() Denied In Connection Attempt"; - std::cerr << std::endl; - } - else - { - it->second.deniedInConnAttempt = false; - std::cerr << "p3LinkMgrIMPL::notifyDeniedConnection() Denied NOT In Connection Attempt"; - std::cerr << std::endl; - } - return; -} - - void p3LinkMgrIMPL::peerStatus(const RsPeerId& id, const pqiIpAddrSet &addrs, uint32_t type, uint32_t flags, uint32_t source) { diff --git a/libretroshare/src/pqi/p3linkmgr.h b/libretroshare/src/pqi/p3linkmgr.h index d10a3fbfe..c9d57ddef 100644 --- a/libretroshare/src/pqi/p3linkmgr.h +++ b/libretroshare/src/pqi/p3linkmgr.h @@ -171,8 +171,6 @@ virtual bool connectAttempt(const RsPeerId &id, struct sockaddr_storage &raddr, virtual bool connectResult(const RsPeerId &id, bool success, bool isIncomingConnection, uint32_t flags, const struct sockaddr_storage &remote_peer_address) = 0; virtual bool retryConnect(const RsPeerId &id) = 0; -virtual void notifyDeniedConnection(const RsPgpId& gpgid,const RsPeerId& sslid,const std::string& sslcn,const struct sockaddr_storage &addr, bool incoming) = 0; - /* Network Addresses */ virtual bool setLocalAddress(const struct sockaddr_storage &addr) = 0; virtual bool getLocalAddress(struct sockaddr_storage &addr) = 0; @@ -230,8 +228,6 @@ virtual bool connectAttempt(const RsPeerId &id, struct sockaddr_storage &raddr, virtual bool connectResult(const RsPeerId &id, bool success, bool isIncomingConnection, uint32_t flags, const struct sockaddr_storage &remote_peer_address); virtual bool retryConnect(const RsPeerId &id); -virtual void notifyDeniedConnection(const RsPgpId& gpgid,const RsPeerId& sslid,const std::string& sslcn,const struct sockaddr_storage &addr, bool incoming); - /* Network Addresses */ virtual bool setLocalAddress(const struct sockaddr_storage &addr); virtual bool getLocalAddress(struct sockaddr_storage &addr); diff --git a/libretroshare/src/pqi/pqissl.cc b/libretroshare/src/pqi/pqissl.cc index 2b9392ddf..85c2548b6 100644 --- a/libretroshare/src/pqi/pqissl.cc +++ b/libretroshare/src/pqi/pqissl.cc @@ -1115,9 +1115,6 @@ int pqissl::SSL_Connection_Complete() rslog(RSL_WARNING, pqisslzone, out); - // attempt real error. - Extract_Failed_SSL_Certificate(); - rslog(RSL_ALERT, pqisslzone, "pqissl::SSL_Connection_Complete() -> calling reset()"); reset_locked(); waiting = WAITING_FAIL_INTERFACE; @@ -1132,159 +1129,87 @@ int pqissl::SSL_Connection_Complete() return 1; } -int pqissl::Extract_Failed_SSL_Certificate() -{ - std::cerr << "pqissl::Extract_Failed_SSL_Certificate() FAILED Connection due to Security Issues"; - std::cerr << std::endl; - -#ifdef PQISSL_LOG_DEBUG - rslog(RSL_DEBUG_BASIC, pqisslzone, - "pqissl::Extract_Failed_SSL_Certificate()"); -#endif - - // Get the Peer Certificate.... - X509 *peercert = SSL_get_peer_certificate(ssl_connection); - - if (peercert == NULL) - { - rslog(RSL_WARNING, pqisslzone, - "pqissl::Extract_Failed_SSL_Certificate() Peer Didnt Give Cert"); - - std::cerr << "pqissl::Extract_Failed_SSL_Certificate() ERROR Peer Didn't Give Us Certificate"; - std::cerr << std::endl; - - return -1; - } - -#ifdef PQISSL_LOG_DEBUG - rslog(RSL_DEBUG_BASIC, pqisslzone, - "pqissl::Extract_Failed_SSL_Certificate() Have Peer Cert - Registering"); -#endif - - std::cerr << "pqissl::Extract_Failed_SSL_Certificate() Passing FAILED Cert to AuthSSL for analysis"; - std::cerr << std::endl; - - // save certificate... (and ip locations) - // false for outgoing.... - // we actually connected to remote_addr, - // which could be - // (pqissl's case) sslcert->serveraddr or sslcert->localaddr. - - RsPeerId sslid ; - getX509id(peercert, sslid) ; - -#if OPENSSL_VERSION_NUMBER < 0x10100000L || defined(LIBRESSL_VERSION_NUMBER) - RsPgpId gpgid(getX509CNString(peercert->cert_info->issuer)); - std::string sslcn = getX509CNString(peercert->cert_info->subject); -#else - RsPgpId gpgid(getX509CNString(X509_get_issuer_name(peercert))); - std::string sslcn = getX509CNString(X509_get_subject_name(peercert)); -#endif - - AuthSSL::getAuthSSL()->FailedCertificate(peercert, gpgid,sslid,sslcn,remote_addr, false); - mLinkMgr->notifyDeniedConnection(gpgid, sslid, sslcn, remote_addr, false); - - return 1; -} - - - - int pqissl::Authorise_SSL_Connection() { -#ifdef PQISSL_DEBUG - std::cerr << __PRETTY_FUNCTION__ << std::endl; -#endif + Dbg3() << __PRETTY_FUNCTION__ << std::endl; - if (time(NULL) > ssl_connect_timeout) + constexpr int failure = -1; + + if (time(nullptr) > ssl_connect_timeout) { - std::cerr << __PRETTY_FUNCTION__ << " Connection timed out reset!" - << std::endl; + RsInfo() << __PRETTY_FUNCTION__ << " Connection timed out reset!" + << std::endl; reset_locked(); } int err; if (0 >= (err = SSL_Connection_Complete())) return err; -#ifdef PQISSL_LOG_DEBUG - rslog(RSL_DEBUG_BASIC, pqisslzone, - "pqissl::Authorise_SSL_Connection() SSL_Connection_Complete"); -#endif + Dbg3() << __PRETTY_FUNCTION__ << "SSL_Connection_Complete success." + << std::endl; // reset switch. waiting = WAITING_NOT; - X509 *peercert = SSL_get_peer_certificate(ssl_connection); - - if (peercert == NULL) + X509* peercert = SSL_get_peer_certificate(ssl_connection); + if (!peercert) { - rslog(RSL_WARNING, pqisslzone, - "pqissl::Authorise_SSL_Connection() Peer Didnt Give Cert"); - - rslog(RSL_ALERT, pqisslzone, "pqissl::Authorise_Connection_Complete() -> calling reset()"); - // Failed completely - reset_locked(); - return -1; + RsFatal() << __PRETTY_FUNCTION__ << " failed to retrieve peer " + << "certificate at this point this should never happen!" + << std::endl; + print_stacktrace(); + exit(failure); } - RsPeerId certPeerId; - getX509id(peercert, certPeerId); - if (RsPeerId(certPeerId) != PeerId()) { - rslog(RSL_WARNING, pqisslzone, - "pqissl::Authorise_SSL_Connection() the cert Id doesn't match the Peer id we're trying to connect to."); - - rslog(RSL_ALERT, pqisslzone, "pqissl::Authorise_Connection_Complete() -> calling reset()"); - // Failed completely - reset_locked(); - return -1; - } - -#ifdef PQISSL_LOG_DEBUG - rslog(RSL_DEBUG_BASIC, pqisslzone, - "pqissl::Authorise_SSL_Connection() Have Peer Cert"); -#endif - - // save certificate... (and ip locations) - // false for outgoing.... - // we actually connected to remote_addr, - // which could be - // (pqissl's case) sslcert->serveraddr or sslcert->localaddr. - - AuthSSL::getAuthSSL()->CheckCertificate(PeerId(), peercert); - bool certCorrect = true; /* WE know it okay already! */ - - uint32_t check_result ; - uint32_t checking_flags = RSBANLIST_CHECKING_FLAGS_BLACKLIST; - if (rsPeers->servicePermissionFlags(PeerId()) & RS_NODE_PERM_REQUIRE_WL) - checking_flags |= RSBANLIST_CHECKING_FLAGS_WHITELIST; - - if(rsBanList!=NULL && !rsBanList->isAddressAccepted(remote_addr,checking_flags,&check_result)) - { - std::cerr << "(SS) refusing connection attempt from IP address " << sockaddr_storage_iptostring(remote_addr) << ". Reason: " << - ((check_result == RSBANLIST_CHECK_RESULT_NOT_WHITELISTED)?"not whitelisted (peer requires whitelist)":"blacklisted") << std::endl; - - RsServer::notify()->AddFeedItem(RS_FEED_ITEM_SEC_IP_BLACKLISTED, PeerId().toStdString(), sockaddr_storage_iptostring(remote_addr), "", "", check_result); - reset_locked(); - return 0 ; - } - // check it's the right one. - if (certCorrect) + RsPeerId certPeerId = RsX509Cert::getCertSslId(*peercert); + if (RsPeerId(certPeerId) != PeerId()) { - // then okay... - rslog(RSL_WARNING, pqisslzone, "pqissl::Authorise_SSL_Connection() Accepting Conn. Peer: " + PeerId().toStdString()); + RsErr() << __PRETTY_FUNCTION__ << " the cert Id doesn't match the peer " + << "id we're trying to connect to." << std::endl; - //std::cerr << "pqissl::Authorise_SSL_Connection(): accepting connection from " << sockaddr_storage_iptostring(remote_addr) << std::endl; - accept_locked(ssl_connection, sockfd, remote_addr); - return 1; + /* TODO: Considering how difficult is managing to get a connection to a + * friend nowadays on the Internet because of evil NAT everywhere. + * 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... */ + + reset_locked(); + return failure; } - rslog(RSL_WARNING, pqisslzone, "pqissl::Authorise_SSL_Connection() Something Wrong ... Shutdown. Peer: " + PeerId().toStdString()); +#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. */ - // else shutdown ssl connection. - rslog(RSL_ALERT, pqisslzone, "pqissl::Authorise_Connection_Complete() -> calling reset()"); + uint32_t authErrCode = 0; + if(!AuthSSL::instance().AuthX509WithGPG(peercert, authErrCode)) + { + RsFatal() << __PRETTY_FUNCTION__ << " failure verifying peer " + << "certificate signature. This should never happen at this " + << "point!" << std::endl; + print_stacktrace(); + exit(failure); + } - reset_locked(); - return 0; + 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; + print_stacktrace(); + exit(failure); + } +#endif // def RS_PQISSL_AUTH_REDUNDANT_CHECK + + Dbg2() << __PRETTY_FUNCTION__ << " Accepting connection to peer: " + << PeerId() << " with address: " << remote_addr << std::endl; + + return accept_locked(ssl_connection, sockfd, remote_addr); } @@ -1300,13 +1225,18 @@ int pqissl::accept( SSL *ssl, int fd, return accept_locked(ssl, fd, foreign_addr); } -int pqissl::accept_locked( SSL *ssl, int fd, +int pqissl::accept_locked( SSL *ssl, int fd, const sockaddr_storage &foreign_addr ) { -#ifdef PQISSL_DEBUG - std::cerr << __PRETTY_FUNCTION__ << std::endl; -#endif + Dbg3() << __PRETTY_FUNCTION__ << std::endl; + constexpr int failure = -1; + constexpr int success = 1; + +#ifdef RS_PQISSL_BANLIST_REDUNDANT_CHECK + /* TODO: It make no sense to check banlist at this point, as we are actively + * attempting the connection, we decide the address to which to connect to, + * banned addresses should never get here */ uint32_t check_result; uint32_t checking_flags = RSBANLIST_CHECKING_FLAGS_BLACKLIST; @@ -1329,14 +1259,15 @@ int pqissl::accept_locked( SSL *ssl, int fd, sockaddr_storage_iptostring(foreign_addr), "", "", check_result); reset_locked(); - return -1; + return failure; } +#endif //def RS_BANLIST_REDUNDANT_CHECK if (waiting != WAITING_NOT) { - std::cerr << __PRETTY_FUNCTION__ << " Peer: " << PeerId().toStdString() - << " - Two connections in progress - Shut 1 down!" - << std::endl; + RsInfo() << __PRETTY_FUNCTION__ << " Peer: " << PeerId() + << " - Two connections in progress - Shut 1 down!" + << std::endl; // outgoing connection in progress. // shut this baby down. @@ -1431,7 +1362,7 @@ int pqissl::accept_locked( SSL *ssl, int fd, waiting = WAITING_FAIL_INTERFACE; // failed completely. reset_locked(); - return -1; + return failure; } #ifdef PQISSL_DEBUG else std::cerr << __PRETTY_FUNCTION__ << " Socket made non-nlocking!" @@ -1456,7 +1387,7 @@ int pqissl::accept_locked( SSL *ssl, int fd, sockaddr_storage addr; sockaddr_storage_copy(remote_addr, addr); parent()->notifyEvent(this, NET_CONNECT_SUCCESS, addr); } - return 1; + return success; } /********** Implementation of BinInterface ************************** diff --git a/libretroshare/src/pqi/pqissl.h b/libretroshare/src/pqi/pqissl.h index 99408917c..72816c35c 100644 --- a/libretroshare/src/pqi/pqissl.h +++ b/libretroshare/src/pqi/pqissl.h @@ -3,8 +3,8 @@ * * * libretroshare: retroshare core library * * * - * Copyright 2004-2006 by Robert Fernie * - * Copyright (C) 2015-2018 Gioacchino Mazzurco * + * Copyright (C) 2004-2006 Robert Fernie * + * Copyright (C) 2015-2019 Gioacchino Mazzurco * * * * This program is free software: you can redistribute it and/or modify * * it under the terms of the GNU Lesser General Public License as * @@ -20,8 +20,7 @@ * along with this program. If not, see . * * * *******************************************************************************/ -#ifndef MRK_PQI_SSL_HEADER -#define MRK_PQI_SSL_HEADER +#pragma once // operating system specific network header. #include "pqi/pqinetwork.h" @@ -32,6 +31,9 @@ #include "pqi/pqi_base.h" #include "pqi/authssl.h" +#define RS_PQISSL_AUTH_REDUNDANT_CHECK 1 + + #define WAITING_NOT 0 #define WAITING_DELAY 1 #define WAITING_SOCK_CONNECT 2 @@ -159,8 +161,6 @@ int Initiate_SSL_Connection(); int SSL_Connection_Complete(); int Authorise_SSL_Connection(); -int Extract_Failed_SSL_Certificate(); // try to get cert anyway. - // check connection timeout. bool CheckConnectionTimeout(); @@ -207,9 +207,6 @@ bool CheckConnectionTimeout(); private: // ssl only fns. int connectInterface(const struct sockaddr_storage &addr); + + RS_SET_CONTEXT_DEBUG_LEVEL(1) }; - - - - -#endif // MRK_PQI_SSL_HEADER diff --git a/libretroshare/src/pqi/pqissllistener.cc b/libretroshare/src/pqi/pqissllistener.cc index a80e9b572..c54837f7d 100644 --- a/libretroshare/src/pqi/pqissllistener.cc +++ b/libretroshare/src/pqi/pqissllistener.cc @@ -20,20 +20,20 @@ * along with this program. If not, see . * * * *******************************************************************************/ + #include "pqi/pqissl.h" #include "pqi/pqissllistener.h" #include "pqi/pqinetwork.h" #include "pqi/sslfns.h" - #include "pqi/p3peermgr.h" - -#include -#include - #include "util/rsdebug.h" #include "util/rsstring.h" #include "retroshare/rsbanlist.h" +#include "pqi/authgpg.h" + #include +#include +#include static struct RsLog::logInfo pqissllistenzoneInfo = {RsLog::Default, "p3peermgr"}; #define pqissllistenzone &pqissllistenzoneInfo @@ -486,9 +486,6 @@ int pqissllistenbase::continueSSL(IncomingSSLInfo& incoming_connexion_info, bool break; } - /* we have failed -> get certificate if possible */ - Extract_Failed_SSL_Certificate(incoming_connexion_info); - closeConnection(fd, incoming_connexion_info.ssl) ; pqioutput(PQL_WARNING, pqissllistenzone, "Read Error on the SSL Socket\nShutting it down!"); @@ -502,20 +499,11 @@ int pqissllistenbase::continueSSL(IncomingSSLInfo& incoming_connexion_info, bool // X509 *x509 = SSL_get_peer_certificate(incoming_connexion_info.ssl) ; -#ifdef DEBUG_LISTENNER - std::cerr << "Info from certificate: " << std::endl; -#endif - if(x509 != NULL) - { -#if OPENSSL_VERSION_NUMBER < 0x10100000L || defined(LIBRESSL_VERSION_NUMBER) - incoming_connexion_info.gpgid = RsPgpId(std::string(getX509CNString(x509->cert_info->issuer))); - incoming_connexion_info.sslcn = getX509CNString(x509->cert_info->subject); -#else - incoming_connexion_info.gpgid = RsPgpId(std::string(getX509CNString(X509_get_issuer_name(x509)))); - incoming_connexion_info.sslcn = getX509CNString(X509_get_subject_name(x509)); -#endif - - getX509id(x509,incoming_connexion_info.sslid); + if(x509) + { + incoming_connexion_info.gpgid = RsX509Cert::getCertIssuer(*x509); + incoming_connexion_info.sslcn = RsX509Cert::getCertName(*x509); + incoming_connexion_info.sslid = RsX509Cert::getCertSslId(*x509); #ifdef DEBUG_LISTENNER std::cerr << " Got PGP Id = " << incoming_connexion_info.gpgid << std::endl; @@ -571,61 +559,6 @@ int pqissllistenbase::closeConnection(int fd, SSL *ssl) return 1; } - - - -int pqissllistenbase::Extract_Failed_SSL_Certificate(const IncomingSSLInfo& info) -{ - pqioutput(PQL_DEBUG_BASIC, pqissllistenzone, "pqissllistenbase::Extract_Failed_SSL_Certificate()"); - - std::cerr << "pqissllistenbase::Extract_Failed_SSL_Certificate() FAILED CONNECTION due to security!"; - std::cerr << std::endl; - - // Get the Peer Certificate.... - X509 *peercert = SSL_get_peer_certificate(info.ssl); - - std::cerr << "Extract_Failed_SSL_Certificate: " << std::endl; - std::cerr << " SSL = " << (void*)info.ssl << std::endl; - std::cerr << " GPG id = " << info.gpgid << std::endl; - std::cerr << " SSL id = " << info.sslid << std::endl; - std::cerr << " SSL cn = " << info.sslcn << std::endl; - std::cerr << " addr+p = " << sockaddr_storage_tostring(info.addr) << std::endl; - - if (peercert == NULL) - { - std::string out; - out += "pqissllistenbase::Extract_Failed_SSL_Certificate() from: "; - out += sockaddr_storage_tostring(info.addr); - out += " ERROR Peer didn't give Cert!"; - std::cerr << out << std::endl; - AuthSSL::getAuthSSL()->FailedCertificate(peercert, info.gpgid,info.sslid,info.sslcn,info.addr, true); - - pqioutput(PQL_WARNING, pqissllistenzone, out); - return -1; - } - - pqioutput(PQL_DEBUG_BASIC, pqissllistenzone, - "pqissllistenbase::Extract_Failed_SSL_Certificate() Have Peer Cert - Registering"); - - { - std::string out; - out += "pqissllistenbase::Extract_Failed_SSL_Certificate() from: "; - out += sockaddr_storage_tostring(info.addr); - out += " Passing Cert to AuthSSL() for analysis"; - std::cerr << out << std::endl; - - pqioutput(PQL_WARNING, pqissllistenzone, out); - std::cerr << out << std::endl; - } - - // save certificate... (and ip locations) - // false for outgoing.... - AuthSSL::getAuthSSL()->FailedCertificate(peercert, info.gpgid,info.sslid,info.sslcn,info.addr, true); - - return 1; -} - - int pqissllistenbase::continueaccepts() { @@ -830,91 +763,68 @@ int pqissllistener::status() } int pqissllistener::completeConnection(int fd, IncomingSSLInfo& info) -{ +{ + constexpr int failure = -1; + constexpr int success = 1; // Get the Peer Certificate.... - X509 *peercert = SSL_get_peer_certificate(info.ssl); - - if (peercert == NULL) + X509* peercert = SSL_get_peer_certificate(info.ssl); + if(!peercert) { - pqioutput(PQL_WARNING, pqissllistenzone, - "pqissllistener::completeConnection() Peer Did Not Provide Cert!"); - - // failure -1, pending 0, sucess 1. - // pqissllistenbase will shutdown! - return -1; + RsFatal() << __PRETTY_FUNCTION__ << " failed to retrieve peer " + << "certificate at this point this should never happen!" + << std::endl; + print_stacktrace(); + exit(failure); } - // Check cert. - RsPeerId newPeerId; + RsPgpId pgpId = RsX509Cert::getCertIssuer(*peercert); + RsPeerId newPeerId = RsX509Cert::getCertSslId(*peercert); +#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. */ - /**** - * As the validation is actually done before this... - * we should only need to call CheckCertificate here! - ****/ + uint32_t authErrCode = 0; + if(!AuthSSL::instance().AuthX509WithGPG(peercert, authErrCode)) + { + RsFatal() << __PRETTY_FUNCTION__ << " failure verifying peer " + << "certificate signature. This should never happen at this " + << "point!" << std::endl; + print_stacktrace(); + exit(failure); + } - bool certOk = AuthSSL::getAuthSSL()->ValidateCertificate(peercert, newPeerId); + 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; + print_stacktrace(); + exit(failure); + } +#endif //def RS_PQISSL_AUTH_REDUNDANT_CHECK bool found = false; - std::map::iterator it; - - // Let connected one through as well! if ((npc == NULL) || (npc -> Connected())) - if (!certOk) + for(auto it = listenaddr.begin(); !found && it != listenaddr.end(); ) { - pqioutput(PQL_WARNING, pqissllistenzone, - "pqissllistener::completeConnection() registerCertificate Failed!"); - - // bad - shutdown. - // pqissllistenbase will shutdown! - X509_free(peercert); - - return -1; + if (it -> first == newPeerId) found = true; + else ++it; } - else - { - std::string out = "pqissllistener::continueSSL()\nchecking: " + newPeerId.toStdString() + "\n"; - // check if cert is in our list..... - for(it = listenaddr.begin();(found!=true) && (it!=listenaddr.end());) - { - out + "\tagainst: " + it->first.toStdString() + "\n"; - if (it -> first == newPeerId) - { - // accept even if already connected. - out += "\t\tMatch!"; - found = true; - } - else - { - ++it; - } - } - pqioutput(PQL_DEBUG_BASIC, pqissllistenzone, out); - } - if (found == false) { - std::string out = "No Matching Certificate for Connection:"; - out += sockaddr_storage_tostring(info.addr); - out += "\npqissllistenbase: Will shut it down!"; - pqioutput(PQL_WARNING, pqissllistenzone, out); + Dbg1() << __PRETTY_FUNCTION__ << " got secure connection from address: " + << info.addr << " with previously unknown SSL certificate: " + << newPeerId << " signed by PGP friend: " << pgpId + << ". Adding the new location as SSL friend." << std::endl; - // but as it passed the authentication step, - // we can add it into the AuthSSL, and mConnMgr. - - AuthSSL::getAuthSSL()->CheckCertificate(newPeerId, peercert); - - /* now need to get GPG id too */ -#if OPENSSL_VERSION_NUMBER < 0x10100000L || defined(LIBRESSL_VERSION_NUMBER) - RsPgpId pgpid(std::string(getX509CNString(peercert->cert_info->issuer))); -#else - RsPgpId pgpid(std::string(getX509CNString(X509_get_issuer_name(peercert)))); -#endif - mPeerMgr->addFriend(newPeerId, pgpid); - - X509_free(peercert); - return -1; + mPeerMgr->addFriend(newPeerId, pgpId); } // Cleanup cert. @@ -926,17 +836,14 @@ int pqissllistener::completeConnection(int fd, IncomingSSLInfo& info) as.mSSL = info.ssl; as.mPeerId = newPeerId; as.mAddr = info.addr; - as.mAcceptTS = time(NULL); + as.mAcceptTS = time(nullptr); accepted_ssl.push_back(as); - std::string out = "pqissllistener::completeConnection() Successful Connection with: " + newPeerId.toStdString(); - out += " for Connection:"; - out += sockaddr_storage_tostring(info.addr); - out += " Adding to WAIT-ACCEPT Queue"; - pqioutput(PQL_WARNING, pqissllistenzone, out); + Dbg1() << __PRETTY_FUNCTION__ << "Successful Connection with: " + << newPeerId << " with address: " << info.addr << std::endl; - return 1; + return success; } int pqissllistener::finaliseConnection(int fd, SSL *ssl, const RsPeerId& peerId, const struct sockaddr_storage &remote_addr) diff --git a/libretroshare/src/pqi/pqissllistener.h b/libretroshare/src/pqi/pqissllistener.h index 8b072d7c5..cf3b8c93a 100644 --- a/libretroshare/src/pqi/pqissllistener.h +++ b/libretroshare/src/pqi/pqissllistener.h @@ -19,21 +19,20 @@ * along with this program. If not, see . * * * *******************************************************************************/ -#ifndef MRK_PQI_SSL_LISTEN_HEADER -#define MRK_PQI_SSL_LISTEN_HEADER +#pragma once #include -// operating system specific network header. -#include "pqi/pqinetwork.h" - #include #include #include "pqi/pqi_base.h" #include "pqi/pqilistener.h" - #include "pqi/authssl.h" +#include "util/rsdebug.h" +#include "pqi/pqinetwork.h" + +#define RS_PQISSL_AUTH_REDUNDANT_CHECK 1 /***************************** pqi Net SSL Interface ********************************* */ @@ -98,7 +97,7 @@ protected: p3PeerMgr *mPeerMgr; private: - int Extract_Failed_SSL_Certificate(const IncomingSSLInfo&); + bool active; int lsock; std::list incoming_ssl ; @@ -122,7 +121,6 @@ public: private: std::map listenaddr; + + RS_SET_CONTEXT_DEBUG_LEVEL(2) }; - - -#endif // MRK_PQI_SSL_LISTEN_HEADER diff --git a/libretroshare/src/pqi/sslfns.cc b/libretroshare/src/pqi/sslfns.cc index 59bdc884a..59aace0bc 100644 --- a/libretroshare/src/pqi/sslfns.cc +++ b/libretroshare/src/pqi/sslfns.cc @@ -31,6 +31,7 @@ #include "pqi/pqi_base.h" #include "util/rsdir.h" #include "util/rsstring.h" +#include "pqi/authssl.h" #include #include @@ -675,13 +676,6 @@ int pem_passwd_cb(char *buf, int size, int rwflag, void *password) return(strlen(buf)); } -/* XXX FIX */ -bool CheckX509Certificate(X509 */*x509*/) -{ - - return true; -} - uint64_t getX509SerialNumber(X509 *cert) { ASN1_INTEGER *serial = X509_get_serialNumber(cert); @@ -711,67 +705,39 @@ uint32_t getX509RetroshareCertificateVersion(X509 *cert) } } -// Not dependent on sslroot. load, and detroys the X509 memory. -int LoadCheckX509(const char *cert_file, RsPgpId& issuerName, std::string &location, RsPeerId &userId) +int LoadCheckX509( + const char* cert_file, RsPgpId& issuer, std::string& location, + RsPeerId& userId ) { - /* This function loads the X509 certificate from the file, - * and checks the certificate - */ + constexpr int failure = 0; + constexpr int success = 1; FILE *tmpfp = RsDirUtil::rs_fopen(cert_file, "r"); - if (tmpfp == NULL) + if (tmpfp == nullptr) { -#ifdef AUTHSSL_DEBUG - std::cerr << "sslroot::LoadCheckAndGetX509Name()"; - std::cerr << " Failed to open Certificate File:" << cert_file; - std::cerr << std::endl; -#endif - return 0; + RsErr() << __PRETTY_FUNCTION__ << " Failed to open Certificate File: " + << cert_file << std::endl; + return failure; } // get xPGP certificate. - X509 *x509 = PEM_read_X509(tmpfp, NULL, NULL, NULL); + X509* x509 = PEM_read_X509(tmpfp, nullptr, nullptr, nullptr); fclose(tmpfp); - // check the certificate. - bool valid = false; - if (x509) + if(!x509) { - valid = CheckX509Certificate(x509); - if (valid) - { - valid = getX509id(x509, userId); - } + RsErr() << __PRETTY_FUNCTION__ << " PEM_read_X509 failed!" << std::endl; + return failure; } - if (valid) - { - // extract the name. -#if OPENSSL_VERSION_NUMBER < 0x10100000L || defined(LIBRESSL_VERSION_NUMBER) - issuerName = RsPgpId(std::string(getX509CNString(x509->cert_info->issuer))); - location = getX509LocString(x509->cert_info->subject); -#else - issuerName = RsPgpId(std::string(getX509CNString(X509_get_issuer_name(x509)))); - location = getX509LocString(X509_get_subject_name(x509)); -#endif - } + userId = RsX509Cert::getCertSslId(*x509); + issuer = RsX509Cert::getCertIssuer(*x509); + location = RsX509Cert::getCertLocation(*x509); -#ifdef AUTHSSL_DEBUG - std::cout << getX509Info(x509) << std::endl ; -#endif - // clean up. X509_free(x509); - if (valid) - { - // happy! - return 1; - } - else - { - // something went wrong! - return 0; - } + if(userId.isNull() || issuer.isNull()) return failure; + else return success; } std::string getX509NameString(X509_NAME *name) @@ -896,10 +862,9 @@ std::string getX509Info(X509 *cert) /********** SSL ERROR STUFF ******************************************/ -int printSSLError(SSL *ssl, int retval, int err, unsigned long err2, std::string &out) +int printSSLError( + SSL*, int retval, int err, unsigned long err2, std::string& out ) { - (void) ssl; /* remove unused parameter warnings */ - std::string reason; std::string mainreason = std::string("UNKNOWN ERROR CODE"); @@ -939,7 +904,18 @@ int printSSLError(SSL *ssl, int retval, int err, unsigned long err2, std::string { mainreason = std::string("SSL_ERROR_SSL"); } - rs_sprintf_append(out, "RetVal(%d) -> SSL Error: %s\n\t + ERR Error: %s\n", retval, mainreason.c_str(), ERR_error_string(err2, NULL)); + rs_sprintf_append( out, + "RetVal(%d) -> SSL Error: %s\n\t + ERR Error: %s\n", + retval, mainreason.c_str(), + ERR_error_string(err2, nullptr) ); return 1; } + +std::string sslErrorToString(int retval, int err, unsigned long err2) +{ + std::string ret; + // When printSSLError will be removed it's code will be moved here + printSSLError(nullptr, retval, err, err2, ret); + return ret; +} diff --git a/libretroshare/src/pqi/sslfns.h b/libretroshare/src/pqi/sslfns.h index 0a19904e1..c43d10991 100644 --- a/libretroshare/src/pqi/sslfns.h +++ b/libretroshare/src/pqi/sslfns.h @@ -1,4 +1,4 @@ -/******************************************************************************* +/******************************************************************************* * libretroshare/src/pqi: sslfns.h * * * * libretroshare: retroshare core library * @@ -19,8 +19,7 @@ * along with this program. If not, see . * * * *******************************************************************************/ -#ifndef RS_PQI_SSL_HELPER_H -#define RS_PQI_SSL_HELPER_H +#pragma once /* Functions in this file are SSL only, * and have no dependence on SSLRoot() etc. @@ -32,9 +31,12 @@ #include #include -#include -#include #include +#include + +#include "util/rsdeprecate.h" +#include "retroshare/rstypes.h" + /**** * #define AUTHSSL_DEBUG 1 @@ -113,9 +115,12 @@ bool getX509id(X509 *x509, RsPeerId &xid); int pem_passwd_cb(char *buf, int size, int rwflag, void *password); -bool CheckX509Certificate(X509 *x509); -// Not dependent on sslroot. load, and detroys the X509 memory. -int LoadCheckX509(const char *cert_file, RsPgpId& issuer, std::string &location, RsPeerId& userId); +/** This function loads the X509 certificate from the file, and checks the + * certificate. + * Not dependent on sslroot. load, and detroys the X509 memory. */ +int LoadCheckX509( + const char* cert_file, RsPgpId& issuer, std::string& location, + RsPeerId& userId ); std::string getX509NameString(X509_NAME *name); @@ -131,7 +136,8 @@ uint32_t getX509RetroshareCertificateVersion(X509 *cert) ; /********** SSL ERROR STUFF ******************************************/ -int printSSLError(SSL *ssl, int retval, int err, unsigned long err2, std::string &out); - -#endif /* RS_PQI_SSL_HELPER_H */ +RS_DEPRECATED_FOR(sslErrorToString) +int printSSLError( + SSL* unused, int retval, int err, unsigned long err2, std::string& out); +std::string sslErrorToString(int retval, int err, unsigned long err2); diff --git a/libretroshare/src/rsserver/rsinit.cc b/libretroshare/src/rsserver/rsinit.cc index bdf2471df..41173002b 100644 --- a/libretroshare/src/rsserver/rsinit.cc +++ b/libretroshare/src/rsserver/rsinit.cc @@ -397,9 +397,8 @@ int RsInit::InitRetroShare(int argc, char **argv, bool /* strictCheck */) * 2) Get List of Available Accounts. * 4) Get List of GPG Accounts. */ - /* create singletons */ - AuthSSL::AuthSSLInit(); - AuthSSL::getAuthSSL() -> InitAuth(NULL, NULL, NULL, ""); + /* Initialize AuthSSL */ + AuthSSL::instance().InitAuth(nullptr, nullptr, nullptr, ""); rsLoginHelper = new RsLoginHelper; From fb9e17289825113906da0897c2f7039ca28225d5 Mon Sep 17 00:00:00 2001 From: Gioacchino Mazzurco Date: Sat, 4 May 2019 18:03:38 +0200 Subject: [PATCH 2/8] Delete dead code in PGP handler --- libretroshare/src/pgp/pgphandler.cc | 33 +---------------------------- libretroshare/src/pgp/pgphandler.h | 8 +------ 2 files changed, 2 insertions(+), 39 deletions(-) diff --git a/libretroshare/src/pgp/pgphandler.cc b/libretroshare/src/pgp/pgphandler.cc index 45e9a4ad8..87bb462e0 100644 --- a/libretroshare/src/pgp/pgphandler.cc +++ b/libretroshare/src/pgp/pgphandler.cc @@ -1197,37 +1197,7 @@ bool PGPHandler::locked_addOrMergeKey(ops_keyring_t *keyring,std::maptype != OPS_PTAG_CT_PUBLIC_KEY) -// { -// std::cerr << "PGPHandler::encryptTextToFile(): ERROR: supplied id did not return a public key!" << std::endl; -// return false ; -// } -// -// ops_create_info_t *info; -// ops_memory_t *buf = NULL ; -// ops_setup_memory_write(&info, &buf, 0); -// -// ops_encrypt_stream(info, public_key, NULL, ops_false, ops_true); -// ops_write(text.c_str(), text.length(), info); -// ops_writer_close(info); -// -// outstring = std::string((char *)ops_memory_get_data(buf),ops_memory_get_length(buf)) ; -// ops_create_info_delete(info); -// -// return true ; -// } + bool PGPHandler::encryptTextToFile(const RsPgpId& key_id,const std::string& text,const std::string& outfile) { RsStackMutex mtx(pgphandlerMtx) ; // lock access to PGP memory structures. @@ -2129,4 +2099,3 @@ bool PGPHandler::removeKeysFromPGPKeyring(const std::set& keys_to_remov return true ; } - diff --git a/libretroshare/src/pgp/pgphandler.h b/libretroshare/src/pgp/pgphandler.h index 5137d5ba1..e0b563a64 100644 --- a/libretroshare/src/pgp/pgphandler.h +++ b/libretroshare/src/pgp/pgphandler.h @@ -21,10 +21,6 @@ *******************************************************************************/ #pragma once -#pragma once - -// This class implements an abstract pgp handler to be used in RetroShare. -// #include #include #include @@ -80,6 +76,7 @@ class PGPCertificateInfo static const uint8_t PGP_CERTIFICATE_TYPE_RSA = 0x02 ; }; +/// This class offer an abstract pgp handler to be used in RetroShare. class PGPHandler { public: @@ -125,8 +122,6 @@ class PGPHandler bool encryptTextToFile(const RsPgpId& key_id,const std::string& text,const std::string& outfile) ; bool decryptTextFromFile(const RsPgpId& key_id,std::string& text,const std::string& encrypted_inputfile) ; - //bool encryptTextToString(const RsPgpId& key_id,const std::string& text,std::string& outstring) ; - //bool decryptTextFromString(const RsPgpId& key_id,const std::string& encrypted_text,std::string& outstring) ; bool getKeyFingerprint(const RsPgpId& id,PGPFingerprintType& fp) const ; void setAcceptConnexion(const RsPgpId&,bool) ; @@ -232,4 +227,3 @@ class PGPHandler static PassphraseCallback _passphrase_callback ; static bool mergeKeySignatures(ops_keydata_t *dst,const ops_keydata_t *src) ; // returns true if signature lists are different }; - From f9f7e0df18aeb735263bcdd30a9bd019c342bfcd Mon Sep 17 00:00:00 2001 From: Gioacchino Mazzurco Date: Tue, 7 May 2019 12:11:38 +0200 Subject: [PATCH 3/8] 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 From 1599689eab67abfce5d11cd721223a9899b4e24a Mon Sep 17 00:00:00 2001 From: Gioacchino Mazzurco Date: Tue, 14 May 2019 11:43:18 +0200 Subject: [PATCH 4/8] PQI redundant check behave the same as authssl check --- libretroshare/src/pqi/pqissl.cc | 3 ++- libretroshare/src/pqi/pqissllistener.cc | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/libretroshare/src/pqi/pqissl.cc b/libretroshare/src/pqi/pqissl.cc index ca21e70ed..8d3743c00 100644 --- a/libretroshare/src/pqi/pqissl.cc +++ b/libretroshare/src/pqi/pqissl.cc @@ -1195,7 +1195,8 @@ int pqissl::Authorise_SSL_Connection() } RsPgpId pgpId = RsX509Cert::getCertIssuer(*peercert); - if( !AuthGPG::getAuthGPG()->isGPGAccepted(pgpId) ) + if( pgpId != AuthGPG::getAuthGPG()->getGPGOwnId() && + !AuthGPG::getAuthGPG()->isGPGAccepted(pgpId) ) { RsFatal() << __PRETTY_FUNCTION__ << " pgpId: " << pgpId << " is not friend. It is very unlikely to happen at this " diff --git a/libretroshare/src/pqi/pqissllistener.cc b/libretroshare/src/pqi/pqissllistener.cc index ed36417a2..894a3640e 100644 --- a/libretroshare/src/pqi/pqissllistener.cc +++ b/libretroshare/src/pqi/pqissllistener.cc @@ -798,7 +798,8 @@ int pqissllistener::completeConnection(int fd, IncomingSSLInfo& info) exit(failure); } - if( !AuthGPG::getAuthGPG()->isGPGAccepted(pgpId) ) + if( pgpId != AuthGPG::getAuthGPG()->getGPGOwnId() && + !AuthGPG::getAuthGPG()->isGPGAccepted(pgpId) ) { RsFatal() << __PRETTY_FUNCTION__ << " pgpId: " << pgpId << " is not friend. It is very unlikely to happen at this " From 0eee4adaa8e7b8bb3b079f278b59599285c73205 Mon Sep 17 00:00:00 2001 From: Gioacchino Mazzurco Date: Tue, 14 May 2019 14:28:27 +0200 Subject: [PATCH 5/8] Add forgot define enabling banlist redundant check --- libretroshare/src/pqi/pqissl.cc | 20 ++++++++++++-------- libretroshare/src/pqi/pqissl.h | 2 ++ 2 files changed, 14 insertions(+), 8 deletions(-) diff --git a/libretroshare/src/pqi/pqissl.cc b/libretroshare/src/pqi/pqissl.cc index 8d3743c00..becd0b910 100644 --- a/libretroshare/src/pqi/pqissl.cc +++ b/libretroshare/src/pqi/pqissl.cc @@ -1240,9 +1240,11 @@ int pqissl::accept_locked( SSL *ssl, int fd, constexpr int success = 1; #ifdef RS_PQISSL_BANLIST_REDUNDANT_CHECK - /* TODO: It make no sense to check banlist at this point, as we are actively - * attempting the connection, we decide the address to which to connect to, - * banned addresses should never get here */ + /* At this point, as we are actively attempting the connection, we decide + * the address to which to connect to, banned addresses should never get + * here as the filtering for banned addresses happens much before, this + * check is therefore redundant, and if it trigger something really fishy + * must be happening (a bug somewhere else in the code). */ uint32_t check_result; uint32_t checking_flags = RSBANLIST_CHECKING_FLAGS_BLACKLIST; @@ -1253,11 +1255,13 @@ int pqissl::accept_locked( SSL *ssl, int fd, checking_flags, &check_result ) ) { - std::cerr << __PRETTY_FUNCTION__ - << " (SS) refusing incoming SSL connection from blacklisted " - << "foreign address " - << sockaddr_storage_iptostring(foreign_addr) - << ". Reason: " << check_result << "." << std::endl; + RsErr() << __PRETTY_FUNCTION__ + << " Refusing incoming SSL connection from blacklisted " + << "foreign address " << foreign_addr + << ". Reason: " << check_result << ". This should never happen " + << "at this point! Please report full log to developers!" + << std::endl; + print_stacktrace(); RsServer::notify()->AddFeedItem( RS_FEED_ITEM_SEC_IP_BLACKLISTED, diff --git a/libretroshare/src/pqi/pqissl.h b/libretroshare/src/pqi/pqissl.h index 72816c35c..8effae067 100644 --- a/libretroshare/src/pqi/pqissl.h +++ b/libretroshare/src/pqi/pqissl.h @@ -33,6 +33,8 @@ #define RS_PQISSL_AUTH_REDUNDANT_CHECK 1 +#define RS_PQISSL_BANLIST_REDUNDANT_CHECK 1 + #define WAITING_NOT 0 #define WAITING_DELAY 1 From c8082fdcc2872954b1ebce511878e23c0aecbb75 Mon Sep 17 00:00:00 2001 From: Gioacchino Mazzurco Date: Tue, 14 May 2019 15:05:19 +0200 Subject: [PATCH 6/8] Add PGP verification info message --- libretroshare/src/pqi/authssl.cc | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/libretroshare/src/pqi/authssl.cc b/libretroshare/src/pqi/authssl.cc index 6581c2e97..d29bd6619 100644 --- a/libretroshare/src/pqi/authssl.cc +++ b/libretroshare/src/pqi/authssl.cc @@ -974,9 +974,9 @@ bool AuthSSLimpl::AuthX509WithGPG(X509 *x509, uint32_t& diagnostic) RsPeerDetails pd; if (!AuthGPG::getAuthGPG()->getGPGDetails(issuer, pd)) { - RsErr() << __PRETTY_FUNCTION__ << " X509 NOT authenticated : " - << "AuthGPG::getAuthGPG()->getGPGDetails(" << issuer - << ",...) returned false." << std::endl; + RsInfo() << __PRETTY_FUNCTION__ << " X509 NOT authenticated : " + << "AuthGPG::getAuthGPG()->getGPGDetails(" << issuer + << ",...) returned false." << std::endl; diagnostic = RS_SSL_HANDSHAKE_DIAGNOSTIC_ISSUER_UNKNOWN; return false; } @@ -1125,11 +1125,11 @@ bool AuthSSLimpl::AuthX509WithGPG(X509 *x509, uint32_t& diagnostic) goto err; } - Dbg1() << __PRETTY_FUNCTION__ << " Verified: " << sigtypestring - << " signature of certificate sslId: " - << RsX509Cert::getCertSslId(*x509) - << ", Version " << std::hex << certificate_version << std::dec - << " using PGP key " << pd.fpr << " " << pd.name << std::endl; + RsInfo() << __PRETTY_FUNCTION__ << " Verified: " << sigtypestring + << " signature of certificate sslId: " + << RsX509Cert::getCertSslId(*x509) + << ", Version " << std::hex << certificate_version << std::dec + << " using PGP key " << pd.fpr << " " << pd.name << std::endl; } EVP_MD_CTX_destroy(ctx); @@ -1140,9 +1140,9 @@ bool AuthSSLimpl::AuthX509WithGPG(X509 *x509, uint32_t& diagnostic) return true; -err: - RsErr() << __PRETTY_FUNCTION__ << " X509 PGP authentication failed with " - << "diagnostic: " << diagnostic << std::endl; +err: // TODO: this label is very short and might collide every easly + RsInfo() << __PRETTY_FUNCTION__ << " X509 PGP authentication failed with " + << "diagnostic: " << diagnostic << std::endl; if(buf_in) OPENSSL_free(buf_in); @@ -1232,7 +1232,7 @@ int AuthSSLimpl::VerifyX509Callback(int /*preverify_ok*/, X509_STORE_CTX* ctx) } uint32_t auth_diagnostic; - if (!AuthX509WithGPG(x509Cert, auth_diagnostic)) + if(!AuthX509WithGPG(x509Cert, auth_diagnostic)) { std::string errMsg = "Certificate was rejected because PGP " "signature verification failed with diagnostic: " From 0c097c208079009f607027130bea94743c05a4bf Mon Sep 17 00:00:00 2001 From: Gioacchino Mazzurco Date: Tue, 14 May 2019 21:33:08 +0200 Subject: [PATCH 7/8] Rename redundant to double in guarding macros --- libretroshare/src/pqi/pqissl.cc | 4 ++-- libretroshare/src/pqi/pqissl.h | 4 ++-- libretroshare/src/pqi/pqissllistener.cc | 2 +- libretroshare/src/pqi/pqissllistener.h | 2 +- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/libretroshare/src/pqi/pqissl.cc b/libretroshare/src/pqi/pqissl.cc index becd0b910..16fd78cc9 100644 --- a/libretroshare/src/pqi/pqissl.cc +++ b/libretroshare/src/pqi/pqissl.cc @@ -1151,7 +1151,7 @@ int pqissl::Authorise_SSL_Connection() // reset switch. waiting = WAITING_NOT; -#ifdef RS_PQISSL_AUTH_REDUNDANT_CHECK +#ifdef RS_PQISSL_AUTH_DOUBLE_CHECK X509* peercert = SSL_get_peer_certificate(ssl_connection); if (!peercert) { @@ -1239,7 +1239,7 @@ int pqissl::accept_locked( SSL *ssl, int fd, constexpr int failure = -1; constexpr int success = 1; -#ifdef RS_PQISSL_BANLIST_REDUNDANT_CHECK +#ifdef RS_PQISSL_BANLIST_DOUBLE_CHECK /* At this point, as we are actively attempting the connection, we decide * the address to which to connect to, banned addresses should never get * here as the filtering for banned addresses happens much before, this diff --git a/libretroshare/src/pqi/pqissl.h b/libretroshare/src/pqi/pqissl.h index 8effae067..6246145b7 100644 --- a/libretroshare/src/pqi/pqissl.h +++ b/libretroshare/src/pqi/pqissl.h @@ -31,9 +31,9 @@ #include "pqi/pqi_base.h" #include "pqi/authssl.h" -#define RS_PQISSL_AUTH_REDUNDANT_CHECK 1 +#define RS_PQISSL_AUTH_DOUBLE_CHECK 1 -#define RS_PQISSL_BANLIST_REDUNDANT_CHECK 1 +#define RS_PQISSL_BANLIST_DOUBLE_CHECK 1 #define WAITING_NOT 0 diff --git a/libretroshare/src/pqi/pqissllistener.cc b/libretroshare/src/pqi/pqissllistener.cc index 894a3640e..d1a9e31a9 100644 --- a/libretroshare/src/pqi/pqissllistener.cc +++ b/libretroshare/src/pqi/pqissllistener.cc @@ -781,7 +781,7 @@ int pqissllistener::completeConnection(int fd, IncomingSSLInfo& info) RsPgpId pgpId = RsX509Cert::getCertIssuer(*peercert); RsPeerId newPeerId = RsX509Cert::getCertSslId(*peercert); -#ifdef RS_PQISSL_AUTH_REDUNDANT_CHECK +#ifdef RS_PQISSL_AUTH_DOUBLE_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. */ diff --git a/libretroshare/src/pqi/pqissllistener.h b/libretroshare/src/pqi/pqissllistener.h index cf3b8c93a..42ad0d0c2 100644 --- a/libretroshare/src/pqi/pqissllistener.h +++ b/libretroshare/src/pqi/pqissllistener.h @@ -32,7 +32,7 @@ #include "util/rsdebug.h" #include "pqi/pqinetwork.h" -#define RS_PQISSL_AUTH_REDUNDANT_CHECK 1 +#define RS_PQISSL_AUTH_DOUBLE_CHECK 1 /***************************** pqi Net SSL Interface ********************************* */ From 16d606b51388ddf7fa2b70dbffd31d894f52978d Mon Sep 17 00:00:00 2001 From: Gioacchino Mazzurco Date: Tue, 14 May 2019 22:05:42 +0200 Subject: [PATCH 8/8] refactor LoadCheckX509 into safer AuthSSL::parseX509DetailsFromFile --- libretroshare/src/pqi/authssl.cc | 44 ++++++++++++++++++++++-- libretroshare/src/pqi/authssl.h | 17 ++++++++- libretroshare/src/pqi/sslfns.cc | 35 ------------------- libretroshare/src/pqi/sslfns.h | 8 ----- libretroshare/src/rsserver/rsaccounts.cc | 10 ++++-- 5 files changed, 65 insertions(+), 49 deletions(-) diff --git a/libretroshare/src/pqi/authssl.cc b/libretroshare/src/pqi/authssl.cc index d29bd6619..5d56b3843 100644 --- a/libretroshare/src/pqi/authssl.cc +++ b/libretroshare/src/pqi/authssl.cc @@ -1276,8 +1276,7 @@ int AuthSSLimpl::VerifyX509Callback(int /*preverify_ok*/, X509_STORE_CTX* ctx) return verificationFailed; } - AuthSSL::instance().setCurrentConnectionAttemptInfo(pgpId, sslId, sslCn); - + setCurrentConnectionAttemptInfo(pgpId, sslId, sslCn); LocalStoreCert(x509Cert); Dbg1() << __PRETTY_FUNCTION__ << " authentication successfull!" << std::endl; @@ -1294,6 +1293,47 @@ int AuthSSLimpl::VerifyX509Callback(int /*preverify_ok*/, X509_STORE_CTX* ctx) return verificationSuccess; } +bool AuthSSLimpl::parseX509DetailsFromFile( + const std::string& certFilePath, RsPeerId& certId, + RsPgpId& issuer, std::string& location ) +{ + FILE* tmpfp = RsDirUtil::rs_fopen(certFilePath.c_str(), "r"); + if(!tmpfp) + { + RsErr() << __PRETTY_FUNCTION__ << " Failed to open Certificate File: " + << certFilePath << std::endl; + return false; + } + + // get xPGP certificate. + X509* x509 = PEM_read_X509(tmpfp, nullptr, nullptr, nullptr); + fclose(tmpfp); + + if(!x509) + { + RsErr() << __PRETTY_FUNCTION__ << " PEM_read_X509 failed!" << std::endl; + return false; + } + + uint32_t diagnostic = 0; + if(!AuthX509WithGPG(x509, diagnostic)) + { + RsErr() << __PRETTY_FUNCTION__ << " AuthX509WithGPG failed with " + << "diagnostic: " << diagnostic << std::endl; + return false; + } + + certId = RsX509Cert::getCertSslId(*x509); + issuer = RsX509Cert::getCertIssuer(*x509); + location = RsX509Cert::getCertLocation(*x509); + + X509_free(x509); + + if(certId.isNull() || issuer.isNull()) return false; + + return true; +} + /********************************************************************************/ /********************************************************************************/ diff --git a/libretroshare/src/pqi/authssl.h b/libretroshare/src/pqi/authssl.h index 0d523c189..1501eff3f 100644 --- a/libretroshare/src/pqi/authssl.h +++ b/libretroshare/src/pqi/authssl.h @@ -169,6 +169,16 @@ public: virtual void getCurrentConnectionAttemptInfo( RsPgpId& gpg_id, RsPeerId& ssl_id, std::string& ssl_cn ) = 0; + + /** + * This function parse X509 certificate from the file and return some + * verified informations, like ID and signer + * @return false on error, true otherwise + */ + virtual bool parseX509DetailsFromFile( + const std::string& certFilePath, RsPeerId& certId, RsPgpId& issuer, + std::string& location ) = 0; + virtual ~AuthSSL(); protected: @@ -223,11 +233,16 @@ public: virtual X509* SignX509ReqWithGPG(X509_REQ *req, long days) override; /// @see AuthSSL - bool AuthX509WithGPG(X509 *x509,uint32_t& auth_diagnostic) override; + bool AuthX509WithGPG(X509 *x509, uint32_t& auth_diagnostic) override; /// @see AuthSSL int VerifyX509Callback(int preverify_ok, X509_STORE_CTX *ctx) override; + /// @see AuthSSL + bool parseX509DetailsFromFile( + const std::string& certFilePath, RsPeerId& certId, + RsPgpId& issuer, std::string& location ) override; + /*****************************************************************/ /*********************** p3config ******************************/ diff --git a/libretroshare/src/pqi/sslfns.cc b/libretroshare/src/pqi/sslfns.cc index 59aace0bc..cb799ed86 100644 --- a/libretroshare/src/pqi/sslfns.cc +++ b/libretroshare/src/pqi/sslfns.cc @@ -705,41 +705,6 @@ uint32_t getX509RetroshareCertificateVersion(X509 *cert) } } -int LoadCheckX509( - const char* cert_file, RsPgpId& issuer, std::string& location, - RsPeerId& userId ) -{ - constexpr int failure = 0; - constexpr int success = 1; - - FILE *tmpfp = RsDirUtil::rs_fopen(cert_file, "r"); - if (tmpfp == nullptr) - { - RsErr() << __PRETTY_FUNCTION__ << " Failed to open Certificate File: " - << cert_file << std::endl; - return failure; - } - - // get xPGP certificate. - X509* x509 = PEM_read_X509(tmpfp, nullptr, nullptr, nullptr); - fclose(tmpfp); - - if(!x509) - { - RsErr() << __PRETTY_FUNCTION__ << " PEM_read_X509 failed!" << std::endl; - return failure; - } - - userId = RsX509Cert::getCertSslId(*x509); - issuer = RsX509Cert::getCertIssuer(*x509); - location = RsX509Cert::getCertLocation(*x509); - - X509_free(x509); - - if(userId.isNull() || issuer.isNull()) return failure; - else return success; -} - std::string getX509NameString(X509_NAME *name) { std::string namestr; diff --git a/libretroshare/src/pqi/sslfns.h b/libretroshare/src/pqi/sslfns.h index c43d10991..c22076c51 100644 --- a/libretroshare/src/pqi/sslfns.h +++ b/libretroshare/src/pqi/sslfns.h @@ -115,14 +115,6 @@ bool getX509id(X509 *x509, RsPeerId &xid); int pem_passwd_cb(char *buf, int size, int rwflag, void *password); -/** This function loads the X509 certificate from the file, and checks the - * certificate. - * Not dependent on sslroot. load, and detroys the X509 memory. */ -int LoadCheckX509( - const char* cert_file, RsPgpId& issuer, std::string& location, - RsPeerId& userId ); - - std::string getX509NameString(X509_NAME *name); std::string getX509CNString(X509_NAME *name); std::string getX509TypeString(X509_NAME *name, const char *type, int len); diff --git a/libretroshare/src/rsserver/rsaccounts.cc b/libretroshare/src/rsserver/rsaccounts.cc index 1203a63a2..8be56073c 100644 --- a/libretroshare/src/rsserver/rsaccounts.cc +++ b/libretroshare/src/rsserver/rsaccounts.cc @@ -686,7 +686,8 @@ static bool checkAccount(const std::string &accountdir, AccountDetails &account, bool ret = false; /* check against authmanagers private keys */ - if (LoadCheckX509(cert_name.c_str(), account.mPgpId, account.mLocation, account.mSslId)) + if(AuthSSL::instance().parseX509DetailsFromFile( + cert_name, account.mSslId, account.mPgpId, account.mLocation )) { // new locations store the name in an extra file if(account.mLocation == "") @@ -1117,8 +1118,11 @@ bool RsAccountsDetail::GenerateSSLCertificate(const RsPgpId& pgp_id, const s std::string location; RsPgpId pgpid_retrieved; - if (LoadCheckX509(cert_name.c_str(), pgpid_retrieved, location, sslId) == 0) { - std::cerr << "RsInit::GenerateSSLCertificate() Cannot check own signature, maybe the files are corrupted." << std::endl; + if(!AuthSSL::instance().parseX509DetailsFromFile( + cert_name, sslId, pgpid_retrieved, location )) + { + RsErr() << __PRETTY_FUNCTION__ << " Cannot check own signature, maybe " + << "the files are corrupted." << std::endl; return false; }