Require non-NULL store in X509_STORE_CTX_init.
X509_STORE_CTX_init is documented upstream to allow a NULL store and has
logic to account for it. However, attempting to use such an
X509_STORE_CTX crashes in X509_verify_cert due to the
additional_untrusted logic we added.
Moreover, before that change, it still crashes because
X509_STORE_CTX_get1_issuer (the default get_issuer hook) assumes
ctx->ctx (the store) is non-null. This was also true in upstream but
later fixed in https://github.com/openssl/openssl/pull/6001. However,
without a store, there is no trust anchor, so this is not very useful.
Reject NULL stores in X509_STORE_CTX_init and remove the logic allowing
for a NULL one.
Thanks to Danny Halawi for catching this.
Update-Note: X509_STORE_CTX_init will now fail when the store is NULL,
rather than report success, only to crash later in X509_verify_cert.
Breakage should thus be limited to code which was passing in a NULL
store but never used the resulting X509_STORE_CTX.
Change-Id: I9db0289612cc245a8d62d6fa647d6b56b2daabda
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/42728
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
diff --git a/crypto/x509/x509_test.cc b/crypto/x509/x509_test.cc
index d71dee9..426e181 100644
--- a/crypto/x509/x509_test.cc
+++ b/crypto/x509/x509_test.cc
@@ -2445,3 +2445,13 @@
EXPECT_FALSE(CertFromPEM(kV1WithIssuerUniqueIDPEM));
EXPECT_FALSE(CertFromPEM(kV1WithSubjectUniqueIDPEM));
}
+
+// Unlike upstream OpenSSL, we require a non-null store in
+// |X509_STORE_CTX_init|.
+TEST(X509Test, NullStore) {
+ bssl::UniquePtr<X509> leaf(CertFromPEM(kLeafPEM));
+ ASSERT_TRUE(leaf);
+ bssl::UniquePtr<X509_STORE_CTX> ctx(X509_STORE_CTX_new());
+ ASSERT_TRUE(ctx);
+ EXPECT_FALSE(X509_STORE_CTX_init(ctx.get(), nullptr, leaf.get(), nullptr));
+}
