Bug 1434936 - Use nsNSSCertList in NSSCertDBTrustDomain::IsChainValid r?keeler r?fkiefer
This change is to use the higher-level structure nsNSSCertList when checking
IsChainValid so that we can use the more powerful (and tested) methods of that
object instead of the ad-hoc iterators.
This will also permit the Symantec Distrust code in
Bug 1434300 to use these
methods, which keeps the code the same from the earlier
Bug 1409259.
MozReview-Commit-ID: B5KmDa1JLE
--- a/security/certverifier/NSSCertDBTrustDomain.cpp
+++ b/security/certverifier/NSSCertDBTrustDomain.cpp
@@ -780,68 +780,99 @@ NSSCertDBTrustDomain::IsChainValid(const
return Result::FATAL_ERROR_LIBRARY_FAILURE;
}
Result rv = CheckForStartComOrWoSign(certList);
if (rv != Success) {
return rv;
}
- CERTCertListNode* rootNode = CERT_LIST_TAIL(certList);
- if (!rootNode) {
+ // Modernization in-progress: Keep certList as a CERTCertList for storage into
+ // the mBuiltChain variable at the end, but let's use nsNSSCertList for the
+ // validity calculations.
+ UniqueCERTCertList certListCopy = nsNSSCertList::DupCertList(certList);
+
+ // This adopts the list
+ RefPtr<nsNSSCertList> nssCertList = new nsNSSCertList(Move(certListCopy));
+ if (!nssCertList) {
return Result::FATAL_ERROR_LIBRARY_FAILURE;
}
- CERTCertificate* root = rootNode->cert;
+
+ nsCOMPtr<nsIX509Cert> rootCert;
+ nsresult nsrv = nssCertList->GetRootCertificate(rootCert);
+ if (NS_FAILED(nsrv)) {
+ return Result::FATAL_ERROR_LIBRARY_FAILURE;
+ }
+ UniqueCERTCertificate root(rootCert->GetCert());
if (!root) {
return Result::FATAL_ERROR_LIBRARY_FAILURE;
}
bool isBuiltInRoot = false;
- rv = IsCertBuiltInRoot(root, isBuiltInRoot);
- if (rv != Success) {
- return rv;
+ nsrv = rootCert->GetIsBuiltInRoot(&isBuiltInRoot);
+ if (NS_FAILED(nsrv)) {
+ return Result::FATAL_ERROR_LIBRARY_FAILURE;
}
bool skipPinningChecksBecauseOfMITMMode =
(!isBuiltInRoot && mPinningMode == CertVerifier::pinningAllowUserCAMITM);
// If mHostname isn't set, we're not verifying in the context of a TLS
// handshake, so don't verify HPKP in those cases.
if (mHostname && (mPinningMode != CertVerifier::pinningDisabled) &&
!skipPinningChecksBecauseOfMITMMode) {
bool enforceTestMode =
(mPinningMode == CertVerifier::pinningEnforceTestMode);
bool chainHasValidPins;
- nsresult nsrv = PublicKeyPinningService::ChainHasValidPins(
+ nsrv = PublicKeyPinningService::ChainHasValidPins(
certList, mHostname, time, enforceTestMode, mOriginAttributes,
chainHasValidPins, mPinningTelemetryInfo);
if (NS_FAILED(nsrv)) {
return Result::FATAL_ERROR_LIBRARY_FAILURE;
}
if (!chainHasValidPins) {
return Result::ERROR_KEY_PINNING_FAILURE;
}
}
// See bug 1349762. If the root is "GlobalSign Root CA - R2", don't consider
// the end-entity valid for EV unless the
// "GlobalSign Extended Validation CA - SHA256 - G2" intermediate is in the
// chain as well. It should be possible to remove this workaround after
// January 2019 as per bug 1349727 comment 17.
if (requiredPolicy == sGlobalSignEVPolicy &&
- CertMatchesStaticData(root, sGlobalSignRootCAR2SubjectBytes,
+ CertMatchesStaticData(root.get(), sGlobalSignRootCAR2SubjectBytes,
sGlobalSignRootCAR2SPKIBytes)) {
+
+ rootCert = nullptr; // Clear the state for Segment...
+ nsCOMPtr<nsIX509CertList> intCerts;
+ nsCOMPtr<nsIX509Cert> eeCert;
+
+ nsrv = nssCertList->SegmentCertificateChain(rootCert, intCerts, eeCert);
+ if (NS_FAILED(nsrv)) {
+ // This chain is supposed to be complete, so this is an error. There
+ // are no intermediates, so return before searching just as if the
+ // search failed.
+ return Result::ERROR_POLICY_VALIDATION_FAILED;
+ }
+
bool foundRequiredIntermediate = false;
- for (CERTCertListNode* node = CERT_LIST_HEAD(certList);
- !CERT_LIST_END(node, certList); node = CERT_LIST_NEXT(node)) {
- if (CertMatchesStaticData(
- node->cert,
+ RefPtr<nsNSSCertList> intCertList = intCerts->GetCertList();
+ intCertList->ForEachCertificateInChain(
+ [&foundRequiredIntermediate] (nsCOMPtr<nsIX509Cert> aCert, bool aHasMore,
+ /* out */ bool& aContinue) {
+ // We need an owning handle when calling nsIX509Cert::GetCert().
+ UniqueCERTCertificate nssCert(aCert->GetCert());
+ if (CertMatchesStaticData(
+ nssCert.get(),
sGlobalSignExtendedValidationCASHA256G2SubjectBytes,
sGlobalSignExtendedValidationCASHA256G2SPKIBytes)) {
- foundRequiredIntermediate = true;
- break;
- }
- }
+ foundRequiredIntermediate = true;
+ aContinue = false;
+ }
+ return NS_OK;
+ });
+
if (!foundRequiredIntermediate) {
return Result::ERROR_POLICY_VALIDATION_FAILED;
}
}
mBuiltChain = Move(certList);
return Success;
--- a/security/manager/ssl/nsNSSCertificate.cpp
+++ b/security/manager/ssl/nsNSSCertificate.cpp
@@ -1263,17 +1263,17 @@ nsNSSCertList::SegmentCertificateChain(/
return NS_OK;
});
if (NS_FAILED(rv)) {
return rv;
}
if (!aRoot || !aEndEntity) {
- // No self-sigend (or empty) chains allowed
+ // No self-signed (or empty) chains allowed
return NS_ERROR_INVALID_ARG;
}
return NS_OK;
}
nsresult
nsNSSCertList::GetRootCertificate(/* out */ nsCOMPtr<nsIX509Cert>& aRoot)