-
Notifications
You must be signed in to change notification settings - Fork 4
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 thread safety issue in Symmetric cipher #31
Conversation
To make it easier to run tests in a container.
578edc5
to
7fa6a23
Compare
Previously using Symmetric encryption from multiple threads could result in errors from OpenSSL. This commit adds a mutex to synchronize access to the OpenSSL instance for encryption and decryption.
7fa6a23
to
c9408a0
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good. Any idea if there are other parts of Slosilo that call out to OpenSSL that may suffer similar problems?
@jvanderhoof Thanks! I didn't see any other issues like this in a quick survey through the code. All of the other uses of OpenSSL seem to be single function calls, or use this Symmetric class. |
For more detail, see: cyberark/slosilo#31
For more detail, see: cyberark/slosilo#31
For more detail, see: cyberark/slosilo#31
For more detail, see: cyberark/slosilo#31
For more detail, see: cyberark/slosilo#31
Desired Outcome
The desired outcome of this PR is to allow thread-safe use of the Symmetric cipher class.
Implemented Changes
This PR adds a synchronization mutex to the Symmetric class to manage thread
access to the critical portions of OpenSSL encryption and decryption.
Previously, if this steps were allowed to interleave among threads, it would corrupt
the OpenSSL state, leading to intermittent CipherErrors from OpenSSL.
Connected Issue/Story
CyberArk internal issue ID: CNJR-479
Definition of Done
At least 1 todo must be completed in the sections below for the PR to be
merged.
Changelog
CHANGELOG update
Test coverage
changes, or
Documentation
README
s) were updated in this PRBehavior
Security