Skip to content
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

v4.4: feat(listener): TLS partial_chain validation #10553

Merged

Conversation

qzhuyan
Copy link
Contributor

@qzhuyan qzhuyan commented Apr 27, 2023

Fixes EMQX-9789

Summary

🤖 Generated by Copilot at 76659f4

This pull request adds a new feature to enable partial chain validation for TLS listeners, which can accept client certificates verified by any certificate in the server's CA file. It modifies the emqx.schema, emqx_listeners, and emqx_tls_lib files to define and implement the feature, and adds a new module emqx_const_v2 to create a root_fun function for validation. It also adds a new test suite emqx_listener_tls_verify_SUITE and a helper module emqx_test_tls_certs_helper to test the feature with different scenarios and certificates.

PR Checklist

Please convert it to a draft if any of the following conditions are not met. Reviewers may skip over until all the items are checked:

  • Added tests for the changes
  • Changed lines covered in coverage report
  • Change log has been added to changes/{ce,ee}/(feat|perf|fix)-<PR-id>.en.md files
  • For internal contributor: there is a jira ticket to track this change
  • If there should be document changes, a PR to emqx-docs.git is sent, or a jira ticket is created to follow up
  • Schema changes are backward compatible

@coveralls
Copy link
Collaborator

coveralls commented Apr 28, 2023

Pull Request Test Coverage Report for Build 4927304571

  • 24 of 26 (92.31%) changed or added relevant lines in 3 files are covered.
  • 27 unchanged lines in 7 files lost coverage.
  • Overall coverage increased (+0.07%) to 74.82%

Changes Missing Coverage Covered Lines Changed/Added Lines %
src/emqx_tls_lib.erl 16 18 88.89%
Files with Coverage Reduction New Missed Lines %
apps/emqx_auth_mnesia/src/emqx_acl_mnesia_migrator.erl 1 94.37%
src/emqx_shared_sub.erl 1 93.33%
apps/emqx_lwm2m/src/emqx_lwm2m_coap_resource.erl 2 63.69%
apps/emqx_lwm2m/src/emqx_lwm2m_protocol.erl 2 73.91%
src/emqx_broker.erl 3 83.23%
apps/emqx_management/src/emqx_mgmt_data_backup.erl 8 66.03%
src/emqx_reason_codes.erl 10 89.29%
Totals Coverage Status
Change from base Build 4900572729: 0.07%
Covered Lines: 14827
Relevant Lines: 19817

💛 - Coveralls

@qzhuyan qzhuyan force-pushed the dev/william/tls-root-fun-verify-partial-chain branch 2 times, most recently from d720bc8 to 491a238 Compare May 2, 2023 07:39
@qzhuyan qzhuyan marked this pull request as ready for review May 2, 2023 07:46
@qzhuyan qzhuyan requested review from a team and id as code owners May 2, 2023 07:46
@qzhuyan qzhuyan changed the title feat(listener): TLS partial_chain validation v4.4: feat(listener): TLS partial_chain validation May 3, 2023
src/emqx_const_v2.erl Outdated Show resolved Hide resolved
changes/v4.4.18-en.md Outdated Show resolved Hide resolved
@qzhuyan qzhuyan force-pushed the dev/william/tls-root-fun-verify-partial-chain branch from d7fc6db to 245be16 Compare May 3, 2023 13:13
src/emqx_tls_lib.erl Outdated Show resolved Hide resolved
@qzhuyan qzhuyan force-pushed the dev/william/tls-root-fun-verify-partial-chain branch 3 times, most recently from 813ac21 to 57ea9b8 Compare May 5, 2023 08:49
src/emqx_tls_lib.erl Outdated Show resolved Hide resolved
src/emqx_const_v2.erl Outdated Show resolved Hide resolved
@qzhuyan qzhuyan force-pushed the dev/william/tls-root-fun-verify-partial-chain branch from 3d0cc70 to c881ae1 Compare May 9, 2023 11:10
@qzhuyan qzhuyan force-pushed the dev/william/tls-root-fun-verify-partial-chain branch from 7664121 to 8260f89 Compare May 9, 2023 14:07
@qzhuyan qzhuyan force-pushed the dev/william/tls-root-fun-verify-partial-chain branch from 8260f89 to 7894bb0 Compare May 9, 2023 15:00
@qzhuyan qzhuyan removed the request for review from id May 10, 2023 15:19
@qzhuyan qzhuyan merged commit 6fa4b97 into emqx:main-v4.4 May 10, 2023
137 checks passed
@@ -7,5 +7,9 @@
某些动作的参数支持使用占位符语法,来动态的填充字符串的内容,占位符语法的格式为 `${key}`。
改进前,`${key}` 中的 `key` 只能包含字母、数字和下划线。改进后 `key` 支持任意的 UTF8 字符了。

- 增加了一个新的功能,为TLS监听器启用部分证书链验证[#10553](https://github.com/emqx/emqx/pull/10553)。
如果 partial_chain 设置为“true”,cacertfile 中的最后一个证书将被视为证书信任链的顶端证书。 也就是说,TLS 握手不需要完整的链,并且 EMQX 不会尝试一直验证链直到根 CA。
Copy link
Member

Choose a reason for hiding this comment

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

最后一个证书 ->
最后一个或两个证书

@@ -1646,6 +1646,10 @@ end}.
{datatype, atom}
]}.

{mapping, "listener.ssl.$name.partial_chain", "emqx.listeners", [
{datatype, atom}
Copy link
Member

@HJianBo HJianBo May 15, 2023

Choose a reason for hiding this comment

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

{datatype, {enum, [true, two_cacerts_from_cacertfile, cacert_from_cacertfile]}}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants