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

http: fix curl ssl version #3083

Merged
merged 2 commits into from
Jan 20, 2020
Merged

Conversation

furiel
Copy link
Collaborator

@furiel furiel commented Jan 20, 2020

Some ssl-version options were not accepted by http destination, even though they should have been:

  • tlsv1_0
  • tlsv1_1
  • tlsv1_2
  • tlsv1_3

The reason was that the used compiler switches did not exist. They were an enum instead.

Hence the relevant code was disabled.

The previous behaviour was that syslog-ng starts no matter what was the ssl-version, just emit a warning if something unexpected is detected.

This patch fixes:

  • The compiler switches
  • Syslog-ng will not start if invalid ssl-version string was added.

This latter might break some existing configuration: if users accidentally mistyped the option, syslog-ng will not start. I think this is acceptable, considering, this options specifies the minimum version of used ssl version, and thus might have security impact.

CURL_SSLVERSION_TLSv1_* are not defines, hence need proper detection.

Due to the bug, some of the ssl-version options were not available for
the user.

Signed-off-by: Antal Nemes <antal.nemes.hu@gmail.com>
Prior this patch, we only emitted a warning, otherwise went as
default. This warnig is easy to be overlooked, and has effect on
security. From now, syslog-ng will not start in this case.

Signed-off-by: Antal Nemes <antal.nemes.hu@gmail.com>
@kira-syslogng
Copy link
Contributor

Build SUCCESS

Copy link
Collaborator

@alltilla alltilla left a comment

Choose a reason for hiding this comment

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

It might be easier to use, and maintain the code with #if CURL_AT_LEAST_VERSION(..., ..., ...)
We only have to check for 2 different versions: https://curl.haxx.se/libcurl/c/CURLOPT_SSLVERSION.html

Copy link
Collaborator

@lbudai lbudai left a comment

Choose a reason for hiding this comment

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

I think we should test syslog-ng with different TLS versions...

@lbudai lbudai merged commit 184e145 into syslog-ng:master Jan 20, 2020
@furiel furiel deleted the fix-curl-ssl-version branch February 6, 2020 06:10
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.

5 participants