Fix bugs in getting certs and pass check

Password check on pkcs12 makes undefined behavior. peaking last error is
suspicious so ERR_get_error used and works well.

Parsing certificate of PEM format with TRUSTED CERTIFICATE header didn't
work. For trusted certificate case, use PEM_read_bio_X509_AUX first
because it works well on both of TRUSETD CERTIFICATE and CERTIFICATE.
Try 4 formats step by step. PEM(AUX), PEM, BASE64, DER.

Change-Id: I6d81393bc31b2e740365ae3b0b4962fd9a6e55dc
Signed-off-by: Kyungwook Tak <k.tak@samsung.com>

3 files changed, 77 insertions, 61 deletions

diff --git a/vcore/vcore/Client.cpp b/vcore/vcore/Client.cpp
index 6767ca8..d0f48ae 100644
--- a/vcore/vcore/Client.cpp
+++ b/vcore/vcore/Client.cpp
@@ -489,16 +489,19 @@ int vcore_client_get_certificate_from_store(CertStoreType storeType, const char
 
 	if (recvData.dataBlockLen > 0 && recvData.dataBlockLen <= VCORE_MAX_RECV_DATA_SIZE) {
 		outData = (char *)malloc(recvData.dataBlockLen + 1);
+		if (outData == nullptr)
+			return CERTSVC_BAD_ALLOC;
+
 		memset(outData, 0x00, recvData.dataBlockLen + 1);
 		memcpy(outData, recvData.dataBlock, recvData.dataBlockLen);
 		*certData = outData;
 		*certSize = recvData.dataBlockLen;
+
+		return recvData.result;
 	} else {
 		LogError("revcData length is wrong : " << recvData.dataBlockLen);
 		return CERTSVC_WRONG_ARGUMENT;
 	}
-
-	return recvData.result;
 }
 
 int vcore_client_delete_certificate_from_store(CertStoreType storeType, const char *gname)
