From 8b26ecac0b50cec9372f35773a1c52e2e3deb15c Mon Sep 17 00:00:00 2001 From: Brian Smith Date: Sat, 30 Aug 2014 23:11:23 -0700 Subject: [PATCH] Bug 1061021, Part 1: Stop using NSS to encode names in tests, r=keeler --HG-- extra : rebase_source : 1fa1826fe356314e80784915e08d5a787bf2259f --- security/pkix/lib/pkixder.h | 2 + security/pkix/test/gtest/pkixbuild_tests.cpp | 29 +-- .../test/gtest/pkixcert_extension_tests.cpp | 24 +- ...kixocsp_CreateEncodedOCSPRequest_tests.cpp | 54 ++-- .../pkixocsp_VerifyEncodedOCSPResponse.cpp | 73 +++--- security/pkix/test/lib/pkixtestutil.cpp | 235 ++++++++++-------- security/pkix/test/lib/pkixtestutil.h | 14 +- 7 files changed, 236 insertions(+), 195 deletions(-) diff --git a/security/pkix/lib/pkixder.h b/security/pkix/lib/pkixder.h index 31fea8910e58..e6590d46d0d3 100644 --- a/security/pkix/lib/pkixder.h +++ b/security/pkix/lib/pkixder.h @@ -64,7 +64,9 @@ enum Tag NULLTag = UNIVERSAL | 0x05, OIDTag = UNIVERSAL | 0x06, ENUMERATED = UNIVERSAL | 0x0a, + UTF8String = UNIVERSAL | 0x0c, SEQUENCE = UNIVERSAL | CONSTRUCTED | 0x10, // 0x30 + SET = UNIVERSAL | CONSTRUCTED | 0x11, // 0x31 UTCTime = UNIVERSAL | 0x17, GENERALIZED_TIME = UNIVERSAL | 0x18, }; diff --git a/security/pkix/test/gtest/pkixbuild_tests.cpp b/security/pkix/test/gtest/pkixbuild_tests.cpp index 0fb1071e5016..0494d0d4a150 100644 --- a/security/pkix/test/gtest/pkixbuild_tests.cpp +++ b/security/pkix/test/gtest/pkixbuild_tests.cpp @@ -40,8 +40,8 @@ typedef ScopedPtr ScopedCERTCertList; // The result is owned by the arena static Input -CreateCert(PLArenaPool* arena, const char* issuerStr, - const char* subjectStr, EndEntityOrCA endEntityOrCA, +CreateCert(PLArenaPool* arena, const char* issuerCN, + const char* subjectCN, EndEntityOrCA endEntityOrCA, /*optional*/ SECKEYPrivateKey* issuerKey, /*out*/ ScopedSECKEYPrivateKey& subjectKey, /*out*/ ScopedCERTCertificate* subjectCert = nullptr) @@ -51,10 +51,11 @@ CreateCert(PLArenaPool* arena, const char* issuerStr, const SECItem* serialNumber(CreateEncodedSerialNumber(arena, serialNumberValue)); EXPECT_TRUE(serialNumber); - const SECItem* issuerDER(ASCIIToDERName(arena, issuerStr)); - EXPECT_TRUE(issuerDER); - const SECItem* subjectDER(ASCIIToDERName(arena, subjectStr)); - EXPECT_TRUE(subjectDER); + + ByteString issuerDER(CNToDERName(issuerCN)); + EXPECT_NE(ENCODING_FAILED, issuerDER); + ByteString subjectDER(CNToDERName(subjectCN)); + EXPECT_NE(ENCODING_FAILED, subjectDER); const SECItem* extensions[2] = { nullptr, nullptr }; if (endEntityOrCA == EndEntityOrCA::MustBeCA) { @@ -90,8 +91,7 @@ public: bool SetUpCertChainTail() { static char const* const names[] = { - "CN=CA1 (Root)", "CN=CA2", "CN=CA3", "CN=CA4", "CN=CA5", "CN=CA6", - "CN=CA7" + "CA1 (Root)", "CA2", "CA3", "CA4", "CA5", "CA6", "CA7" }; static_assert(MOZILLA_PKIX_ARRAY_LENGTH(names) == @@ -104,8 +104,7 @@ public: } for (size_t i = 0; i < MOZILLA_PKIX_ARRAY_LENGTH(names); ++i) { - const char* issuerName = i == 0 ? names[0] - : certChainTail[i - 1]->subjectName; + const char* issuerName = i == 0 ? names[0] : names[i-1]; (void) CreateCert(arena.get(), issuerName, names[i], EndEntityOrCA::MustBeCA, leafCAKey.get(), leafCAKey, &certChainTail[i]); @@ -245,8 +244,7 @@ TEST_F(pkixbuild, MaxAcceptableCertChainLength) ScopedSECKEYPrivateKey privateKey; ScopedCERTCertificate cert; Input certDER(CreateCert(arena.get(), - trustDomain.GetLeafCACert()->subjectName, - "CN=Direct End-Entity", + "CA7", "Direct End-Entity", EndEntityOrCA::MustBeEndEntity, trustDomain.leafCAKey.get(), privateKey)); ASSERT_EQ(Success, @@ -261,7 +259,7 @@ TEST_F(pkixbuild, MaxAcceptableCertChainLength) TEST_F(pkixbuild, BeyondMaxAcceptableCertChainLength) { - static char const* const caCertName = "CN=CA Too Far"; + static char const* const caCertName = "CA Too Far"; ScopedSECKEYPrivateKey caPrivateKey; // We need a CERTCertificate for caCert so that the trustdomain's FindIssuer @@ -269,8 +267,7 @@ TEST_F(pkixbuild, BeyondMaxAcceptableCertChainLength) ScopedCERTCertificate caCert; { - Input cert(CreateCert(arena.get(), - trustDomain.GetLeafCACert()->subjectName, + Input cert(CreateCert(arena.get(), "CA7", caCertName, EndEntityOrCA::MustBeCA, trustDomain.leafCAKey.get(), caPrivateKey, &caCert)); @@ -286,7 +283,7 @@ TEST_F(pkixbuild, BeyondMaxAcceptableCertChainLength) { ScopedSECKEYPrivateKey privateKey; Input cert(CreateCert(arena.get(), caCertName, - "CN=End-Entity Too Far", + "End-Entity Too Far", EndEntityOrCA::MustBeEndEntity, caPrivateKey.get(), privateKey)); ASSERT_EQ(Result::ERROR_UNKNOWN_ISSUER, diff --git a/security/pkix/test/gtest/pkixcert_extension_tests.cpp b/security/pkix/test/gtest/pkixcert_extension_tests.cpp index 72aaa472dbd1..2f7c5aaf2449 100644 --- a/security/pkix/test/gtest/pkixcert_extension_tests.cpp +++ b/security/pkix/test/gtest/pkixcert_extension_tests.cpp @@ -33,7 +33,7 @@ using namespace mozilla::pkix::test; // Creates a self-signed certificate with the given extension. static Input -CreateCert(PLArenaPool* arena, const char* subjectStr, +CreateCert(PLArenaPool* arena, const char* subjectCN, SECItem const* const* extensions, // null-terminated array /*out*/ ScopedSECKEYPrivateKey& subjectKey) { @@ -42,10 +42,10 @@ CreateCert(PLArenaPool* arena, const char* subjectStr, const SECItem* serialNumber(CreateEncodedSerialNumber(arena, serialNumberValue)); EXPECT_TRUE(serialNumber); - const SECItem* issuerDER(ASCIIToDERName(arena, subjectStr)); - EXPECT_TRUE(issuerDER); - const SECItem* subjectDER(ASCIIToDERName(arena, subjectStr)); - EXPECT_TRUE(subjectDER); + ByteString issuerDER(CNToDERName(subjectCN)); + EXPECT_NE(ENCODING_FAILED, issuerDER); + ByteString subjectDER(CNToDERName(subjectCN)); + EXPECT_NE(ENCODING_FAILED, subjectDER); SECItem* cert = CreateEncodedCertificate( arena, v3, sha256WithRSAEncryption, serialNumber, issuerDER, @@ -152,7 +152,7 @@ TEST_F(pkixcert_extension, UnknownCriticalExtension) const_cast(unknownCriticalExtensionBytes), sizeof(unknownCriticalExtensionBytes) }; - const char* certCN = "CN=Cert With Unknown Critical Extension"; + const char* certCN = "Cert With Unknown Critical Extension"; ScopedSECKEYPrivateKey key; // cert is owned by the arena Input cert(CreateCert(arena.get(), certCN, @@ -183,7 +183,7 @@ TEST_F(pkixcert_extension, UnknownNonCriticalExtension) const_cast(unknownNonCriticalExtensionBytes), sizeof(unknownNonCriticalExtensionBytes) }; - const char* certCN = "CN=Cert With Unknown NonCritical Extension"; + const char* certCN = "Cert With Unknown NonCritical Extension"; ScopedSECKEYPrivateKey key; // cert is owned by the arena Input cert(CreateCert(arena.get(), certCN, @@ -215,7 +215,7 @@ TEST_F(pkixcert_extension, WrongOIDCriticalExtension) const_cast(wrongOIDCriticalExtensionBytes), sizeof(wrongOIDCriticalExtensionBytes) }; - const char* certCN = "CN=Cert With Critical Wrong OID Extension"; + const char* certCN = "Cert With Critical Wrong OID Extension"; ScopedSECKEYPrivateKey key; // cert is owned by the arena Input cert(CreateCert(arena.get(), certCN, @@ -249,7 +249,7 @@ TEST_F(pkixcert_extension, CriticalAIAExtension) const_cast(criticalAIAExtensionBytes), sizeof(criticalAIAExtensionBytes) }; - const char* certCN = "CN=Cert With Critical AIA Extension"; + const char* certCN = "Cert With Critical AIA Extension"; ScopedSECKEYPrivateKey key; // cert is owned by the arena Input cert(CreateCert(arena.get(), certCN, &criticalAIAExtension, key)); @@ -279,7 +279,7 @@ TEST_F(pkixcert_extension, UnknownCriticalCEExtension) const_cast(unknownCriticalCEExtensionBytes), sizeof(unknownCriticalCEExtensionBytes) }; - const char* certCN = "CN=Cert With Unknown Critical id-ce Extension"; + const char* certCN = "Cert With Unknown Critical id-ce Extension"; ScopedSECKEYPrivateKey key; // cert is owned by the arena Input cert(CreateCert(arena.get(), certCN, @@ -310,7 +310,7 @@ TEST_F(pkixcert_extension, KnownCriticalCEExtension) const_cast(criticalCEExtensionBytes), sizeof(criticalCEExtensionBytes) }; - const char* certCN = "CN=Cert With Known Critical id-ce Extension"; + const char* certCN = "Cert With Known Critical id-ce Extension"; ScopedSECKEYPrivateKey key; // cert is owned by the arena Input cert(CreateCert(arena.get(), certCN, &criticalCEExtension, key)); @@ -341,7 +341,7 @@ TEST_F(pkixcert_extension, DuplicateSubjectAltName) sizeof(DER_BYTES) }; static SECItem const* const extensions[] = { &DER, &DER, nullptr }; - static const char* certCN = "CN=Cert With Duplicate subjectAltName"; + static const char* certCN = "Cert With Duplicate subjectAltName"; ScopedSECKEYPrivateKey key; // cert is owned by the arena Input cert(CreateCert(arena.get(), certCN, extensions, key)); diff --git a/security/pkix/test/gtest/pkixocsp_CreateEncodedOCSPRequest_tests.cpp b/security/pkix/test/gtest/pkixocsp_CreateEncodedOCSPRequest_tests.cpp index fa0de6da78d4..a707312101bd 100644 --- a/security/pkix/test/gtest/pkixocsp_CreateEncodedOCSPRequest_tests.cpp +++ b/security/pkix/test/gtest/pkixocsp_CreateEncodedOCSPRequest_tests.cpp @@ -113,15 +113,12 @@ protected: longestRequiredSerialNumber->data[2] = 0x01; // value is 0x010000...00 } - // The resultant issuerDER and issuerSPKI are owned by the arena. void MakeIssuerCertIDComponents(const char* issuerASCII, - /*out*/ Input& issuerDER, - /*out*/ Input& issuerSPKI) + /*out*/ ByteString& issuerDER, + /*out*/ ByteString& issuerSPKI) { - const SECItem* issuerDERSECItem = ASCIIToDERName(arena.get(), issuerASCII); - ASSERT_TRUE(issuerDERSECItem); - ASSERT_EQ(Success, - issuerDER.Init(issuerDERSECItem->data, issuerDERSECItem->len)); + issuerDER = CNToDERName(issuerASCII); + ASSERT_NE(ENCODING_FAILED, issuerDER); ScopedSECKEYPublicKey issuerPublicKey; ScopedSECKEYPrivateKey issuerPrivateKey; @@ -131,12 +128,7 @@ protected: SECKEY_EncodeDERSubjectPublicKeyInfo(issuerPublicKey.get())); ASSERT_TRUE(issuerSPKIOriginal); - SECItem issuerSPKICopy; - ASSERT_EQ(SECSuccess, - SECITEM_CopyItem(arena.get(), &issuerSPKICopy, - issuerSPKIOriginal.get())); - ASSERT_EQ(Success, - issuerSPKI.Init(issuerSPKICopy.data, issuerSPKICopy.len)); + issuerSPKI.assign(issuerSPKIOriginal->data, issuerSPKIOriginal->len); } CreateEncodedOCSPRequestTrustDomain trustDomain; @@ -146,18 +138,26 @@ protected: // CreateEncodedOCSPRequest to fail. TEST_F(pkixocsp_CreateEncodedOCSPRequest, ChildCertLongSerialNumberTest) { - Input issuerDER; - Input issuerSPKI; - MakeIssuerCertIDComponents("CN=CA", issuerDER, issuerSPKI); + ByteString issuerDER; + ByteString issuerSPKI; + ASSERT_NO_FATAL_FAILURE(MakeIssuerCertIDComponents("CA", issuerDER, + issuerSPKI)); + + Input issuer; + ASSERT_EQ(Success, issuer.Init(issuerDER.data(), issuerDER.length())); + + Input spki; + ASSERT_EQ(Success, spki.Init(issuerSPKI.data(), issuerSPKI.length())); + Input serialNumber; ASSERT_EQ(Success, serialNumber.Init(unsupportedLongSerialNumber->data, unsupportedLongSerialNumber->len)); + uint8_t ocspRequest[OCSP_REQUEST_MAX_LENGTH]; size_t ocspRequestLength; ASSERT_EQ(Result::ERROR_BAD_DER, CreateEncodedOCSPRequest(trustDomain, - CertID(issuerDER, issuerSPKI, - serialNumber), + CertID(issuer, spki, serialNumber), ocspRequest, ocspRequestLength)); } @@ -165,17 +165,25 @@ TEST_F(pkixocsp_CreateEncodedOCSPRequest, ChildCertLongSerialNumberTest) // it's required to support (i.e. 20 octets). TEST_F(pkixocsp_CreateEncodedOCSPRequest, LongestSupportedSerialNumberTest) { - Input issuerDER; - Input issuerSPKI; - MakeIssuerCertIDComponents("CN=CA", issuerDER, issuerSPKI); + ByteString issuerDER; + ByteString issuerSPKI; + ASSERT_NO_FATAL_FAILURE(MakeIssuerCertIDComponents("CA", issuerDER, + issuerSPKI)); + + Input issuer; + ASSERT_EQ(Success, issuer.Init(issuerDER.data(), issuerDER.length())); + + Input spki; + ASSERT_EQ(Success, spki.Init(issuerSPKI.data(), issuerSPKI.length())); + Input serialNumber; ASSERT_EQ(Success, serialNumber.Init(longestRequiredSerialNumber->data, longestRequiredSerialNumber->len)); + uint8_t ocspRequest[OCSP_REQUEST_MAX_LENGTH]; size_t ocspRequestLength; ASSERT_EQ(Success, CreateEncodedOCSPRequest(trustDomain, - CertID(issuerDER, issuerSPKI, - serialNumber), + CertID(issuer, spki, serialNumber), ocspRequest, ocspRequestLength)); } diff --git a/security/pkix/test/gtest/pkixocsp_VerifyEncodedOCSPResponse.cpp b/security/pkix/test/gtest/pkixocsp_VerifyEncodedOCSPResponse.cpp index b32df13389e4..c0996f3c0a9b 100644 --- a/security/pkix/test/gtest/pkixocsp_VerifyEncodedOCSPResponse.cpp +++ b/security/pkix/test/gtest/pkixocsp_VerifyEncodedOCSPResponse.cpp @@ -98,7 +98,7 @@ private: }; namespace { -char const* const rootName = "CN=Test CA 1"; +char const* const rootName = "Test CA 1"; void deleteCertID(CertID* certID) { delete certID; } } // unnamed namespace @@ -131,10 +131,13 @@ public: { NSSTest::SetUp(); - Input rootNameDER; - // The result of ASCIIToDERName is owned by the arena - if (InitInputFromSECItem(ASCIIToDERName(arena.get(), rootName), - rootNameDER) != Success) { + rootNameDER = CNToDERName(rootName); + if (rootNameDER == ENCODING_FAILED) { + abort(); + } + Input rootNameDERInput; + if (rootNameDERInput.Init(rootNameDER.data(), rootNameDER.length()) + != Success) { abort(); } @@ -150,7 +153,7 @@ public: if (InitInputFromSECItem(rootSPKI.get(), rootSPKIDER) != Success) { abort(); } - endEntityCertID = new (std::nothrow) CertID(rootNameDER, rootSPKIDER, + endEntityCertID = new (std::nothrow) CertID(rootNameDERInput, rootSPKIDER, serialNumberDER); if (!endEntityCertID) { abort(); @@ -160,9 +163,10 @@ public: static ScopedSECKEYPrivateKey rootPrivateKey; static ScopedSECItem rootSPKI; static long rootIssuedCount; - OCSPTestTrustDomain trustDomain; - // endEntityCertID references items owned by arena and rootSPKI. + + ByteString rootNameDER; + // endEntityCertID references items owned by arena, rootSPKI, and rootNameDER. ScopedPtr endEntityCertID; }; @@ -269,8 +273,8 @@ public: { OCSPResponseContext context(arena.get(), certID, producedAt); if (signerName) { - context.signerNameDER = ASCIIToDERName(arena.get(), signerName); - EXPECT_TRUE(context.signerNameDER); + context.signerNameDER = CNToDERName(signerName); + EXPECT_NE(ENCODING_FAILED, context.signerNameDER); } context.signerPrivateKey = SECKEY_CopyPrivateKey(signerPrivateKey.get()); EXPECT_TRUE(context.signerPrivateKey); @@ -412,10 +416,10 @@ protected: signerDEROut->Init(signerDER->data, signerDER->len)); } - const SECItem* signerNameDER = nullptr; + ByteString signerNameDER; if (signerName) { - signerNameDER = ASCIIToDERName(arena.get(), signerName); - EXPECT_TRUE(signerNameDER); + signerNameDER = CNToDERName(signerName); + EXPECT_NE(ENCODING_FAILED, signerNameDER); } SECItem const* const certs[] = { signerDER, nullptr }; return CreateEncodedOCSPSuccessfulResponse(certStatus, *endEntityCertID, @@ -440,12 +444,12 @@ protected: if (!serialNumberDER) { return nullptr; } - const SECItem* issuerDER(ASCIIToDERName(arena, issuer)); - if (!issuerDER) { + ByteString issuerDER(CNToDERName(issuer)); + if (issuerDER == ENCODING_FAILED) { return nullptr; } - const SECItem* subjectDER(ASCIIToDERName(arena, subject)); - if (!subjectDER) { + ByteString subjectDER(CNToDERName(subject)); + if (subjectDER == ENCODING_FAILED) { return nullptr; } return ::mozilla::pkix::test::CreateEncodedCertificate( @@ -467,7 +471,7 @@ protected: TEST_F(pkixocsp_VerifyEncodedResponse_DelegatedResponder, good_byKey) { Input response(CreateEncodedIndirectOCSPSuccessfulResponse( - "CN=good_indirect_byKey", OCSPResponseContext::good, + "good_indirect_byKey", OCSPResponseContext::good, byKey)); bool expired; ASSERT_EQ(Success, @@ -480,8 +484,8 @@ TEST_F(pkixocsp_VerifyEncodedResponse_DelegatedResponder, good_byKey) TEST_F(pkixocsp_VerifyEncodedResponse_DelegatedResponder, good_byName) { Input response(CreateEncodedIndirectOCSPSuccessfulResponse( - "CN=good_indirect_byName", OCSPResponseContext::good, - "CN=good_indirect_byName")); + "good_indirect_byName", OCSPResponseContext::good, + "good_indirect_byName")); bool expired; ASSERT_EQ(Success, VerifyEncodedOCSPResponse(trustDomain, *endEntityCertID, Now(), @@ -518,7 +522,7 @@ TEST_F(pkixocsp_VerifyEncodedResponse_DelegatedResponder, missingSignerPrivateKey)); Input response(CreateEncodedOCSPSuccessfulResponse( OCSPResponseContext::good, *endEntityCertID, - "CN=missing", missingSignerPrivateKey, + "missing", missingSignerPrivateKey, oneDayBeforeNow, oneDayBeforeNow, nullptr)); bool expired; ASSERT_EQ(Result::ERROR_OCSP_INVALID_SIGNING_CERT, @@ -530,7 +534,7 @@ TEST_F(pkixocsp_VerifyEncodedResponse_DelegatedResponder, TEST_F(pkixocsp_VerifyEncodedResponse_DelegatedResponder, good_expired) { - static const char* signerName = "CN=good_indirect_expired"; + static const char* signerName = "good_indirect_expired"; const SECItem* extensions[] = { CreateEncodedEKUExtension(arena.get(), OCSPSigningEKUDER, @@ -562,7 +566,7 @@ TEST_F(pkixocsp_VerifyEncodedResponse_DelegatedResponder, good_expired) TEST_F(pkixocsp_VerifyEncodedResponse_DelegatedResponder, good_future) { - static const char* signerName = "CN=good_indirect_future"; + static const char* signerName = "good_indirect_future"; const SECItem* extensions[] = { CreateEncodedEKUExtension(arena.get(), OCSPSigningEKUDER, @@ -596,7 +600,7 @@ TEST_F(pkixocsp_VerifyEncodedResponse_DelegatedResponder, good_future) TEST_F(pkixocsp_VerifyEncodedResponse_DelegatedResponder, good_no_eku) { Input response(CreateEncodedIndirectOCSPSuccessfulResponse( - "CN=good_indirect_wrong_eku", + "good_indirect_wrong_eku", OCSPResponseContext::good, byKey, nullptr)); bool expired; ASSERT_EQ(Result::ERROR_OCSP_INVALID_SIGNING_CERT, @@ -612,9 +616,8 @@ TEST_F(pkixocsp_VerifyEncodedResponse_DelegatedResponder, good_indirect_wrong_eku) { Input response(CreateEncodedIndirectOCSPSuccessfulResponse( - "CN=good_indirect_wrong_eku", - OCSPResponseContext::good, byKey, - &serverAuthEKUDER)); + "good_indirect_wrong_eku", + OCSPResponseContext::good, byKey, &serverAuthEKUDER)); bool expired; ASSERT_EQ(Result::ERROR_OCSP_INVALID_SIGNING_CERT, VerifyEncodedOCSPResponse(trustDomain, *endEntityCertID, Now(), @@ -627,7 +630,7 @@ TEST_F(pkixocsp_VerifyEncodedResponse_DelegatedResponder, TEST_F(pkixocsp_VerifyEncodedResponse_DelegatedResponder, good_tampered_eku) { Input response(CreateEncodedIndirectOCSPSuccessfulResponse( - "CN=good_indirect_tampered_eku", + "good_indirect_tampered_eku", OCSPResponseContext::good, byKey, &serverAuthEKUDER)); ByteString tamperedResponse(response.UnsafeGetData(), @@ -651,8 +654,8 @@ TEST_F(pkixocsp_VerifyEncodedResponse_DelegatedResponder, good_tampered_eku) TEST_F(pkixocsp_VerifyEncodedResponse_DelegatedResponder, good_unknown_issuer) { - static const char* subCAName = "CN=good_indirect_unknown_issuer sub-CA"; - static const char* signerName = "CN=good_indirect_unknown_issuer OCSP signer"; + static const char* subCAName = "good_indirect_unknown_issuer sub-CA"; + static const char* signerName = "good_indirect_unknown_issuer OCSP signer"; // unknown issuer ScopedSECKEYPublicKey unknownPublicKey; @@ -692,8 +695,8 @@ TEST_F(pkixocsp_VerifyEncodedResponse_DelegatedResponder, good_unknown_issuer) TEST_F(pkixocsp_VerifyEncodedResponse_DelegatedResponder, good_indirect_subca_1_first) { - static const char* subCAName = "CN=good_indirect_subca_1_first sub-CA"; - static const char* signerName = "CN=good_indirect_subca_1_first OCSP signer"; + static const char* subCAName = "good_indirect_subca_1_first sub-CA"; + static const char* signerName = "good_indirect_subca_1_first OCSP signer"; // sub-CA of root (root is the direct issuer of endEntity) const SECItem* subCAExtensions[] = { @@ -745,8 +748,8 @@ TEST_F(pkixocsp_VerifyEncodedResponse_DelegatedResponder, TEST_F(pkixocsp_VerifyEncodedResponse_DelegatedResponder, good_indirect_subca_1_second) { - static const char* subCAName = "CN=good_indirect_subca_1_second sub-CA"; - static const char* signerName = "CN=good_indirect_subca_1_second OCSP signer"; + static const char* subCAName = "good_indirect_subca_1_second sub-CA"; + static const char* signerName = "good_indirect_subca_1_second OCSP signer"; // sub-CA of root (root is the direct issuer of endEntity) const SECItem* subCAExtensions[] = { @@ -802,7 +805,7 @@ public: Input createdResponse( CreateEncodedIndirectOCSPSuccessfulResponse( - "CN=OCSPGetCertTrustTest Signer", OCSPResponseContext::good, + "OCSPGetCertTrustTest Signer", OCSPResponseContext::good, byKey, &OCSPSigningEKUDER, &signerCertDER)); if (response.Init(createdResponse) != Success) { abort(); diff --git a/security/pkix/test/lib/pkixtestutil.cpp b/security/pkix/test/lib/pkixtestutil.cpp index 8e8fba0af30b..767e705a6162 100644 --- a/security/pkix/test/lib/pkixtestutil.cpp +++ b/security/pkix/test/lib/pkixtestutil.cpp @@ -122,6 +122,33 @@ 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 +TLV(uint8_t tag, const ByteString& value) +{ + ByteString result; + result.push_back(tag); + + if (value.length() < 128) { + result.push_back(value.length()); + } else if (value.length() < 256) { + result.push_back(0x81u); + result.push_back(value.length()); + } else if (value.length() < 65536) { + result.push_back(0x82u); + result.push_back(static_cast(value.length() / 256)); + result.push_back(static_cast(value.length() % 256)); + } else { + assert(false); + return ENCODING_FAILED; + } + result.append(value); + return result; +} + Result InitInputFromSECItem(const SECItem* secItem, /*out*/ Input& input) { @@ -135,8 +162,6 @@ class Output { public: Output() - : numItems(0) - , length(0) { } @@ -146,40 +171,32 @@ public: { assert(item); assert(item->data); + contents.append(item->data, item->len); + return Success; // XXX: return type should be void + } - if (numItems >= MaxSequenceItems) { - return Result::FATAL_ERROR_INVALID_ARGS; - } - if (length + item->len > 65535) { - return Result::FATAL_ERROR_INVALID_ARGS; - } - - contents[numItems] = item; - numItems++; - length += item->len; - return Success; + void Add(const ByteString& item) + { + contents.append(item); } SECItem* Squash(PLArenaPool* arena, uint8_t tag) { assert(arena); - size_t lengthLength = length < 128 ? 1 - : length < 256 ? 2 - : 3; - size_t totalLength = 1 + lengthLength + length; + size_t lengthLength = contents.length() < 128 ? 1 + : contents.length() < 256 ? 2 + : 3; + size_t totalLength = 1 + lengthLength + contents.length(); SECItem* output = SECITEM_AllocItem(arena, nullptr, totalLength); if (!output) { return nullptr; } uint8_t* d = output->data; *d++ = tag; - EncodeLength(d, length, lengthLength); + EncodeLength(d, contents.length(), lengthLength); d += lengthLength; - for (size_t i = 0; i < numItems; i++) { - memcpy(d, contents[i]->data, contents[i]->len); - d += contents[i]->len; - } + memcpy(d, contents.data(), contents.length()); return output; } @@ -205,13 +222,7 @@ private: } } - static const size_t MaxSequenceItems = 10; - const SECItem* contents[MaxSequenceItems]; - size_t numItems; - size_t length; - - Output(const Output&) /* = delete */; - void operator=(const Output&) /* = delete */; + ByteString contents; }; OCSPResponseContext::OCSPResponseContext(PLArenaPool* arena, @@ -220,7 +231,6 @@ OCSPResponseContext::OCSPResponseContext(PLArenaPool* arena, , certID(certID) , responseStatus(successful) , skipResponseBytes(false) - , signerNameDER(nullptr) , producedAt(time) , extensions(nullptr) , includeEmptyExtensions(false) @@ -238,8 +248,8 @@ OCSPResponseContext::OCSPResponseContext(PLArenaPool* arena, static SECItem* ResponseBytes(OCSPResponseContext& context); static SECItem* BasicOCSPResponse(OCSPResponseContext& context); static SECItem* ResponseData(OCSPResponseContext& context); -static SECItem* ResponderID(OCSPResponseContext& context); -static SECItem* KeyHash(OCSPResponseContext& context); +static ByteString ResponderID(OCSPResponseContext& context); +static ByteString KeyHash(OCSPResponseContext& context); static SECItem* SingleResponse(OCSPResponseContext& context); static SECItem* CertID(OCSPResponseContext& context); static SECItem* CertStatus(OCSPResponseContext& context); @@ -254,32 +264,27 @@ EncodeNested(PLArenaPool* arena, uint8_t tag, const SECItem* inner) return output.Squash(arena, tag); } -static SECItem* -HashedOctetString(PLArenaPool* arena, const SECItem& bytes) +static ByteString +HashedOctetString(const SECItem& bytes) { - SECItem* hashBuf = SECITEM_AllocItem(arena, nullptr, SHA1_LENGTH); - if (!hashBuf) { - return nullptr; - } - + uint8_t hashBuf[TrustDomain::DIGEST_LENGTH]; Input input; if (input.Init(bytes.data, bytes.len) != Success) { - return nullptr; + return ENCODING_FAILED; } - if (DigestBuf(input, hashBuf->data, hashBuf->len) != Success) { - return nullptr; + if (DigestBuf(input, hashBuf, sizeof(hashBuf)) != Success) { + return ENCODING_FAILED; } - - return EncodeNested(arena, der::OCTET_STRING, hashBuf); + return TLV(der::OCTET_STRING, ByteString(hashBuf, sizeof(hashBuf))); } -static SECItem* -KeyHashHelper(PLArenaPool* arena, const CERTSubjectPublicKeyInfo* spki) +static ByteString +KeyHashHelper(const CERTSubjectPublicKeyInfo* spki) { // We only need a shallow copy here. SECItem spk = spki->subjectPublicKey; DER_ConvertBitString(&spk); // bits to bytes - return HashedOctetString(arena, spk); + return HashedOctetString(spk); } static SECItem* @@ -695,8 +700,8 @@ GenerateKeyPair(/*out*/ ScopedSECKEYPublicKey& publicKey, static SECItem* TBSCertificate(PLArenaPool* arena, long version, const SECItem* serialNumber, Input signature, - const SECItem* issuer, time_t notBefore, - time_t notAfter, const SECItem* subject, + const ByteString& issuer, time_t notBefore, + time_t notAfter, const ByteString& subject, const SECKEYPublicKey* subjectPublicKey, /*optional*/ SECItem const* const* extensions); @@ -705,19 +710,18 @@ static SECItem* TBSCertificate(PLArenaPool* arena, long version, // signatureAlgorithm AlgorithmIdentifier, // signatureValue BIT STRING } SECItem* -CreateEncodedCertificate(PLArenaPool* arena, long version, - Input signature, const SECItem* serialNumber, - const SECItem* issuerNameDER, time_t notBefore, - time_t notAfter, const SECItem* subjectNameDER, +CreateEncodedCertificate(PLArenaPool* arena, long version, Input signature, + const SECItem* serialNumber, + const ByteString& issuerNameDER, + time_t notBefore, time_t notAfter, + const ByteString& subjectNameDER, /*optional*/ SECItem const* const* extensions, /*optional*/ SECKEYPrivateKey* issuerPrivateKey, SignatureAlgorithm signatureAlgorithm, /*out*/ ScopedSECKEYPrivateKey& privateKeyResult) { assert(arena); - assert(issuerNameDER); - assert(subjectNameDER); - if (!arena || !issuerNameDER || !subjectNameDER) { + if (!arena) { return nullptr; } @@ -768,16 +772,14 @@ CreateEncodedCertificate(PLArenaPool* arena, long version, static SECItem* TBSCertificate(PLArenaPool* arena, long versionValue, const SECItem* serialNumber, Input signature, - const SECItem* issuer, time_t notBeforeTime, - time_t notAfterTime, const SECItem* subject, + const ByteString& issuer, time_t notBeforeTime, + time_t notAfterTime, const ByteString& subject, const SECKEYPublicKey* subjectPublicKey, /*optional*/ SECItem const* const* extensions) { assert(arena); - assert(issuer); - assert(subject); assert(subjectPublicKey); - if (!arena || !issuer || !subject || !subjectPublicKey) { + if (!arena || !subjectPublicKey) { return nullptr; } @@ -808,9 +810,7 @@ TBSCertificate(PLArenaPool* arena, long versionValue, return nullptr; } - if (output.Add(issuer) != Success) { - return nullptr; - } + output.Add(issuer); // Validity ::= SEQUENCE { // notBefore Time, @@ -841,9 +841,7 @@ TBSCertificate(PLArenaPool* arena, long versionValue, return nullptr; } - if (output.Add(subject) != Success) { - return nullptr; - } + output.Add(subject); // SubjectPublicKeyInfo ::= SEQUENCE { // algorithm AlgorithmIdentifier, @@ -883,15 +881,57 @@ TBSCertificate(PLArenaPool* arena, long versionValue, return output.Squash(arena, der::SEQUENCE); } -const SECItem* -ASCIIToDERName(PLArenaPool* arena, const char* cn) +ByteString +CNToDERName(const char* cn) { - ScopedPtr certName(CERT_AsciiToName(cn)); - if (!certName) { - return nullptr; + // Name ::= CHOICE { -- only one possibility for now -- + // rdnSequence RDNSequence } + // + // RDNSequence ::= SEQUENCE OF RelativeDistinguishedName + // + // RelativeDistinguishedName ::= + // SET SIZE (1..MAX) OF AttributeTypeAndValue + // + // AttributeTypeAndValue ::= SEQUENCE { + // type AttributeType, + // value AttributeValue } + // + // AttributeType ::= OBJECT IDENTIFIER + // + // AttributeValue ::= ANY -- DEFINED BY AttributeType + // + // DirectoryString ::= CHOICE { + // teletexString TeletexString (SIZE (1..MAX)), + // printableString PrintableString (SIZE (1..MAX)), + // universalString UniversalString (SIZE (1..MAX)), + // utf8String UTF8String (SIZE (1..MAX)), + // bmpString BMPString (SIZE (1..MAX)) } + // + // id-at OBJECT IDENTIFIER ::= { joint-iso-ccitt(2) ds(5) 4 } + // id-at-commonName AttributeType ::= { id-at 3 } + + // python DottedOIDToCode.py --tlv id-at-commonName 2.5.4.3 + static const uint8_t tlv_id_at_commonName[] = { + 0x06, 0x03, 0x55, 0x04, 0x03 + }; + + ByteString value(reinterpret_cast(cn)); + value = TLV(der::UTF8String, value); + + ByteString ava; + 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; } - return SEC_ASN1EncodeItem(arena, nullptr, certName.get(), - SEC_ASN1_GET(CERT_NameTemplate)); + + ByteString rdn(TLV(der::SET, ava)); + if (rdn == ENCODING_FAILED) { + return ENCODING_FAILED; + } + + return TLV(der::SEQUENCE, rdn); } SECItem* @@ -1152,8 +1192,8 @@ Extensions(OCSPResponseContext& context) SECItem* ResponseData(OCSPResponseContext& context) { - SECItem* responderID = ResponderID(context); - if (!responderID) { + ByteString responderID(ResponderID(context)); + if (responderID == ENCODING_FAILED) { return nullptr; } SECItem* producedAtEncoded = TimeToGeneralizedTime(context.arena, @@ -1176,9 +1216,7 @@ ResponseData(OCSPResponseContext& context) } Output output; - if (output.Add(responderID) != Success) { - return nullptr; - } + output.Add(responderID); if (output.Add(producedAtEncoded) != Success) { return nullptr; } @@ -1197,27 +1235,24 @@ ResponseData(OCSPResponseContext& context) // byName [1] Name, // byKey [2] KeyHash } // } -SECItem* +ByteString ResponderID(OCSPResponseContext& context) { - const SECItem* contents; + ByteString contents; uint8_t responderIDType; - if (context.signerNameDER) { + if (!context.signerNameDER.empty()) { contents = context.signerNameDER; responderIDType = 1; // byName } else { contents = KeyHash(context); + if (contents == ENCODING_FAILED) { + return ENCODING_FAILED; + } responderIDType = 2; // byKey } - if (!contents) { - return nullptr; - } - return EncodeNested(context.arena, - der::CONSTRUCTED | - der::CONTEXT_SPECIFIC | - responderIDType, - contents); + return TLV(der::CONSTRUCTED | der::CONTEXT_SPECIFIC | responderIDType, + contents); } // KeyHash ::= OCTET STRING -- SHA-1 hash of responder's public key @@ -1225,7 +1260,7 @@ ResponderID(OCSPResponseContext& context) // -- BIT STRING subjectPublicKey [excluding // -- the tag, length, and number of unused // -- bits] in the responder's certificate) -SECItem* +ByteString KeyHash(OCSPResponseContext& context) { ScopedSECKEYPublicKey @@ -1238,7 +1273,7 @@ KeyHash(OCSPResponseContext& context) if (!signerSPKI) { return nullptr; } - return KeyHashHelper(context.arena, signerSPKI.get()); + return KeyHashHelper(signerSPKI.get()); } // SingleResponse ::= SEQUENCE { @@ -1307,8 +1342,8 @@ SECItem* CertID(OCSPResponseContext& context) { SECItem issuerSECItem = UnsafeMapInputToSECItem(context.certID.issuer); - SECItem* issuerNameHash = HashedOctetString(context.arena, issuerSECItem); - if (!issuerNameHash) { + ByteString issuerNameHash(HashedOctetString(issuerSECItem)); + if (issuerNameHash == ENCODING_FAILED) { return nullptr; } @@ -1320,8 +1355,8 @@ CertID(OCSPResponseContext& context) if (!spki) { return nullptr; } - SECItem* issuerKeyHash(KeyHashHelper(context.arena, spki.get())); - if (!issuerKeyHash) { + ByteString issuerKeyHash(KeyHashHelper(spki.get())); + if (issuerKeyHash == ENCODING_FAILED) { return nullptr; } @@ -1353,12 +1388,8 @@ CertID(OCSPResponseContext& context) if (output.Add(&id_sha1) != Success) { return nullptr; } - if (output.Add(issuerNameHash) != Success) { - return nullptr; - } - if (output.Add(issuerKeyHash) != Success) { - return nullptr; - } + output.Add(issuerNameHash); + output.Add(issuerKeyHash); if (output.Add(serialNumber) != Success) { return nullptr; } diff --git a/security/pkix/test/lib/pkixtestutil.h b/security/pkix/test/lib/pkixtestutil.h index d25d98f854df..03421526a162 100644 --- a/security/pkix/test/lib/pkixtestutil.h +++ b/security/pkix/test/lib/pkixtestutil.h @@ -38,6 +38,7 @@ namespace mozilla { namespace pkix { namespace test { typedef std::basic_string ByteString; +extern const ByteString ENCODING_FAILED; // XXX: Ideally, we should define this instead: // @@ -99,8 +100,7 @@ mozilla::pkix::Time YMDHMS(int16_t year, int16_t month, int16_t day, Result GenerateKeyPair(/*out*/ ScopedSECKEYPublicKey& publicKey, /*out*/ ScopedSECKEYPrivateKey& privateKey); -// The result will be owned by the arena -const SECItem* ASCIIToDERName(PLArenaPool* arena, const char* cn); +ByteString CNToDERName(const char* cn); // Replace one substring in item with another of the same length, but only if // the substring was found exactly once. The "same length" restriction is @@ -136,9 +136,9 @@ enum Version { v1 = 0, v2 = 1, v3 = 2 }; SECItem* CreateEncodedCertificate(PLArenaPool* arena, long version, Input signature, const SECItem* serialNumber, - const SECItem* issuerNameDER, + const ByteString& issuerNameDER, std::time_t notBefore, std::time_t notAfter, - const SECItem* subjectNameDER, + const ByteString& subjectNameDER, /*optional*/ SECItem const* const* extensions, /*optional*/ SECKEYPrivateKey* issuerPrivateKey, SignatureAlgorithm signatureAlgorithm, @@ -194,9 +194,9 @@ public: bool skipResponseBytes; // If true, don't include responseBytes // responderID - const SECItem* signerNameDER; // If set, responderID will use the byName - // form; otherwise responderID will use the - // byKeyHash form. + ByteString signerNameDER; // If set, responderID will use the byName + // form; otherwise responderID will use the + // byKeyHash form. std::time_t producedAt;