wine/gecko
0
0
Fork 0
mirror of https://gitlab.winehq.org/wine/wine-gecko.git synced 2024-09-13 09:24:08 -07:00
Code Issues Packages Projects Releases Wiki Activity

Bug 962833: ensure-certverify-returns secfailure on MUST_BE_EV and no ev certificate. r=dkeeler

Browse Source
This commit is contained in:
Camilo Viecco 2014-01-24 13:57:35 -08:00
parent 2ca93fc51b
commit ad891a09df
6 changed files with 75 additions and 18 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

33
security/certverifier/CertVerifier.cpp
View File

@ -29,7 +29,7 @@ static PRLogModuleInfo* gCertVerifierLog = nullptr;
namespace mozilla { namespace psm {
const CertVerifier::Flags CertVerifier::FLAG_LOCAL_ONLY = 1;
const CertVerifier::Flags CertVerifier::FLAG_NO_DV_FALLBACK_FOR_EV = 2;
const CertVerifier::Flags CertVerifier::FLAG_MUST_BE_EV = 2;
CertVerifier::CertVerifier(implementation_config ic,
missing_cert_download_config mcdc,
@ -159,9 +159,7 @@ CertVerifier::VerifyCert(CERTCertificate* cert,
/*optional out*/ SECOidTag* evOidPolicy,
/*optional out*/ CERTVerifyLog* verifyLog)
{
if (!cert ||
((flags & FLAG_NO_DV_FALLBACK_FOR_EV) &&
(usage != certificateUsageSSLServer || !evOidPolicy)))
if (!cert)
{
PR_NOT_REACHED("Invalid arguments to CertVerifier::VerifyCert");
PORT_SetError(SEC_ERROR_INVALID_ARGS);
@ -188,6 +186,11 @@ CertVerifier::VerifyCert(CERTCertificate* cert,
return SECFailure;
}
if ((flags & FLAG_MUST_BE_EV) && usage != certificateUsageSSLServer) {
PORT_SetError(SEC_ERROR_INVALID_ARGS);
return SECFailure;
}
#ifndef NSS_NO_LIBPKIX
ScopedCERTCertList trustAnchors;
SECStatus rv;
@ -210,9 +213,18 @@ CertVerifier::VerifyCert(CERTCertificate* cert,
evPolicy = SEC_OID_UNKNOWN;
}
} else {
// No known EV policy found
if (flags & FLAG_MUST_BE_EV) {
PORT_SetError(SEC_ERROR_EXTENSION_NOT_FOUND);
return SECFailure;
}
// Do not setup EV verification params
evPolicy = SEC_OID_UNKNOWN;
}
if ((evPolicy == SEC_OID_UNKNOWN) && (flags & FLAG_MUST_BE_EV)) {
PORT_SetError(SEC_ERROR_UNKNOWN_ISSUER);
return SECFailure;
}
}
PR_ASSERT(evPolicy == SEC_OID_UNKNOWN || trustAnchors);
@ -322,7 +334,9 @@ CertVerifier::VerifyCert(CERTCertificate* cert,
}
PR_LOG(gCertVerifierLog, PR_LOG_DEBUG,
("VerifyCert: failed CERT_PKIXVerifyCert(ev)\n"));
if (flags & FLAG_MUST_BE_EV) {
return rv;
}
if (validationChain) {
destroyCertListThatShouldNotExist(
&cvout[validationChainLocation].value.pointer.chain);
@ -348,9 +362,14 @@ CertVerifier::VerifyCert(CERTCertificate* cert,
// If we're here, PKIX EV verification failed.
// If requested, don't do DV fallback.
if (flags & FLAG_NO_DV_FALLBACK_FOR_EV) {
if (flags & FLAG_MUST_BE_EV) {
PR_ASSERT(*evOidPolicy == SEC_OID_UNKNOWN);
return SECSuccess;
#ifdef NSS_NO_LIBPKIX
PR_SetError(SEC_ERROR_INVALID_ARGS, 0);
#else
PR_SetError(PR_INVALID_STATE_ERROR, 0);
#endif
return SECFailure;
}
if (mImplementation == classic) {

2
security/certverifier/CertVerifier.h
View File

@ -19,7 +19,7 @@ public:
// XXX: FLAG_LOCAL_ONLY is ignored in the classic verification case
static const Flags FLAG_LOCAL_ONLY;
// Don't perform fallback DV validation on EV validation failure.
static const Flags FLAG_NO_DV_FALLBACK_FOR_EV;
static const Flags FLAG_MUST_BE_EV;
// *evOidPolicy == SEC_OID_UNKNOWN means the cert is NOT EV
// Only one usage per verification is supported.

4
security/manager/ssl/public/nsIX509CertDB.idl
View File

@ -31,7 +31,7 @@ interface nsIOpenSignedJARFileCallback : nsISupports
* This represents a service to access and manipulate
* X.509 certificates stored in a database.
*/
[scriptable, uuid(6502291d-4477-43a3-9688-4b453d055c1d)]
[scriptable, uuid(38463592-8527-11e3-b240-180373d97f23)]
interface nsIX509CertDB : nsISupports {
/**
@ -306,7 +306,7 @@ interface nsIX509CertDB : nsISupports {
// Do not fall back to DV verification after attempting EV validation.
// Actually does prevent network traffic, but can cause a valid EV
// certificate to not be considered valid.
const uint32_t FLAG_NO_DV_FALLBACK_FOR_EV = 1 << 1;
const uint32_t FLAG_MUST_BE_EV = 1 << 1;
/** Warning: This interface is inteded to use only for testing only as:
* 1. It can create IO on the main thread.

2
security/manager/ssl/src/nsNSSCertificate.cpp
View File

@ -1441,7 +1441,7 @@ nsNSSCertificate::hasValidEVOidTag(SECOidTag& resultOidTag, bool& validEV)
resultOidTag = SEC_OID_UNKNOWN;
uint32_t flags = mozilla::psm::CertVerifier::FLAG_LOCAL_ONLY |
mozilla::psm::CertVerifier::FLAG_NO_DV_FALLBACK_FOR_EV;
mozilla::psm::CertVerifier::FLAG_MUST_BE_EV;
SECStatus rv = certVerifier->VerifyCert(mCert.get(), nullptr,
certificateUsageSSLServer, PR_Now(),
nullptr /* XXX pinarg */,

1
security/manager/ssl/tests/unit/head_psm.js
View File

@ -24,6 +24,7 @@ const SEC_ERROR_BASE = Ci.nsINSSErrorsService.NSS_SEC_ERROR_BASE;
const SEC_ERROR_REVOKED_CERTIFICATE = SEC_ERROR_BASE + 12;
const SEC_ERROR_BAD_DATABASE = SEC_ERROR_BASE + 18;
const SEC_ERROR_UNTRUSTED_ISSUER = SEC_ERROR_BASE + 20;
const SEC_ERROR_EXTENSION_NOT_FOUND = SEC_ERROR_BASE + 35;
const SEC_ERROR_OCSP_MALFORMED_REQUEST = SEC_ERROR_BASE + 120;
const SEC_ERROR_OCSP_SERVER_ERROR = SEC_ERROR_BASE + 121;
const SEC_ERROR_OCSP_TRY_SERVER_LATER = SEC_ERROR_BASE + 122;

51
security/manager/ssl/tests/unit/test_ev_certs.js
View File

@ -173,38 +173,75 @@ add_test(function() {
ocspResponder.stop(run_next_test);
});
// bug 950240: add FLAG_NO_DV_FALLBACK_FOR_EV to CertVerifier::VerifyCert
// bug 950240: add FLAG_MUST_BE_EV to CertVerifier::VerifyCert
// to prevent spurious OCSP requests that race with OCSP stapling.
// This has the side-effect of saying an EV certificate is not EV if
// it hasn't already been verified (e.g. on the verification thread when
// connecting to a site).
function check_no_ocsp_requests(cert_name) {
// This flag is mostly a hack that should be removed once FLAG_LOCAL_ONLY
// works as intended.
function check_no_ocsp_requests(cert_name, expected_error) {
clearOCSPCache();
let ocspResponder = failingOCSPResponder();
let cert = certdb.findCertByNickname(null, cert_name);
let hasEVPolicy = {};
let verifiedChain = {};
let flags = Ci.nsIX509CertDB.FLAG_LOCAL_ONLY |
Ci.nsIX509CertDB.FLAG_NO_DV_FALLBACK_FOR_EV;
Ci.nsIX509CertDB.FLAG_MUST_BE_EV;
let error = certdb.verifyCertNow(cert, certificateUsageSSLServer, flags,
verifiedChain, hasEVPolicy);
// Since we're not doing OCSP requests, no certificate will be EV.
do_check_eq(hasEVPolicy.value, false);
do_check_eq(0, error);
do_check_eq(expected_error, error);
// Also check that isExtendedValidation doesn't cause OCSP requests.
let identityInfo = cert.QueryInterface(Ci.nsIIdentityInfo);
do_check_eq(identityInfo.isExtendedValidation, false);
ocspResponder.stop(run_next_test);
}
add_test(function() {
check_no_ocsp_requests("ev-valid");
check_no_ocsp_requests("ev-valid", isDebugBuild ?
SEC_ERROR_REVOKED_CERTIFICATE:
SEC_ERROR_EXTENSION_NOT_FOUND);
});
add_test(function() {
check_no_ocsp_requests("non-ev-root");
check_no_ocsp_requests("non-ev-root", isDebugBuild ?
SEC_ERROR_UNTRUSTED_ISSUER:
SEC_ERROR_EXTENSION_NOT_FOUND);
});
add_test(function() {
check_no_ocsp_requests("no-ocsp-url-cert");
check_no_ocsp_requests("no-ocsp-url-cert", isDebugBuild?
SEC_ERROR_REVOKED_CERTIFICATE:
SEC_ERROR_EXTENSION_NOT_FOUND);
});
// Test the EV continues to work with flags after successful EV verification
add_test(function() {
clearOCSPCache();
let ocspResponder = start_ocsp_responder(
isDebugBuild ? ["int-ev-valid", "ev-valid"]
: ["ev-valid"]);
check_ee_for_ev("ev-valid", isDebugBuild);
ocspResponder.stop(run_next_test);
// without net it must be able to EV verify
let failingOcspResponder = failingOCSPResponder();
let cert = certdb.findCertByNickname(null, "ev-valid");
let hasEVPolicy = {};
let verifiedChain = {};
let flags = Ci.nsIX509CertDB.FLAG_LOCAL_ONLY |
Ci.nsIX509CertDB.FLAG_MUST_BE_EV;
let error = certdb.verifyCertNow(cert, certificateUsageSSLServer,
flags, verifiedChain, hasEVPolicy);
do_check_eq(hasEVPolicy.value, isDebugBuild);
do_check_eq(error, isDebugBuild ? 0 : SEC_ERROR_EXTENSION_NOT_FOUND);
failingOcspResponder.stop(run_next_test);
});