Skip to content

fix(upstream): use cert and key instead of stale ok in mTLS error checks#13442

Open
okaybase wants to merge 1 commit into
apache:masterfrom
okaybase:fix-upstream
Open

fix(upstream): use cert and key instead of stale ok in mTLS error checks#13442
okaybase wants to merge 1 commit into
apache:masterfrom
okaybase:fix-upstream

Conversation

@okaybase
Copy link
Copy Markdown
Member

Description

The branch uses a stale ok variable from fill_node_info() to check whether fetch_cert and fetch_pkey succeeded, instead of checking the cert and key variables they actually return. Since ok was already true at that point, both error branches were unreachable — if either fetch returned false, the function would silently proceed with an invalid value rather than returning a 503 error.

Checklist

  • I have explained the need for this PR and the problem it solves
  • I have explained the changes or the new features added to this PR
  • I have verified that this change is backward compatible (If not, please discuss on the APISIX mailing list first)

@dosubot dosubot Bot added size:XS This PR changes 0-9 lines, ignoring generated files. bug Something isn't working labels May 26, 2026
Copy link
Copy Markdown
Contributor

@Baoyuantop Baoyuantop left a comment

Choose a reason for hiding this comment

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

The remaining blocker is test coverage. The existing t/node/upstream-mtls.t covers several upstream mTLS schema validation and successful handshake paths, but it does not directly cover the runtime path in apisix/upstream.lua where fetch_cert or fetch_pkey fails and set_by_route should return 503 instead of passing an invalid value to the later TLS setup logic.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working size:XS This PR changes 0-9 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants