Regression test for I7cdf6cbdbd5f07093cdf3c191bfbd5c46a6a6964.

Bug: 495832128
Change-Id: Id95713074dd97cee8fa8fff78e59e0666a6a6964
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/91528
Presubmit-BoringSSL-Verified: Boringssl LUCI CQ <boringssl-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: Rudolf Polzer <rpolzer@google.com>

crypto/x509/x509_test.cc

diff --git a/crypto/x509/x509_test.cc b/crypto/x509/x509_test.cc
index d0badfb..8d41872 100644
--- a/crypto/x509/x509_test.cc
+++ b/crypto/x509/x509_test.cc
@@ -9728,5 +9728,76 @@
   EXPECT_EQ(param_type, V_ASN1_NULL);
 }
 
+TEST(X509Test, X509StoreGet1IssuerMultipleMatches) {
+  // |kLeafPEM| is signed by |kIntermediatePEM|.
+  UniquePtr<X509> cert = CertFromPEM(kLeafPEM);
+  ASSERT_TRUE(cert);
+
+  // Get an intermediate certificate that will match |cert|.
+  UniquePtr<X509> issuer_ok = CertFromPEM(kIntermediatePEM);
+  ASSERT_TRUE(issuer_ok);
+
+  // |issuer_fail| has the same name but a different SKID, so it won't match the
+  // AKID in |cert|, thereby failing validation in
+  // |x509_check_issued_with_callback| via |X509_check_issued|.
+  UniquePtr<X509> issuer_fail(X509_dup(issuer_ok.get()));
+  ASSERT_TRUE(issuer_fail);
+  int loc =
+      X509_get_ext_by_NID(issuer_fail.get(), NID_subject_key_identifier, -1);
+  ASSERT_GE(loc, 0);
+  UniquePtr<X509_EXTENSION> ext(X509_delete_ext(issuer_fail.get(), loc));
+  ASSERT_TRUE(ext);
+
+  // Add a different SKID. 00 sorts before the original.
+  const uint8_t kTag[] = {0x00, 0x00, 0x00};
+  UniquePtr<ASN1_OCTET_STRING> skid(ASN1_OCTET_STRING_new());
+  ASSERT_TRUE(skid);
+  ASSERT_TRUE(ASN1_OCTET_STRING_set(skid.get(), kTag, sizeof(kTag)));
+  ASSERT_TRUE(X509_add1_ext_i2d(issuer_fail.get(), NID_subject_key_identifier,
+                                skid.get(), 0, 0));
+
+  // Sign it. The certificate will be rejected anyway, but it must be signed by
+  // _some_ key to be able to be added - otherwise its |cert_hash| will match
+  // |issuer_ok|'s and |X509_STORE_add_cert| will not even store it.
+  UniquePtr<EVP_PKEY> key = PrivateKeyFromPEM(kRSAKey);
+  ASSERT_TRUE(key);
+  ASSERT_TRUE(X509_sign(issuer_fail.get(), key.get(), EVP_sha256()));
+
+  // Find an unrelated certificate that sorts _before_ the others.
+  // "O=BoringSSL TESTING, CN=Root CA" sorts before "O=BoringSSL TESTING,
+  // CN=Intermediate CA" because it is _shorter_. See |X509_NAME_cmp|.
+  UniquePtr<X509> unrelated_before = CertFromPEM(kRootCAPEM);
+  ASSERT_TRUE(unrelated_before);
+
+  // Create a store, adding |unrelated_before|, then |issuer_fail|, then
+  // |issuer_ok|. |X509_OBJECT_idx_by_subject| will get |issuer_fail|.
+  UniquePtr<X509_STORE> store(X509_STORE_new());
+  ASSERT_TRUE(store);
+  ASSERT_TRUE(X509_STORE_add_cert(store.get(), unrelated_before.get()));
+  ASSERT_TRUE(X509_STORE_add_cert(store.get(), issuer_fail.get()));
+  ASSERT_TRUE(X509_STORE_add_cert(store.get(), issuer_ok.get()));
+  UniquePtr<X509_STORE_CTX> ctx(X509_STORE_CTX_new());
+  ASSERT_TRUE(ctx);
+  ASSERT_TRUE(X509_STORE_CTX_init(ctx.get(), store.get(), cert.get(), nullptr));
+
+  // Validate that a lookup by issuer name will return |issuer_fail|.
+  X509_NAME *xn = X509_get_issuer_name(cert.get());
+  X509_OBJECT obj;
+  ASSERT_EQ(1,
+            X509_STORE_CTX_get_by_subject(ctx.get(), X509_LU_X509, xn, &obj));
+  ASSERT_EQ(0, X509_cmp(X509_OBJECT_get0_X509(&obj), issuer_fail.get()));
+  X509_OBJECT_free_contents(&obj);
+
+  // Check that by actually looking up using the certificate and not just its
+  // issuer name, we can do better and get |issuer_ok|.
+  X509 *found_issuer = nullptr;
+  int get1_ret =
+      X509_STORE_CTX_get1_issuer(&found_issuer, ctx.get(), cert.get());
+  ASSERT_EQ(1, get1_ret);
+  ASSERT_TRUE(found_issuer);
+  EXPECT_EQ(0, X509_cmp(found_issuer, issuer_ok.get()));
+  X509_free(found_issuer);
+}
+
 }  // namespace
 BSSL_NAMESPACE_END