From 9e344e02562ff24890b61a392e87b022cb5f6a68 Mon Sep 17 00:00:00 2001 From: Brian Smith Date: Fri, 3 Oct 2014 15:52:38 -0700 Subject: [PATCH] Bug 1077859: Make ENCODING_FAILED safe to use in static initializers, r=mmc --HG-- extra : rebase_source : 78e1410ab6c94bd6b20a78208a2421db338aed94 --- .../tests/unit/tlsserver/lib/OCSPCommon.cpp | 2 +- security/pkix/test/gtest/pkixbuild_tests.cpp | 22 +- .../test/gtest/pkixcert_extension_tests.cpp | 20 +- ...kixocsp_CreateEncodedOCSPRequest_tests.cpp | 2 +- .../pkixocsp_VerifyEncodedOCSPResponse.cpp | 38 ++-- security/pkix/test/lib/pkixtestnss.cpp | 4 +- security/pkix/test/lib/pkixtestutil.cpp | 212 +++++++----------- security/pkix/test/lib/pkixtestutil.h | 4 +- 8 files changed, 119 insertions(+), 185 deletions(-) diff --git a/security/manager/ssl/tests/unit/tlsserver/lib/OCSPCommon.cpp b/security/manager/ssl/tests/unit/tlsserver/lib/OCSPCommon.cpp index e24957888a6e..0c1c8a54c56c 100644 --- a/security/manager/ssl/tests/unit/tlsserver/lib/OCSPCommon.cpp +++ b/security/manager/ssl/tests/unit/tlsserver/lib/OCSPCommon.cpp @@ -199,7 +199,7 @@ GetOCSPResponseForType(OCSPResponseType aORT, CERTCertificate *aCert, } ByteString response(CreateEncodedOCSPResponse(context)); - if (response == ENCODING_FAILED) { + if (ENCODING_FAILED(response)) { PrintPRError("CreateEncodedOCSPResponse failed"); return nullptr; } diff --git a/security/pkix/test/gtest/pkixbuild_tests.cpp b/security/pkix/test/gtest/pkixbuild_tests.cpp index a7ac7dd4e49a..3c14ca13e31e 100644 --- a/security/pkix/test/gtest/pkixbuild_tests.cpp +++ b/security/pkix/test/gtest/pkixbuild_tests.cpp @@ -47,19 +47,17 @@ CreateCert(const char* issuerCN, static long serialNumberValue = 0; ++serialNumberValue; ByteString serialNumber(CreateEncodedSerialNumber(serialNumberValue)); - EXPECT_NE(ENCODING_FAILED, serialNumber); + EXPECT_FALSE(ENCODING_FAILED(serialNumber)); ByteString issuerDER(CNToDERName(issuerCN)); - EXPECT_NE(ENCODING_FAILED, issuerDER); ByteString subjectDER(CNToDERName(subjectCN)); - EXPECT_NE(ENCODING_FAILED, subjectDER); ByteString extensions[2]; if (endEntityOrCA == EndEntityOrCA::MustBeCA) { extensions[0] = CreateEncodedBasicConstraints(true, nullptr, ExtensionCriticality::Critical); - EXPECT_NE(ENCODING_FAILED, extensions[0]); + EXPECT_FALSE(ENCODING_FAILED(extensions[0])); } ByteString certDER(CreateEncodedCertificate( @@ -69,7 +67,7 @@ CreateCert(const char* issuerCN, subjectDER, extensions, issuerKey, SignatureAlgorithm::rsa_pkcs1_with_sha256, subjectKey)); - EXPECT_NE(ENCODING_FAILED, certDER); + EXPECT_FALSE(ENCODING_FAILED(certDER)); if (subjectCert) { SECItem certDERItem = { siBuffer, @@ -245,7 +243,7 @@ TEST_F(pkixbuild, MaxAcceptableCertChainLength) ByteString certDER(CreateCert("CA7", "Direct End-Entity", EndEntityOrCA::MustBeEndEntity, trustDomain.leafCAKey.get(), unusedKeyPair)); - ASSERT_NE(ENCODING_FAILED, certDER); + ASSERT_FALSE(ENCODING_FAILED(certDER)); Input certDERInput; ASSERT_EQ(Success, certDERInput.Init(certDER.data(), certDER.length())); ASSERT_EQ(Success, @@ -271,7 +269,7 @@ TEST_F(pkixbuild, BeyondMaxAcceptableCertChainLength) ByteString certDER(CreateCert("CA7", caCertName, EndEntityOrCA::MustBeCA, trustDomain.leafCAKey.get(), caKeyPair, &caCert)); - ASSERT_NE(ENCODING_FAILED, certDER); + ASSERT_FALSE(ENCODING_FAILED(certDER)); Input certDERInput; ASSERT_EQ(Success, certDERInput.Init(certDER.data(), certDER.length())); ASSERT_EQ(Result::ERROR_UNKNOWN_ISSUER, @@ -288,7 +286,7 @@ TEST_F(pkixbuild, BeyondMaxAcceptableCertChainLength) ByteString certDER(CreateCert(caCertName, "End-Entity Too Far", EndEntityOrCA::MustBeEndEntity, caKeyPair.get(), unusedKeyPair)); - ASSERT_NE(ENCODING_FAILED, certDER); + ASSERT_FALSE(ENCODING_FAILED(certDER)); Input certDERInput; ASSERT_EQ(Success, certDERInput.Init(certDER.data(), certDER.length())); ASSERT_EQ(Result::ERROR_UNKNOWN_ISSUER, @@ -388,15 +386,13 @@ TEST_F(pkixbuild, NoRevocationCheckingForExpiredCert) ScopedTestKeyPair rootKey; ByteString rootDER(CreateCert(rootCN, rootCN, EndEntityOrCA::MustBeCA, nullptr, rootKey, nullptr)); - EXPECT_NE(ENCODING_FAILED, rootDER); + EXPECT_FALSE(ENCODING_FAILED(rootDER)); ExpiredCertTrustDomain expiredCertTrustDomain(rootDER); ByteString serialNumber(CreateEncodedSerialNumber(100)); - EXPECT_NE(ENCODING_FAILED, serialNumber); + EXPECT_FALSE(ENCODING_FAILED(serialNumber)); ByteString issuerDER(CNToDERName(rootCN)); - EXPECT_NE(ENCODING_FAILED, issuerDER); ByteString subjectDER(CNToDERName("Expired End-Entity Cert")); - EXPECT_NE(ENCODING_FAILED, subjectDER); ScopedTestKeyPair unusedSubjectKey; ByteString certDER(CreateEncodedCertificate( v3, sha256WithRSAEncryption, @@ -406,7 +402,7 @@ TEST_F(pkixbuild, NoRevocationCheckingForExpiredCert) subjectDER, nullptr, rootKey.get(), SignatureAlgorithm::rsa_pkcs1_with_sha256, unusedSubjectKey)); - EXPECT_NE(ENCODING_FAILED, certDER); + EXPECT_FALSE(ENCODING_FAILED(certDER)); Input cert; ASSERT_EQ(Success, cert.Init(certDER.data(), certDER.length())); diff --git a/security/pkix/test/gtest/pkixcert_extension_tests.cpp b/security/pkix/test/gtest/pkixcert_extension_tests.cpp index 3a2d7d67e274..371928e1379c 100644 --- a/security/pkix/test/gtest/pkixcert_extension_tests.cpp +++ b/security/pkix/test/gtest/pkixcert_extension_tests.cpp @@ -38,11 +38,11 @@ CreateCert(const char* subjectCN, static long serialNumberValue = 0; ++serialNumberValue; ByteString serialNumber(CreateEncodedSerialNumber(serialNumberValue)); - EXPECT_NE(ENCODING_FAILED, serialNumber); + EXPECT_FALSE(ENCODING_FAILED(serialNumber)); ByteString issuerDER(CNToDERName(subjectCN)); - EXPECT_NE(ENCODING_FAILED, issuerDER); + EXPECT_FALSE(ENCODING_FAILED(issuerDER)); ByteString subjectDER(CNToDERName(subjectCN)); - EXPECT_NE(ENCODING_FAILED, subjectDER); + EXPECT_FALSE(ENCODING_FAILED(subjectDER)); return CreateEncodedCertificate(v3, sha256WithRSAEncryption, serialNumber, issuerDER, oneDayBeforeNow, oneDayAfterNow, @@ -138,7 +138,7 @@ TEST_F(pkixcert_extension, UnknownCriticalExtension) const char* certCN = "Cert With Unknown Critical Extension"; ScopedTestKeyPair key; ByteString cert(CreateCert(certCN, unknownCriticalExtension, key)); - ASSERT_NE(ENCODING_FAILED, cert); + ASSERT_FALSE(ENCODING_FAILED(cert)); Input certInput; ASSERT_EQ(Success, certInput.Init(cert.data(), cert.length())); ASSERT_EQ(Result::ERROR_UNKNOWN_CRITICAL_EXTENSION, @@ -168,7 +168,7 @@ TEST_F(pkixcert_extension, UnknownNonCriticalExtension) const char* certCN = "Cert With Unknown NonCritical Extension"; ScopedTestKeyPair key; ByteString cert(CreateCert(certCN, unknownNonCriticalExtension, key)); - ASSERT_NE(ENCODING_FAILED, cert); + ASSERT_FALSE(ENCODING_FAILED(cert)); Input certInput; ASSERT_EQ(Success, certInput.Init(cert.data(), cert.length())); ASSERT_EQ(Success, @@ -199,7 +199,7 @@ TEST_F(pkixcert_extension, WrongOIDCriticalExtension) const char* certCN = "Cert With Critical Wrong OID Extension"; ScopedTestKeyPair key; ByteString cert(CreateCert(certCN, wrongOIDCriticalExtension, key)); - ASSERT_NE(ENCODING_FAILED, cert); + ASSERT_FALSE(ENCODING_FAILED(cert)); Input certInput; ASSERT_EQ(Success, certInput.Init(cert.data(), cert.length())); ASSERT_EQ(Result::ERROR_UNKNOWN_CRITICAL_EXTENSION, @@ -232,7 +232,7 @@ TEST_F(pkixcert_extension, CriticalAIAExtension) const char* certCN = "Cert With Critical AIA Extension"; ScopedTestKeyPair key; ByteString cert(CreateCert(certCN, criticalAIAExtension, key)); - ASSERT_NE(ENCODING_FAILED, cert); + ASSERT_FALSE(ENCODING_FAILED(cert)); Input certInput; ASSERT_EQ(Success, certInput.Init(cert.data(), cert.length())); ASSERT_EQ(Success, @@ -262,7 +262,7 @@ TEST_F(pkixcert_extension, UnknownCriticalCEExtension) const char* certCN = "Cert With Unknown Critical id-ce Extension"; ScopedTestKeyPair key; ByteString cert(CreateCert(certCN, unknownCriticalCEExtension, key)); - ASSERT_NE(ENCODING_FAILED, cert); + ASSERT_FALSE(ENCODING_FAILED(cert)); Input certInput; ASSERT_EQ(Success, certInput.Init(cert.data(), cert.length())); ASSERT_EQ(Result::ERROR_UNKNOWN_CRITICAL_EXTENSION, @@ -292,7 +292,7 @@ TEST_F(pkixcert_extension, KnownCriticalCEExtension) const char* certCN = "Cert With Known Critical id-ce Extension"; ScopedTestKeyPair key; ByteString cert(CreateCert(certCN, criticalCEExtension, key)); - ASSERT_NE(ENCODING_FAILED, cert); + ASSERT_FALSE(ENCODING_FAILED(cert)); Input certInput; ASSERT_EQ(Success, certInput.Init(cert.data(), cert.length())); ASSERT_EQ(Success, @@ -321,7 +321,7 @@ TEST_F(pkixcert_extension, DuplicateSubjectAltName) static const char* certCN = "Cert With Duplicate subjectAltName"; ScopedTestKeyPair key; ByteString cert(CreateCert(certCN, extensions, key)); - ASSERT_NE(ENCODING_FAILED, cert); + ASSERT_FALSE(ENCODING_FAILED(cert)); Input certInput; ASSERT_EQ(Success, certInput.Init(cert.data(), cert.length())); ASSERT_EQ(Result::ERROR_EXTENSION_VALUE_INVALID, diff --git a/security/pkix/test/gtest/pkixocsp_CreateEncodedOCSPRequest_tests.cpp b/security/pkix/test/gtest/pkixocsp_CreateEncodedOCSPRequest_tests.cpp index c8e93b6bb3ba..f8d8560aae29 100644 --- a/security/pkix/test/gtest/pkixocsp_CreateEncodedOCSPRequest_tests.cpp +++ b/security/pkix/test/gtest/pkixocsp_CreateEncodedOCSPRequest_tests.cpp @@ -87,7 +87,7 @@ protected: /*out*/ ByteString& issuerSPKI) { issuerDER = CNToDERName(issuerASCII); - ASSERT_NE(ENCODING_FAILED, issuerDER); + ASSERT_FALSE(ENCODING_FAILED(issuerDER)); ScopedTestKeyPair keyPair(GenerateKeyPair()); ASSERT_TRUE(keyPair); diff --git a/security/pkix/test/gtest/pkixocsp_VerifyEncodedOCSPResponse.cpp b/security/pkix/test/gtest/pkixocsp_VerifyEncodedOCSPResponse.cpp index 75320ecce931..27ccf630bb75 100644 --- a/security/pkix/test/gtest/pkixocsp_VerifyEncodedOCSPResponse.cpp +++ b/security/pkix/test/gtest/pkixocsp_VerifyEncodedOCSPResponse.cpp @@ -110,7 +110,7 @@ public: void SetUp() { rootNameDER = CNToDERName(rootName); - if (rootNameDER == ENCODING_FAILED) { + if (ENCODING_FAILED(rootNameDER)) { abort(); } Input rootNameDERInput; @@ -120,7 +120,7 @@ public: } serialNumberDER = CreateEncodedSerialNumber(++rootIssuedCount); - if (serialNumberDER == ENCODING_FAILED) { + if (ENCODING_FAILED(serialNumberDER)) { abort(); } Input serialNumberDERInput; @@ -248,7 +248,7 @@ public: OCSPResponseContext context(certID, producedAt); if (signerName) { context.signerNameDER = CNToDERName(signerName); - EXPECT_NE(ENCODING_FAILED, context.signerNameDER); + EXPECT_FALSE(ENCODING_FAILED(context.signerNameDER)); } context.signerKeyPair = signerKeyPair.Clone(); EXPECT_TRUE(context.signerKeyPair); @@ -398,7 +398,7 @@ protected: oneDayBeforeNow, oneDayAfterNow, certSubjectName, signerEKUDER ? extensions : nullptr, rootKeyPair.get(), signerKeyPair)); - EXPECT_NE(ENCODING_FAILED, signerDER); + EXPECT_FALSE(ENCODING_FAILED(signerDER)); if (signerDEROut) { *signerDEROut = signerDER; } @@ -406,7 +406,7 @@ protected: ByteString signerNameDER; if (signerName) { signerNameDER = CNToDERName(signerName); - EXPECT_NE(ENCODING_FAILED, signerNameDER); + EXPECT_FALSE(ENCODING_FAILED(signerNameDER)); } ByteString certs[] = { signerDER, ByteString() }; return CreateEncodedOCSPSuccessfulResponse(certStatus, *endEntityCertID, @@ -426,16 +426,16 @@ protected: /*out*/ ScopedTestKeyPair& keyPair) { ByteString serialNumberDER(CreateEncodedSerialNumber(serialNumber)); - if (serialNumberDER == ENCODING_FAILED) { - return ENCODING_FAILED; + if (ENCODING_FAILED(serialNumberDER)) { + return ByteString(); } ByteString issuerDER(CNToDERName(issuer)); - if (issuerDER == ENCODING_FAILED) { - return ENCODING_FAILED; + if (ENCODING_FAILED(issuerDER)) { + return ByteString(); } ByteString subjectDER(CNToDERName(subject)); - if (subjectDER == ENCODING_FAILED) { - return ENCODING_FAILED; + if (ENCODING_FAILED(subjectDER)) { + return ByteString(); } return ::mozilla::pkix::test::CreateEncodedCertificate( v3, @@ -547,7 +547,7 @@ TEST_F(pkixocsp_VerifyEncodedResponse_DelegatedResponder, good_expired) now - (2 * Time::ONE_DAY_IN_SECONDS), signerName, extensions, rootKeyPair.get(), signerKeyPair)); - ASSERT_NE(ENCODING_FAILED, signerDER); + ASSERT_FALSE(ENCODING_FAILED(signerDER)); ByteString certs[] = { signerDER, ByteString() }; ByteString responseString( @@ -582,7 +582,7 @@ TEST_F(pkixocsp_VerifyEncodedResponse_DelegatedResponder, good_future) now + (10 * Time::ONE_DAY_IN_SECONDS), signerName, extensions, rootKeyPair.get(), signerKeyPair)); - ASSERT_NE(ENCODING_FAILED, signerDER); + ASSERT_FALSE(ENCODING_FAILED(signerDER)); ByteString certs[] = { signerDER, ByteString() }; ByteString responseString( @@ -682,7 +682,7 @@ TEST_F(pkixocsp_VerifyEncodedResponse_DelegatedResponder, good_unknown_issuer) 1, subCAName, oneDayBeforeNow, oneDayAfterNow, signerName, extensions, unknownKeyPair.get(), signerKeyPair)); - ASSERT_NE(ENCODING_FAILED, signerDER); + ASSERT_FALSE(ENCODING_FAILED(signerDER)); // OCSP response signed by that delegated responder ByteString certs[] = { signerDER, ByteString() }; @@ -722,7 +722,7 @@ TEST_F(pkixocsp_VerifyEncodedResponse_DelegatedResponder, oneDayBeforeNow, oneDayAfterNow, subCAName, subCAExtensions, rootKeyPair.get(), subCAKeyPair)); - ASSERT_NE(ENCODING_FAILED, subCADER); + ASSERT_FALSE(ENCODING_FAILED(subCADER)); // Delegated responder cert signed by that sub-CA const ByteString extensions[] = { @@ -735,7 +735,7 @@ TEST_F(pkixocsp_VerifyEncodedResponse_DelegatedResponder, 1, subCAName, oneDayBeforeNow, oneDayAfterNow, signerName, extensions, subCAKeyPair.get(), signerKeyPair)); - ASSERT_NE(ENCODING_FAILED, signerDER); + ASSERT_FALSE(ENCODING_FAILED(signerDER)); // OCSP response signed by the delegated responder issued by the sub-CA // that is trying to impersonate the root. @@ -776,7 +776,7 @@ TEST_F(pkixocsp_VerifyEncodedResponse_DelegatedResponder, subCAName, subCAExtensions, rootKeyPair.get(), subCAKeyPair)); - ASSERT_NE(ENCODING_FAILED, subCADER); + ASSERT_FALSE(ENCODING_FAILED(subCADER)); // Delegated responder cert signed by that sub-CA const ByteString extensions[] = { @@ -789,7 +789,7 @@ TEST_F(pkixocsp_VerifyEncodedResponse_DelegatedResponder, 1, subCAName, oneDayBeforeNow, oneDayAfterNow, signerName, extensions, subCAKeyPair.get(), signerKeyPair)); - ASSERT_NE(ENCODING_FAILED, signerDER); + ASSERT_FALSE(ENCODING_FAILED(signerDER)); // OCSP response signed by the delegated responder issued by the sub-CA // that is trying to impersonate the root. @@ -821,7 +821,7 @@ public: CreateEncodedIndirectOCSPSuccessfulResponse( "OCSPGetCertTrustTest Signer", OCSPResponseContext::good, byKey, &OCSPSigningEKUDER, &signerCertDER); - if (responseString == ENCODING_FAILED) { + if (ENCODING_FAILED(responseString)) { abort(); } if (response.Init(responseString.data(), responseString.length()) diff --git a/security/pkix/test/lib/pkixtestnss.cpp b/security/pkix/test/lib/pkixtestnss.cpp index 41452a1c0256..2ba33b529982 100644 --- a/security/pkix/test/lib/pkixtestnss.cpp +++ b/security/pkix/test/lib/pkixtestnss.cpp @@ -191,14 +191,14 @@ ByteString SHA1(const ByteString& toHash) { if (InitNSSIfNeeded() != Success) { - return ENCODING_FAILED; + return ByteString(); } uint8_t digestBuf[SHA1_LENGTH]; SECStatus srv = PK11_HashBuf(SEC_OID_SHA1, digestBuf, toHash.data(), static_cast(toHash.length())); if (srv != SECSuccess) { - return ENCODING_FAILED; + return ByteString(); } return ByteString(digestBuf, sizeof(digestBuf)); } diff --git a/security/pkix/test/lib/pkixtestutil.cpp b/security/pkix/test/lib/pkixtestutil.cpp index 7066e213e5ad..708d13720da7 100644 --- a/security/pkix/test/lib/pkixtestutil.cpp +++ b/security/pkix/test/lib/pkixtestutil.cpp @@ -97,11 +97,8 @@ TamperOnce(/*in/out*/ ByteString& item, const ByteString& from, return Success; } -// An empty string returned from an encoding function signifies failure. -const ByteString ENCODING_FAILED; - // Given a tag and a value, generates a DER-encoded tag-length-value item. -static ByteString +ByteString TLV(uint8_t tag, const ByteString& value) { ByteString result; @@ -117,8 +114,9 @@ TLV(uint8_t tag, const ByteString& value) result.push_back(static_cast(value.length() / 256)); result.push_back(static_cast(value.length() % 256)); } else { - assert(false); - return ENCODING_FAILED; + // It is MUCH more convenient for TLV to be infallible than for it to have + // "proper" error handling. + abort(); } result.append(value); return result; @@ -155,8 +153,8 @@ static ByteString HashedOctetString(const ByteString& bytes) { ByteString digest(SHA1(bytes)); - if (digest == ENCODING_FAILED) { - return ENCODING_FAILED; + if (ENCODING_FAILED(digest)) { + return ByteString(); } return TLV(der::OCTET_STRING, digest); } @@ -190,7 +188,9 @@ Integer(long value) { if (value < 0 || value > 127) { // TODO: add encoding of larger values - return ENCODING_FAILED; + // It is MUCH more convenient for Integer to be infallible than for it to + // have "proper" error handling. + abort(); } ByteString encodedValue; @@ -225,7 +225,7 @@ TimeToEncodedTime(time_t time, TimeEncoding encoding) tm exploded; if (!gmtime_r(&time, &exploded)) { - return ENCODING_FAILED; + return ByteString(); } if (exploded.tm_sec >= 60) { @@ -237,7 +237,7 @@ TimeToEncodedTime(time_t time, TimeEncoding encoding) int year = exploded.tm_year + 1900; if (encoding == UTCTime && (year < 1950 || year >= 2050)) { - return ENCODING_FAILED; + return ByteString(); } ByteString value; @@ -281,7 +281,7 @@ TimeToTimeChoice(time_t time) { tm exploded; if (!gmtime_r(&time, &exploded)) { - return ENCODING_FAILED; + return ByteString(); } TimeEncoding encoding = (exploded.tm_year + 1900 >= 1950 && exploded.tm_year + 1900 < 2050) @@ -341,14 +341,17 @@ YMDHMS(int16_t year, int16_t month, int16_t day, static ByteString SignedData(const ByteString& tbsData, - TestKeyPair& keyPair, + /*optional*/ TestKeyPair* keyPair, SignatureAlgorithm signatureAlgorithm, bool corrupt, /*optional*/ const ByteString* certs) { ByteString signature; - if (keyPair.SignData(tbsData, signatureAlgorithm, signature) != Success) { - return ENCODING_FAILED; - } + if (keyPair) { + if (keyPair->SignData(tbsData, signatureAlgorithm, signature) + != Success) { + return ByteString(); + } + } ByteString signatureAlgorithmDER; switch (signatureAlgorithm) { @@ -357,14 +360,14 @@ SignedData(const ByteString& tbsData, sizeof(alg_sha256WithRSAEncryption)); break; default: - return ENCODING_FAILED; + return ByteString(); } // TODO: add ability to have signatures of bit length not divisible by 8, // resulting in unused bits in the bitstring encoding ByteString signatureNested(BitString(signature, corrupt)); - if (signatureNested == ENCODING_FAILED) { - return ENCODING_FAILED; + if (ENCODING_FAILED(signatureNested)) { + return ByteString(); } ByteString certsNested; @@ -375,14 +378,8 @@ SignedData(const ByteString& tbsData, ++certs; } ByteString certsSequence(TLV(der::SEQUENCE, certsSequenceValue)); - if (certsSequence == ENCODING_FAILED) { - return ENCODING_FAILED; - } certsNested = TLV(der::CONTEXT_SPECIFIC | der::CONSTRUCTED | 0, certsSequence); - if (certsNested == ENCODING_FAILED) { - return ENCODING_FAILED; - } } ByteString value; @@ -411,20 +408,11 @@ Extension(Input extnID, ExtensionCriticality criticality, if (criticality == ExtensionCriticality::Critical) { ByteString critical(Boolean(true)); - if (critical == ENCODING_FAILED) { - return ENCODING_FAILED; - } encoded.append(critical); } ByteString extnValueSequence(TLV(der::SEQUENCE, extnValueBytes)); - if (extnValueBytes == ENCODING_FAILED) { - return ENCODING_FAILED; - } ByteString extnValue(TLV(der::OCTET_STRING, extnValueSequence)); - if (extnValue == ENCODING_FAILED) { - return ENCODING_FAILED; - } encoded.append(extnValue); return TLV(der::SEQUENCE, encoded); } @@ -487,7 +475,7 @@ CreateEncodedCertificate(long version, Input signature, // with issuerKeyPair. ScopedTestKeyPair subjectKeyPair(GenerateKeyPair()); if (!subjectKeyPair) { - return ENCODING_FAILED; + return ByteString(); } ByteString tbsCertificate(TBSCertificate(version, serialNumber, @@ -495,16 +483,16 @@ CreateEncodedCertificate(long version, Input signature, notAfter, subjectNameDER, subjectKeyPair->subjectPublicKeyInfo, extensions)); - if (tbsCertificate == ENCODING_FAILED) { - return ENCODING_FAILED; + if (ENCODING_FAILED(tbsCertificate)) { + return ByteString(); } ByteString result(SignedData(tbsCertificate, - issuerKeyPair ? *issuerKeyPair - : *subjectKeyPair, + issuerKeyPair ? issuerKeyPair + : subjectKeyPair.get(), signatureAlgorithm, false, nullptr)); - if (result == ENCODING_FAILED) { - return ENCODING_FAILED; + if (ENCODING_FAILED(result)) { + return ByteString(); } MaybeLogOutput(result, "cert"); @@ -540,14 +528,8 @@ TBSCertificate(long versionValue, if (versionValue != static_cast(der::Version::v1)) { ByteString versionInteger(Integer(versionValue)); - if (versionInteger == ENCODING_FAILED) { - return ENCODING_FAILED; - } ByteString version(TLV(der::CONTEXT_SPECIFIC | der::CONSTRUCTED | 0, versionInteger)); - if (version == ENCODING_FAILED) { - return ENCODING_FAILED; - } value.append(version); } @@ -561,19 +543,19 @@ TBSCertificate(long versionValue, ByteString validity; { ByteString notBefore(TimeToTimeChoice(notBeforeTime)); - if (notBefore == ENCODING_FAILED) { - return ENCODING_FAILED; + if (ENCODING_FAILED(notBefore)) { + return ByteString(); } ByteString notAfter(TimeToTimeChoice(notAfterTime)); - if (notAfter == ENCODING_FAILED) { - return ENCODING_FAILED; + if (ENCODING_FAILED(notAfter)) { + return ByteString(); } ByteString validityValue; validityValue.append(notBefore); validityValue.append(notAfter); validity = TLV(der::SEQUENCE, validityValue); - if (validity == ENCODING_FAILED) { - return ENCODING_FAILED; + if (ENCODING_FAILED(validity)) { + return ByteString(); } } value.append(validity); @@ -589,13 +571,13 @@ TBSCertificate(long versionValue, ++extensions; } ByteString extensionsSequence(TLV(der::SEQUENCE, extensionsValue)); - if (extensionsSequence == ENCODING_FAILED) { - return ENCODING_FAILED; + if (ENCODING_FAILED(extensionsSequence)) { + return ByteString(); } ByteString extensionsWrapped( TLV(der::CONTEXT_SPECIFIC | der::CONSTRUCTED | 3, extensionsSequence)); - if (extensionsWrapped == ENCODING_FAILED) { - return ENCODING_FAILED; + if (ENCODING_FAILED(extensionsWrapped)) { + return ByteString(); } value.append(extensionsWrapped); } @@ -644,15 +626,7 @@ CNToDERName(const char* cn) ava.append(tlv_id_at_commonName, sizeof(tlv_id_at_commonName)); ava.append(value); ava = TLV(der::SEQUENCE, ava); - if (ava == ENCODING_FAILED) { - return ENCODING_FAILED; - } - ByteString rdn(TLV(der::SET, ava)); - if (rdn == ENCODING_FAILED) { - return ENCODING_FAILED; - } - return TLV(der::SEQUENCE, rdn); } @@ -674,17 +648,11 @@ CreateEncodedBasicConstraints(bool isCA, if (isCA) { ByteString cA(Boolean(true)); - if (cA == ENCODING_FAILED) { - return ENCODING_FAILED; - } value.append(cA); } if (pathLenConstraintValue) { ByteString pathLenConstraint(Integer(*pathLenConstraintValue)); - if (pathLenConstraint == ENCODING_FAILED) { - return ENCODING_FAILED; - } value.append(pathLenConstraint); } @@ -718,7 +686,7 @@ CreateEncodedOCSPResponse(OCSPResponseContext& context) { if (!context.skipResponseBytes) { if (!context.signerKeyPair) { - return ENCODING_FAILED; + return ByteString(); } } @@ -738,31 +706,22 @@ CreateEncodedOCSPResponse(OCSPResponseContext& context) ByteString reponseStatusValue; reponseStatusValue.push_back(context.responseStatus); ByteString responseStatus(TLV(der::ENUMERATED, reponseStatusValue)); - if (responseStatus == ENCODING_FAILED) { - return ENCODING_FAILED; - } ByteString responseBytesNested; if (!context.skipResponseBytes) { ByteString responseBytes(ResponseBytes(context)); - if (responseBytes == ENCODING_FAILED) { - return ENCODING_FAILED; + if (ENCODING_FAILED(responseBytes)) { + return ByteString(); } responseBytesNested = TLV(der::CONSTRUCTED | der::CONTEXT_SPECIFIC, responseBytes); - if (responseBytesNested == ENCODING_FAILED) { - return ENCODING_FAILED; - } } ByteString value; value.append(responseStatus); value.append(responseBytesNested); ByteString result(TLV(der::SEQUENCE, value)); - if (result == ENCODING_FAILED) { - return ENCODING_FAILED; - } MaybeLogOutput(result, "ocsp"); @@ -780,13 +739,10 @@ ResponseBytes(OCSPResponseContext& context) 0x06, 0x09, 0x2B, 0x06, 0x01, 0x05, 0x05, 0x07, 0x30, 0x01, 0x01 }; ByteString response(BasicOCSPResponse(context)); - if (response == ENCODING_FAILED) { - return ENCODING_FAILED; + if (ENCODING_FAILED(response)) { + return ByteString(); } ByteString responseNested = TLV(der::OCTET_STRING, response); - if (responseNested == ENCODING_FAILED) { - return ENCODING_FAILED; - } ByteString value; value.append(id_pkix_ocsp_basic_encoded, @@ -804,12 +760,12 @@ ByteString BasicOCSPResponse(OCSPResponseContext& context) { ByteString tbsResponseData(ResponseData(context)); - if (tbsResponseData == ENCODING_FAILED) { - return ENCODING_FAILED; + if (ENCODING_FAILED(tbsResponseData)) { + return ByteString(); } // TODO(bug 980538): certs - return SignedData(tbsResponseData, *context.signerKeyPair, + return SignedData(tbsResponseData, context.signerKeyPair.get(), SignatureAlgorithm::rsa_pkcs1_with_sha256, context.badSignature, context.certs); } @@ -826,15 +782,9 @@ OCSPExtension(OCSPResponseContext& context, OCSPResponseExtension& extension) encoded.append(extension.id); if (extension.critical) { ByteString critical(Boolean(true)); - if (critical == ENCODING_FAILED) { - return ENCODING_FAILED; - } encoded.append(critical); } ByteString value(TLV(der::OCTET_STRING, extension.value)); - if (value == ENCODING_FAILED) { - return ENCODING_FAILED; - } encoded.append(value); return TLV(der::SEQUENCE, encoded); } @@ -849,15 +799,12 @@ Extensions(OCSPResponseContext& context) for (OCSPResponseExtension* extension = context.extensions; extension; extension = extension->next) { ByteString extensionEncoded(OCSPExtension(context, *extension)); - if (extensionEncoded == ENCODING_FAILED) { - return ENCODING_FAILED; + if (ENCODING_FAILED(extensionEncoded)) { + return ByteString(); } value.append(extensionEncoded); } ByteString sequence(TLV(der::SEQUENCE, value)); - if (sequence == ENCODING_FAILED) { - return ENCODING_FAILED; - } return TLV(der::CONSTRUCTED | der::CONTEXT_SPECIFIC | 1, sequence); } @@ -871,21 +818,18 @@ ByteString ResponseData(OCSPResponseContext& context) { ByteString responderID(ResponderID(context)); - if (responderID == ENCODING_FAILED) { - return ENCODING_FAILED; + if (ENCODING_FAILED(responderID)) { + return ByteString(); } ByteString producedAtEncoded(TimeToGeneralizedTime(context.producedAt)); - if (producedAtEncoded == ENCODING_FAILED) { - return ENCODING_FAILED; + if (ENCODING_FAILED(producedAtEncoded)) { + return ByteString(); } ByteString response(SingleResponse(context)); - if (response == ENCODING_FAILED) { - return ENCODING_FAILED; + if (ENCODING_FAILED(response)) { + return ByteString(); } ByteString responses(TLV(der::SEQUENCE, response)); - if (responses == ENCODING_FAILED) { - return ENCODING_FAILED; - } ByteString responseExtensions; if (context.extensions || context.includeEmptyExtensions) { responseExtensions = Extensions(context); @@ -913,8 +857,8 @@ ResponderID(OCSPResponseContext& context) responderIDType = 1; // byName } else { contents = KeyHash(context.signerKeyPair->subjectPublicKey); - if (contents == ENCODING_FAILED) { - return ENCODING_FAILED; + if (ENCODING_FAILED(contents)) { + return ByteString(); } responderIDType = 2; // byKey } @@ -944,28 +888,25 @@ ByteString SingleResponse(OCSPResponseContext& context) { ByteString certID(CertID(context)); - if (certID == ENCODING_FAILED) { - return ENCODING_FAILED; + if (ENCODING_FAILED(certID)) { + return ByteString(); } ByteString certStatus(CertStatus(context)); - if (certStatus == ENCODING_FAILED) { - return ENCODING_FAILED; + if (ENCODING_FAILED(certStatus)) { + return ByteString(); } ByteString thisUpdateEncoded(TimeToGeneralizedTime(context.thisUpdate)); - if (thisUpdateEncoded == ENCODING_FAILED) { - return ENCODING_FAILED; + if (ENCODING_FAILED(thisUpdateEncoded)) { + return ByteString(); } ByteString nextUpdateEncodedNested; if (context.includeNextUpdate) { ByteString nextUpdateEncoded(TimeToGeneralizedTime(context.nextUpdate)); - if (nextUpdateEncoded == ENCODING_FAILED) { - return ENCODING_FAILED; + if (ENCODING_FAILED(nextUpdateEncoded)) { + return ByteString(); } nextUpdateEncodedNested = TLV(der::CONSTRUCTED | der::CONTEXT_SPECIFIC | 0, nextUpdateEncoded); - if (nextUpdateEncodedNested == ENCODING_FAILED) { - return ENCODING_FAILED; - } } ByteString value; @@ -987,8 +928,8 @@ CertID(OCSPResponseContext& context) ByteString issuerName(context.certID.issuer.UnsafeGetData(), context.certID.issuer.GetLength()); ByteString issuerNameHash(HashedOctetString(issuerName)); - if (issuerNameHash == ENCODING_FAILED) { - return ENCODING_FAILED; + if (ENCODING_FAILED(issuerNameHash)) { + return ByteString(); } ByteString issuerKeyHash; @@ -999,30 +940,27 @@ CertID(OCSPResponseContext& context) Reader input(context.certID.issuerSubjectPublicKeyInfo); Reader contents; if (der::ExpectTagAndGetValue(input, der::SEQUENCE, contents) != Success) { - return ENCODING_FAILED; + return ByteString(); } // Skip AlgorithmIdentifier if (der::ExpectTagAndSkipValue(contents, der::SEQUENCE) != Success) { - return ENCODING_FAILED; + return ByteString(); } Input subjectPublicKey; if (der::BitStringWithNoUnusedBits(contents, subjectPublicKey) != Success) { - return ENCODING_FAILED; + return ByteString(); } issuerKeyHash = KeyHash(ByteString(subjectPublicKey.UnsafeGetData(), subjectPublicKey.GetLength())); - if (issuerKeyHash == ENCODING_FAILED) { - return ENCODING_FAILED; + if (ENCODING_FAILED(issuerKeyHash)) { + return ByteString(); } } ByteString serialNumberValue(context.certID.serialNumber.UnsafeGetData(), context.certID.serialNumber.GetLength()); ByteString serialNumber(TLV(der::INTEGER, serialNumberValue)); - if (serialNumber == ENCODING_FAILED) { - return ENCODING_FAILED; - } // python DottedOIDToCode.py --alg id-sha1 1.3.14.3.2.26 static const uint8_t alg_id_sha1[] = { @@ -1062,8 +1000,8 @@ CertStatus(OCSPResponseContext& context) case 1: { ByteString revocationTime(TimeToGeneralizedTime(context.revocationTime)); - if (revocationTime == ENCODING_FAILED) { - return ENCODING_FAILED; + if (ENCODING_FAILED(revocationTime)) { + return ByteString(); } // TODO(bug 980536): add support for revocationReason return TLV(der::CONTEXT_SPECIFIC | der::CONSTRUCTED | 1, revocationTime); @@ -1072,7 +1010,7 @@ CertStatus(OCSPResponseContext& context) assert(false); // fall through } - return ENCODING_FAILED; + return ByteString(); } } } } // namespace mozilla::pkix::test diff --git a/security/pkix/test/lib/pkixtestutil.h b/security/pkix/test/lib/pkixtestutil.h index 57cf057ac9cd..411221c51ea5 100644 --- a/security/pkix/test/lib/pkixtestutil.h +++ b/security/pkix/test/lib/pkixtestutil.h @@ -36,7 +36,8 @@ namespace mozilla { namespace pkix { namespace test { typedef std::basic_string ByteString; -extern const ByteString ENCODING_FAILED; + +inline bool ENCODING_FAILED(const ByteString& bs) { return bs.empty(); } // XXX: Ideally, we should define this instead: // @@ -79,7 +80,6 @@ extern const Input sha256WithRSAEncryption; mozilla::pkix::Time YMDHMS(int16_t year, int16_t month, int16_t day, int16_t hour, int16_t minutes, int16_t seconds); - ByteString CNToDERName(const char* cn); class TestKeyPair