Skip to content

fix: Fetch secrets from upstream ssl config #13062

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
cybersec-jochenarnold wants to merge 1 commit into
base: master
Choose a base branch
from
+185 −24
Open

fix: Fetch secrets from upstream ssl config #13062

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion apisix/cli/file.lua
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -328,7 +328,7 @@ function _M.read_yaml_conf(apisix_home)
-- Therefore we need to check the absolute version instead
local cert_path = pl_path.abspath(apisix_ssl.ssl_trusted_certificate)
if not pl_path.exists(cert_path) then
util.die("certificate path", cert_path, "doesn't exist\n")
util.die("certificate path ", cert_path, " doesn't exist\n")
end
apisix_ssl.ssl_trusted_certificate = cert_path
end
Expand Down
15 changes: 14 additions & 1 deletion apisix/init.lua
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -552,9 +552,22 @@ function _M.handle_upstream(api_ctx, route, enable_websocket)
return ngx_exit(1)
end

local new_upstream_ssl = apisix_secret.fetch_secrets(upstream_ssl, true)

if not new_upstream_ssl then
Copy link

Copilot AI Mar 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

apisix_secret.fetch_secrets only returns nil when the input is missing or not a table; for a valid upstream_ssl table it will always return a (deep-copied) table even if individual secret lookups fail (it leaves the original $env:///$secret:// string in place and logs). As a result, the if not new_upstream_ssl branch is effectively unreachable and won’t prevent the later TLS/decrypt errors this PR is trying to avoid. Consider removing this check, or updating the secrets API/usage to surface lookup failures (e.g., return nil, err or explicitly detect unresolved secret refs after substitution) and handle that here.

Suggested change
if not new_upstream_ssl then
local function has_unresolved_secret_ref(value)
if type(value) ~= "table" then
return false
end
for _, v in pairs(value) do
local vt = type(v)
if vt == "string" then
if core.string.has_prefix(v, "$env://")
or core.string.has_prefix(v, "$secret://") then
return true
end
elseif vt == "table" then
if has_unresolved_secret_ref(v) then
return true
end
end
end
return false
end
if not new_upstream_ssl or has_unresolved_secret_ref(new_upstream_ssl) then

Copilot uses AI. Check for mistakes.
core.log.error("failed to get ssl cert: error fetching secrets")

if is_http then
return core.response.exit(502)
end

return ngx_exit(1)
end

core.log.info("matched ssl: ",
core.json.delay_encode(upstream_ssl, true))
api_ctx.upstream_ssl = upstream_ssl

api_ctx.upstream_ssl = new_upstream_ssl
end

if enable_websocket then
Expand Down
192 changes: 170 additions & 22 deletions t/node/upstream-mtls.t
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,22 @@
# See the License for the specific language governing permissions and
# limitations under the License.
#

BEGIN {
sub set_env_from_file {
my ($env_name, $file_path) = @_;

open my $fh, '<', $file_path or die $!;
my $content = do { local $/; <$fh> };
close $fh;

$ENV{$env_name} = $content;
}
# set env
set_env_from_file('MTLS_CERT_VAR', 't/certs/mtls_client.crt');
set_env_from_file('MTLS_KEY_VAR', 't/certs/mtls_client.key');
}

use t::APISIX;

my $nginx_binary = $ENV{'TEST_NGINX_BINARY'} || 'nginx';
Expand All @@ -38,7 +54,17 @@ run_tests();

__DATA__

=== TEST 1: tls without key
=== TEST 1: store cert and key in vault
--- exec
VAULT_TOKEN='root' VAULT_ADDR='http://0.0.0.0:8200' vault kv put kv/apisix/ssl \
mtls_client.crt=@t/certs/mtls_client.crt \
mtls_client.key=@t/certs/mtls_client.key
--- response_body
Success! Data written to: kv/apisix/ssl



=== TEST 2: tls without key
--- config
location /t {
content_by_lua_block {
Expand Down Expand Up @@ -77,7 +103,7 @@ GET /t



=== TEST 2: tls with bad key
=== TEST 3: tls with bad key
--- config
location /t {
content_by_lua_block {
Expand Down Expand Up @@ -119,7 +145,7 @@ decrypt ssl key failed



=== TEST 3: encrypt key by default
=== TEST 4: encrypt key by default
--- config
location /t {
content_by_lua_block {
Expand Down Expand Up @@ -248,7 +274,7 @@ false



=== TEST 4: hit
=== TEST 5: hit
--- upstream_server_config
ssl_client_certificate ../../certs/mtls_ca.crt;
ssl_verify_client on;
Expand All @@ -259,7 +285,7 @@ hello world



=== TEST 5: wrong cert
=== TEST 6: wrong cert
--- config
location /t {
content_by_lua_block {
Expand Down Expand Up @@ -300,7 +326,7 @@ passed



=== TEST 6: hit
=== TEST 7: hit
--- upstream_server_config
ssl_client_certificate ../../certs/mtls_ca.crt;
ssl_verify_client on;
Expand All @@ -312,7 +338,7 @@ client SSL certificate verify error



=== TEST 7: clean old data
=== TEST 8: clean old data
--- config
location /t {
content_by_lua_block {
Expand All @@ -333,7 +359,7 @@ GET /t



=== TEST 8: don't encrypt key
=== TEST 9: don't encrypt key
--- yaml_config
apisix:
node_listen: 1984
Expand Down Expand Up @@ -467,7 +493,7 @@ true



=== TEST 9: bind upstream
=== TEST 10: bind upstream
--- config
location /t {
content_by_lua_block {
Expand All @@ -494,7 +520,7 @@ GET /t



=== TEST 10: hit
=== TEST 11: hit
--- upstream_server_config
ssl_client_certificate ../../certs/mtls_ca.crt;
ssl_verify_client on;
Expand All @@ -505,7 +531,7 @@ GET /server_port



=== TEST 11: bind service
=== TEST 12: bind service
--- config
location /t {
content_by_lua_block {
Expand All @@ -532,7 +558,7 @@ GET /t



=== TEST 12: hit
=== TEST 13: hit
--- upstream_server_config
ssl_client_certificate ../../certs/mtls_ca.crt;
ssl_verify_client on;
Expand All @@ -543,7 +569,7 @@ hello world



=== TEST 13: get cert by tls.client_cert_id
=== TEST 14: get cert by tls.client_cert_id
--- config
location /t {
content_by_lua_block {
Expand Down Expand Up @@ -598,7 +624,7 @@ GET /t



=== TEST 14: hit
=== TEST 15: hit
--- upstream_server_config
ssl_client_certificate ../../certs/mtls_ca.crt;
ssl_verify_client on;
Expand All @@ -609,7 +635,7 @@ hello world



=== TEST 15: change ssl object type
=== TEST 16: change ssl object type
--- config
location /t {
content_by_lua_block {
Expand Down Expand Up @@ -640,7 +666,7 @@ GET /t



=== TEST 16: hit, ssl object type mismatch
=== TEST 17: hit, ssl object type mismatch
--- upstream_server_config
ssl_client_certificate ../../certs/mtls_ca.crt;
ssl_verify_client on;
Expand All @@ -652,7 +678,7 @@ failed to get ssl cert: ssl type should be 'client'



=== TEST 17: delete ssl object
=== TEST 18: delete ssl object
--- config
location /t {
content_by_lua_block {
Expand All @@ -673,7 +699,7 @@ GET /t



=== TEST 18: hit, ssl object not exits
=== TEST 19: hit, ssl object not exits
--- upstream_server_config
ssl_client_certificate ../../certs/mtls_ca.crt;
ssl_verify_client on;
Expand All @@ -685,7 +711,7 @@ failed to get ssl cert: ssl id [1] not exits



=== TEST 19: `tls.verify` only
=== TEST 20: `tls.verify` only
--- config
location /t {
content_by_lua_block {
Expand Down Expand Up @@ -723,7 +749,7 @@ passed



=== TEST 20: hit
=== TEST 21: hit
When only `tls.verify` is present, the matching logic related to
`client_cert`, `client_key` or `client_cert_id` should not be entered
--- request
Expand All @@ -733,7 +759,7 @@ hello world



=== TEST 21: set `verify` with `client_cert`, `client_key`
=== TEST 22: set `verify` with `client_cert`, `client_key`
--- config
location /t {
content_by_lua_block {
Expand Down Expand Up @@ -774,7 +800,7 @@ passed



=== TEST 22: hit
=== TEST 23: hit
`tls.verify` does not affect the parsing of `client_cert`, `client_key`
--- upstream_server_config
ssl_client_certificate ../../certs/mtls_ca.crt;
Expand All @@ -783,3 +809,125 @@ passed
GET /hello
--- response_body
hello world

=== TEST 24: get cert by tls.client_cert_id with secrets using secrets
Copy link

Copilot AI Mar 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Test title is redundant/grammatically awkward: "get cert by tls.client_cert_id with secrets using secrets". Consider renaming to something like "... with secrets using Vault" or "... with $secret:// refs" so it reads clearly in test output.

Suggested change
=== TEST 24: get cert by tls.client_cert_id with secrets using secrets
=== TEST 24: get cert by tls.client_cert_id with $secret:// refs

Copilot uses AI. Check for mistakes.
--- config
location /t {
content_by_lua_block {
local t = require("lib.test_admin")
local json = require("toolkit.json")

local data = {
type = "client",
cert = "$secret://vault/test/ssl/mtls_client.crt",
key = "$secret://vault/test/ssl/mtls_client.key"
}
Comment on lines +820 to +824
Copy link

Copilot AI Mar 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This file writes the cert/key into Vault and then later references $secret://vault/test/..., but it never configures the Vault secret backend in APISIX (e.g., via PUT /apisix/admin/secrets/vault/test with uri/prefix/token like other tests do). Without that, secret resolution will fail and APISIX will keep the $secret://... strings, causing the mTLS handshake to fail for reasons unrelated to the upstream client_cert_id fix. Add a step to create the Vault secret config before using $secret://vault/test/....

Copilot uses AI. Check for mistakes.
local code, body = t.test('/apisix/admin/ssls/1',
ngx.HTTP_PUT,
json.encode(data)
)

if code >= 300 then
ngx.status = code
ngx.say(body)
return
end

local data = {
upstream = {
scheme = "https",
type = "roundrobin",
nodes = {
["127.0.0.1:1983"] = 1,
},
tls = {
client_cert_id = 1
}
},
uri = "/hello"
}
local code, body = t.test('/apisix/admin/routes/1',
ngx.HTTP_PUT,
json.encode(data)
)

if code >= 300 then
ngx.status = code
ngx.say(body)
return
end
}
}
--- request
GET /t


=== TEST 25: hit
--- upstream_server_config
ssl_client_certificate ../../certs/mtls_ca.crt;
ssl_verify_client on;
--- request
GET /hello
--- response_body
hello world

=== TEST 26: get cert by tls.client_cert_id with secrets using env
--- config
location /t {
content_by_lua_block {
local t = require("lib.test_admin")
local json = require("toolkit.json")

local data = {
type = "client",
cert = "$env://MTLS_CERT_VAR",
key = "$env://MTLS_KEY_VAR"
}
local code, body = t.test('/apisix/admin/ssls/1',
ngx.HTTP_PUT,
json.encode(data)
)

if code >= 300 then
ngx.status = code
ngx.say(body)
return
end

local data = {
upstream = {
scheme = "https",
type = "roundrobin",
nodes = {
["127.0.0.1:1983"] = 1,
},
tls = {
client_cert_id = 1
}
},
uri = "/hello"
}
local code, body = t.test('/apisix/admin/routes/1',
ngx.HTTP_PUT,
json.encode(data)
)

if code >= 300 then
ngx.status = code
ngx.say(body)
return
end
}
}
--- request
GET /t


=== TEST 27: hit
--- upstream_server_config
ssl_client_certificate ../../certs/mtls_ca.crt;
ssl_verify_client on;
--- request
GET /hello
--- response_body
hello world
Loading