--- a/security/manager/ssl/SSLServerCertVerification.cpp
+++ b/security/manager/ssl/SSLServerCertVerification.cpp
@@ -1388,17 +1388,17 @@ AuthCertificate(CertVerifier& certVerifi
MOZ_ASSERT(cert);
// We want to avoid storing any intermediate cert information when browsing
// in private, transient contexts.
bool saveIntermediates =
!(providerFlags & nsISocketProvider::NO_PERMANENT_STORAGE);
SECOidTag evOidPolicy;
- UniqueCERTCertList certList;
+ UniqueCERTCertList builtCertChain;
CertVerifier::OCSPStaplingStatus ocspStaplingStatus =
CertVerifier::OCSP_STAPLING_NEVER_CHECKED;
KeySizeStatus keySizeStatus = KeySizeStatus::NeverChecked;
SHA1ModeResult sha1ModeResult = SHA1ModeResult::NeverChecked;
PinningTelemetryInfo pinningTelemetryInfo;
CertificateTransparencyInfo certificateTransparencyInfo;
int flags = 0;
@@ -1406,18 +1406,19 @@ AuthCertificate(CertVerifier& certVerifi
!infoObject->SharedState().IsOCSPMustStapleEnabled()) {
flags |= CertVerifier::FLAG_TLS_IGNORE_STATUS_REQUEST;
}
Result rv = certVerifier.VerifySSLServerCert(cert, stapledOCSPResponse,
sctsFromTLSExtension, time,
infoObject,
infoObject->GetHostName(),
- certList, saveIntermediates,
- flags, infoObject->
+ builtCertChain,
+ saveIntermediates, flags,
+ infoObject->
GetOriginAttributes(),
&evOidPolicy,
&ocspStaplingStatus,
&keySizeStatus, &sha1ModeResult,
&pinningTelemetryInfo,
&certificateTransparencyInfo);
uint32_t evStatus = (rv != Success) ? 0 // 0 = Failure
@@ -1448,18 +1449,18 @@ AuthCertificate(CertVerifier& certVerifi
}
if (rv == Success) {
// Certificate verification succeeded. Delete any potential record of
// certificate error bits.
RememberCertErrorsTable::GetInstance().RememberCertHasError(infoObject,
nullptr,
SECSuccess);
- GatherSuccessfulValidationTelemetry(certList);
- GatherCertificateTransparencyTelemetry(certList,
+ GatherSuccessfulValidationTelemetry(builtCertChain);
+ GatherCertificateTransparencyTelemetry(builtCertChain,
/*isEV*/ evOidPolicy != SEC_OID_UNKNOWN,
certificateTransparencyInfo);
// The connection may get terminated, for example, if the server requires
// a client cert. Let's provide a minimal SSLStatus
// to the caller that contains at least the cert and its status.
RefPtr<nsSSLStatus> status(infoObject->SSLStatus());
if (!status) {
@@ -1472,27 +1473,27 @@ AuthCertificate(CertVerifier& certVerifi
evStatus = EVStatus::NotEV;
} else {
evStatus = EVStatus::EV;
}
RefPtr<nsNSSCertificate> nsc = nsNSSCertificate::Create(cert.get());
status->SetServerCert(nsc, evStatus);
- status->SetSucceededCertChain(Move(certList));
+ status->SetSucceededCertChain(Move(builtCertChain));
MOZ_LOG(gPIPNSSLog, LogLevel::Debug,
- ("AuthCertificate setting NEW cert %p", nsc.get()));
+ ("AuthCertificate setting NEW cert %p", nsc.get()));
status->SetCertificateTransparencyInfo(certificateTransparencyInfo);
}
if (rv != Success) {
// Certificate validation failed; store the peer certificate chain on
// infoObject so it can be used for error reporting.
- infoObject->SetFailedCertChain(Move(certList));
+ infoObject->SetFailedCertChain(Move(peerCertChain));
PR_SetError(MapResultToPRErrorCode(rv), 0);
}
return rv == Success ? SECSuccess : SECFailure;
}
/*static*/ SECStatus
SSLServerCertVerificationJob::Dispatch(
@@ -1570,17 +1571,18 @@ SSLServerCertVerificationJob::Run()
// Reset the error code here so we can detect if AuthCertificate fails to
// set the error code if/when it fails.
PR_SetError(0, 0);
SECStatus rv = AuthCertificate(*mCertVerifier, mInfoObject, mCert,
mPeerCertChain, mStapledOCSPResponse.get(),
mSCTsFromTLSExtension.get(),
mProviderFlags, mTime);
- MOZ_ASSERT(mPeerCertChain || rv != SECSuccess,
+ MOZ_ASSERT((mPeerCertChain && rv == SECSuccess) ||
+ (!mPeerCertChain && rv != SECSuccess),
"AuthCertificate() should take ownership of chain on failure");
if (rv == SECSuccess) {
uint32_t interval = (uint32_t) ((TimeStamp::Now() - mJobStartTime).ToMilliseconds());
RefPtr<SSLServerCertVerificationResult> restart(
new SSLServerCertVerificationResult(mInfoObject, 0,
successTelemetry, interval));
restart->Dispatch();
Telemetry::Accumulate(Telemetry::SSL_CERT_ERROR_OVERRIDES, 1);
@@ -1750,17 +1752,18 @@ AuthCertificateHook(void* arg, PRFileDes
// We can't do certificate verification on a background thread, because the
// thread doing the network I/O may not interrupt its network I/O on receipt
// of our SSLServerCertVerificationResult event, and/or it might not even be
// a non-blocking socket.
SECStatus rv = AuthCertificate(*certVerifier, socketInfo, serverCert,
peerCertChain, stapledOCSPResponse,
sctsFromTLSExtension, providerFlags, now);
- MOZ_ASSERT(peerCertChain || rv != SECSuccess,
+ MOZ_ASSERT((peerCertChain && rv == SECSuccess) ||
+ (!peerCertChain && rv != SECSuccess),
"AuthCertificate() should take ownership of chain on failure");
if (rv == SECSuccess) {
Telemetry::Accumulate(Telemetry::SSL_CERT_ERROR_OVERRIDES, 1);
return SECSuccess;
}
PRErrorCode error = PR_GetError();
if (error != 0) {
--- a/security/manager/ssl/tests/unit/test_cert_chains.js
+++ b/security/manager/ssl/tests/unit/test_cert_chains.js
@@ -97,16 +97,34 @@ function run_test() {
" connection failure");
let originalCertChain = build_cert_chain(["expired-ee", "test-ca"]);
ok(originalCertChain.equals(securityInfo.failedCertChain),
"failedCertChain should equal the original cert chain for an" +
" overrideable connection failure");
}
);
+ // Test overrideable connection failure (failedCertChain should be non-null)
+ add_connection_test(
+ "unknownissuer.example.com",
+ SEC_ERROR_UNKNOWN_ISSUER,
+ null,
+ function withSecurityInfo(securityInfo) {
+ securityInfo.QueryInterface(Ci.nsITransportSecurityInfo);
+ test_security_info_serialization(securityInfo, SEC_ERROR_UNKNOWN_ISSUER);
+ notEqual(securityInfo.failedCertChain, null,
+ "failedCertChain should not be null for an overrideable" +
+ " connection failure");
+ let originalCertChain = build_cert_chain(["unknownissuer"]);
+ ok(originalCertChain.equals(securityInfo.failedCertChain),
+ "failedCertChain should equal the original cert chain for an" +
+ " overrideable connection failure");
+ }
+ );
+
// Test non-overrideable error (failedCertChain should be non-null)
add_connection_test(
"inadequatekeyusage.example.com",
SEC_ERROR_INADEQUATE_KEY_USAGE,
null,
function withSecurityInfo(securityInfo) {
securityInfo.QueryInterface(Ci.nsITransportSecurityInfo);
test_security_info_serialization(securityInfo, SEC_ERROR_INADEQUATE_KEY_USAGE);