You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@apisix.apache.org by "kingluo (via GitHub)" <gi...@apache.org> on 2023/04/18 01:47:19 UTC
[GitHub] [apisix] kingluo opened a new pull request, #9322: feat: route-level MTLS
kingluo opened a new pull request, #9322:
URL: https://github.com/apache/apisix/pull/9322
### Description
<!-- Please include a summary of the change and which issue is fixed. -->
<!-- Please also include relevant motivation and context. -->
https://lists.apache.org/thread/t8kmt7hf0krvf2mc0cf0zp5q330dmo1d
### Checklist
- [x] I have explained the need for this PR and the problem it solves
- [x] I have explained the changes or the new features added to this PR
- [x] I have added tests corresponding to this change
- [x] I have updated the documentation to reflect this change
- [x] I have verified that this change is backward compatible (If not, please discuss on the [APISIX mailing list](https://github.com/apache/apisix/tree/master#community) first)
<!--
Note
1. Mark the PR as draft until it's ready to be reviewed.
2. Always add/update tests for any changes unless you have a good reason.
3. Always update the documentation to reflect the changes made in the PR.
4. Make a new commit to resolve conversations instead of `push -f`.
5. To resolve merge conflicts, merge master instead of rebasing.
6. Use "request review" to notify the reviewer after making changes.
7. Only a reviewer can mark a conversation as resolved.
-->
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] monkeyDluffy6017 merged pull request #9322: feat: route-level MTLS
Posted by "monkeyDluffy6017 (via GitHub)" <gi...@apache.org>.
monkeyDluffy6017 merged PR #9322:
URL: https://github.com/apache/apisix/pull/9322
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] soulbird commented on a diff in pull request #9322: feat: route-level MTLS
Posted by "soulbird (via GitHub)" <gi...@apache.org>.
soulbird commented on code in PR #9322:
URL: https://github.com/apache/apisix/pull/9322#discussion_r1173567477
##########
apisix/init.lua:
##########
@@ -179,6 +180,8 @@ function _M.http_ssl_phase()
local ok, err = router.router_ssl.match_and_set(api_ctx)
+ ngx_ctx.matched_ssl = api_ctx.matched_ssl
+ ngx.log(ngx.WARN, require("inspect")(api_ctx.matched_ssl))
Review Comment:
remove this
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] monkeyDluffy6017 commented on a diff in pull request #9322: feat: route-level MTLS
Posted by "monkeyDluffy6017 (via GitHub)" <gi...@apache.org>.
monkeyDluffy6017 commented on code in PR #9322:
URL: https://github.com/apache/apisix/pull/9322#discussion_r1185855271
##########
docs/en/latest/tutorials/client-to-apisix-mtls.md:
##########
@@ -193,6 +193,127 @@ curl --resolve "test.com:9443:127.0.0.1" https://test.com:9443/anything -k --cer
Since we configured the [proxy-rewrite](../plugins/proxy-rewrite.md) plugin in the example, we can see that the response body contains the request body received upstream, containing the correct data.
+## MTLS bypass based on regular expression matching against URI
+
+APISIX allows configuring an URI whitelist to bypass MTLS.
+If the URI of a request is in the whitelist, then the client certificate will not be checked.
+Note that other URIs of the associated SNI will get HTTP 400 response
+instead of alert error in the SSL handshake phase, if the client certificate is missing or invalid.
+
+### Timing diagram
+
+
+
+### Example
+
+```bash
+curl http://127.0.0.1:9180/apisix/admin/routes/1 \
Review Comment:
Maybe we should use three blocks, don't wrap them into one code block
1. config the route and ssl
2. if the client certificate is missing and the URI is not in the whitelis
3. the client certificate is missing, but the URI is in the whitelist
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] monkeyDluffy6017 commented on a diff in pull request #9322: feat: route-level MTLS
Posted by "monkeyDluffy6017 (via GitHub)" <gi...@apache.org>.
monkeyDluffy6017 commented on code in PR #9322:
URL: https://github.com/apache/apisix/pull/9322#discussion_r1185845770
##########
apisix/init.lua:
##########
@@ -310,12 +313,38 @@ local function verify_tls_client(ctx)
end
+local function uri_matches_skip_mtls_route_patterns(ssl, uri)
+ for _, pat in ipairs(ssl.value.client.skip_mtls_uri_regex) do
+ if ngx_re_match(uri, pat, "jo") then
+ return true
+ end
+ end
+end
+
+
local function verify_https_client(ctx)
local scheme = ctx.var.scheme
if scheme ~= "https" then
return true
end
+ local matched_ssl = ngx.ctx.matched_ssl
+ if matched_ssl.value.client
+ and matched_ssl.value.client.skip_mtls_uri_regex
+ and apisix_ssl.support_client_verification()
+ and (not uri_matches_skip_mtls_route_patterns(matched_ssl, ngx.var.uri)) then
+ local res = ctx.var.ssl_client_verify
+ if res ~= "SUCCESS" then
+ if res == "NONE" then
+ core.log.error("client certificate was not present")
+ else
+ core.log.error("client certificate verification is not passed: ", res)
+ end
+
+ return false
+ end
Review Comment:
This code and the code below (359 - 368) can be wrapped into a function,
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] monkeyDluffy6017 commented on a diff in pull request #9322: feat: route-level MTLS
Posted by "monkeyDluffy6017 (via GitHub)" <gi...@apache.org>.
monkeyDluffy6017 commented on code in PR #9322:
URL: https://github.com/apache/apisix/pull/9322#discussion_r1185845770
##########
apisix/init.lua:
##########
@@ -310,12 +313,38 @@ local function verify_tls_client(ctx)
end
+local function uri_matches_skip_mtls_route_patterns(ssl, uri)
+ for _, pat in ipairs(ssl.value.client.skip_mtls_uri_regex) do
+ if ngx_re_match(uri, pat, "jo") then
+ return true
+ end
+ end
+end
+
+
local function verify_https_client(ctx)
local scheme = ctx.var.scheme
if scheme ~= "https" then
return true
end
+ local matched_ssl = ngx.ctx.matched_ssl
+ if matched_ssl.value.client
+ and matched_ssl.value.client.skip_mtls_uri_regex
+ and apisix_ssl.support_client_verification()
+ and (not uri_matches_skip_mtls_route_patterns(matched_ssl, ngx.var.uri)) then
+ local res = ctx.var.ssl_client_verify
+ if res ~= "SUCCESS" then
+ if res == "NONE" then
+ core.log.error("client certificate was not present")
+ else
+ core.log.error("client certificate verification is not passed: ", res)
+ end
+
+ return false
+ end
Review Comment:
This code can be wrapped into a function
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] monkeyDluffy6017 commented on a diff in pull request #9322: feat: route-level MTLS
Posted by "monkeyDluffy6017 (via GitHub)" <gi...@apache.org>.
monkeyDluffy6017 commented on code in PR #9322:
URL: https://github.com/apache/apisix/pull/9322#discussion_r1185828351
##########
apisix/init.lua:
##########
@@ -179,6 +180,8 @@ function _M.http_ssl_phase()
local ok, err = router.router_ssl.match_and_set(api_ctx)
Review Comment:
if reject, is ok equal to `false` ?
if ok equal to `false`, does `ngx_exit(-1)` interrupt the execution of the current request ?
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] monkeyDluffy6017 commented on a diff in pull request #9322: feat: route-level MTLS
Posted by "monkeyDluffy6017 (via GitHub)" <gi...@apache.org>.
monkeyDluffy6017 commented on code in PR #9322:
URL: https://github.com/apache/apisix/pull/9322#discussion_r1185855271
##########
docs/en/latest/tutorials/client-to-apisix-mtls.md:
##########
@@ -193,6 +193,127 @@ curl --resolve "test.com:9443:127.0.0.1" https://test.com:9443/anything -k --cer
Since we configured the [proxy-rewrite](../plugins/proxy-rewrite.md) plugin in the example, we can see that the response body contains the request body received upstream, containing the correct data.
+## MTLS bypass based on regular expression matching against URI
+
+APISIX allows configuring an URI whitelist to bypass MTLS.
+If the URI of a request is in the whitelist, then the client certificate will not be checked.
+Note that other URIs of the associated SNI will get HTTP 400 response
+instead of alert error in the SSL handshake phase, if the client certificate is missing or invalid.
+
+### Timing diagram
+
+
+
+### Example
+
+```bash
+curl http://127.0.0.1:9180/apisix/admin/routes/1 \
Review Comment:
Maybe we should use three blocks, don't wrap them into one code block, it's hard to know
1. config the route and ssl
2. if the client certificate is missing and the URI is not in the whitelis
3. the client certificate is missing, but the URI is in the whitelist
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] kingluo commented on a diff in pull request #9322: feat: route-level MTLS
Posted by "kingluo (via GitHub)" <gi...@apache.org>.
kingluo commented on code in PR #9322:
URL: https://github.com/apache/apisix/pull/9322#discussion_r1185883255
##########
apisix/init.lua:
##########
@@ -179,6 +180,8 @@ function _M.http_ssl_phase()
local ok, err = router.router_ssl.match_and_set(api_ctx)
Review Comment:
It's the original logic and irrelevant to this PR.
On the contrary, the route-level MTLS bypasses the client cert error in the SSL phase, i.e. it relaxes the success restrictions.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] kingluo commented on a diff in pull request #9322: feat: route-level MTLS
Posted by "kingluo (via GitHub)" <gi...@apache.org>.
kingluo commented on code in PR #9322:
URL: https://github.com/apache/apisix/pull/9322#discussion_r1186610156
##########
docs/en/latest/tutorials/client-to-apisix-mtls.md:
##########
@@ -193,6 +193,127 @@ curl --resolve "test.com:9443:127.0.0.1" https://test.com:9443/anything -k --cer
Since we configured the [proxy-rewrite](../plugins/proxy-rewrite.md) plugin in the example, we can see that the response body contains the request body received upstream, containing the correct data.
+## MTLS bypass based on regular expression matching against URI
+
+APISIX allows configuring an URI whitelist to bypass MTLS.
+If the URI of a request is in the whitelist, then the client certificate will not be checked.
+Note that other URIs of the associated SNI will get HTTP 400 response
+instead of alert error in the SSL handshake phase, if the client certificate is missing or invalid.
+
+### Timing diagram
+
+
+
+### Example
+
+```bash
+curl http://127.0.0.1:9180/apisix/admin/routes/1 \
Review Comment:
done
##########
docs/zh/latest/tutorials/client-to-apisix-mtls.md:
##########
@@ -193,6 +193,122 @@ curl --resolve "test.com:9443:127.0.0.1" https://test.com:9443/anything -k --cer
由于我们在示例中配置了 `proxy-rewrite` 插件，我们可以看到响应体中包含上游收到的请求体，包含了正确数据。
+## 基于对 URI 正则表达式匹配，绕过 MTLS
+
+APISIX 允许配置 URI 白名单以便绕过 MTLS。如果请求的 URI 在白名单内，客户端证书将不被检查。注意，如果针对白名单外的 URI 发请求，而该请求缺乏客户端证书或者提供了非法客户端证书，会得到 HTTP 400 响应，而不是在 SSL 握手阶段被拒绝。
+
+### 时序图
+
+
+
+### 例子
+
+```bash
Review Comment:
done
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] monkeyDluffy6017 commented on a diff in pull request #9322: feat: route-level MTLS
Posted by "monkeyDluffy6017 (via GitHub)" <gi...@apache.org>.
monkeyDluffy6017 commented on code in PR #9322:
URL: https://github.com/apache/apisix/pull/9322#discussion_r1185858312
##########
docs/zh/latest/tutorials/client-to-apisix-mtls.md:
##########
@@ -193,6 +193,122 @@ curl --resolve "test.com:9443:127.0.0.1" https://test.com:9443/anything -k --cer
由于我们在示例中配置了 `proxy-rewrite` 插件，我们可以看到响应体中包含上游收到的请求体，包含了正确数据。
+## 基于对 URI 正则表达式匹配，绕过 MTLS
+
+APISIX 允许配置 URI 白名单以便绕过 MTLS。如果请求的 URI 在白名单内，客户端证书将不被检查。注意，如果针对白名单外的 URI 发请求，而该请求缺乏客户端证书或者提供了非法客户端证书，会得到 HTTP 400 响应，而不是在 SSL 握手阶段被拒绝。
+
+### 时序图
+
+
+
+### 例子
+
+```bash
Review Comment:
ditto
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] kingluo commented on a diff in pull request #9322: feat: route-level MTLS
Posted by "kingluo (via GitHub)" <gi...@apache.org>.
kingluo commented on code in PR #9322:
URL: https://github.com/apache/apisix/pull/9322#discussion_r1185877186
##########
apisix/init.lua:
##########
@@ -310,12 +313,38 @@ local function verify_tls_client(ctx)
end
+local function uri_matches_skip_mtls_route_patterns(ssl, uri)
+ for _, pat in ipairs(ssl.value.client.skip_mtls_uri_regex) do
+ if ngx_re_match(uri, pat, "jo") then
+ return true
+ end
+ end
+end
+
+
local function verify_https_client(ctx)
local scheme = ctx.var.scheme
if scheme ~= "https" then
return true
end
+ local matched_ssl = ngx.ctx.matched_ssl
+ if matched_ssl.value.client
+ and matched_ssl.value.client.skip_mtls_uri_regex
+ and apisix_ssl.support_client_verification()
+ and (not uri_matches_skip_mtls_route_patterns(matched_ssl, ngx.var.uri)) then
+ local res = ctx.var.ssl_client_verify
+ if res ~= "SUCCESS" then
+ if res == "NONE" then
+ core.log.error("client certificate was not present")
+ else
+ core.log.error("client certificate verification is not passed: ", res)
+ end
+
+ return false
+ end
Review Comment:
Not necessary, only logging statements there. Not so much intersection.
The original code is for host check, which is a different logic, and it's better to keep the original version.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] An-DJ commented on a diff in pull request #9322: feat: route-level MTLS
Posted by "An-DJ (via GitHub)" <gi...@apache.org>.
An-DJ commented on code in PR #9322:
URL: https://github.com/apache/apisix/pull/9322#discussion_r1174491459
##########
docs/en/latest/tutorials/client-to-apisix-mtls.md:
##########
@@ -193,6 +193,127 @@ curl --resolve "test.com:9443:127.0.0.1" https://test.com:9443/anything -k --cer
Since we configured the [proxy-rewrite](../plugins/proxy-rewrite.md) plugin in the example, we can see that the response body contains the request body received upstream, containing the correct data.
+## MTLS bypass based on regular expression matching against URI
+
+APISIX allows configuring an URI whitelist to bypass MTLS.
+If the URI of a request is in the whitelist, then the client certificate will not be checked.
+Note that other URIs of the associated SNI will get HTTP 400 response
+instead of alert error in the SSL handshake phase, if the client certificate is missing or invalid.
+
+### Timing diagram
+
+
Review Comment:
Better to use relative path as below:
https://github.com/apache/apisix/blob/master/docs/en/latest/benchmark.md?plain=1#L40
##########
docs/zh/latest/tutorials/client-to-apisix-mtls.md:
##########
@@ -193,6 +193,122 @@ curl --resolve "test.com:9443:127.0.0.1" https://test.com:9443/anything -k --cer
由于我们在示例中配置了 `proxy-rewrite` 插件，我们可以看到响应体中包含上游收到的请求体，包含了正确数据。
+## 基于对 URI 正则表达式匹配，绕过 MTLS
+
+APISIX 允许配置 URI 白名单以便绕过 MTLS。如果请求的 URI 在白名单内，客户端证书将不被检查。注意，如果针对白名单外的 URI 发请求，而该请求缺乏客户端证书或者提供了非法客户端证书，会得到 HTTP 400 响应，而不是在 SSL 握手阶段被拒绝。
+
+### 时序图
+
+
Review Comment:
ditto
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org