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

transport tls: use SSL_VERIFY_NONE by default #4718

Merged

Conversation

daipom
Copy link
Contributor

@daipom daipom commented Nov 28, 2024

Which issue(s) this PR fixes:

What this PR does / why we need it:
Use SSL_VERIFY_NONE by default for transport tls.
(VERIFY_PEER is used by default now).

VERIFY_NONE should be used when client_cert_auth false (default).

Before this fix, we need to set insecure true for this.
However, insecure option should mainly be for cipher strength.
It would not be intended VERIFY_PEER without VERIFY_FAIL_IF_NO_PEER_CERT was used even if client_cert_auth false.
(When VERIFY_PEER without VERIFY_FAIL_IF_NO_PEER_CERT, server does certification only when clients send its certificate.
This would be why we overlooked it long time)

Before:

insecure client_cert_auth verify_mode
false fales VERIFY_PEER
false true VERIFY_PEER, VERIFY_FAIL_IF_NO_PEER_CERT
true false VERIFY_NONE
true true VERIFY_PEER, VERIFY_FAIL_IF_NO_PEER_CERT

After:

insecure client_cert_auth verify_mode
false fales VERIFY_NONE
false true VERIFY_PEER, VERIFY_FAIL_IF_NO_PEER_CERT
true false VERIFY_NONE
true true VERIFY_PEER, VERIFY_FAIL_IF_NO_PEER_CERT

Docs Changes:
Not needed.

Release Note:
The same as the title.

VERIFY_NONE should be used when `client_cert_auth false` (default).

Before this fix, we need to set `insecure true` for this.
However, `insecure` option should mainly be for cipher strength.
It would not be intended VERIFY_PEER without VERIFY_FAIL_IF_NO_PEER_CERT
was used even if `client_cert_auth false`.
(When VERIFY_PEER without VERIFY_FAIL_IF_NO_PEER_CERT, server
does certification only when clients send its certificate.
This would be why we overlooked it long time)

Before:

| insecure | client_cert_auth | verify_mode                              |
| false    | fales            | VERIFY_PEER                              |
| false    | true             | VERIFY_PEER, VERIFY_FAIL_IF_NO_PEER_CERT |
| true     | false            | VERIFY_NONE                              |
| true     | true             | VERIFY_PEER, VERIFY_FAIL_IF_NO_PEER_CERT |

After:

| insecure | client_cert_auth | verify_mode                              |
| false    | fales            | VERIFY_NONE                              |
| false    | true             | VERIFY_PEER, VERIFY_FAIL_IF_NO_PEER_CERT |
| true     | false            | VERIFY_NONE                              |
| true     | true             | VERIFY_PEER, VERIFY_FAIL_IF_NO_PEER_CERT |

Signed-off-by: Daijiro Fukuda <[email protected]>
@daipom daipom added this to the v1.18.0 milestone Nov 28, 2024
@daipom daipom requested review from kenhys and Watson1978 November 28, 2024 03:17
Copy link
Contributor

@kenhys kenhys left a comment

Choose a reason for hiding this comment

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

LGTM.

@kenhys kenhys merged commit 4db97a3 into fluent:master Nov 28, 2024
16 checks passed
@daipom daipom deleted the transport-tls-use-SSL_VERIFY_NONE-by-default branch November 28, 2024 05:08
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.

Syslog TLS: [client_cert_auth false] settings is not applied if [insecure true] is not set.
2 participants