tokers commented on a change in pull request #4034:
URL: https://github.com/apache/apisix/pull/4034#discussion_r612052183



##########
File path: apisix/schema_def.lua
##########
@@ -642,6 +642,18 @@ _M.ssl = {
             type = "array",
             items = private_key_schema,
         },
+        client = {
+            type = "object",
+            properties = {
+                ca = certificate_scheme,

Review comment:
       We may support CA cert chains in the future.

##########
File path: apisix/ssl/router/radixtree_sni.lua
##########
@@ -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

Review comment:
       The name `client_cert` is confusing, actually, it's the CA cert(s) to 
verify the client cert. What about `ca_cert`.

##########
File path: apisix/admin/ssl.lua
##########
@@ -70,6 +70,17 @@ local function check_conf(id, conf, need_id)
         end
     end
 
+    if conf.client then
+        if not apisix_ssl.support_client_verification() then

Review comment:
       This check is difficult when we using manager-api, unless we have a 
mechanism to advertise this capability to it (like rolling feature sets in the 
server-info plugin).




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


Reply via email to