fix(upstream): apply the SSL trusted store without a client certificate - #126
Conversation
`ngx_http_apisix_set_upstream_ssl()` read `ctx->upstream_trusted_store` inside the `ctx->upstream_cert != NULL` branch, so a store installed by `upstream.set_ssl_trusted_store()` was silently dropped whenever the caller had not also installed a client certificate. Verifying an upstream against a caller-supplied CA is independent of mTLS, and that is the more common case: the SSL_CTX only carries `proxy_ssl_trusted_certificate` when `proxy_ssl_verify` is on at configuration time, so a caller that turns verification on at runtime through `upstream.set_ssl_verify()` has no trust anchors at all unless the store applies. Move the `SSL_set1_verify_cert_store()` call out of that branch.
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 53 minutes Limit details: You’ve used the included review currently available. Your 60 included PR review attempts over the past 7 days set your current allowance at 1 review per hour. You’re in a promotional period — use the checkbox below to run this review for free:
On-demand reviews are free for the next 30 days. After that, they cost $0.25 per reviewed file. How can I continue?Run this review now using the option above, or comment You can also wait for the limit to reset, then comment An organization admin can change what happens after included review limits in Billing. How do review limits work?CodeRabbit enforces per-developer PR review limits within each organization. For paid Pro and Pro+ reviews, CodeRabbit uses a developer's included PR review attempts over the past 7 days to set the current hourly allowance. At typical activity levels, the full plan allowance applies. Higher sustained activity can lower the allowance until earlier attempts leave the 7-day window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
Comment |
1.3.17 builds against apisix-nginx-module 1.19.10, which makes `upstream.set_ssl_trusted_store()` take effect without a client certificate (api7/apisix-nginx-module#126) - `upstream.tls.ca_certs` in apache#13863 depends on it - and picks up ngx_http_ffi_client v0.1.2/v0.1.3.
Problem
upstream.set_ssl_trusted_store()only takes effect when the caller also installed a client certificate. Inngx_http_apisix_set_upstream_ssl()the store was read and applied inside thectx->upstream_cert != NULLbranch, so without mTLS the store was silently dropped and the handshake fell back to whatever the SSL_CTX carried.That fallback is usually nothing. nginx loads
proxy_ssl_trusted_certificateinto the SSL_CTX only whenproxy_ssl_verifyis on at configuration time (ngx_http_proxy_set_ssl()), so a caller that turns verification on at runtime withupstream.set_ssl_verify()and supplies its own CA throughset_ssl_trusted_store()ends up with an empty trust store and every handshake fails with(20:unable to get local issuer certificate).Verifying the upstream certificate against a caller-supplied CA has nothing to do with presenting a client certificate, and the no-mTLS case is the more common one.
Solution
Move the
SSL_set1_verify_cert_store()call out of the client-certificate branch so the trusted store is applied whenever one was installed.Tests
t/upstream_mtls2.tgains two blocks against an upstream that does not ask for a client certificate: one showing the handshake is verified againstproxy_ssl_trusted_certificateand fails, and one showing that installing only the trusted store makes it succeed. The new blocks fail onmainand pass with this change;t/upstream_mtls.tandt/upstream_ssl_verify.tstill pass.