bug 1437214 - if PathBuildingStep::Check fails due to a problem with the subject certificate rather than the potential issuer, set keepGoing to false r=jcj

MozReview-Commit-ID: DEr4YgXfkOL

--HG--
extra : rebase_source : daea8346adeb56cc34c0fb284dba2e571fd3621e
This commit is contained in:
David Keeler 2018-02-09 16:35:54 -08:00
parent 74167930bd
commit 12125be772
2 changed files with 174 additions and 2 deletions

View File

@ -241,7 +241,12 @@ PathBuildingStep::Check(Input potentialIssuerDER,
validityDuration, stapledOCSPResponse,
subject.GetAuthorityInfoAccess());
if (rv != Success) {
return RecordResult(rv, keepGoing);
// Since this is actually a problem with the current subject certificate
// (rather than the issuer), it doesn't make sense to keep going; all
// paths through this certificate will fail.
Result savedRv = RecordResult(rv, keepGoing);
keepGoing = false;
return savedRv;
}
if (subject.endEntityOrCA == EndEntityOrCA::MustBeEndEntity) {
@ -251,7 +256,11 @@ PathBuildingStep::Check(Input potentialIssuerDER,
rv = ExtractSignedCertificateTimestampListFromExtension(*sctExtension,
sctList);
if (rv != Success) {
return RecordResult(rv, keepGoing);
// Again, the problem is with this certificate, and all paths through
// it will fail.
Result savedRv = RecordResult(rv, keepGoing);
keepGoing = false;
return savedRv;
}
trustDomain.NoteAuxiliaryExtension(AuxiliaryExtension::EmbeddedSCTList,
sctList);

View File

@ -582,3 +582,166 @@ TEST_F(pkixbuild, CertificateTransparencyExtension)
ASSERT_EQ(BytesToByteString(dummySctList),
extTrustDomain.signedCertificateTimestamps);
}
// This TrustDomain implements a hierarchy like so:
//
// A B
// | |
// C D
// \ /
// E
//
// where A is a trust anchor, B is not a trust anchor and has no known issuer, C
// and D are intermediates with the same subject and subject public key, and E
// is an end-entity (in practice, the end-entity will be generated by the test
// functions using this trust domain).
class MultiplePathTrustDomain: public DefaultCryptoTrustDomain
{
public:
void SetUpCerts()
{
ASSERT_FALSE(ENCODING_FAILED(CreateCert("UntrustedRoot", "UntrustedRoot",
EndEntityOrCA::MustBeCA,
&subjectDERToCertDER)));
// The subject DER -> cert DER mapping would be overwritten for subject
// "Intermediate" when we create the second "Intermediate" certificate, so
// we keep a copy of this "Intermediate".
intermediateSignedByUntrustedRootCertDER =
CreateCert("UntrustedRoot", "Intermediate", EndEntityOrCA::MustBeCA);
ASSERT_FALSE(ENCODING_FAILED(intermediateSignedByUntrustedRootCertDER));
rootCACertDER = CreateCert("TrustedRoot", "TrustedRoot",
EndEntityOrCA::MustBeCA, &subjectDERToCertDER);
ASSERT_FALSE(ENCODING_FAILED(rootCACertDER));
ASSERT_FALSE(ENCODING_FAILED(CreateCert("TrustedRoot", "Intermediate",
EndEntityOrCA::MustBeCA,
&subjectDERToCertDER)));
}
private:
Result GetCertTrust(EndEntityOrCA, const CertPolicyId&, Input candidateCert,
/*out*/ TrustLevel& trustLevel) override
{
trustLevel = InputEqualsByteString(candidateCert, rootCACertDER)
? TrustLevel::TrustAnchor
: TrustLevel::InheritsTrust;
return Success;
}
Result CheckCert(ByteString& certDER, IssuerChecker& checker, bool& keepGoing)
{
Input derCert;
Result rv = derCert.Init(certDER.data(), certDER.length());
if (rv != Success) {
return rv;
}
return checker.Check(derCert, nullptr/*additionalNameConstraints*/,
keepGoing);
}
Result FindIssuer(Input encodedIssuerName, IssuerChecker& checker, Time)
override
{
ByteString subjectDER(InputToByteString(encodedIssuerName));
ByteString certDER(subjectDERToCertDER[subjectDER]);
assert(!ENCODING_FAILED(certDER));
bool keepGoing;
Result rv = CheckCert(certDER, checker, keepGoing);
if (rv != Success) {
return rv;
}
// Also try the other intermediate.
if (keepGoing) {
rv = CheckCert(intermediateSignedByUntrustedRootCertDER, checker,
keepGoing);
if (rv != Success) {
return rv;
}
}
return Success;
}
Result CheckRevocation(EndEntityOrCA, const CertID&, Time, Duration,
/*optional*/ const Input*,
/*optional*/ const Input*) override
{
return Success;
}
Result IsChainValid(const DERArray&, Time, const CertPolicyId&) override
{
return Success;
}
std::map<ByteString, ByteString> subjectDERToCertDER;
ByteString rootCACertDER;
ByteString intermediateSignedByUntrustedRootCertDER;
};
TEST_F(pkixbuild, BadEmbeddedSCTWithMultiplePaths)
{
MultiplePathTrustDomain trustDomain;
trustDomain.SetUpCerts();
// python security/pkix/tools/DottedOIDToCode.py --tlv
// id-embeddedSctList 1.3.6.1.4.1.11129.2.4.2
static const uint8_t tlv_id_embeddedSctList[] = {
0x06, 0x0a, 0x2b, 0x06, 0x01, 0x04, 0x01, 0xd6, 0x79, 0x02, 0x04, 0x02
};
static const uint8_t dummySctList[] = {
0x01, 0x02, 0x03, 0x04, 0x05
};
ByteString ctExtension = TLV(der::SEQUENCE,
BytesToByteString(tlv_id_embeddedSctList) +
Boolean(false) +
// The contents of the OCTET STRING are supposed to consist of an OCTET
// STRING of useful data. We're testing what happens if it isn't, so shove
// some bogus (non-OCTET STRING) data in there.
TLV(der::OCTET_STRING, BytesToByteString(dummySctList)));
ByteString certDER(CreateCert("Intermediate", "Cert with bogus SCT list",
EndEntityOrCA::MustBeEndEntity,
nullptr, /*subjectDERToCertDER*/
&ctExtension));
ASSERT_FALSE(ENCODING_FAILED(certDER));
Input certDERInput;
ASSERT_EQ(Success, certDERInput.Init(certDER.data(), certDER.length()));
ASSERT_EQ(Result::ERROR_BAD_DER,
BuildCertChain(trustDomain, certDERInput, Now(),
EndEntityOrCA::MustBeEndEntity,
KeyUsage::noParticularKeyUsageRequired,
KeyPurposeId::id_kp_serverAuth,
CertPolicyId::anyPolicy,
nullptr/*stapledOCSPResponse*/));
}
// Same as a MultiplePathTrustDomain, but the end-entity is revoked.
class RevokedEndEntityTrustDomain final : public MultiplePathTrustDomain
{
public:
Result CheckRevocation(EndEntityOrCA endEntityOrCA, const CertID&, Time,
Duration, /*optional*/ const Input*,
/*optional*/ const Input*) override
{
if (endEntityOrCA == EndEntityOrCA::MustBeEndEntity) {
return Result::ERROR_REVOKED_CERTIFICATE;
}
return Success;
}
};
TEST_F(pkixbuild, RevokedEndEntityWithMultiplePaths)
{
RevokedEndEntityTrustDomain trustDomain;
trustDomain.SetUpCerts();
ByteString certDER(CreateCert("Intermediate", "RevokedEndEntity",
EndEntityOrCA::MustBeEndEntity));
ASSERT_FALSE(ENCODING_FAILED(certDER));
Input certDERInput;
ASSERT_EQ(Success, certDERInput.Init(certDER.data(), certDER.length()));
ASSERT_EQ(Result::ERROR_REVOKED_CERTIFICATE,
BuildCertChain(trustDomain, certDERInput, Now(),
EndEntityOrCA::MustBeEndEntity,
KeyUsage::noParticularKeyUsageRequired,
KeyPurposeId::id_kp_serverAuth,
CertPolicyId::anyPolicy,
nullptr/*stapledOCSPResponse*/));
}