refactor LoadCheckX509 into safer AuthSSL::parseX509DetailsFromFile

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;
+}
+
 
 /********************************************************************************/
 /********************************************************************************/

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  ******************************/

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;

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);

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) {
+	if(!AuthSSL::instance().parseX509DetailsFromFile(
-		std::cerr << "RsInit::GenerateSSLCertificate() Cannot check own signature, maybe the files are corrupted." << std::endl;
+	            cert_name, sslId, pgpid_retrieved, location ))
+	{
+		RsErr() << __PRETTY_FUNCTION__ << " Cannot check own signature, maybe "
+		        << "the files are corrupted." << std::endl;
 		return false;
 	}