Process author signiture validation

[AS-IS]
* Since duplicated check during validation,
  author signiture validation was skip.
[TO-BE]
* Process author signiture validation.
* Duplicated check will improve additional API.

Change-Id: I9aff5589a4ee7ec97fb0f7b4206b322a1b3a6b98

6 files changed, 77 insertions, 59 deletions

diff --git a/tests/vcore/test-signature-validator.cpp b/tests/vcore/test-signature-validator.cpp
index 640e77e..e6807f1 100644
--- a/tests/vcore/test-signature-validator.cpp
+++ b/tests/vcore/test-signature-validator.cpp
@@ -432,6 +432,7 @@ RUNNER_TEST(T00154_negative_signature_uncheck_ref)
                 false,
                 data);
 
+        // TODO(sangwan.kwon) : delete if condition about author signature
         if (!data.isAuthorSignature())
             RUNNER_ASSERT_MSG(result == E_SIG_INVALID_SIG,
                 "dist sig should be failed: "
@@ -456,14 +457,9 @@ RUNNER_TEST(T00155_negative_tpk_with_added_malfile)
                 true,
                 data);
 
-        if (data.isAuthorSignature())
-            RUNNER_ASSERT_MSG(result == E_SIG_NONE,
-                "author sig validation should be success: "
-                << validator.errorToString(result));
-        else
-            RUNNER_ASSERT_MSG(result == E_SIG_INVALID_REF,
-                "dist sig validation should be failed: "
-                << validator.errorToString(result));
+        RUNNER_ASSERT_MSG(result == E_SIG_INVALID_REF,
+            "dist sig validation should be failed: "
+            << validator.errorToString(result));
     }
 }
 
diff --git a/vcore/vcore/Certificate.cpp b/vcore/vcore/Certificate.cpp
index 440020f..7d1109b 100644
--- a/vcore/vcore/Certificate.cpp
+++ b/vcore/vcore/Certificate.cpp
@@ -349,8 +349,6 @@ std::string Certificate::getNameHash(FieldType type) const
 
     snprintf(buf, 9, "%08lx", ulNameHash);
 
-    LogDebug("str name hash [" << buf << "]");
-
     return std::string(buf);
 }
 
@@ -441,29 +439,37 @@ Certificate::AltNameSet Certificate::getAlternativeNameDNS() const
     return set;
 }
 
-time_t Certificate::getNotAfter() const
+ASN1_TIME* Certificate::getNotAfterTime() const
 {
-    ASN1_TIME *time = X509_get_notAfter(m_x509);
-    if (!time)
+    auto timeafter = X509_get_notAfter(m_x509);
+    if (!timeafter)
         VcoreThrowMsg(Certificate::Exception::OpensslInternalError,
                       "Reading Not After error.");
 
-    time_t output;
-    if (asn1TimeToTimeT(time, &output) == 0)
-        VcoreThrowMsg(Certificate::Exception::OpensslInternalError,
-                      "Converting ASN1_time to time_t error.");
+    LogDebug("Get notAfter ASN1_TIME : " <<
+        reinterpret_cast<char *>(timeafter->data));
 
-    return output;
+    return timeafter;
 }
 
-time_t Certificate::getNotBefore() const
+ASN1_TIME* Certificate::getNotBeforeTime() const
 {
-    ASN1_TIME *time = X509_get_notBefore(m_x509);
-    if (!time)
+    auto timebefore = X509_get_notBefore(m_x509);
+    if (!timebefore)
         VcoreThrowMsg(Certificate::Exception::OpensslInternalError,
                       "Reading Not Before error.");
 
+    LogDebug("Get notBefore ASN1_TIME : " <<
+        reinterpret_cast<char *>(timebefore->data));
+
+    return timebefore;
+}
+
+time_t Certificate::getNotAfter() const
+{
+    auto time = getNotAfterTime();
     time_t output;
+
     if (asn1TimeToTimeT(time, &output) == 0)
         VcoreThrowMsg(Certificate::Exception::OpensslInternalError,
                       "Converting ASN1_time to time_t error.");
@@ -471,24 +477,16 @@ time_t Certificate::getNotBefore() const
     return output;
 }
 
-ASN1_TIME* Certificate::getNotAfterTime() const
+time_t Certificate::getNotBefore() const
 {
-    ASN1_TIME *timeafter = X509_get_notAfter(m_x509);
-    if (!timeafter)
-        VcoreThrowMsg(Certificate::Exception::OpensslInternalError,
-                      "Reading Not After error.");
-
-    return timeafter;
-}
+    auto time = getNotBeforeTime();
+    time_t output;
 
-ASN1_TIME* Certificate::getNotBeforeTime() const
-{
-    ASN1_TIME *timebefore = X509_get_notBefore(m_x509);
-    if (!timebefore)
+    if (asn1TimeToTimeT(time, &output) == 0)
         VcoreThrowMsg(Certificate::Exception::OpensslInternalError,
-                      "Reading Not Before error.");
+                      "Converting ASN1_time to time_t error.");
 
-    return timebefore;
+    return output;
 }
 
 bool Certificate::isRootCert()
