Skip to content

Commit 9fbfa70

Browse files
Align X509 PARTIAL_CHAIN behavior with 1.1.1 (aws#1917)
Some consumers noticed that a behavior difference between AWS-LC and OpenSSL when the trust store contains certificates that are issued by other certificates that are also in the trust store. A common example of this would be the trust store containing both the intermediate and the root for the cert chain (`leaf -> intermediate -> root`). The default settings of AWS-LC and OpenSSL require `root` to be self signed for the chain to be verified. Many TLS implementations set the `X509_V_FLAG_PARTIAL_CHAIN` flag however, which allows non self signed certificates to be trusted in the trust store. When `X509_V_FLAG_PARTIAL_CHAIN` is set, OpenSSL 1.1.1 will only verify leaf and intermediate, since intermediate is a trusted certificate. However, AWS-LC will continue building a certificate chain and include root within the chain of trust. This causes a behavioral difference with `X509_STORE_CTX_get0_chain`, where AWS-LC will return all 3 certificates (`leaf -> intermediate -> root`) and OpenSSL 1.1.1 will only return the first two (`leaf -> intermediate `). Our upstream forked a bit before OpenSSL 1.0.2, so we don't have the new behavior. This described behavioral difference was introduced in openssl/openssl@d9b8b89 (along with many others), but the commit introduces too many backwards incompatible changes for us to take as a whole. This subtle difference was due to [OpenSSL 1.1.1 continuously checking for trust while the chain's being established. The search for the next valid cert breaks early as soon as a valid chain has been built. Our current behavior builds the chain with all possible certs first and only breaks the loop if the final cert in the chain is self-signed. We can inherit this part of 1.1.1's new behavior to fix this issue. ### Call-outs: I don't believe this really changes our X509 chain building or verification by much. We're only adding an additional check for trust while the chain is being established and the final chain still needs to go through the same building/verification process that exists in AWS-LC today. ### Testing: Specific test for new behavior in `X509_V_FLAG_PARTIAL_CHAIN` By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license and the ISC license.
1 parent 7ff9840 commit 9fbfa70

File tree

2 files changed

+60
-0
lines changed

2 files changed

+60
-0
lines changed

crypto/x509/x509_test.cc

+46
Original file line numberDiff line numberDiff line change
@@ -1702,6 +1702,52 @@ TEST(X509Test, TestVerify) {
17021702
}
17031703
}
17041704

1705+
TEST(X509Test, PartialChain) {
1706+
bssl::UniquePtr<X509> root(CertFromPEM(kRootCAPEM));
1707+
bssl::UniquePtr<X509> intermediate(CertFromPEM(kIntermediatePEM));
1708+
bssl::UniquePtr<X509> leaf(CertFromPEM(kLeafPEM));
1709+
ASSERT_TRUE(root);
1710+
ASSERT_TRUE(intermediate);
1711+
ASSERT_TRUE(leaf);
1712+
1713+
// We're intentionally placing the intermediate cert in the trust store here.
1714+
// Many TLS implementations set |X509_V_FLAG_PARTIAL_CHAIN|, which allows
1715+
// non-self-signed certificates in the trust store to be trusted.
1716+
// See https://github.com/openssl/openssl/issues/7871.
1717+
bssl::UniquePtr<STACK_OF(X509)> intermediates_stack(CertsToStack({}));
1718+
bssl::UniquePtr<STACK_OF(X509)> roots_stack(
1719+
CertsToStack({intermediate.get(), root.get()}));
1720+
1721+
for (bool partial_chain : {true, false}) {
1722+
SCOPED_TRACE(partial_chain);
1723+
bssl::UniquePtr<X509_STORE_CTX> ctx(X509_STORE_CTX_new());
1724+
bssl::UniquePtr<X509_STORE> store(X509_STORE_new());
1725+
ASSERT_TRUE(ctx);
1726+
ASSERT_TRUE(store);
1727+
1728+
ASSERT_TRUE(X509_STORE_CTX_init(ctx.get(), store.get(), leaf.get(),
1729+
intermediates_stack.get()));
1730+
X509_STORE_CTX_set0_trusted_stack(ctx.get(), roots_stack.get());
1731+
1732+
X509_VERIFY_PARAM *param = X509_STORE_CTX_get0_param(ctx.get());
1733+
time_t current_time = time(nullptr);
1734+
X509_VERIFY_PARAM_set_time_posix(param, current_time);
1735+
1736+
if (partial_chain) {
1737+
X509_VERIFY_PARAM_set_flags(param, X509_V_FLAG_PARTIAL_CHAIN);
1738+
}
1739+
1740+
EXPECT_EQ(X509_verify_cert(ctx.get()), 1);
1741+
1742+
STACK_OF(X509) *chain = X509_STORE_CTX_get0_chain(ctx.get());
1743+
ASSERT_TRUE(chain);
1744+
1745+
// |root| will be included in the chain if |X509_V_FLAG_PARTIAL_CHAIN| is
1746+
// not set.
1747+
EXPECT_EQ(sk_X509_num(chain), partial_chain ? 2u : 3u);
1748+
}
1749+
}
1750+
17051751
#if defined(OPENSSL_THREADS)
17061752
// Verifying the same |X509| objects on two threads should be safe.
17071753
TEST(X509Test, VerifyThreads) {

crypto/x509/x509_vfy.c

+14
Original file line numberDiff line numberDiff line change
@@ -362,6 +362,20 @@ int X509_verify_cert(X509_STORE_CTX *ctx) {
362362
ok = 0;
363363
goto end;
364364
}
365+
366+
// OpenSSL 1.1.1 continuously re-checks for trust and breaks the loop
367+
// as soon as trust has been established. 1.0.2 builds the chain with all
368+
// possible certs first and only checks for trust if the final cert in
369+
// the chain is self-signed.
370+
// This caused additional unanticipated certs to be in the established
371+
// certificate chain, particularly when |X509_V_FLAG_PARTIAL_CHAIN| was
372+
// set. We try checking continuously for trust here for better 1.1.1
373+
// compatibility.
374+
trust = check_trust(ctx);
375+
if (trust == X509_TRUST_TRUSTED || trust == X509_TRUST_REJECTED) {
376+
break;
377+
}
378+
365379
num++;
366380
}
367381

0 commit comments

Comments
 (0)