Skip to content

Conversation

bazsi
Copy link
Collaborator

@bazsi bazsi commented Oct 23, 2023

Fixes the issues found in #4681

@OverOrion
Copy link
Collaborator

CentOS 7 strikes again:

lib/transport/tls-context.c: In function 'tls_context_setup_ssl_version':
lib/transport/tls-context.c:205:3: warning: implicit declaration of function 'SSL_CTX_set_min_proto_version' [-Wimplicit-function-declaration]
   SSL_CTX_set_min_proto_version(self->ssl_ctx, self->ssl_version);
   ^
lib/transport/tls-context.c: In function 'tls_context_set_ssl_version_by_name':
lib/transport/tls-context.c:691:25: error: 'TLS1_3_VERSION' undeclared (first use in this function)
     self->ssl_version = TLS1_3_VERSION;

SSL_CTX_set_min_proto_version docs say:

The setter functions were added in OpenSSL 1.1.0. The getter functions were added in OpenSSL 1.1.1.

But the default version is 1.0.2 (), however there seems to be an EPEL package for 1.1.1: https://src.fedoraproject.org/rpms/openssl11.
I haven't tried it, just found it, but it seems that it has not been updated for a while (1.1.1k [25 Mar 2021] instead of the latest 1.1.1w [11 Sep 2023]).

Taking into account that the package is not up-to-date and the fact that CentOS 7 becomes EOL on June 30, 2024, I am not sure if it would be worth adding support for it, what do you think Bazsi?

This is the newer mechanism to specify SSL versions in OpenSSL, instead of the
flag based mechanism that is supported by SSL_CTX_set_ssl_options.

Signed-off-by: Balazs Scheidler <balazs.scheidler@axoflow.com>
@bazsi bazsi force-pushed the afsocket-to-support-ssl-version branch from 724a097 to 9e849cb Compare October 30, 2023 10:24
bazsi added 3 commits October 30, 2023 11:37
…meters

Signed-off-by: Balazs Scheidler <balazs.scheidler@axoflow.com>
fluentd uses the same ssl_version parameter but uses upper case. We only
accepted lower case. Logging Operator does both fluentd and syslog-ng and
this difference is confusing.

Signed-off-by: Balazs Scheidler <balazs.scheidler@axoflow.com>
Signed-off-by: Balazs Scheidler <balazs.scheidler@axoflow.com>
@bazsi bazsi force-pushed the afsocket-to-support-ssl-version branch from 9e849cb to 2dd077e Compare October 30, 2023 10:37
@bazsi
Copy link
Collaborator Author

bazsi commented Oct 30, 2023

@OverOrion thanks for the review. I've now fixed up CentOS7 support so all should be good now.

@OverOrion OverOrion self-requested a review October 30, 2023 16:03
Copy link
Collaborator

@OverOrion OverOrion left a comment

Choose a reason for hiding this comment

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

Thanks Bazsi!

I checked what happens if the user configures it like this:

ssl-options(no-tlsv13)
ssl-version(tlsv1_3)

Fortunately it is a pretty self-explanatory error (I feared that it would be something cryptic or worse, just silent failure):

[2023-10-31T09:59:50.908714] SSL error while writing stream; tls_error='error:0A0000BF:SSL routines::no protocols available', location='./etc/syslog-ng-ssl-version.conf:19:3'

@OverOrion OverOrion merged commit 08d2a6d into syslog-ng:master Oct 31, 2023
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.

3 participants