Bug 1035942: Decide whether to consider end-entity CN as a dnsName in CheckNameConstraints instead of in BuildCertChain, r=cviecco

--HG--
extra : rebase_source : 19c5949253e4e631b0bd841f17f000885001b327
extra : histedit_source : dce57eb862a2a13d07d11fdf6917afcf6cb4136c
This commit is contained in:
Brian Smith 2014-07-08 13:04:17 -07:00
parent 3f110246be
commit 94e53dc0be
5 changed files with 26 additions and 43 deletions

View File

@ -33,7 +33,6 @@ namespace mozilla { namespace pkix {
static Result BuildForward(TrustDomain& trustDomain,
const BackCert& subject,
PRTime time,
EndEntityOrCA endEntityOrCA,
KeyUsage requiredKeyUsageIfPresent,
KeyPurposeId requiredEKUIfPresent,
const CertPolicyId& requiredPolicy,
@ -49,15 +48,13 @@ class PathBuildingStep : public TrustDomain::IssuerChecker
{
public:
PathBuildingStep(TrustDomain& trustDomain, const BackCert& subject,
PRTime time, EndEntityOrCA endEntityOrCA,
KeyPurposeId requiredEKUIfPresent,
PRTime time, KeyPurposeId requiredEKUIfPresent,
const CertPolicyId& requiredPolicy,
/*optional*/ const SECItem* stapledOCSPResponse,
unsigned int subCACount)
: trustDomain(trustDomain)
, subject(subject)
, time(time)
, endEntityOrCA(endEntityOrCA)
, requiredEKUIfPresent(requiredEKUIfPresent)
, requiredPolicy(requiredPolicy)
, stapledOCSPResponse(stapledOCSPResponse)
@ -76,7 +73,6 @@ private:
TrustDomain& trustDomain;
const BackCert& subject;
const PRTime time;
const EndEntityOrCA endEntityOrCA;
const KeyPurposeId requiredEKUIfPresent;
const CertPolicyId& requiredPolicy;
/*optional*/ SECItem const* const stapledOCSPResponse;
@ -136,8 +132,8 @@ SECStatus
PathBuildingStep::Check(const SECItem& potentialIssuerDER,
/*out*/ bool& keepGoing)
{
BackCert potentialIssuer(potentialIssuerDER, &subject,
BackCert::IncludeCN::No);
BackCert potentialIssuer(potentialIssuerDER, EndEntityOrCA::MustBeCA,
&subject);
Result rv = potentialIssuer.Init();
if (rv != Success) {
return RecordResult(PR_GetError(), keepGoing);
@ -161,7 +157,7 @@ PathBuildingStep::Check(const SECItem& potentialIssuerDER,
}
}
rv = CheckNameConstraints(potentialIssuer);
rv = CheckNameConstraints(potentialIssuer, requiredEKUIfPresent);
if (rv != Success) {
return RecordResult(PR_GetError(), keepGoing);
}
@ -169,9 +165,8 @@ PathBuildingStep::Check(const SECItem& potentialIssuerDER,
// RFC 5280, Section 4.2.1.3: "If the keyUsage extension is present, then the
// subject public key MUST NOT be used to verify signatures on certificates
// or CRLs unless the corresponding keyCertSign or cRLSign bit is set."
rv = BuildForward(trustDomain, potentialIssuer, time, EndEntityOrCA::MustBeCA,
KeyUsage::keyCertSign, requiredEKUIfPresent,
requiredPolicy, nullptr, subCACount);
rv = BuildForward(trustDomain, potentialIssuer, time, KeyUsage::keyCertSign,
requiredEKUIfPresent, requiredPolicy, nullptr, subCACount);
if (rv != Success) {
return RecordResult(PR_GetError(), keepGoing);
}
@ -185,7 +180,7 @@ PathBuildingStep::Check(const SECItem& potentialIssuerDER,
CertID certID(subject.GetIssuer(), potentialIssuer.GetSubjectPublicKeyInfo(),
subject.GetSerialNumber());
srv = trustDomain.CheckRevocation(endEntityOrCA, certID, time,
srv = trustDomain.CheckRevocation(subject.endEntityOrCA, certID, time,
stapledOCSPResponse,
subject.GetAuthorityInfoAccess());
if (srv != SECSuccess) {
@ -205,7 +200,6 @@ static Result
BuildForward(TrustDomain& trustDomain,
const BackCert& subject,
PRTime time,
EndEntityOrCA endEntityOrCA,
KeyUsage requiredKeyUsageIfPresent,
KeyPurposeId requiredEKUIfPresent,
const CertPolicyId& requiredPolicy,
@ -219,13 +213,12 @@ BuildForward(TrustDomain& trustDomain,
// any error found here until after attempting to find a valid chain.
// See the explanation of error prioritization in pkix.h.
rv = CheckIssuerIndependentProperties(trustDomain, subject, time,
endEntityOrCA,
requiredKeyUsageIfPresent,
requiredEKUIfPresent, requiredPolicy,
subCACount, &trustLevel);
PRErrorCode deferredEndEntityError = 0;
if (rv != Success) {
if (endEntityOrCA == EndEntityOrCA::MustBeEndEntity &&
if (subject.endEntityOrCA == EndEntityOrCA::MustBeEndEntity &&
trustLevel != TrustLevel::TrustAnchor) {
deferredEndEntityError = PR_GetError();
} else {
@ -255,7 +248,7 @@ BuildForward(TrustDomain& trustDomain,
return Success;
}
if (endEntityOrCA == EndEntityOrCA::MustBeCA) {
if (subject.endEntityOrCA == EndEntityOrCA::MustBeCA) {
// Avoid stack overflows and poor performance by limiting cert chain
// length.
static const unsigned int MAX_SUBCA_COUNT = 6;
@ -272,7 +265,7 @@ BuildForward(TrustDomain& trustDomain,
// Find a trusted issuer.
PathBuildingStep pathBuilder(trustDomain, subject, time, endEntityOrCA,
PathBuildingStep pathBuilder(trustDomain, subject, time,
requiredEKUIfPresent, requiredPolicy,
stapledOCSPResponse, subCACount);
@ -307,21 +300,15 @@ BuildCertChain(TrustDomain& trustDomain, const SECItem& certDER,
{
// XXX: Support the legacy use of the subject CN field for indicating the
// domain name the certificate is valid for.
BackCert::IncludeCN includeCN
= endEntityOrCA == EndEntityOrCA::MustBeEndEntity &&
requiredEKUIfPresent == KeyPurposeId::id_kp_serverAuth
? BackCert::IncludeCN::Yes
: BackCert::IncludeCN::No;
BackCert cert(certDER, nullptr, includeCN);
BackCert cert(certDER, endEntityOrCA, nullptr);
Result rv = cert.Init();
if (rv != Success) {
return SECFailure;
}
rv = BuildForward(trustDomain, cert, time, endEntityOrCA,
requiredKeyUsageIfPresent, requiredEKUIfPresent,
requiredPolicy, stapledOCSPResponse, 0);
rv = BuildForward(trustDomain, cert, time, requiredKeyUsageIfPresent,
requiredEKUIfPresent, requiredPolicy, stapledOCSPResponse,
0/*subCACount*/);
if (rv != Success) {
return SECFailure;
}

View File

@ -412,7 +412,7 @@ PORT_FreeArena_false(PLArenaPool* arena) {
}
Result
CheckNameConstraints(const BackCert& cert)
CheckNameConstraints(const BackCert& cert, KeyPurposeId requiredEKUIfPresent)
{
// These hardcoded consts are to handle a post certificate creation
// name constraints. In this case for ANSSI.
@ -510,7 +510,8 @@ CheckNameConstraints(const BackCert& cert)
return MapSECStatus(SECFailure);
}
bool includeCN = child->includeCN == BackCert::IncludeCN::Yes;
bool includeCN = child->endEntityOrCA == EndEntityOrCA::MustBeEndEntity &&
requiredEKUIfPresent == KeyPurposeId::id_kp_serverAuth;
// owned by arena
const CERTGeneralName*
names(CERT_GetConstrainedCertificateNames(nssCert.get(), arena.get(),
@ -697,7 +698,6 @@ Result
CheckIssuerIndependentProperties(TrustDomain& trustDomain,
const BackCert& cert,
PRTime time,
EndEntityOrCA endEntityOrCA,
KeyUsage requiredKeyUsageIfPresent,
KeyPurposeId requiredEKUIfPresent,
const CertPolicyId& requiredPolicy,
@ -706,6 +706,8 @@ CheckIssuerIndependentProperties(TrustDomain& trustDomain,
{
Result rv;
const EndEntityOrCA endEntityOrCA = cert.endEntityOrCA;
TrustLevel trustLevel;
rv = MapSECStatus(trustDomain.GetCertTrust(endEntityOrCA, requiredPolicy,
cert.GetDER(), &trustLevel));

View File

@ -35,14 +35,14 @@ Result CheckIssuerIndependentProperties(
TrustDomain& trustDomain,
const BackCert& cert,
PRTime time,
EndEntityOrCA endEntityOrCA,
KeyUsage requiredKeyUsageIfPresent,
KeyPurposeId requiredEKUIfPresent,
const CertPolicyId& requiredPolicy,
unsigned int subCACount,
/*optional out*/ TrustLevel* trustLevel = nullptr);
Result CheckNameConstraints(const BackCert& cert);
Result CheckNameConstraints(const BackCert& cert,
KeyPurposeId requiredEKUIfPresent);
} } // namespace mozilla::pkix

View File

@ -138,7 +138,6 @@ CheckOCSPResponseSignerCert(TrustDomain& trustDomain,
// TODO(bug 926261): If we're validating for a policy then the policy OID we
// are validating for should be passed to CheckIssuerIndependentProperties.
rv = CheckIssuerIndependentProperties(trustDomain, potentialSigner, time,
EndEntityOrCA::MustBeEndEntity,
KeyUsage::noParticularKeyUsageRequired,
KeyPurposeId::id_kp_OCSPSigning,
CertPolicyId::anyPolicy, 0);
@ -272,7 +271,7 @@ VerifySignature(Context& context, ResponderIDType responderIDType,
}
for (size_t i = 0; i < numCerts; ++i) {
BackCert cert(certs[i], nullptr, BackCert::IncludeCN::No);
BackCert cert(certs[i], EndEntityOrCA::MustBeEndEntity, nullptr);
rv = cert.Init();
if (rv != Success) {
return rv;

View File

@ -90,17 +90,12 @@ MapSECStatus(SECStatus srv)
class BackCert
{
public:
// IncludeCN::No means that name constraint enforcement should not consider
// the subject CN as a possible dNSName. IncludeCN::Yes means that name
// constraint enforcement will consider the subject CN as a possible dNSName.
MOZILLA_PKIX_ENUM_CLASS IncludeCN { No = 0, Yes = 1 };
// certDER and childCert must be valid for the lifetime of BackCert.
BackCert(const SECItem& certDER, const BackCert* childCert,
IncludeCN includeCN)
BackCert(const SECItem& certDER, EndEntityOrCA endEntityOrCA,
const BackCert* childCert)
: der(certDER)
, endEntityOrCA(endEntityOrCA)
, childCert(childCert)
, includeCN(includeCN)
{
}
@ -147,8 +142,8 @@ private:
const SECItem& der;
public:
const EndEntityOrCA endEntityOrCA;
BackCert const* const childCert;
const IncludeCN includeCN;
private:
der::Version version;