f97c316437be30468006e1278006f29057a4aa1c - chromium/src

[ma...@chromium.org]

commit f97c316437be30468006e1278006f29057a4aa1c
Author: Matt Mueller <ma...@chromium.org>
AuthorDate: Wed Mar 02 00:40:18 2022
Commit: Chromium LUCI CQ <chromiu...@luci-project-accounts.iam.gserviceaccount.com>
CommitDate: Wed Mar 02 00:40:18 2022

X509UtilTest.CreateSecCertificateArrayForX509CertificateErrors: update and re-enable

After cfa2b8c65afd1d0474da0c84d4df8c61ca1d70f9,
CreateSecCertificateFromBytes on macOS no longer forces lazy parsing of
SecCertificate objects to occur. Relax the test so that on older macOS
versions where lazy parsing is still a thing, the test will pass if a
SecCertificate is returned for the invalid input.

Bug: 1301978
Change-Id: If9a3f011d33beb840fed11f1477736478e47ba2c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3498904
Auto-Submit: Matt Mueller <ma...@chromium.org>
Reviewed-by: David Benjamin <davi...@chromium.org>
Commit-Queue: David Benjamin <davi...@chromium.org>
Cr-Commit-Position: refs/heads/main@{#976483}

diff --git a/net/cert/x509_util_ios_and_mac_unittest.cc b/net/cert/x509_util_ios_and_mac_unittest.cc
index 0a4dc6b..34dcfed 100644
--- a/net/cert/x509_util_ios_and_mac_unittest.cc
+++ b/net/cert/x509_util_ios_and_mac_unittest.cc
@@ -69,7 +69,7 @@
BytesForSecCert(CFArrayGetValueAtIndex(sec_certs.get(), 3)));
}

-TEST(X509UtilTest, DISABLED_CreateSecCertificateArrayForX509CertificateErrors) {
+TEST(X509UtilTest, CreateSecCertificateArrayForX509CertificateErrors) {
scoped_refptr<X509Certificate> ok_cert(
ImportCertFromFile(GetTestCertsDirectory(), "ok_cert.pem"));
ASSERT_TRUE(ok_cert);
@@ -83,7 +83,7 @@
ASSERT_TRUE(ok_cert);

std::vector<bssl::UniquePtr<CRYPTO_BUFFER>> intermediates;
- intermediates.push_back(std::move(bad_cert));
+ intermediates.push_back(bssl::UpRef(bad_cert));
intermediates.push_back(bssl::UpRef(ok_cert2->cert_buffer()));
scoped_refptr<X509Certificate> cert_with_intermediates(
X509Certificate::CreateFromBuffer(bssl::UpRef(ok_cert->cert_buffer()),
@@ -91,25 +91,46 @@
ASSERT_TRUE(cert_with_intermediates);
EXPECT_EQ(2U, cert_with_intermediates->intermediate_buffers().size());

- // Normal CreateSecCertificateArrayForX509Certificate fails with invalid
- // certs in chain.
- EXPECT_FALSE(CreateSecCertificateArrayForX509Certificate(
- cert_with_intermediates.get()));
-
// With InvalidIntermediateBehavior::kIgnore, invalid intermediate certs
// should be silently dropped.
base::ScopedCFTypeRef<CFMutableArrayRef> sec_certs(
CreateSecCertificateArrayForX509Certificate(
cert_with_intermediates.get(), InvalidIntermediateBehavior::kIgnore));
ASSERT_TRUE(sec_certs);
- ASSERT_EQ(2, CFArrayGetCount(sec_certs.get()));
- for (int i = 0; i < 2; ++i)
+ for (int i = 0; i < CFArrayGetCount(sec_certs.get()); ++i)
ASSERT_TRUE(CFArrayGetValueAtIndex(sec_certs.get(), i));

- EXPECT_EQ(x509_util::CryptoBufferAsStringPiece(ok_cert->cert_buffer()),
- BytesForSecCert(CFArrayGetValueAtIndex(sec_certs.get(), 0)));
- EXPECT_EQ(x509_util::CryptoBufferAsStringPiece(ok_cert2->cert_buffer()),
- BytesForSecCert(CFArrayGetValueAtIndex(sec_certs.get(), 1)));
+ if (CFArrayGetCount(sec_certs.get()) == 2) {
+ EXPECT_EQ(x509_util::CryptoBufferAsStringPiece(ok_cert->cert_buffer()),
+ BytesForSecCert(CFArrayGetValueAtIndex(sec_certs.get(), 0)));
+ EXPECT_EQ(x509_util::CryptoBufferAsStringPiece(ok_cert2->cert_buffer()),
+ BytesForSecCert(CFArrayGetValueAtIndex(sec_certs.get(), 1)));
+
+ // Normal CreateSecCertificateArrayForX509Certificate should fail with
+ // invalid certs in chain.
+ EXPECT_FALSE(CreateSecCertificateArrayForX509Certificate(
+ cert_with_intermediates.get()));
+ } else if (CFArrayGetCount(sec_certs.get()) == 3) {
+ // On older macOS versions that do lazy parsing of SecCertificates, the
+ // invalid certificate may be accepted, which is okay. The test is just
+ // verifying that *if* creating a SecCertificate from one of the
+ // intermediates fails, that cert is ignored and the other certs are still
+ // returned.
+ EXPECT_EQ(x509_util::CryptoBufferAsStringPiece(ok_cert->cert_buffer()),
+ BytesForSecCert(CFArrayGetValueAtIndex(sec_certs.get(), 0)));
+ EXPECT_EQ(x509_util::CryptoBufferAsStringPiece(bad_cert.get()),
+ BytesForSecCert(CFArrayGetValueAtIndex(sec_certs.get(), 1)));
+ EXPECT_EQ(x509_util::CryptoBufferAsStringPiece(ok_cert2->cert_buffer()),
+ BytesForSecCert(CFArrayGetValueAtIndex(sec_certs.get(), 2)));
+
+ // Normal CreateSecCertificateArrayForX509Certificate should also
+ // succeed in this case.
+ EXPECT_TRUE(CreateSecCertificateArrayForX509Certificate(
+ cert_with_intermediates.get()));
+ } else {
+ ADD_FAILURE() << "CFArrayGetCount(sec_certs.get()) = "
+ << CFArrayGetCount(sec_certs.get());
+ }
}

TEST(X509UtilTest,