From 181f3b3cb5e5803fd94eae3c8f63f5f50d616e77 Mon Sep 17 00:00:00 2001 From: Camilo Viecco Date: Mon, 15 Apr 2013 16:44:38 -0700 Subject: [PATCH] Bug 813418 - Backout 7bcdee03b55 due to bustage CLOSED TREE --- security/manager/ssl/src/CertVerifier.cpp | 14 +-- .../ssl/src/SSLServerCertVerification.cpp | 82 ++++++------ .../manager/ssl/src/nsIdentityChecking.cpp | 4 +- .../manager/ssl/src/nsNSSCertificateDB.cpp | 119 ++++++++++-------- security/manager/ssl/src/nsNSSComponent.cpp | 6 +- 5 files changed, 119 insertions(+), 106 deletions(-) diff --git a/security/manager/ssl/src/CertVerifier.cpp b/security/manager/ssl/src/CertVerifier.cpp index 355f85cdf7a8..6b021a8e01d7 100644 --- a/security/manager/ssl/src/CertVerifier.cpp +++ b/security/manager/ssl/src/CertVerifier.cpp @@ -217,12 +217,12 @@ CertVerifier::VerifyCert(CERTCertificate * cert, if (evPolicy != SEC_OID_UNKNOWN) { // EV setup! - // XXX 859872 The current flags are not quite correct. (use - // of ocsp flags for crl preferences). + // This flags are not quite correct, but it is what we have now, so keeping + // them identical for bug landing purposes. Should be fixed later! uint64_t revMethodFlags = CERT_REV_M_TEST_USING_THIS_METHOD - | ((mOCSPDownloadEnabled && !localOnly) ? - CERT_REV_M_ALLOW_NETWORK_FETCHING : CERT_REV_M_FORBID_NETWORK_FETCHING) + | (mOCSPDownloadEnabled ? CERT_REV_M_ALLOW_NETWORK_FETCHING + : CERT_REV_M_FORBID_NETWORK_FETCHING) | CERT_REV_M_ALLOW_IMPLICIT_DEFAULT_SOURCE | CERT_REV_M_REQUIRE_INFO_ON_MISSING_SOURCE | CERT_REV_M_IGNORE_MISSING_FRESH_INFO @@ -371,7 +371,6 @@ pkix_done: if (rv == SECSuccess) { if (! cvout[validationChainLocation].value.pointer.chain) { - PR_SetError(PR_UNKNOWN_ERROR, 0); return SECFailure; } PR_LOG(gPIPNSSLog, PR_LOG_DEBUG, ("VerifyCert: I have a chain\n")); @@ -383,8 +382,8 @@ pkix_done: if (!CERT_CompareCerts(trustAnchor, cert)) { PR_LOG(gPIPNSSLog, PR_LOG_DEBUG, ("VerifyCert: adding issuer to tail for display\n")); // note: rv is reused to catch errors on cert creation! - ScopedCERTCertificate tempCert(CERT_DupCertificate(trustAnchor)); - rv = CERT_AddCertToListTail(*validationChain, tempCert); + rv = CERT_AddCertToListTail(*validationChain, + CERT_DupCertificate(trustAnchor)); if (rv != SECSuccess) { CERT_DestroyCertList(*validationChain); *validationChain = nullptr; @@ -398,7 +397,6 @@ pkix_done: } } } - return rv; } diff --git a/security/manager/ssl/src/SSLServerCertVerification.cpp b/security/manager/ssl/src/SSLServerCertVerification.cpp index a212c2b4954f..773189a2305a 100644 --- a/security/manager/ssl/src/SSLServerCertVerification.cpp +++ b/security/manager/ssl/src/SSLServerCertVerification.cpp @@ -478,14 +478,16 @@ CreateCertErrorRunnable(PRErrorCode defaultErrorCodeToReport, PRTime now = PR_Now(); - PLArenaPool *log_arena = PORT_NewArena(DER_DEFAULT_CHUNKSIZE); + PLArenaPool *log_arena = nullptr; PLArenaPoolCleanerFalseParam log_arena_cleaner(log_arena); + CERTVerifyLog * verify_log = nullptr; + + log_arena = PORT_NewArena(DER_DEFAULT_CHUNKSIZE); if (!log_arena) { NS_ERROR("PORT_NewArena failed"); return nullptr; // PORT_NewArena set error code } - - CERTVerifyLog * verify_log = PORT_ArenaZNew(log_arena, CERTVerifyLog); + verify_log = PORT_ArenaZNew(log_arena, CERTVerifyLog); if (!verify_log) { NS_ERROR("PORT_ArenaZNew failed"); return nullptr; // PORT_ArenaZNew set error code @@ -494,7 +496,7 @@ CreateCertErrorRunnable(PRErrorCode defaultErrorCodeToReport, verify_log->arena = log_arena; srv = certVerifier->VerifyCert(cert, certificateUsageSSLServer, now, - infoObject, 0, nullptr, nullptr, verify_log); + infoObject, 0, nullptr, nullptr, verify_log); // We ignore the result code of the cert verification. // Either it is a failure, which is expected, and we'll process the @@ -519,40 +521,44 @@ CreateCertErrorRunnable(PRErrorCode defaultErrorCodeToReport, errorCodeMismatch = SSL_ERROR_BAD_CERT_DOMAIN; } - CERTVerifyLogNode *i_node; - for (i_node = verify_log->head; i_node; i_node = i_node->next) - { - switch (i_node->error) + if (verify_log) { + CERTVerifyLogNode *i_node; + for (i_node = verify_log->head; i_node; i_node = i_node->next) { - case SEC_ERROR_UNKNOWN_ISSUER: - case SEC_ERROR_CA_CERT_INVALID: - case SEC_ERROR_UNTRUSTED_ISSUER: - case SEC_ERROR_EXPIRED_ISSUER_CERTIFICATE: - case SEC_ERROR_UNTRUSTED_CERT: - case SEC_ERROR_INADEQUATE_KEY_USAGE: - case SEC_ERROR_CERT_SIGNATURE_ALGORITHM_DISABLED: - // We group all these errors as "cert not trusted" - collected_errors |= nsICertOverrideService::ERROR_UNTRUSTED; - if (errorCodeTrust == SECSuccess) { - errorCodeTrust = i_node->error; - } - break; - case SSL_ERROR_BAD_CERT_DOMAIN: - collected_errors |= nsICertOverrideService::ERROR_MISMATCH; - if (errorCodeMismatch == SECSuccess) { - errorCodeMismatch = i_node->error; - } - break; - case SEC_ERROR_EXPIRED_CERTIFICATE: - collected_errors |= nsICertOverrideService::ERROR_TIME; - if (errorCodeExpired == SECSuccess) { - errorCodeExpired = i_node->error; - } - break; - default: - PR_SetError(i_node->error, 0); - return nullptr; + switch (i_node->error) + { + case SEC_ERROR_UNKNOWN_ISSUER: + case SEC_ERROR_CA_CERT_INVALID: + case SEC_ERROR_UNTRUSTED_ISSUER: + case SEC_ERROR_EXPIRED_ISSUER_CERTIFICATE: + case SEC_ERROR_UNTRUSTED_CERT: + case SEC_ERROR_INADEQUATE_KEY_USAGE: + case SEC_ERROR_CERT_SIGNATURE_ALGORITHM_DISABLED: + // We group all these errors as "cert not trusted" + collected_errors |= nsICertOverrideService::ERROR_UNTRUSTED; + if (errorCodeTrust == SECSuccess) { + errorCodeTrust = i_node->error; + } + break; + case SSL_ERROR_BAD_CERT_DOMAIN: + collected_errors |= nsICertOverrideService::ERROR_MISMATCH; + if (errorCodeMismatch == SECSuccess) { + errorCodeMismatch = i_node->error; + } + break; + case SEC_ERROR_EXPIRED_CERTIFICATE: + collected_errors |= nsICertOverrideService::ERROR_TIME; + if (errorCodeExpired == SECSuccess) { + errorCodeExpired = i_node->error; + } + break; + default: + PR_SetError(i_node->error, 0); + return nullptr; + } } + } else { + // XXX set errorCodeTrust, errorCodeMismatch, errorCodeExpired, collected_errors } if (!collected_errors) @@ -890,8 +896,8 @@ AuthCertificate(TransportSecurityInfo * infoObject, CERTCertificate * cert, } } - ScopedCERTCertList certList(verifyCertChain); - + ScopedCERTCertList certList(CERT_GetCertChainFromCert(cert, PR_Now(), + certUsageSSLCA)); if (!certList) { rv = SECFailure; } else { diff --git a/security/manager/ssl/src/nsIdentityChecking.cpp b/security/manager/ssl/src/nsIdentityChecking.cpp index 52fc227df540..80e4e5797344 100644 --- a/security/manager/ssl/src/nsIdentityChecking.cpp +++ b/security/manager/ssl/src/nsIdentityChecking.cpp @@ -1038,7 +1038,7 @@ getRootsForOid(SECOidTag oid_tag) return certList; } -} } // namespace mozilla::psm +}} static bool isApprovedForEV(SECOidTag policyOIDTag, CERTCertificate *rootCert) @@ -1181,7 +1181,7 @@ SECStatus getFirstEVPolicy(CERTCertificate *cert, SECOidTag &outOidTag) return SECFailure; } -} } // namespace mozilla::psm +}} NS_IMETHODIMP nsSSLStatus::GetIsExtendedValidation(bool* aIsEV) diff --git a/security/manager/ssl/src/nsNSSCertificateDB.cpp b/security/manager/ssl/src/nsNSSCertificateDB.cpp index 9debd7ed20ee..ade4fdb62a8a 100644 --- a/security/manager/ssl/src/nsNSSCertificateDB.cpp +++ b/security/manager/ssl/src/nsNSSCertificateDB.cpp @@ -32,6 +32,7 @@ #include "nsComponentManagerUtils.h" #include "nsIPrompt.h" #include "nsThreadUtils.h" +#include "ScopedNSSTypes.h" #include "nsIObserverService.h" #include "nsRecentBadCerts.h" #include "SharedSSLState.h" @@ -479,42 +480,6 @@ nsNSSCertificateDB::ImportCertificates(uint8_t * data, uint32_t length, return nsrv; } -static -SECStatus -ImportCertsIntoPermanentStorage(const ScopedCERTCertList &certChain, const SECCertUsage usage, - const PRBool caOnly) -{ - CERTCertDBHandle *certdb = CERT_GetDefaultCertDB(); - const PRTime now = PR_Now(); - - int chainLen=0; - - for (CERTCertListNode *chainNode = CERT_LIST_HEAD(certChain); - !CERT_LIST_END(chainNode, certChain); - chainNode = CERT_LIST_NEXT(chainNode)) { - chainLen++; - } - SECItem **rawArray; - rawArray = (SECItem **) PORT_Alloc(chainLen * sizeof(SECItem *)); - if (!rawArray) { - return SECFailure; - } - - int i=0; - for (CERTCertListNode *chainNode = CERT_LIST_HEAD(certChain); - !CERT_LIST_END(chainNode, certChain); - chainNode = CERT_LIST_NEXT(chainNode), i++) { - rawArray[i] = &chainNode->cert->derCert; - } - CERT_ImportCerts(certdb, usage, chainLen, - rawArray, nullptr, true, caOnly, nullptr); - - PORT_Free(rawArray); - - - return SECSuccess; -} - /* * [noscript] void importEmailCertificates(in charPtr data, in unsigned long length, @@ -597,29 +562,53 @@ nsNSSCertificateDB::ImportEmailCertificate(uint8_t * data, uint32_t length, !CERT_LIST_END(node,certList); node = CERT_LIST_NEXT(node)) { + bool alert_and_skip = false; + if (!node->cert) { continue; } - CERTCertList *verifyCertChain = nullptr; - SECStatus rv = certVerifier->VerifyCert(node->cert, certificateUsageEmailRecipient, - now, ctx, 0, &verifyCertChain); - - ScopedCERTCertList certChain(verifyCertChain); - + now, ctx); if (rv != SECSuccess) { + alert_and_skip = true; + } + + ScopedCERTCertificateList certChain; + + if (!alert_and_skip) { + certChain = CERT_CertChainFromCert(node->cert, certUsageEmailRecipient, + false); + if (!certChain) { + alert_and_skip = true; + } + } + + if (alert_and_skip) { nsCOMPtr certToShow = nsNSSCertificate::Create(node->cert); DisplayCertificateAlert(ctx, "NotImportingUnverifiedCert", certToShow); continue; } - rv = ImportCertsIntoPermanentStorage(certChain, certUsageEmailRecipient, false); - if (rv != SECSuccess) { - goto loser; - } + + /* + * CertChain returns an array of SECItems, import expects an array of + * SECItem pointers. Create the SECItem Pointers from the array of + * SECItems. + */ + rawArray = (SECItem **) PORT_Alloc(certChain->len * sizeof(SECItem *)); + if (!rawArray) { + continue; + } + for (i=0; i < certChain->len; i++) { + rawArray[i] = &certChain->certs[i]; + } + CERT_ImportCerts(certdb, certUsageEmailRecipient, certChain->len, + rawArray, nullptr, true, false, nullptr); + CERT_SaveSMimeProfile(node->cert, nullptr, nullptr); + PORT_Free(rawArray); } loser: @@ -768,21 +757,45 @@ nsNSSCertificateDB::ImportValidCACertsInList(CERTCertList *certList, nsIInterfac !CERT_LIST_END(node,certList); node = CERT_LIST_NEXT(node)) { - //bool alert_and_skip = false; - CERTCertList *verifyCertChain = nullptr; + bool alert_and_skip = false; SECStatus rv = certVerifier->VerifyCert(node->cert, certificateUsageVerifyCA, - PR_Now(), ctx, 0, &verifyCertChain); - - ScopedCERTCertList certChain(verifyCertChain); - + PR_Now(), ctx); if (rv != SECSuccess) { + alert_and_skip = true; + } + + ScopedCERTCertificateList certChain; + + if (!alert_and_skip) { + certChain = CERT_CertChainFromCert(node->cert, certUsageAnyCA, false); + if (!certChain) { + alert_and_skip = true; + } + } + + if (alert_and_skip) { nsCOMPtr certToShow = nsNSSCertificate::Create(node->cert); DisplayCertificateAlert(ctx, "NotImportingUnverifiedCert", certToShow); continue; } - ImportCertsIntoPermanentStorage(certChain, certUsageAnyCA, true); + /* + * CertChain returns an array of SECItems, import expects an array of + * SECItem pointers. Create the SECItem Pointers from the array of + * SECItems. + */ + rawArray = (SECItem **) PORT_Alloc(certChain->len * sizeof(SECItem *)); + if (!rawArray) { + continue; + } + for (int i=0; i < certChain->len; i++) { + rawArray[i] = &certChain->certs[i]; + } + CERT_ImportCerts(CERT_GetDefaultCertDB(), certUsageAnyCA, certChain->len, + rawArray, nullptr, true, true, nullptr); + + PORT_Free(rawArray); } return NS_OK; diff --git a/security/manager/ssl/src/nsNSSComponent.cpp b/security/manager/ssl/src/nsNSSComponent.cpp index 247a36be2cfe..57f8ac0528f4 100644 --- a/security/manager/ssl/src/nsNSSComponent.cpp +++ b/security/manager/ssl/src/nsNSSComponent.cpp @@ -2048,13 +2048,9 @@ nsNSSComponent::VerifySignature(const char* aRSABuf, uint32_t aRSABufLen, GetDecryptKeyCallback, nullptr, DecryptionAllowedCallback); - if (!p7_info) { - return NS_ERROR_FAILURE; - } - // Make sure we call SEC_PKCS7DestroyContentInfo after this point; // otherwise we leak data in p7_info - + //-- If a plaintext was provided, hash it. SECItem digest; digest.data = nullptr;