Motivation:
BoringSSL supports to automatically manage the session tickets to be used and so also rotate them etc. This is often prefered by users as it removed some complexity. We should support to make use of this.
Modifications:
- Allow to have setSessionTickets() called without an argument or an empty array
- Add tests
Result:
Easier usage of session tickets
Motivation:
OpenSslSession.getLocalCertificates() and getLocalPrincipal() must return null on client side if mTLS is not used as stated in the API documentation. At the moment this is not always the case
Modifications:
- Ensure we only return non-null if mTLS is used
- Add unit tests
Result:
Follow SSLSession API contract
Motivation:
EC is better than RSA because of the small key size, efficient and secure which makes it perfect for testing purposes.
Modification:
Added support to specify an algorithm (EC or RSA) in constructors for key pair generation. The default key size is 256-bits as promulgated by NSA Suite B.
Result:
Able to generate a self-signed certificate of EC or RSA.
Motivation:
We are far behind with the version of Conscrypt we are using during testing. We should ensure we use the latest.
Modifications:
- Update conscrypt dependency
- Ensure we use conscrypt provider in tests
- Add workarounds for conscrypt bugs in testsuite
Result:
Use latest Conscrypt release
Motivation:
Currently calculateSize method in AbstractTrafficShapingHandler calculates size for object of type ByteBuf or ByteBufHolder. Adding a check for FileRegion, makes it possible to do traffic shaping for FileRegion objects as well
Modification:
Check if object to be sent is of type FileRegion, if yes calculate the size using its count() method.
Co-authored-by: Dinesh Joshi <dinesh.joshi@apple.com>
Motivation:
We need to make some slightly changes to be able to build on Java15 as some previous deprecated methods now throw UnsupportedOperationException
Modifications:
- Add code to handle UnsupportedOperationException
- Revert previous applied workaround for bug that was fixed in Java15
- Add maven profile
Result:
Be able to build with latest Java15 EA release
Motivation:
We should provide more informations when ALPN is not supported and a user tries to use it.
Modifications:
- Use UnsupportedOperationException
Result:
Easier to debug ALPN problems
Motivation:
ALPN support was backported to java 8 lately. Ensure we support it if the user uses the latest java 8 release
Modifications:
- Update logic to be able to detect if ALPN is supported out of the box when using Java8
- Update jetty alpn version
Result:
Be able to use ALPN out of the box when using java 8u251
Motivation:
The `AbstractTrafficShapingHandler` caches the `ReopenReadTimerTask` instance in the channel attribute. However, if this handler is removed from the channel pipeline, this `ReopenReadTimerTask` instance may not be released.
Modification:
Release the channel attribute `REOPEN_TASK` in `handlerRemoved` method.
Result:
Avoid a channel attribute leak.
Motivation:
Only cipher suite is logged during handshake. Picked protocol is interesting too.
Modification:
Log protocol as well.
Result:
More interesting information when debugging TLS handshakes.
**Motivation:**
Following up on https://github.com/netty/netty/issues/10181
I was able to successfully setup a Netty server with TLS on Android, using Conscrypt, with the change proposed here. Conscrypt publishes an [Android specific version](https://github.com/google/conscrypt#android). But the current availability check prevents Netty from using it (the call `PlatformDependent.javaVersion()` returns `6` on Android).
**Modification:**
Check whether the java version is above 8, or if it's running on an Android device, to flag Conscrypt as available (besides the remaining checks).
**Result:**
Netty with TLS runs fine on Android 👍
Motivation:
If the SslHandler is removed from the pipeline we also need to ensure we fail the handshake / close promise if it was not notified before as otherwise we may never do so.
Modifications:
- Correctly fail promise and notify pipeline if handshake was not done yet when the SslHandler is removed
- Add unit test
Result:
Fix https://github.com/netty/netty/issues/10158
Motivation:
JDK 14 was released and need some special settings to be able to build with. Also there seems to be one regression that we need to workaround for now.
Modifications:
- Add maven profile for JDK 14
- Update blockhound version to be able to work on JDK 14
- Add workaround for possible JDK 14 regression
Result:
Be able to build on JDK 14
Motivation:
Before release 4.1.23, there was only ONE call to exceptionCaught method when an ssl handshake failure occurs, now we have two when using the JDK provider.
Modifications:
- Ensure we only propagate one exception fi we already failed the handshake and channelInactive(...) produce an exception again
- Add unit test
Result:
Fixes https://github.com/netty/netty/issues/10119
Motivation:
In general, we will close the debug log in a product environment. However, logging without external level check may still affect performance as varargs will need to allocate an array.
Modification:
Add log level check simply before logging.
Result:
Improve performance slightly in a product environment.
Motivation:
Often it is useful to be able to detect different sorts of SSL errors that cause the handshake to fail. To make this easier we should throw and explicit exception type for handshake timeouts.
Modifications:
- Add SslHandshakeTimeoutException (which extends SSLHandshakeException) and use it for handshake timeouts
- Adjust testcases
Result:
Easier to detect that handshake failed because of a timeout
Motivation:
We did not correctly account for produced bytes when SSL_write(...) returns -1 in all cases. This could lead to lost data and so a corrupt SSL connection.
Modifications:
- Always ensure we calculate the produced bytes correctly
- Add unit tests
Result:
Fixes https://github.com/netty/netty/issues/10041
Motivation:
In SniHandlerTest we depended on implementation details of the SSLEngine. We should better not doing this
Modifications:
Just release all outbound data
Result:
Dont depend on implementation details
Motivation:
Due incorrectly handling of reference count of the clientHello ByteBuf we may overrelease the buffer. This did show up in the log of a test:
11:55:16.595 [main] DEBUG i.n.h.ssl.SslClientHelloHandler - Unexpected client hello packet: 16030100bd010000b90303a74225676d1814ba57faff3b3663656ed05ee9dbb2a4dbb1bb1c32d2ea5fc39e0000000100008c0000001700150000164348415434e380824c45414e434c4f5544e38082434e000b000403000102000a00340032000e000d0019000b000c00180009000a00160017000800060007001400150004000500120013000100020003000f0010001100230000000d0020001e060106020603050105020503040104020403030103020303020102020203000f00010133740000
io.netty.util.IllegalReferenceCountException: refCnt: 0, decrement: 1
at io.netty.util.internal.ReferenceCountUpdater.toLiveRealRefCnt(ReferenceCountUpdater.java:74)
at io.netty.util.internal.ReferenceCountUpdater.release(ReferenceCountUpdater.java:138)
at io.netty.buffer.AbstractReferenceCountedByteBuf.release(AbstractReferenceCountedByteBuf.java:100)
at io.netty.handler.ssl.SslClientHelloHandler.releaseIfNotNull(SslClientHelloHandler.java:181)
at io.netty.handler.ssl.SslClientHelloHandler.select(SslClientHelloHandler.java:225)
at io.netty.handler.ssl.SslClientHelloHandler.decode(SslClientHelloHandler.java:149)
at io.netty.handler.codec.ByteToMessageDecoder.decodeRemovalReentryProtection(ByteToMessageDecoder.java:498)
at io.netty.handler.codec.ByteToMessageDecoder.callDecode(ByteToMessageDecoder.java:437)
at io.netty.handler.codec.ByteToMessageDecoder.channelRead(ByteToMessageDecoder.java:276)
at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:377)
at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:363)
at io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:355)
at io.netty.channel.DefaultChannelPipeline$HeadContext.channelRead(DefaultChannelPipeline.java:1410)
at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:377)
at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:363)
at io.netty.channel.DefaultChannelPipeline.fireChannelRead(DefaultChannelPipeline.java:919)
at io.netty.channel.embedded.EmbeddedChannel.writeInbound(EmbeddedChannel.java:343)
at io.netty.handler.ssl.SniHandlerTest.testNonAsciiServerNameParsing(SniHandlerTest.java:297)
at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
at java.lang.reflect.Method.invoke(Method.java:498)
at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:50)
at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:47)
at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)
at org.junit.internal.runners.statements.ExpectException.evaluate(ExpectException.java:19)
at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:325)
at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:78)
at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:57)
at org.junit.runners.ParentRunner$3.run(ParentRunner.java:290)
at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:71)
at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:288)
at org.junit.runners.ParentRunner.access$000(ParentRunner.java:58)
at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:268)
at org.junit.runners.ParentRunner.run(ParentRunner.java:363)
at org.junit.runners.Suite.runChild(Suite.java:128)
at org.junit.runners.Suite.runChild(Suite.java:27)
at org.junit.runners.ParentRunner$3.run(ParentRunner.java:290)
at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:71)
at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:288)
at org.junit.runners.ParentRunner.access$000(ParentRunner.java:58)
at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:268)
at org.junit.runners.ParentRunner.run(ParentRunner.java:363)
at org.junit.runner.JUnitCore.run(JUnitCore.java:137)
at com.intellij.junit4.JUnit4IdeaTestRunner.startRunnerWithArgs(JUnit4IdeaTestRunner.java:68)
at com.intellij.rt.execution.junit.IdeaTestRunner$Repeater.startRunnerWithArgs(IdeaTestRunner.java:47)
at com.intellij.rt.execution.junit.JUnitStarter.prepareStreamsAndStart(JUnitStarter.java:242)
at com.intellij.rt.execution.junit.JUnitStarter.main(JUnitStarter.java:70)
Modifications:
Correctly transfer lifetime of buffer and so not over-release it.
Result:
Correctly handle buffer lifecycle and so not swallow the original exception
Motivation:
Sometimes it is useful to do something depending on the Ssl ClientHello (like for example select a SslContext to use). At the moment we only allow to hook into the SNI extension but this is not enough.
Modifications:
Add SslClientHelloHandler which allows to hook into ClientHello messages. This class is now also the super class of AbstractSniHandler
Result:
More flexible processing of SSL handshakes
Motivation:
JDK is the default SSL provider and internally uses blocking IO operations.
Modifications:
Add allowBlockingCallsInside configuration for SslHandler runAllDelegate function.
Result:
When BlockHound is installed, SSL works out of the box with the default SSL provider.
Co-authored-by: violetagg <milesg78@gmail.com>
Motivation:
When SslHandler.finishWrap throws an exception, ensure that the promise and buf is not reused to avoid throwing IllegalArgumentException or IllegalReferenceCountException which causes the original exception to be lost.
Modification:
The change ensures that the values for the promise and bytebuf are nulled before calling finishWrap so that it will not be called again with the same arguments.
Result:
Fixes#9971 .
Co-authored-by: Norman Maurer <norman_maurer@apple.com>
Co-authored-by: Antony T Curtis <atcurtis@gmail.com>
Motivation:
In the PR #9003, the issue #8998 was supposedly solved. But actually the fix only made it so the TrustManager can use the specified keystore type instead of also supporting it in the KeyManagerFactory.
In my environment, we are using PKCS#11 Keys which are then rejected by the PKCS#12 keystore. A PKCS#12 keystore is created since the keystoreType was set to null by the code from the mentioned PR.
Modification:
Do not ignore the keystoreType parameter during the creation of the JdkSslClientContext KeyManagerFactory.
Result:
Fixes#8998.
Motivation
LoggingHandler is a very useful tool for debugging and for tracking the
sequence of events in a pipeline. LoggingHandler also includes the
functionality to log a hex dump of all written and received ByteBufs.
This can be useful for small messages, but for large messages, this can
potentially result in extremely large logs. E.g., a 1 MB payload will
result in over a 1 MB log message being recorded. While LoggingHandler
may only be intended for debugging, this can still be too excessive in
some debugging scenarios.
Modifications
* Create a new ByteBufFormat enum that allows users to specify "SIMPLE"
or "HEX_DUMP" logging for ByteBufs.
* For all constructors that currently accept a LogLevel parameter,
create new overloaded constructors that also accept this enum as a
parameter.
* Continue to record hex dumps by default.
Result
Users will be able to opt out of full hex dump recording, if they wish
to.
Motivation:
At the moment we create a new ChannelFutureListener per chunk when trying to write these to the underlying transport. This can be optimized by replacing the seperate write and flush call with writeAndFlush and only allocate the listener if the future is not complete yet.
Modifications:
- Replace seperate write and flush calls with writeAndFlush
- Only create listener if needed, otherwise execute directly
Result:
Less allocations
Motivation:
At the moment we use an extra field in ChunedWriteHandler to hold the current write. This is not needed and makes sense even more error-prone. We can just peek in the queue.
Modifications:
Use Queue.peek() to keep track of current write
Result:
Less error-prone code
Motivation:
At the moment resolving addresses during connect is done via setting an AddressResolverGroup on the Bootstrap. While this works most of the times as expected sometimes the user want to trigger the connect() from the Channel itself and not via the Bootstrap. For this cases we should provide a ChannelHandler that the user can use that will do the resolution.
Modifications:
Add ResolveAddressHandler and tests
Result:
Be able to resolve addresses without Bootstrap
Motivation:
41c47b4 introduced a change in an existing testcase which let the build fail when jdkCompatibilityMode is false.
Modifications:
Fix unit tests
Result:
Build passes when jdkCompatibilityMode is false as well
Motivation
A bug was introduced in #9806 which looks likely to be the cause of
#9919. SniHandler will enter an infinite loop if an SSL record is
received with SSL major version byte != 3 (i.e. something other than TLS
or SSL3.0)
Modifications
- Follow default path as intended for majorVersion != 3 in
AbstractSniHandler#decode(...)
- Add unit test to reproduce the hang
Result
Fixes#9919
Motivation:
When `consolidatedWhenNoReadInProgress` is true, `channel.writeAndFlush (data) .addListener (f-> channel.writeAndFlush (data2))` Will cause data2 to never be flushed.
Because the flush operation will synchronously execute the `channel.writeAndFlush (data2))` in the `listener`, and at this time, since the current execution thread is still an `eventloop`(`executor.inEventLoop()` was true), all handlers will be executed synchronously. At this time, since `nextScheduledFlush` is still not null, the `flush` operation of `data2` will be ignored in `FlushConsolidationHandler#scheduleFlush`.
Modification:
- reset `nextScheduledFlush` before `ctx.flush`
- use `ObjectUtil` to polish code
Result:
Fixes https://github.com/netty/netty/issues/9923
Motivation:
At the moment our AbstractSniHandler makes the assemption that Handshake messages are not fragmented. This is incorrect as it is completely valid to split these across multiple TLSPlaintext records.
Thanks to @sskrobotov for bringing this to my attentation and to @Lukasa for the help.
Modifications:
- Adjust logic in AbstractSniHandler to handle fragmentation
- Add unit tests
Result:
Correctly handle fragmented Handshake message in AbstractSniHandler (and so SniHandler).
Motivation:
This is a PR to solve the problem described here: https://github.com/netty/netty/issues/9767
Basically this PR is to add two more APIs in SslContextBuilder, for users to directly specify
the KeyManager or TrustManager they want to use when building SslContext. This is very helpful
when users want to pass in some customized implementation of KeyManager or TrustManager.
Modification:
This PR takes the first approach in here:
https://github.com/netty/netty/issues/9767#issuecomment-551927994 (comment)
which is to immediately convert the managers into factories and let factories continue to pass
through Netty.
1. Add in SslContextBuilder the two APIs mentioned above
2. Create a KeyManagerFactoryWrapper and a TrustManagerFactoryWrapper, which take a KeyManager
and a TrustManager respectively. These are two simple wrappers that do the conversion from
XXXManager class to XXXManagerFactory class
3.Create a SimpleKeyManagerFactory class(and internally X509KeyManagerWrapper for compatibility),
which hides the unnecessary details such as KeyManagerFactorySpi. This serves the similar
functionalities with SimpleTrustManagerFactory, which was already inside Netty.
Result:
Easier usage.
Motivation:
MINIMAL_WAIT is the key constant. Thus, When we see the constant, we must read more code logic to see if it is ms or ns. So improving java doc will be better.
Modifications:
Improve java doc by add "10ms" such as DEFAULT_CHECK_INTERVAL with "1s".
Result:
Easy to know it is ms and keep same java doc style with other constants such as DEFAULT_CHECK_INTERVAL.
