Motivation:
Users should use PromiseNotifier.cascade(...) methods and so the UnaryPromiseNotifier becomes useless.
Modifications:
- Mark UnaryPromiseNotifier as deprecated
- Replaces usages with PromiseNotifier.cascade(...)
Result:
Cleanup
Motivation:
There are some redundant imports and unnecessary type cast
Modification:
Remove useless imports and unnecessary type cast
Result:
The code is cleaner than original
Signed-off-by: xingrufei <xingrufei@sogou-inc.com>
Motivation:
Overspecifying interfaces can increase coupling and make refactoring harder down the line.
Modification:
Places that were specifying fields, variables, and return types as ScheduledFuture, but did not use any features specific to ScheduledFuture, have been changed to specify Future instead.
Result:
Cleaner code.
Motivation:
When we override a test with params we also need to ensure we add the correct annotations to the override
Modifications:
Add the correct annotations so the tests are actually run in intellij
Result:
Each test can be run
Motivation:
While OpenSSK is provided support for the "RSASSA-PSS" algorithm this was still not valid from netty. Which was causing issue in validating certificates which was signed using this algorithm.
Modification:
Added "RSASSA-PSS" in LOCAL_SUPPORTED_SIGNATURE_ALGORITHMS.
validation:
Validated and tested with CA and User cert singed with RSASSA-PSS algorithm.
Result:
Fixes#11360
Co-authored-by: Norman Maurer <norman_maurer@apple.com>
Motivation:
We should allow server initiated renegotiation when OpenSSL / BoringSSL bases SSLEngine is used as it might be used for client auth.
Modifications:
- Upgrade netty-tcnative version to be able to allow renegotiate once
- Adjust code
Result
Fixes https://github.com/netty/netty/issues/11529
Motivation:
Make SslHandler's handlerRemoved0 method release the sslEngine
even if it fails in the middle.
See details in https://github.com/netty/netty/issues/11595.
Modifications:
Wrap the release of sslEngine into a finally block.
Result:
The sslEngine would be released eventually.
Co-authored-by: Chen Liu <cliu@splunk.com>
Motivation:
We tightly integrate the TrustManger and KeyManager into our native SSL implementation which means that both of them need to support TLSv1.3 as protocol. This is not always the case and so can produce runtime exceptions.
As TLSv1.3 support was backported to Java8 quite some time now we should be a bit more conservative and only enable TLSv1.3 for our native implementation if the JDK implementation supports it as well. This also allows us to remove some hacks we had in place to be able to support it before in Java8.
Modifications:
- Only enable TLSv1.3 support for our native SSL implementation when the JDK supports it as well
- Remove OpenSslTlsv13X509ExtendedTrustManager as its not needed anymore
Result:
Fixes https://github.com/netty/netty/issues/11589
Motivation:
At the moment why you try to build a SslContext via SslProvider.OPENSSL* and netty-tcnative* is not on the classpath it will fail with:
```
Exception in thread "main" java.lang.NoClassDefFoundError: io/netty/internal/tcnative/SSLPrivateKeyMethod
at io.netty.handler.ssl.SslContext.newClientContextInternal(SslContext.java:830)
at io.netty.handler.ssl.SslContextBuilder.build(SslContextBuilder.java:611)
at io.netty.handler.codec.http.Main.main(Main.java:34)
Caused by: java.lang.ClassNotFoundException: io.netty.internal.tcnative.SSLPrivateKeyMethod
at java.base/jdk.internal.loader.BuiltinClassLoader.loadClass(BuiltinClassLoader.java:581)
at java.base/jdk.internal.loader.ClassLoaders$AppClassLoader.loadClass(ClassLoaders.java:178)
at java.base/java.lang.ClassLoader.loadClass(ClassLoader.java:522)
... 3 more
```
We should do better here.
Modifications:
Call `OpenSsl.ensureAvailability()` before trying to construct OpenSsl*Context instances
Result:
More clear error message like:
```
java.lang.UnsatisfiedLinkError: failed to load the required native library
at io.netty.handler.ssl.OpenSsl.ensureAvailability(OpenSsl.java:540)
at io.netty.handler.ssl.SslContext.newClientContextInternal(SslContext.java:830)
at io.netty.handler.ssl.SslContextBuilder.build(SslContextBuilder.java:611)
...
Caused by: java.lang.ClassNotFoundException: io.netty.internal.tcnative.SSLContext
at java.base/jdk.internal.loader.BuiltinClassLoader.loadClass(BuiltinClassLoader.java:581)
at java.base/jdk.internal.loader.ClassLoaders$AppClassLoader.loadClass(ClassLoaders.java:178)
at java.base/java.lang.ClassLoader.loadClass(ClassLoader.java:522)
at java.base/java.lang.Class.forName0(Native Method)
at java.base/java.lang.Class.forName(Class.java:398)
at io.netty.handler.ssl.OpenSsl.<clinit>(OpenSsl.java:126)
... 68 more
```
Motivation:
Sometimes intellij fails to build because the compiler cant really figure out what to do. We can workaround this by adding some explicit casts.
Modifications:
Add explicits casts
Result:
No more problems with intellij
Motivation:
0c9a86db81 added a change to log a message if someone tried to change the TLSv1.3 ciphers when using BoringSSL. Unfortunally the code had some error and so even if the user did not change these we logged something.
Modifications:
- Ensure there are no duplicates in the ciphers
- Correctly take TLSv1.3 extra ciphers into account when using BoringSSL
Result:
Correctly log or not log
Motivation:
We should make variables `final` which are not reinstated again in code to match the code style and makes the code look better.
Modification:
Made couples of variables as `final`.
Result:
Variables marked as `final`.
Motivation:
We should keep bitwise operations simple and easy to understand.
Modification:
Simplify few Bitwise operations.
Result:
Less complicated bitwise operation code
Motivation:
We should get rid of the unnecessary toString calls because they're redundant in nature.
Modification:
Removed unnecessary toString calls.
Result:
Better code
Motivation:
We should get rid of unnecessary semicolons because they don't do anything in code.
Modification:
Removed unnecessary semicolons.
Result:
Better code
Motivation:
There are lots of imports which are unused. We should get rid of them to make the code look better,
Modification:
Removed unused imports.
Result:
No unused imports.
Motivation:
This bug could occasionally cause SSL handshakes to time out, because the server-side handshake would fail to resume its event loop.
Modification:
Async delegate SSL tasks now lower their NEED_TASK status after they have executed, but before they run their completion callback.
This is important because the completion callback could be querying the handshake status.
This could cause the task delegator thread and the event look to race.
If the event look queries the handshake status first, it might think that it still needs to delegate another task.
If this happens, the delegator find a null task, and then fail to resume the event loop, causing the handshake to stall.
Result:
This data race no longer causes handshake timeouts.
Motivation:
We need to ensure we call wrap as long as there is something left to be send to the remote peer in cases of non-application data (like for example alerts).
Modifications:
Check the pending data and based on it return NEED_WRAP even when the handshake was done.
Result:
Always produce alerts etc
Motivation:
750d235 did introduce a change in ApplicationProtocolNegotiationHandler that let the handler buffer all inbound data until the SSL handshake was completed. Due of this change a user might get buffer data forever if the SslHandler is not in the pipeline but the ApplicationProtocolNegotiationHandler was added. We should better guard the user against this "missconfiguration" of the ChannelPipeline.
Modifications:
- Remove the ApplicationProtocolNegotiationHandler when no SslHandler is present when the first message was received
- Add unit test
Result:
No possible that we buffer forever
Motivation:
We did observe that the mutal auth tests are flaky on windows when running on the CI. Let's disable these for now.
Modifications:
Disable mutual auth tests on windows
Result:
More stable build. Related to https://github.com/netty/netty/issues/11489
Motivation:
SelfSignedCertificate generates EC/RSA key pair and this should be explicitly mentioned in JavaDoc. Currently, only "RSA" was mentioned not "EC".
Modification:
Changed RSA to EC/RSA
Result:
Correct JavaDoc
Motivation:
As the release of JDK17 is getting closer and there are ea builds already we should ensure we can actually build netty with it.
Modifications:
- Add profile for JDK17
- Remove test-code that would fail with JDK17 due the changes in 4f4d0f5366.
Result:
Be able to build and run testsuite with JDK17
Motivation:
At the moment we not correctly propagate cancellation in some case when we use the PromiseNotifier.
Modifications:
- Add PromiseNotifier static method which takes care of cancellation
- Add unit test
- Deprecate ChannelPromiseNotifier
Result:
Correctly propagate cancellation of operation
Co-authored-by: Nitesh Kant <nitesh_kant@apple.com>
Motivation:
In JDK version >= 9 the access to sun.* is not permitted anymore by default. Because of this we should better first try the BouncyCastle based implementation before falling back to the JDK based version.
Modifications:
Switch ordering of usage of BouncyCastle vs JDK internals.
Result:
Less surprising errors when using SelfSignedCertificate in Java >9
Motivation:
JdkOpenSslEngineInteroptTest.testMutualAuthSameCerts() is flaky on the CI and so fails the PR build quite often.
Let's disable it for now until we were able to reproduce it locally and fix it.
Modifications:
Disable flaky test
Result:
More stable CI builds
Motivation:
At the moment we only support signing / decrypting the private key in a synchronous fashion. This is quite limited as we may want to do a network call to do so on a remote system for example.
Modifications:
- Update to latest netty-tcnative which supports running tasks in an asynchronous fashion.
- Add OpenSslAsyncPrivateKeyMethod interface
- Adjust SslHandler to be able to handle asynchronous task execution
- Adjust unit tests to test that asynchronous task execution works in all cases
Result:
Be able to asynchronous do key signing operations
Motivation:
We should only run one SSL task per delegation to allow more SSLEngines to make progress in a timely manner
Modifications:
- Only run one task per delegation to the executor
- Only create new SSL task if really needed
- Only schedule if not on the EventExecutor thread
Result:
More fair usage of resources and less allocations
Motivation:
Protocols and Cipher suites constants to prevent typos in protocol and cipher suites names and ease of use.
Modification:
Added Protocols and Cipher suites as constants in their respective classes.
Result:
Fixes#11393
Motivation:
7c57c4be17 did add a way to async sign keys but did not guard against the handler been removed before try to wrap in cause of an error which could lead to a harmless NPE.
Modifications:
Add check
Result:
No more harmless NPE
__Motivation__
`ApplicationProtocolNegotiationHandler` buffers messages which are read before SSL handshake complete event is received and drains them when the handler is removed. However, the channel may be closed (or input shutdown) before SSL handshake event is received in which case we may fire channel read after channel closure (from `handlerRemoved()`).
__Modification__
Intercept `channelInactive()` and input closed event and drain the buffer.
__Result__
If channel is closed before SSL handshake complete event is received, we still maintain the order of message read and channel closure.
Co-authored-by: Norman Maurer <norman_maurer@apple.com>
Motivation:
At the moment we only support signing / decrypting the private key in a synchronous fashion. This is quite limited as we may want to do a network call to do so on a remote system for example.
Modifications:
- Update to latest netty-tcnative which supports running tasks in an asynchronous fashion.
- Add OpenSslAsyncPrivateKeyMethod interface
- Adjust SslHandler to be able to handle asynchronous task execution
- Adjust unit tests to test that asynchronous task execution works in all cases
Result:
Be able to asynchronous do key signing operations
Motivation:
Currently, Netty cannot handle HTTP/2 Preface messages if the client used the Prior knowledge technique. In Prior knowledge, the client sends an HTTP/2 preface message immediately after finishing TLS Handshake. But in Netty, when TLS Handshake is finished, ALPNHandler is triggered to configure the pipeline. And between these 2 operations, if an HTTP/2 preface message arrives, it gets dropped.
Modification:
Buffer messages until we are done with the ALPN handling.
Result:
Fixes#11403.
Co-authored-by: Norman Maurer <norman_maurer@apple.com>
__Motivation__
Add support for GMSSL protocol to SslUtils.
__Modification__
Modify `SslUtils.getEncryptedPacketLength(ByteBuf buffer, int offset)` to get packet length when protocol is GMSSL.
Modify `SslUtils.getEncryptedPacketLength(ByteBuffer buffer)` to get packet length when protocol is GMSSL.
__Result__
`SslUtils.getEncryptedPacketLength` now supports GMSSL protocol. Fixes https://github.com/netty/netty/issues/11406
__Motivation__
`LoggingHandler` misses a constructor variant that only takes `ByteBufFormat`
__Modification__
Added the missing constructor variant.
__Result__
`LoggingHandler` can be constructed with `ByteBufFormat` only.
Co-authored-by: Nitesh Kant <nitesh_kant@apple.com>
Motivation:
We need to ensure we always "consumed" all alerts etc via SSLEngine.wrap(...) before we teardown the engine. Failing to do so may lead to a situation where the remote peer will not be able to see the actual cause of the handshake failure but just see the connection being closed.
Modifications:
Correctly return HandshakeStatus.NEED_WRAP when we need to wrap some data first before we shutdown the engine because of a handshake failure.
Result:
Fixes https://github.com/netty/netty/issues/11388
Motivation:
At the moment BoringSSL doesnt support explicit set the TLSv1.3 ciphers that should be used. If TLSv1.3 should be used it just enables all ciphers. We should better log if the user tries to explicit set a specific ciphers and using BoringSSL to inform the user that what is tried doesnt really work.
Modifications:
Log if the user tries to not use all TLSv1.3 ciphers and use BoringSSL
Result:
Easier for the user to understand why always all TLSv1.3 ciphers are enabled when using BoringSSL
Co-authored-by: Trustin Lee <trustin@gmail.com>
Norman Maurer
skyguard1
Chris Vest
kushalagrawal
Chen Liu
Netty Project Bot
Aayush Atharva
Ikko Ashimine
Nitesh Kant
wujimin
Unev