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

OpenSSL: fix spurious SSL connection aborts #3054

Closed
wants to merge 1 commit into from

Conversation

koranyellow
Copy link

Description

Was seeing spurious SSL connection aborts using libmosquitto and OpenSSL. I tracked it down to uncleared error state on the OpenSSL error stack - patch attached deals with that.

Rough idea of problem:

Code that uses libmosquitto calls some library that uses OpenSSL but don't clear the OpenSSL error stack after an error. lib/net_mosq.c calls SSL_read which eventually gets an EWOULDBLOCK from the OS. Returns -1 to indicate an error lib/net_mosq.c calls SSL_get_error. First thing, SSL_get_error calls ERR_get_error to check the OpenSSL error stack, finds an old error and returns SSL_ERROR_SSL instead of SSL_ERROR_WANT_READ or SSL_ERROR_WANT_WRITE.

lib/net_mosq.c returns an error and aborts the connection

Solution:

Clear the openssl error stack before calling SSL_* operation if we're going to call SSL_get_error afterwards.

Notes:

This is much more likely to happen with multi because it's easier to intersperse other calls to the OpenSSL library in the same thread.


Thank you for contributing your time to the Mosquitto project!

Before you go any further, please note that we cannot accept contributions if
you haven't signed the Eclipse Contributor Agreement.
If you aren't able to do that, or just don't want to, please describe your bug
fix/feature change in an issue. For simple bug fixes it is can be just as easy
for us to be told about the problem and then go fix it directly.

Then please check the following list of things we ask for in your pull request:

  • Have you signed the Eclipse Contributor Agreement, using the same email address as you used in your commits?
  • Do each of your commits have a "Signed-off-by" line, with the correct email address? Use "git commit -s" to generate this line for you.
  • [-] If you are contributing a new feature, is your work based off the develop branch?
  • If you are contributing a bugfix, is your work based off the fixes branch?
  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you successfully run make test with your changes locally?

The problem appears after #commit.

Was seeing spurious SSL connection aborts using libmosquitto and OpenSSL. I tracked it down to uncleared error state on the OpenSSL error stack - patch attached deals with that.

- Rough idea of problem:

Code that uses libmosquitto calls some library that uses OpenSSL but don't clear the OpenSSL error stack after an error.
lib/net_mosq.c calls SSL_read which eventually gets an EWOULDBLOCK from the OS. Returns -1 to indicate an error
lib/net_mosq.c calls SSL_get_error. First thing, SSL_get_error calls ERR_get_error to check the OpenSSL error stack, finds an old error and returns SSL_ERROR_SSL instead of SSL_ERROR_WANT_READ or SSL_ERROR_WANT_WRITE.

lib/net_mosq.c returns an error and aborts the connection

Solution:
Clear the openssl error stack before calling SSL_* operation if we're going to call SSL_get_error afterwards.

Notes:
This is much more likely to happen with multi because it's easier to intersperse other calls to the OpenSSL library in the same thread.
ralight added a commit that referenced this pull request Sep 6, 2024
@ralight
Copy link
Contributor

ralight commented Sep 6, 2024

Thank you, I've added this change to a separate commit because you haven't signed the ECA - but there is no other way to implement this change. It will be in 2.0.19.

@ralight ralight closed this Sep 6, 2024
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.

2 participants