OpenSslEngine return NEED_WRAP if the destination buffered filled #6803
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Scottmitch
commented
Jun 1, 2017
| SSLEngineResult result = wrap(alloc, engine, Unpooled.EMPTY_BUFFER, out); | ||
|
|
||
| if (previousNeedUnwrap) { |
There was a problem hiding this comment.
changes to SslHandler are just to support debugging and will be removed if this PR is accepted.
|
@rkapsi - would you mind trying this PR with OPENSSL and report back the same log statements you did as in #6796 (comment)? |
|
@Scottmitch - will do first thing in the morning tomorrow |
|
@rkapsi verified this patch works as expected in #6796 (comment) |
normanmaurer
approved these changes
Jun 2, 2017
Scottmitch
force-pushed
the
openssl_wrap_status
branch
from
b833440
to
394dce3
Compare
Motivation: If the destination buffer is completely filled during a call to OpenSslEngine#wrap(..) we may return NEED_UNWRAP because there is no data pending in the SSL buffers. However during a handshake if the SSL buffers were just drained, and filled up the destination buffer it is possible OpenSSL may produce more data on the next call to SSL_write. This means we should keep trying to call SSL_write as long as the destination buffer is filled and only return NEED_UNWRAP when the destination buffer is not full and there is no data pending in OpenSSL's buffers. Modifications: - If the handshake produces data in OpenSslEngine#wrap(..) we should return NEED_WRAP if the destination buffer is completely filled Result: OpenSslEngine returns the correct handshake status from wrap(). Fixes netty#6796.
Scottmitch
force-pushed
the
openssl_wrap_status
branch
from
394dce3
to
b5ff340
Compare
|
I will pull this in now as even if it doesn't fix #6796 it is still an issue, and the behavior from the log statements provided in #6796 (comment) demonstrates the correct behavior. |
|
@Scottmitch running 24f801c built from HEAD of 4.1 now. Looks good, no issues. 💯 |
Scottmitch
added a commit
to Scottmitch/netty
that referenced
this pull request
Jun 2, 2017
Motivation: PR netty#6803 corrected an error in the return status of the OpenSslEngine. We should now be able to restore the SslHandler optimization. Modifications: - This reverts commit 7f3b75a. Result: SslHandler optimization is restored.
liuzhengyang
pushed a commit
to liuzhengyang/netty
that referenced
this pull request
Sep 10, 2017
Motivation: PR netty#6803 corrected an error in the return status of the OpenSslEngine. We should now be able to restore the SslHandler optimization. Modifications: - This reverts commit 7f3b75a. Result: SslHandler optimization is restored.
kiril-me
pushed a commit
to kiril-me/netty
that referenced
this pull request
Feb 28, 2018
Motivation: PR netty#6803 corrected an error in the return status of the OpenSslEngine. We should now be able to restore the SslHandler optimization. Modifications: - This reverts commit 7f3b75a. Result: SslHandler optimization is restored.
pulllock
pushed a commit
to pulllock/netty
that referenced
this pull request
Oct 19, 2023
Motivation: PR netty#6803 corrected an error in the return status of the OpenSslEngine. We should now be able to restore the SslHandler optimization. Modifications: - This reverts commit 7f3b75a. Result: SslHandler optimization is restored.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Motivation:
If the destination buffer is completely filled during a call to OpenSslEngine#wrap(..) we may return NEED_UNWRAP because there is no data pending in the SSL buffers. However during a handshake if the SSL buffers were just drained, and filled up the destination buffer it is possible OpenSSL may produce more data on the next call to SSL_write. This means we should keep trying to call SSL_write as long as the destination buffer is filled and only return NEED_UNWRAP when the destination buffer is not full and there is no data pending in OpenSSL's buffers.
Modifications:
Result:
OpenSslEngine returns the correct handshake status from wrap().
Fixes #6796.