From 2b87715a3057f7740b0168f1e6253342bd1db89d Mon Sep 17 00:00:00 2001 From: "sangwan.kwon" Date: Wed, 20 Apr 2016 15:08:23 +0900 Subject: 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 --- tests/vcore/test-signature-validator.cpp | 12 +++---- vcore/vcore/Certificate.cpp | 52 ++++++++++++++-------------- vcore/vcore/CertificateCollection.cpp | 3 +- vcore/vcore/SignatureValidator.cpp | 59 ++++++++++++++++++++------------ vcore/vcore/TimeConversion.cpp | 9 ++++- vcore/vcore/XmlsecAdapter.cpp | 1 + 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(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(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 +#include #include #include @@ -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()); -- cgit v1.2.3