diff --git a/crypto/x509/x509_test.cc b/crypto/x509/x509_test.cc index e09ea2ad83..178edde9d5 100644 --- a/crypto/x509/x509_test.cc +++ b/crypto/x509/x509_test.cc @@ -1702,6 +1702,52 @@ TEST(X509Test, TestVerify) { } } +TEST(X509Test, PartialChain) { + bssl::UniquePtr root(CertFromPEM(kRootCAPEM)); + bssl::UniquePtr intermediate(CertFromPEM(kIntermediatePEM)); + bssl::UniquePtr leaf(CertFromPEM(kLeafPEM)); + ASSERT_TRUE(root); + ASSERT_TRUE(intermediate); + ASSERT_TRUE(leaf); + + // We're intentionally placing the intermediate cert in the trust store here. + // Many TLS implementations set |X509_V_FLAG_PARTIAL_CHAIN|, which allows + // non-self-signed certificates in the trust store to be trusted. + // See https://github.com/openssl/openssl/issues/7871. + bssl::UniquePtr intermediates_stack(CertsToStack({})); + bssl::UniquePtr roots_stack( + CertsToStack({intermediate.get(), root.get()})); + + for (bool partial_chain : {true, false}) { + SCOPED_TRACE(partial_chain); + bssl::UniquePtr ctx(X509_STORE_CTX_new()); + bssl::UniquePtr store(X509_STORE_new()); + ASSERT_TRUE(ctx); + ASSERT_TRUE(store); + + ASSERT_TRUE(X509_STORE_CTX_init(ctx.get(), store.get(), leaf.get(), + intermediates_stack.get())); + X509_STORE_CTX_set0_trusted_stack(ctx.get(), roots_stack.get()); + + X509_VERIFY_PARAM *param = X509_STORE_CTX_get0_param(ctx.get()); + time_t current_time = time(nullptr); + X509_VERIFY_PARAM_set_time_posix(param, current_time); + + if (partial_chain) { + X509_VERIFY_PARAM_set_flags(param, X509_V_FLAG_PARTIAL_CHAIN); + } + + EXPECT_EQ(X509_verify_cert(ctx.get()), 1); + + STACK_OF(X509) *chain = X509_STORE_CTX_get0_chain(ctx.get()); + ASSERT_TRUE(chain); + + // |root| will be included in the chain if |X509_V_FLAG_PARTIAL_CHAIN| is + // not set. + EXPECT_EQ(sk_X509_num(chain), partial_chain ? 2u : 3u); + } +} + #if defined(OPENSSL_THREADS) // Verifying the same |X509| objects on two threads should be safe. TEST(X509Test, VerifyThreads) { diff --git a/crypto/x509/x509_vfy.c b/crypto/x509/x509_vfy.c index 8fdd82cd92..75773131ea 100644 --- a/crypto/x509/x509_vfy.c +++ b/crypto/x509/x509_vfy.c @@ -362,6 +362,20 @@ int X509_verify_cert(X509_STORE_CTX *ctx) { ok = 0; goto end; } + + // OpenSSL 1.1.1 continuously re-checks for trust and breaks the loop + // as soon as trust has been established. 1.0.2 builds the chain with all + // possible certs first and only checks for trust if the final cert in + // the chain is self-signed. + // This caused additional unanticipated certs to be in the established + // certificate chain, particularly when |X509_V_FLAG_PARTIAL_CHAIN| was + // set. We try checking continuously for trust here for better 1.1.1 + // compatibility. + trust = check_trust(ctx); + if (trust == X509_TRUST_TRUSTED || trust == X509_TRUST_REJECTED) { + break; + } + num++; }