Magic-Mirror/gecko-dev
1
0
Fork 0
mirror of https://github.com/mozilla/gecko-dev.git synced 2024-10-19 00:05:36 +00:00
Code Issues Actions Packages Projects Releases Wiki Activity

bug 1434831 - ensure nsITransportSecurityInfo.failedCertChain contains the *peer cert chain* r=jcj

Browse Source 
In bug 1406856 the failedCertChain property of nsITransportSecurityInfo was
changed to hold the built certificate chain out parameter from the call to
CertVerifier::VerifySSLServerCert. However, this was incorrect for two reasons:
a) failedCertChain is supposed to be the peer cert chain delivered by the server
in the TLS handshake and
b) if VerifySSLServerCert returns a failing result, the out parameter is not
guaranteed to hold any meaningful information, and must not be used.
This patch sets failedCertChain to the appropriate value.

MozReview-Commit-ID: BEXs5XH9SpK

--HG--
extra : rebase_source : f50ea725ccb67408ab1ce33cd76d3956ebd10e29
This commit is contained in:
David Keeler 2018-02-01 12:29:04 -08:00
parent 96f93d2480
commit c470850884
2 changed files with 31 additions and 10 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

23
security/manager/ssl/SSLServerCertVerification.cpp
View File

@ -1393,7 +1393,7 @@ AuthCertificate(CertVerifier& certVerifier,
!(providerFlags & nsISocketProvider::NO_PERMANENT_STORAGE);
SECOidTag evOidPolicy;
UniqueCERTCertList certList;
UniqueCERTCertList builtCertChain;
CertVerifier::OCSPStaplingStatus ocspStaplingStatus =
CertVerifier::OCSP_STAPLING_NEVER_CHECKED;
KeySizeStatus keySizeStatus = KeySizeStatus::NeverChecked;
@ -1411,8 +1411,9 @@ AuthCertificate(CertVerifier& certVerifier,
sctsFromTLSExtension, time,
infoObject,
infoObject->GetHostName(),
certList, saveIntermediates,
flags, infoObject->
builtCertChain,
saveIntermediates, flags,
infoObject->
GetOriginAttributes(),
&evOidPolicy,
&ocspStaplingStatus,
@ -1453,8 +1454,8 @@ AuthCertificate(CertVerifier& certVerifier,
RememberCertErrorsTable::GetInstance().RememberCertHasError(infoObject,
nullptr,
SECSuccess);
GatherSuccessfulValidationTelemetry(certList);
GatherCertificateTransparencyTelemetry(certList,
GatherSuccessfulValidationTelemetry(builtCertChain);
GatherCertificateTransparencyTelemetry(builtCertChain,
/*isEV*/ evOidPolicy != SEC_OID_UNKNOWN,
certificateTransparencyInfo);
@ -1477,9 +1478,9 @@ AuthCertificate(CertVerifier& certVerifier,
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);
}
@ -1487,7 +1488,7 @@ AuthCertificate(CertVerifier& certVerifier,
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);
}
@ -1575,7 +1576,8 @@ SSLServerCertVerificationJob::Run()
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());
@ -1755,7 +1757,8 @@ AuthCertificateHook(void* arg, PRFileDesc* fd, PRBool checkSig, PRBool isServer)
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);

18
security/manager/ssl/tests/unit/test_cert_chains.js
View File

@ -102,6 +102,24 @@ function run_test() {
}
);
// 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",