-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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.
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
feat: support client certificate verification #4034
Conversation
Signed-off-by: spacewander <spacewanderlzx@gmail.com>
apisix/ssl/router/radixtree_sni.lua
Outdated
@@ -194,6 +195,24 @@ function _M.match_and_set(api_ctx) | |||
end | |||
end | |||
|
|||
if matched_ssl.value.client then | |||
local client_cert = matched_ssl.value.client.ca |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The name client_cert
is confusing, actually, it's the CA cert(s) to verify the client cert. What about ca_cert
.
Signed-off-by: spacewander <spacewanderlzx@gmail.com>
| client.ca | False | Certificate| set the CA certificate which will use to verify client. This feature requires OpenResty 1.19+. | | | ||
| client.depth | False | Certificate| set the verification depth in the client certificates chain, default to 1. This feature requires OpenResty 1.19+. | | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The first letter should be capitalized. set --> Set
set the CA certificate which will use to verify client.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
None of the description in this table start with uppercase letter.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Got it.
t/node/client-mtls.t
Outdated
local ssl_ca_cert = t.read_file("t/certs/mtls_ca.crt") | ||
local ssl_cert = t.read_file("t/certs/mtls_client.crt") | ||
local ssl_key = t.read_file("t/certs/mtls_client.key") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These certificates were not used in this test, I think they can be removed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Firstsawyou
Updated.
Signed-off-by: spacewander spacewanderlzx@gmail.com
What this PR does / why we need it:
Pre-submission checklist: