Process author signiture validation 92/67492/6
authorsangwan.kwon <sangwan.kwon@samsung.com>
Wed, 20 Apr 2016 06:08:23 +0000 (15:08 +0900)
committersangwan.kwon <sangwan.kwon@samsung.com>
Wed, 27 Apr 2016 04:51:25 +0000 (13:51 +0900)
[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

tests/vcore/test-signature-validator.cpp
vcore/vcore/Certificate.cpp
vcore/vcore/CertificateCollection.cpp
vcore/vcore/SignatureValidator.cpp
vcore/vcore/TimeConversion.cpp
vcore/vcore/XmlsecAdapter.cpp

index 640e77e..e6807f1 100644 (file)
@@ -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));
     }
 }
 
index 440020f..7d1109b 100644 (file)
@@ -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()
index 61040fd..6dc6e85 100644 (file)
@@ -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");
index e3220a4..187c4d4 100644 (file)
@@ -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
index 67c258b..ad0cf38 100644 (file)
@@ -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;
 }
index 8b09326..8093412 100644 (file)
@@ -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());