Support mTLS #222
Open
Support mTLS #222
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
By default server checked server and client certificates which should do with mTLS configuration. Since it is not expected behaviour, after the patch `ca_file` configuration won't ask for client certificates authorization Closes #217
themilchenko
force-pushed
the
themilchenko/support-mtls
branch
from
7fed070 to
b5f1cf3
Compare
Member
DifferentialOrange
left a comment
DifferentialOrange
left a comment
There was a problem hiding this comment.
Let's wait for the default option feedback
Comment on lines
1299
to
1303
| local VERIFY_CLIENT_OPTS = { | ||
| off = sslsocket.SET_VERIFY_FLAGS.SSL_VERIFY_NONE, | ||
| optional = sslsocket.SET_VERIFY_FLAGS.SSL_VERIFY_PEER, | ||
| on = sslsocket.SET_VERIFY_FLAGS.SSL_VERIFY_PEER + sslsocket.SET_VERIFY_FLAGS.SSL_VERIFY_FAIL_IF_NO_PEER, | ||
| } |
Member
There was a problem hiding this comment.
Let's reference NGINX in the commit message as a motivation (if we actually use the same names and defaults)
http/server.lua
Outdated
| local VERIFY_CLIENT_OPTS = { | ||
| off = sslsocket.SET_VERIFY_FLAGS.SSL_VERIFY_NONE, | ||
| optional = sslsocket.SET_VERIFY_FLAGS.SSL_VERIFY_PEER, | ||
| on = sslsocket.SET_VERIFY_FLAGS.SSL_VERIFY_PEER + sslsocket.SET_VERIFY_FLAGS.SSL_VERIFY_FAIL_IF_NO_PEER, |
Member
There was a problem hiding this comment.
Does some bit operation is actually expected to be here? There is a proper bit library for it.
| }, | ||
| test_verify_client_optional_with_certs_valid = { | ||
| ssl_opts = { | ||
| ssl_verify_client = "optional", |
Member
There was a problem hiding this comment.
Suggested change
| ssl_verify_client = "optional", | |
| ssl_verify_client = 'optional', |
etc
CHANGELOG.md
Outdated
|
|
||
| - Do not recreate server if it's address and port were not changed (#219). | ||
| - Server doesn't change after updating parameters on config reload (#216). | ||
| - Mutual TLS with `ca_file` option enabled (#217). |
Member
There was a problem hiding this comment.
Suggested change
| - Mutual TLS with `ca_file` option enabled (#217). | |
| - **Breaking change**: mutual TLS with `ca_file` option enabled (#217). |
This patch allows to set a new `ssl_verify_client` option. It uses in pair with `ssl_ca_file` option and needs for client validation. It could have following values: * `off` (default one) means that no client's certs will be verified; * `on` means that server will verify client's certs; * `optional` means that server will verify client's certs only if it exist. This set of options was was built on top of the NGINX API (https://nginx.org/en/docs/http/ngx_http_ssl_module.html#ssl_verify_client). In effect, this option forces the server to work with mutual TLS. Part of #207
Since http server supports a new `ssl_verify_client` option it is necessary to support it in role api as well. This patch introduces a new config parameter in httpd role with the same `ssl_verify_client` name. Closes #207
themilchenko
force-pushed
the
themilchenko/support-mtls
branch
from
b5f1cf3 to
0736446
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This patch allows to set a new
ssl_verify_clientoption. It uses in pair withssl_ca_fileoption and needs for client validation. It could have following values:off(default one) means that no client's certs will be verified;onmeans that server will verify client's certs;optionalmeans that server will verify client's certs only if it exist.In effect, this option forces the server to work with mutual TLS.
The same parameter was supported for httpd role as well.
Closes #207
Closes #217