Skip to content

openssl: Clear error queue after an incomplete SSL_shutdown #29

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

Closed
wants to merge 1 commit into from

Conversation

manuelm
Copy link
Contributor

@manuelm manuelm commented Dec 7, 2016

If the SSL_shutdown-call fails (e.g. because the underlaying socket has
already been closed) OpenSSL puts the corresponding error into the
queue. We don't care about details so we need to clear the queue.

Otherwise the error will be pulled while error checking the next OpenSSL
call of an unrelated connection.

Issue can be reproduced by:

  • setting service imap-login { service_count = 0 }
  • imap-connection 1 (TLS) is doing a NOOP every second
  • tcp-connection 2 is connecting to and immediately disconnecting from the SSL/TLS port (without any handshake).

@cmouse
Copy link
Contributor

cmouse commented Dec 7, 2016

Thanks we'll take a look at this.

@BinaryKhaos
Copy link

A related change in OpenSSL (SSL_shutdown() no longer returns success if called early in the handshake, i.e. connection closed early and the handshake never went through) that triggered problems in Dovecot:
openssl/openssl@64f9f40

@sirainen
Copy link
Contributor

sirainen commented Dec 7, 2016

I'm a bit worried about this: "because the underlaying socket has already been closed" - do you see EBADF errors because OpenSSL is trying to access an already-closed socket fd? If yes, that is still a bug that should be fixed, since with bad luck maybe the socket is already used by something else and that unrelated socket is disconnected.

@manuelm
Copy link
Contributor Author

manuelm commented Dec 7, 2016

As far as I remember in my example above it's ECONNRESET. OpenSSL then puts SSL_R_SHUTDOWN_WHILE_IN_INIT into the error queue. And the error checking in the next SSL_read-loop will revive the unrelated error from the queue and bail out.

So with closed I'm mean the other side has closed the connection and the bidirectional SSL shutdown can't happen any more.

@manuelm manuelm changed the title openssl: Fix dropping unrelated connections after an incomplete SSL_shutdown openssl: Clear error queue after after an incomplete SSL_shutdown Dec 8, 2016
@manuelm
Copy link
Contributor Author

manuelm commented Dec 8, 2016

I've changed the title to better reflect the actual change. Dropping the unrelated connection is the symptom. If desired I can amend the commit as well.

@manuelm manuelm changed the title openssl: Clear error queue after after an incomplete SSL_shutdown openssl: Clear error queue after an incomplete SSL_shutdown Dec 8, 2016
@cmouse
Copy link
Contributor

cmouse commented Dec 9, 2016

Please do amend, it's better if the commit message correctly describes the change.

If the SSL_shutdown-call fails (e.g. because the underlaying socket has
already been closed) OpenSSL puts the corresponding error into the
queue. We don't care about details so we need to clear the queue.

Otherwise the error will be pulled while error checking the next OpenSSL
call of an unrelated connection.
@manuelm
Copy link
Contributor Author

manuelm commented Dec 9, 2016

amended

@cmouse
Copy link
Contributor

cmouse commented Dec 9, 2016

merged.

@cmouse cmouse closed this Dec 9, 2016
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.

4 participants