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

Fix TLS-based destinations in case of a missing client key/cert #1917

Merged
merged 3 commits into from
Mar 8, 2018

Conversation

MrAnno
Copy link
Collaborator

@MrAnno MrAnno commented Mar 7, 2018

key_file and cert_file are optional (the client cert is not mandatory).

Fixes #1916

@MrAnno MrAnno changed the title tls: fix tls_context_load_key_and_cert() Fix TLS-based destinations in case of a missing client key/cert Mar 7, 2018
@kira-syslogng
Copy link
Contributor

Build FAILURE, the tests were executed on test branch: master and test suite: functions

@MrAnno MrAnno force-pushed the tls-error-impr branch 2 times, most recently from 4d15bec to a69eec6 Compare March 7, 2018 18:08
key_file and cert_file are optional (the client cert is not mandatory).

Signed-off-by: László Várady <laszlo.varady@balabit.com>
@kira-syslogng
Copy link
Contributor

Build SUCCESS, the tests were executed on test branch: master and test suite: functions

@kira-syslogng
Copy link
Contributor

Build SUCCESS, the tests were executed on test branch: master and test suite: functions

1 similar comment
@kira-syslogng
Copy link
Contributor

Build SUCCESS, the tests were executed on test branch: master and test suite: functions

file_exists() has a side effect: it logs when the file can't be opened.

Since the && operator is evaluated lazily, the error messages of cert_file
were not displayed when the first invocation of file_exists failed
(on key_file).

Signed-off-by: László Várady <laszlo.varady@balabit.com>
Signed-off-by: László Várady <laszlo.varady@balabit.com>
@kira-syslogng
Copy link
Contributor

Build SUCCESS, the tests were executed on test branch: master and test suite: functions

1 similar comment
@kira-syslogng
Copy link
Contributor

Build SUCCESS, the tests were executed on test branch: master and test suite: functions

@lbudai lbudai merged commit 6108e47 into syslog-ng:master Mar 8, 2018
@MrAnno
Copy link
Collaborator Author

MrAnno commented Mar 8, 2018

@furiel I don't think it's possible. That optimization would be far from an equivalent transformation.

But yes, we should refactor this function, its side effect is really misleading.

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