Skip to content

Commit

Permalink
Align X509 PARTIAL_CHAIN behavior with 1.1.1
Browse files Browse the repository at this point in the history
  • Loading branch information
samuel40791765 committed Oct 14, 2024
1 parent 9ff8458 commit d0d4542
Show file tree
Hide file tree
Showing 2 changed files with 60 additions and 0 deletions.
46 changes: 46 additions & 0 deletions crypto/x509/x509_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1702,6 +1702,52 @@ TEST(X509Test, TestVerify) {
}
}

TEST(X509Test, PartialChain) {
bssl::UniquePtr<X509> root(CertFromPEM(kRootCAPEM));
bssl::UniquePtr<X509> intermediate(CertFromPEM(kIntermediatePEM));
bssl::UniquePtr<X509> 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<STACK_OF(X509)> intermediates_stack(CertsToStack({}));
bssl::UniquePtr<STACK_OF(X509)> roots_stack(
CertsToStack({intermediate.get(), root.get()}));

for (bool partial_chain : {true, false}) {
SCOPED_TRACE(partial_chain);
bssl::UniquePtr<X509_STORE_CTX> ctx(X509_STORE_CTX_new());
bssl::UniquePtr<X509_STORE> 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) {
Expand Down
14 changes: 14 additions & 0 deletions crypto/x509/x509_vfy.c
Original file line number Diff line number Diff line change
Expand Up @@ -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++;
}

Expand Down

0 comments on commit d0d4542

Please sign in to comment.