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.

Sign up for GitHub

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

Jump to bottom

SecureStreamSocket is not thread-safe #4435

Closed
obiltschnig opened this issue Feb 1, 2024 · 0 comments
Closed

SecureStreamSocket is not thread-safe #4435

obiltschnig opened this issue Feb 1, 2024 · 0 comments
Assignees
@obiltschnig
Labels
bug fixed
Milestone
Release 1.13.3

Comments

@obiltschnig
Copy link
Member

obiltschnig commented Feb 1, 2024 •
edited
Loading

Plain sockets are thread-safe with regards to different threads calling sendBytes() and receiveBytes() simultaneously.
This is not the case with SecureStreamSocket. While it seems to work most of the time, there are situations (e.g., if a TLS renegotiation takes place) where an error:0A00010F:SSL routines::bad length error is seen, indicating that there are mismatched calls to SSL_write() and/or SSL_read() in case SSL_ERROR_WANT_READ or SSL_ERROR_WANT_WRITE was returned previously.

A fix would be to protect SecureSocketImpl::sendBytes() and SecureSocketImpl::receiveBytes() (and probably other methods as well) with a mutex.
The OpenSSL documentation states that operations on SSL objects are not thread-safe, so these should be protected.
@obiltschnig obiltschnig added the bug label Feb 1, 2024
@obiltschnig obiltschnig added this to the Release 1.13.1 milestone Feb 1, 2024
@obiltschnig obiltschnig self-assigned this Feb 1, 2024
@aleks-f aleks-f added this to 1.13 Feb 4, 2024
aleks-f added a commit that referenced this issue Mar 26, 2024
@aleks-f
fix(SecureSocketImpl): not thread-safe (1st attempt) #4435
fc7504f
@matejk matejk moved this to In Progress in 1.13 Mar 29, 2024
aleks-f added a commit that referenced this issue Apr 2, 2024
@aleks-f @matejk
4435 secure sock thread (#4512)
ece3603 
* fix(SecureSocket): Refactor detection of timeout when reading, writing or handshaking. (#3725)

* enh(SecureSocket): some trivial C++17 modernisation changes.

* chore: indentation and compiler warning

* fix(SecureSocketImpl): not thread-safe (1st attempt) #4435

* fix(SecureSocketImpl): silence CodeQL cpp/certificate-not-checked

---------

Co-authored-by: Matej Kenda <[email protected]>
@aleks-f aleks-f added the fixed label Apr 2, 2024
@aleks-f aleks-f moved this from In Progress to Done in 1.13 Apr 2, 2024
aleks-f added a commit that referenced this issue Apr 2, 2024
@aleks-f @matejk
4435 secure sock thread (#4512)
facf937 
* fix(SecureSocket): Refactor detection of timeout when reading, writing or handshaking. (#3725)

* enh(SecureSocket): some trivial C++17 modernisation changes.

* chore: indentation and compiler warning

* fix(SecureSocketImpl): not thread-safe (1st attempt) #4435

* fix(SecureSocketImpl): silence CodeQL cpp/certificate-not-checked

---------

Co-authored-by: Matej Kenda <[email protected]>
@matejk matejk closed this as completed Apr 4, 2024
@acarioni acarioni mentioned this issue Apr 8, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@obiltschnig obiltschnig

Labels
bug fixed
Projects
No open projects
1.13
Status: Done
Milestone
Release 1.13.3
Development

No branches or pull requests
3 participants
@obiltschnig @matejk @aleks-f