diff --git a/vcore/vcore/CertificateCollection.cpp b/vcore/vcore/CertificateCollection.cpp
index 61040fd..6dc6e85 100644
--- a/vcore/vcore/CertificateCollection.cpp
+++ b/vcore/vcore/CertificateCollection.cpp
@@ -67,8 +67,6 @@ bool isHashMatchedFile(const std::string &path, const std::string &hash)
 	CertificatePtr certPtr = Certificate::createFromFile(path);
 	std::string name = certPtr->getNameHash(Certificate::FIELD_SUBJECT);
 
-	LogDebug("candidate file path[" << path << "] name[" << name << "] hash[" << hash << "]");
-
 	return isHashMatchedName(name, hash);
 }
 
@@ -139,6 +137,7 @@ CertificatePtr searchCert(const std::string &dir, const CertificatePtr &certPtr,
 
 CertificatePtr getIssuerCertFromStore(const CertificatePtr &certPtr)
 {
+	LogDebug("Start to get issuer from store.");
 	CertificatePtr found = searchCert(TZ_SYS_CA_CERTS_TIZEN, certPtr, false);
 	if (found.get() != NULL) {
 		LogDebug("Found issuer cert in tizen root CA dir");
diff --git a/vcore/vcore/SignatureValidator.cpp b/vcore/vcore/SignatureValidator.cpp
index e3220a4..187c4d4 100644
--- a/vcore/vcore/SignatureValidator.cpp
+++ b/vcore/vcore/SignatureValidator.cpp
@@ -219,6 +219,7 @@ VCerr SignatureValidator::Impl::parseSignature(void)
  */
 VCerr SignatureValidator::Impl::makeDataBySignature(bool completeWithSystemCert)
 {
+	LogDebug("Start to make chain.");
 	m_data = SignatureData(m_fileInfo.getFileName(), m_fileInfo.getFileNumber());
 
 	if (parseSignature()) {
@@ -231,24 +232,28 @@ VCerr SignatureValidator::Impl::makeDataBySignature(bool completeWithSystemCert)
 
 	try {
 		CertificateCollection collection;
-		collection.load(m_data.getCertList());
 
+		// Load Certificates and make chain.
+		collection.load(m_data.getCertList());
 		if (!collection.sort() || collection.empty()) {
 			LogError("Certificates do not form valid chain.");
 			return E_SIG_INVALID_CHAIN;
 		}
 
+		// Add root certificate to chain.
 		if (completeWithSystemCert && !collection.completeCertificateChain()) {
 			if (m_data.isAuthorSignature() || m_data.getSignatureNumber() == 1) {
 				LogError("Failed to complete cert chain with system cert");
 				return E_SIG_INVALID_CHAIN;
 			} else {
-				LogError("distributor's signature has got unrecognized root CA certificate.");
+				LogDebug("Distributor N's certificate has got "
+					"unrecognized root CA certificate.");
 				m_disregarded = true;
 			}
 		}
 
 		m_data.setSortedCertificateList(collection.getChain());
+		LogDebug("Finish making chain successfully.");
 
 	} catch (const CertificateCollection::Exception::Base &e) {
 		LogError("CertificateCollection exception : " << e.DumpToString());
@@ -266,36 +271,41 @@ VCerr SignatureValidator::Impl::makeDataBySignature(bool completeWithSystemCert)
 
 VCerr SignatureValidator::Impl::preStep(void)
 {
+	// Make chain process.
 	VCerr result = makeDataBySignature(true);
 	if (result != E_SIG_NONE)
 		return result;
 
 	// Get Identifier from fingerprint original, extention file.
+	LogDebug("Start to check certificate domain.");
 	auto certificatePtr = m_data.getCertList().back();
 	auto storeIdSet = createCertificateIdentifier().find(certificatePtr);
 
-	// Is Root CA certificate trusted?
+	// Check root CA certificate has proper domain.
 	LogDebug("root certificate from " << storeIdSet.typeToString() << " domain");
 	if (m_data.isAuthorSignature()) {
 		if (!storeIdSet.contains(TIZEN_DEVELOPER)) {
-			LogError("author-signature.xml's root certificate isn't in tizen developer domain.");
+			LogError("author-signature.xml's root certificate "
+				"isn't in tizen developer domain.");
 			return E_SIG_INVALID_CHAIN;
 		}
 	} else {
 		if (storeIdSet.contains(TIZEN_DEVELOPER)) {
-			LogError("distributor signautre root certificate shouldn't be in tizen developer domain.");
+			LogError("distributor signautre root certificate "
+				"shouldn't be in tizen developer domain.");
 			return E_SIG_INVALID_CHAIN;
 		}
 		if (m_data.getSignatureNumber() == 1 && !storeIdSet.isContainsVis()) {
 			LogError("signature1.xml has got unrecognized root CA certificate.");
 			return E_SIG_INVALID_CHAIN;
 		} else if (!storeIdSet.isContainsVis()) {
-			LogError("signatureN.xml (not 1) has got unrecognized root CA certificate.");
+			LogDebug("signatureN.xml has got unrecognized root CA certificate.");
 			m_disregarded = true;
 		}
 	}
 
 	m_data.setStorageType(storeIdSet);
+	LogDebug("Finish checking certificate domain.");
 
 	/*
 	 * We add only Root CA certificate because the rest
@@ -311,6 +321,7 @@ VCerr SignatureValidator::Impl::preStep(void)
 	CertTimeStatus status = _timeValidation(lower, upper, current);
 
 	if (status != CertTimeStatus::VALID) {
+		LogDebug("Certificate's time is invalid.");
 		if (_isTimeStrict(storeIdSet))
 			return status == CertTimeStatus::EXPIRED
 					? E_SIG_CERT_EXPIRED : E_SIG_CERT_NOT_YET;
@@ -333,31 +344,35 @@ VCerr SignatureValidator::Impl::baseCheck(
 	bool checkReferences)
 {
 	try {
+		// Make certificate chain, check certificate info
 		VCerr result = preStep();
 		if (result != E_SIG_NONE)
 			return result;
 
-		if (!m_data.isAuthorSignature()) {
-			if (!m_data.getSignatureNumber() != 1)
-				m_context.allowBrokenChain = true;
+		// Since disregard case, uncheck root certs in signatureN.xml
+		if (!m_data.isAuthorSignature() && m_data.getSignatureNumber() != 1)
+			m_context.allowBrokenChain = true;
 
-			XmlSecSingleton::Instance().validate(m_context);
+		// XmlSec validate
+		XmlSecSingleton::Instance().validate(m_context);
 
-			m_data.setReference(m_context.referenceSet);
-			if (!checkObjectReferences()) {
-				LogWarning("Failed to check Object References");
-				return E_SIG_INVALID_REF;
-			}
+		// Check reference of 'Object' tag - OID
+		m_data.setReference(m_context.referenceSet);
+		if (!checkObjectReferences()) {
+			LogWarning("Failed to check Object References");
+			return E_SIG_INVALID_REF;
+		}
 
-			if (checkReferences) {
-				ReferenceValidator fileValidator(contentPath);
-				if (ReferenceValidator::NO_ERROR != fileValidator.checkReferences(m_data)) {
-					LogWarning("Invalid package - file references broken");
-					return E_SIG_INVALID_REF;
-				}
+		// Check reference's existence
+		if (checkReferences) {
+			ReferenceValidator fileValidator(contentPath);
+			if (ReferenceValidator::NO_ERROR != fileValidator.checkReferences(m_data)) {
+				LogWarning("Invalid package - file references broken");
+				return E_SIG_INVALID_REF;
 			}
 		}
 
+		// Check OCSP
 		if (checkOcsp && Ocsp::check(m_data) == Ocsp::Result::REVOKED) {
 			LogError("Certificate is Revoked by OCSP server.");
 			return E_SIG_REVOKED;
@@ -405,10 +420,12 @@ VCerr SignatureValidator::Impl::baseCheckList(
 	const UriList &uriList)
 {
 	try {
+		// Make certificate chain, check certificate info
 		VCerr result = preStep();
 		if (result != E_SIG_NONE)
 			return result;
 
+		// XmlSec validate
 		if (uriList.size() == 0)
 			XmlSecSingleton::Instance().validateNoHash(m_context);
 		else
diff --git a/vcore/vcore/TimeConversion.cpp b/vcore/vcore/TimeConversion.cpp
index 67c258b..ad0cf38 100644
--- a/vcore/vcore/TimeConversion.cpp
+++ b/vcore/vcore/TimeConversion.cpp
@@ -21,6 +21,7 @@
  * @brief
  */
 #include <vcore/TimeConversion.h>
+#include <dpl/log/log.h>
 
 #include <cstring>
 #include <climits>
@@ -285,11 +286,17 @@ int asn1TimeToTimeT(ASN1_TIME *t, time_t *res)
     if (ret == 0)
         return 0;
 
+    LogDebug("Convert asn1 to struct tm : "
+        << tm.tm_year + 1900 <<  tm.tm_mon + 1 << tm.tm_mday);
     *res = mktime(&tm);
 
     // If time_t occured overflow, set TIME_MAX.
-    if(*res == -1)
+    if(*res == -1) {
+        LogDebug("Occured overflow time_t. it may year 2038 problem.");
         *res = TIME_MAX;
+    }
+
+    LogDebug("Convert struct tm to time_t : " << asctime(gmtime(res)));
 
     return 1;
 }
diff --git a/vcore/vcore/XmlsecAdapter.cpp b/vcore/vcore/XmlsecAdapter.cpp
index 8b09326..8093412 100644
--- a/vcore/vcore/XmlsecAdapter.cpp
+++ b/vcore/vcore/XmlsecAdapter.cpp
@@ -370,6 +370,7 @@ void XmlSec::loadPEMCertificateFile(XmlSecContext &context, xmlSecKeysMngrPtr mn
 
 void XmlSec::validateInternal(XmlSecContext &context)
 {
+    LogDebug("Start to validate.");
     Assert(!context.signatureFile.empty());
     Assert(!!context.certificatePtr || !context.certificatePath.empty());