diff --git a/vcore/vcore/api.cpp b/vcore/vcore/api.cpp
index 6d125bf..4554e77 100644
--- a/vcore/vcore/api.cpp
+++ b/vcore/vcore/api.cpp
@@ -1559,86 +1559,85 @@ int certsvc_get_certificate(CertSvcInstance instance,
 							const char *gname,
 							CertSvcCertificate *certificate)
 {
-	int result = CERTSVC_SUCCESS;
-	char *certBuffer = NULL;
-	std::string fileName;
-	size_t length = 0;
-	FILE *fp_write = NULL;
-	BIO *pBio = NULL;
-	X509 *x509Struct = NULL;
-
 	try {
-		result = vcore_client_get_certificate_from_store(storeType, gname, &certBuffer, &length, PEM_CRT);
+		size_t len = 0;
+		char *certbuf = nullptr;
+		int result = vcore_client_get_certificate_from_store(storeType, gname, &certbuf,
+															 &len, PEM_CRT);
 
 		if (result != CERTSVC_SUCCESS) {
 			LogError("Failed to get certificate buffer from store.");
 			return result;
 		}
+		std::unique_ptr<char, void(*)(void *)> certbufptr(certbuf, free);
 
-		pBio = BIO_new(BIO_s_mem());
+		LogInfo("certbuf: " << certbuf);
 
-		if (pBio == NULL) {
+		std::unique_ptr<BIO, int(*)(BIO *)> bioptr(BIO_new(BIO_s_mem()), BIO_free);
+		if (bioptr == nullptr) {
 			LogError("Failed to allocate memory.");
-			result = CERTSVC_BAD_ALLOC;
+			return CERTSVC_BAD_ALLOC;
 		}
 
-		length = BIO_write(pBio, (const void *)certBuffer, (int)length);
+		len = BIO_write(bioptr.get(), (const void *)certbuf, (int)len);
 
-		if ((int)length < 1) {
+		if ((int)len < 1) {
 			LogError("Failed to load cert into bio.");
-			result = CERTSVC_BAD_ALLOC;
+			return CERTSVC_BAD_ALLOC;
 		}
 
-		x509Struct = PEM_read_bio_X509(pBio, NULL, 0, NULL);
-
-		if (x509Struct != NULL) {
-			CertificatePtr cert(new Certificate(x509Struct));
-			certificate->privateInstance = instance;
-			certificate->privateHandler = impl(instance)->addCert(cert);
-
-			if (certBuffer != NULL) free(certBuffer);
-		} else {
-			fileName.append(CERTSVC_PKCS12_STORAGE_DIR);
-			fileName.append(gname);
-
-			if (!(fp_write = fopen(fileName.c_str(), "w"))) {
-				LogError("Failed to open the file for writing, [" << fileName << "].");
-				result = CERTSVC_FAIL;
-				goto error;
-			}
+		std::unique_ptr<X509, void(*)(X509 *)> x509ptr(
+				PEM_read_bio_X509_AUX(bioptr.get(), nullptr, nullptr, nullptr), X509_free);
 
-			if (fwrite(certBuffer, sizeof(char), length, fp_write) != length) {
-				LogError("Fail to write certificate.");
-				result = CERTSVC_FAIL;
-				goto error;
-			}
+		CertificatePtr cert;
 
-			result = certsvc_certificate_new_from_file(instance, fileName.c_str(), certificate);
+		if (x509ptr != nullptr) {
+			LogInfo("PEM_read_bio_X509_AUX returned x509 struct!");
+			try {
+				cert.reset(new Certificate(x509ptr.get()));
+				LogInfo("Parse cert success with PEM(AUX) form!");
+			} catch (...) {}
+		}
 
-			if (result != CERTSVC_SUCCESS) {
-				LogError("Failed to construct certificate from buffer.");
-				goto error;
+		if (cert == nullptr) {
+			x509ptr.reset(PEM_read_bio_X509(bioptr.get(), nullptr, nullptr, nullptr));
+			if (x509ptr != nullptr) {
+				LogInfo("PEM_read_bio_X509 returned x509 struct!");
+				try {
+					cert.reset(new Certificate(x509ptr.get()));
+					LogInfo("Parse cert success with PEM form!");
+				} catch (...) {}
 			}
-
-			unlink(fileName.c_str());
 		}
 
-		result = CERTSVC_SUCCESS;
-	} catch (std::bad_alloc &) {
-		result = CERTSVC_BAD_ALLOC;
-	} catch (...) {}
+		if (cert == nullptr) {
+			try {
+				cert.reset(new Certificate(certbuf, Certificate::FormType::FORM_BASE64));
+				LogInfo("Parse cert success with Base64 form!");
+			} catch (...) {}
+		}
 
-error:
-	if (x509Struct)
-		X509_free(x509Struct);
+		if (cert == nullptr) {
+			try {
+				cert.reset(new Certificate(certbuf, Certificate::FormType::FORM_DER));
+				LogInfo("Parse cert success with DER form!");
+			} catch (...) {}
+		}
 
-	if (pBio)
-		BIO_free(pBio);
+		if (cert == nullptr) {
+			LogError("Failed to parse cert on all of PEM/DER/Base64 form!");
+			return CERTSVC_INVALID_CERTIFICATE;
+		}
 
-	if (fp_write)
-		fclose(fp_write);
+		certificate->privateInstance = instance;
+		certificate->privateHandler = impl(instance)->addCert(cert);
 
-	return result;
+		return CERTSVC_SUCCESS;
+	} catch (std::bad_alloc &) {
+		return CERTSVC_BAD_ALLOC;
+	} catch (...) {
+		return CERTSVC_FAIL;
+	}
 }
 
 int certsvc_pkcs12_check_alias_exists_in_store(CertSvcInstance instance,
diff --git a/vcore/vcore/pkcs12.cpp b/vcore/vcore/pkcs12.cpp
index 0d818cc..5985005 100644
--- a/vcore/vcore/pkcs12.cpp
+++ b/vcore/vcore/pkcs12.cpp
@@ -560,7 +560,10 @@ int extractPkcs12(const std::string &path,
 	PKCS12_free(container);
 
 	if (result != 1) {
-		LogError("Failed to parse the file passed. openssl err : " << ERR_get_error());
+		unsigned long e = ERR_get_error();
+		char buf[1024];
+		ERR_error_string_n(e, buf, 1023);
+		LogError("Failed to parse the file passed. openssl err: " << buf);
 		return CERTSVC_FAIL;
 	}
 
@@ -837,9 +840,20 @@ int pkcs12_has_password(const char *filepath, int *passworded)
 	if (cert != NULL)
 		X509_free(cert);
 
-	if (result != 1 && ERR_GET_REASON(ERR_peek_last_error()) != PKCS12_R_MAC_VERIFY_FAILURE)
-		return CERTSVC_FAIL;
+	if (result != 1) {
+		unsigned long e = ERR_get_error();
+		if (ERR_GET_REASON(e) == PKCS12_R_MAC_VERIFY_FAILURE) {
+			LogInfo("verify failed without password. file(" << filepath << ") is password-protected.");
+			*passworded = CERTSVC_TRUE;
+		} else {
+			char buf[1024];
+			ERR_error_string_n(e, buf, 1023);
+			LogError("Error on PKCS12_pasre file(" << filepath << "): " << buf);
+			return CERTSVC_FAIL;
+		}
+	} else {
+		*passworded = CERTSVC_FALSE;
+	}
 
-	*passworded = (result == 1) ? 1 : 0;
 	return CERTSVC_SUCCESS;
 }