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

amqp: handle error from amqp_ssl_socket_new() #3929

Merged
merged 6 commits into from Feb 28, 2022

Conversation

alltilla
Copy link
Collaborator

amqp_ssl_socket_new() can return with NULL, which causes a segfault in amqp_ssl_socket_set_cacert().
This change does not fix the problem coming from amqp_ssl_socket_new(), but handles its error gracefully.

Fixes #3924

Signed-off-by: Attila Szakacs attila.szakacs@oneidentity.com

Signed-off-by: Attila Szakacs <attila.szakacs@oneidentity.com>
Signed-off-by: Attila Szakacs <attila.szakacs@oneidentity.com>
Signed-off-by: Attila Szakacs <attila.szakacs@oneidentity.com>
Signed-off-by: Attila Szakacs <attila.szakacs@oneidentity.com>
Though in the current implementation it only returns NULL if calloc()
fails, which has more serious consequences, it is better to be safe and
check it anyways.

Signed-off-by: Attila Szakacs <attila.szakacs@oneidentity.com>
Signed-off-by: Attila Szakacs <attila.szakacs@oneidentity.com>
@MrAnno MrAnno added this to the syslog-ng-3.36 milestone Feb 28, 2022
@kira-syslogng
Copy link
Contributor

Build SUCCESS

@czanik
Copy link
Contributor

czanik commented Feb 28, 2022

@alltilla : The problem was originally reported with syslog-ng 3.23, as that is the version in EPEL 8. Does this patch work with 3.23? As in that case I can create an updated package for EPEL 8. Unfortunately EPEL uses fixed versions, so I'm not allowed to update to 3.36 :/

Copy link
Collaborator

@MrAnno MrAnno left a comment

Choose a reason for hiding this comment

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

Nice catch.

@alltilla
Copy link
Collaborator Author

@czanik
Unfortunately it does not apply cleanly, but I can backport this to 3.23.

I would emphasize that we cannot fix the root of the problem, this change only makes sure that syslog-ng does not crash, instead it reports an error message. So in order to make amqp()'s tls() work, the user needs to upgrade or downgrade from librabbitmq v0.9.0.

@czanik
Copy link
Contributor

czanik commented Feb 28, 2022

@alltilla If it does not work anyway, then it is not worth the effort (doing the backport, preparing a package, submitting it and praying for testers to check it out).

In this case we should simply wait for an updated AMQP package. I can try to contact the package maintainer.

@gaborznagy gaborznagy merged commit 57f309e into syslog-ng:master Feb 28, 2022
@alltilla alltilla mentioned this pull request Feb 28, 2022
6 tasks
@czanik
Copy link
Contributor

czanik commented Feb 28, 2022

Oops. It's part of the base system, not in EPEL. I'll try to contact the RHEL maintainer, but I suspect that even if it works, it will take a lot of time.

A quicker way forward might be adding an up-to-date version of rabbitmq to my repo and use that. At least as a temporary solution.

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.

Segmentation fault running under AlmaLinux
5 participants