Motivation:
We need to ensure we handle the case when BUFFER_OVERFLOW happens during unwrap but the readable bytes are bigger then the expected applicationBufferSize. Otherwise we may produce an IllegalArgumentException as we will try to allocate a buffer with capacity < 0.
Modifications:
- Guard against this case.
- Ensure we not double release buffer on exception when doing unwrap.
Result:
No more exception when running testsuite with java 9.
Motivation:
boringssl uses different messages for the ssl alerts which are all uppercase. As we try to match case as well this fails in SSLErrorTest as we expect lower-case.
This test was introduced by 9b7fb2f362.
Modifications:
Ensure we first translate everything to lower-case before doing the assert.
Result:
SSLErrorTest also pass when boringssl is used.
Motivation:
We need to ensure the tracked object can not be GC'ed before ResourceLeak.close() is called as otherwise we may get false-positives reported by the ResourceLeakDetector. This can happen as the JIT / GC may be able to figure out that we do not need the tracked object anymore and so already enqueue it for collection before we actually get a chance to close the enclosing ResourceLeak.
Modifications:
- Add ResourceLeakTracker and deprecate the old ResourceLeak
- Fix some javadocs to correctly release buffers.
- Add a unit test for ResourceLeakDetector that shows that ResourceLeakTracker has not the problems.
Result:
No more false-positives reported by ResourceLeakDetector when ResourceLeakDetector.track(...) is used.
Motivation:
We need to ensure we not call handshake() when the engine is already closed. Beside this our implementation of isOutboundDone() was not correct as it not took the pending data in the outbound buffer into acount (which may be also generated as part of an ssl alert). Beside this we also called SSL_shutdown(...) while we were still in init state which will produce an error and so noise in the log with openssl later versions.
This is also in some extend related to #5931 .
Modifications:
- Ensure we not call handshake() when already closed
- Correctly implement isOutboundDone()
- Not call SSL_shutdown(...) when still in init state
- Added test-cases
Result:
More correct behaviour of our openssl SSLEngine implementation.
Motivation:
When non SSL data is passed into SSLEngine.unwrap(...) we need to throw an SSLException. This was not done at the moment. Even worse we threw an IllegalArgumentException as we tried to allocate a direct buffer with capacity of -1.
Modifications:
- Guard against non SSL data and added an unit test.
- Make code more consistent
Result:
Correct behaving SSLEngine implementation.
Motivation:
Java9 will be released soon so we should ensure we can compile netty with Java9 and run all our tests. This will help to make sure Netty will be usable with Java9.
Modification:
- Add some workarounds to be able to compile with Java9, note that the full profile is not supported with Java9 atm.
- Remove some usage of internal APIs to be able to compile on java9
- Not support Alpn / Npn and so not run the tests when using Java9 for now. We will do a follow up PR to add support.
Result:
Its possible to build netty and run its testsuite with Java9.
Motivation:
It's important that we do not pass in the original ChannelPromise to safeClose(...) as when flush(...) will throw an Exception it will be propagated to the AbstractChannelHandlerContext which will try to fail the promise because of this. This will then fail as it was already completed by safeClose(...).
Modifications:
Create a new ChannelPromise and pass it to safeClose(...).
Result:
No more confusing logs because of failing to fail the promise.
Motivation:
We are now more careful to flush alerts that are generated when errors occur. We should also be more careful in unit tests to release any buffers that may be queued due to potential errors wich result in alerts.
Modifications:
- When SslHandlerTest uses EmbeddedChannel we should always call finishAndReleaseAll
Results:
Fixes https://github.com/netty/netty/issues/6057
Motivation:
When the SslHandler.unwrap(...) (which is called via decode(...)) method did produce an SSLException it was possible that the produced alert was not send to the remote peer. This could lead to staling connections if the remote peer did wait for such an alert and the connection was not closed.
Modifications:
- Ensure we try to flush any pending data when a SSLException is thrown during unwrapping.
- Fix SniHandlerTest to correct test this
- Add explicit new test in SslHandlerTest to verify behaviour with all SslProviders.
Result:
The alert is correctly send to the remote peer in all cases.
Motivation:
We tried to detect the correct alert to use depending on the CertificateException that is thrown by the TrustManager. This not worked all the time as depending on the TrustManager implementation it may also wrap a CertPathValidatorException.
Modification:
- Try to unwrap the CertificateException if needed and detect the right alert via the CertPathValidatorException.
- Add unit to verify
Result:
Send the correct alert depending on the CertificateException when using OpenSslEngine.
Motiviation:
We used ReferenceCountUtil.releaseLater(...) in our tests which simplifies a bit the releasing of ReferenceCounted objects. The problem with this is that while it simplifies stuff it increase memory usage a lot as memory may not be freed up in a timely manner.
Modifications:
- Deprecate releaseLater(...)
- Remove usage of releaseLater(...) in tests.
Result:
Less memory needed to build netty while running the tests.
Motivation:
In preparation for support of Conscrypt, I'm consolidating all of the engine-specific details so that it's easier to add new engine types that affect the behavior of SslHandler.
Modifications:
Added an enum SslEngineType that provides SSL engine-specific details.
Result:
SslHandler is more extensible for other engine types.
Motivation:
The SniHandlerTest.testServerNameParsing did fail when SslProvider.JDK was used as it the JDK SSLEngineImpl does not send an alert.
Modifications:
Ensure tests pass with JDK and OPENSSL ssl implementations.
Result:
SniHandlerTest will run with all SslProvider and not fail when SslProvider.JDK is used.
Motiviation:
We need to ensure we only consume as much da as we can maximal put in one ssl record to not produce a BUFFER_OVERFLOW when calling wrap(...).
Modification:
- Limit the amount of data that we consume based on the maximal plain text size that can be put in one ssl record
- Add testcase to verify the fix
- Tighten up testcases to ensure the amount of produced and consumed data in SslEngineResult matches the buffers. If not the tests will fail now.
Result:
Correct and conform behavior of OpenSslEngine.wrap(...) and better test coverage during handshaking in general.
Motivation:
Netty provides a adaptor from ByteBuf to Java's InputStream interface. The JDK Stream interfaces have an explicit lifetime because they implement the Closable interface. This lifetime may be differnt than the ByteBuf which is wrapped, and controlled by the interface which accepts the JDK Stream. However Netty's ByteBufInputStream currently does not take reference count ownership of the underlying ByteBuf. There may be no way for existing classes which only accept the InputStream interface to communicate when they are done with the stream, other than calling close(). This means that when the stream is closed it may be appropriate to release the underlying ByteBuf, as the ownership of the underlying ByteBuf resource may be transferred to the Java Stream.
Motivation:
- ByteBufInputStream.close() supports taking reference count ownership of the underyling ByteBuf
Result:
ByteBufInputStream can assume reference count ownership so the underlying ByteBuf can be cleaned up when the stream is closed.
Motivation:
OpenSslEngine.wrap(...) and OpenSslEngie.unwrap(...) may consume bytes even if an BUFFER_OVERFLOW / BUFFER_UNDERFLOW is detected. This is not correct as it should only consume bytes if it can process them without storing data between unwrap(...) / wrap(...) calls. Beside this it also should only process one record at a time.
Modifications:
- Correctly detect BUFFER_OVERFLOW / BUFFER_UNDERFLOW and only consume bytes if non of them is detected.
- Only process one record per call.
Result:
OpenSslEngine behaves like stated in the javadocs of SSLEngine.
Motivation:
We should not use the InternalThreadLocalMap where access may be done from outside the EventLoop as this may create a lot of memory usage while not be reused anyway.
Modifications:
Not use InternalThreadLocalMap in places where the code-path will likely be executed from outside the EventLoop.
Result:
Less memory bloat.
Motivation:
Since Java 7, X509TrustManager implementation is wrapped by a JDK class
called AbstractTrustManagerWrapper, which performs an additional
certificate validation for Socket or SSLEngine-backed connections.
This makes the TrustManager implementations provided by
InsecureTrustManagerFactory and FingerprintTrustManagerFactory not
insecure enough, where their certificate validation fails even when it
should pass.
Modifications:
- Add X509TrustManagerWrapper which adapts an X509TrustManager into an
X509ExtendedTrustManager
- Make SimpleTrustManagerFactory wrap an X509TrustManager with
X509TrustManagerWrapper is the provided TrustManager does not extend
X509ExtendedTrustManager
Result:
- InsecureTrustManagerFactory and FingerprintTrustManagerFactory are now
insecure as expected.
- Fixes#5910
Motivation:
Our default cipher list has not been updated in a while. We current support some older ciphers not commonly in use and we don't support some newer ciphers which are more commonly used.
Modifications:
- Update the default list of ciphers for JDK and OpenSSL.
Result:
Default cipher list is more likely to connect to peers.
Fixes https://github.com/netty/netty/issues/5859
Motivation:
If the user removes the SslHandler while still in the processing loop we will produce an IllegalReferenceCountException. We should stop looping when the handlerwas removed.
Modifications:
Ensure we stop looping when the handler is removed.
Result:
No more IllegalReferenceCountException.
Motivation:
Add test-case for doing mutal auth with a certificate chain that holds more then one certificate.
Modifications:
Add test case
Result:
more tests.
Motivation:
the build doesnt seem to enforce this, so they piled up
Modifications:
removed unused import lines
Result:
less unused imports
Signed-off-by: radai-rosenblatt <radai.rosenblatt@gmail.com>
Motivation:
Currently FlushConsolidationHandler only consolidates if a read loop is
active for a Channel, otherwise each writeAndFlush(...) call will still
be flushed individually. When these calls are close enough, it can be
beneficial to consolidate them even outside of a read loop.
Modifications:
When we allow a flush to "go through", don't perform it immediately, but
submit it on the channel's executor. Under high pressure, this gives
other writes a chance to enqueue before the task gets executed, and so
we flush multiple writes at once.
Result:
Lower CPU usage and less context switching.
Motivation
Give the user the ability to back out from SNI negoations.
Modifications
Put a try-catch around the select() call and re-fire any caught Exceptions.
Result
Fixes#5787
Motivation:
IdleStateHandler has a number of volatile member variables which are only accessed from the EventLoop thread. These do not have to be volatile. The accessibility of these member variables are not consistent between private and package private. The state variable can also use a byte instead of an int.
Modifications:
- Remove volatile from member variables
- Change access to private for member variables
- Change state from int to byte
Result:
IdleStateHandler member variables cleaned up.
Motivation:
IdleStateHandler and ReadTimeoutHandler could mistakely not fire an event even if no channelRead(...) call happened.
Modifications:
Only set lastReadTime if a read happened before.
Result:
More correct IdleStateHandler / ReadTimeoutHandler.
Motivation:
There is an incoherence in terms of API when one wants to use
startTls: without startTls one can use the SslContextBuilder's
method newHandler, but with startTls, the developper is forced
to call directly the SslHandler constructor.
Modifications:
Introduce startTls as a SslContextBuilder parameter as well as a
member in SslContext (and thus Jdk and OpenSsl implementations!).
Always use this information to call the SslHandler constructor.
Use false by default, in particular in deprecated constructors of
the SSL implementations.
The client Context use false by default
Results:
Fixes#5170 and more generally homogenise the API so that
everything can be done via SslContextBuilder.
Motivation
I'm looking to harden our SSL impl. a little bit and add some guards agaist certain types of abuse. One can think of invalid hostname strings in the SNI extenstion or invalid SNI handshakes altogether. This will require measuring, velocity tracking and other things.
Modifications
Adding a protected `lookup(ctx, hostname)` method that is called from SniHandler's `select(...)` method which users can override and implement custom behaviour. The default implementation will simply call the AsyncMapper.
Result
It's possible to get a hold onto the ChannelHandlerContext. Users can override that method and do something with it right there or they can delegate it to something else. SniHandler is happy as long as a `Future<SslContext>` is being returned.
Motivation:
af632278d2 introduced a test which only worked on some jvm versions and specific os'es.
Modifications:
Fix test to work on different java versions and os'es
Result:
No flacky test.
Motivation:
We need to ensure we not set duplicated certificates when using OpenSslEngine.
Modifications:
- Skip first cert in chain when set the chain itself and so not send duplicated certificates
- Add interopt unit tests to ensure no duplicates are send.
Result:
No more duplicates.
Motivation:
AbstractTrafficShapingHandler has a package-private method called "userDefinedWritabilityIndex()" which a user may need to override if two sub-classes wants to be used in the ChannelPipeline.
Modifications:
Mark method protected.
Result:
Easier to extend AbstractTrafficShapingHandler.
Motivation:
SslHandler can be cleaned up a bit in terms of naming and duplicated code.
Modifications:
- Fix naming of arguments
- Not schedule timeout event if not really needed
- share some code and simplify
Result:
Cleaner code.
Motivation:
When a SecurityManager is in place it may dissallow accessing the property which will lead to not be able to load the application.
Modifications:
Use AccessController.doPrivileged(...)
Result:
No more problems with SecurityManager.
Motivation
The SniHandler is currently hiding its replaceHandler() method and everything that comes with it. The user has no easy way of getting a hold onto the SslContext for the purpose of reference counting for example. The SniHandler does have getter methods for the SslContext and hostname but they're not very practical or useful. For one the SniHandler will remove itself from the pipeline and we'd have to track a reference of it externally and as we saw in #5745 it'll possibly leave its internal "selection" object with the "EMPTY_SELECTION" value (i.e. we've just lost track of the SslContext).
Modifications
Expose replaceHandler() and allow the user to override it and get a hold onto the hostname, SslContext and SslHandler that will replace the SniHandler.
Result
It's possible to get a hold onto the SslContext, the hostname and the SslHandler that is about to replace the SniHandler. Users can add additional behavior.
Motiviation:
Previously the way how CertificateRequestCallback was working had some issues which could cause memory leaks and segfaults. Due of this tcnative code was updated to change the signature of the method provided by the interface.
Modifications:
Update CertificateRequestCallback implementations to match new interface signature.
Result:
No more segfaults / memory leaks when using boringssl or openssl >= 1.1.0
Motivation
SslHandler's handlerRemoved() is supposed to release the SSLEngine (which it does) but there is no Test for it to make sure it really happens and doesn't unexpectedly change in the future.
Modifications
Add a Unit Test that makes sure that SslHandler releases the SSLEngine when the Channel gets closed.
Result
Assurance that SslHandler will not leak (ReferenceCounted) SSLEngines.
Motivation:
The SniHandler attempts to generate a new SslHandler from the selected SslContext in a and insert that SslHandler into the pipeline. However if the underlying channel has been closed or the pipeline has been modified the pipeline.replace(..) operation may fail. Creating the SslHandler may also create a SSLEngine which is of type ReferenceCounted. The SslHandler states that if it is not inserted into a pipeline that it will not take reference count ownership of the SSLEngine. Under these conditions we will leak the SSLEngine if it is reference counted.
Modifications:
- If the pipeline.replace(..) operation fails we should release the SSLEngine object.
Result:
Fixes https://github.com/netty/netty/issues/5678
Motivation:
When SslHandler.close(...) is called (as part of Channel.close()). it will also try to flush pending messages. This may fail for various reasons, but we still should propergate the close operation
Modifications:
- Ensure flush(...) itself will not throw an Exception if we was able to at least fail one pending promise (which should always be the case).
- If flush(...) fails as part of close ensure we still close the channel and then rethrow.
Result:
No more lost close operations possible if an exception is thrown during close
Motivation:
ReferenceCountedOpenSslEngine depends upon the the SslContext to cleanup JNI resources. If we don't wait until the ReferenceCountedOpenSslEngine is done with cleanup before cleaning up the SslContext we may crash the JVM.
Modifications:
- Wait for the channels to close (and thus the ReferenceCountedOpenSslEngine to be cleaned up) before cleaning up the associated SslContext.
Result:
Cleanup sequencing is correct and no more JVM crash.
Fixes https://github.com/netty/netty/issues/5692
Motivation:
We should fail all promises with the correct SSLENGINE_CLOSED exception one the engine is closed. We did not fail the current promise with this exception if the ByteBuf was not readable.
Modifications:
Correctly fail promises.
Result:
More correct handling of promises if the SSLEngine is closed.
Motivation:
Commit b963595988 added a unit that will not work when KeyManagerFactory is used.
Modifications:
Only run the test if OpenSsl.useKeyManagerFactory() returns false.
Result:
Builds with boringssl
Motivation:
The private key and certificate that are passed into #serKeyMaterial() could be PemEncoded in which case the #toPEM() methods return the identity of the value.
That in turn will fail in the #toBIO() step because the underlying ByteBuf is not necessarily direct.
Modifications:
- Use toBIO(...) which also works with non direct PemEncoded values
- Add unit test.
Result:
Correct handling of PemEncoded.
Motivation:
Its completely fine to start writing before the handshake completes when using SslHandler. The writes will be just queued.
Modifications:
Remove the missleading and incorrect javadoc.
Result:
Correct javadoc.
Motivation:
If netty is used in a tomcat container tomcat itself may ship tcnative. Because of this we will try to use OpenSsl in netty and fail because it is different to netty-tcnative.
Modifications:
Ensure if we find tcnative it is really netty-tcnative before using it.
Result:
No more problems when using netty in a tomcat container that also has tcnative installed.
Motivation:
We need to ensure we only call ReferenceCountUtil.safeRelease(...) in finalize() if the refCnt() > 0 as otherwise we will log a message about IllegalReferenceCountException.
Modification:
Check for a refCnt() > 0 before try to release
Result:
No more IllegalReferenceCountException produced when run finalize() on OpenSsl* objects that where explicit released before.
Motivation:
netty-tcnative API has changed to remove a feature that contributed to a memory leak.
Modifications:
- Update to use the modified netty-tcnative API
Result:
Netty can use the latest netty-tcnative.
Motivation:
In latest refeactoring we failed to cleanup imports and also there are some throws declarations which are not needed.
Modifications:
Cleanup imports and throws declarations
Result:
Cleaner code.
Motivation:
OpenSslEngine and OpenSslContext currently rely on finalizers to ensure that native resources are cleaned up. Finalizers require the GC to do extra work, and this extra work can be avoided if the user instead takes responsibility of releasing the native resources.
Modifications:
- Make a base class for OpenSslENgine and OpenSslContext which does not have a finalizer but instead implements ReferenceCounted. If this engine is inserted into the pipeline it will be released by the SslHandler
- Add a new SslProvider which can be used to enable this new feature
Result:
Users can opt-in to a finalizer free OpenSslEngine and OpenSslContext.
Fixes https://github.com/netty/netty/issues/4958
Motivation:
Sometimes it may be useful to explicit disable the usage of the KeyManagerFactory when using OpenSsl.
Modifications:
Add io.netty.handler.ssl.openssl.useKeyManagerFactory which can be used to explicit disable KeyManagerFactory usage.
Result:
More flexible usage.
Motivation:
We should take the readerIndex into account whe write into the BIO. Its currently not a problem as we slice before and so the readerIndex is always 0 but we should better not depend on this as this will break easily if we ever refactor the code and not slice anymore.
Modifications:
Take readerIndex into acount.
Result:
More safe and correct use.
Motivation:
When we try to close the Channel due a timeout we need to ensure we not log if the notification of the promise fails as it may be completed in the meantime.
Modifications:
Add another constructor to ChannelPromiseNotifier and PromiseNotifier which allows to log on notification failure.
Result:
No more miss-leading logs.
Motivation:
FlushConsolidationHandler#flushIfNeeded has a conditional which is fixed based upon code path. This conditional can be removed and instead just manually set in each fixed code path.
Modifications:
- Remove boolean parameter on FlushConsolidationHandler#flushIfNeeded and set readInprogess to false manually when necessary
Result:
Less conditionals in FlushConsolidationHandler
Motivation:
PR #5493 added support for KeyManagerFactories when using the OpenSsl context. This commit corrects a bug causing a NullPointerException that occurs when using a KeyManagerFactory without a certificate chain and private key.
Modifications:
Removes assertNotNull() assertions which were causing a certificate chain and private key to be required even when using a KeyManagerFactory. Also removed a redundant call to buildKeyManagerFactory() which was also causing a exception when a KeyManagerFactory is provided but a certificate chain and private key is not.
Result:
A KeyManagerFactory can now be used in the OpenSslServerContext without an independent certificate chain and private key.
Motivation:
Calling flush() and writeAndFlush(...) are expensive operations in the sense as both will produce a write(...) or writev(...) system call if there are any pending writes in the ChannelOutboundBuffer. Often we can consolidate multiple flush operations into one if currently a read loop is active for a Channel, as we can just flush when channelReadComplete is triggered. Consolidating flushes can give a huge performance win depending on how often is flush is called. The only "downside" may be a bit higher latency in the case of where only one flush is triggered by the user.
Modifications:
Add a FlushConsolidationHandler which will consolidate flushes and so improve the throughput.
Result:
Better performance (throughput). This is especially true for protocols that use some sort of PIPELINING.
Motivation:
ReadTimeoutHandler and IdleStateHandler have duplicated code, we should share whatever possible.
Modifications:
Let ReadTimeoutHandler extend IdleStateHandler.
Result:
Remove code duplication.
Motivation:
Unit test for the OpenSslEngine "OpenSslEngine writePlaintextData WANT_READ with no data in BIO buffer" issue.
Modifications:
- Update SslEngine test to include renegotiation
Result:
More test coverage in OpenSslEngine.
Motivation:
The gRPC interop tests fail due to a NPE in OpenSslEngine.
Caused by: java.lang.NullPointerException
at io.netty.handler.ssl.OpenSslEngine.setSSLParameters(OpenSslEngine.java:1473)
Modifications:
Add a null check
Result:
No more NPE exceptions :-)
Motivation:
To be able to use SslProvider.OpenSsl with existing java apps that use the JDK SSL API we need to also provide a way to use it with an existing KeyManagerFactory.
Modification:
Make use of new tcnative apis and so hook in KeyManagerFactory.
Result:
SslProvider.OpenSsl can be used with KeyManagerFactory as well.
Motivation:
Java8+ adds support set a DH key size via a System property (jdk.tls.ephemeralDHKeySize). We should respect this when using OpenSSL.
Modifications:
Respect system property.
Result:
More consistent SSL implementation.
Motivation:
We recently added support for session ticket statistics which we can expose now.
Modifications:
Expose the statistics.
Result:
Be able to obtain session ticket statistics.
Motivation:
We need to return a correct time for SSLSession.getLastAccessedTime() so it reflect when the handshake was done when the session was reused.
Modifications:
Correctly reflect handshake time in getLastAccessedTime().
Result:
More conform SSLSession implementation.
Motivation:
Sometimes its needed to customize the SSLEngine (like setting protocols etc). For this it would be useful if the user could wrap an SslContext and do init steps on the SSLEngine.
Modifications:
Add new SslContext implementation which can wrap another one and allow to customize the SSLEngine
Result:
More flexible usage of SslContext.
Motivation:
At the moment OpenSslEngine.getSupportedCipherSuites() only return the original openssl cipher names and not the java names. We need also include the java names.
Modifications:
Correctly return the java names as well.
Result:
Correct implementation of OpenSslEngine.getSupportedCipherSuites()
Motivation:
These methods were recently deprecated. However, they remained in use in several locations in Netty's codebase.
Modifications:
Netty's code will now access the bootstrap config to get the group or child group.
Result:
No impact on functionality.
Motivation:
There is no need already use synchronized when validate the args of the methods.
Modifications:
First validate arguments and then use synchronized
Result:
Less code executed in synchronized block.
Motivation:
We use pre-instantiated exceptions in various places for performance reasons. These exceptions don't include a stacktrace which makes it hard to know where the exception was thrown. This is especially true as we use the same exception type (for example ChannelClosedException) in different places. Setting some StackTraceElements will provide more context as to where these exceptions original and make debugging easier.
Modifications:
Set a generated StackTraceElement on these pre-instantiated exceptions which at least contains the origin class and method name. The filename and linenumber are specified as unkown (as stated in the javadocs of StackTraceElement).
Result:
Easier to find the origin of a pre-instantiated exception.
Motivation:
At the moment SSLSession.getId() may always return an empty byte array when OpenSSLEngine is used. This is as we not set SSL_OP_NO_TICKET on the SSLContext and so SSL_SESSION_get_id(...) will return an session id with length of 0 if tickets are not used.
Modifications:
- Set SSL_OP_NO_TICKET by default and only clear it if the user requests the usage of session tickets.
- Add unit test
Result:
Ensure consistent behavior between different SSLEngine implementations.
Motivation:
When using java8+ we should support SSLParameters.setCipherSuiteOrder()
Modifications:
Add support of SLParameters.setCipherSuiteOrder() by using reflection, so we can compile with java7 but still support it.
Result:
Users that use java8+ can use SSLParameters.setCipherSuiteOrder()
Motivation:
Java8 added support for using SNIHostName with SSLParameters. We currently ignore it in OpenSslEngine.
Modifications:
Use reflection to support SNIHostName.
Result:
People using Java8 can use SNIHostName even when OpenSslEngine is used.
Motivation:
We missed to skip some tests for OpenSsl when OpenSsl.isAvailable() is false.
Modifications:
- Correctly skip tests when OpenSsl.isAvailable() is false.
- Simplify some code by using @BeforeClass.
Result:
Be able to compile netty even when OpenSsl is not present on the system.
Motivation:
When the OpenSslContext is gc'ed and the user still hold a reference to OpenSslSessionContext / OpenSslSessionStats it is possible to produce a segfault when calling
a method on any of these that tries to pass down the ctx pointer to the native methods. This is because the OpenSslContext finalizer will free the native pointer.
Modifications:
Change OpenSslSessionContext / OpenSslSessionContext to store a reference to OpenSslContext and so prevent the GC to collect it as long as the user has a reference to OpenSslSessionContext / OpenSslSessionContext.
Result:
No more sefault possible.
Motivation
This bug was introduced with #5377 and affects only users who'd like to share/cache/re-use `PemPrivateKey` and `PemX509Certificate` instances.
Modifications
Use `ByteBuf#writeBytes(src, readerIndex, length)` so that the src's readerIndex doesn't change and can consequently be used more than once.
Result
It's possible to share/cache/re-use `PemPrivateKey` and `PemX509Certificate` instances as long as their refCnt remains >= 1.
Motivation:
2b65258568 only partially addressed the synchronization issues that are present in FlowControlHandlerTest. A few tests are attempting to validate state changes made across an EventLoop thread and the JUnit thread but are not properly synchronized.
Modifications:
- Ensure that conditions which verify expectations set in another thread have synchronization gates to ensure the event has actually occurred.
- Remove the message counter verification in favor of using individual CountDownLatch objects
Result:
FLowControlHanderTest has less race conditions which may lead to test failures.
Motivation
OpenSslContext is expecting Java's PrivateKey and X509Certificate objects as input
(for JdkSslContext API compatibility reasons) but doesn't really use them beyond
turning them into PEM/PKCS#8 strings.
This conversion can be entirely skipped if the user can pass in private keys and
certificates in a format that Netty's OpenSSL code can digest.
Modifications
Two new classes have been added that act as a wrapper around the pre-encoded byte[]
and also retain API compatibility to JdkSslContext.
Result
It's possible to pass PEM encoded bytes straight into OpenSSL without having to
parse them (e.g. File to Java's PrivateKey) and then encode them (i.e. PrivateKey
into PEM/PKCS#8).
File pemPrivateKeyFile;
byte[] pemBytes = readBytes(pemPrivateKeyFile);
PemPrivateKey pemPrivateKey = PemPrivateKey.valueOf(pemBytes);
SslContextBuilder.forServer(pemPrivateKey)
.sslProvider(SslProvider.OPENSSL)
Motivation:
JCTools supports both non-unsafe, unsafe versions of queues and JDK6 which allows us to shade the library in netty-common allowing it to stay "zero dependency".
Modifications:
- Remove copy paste JCTools code and shade the library (dependencies that are shaded should be removed from the <dependencies> section of the generated POM).
- Remove usage of OneTimeTask and remove it all together.
Result:
Less code to maintain and easier to update JCTools and less GC pressure as the queue implementation nt creates so much garbage
Motivation:
OpenSslClientContext / OpenSslServerContext can never be garbage collected as both are part of a reference to a callback that is stored as global reference in jni code.
Modifications:
Ensure the callbacks are static and so not hold the reference.
Result:
No more leak due not collectable OpenSslClientContext / OpenSslServerContext
Motivation:
OpenSslEngine.wrap will only encrypt at most 1 buffer per call. We may be able to encrypt multiple buffers per call.
Modifications:
- OpensslEngine.wrap should continue encrypting data until there is an error, no more data, or until the destination buffer would be overflowed.
Result:
More encryption is done per OpenSslEngine.wrap call
Motivation:
CVE-2016-4970
OpenSslEngine.wrap calls SSL_write which may return SSL_ERROR_WANT_READ, and if in this condition there is nothing to read from the BIO the OpenSslEngine and SslHandler will enter an infinite loop.
Modifications:
- Use the error code provided by OpenSSL and go back to the EventLoop selector to detect if the socket is closed
Result:
OpenSslEngine correctly handles the return codes from OpenSSL and does not enter an infinite loop.
Motivation:
OpenSslEngine calls rejectRemoteInitiatedRenegation in a scenario where the number of handshakes has not been observed to change. The number of handshakes has only been observed to change after readPlaintextData is called.
Modifications:
- Remove the call to rejectRemoteInitiatedRenegation before calls to readPlaintextData
Result:
Less code.
Motivation:
When netty is used with open ssl provider and client authentication the following errors can occur:
error:140D9115:SSL routines:ssl_get_prev_session:session id context uninitialized
error:140A1175:SSL routines:ssl_bytes_to_cipher_list:inappropriate fallback
error:140760FC:SSL routines:SSL23_GET_CLIENT_HELLO:unknown protocol
Modifications:
Set the session id context in OpenSslServerContext so that sessions which use client authentication
which are cached have the same context id value.
Result:
Client authentication now works with open ssl provider.
Motivation:
DomainMappingBuilder should have been named as DomainNameMappingBuilder
because it builds a DomainNameMapping.
Modifications:
- Add DomainNameMappingBuilder that does the same job with
DomainMappingBuilder
- Deprecate DomainMappingBuilder and delegate its logic to
DomainNameMappingBuilder
- Remove the references to the deprecated methods and classes related
with domain name mapping
- Miscellaneous:
- Fix Javadoc of DomainNameMapping.asMap()
- Pre-create the unmodifiable map in DomainNameMapping
Result:
- Consistent naming
- Less use of deprecated API
Related: #4333#4421#5128
Motivation:
slice(), duplicate() and readSlice() currently create a non-recyclable
derived buffer instance. Under heavy load, an application that creates a
lot of derived buffers can put the garbage collector under pressure.
Modifications:
- Add the following methods which creates a non-recyclable derived buffer
- retainedSlice()
- retainedDuplicate()
- readRetainedSlice()
- Add the new recyclable derived buffer implementations, which has its
own reference count value
- Add ByteBufHolder.retainedDuplicate()
- Add ByteBufHolder.replace(ByteBuf) so that..
- a user can replace the content of the holder in a consistent way
- copy/duplicate/retainedDuplicate() can delegate the holder
construction to replace(ByteBuf)
- Use retainedDuplicate() and retainedSlice() wherever possible
- Miscellaneous:
- Rename DuplicateByteBufTest to DuplicatedByteBufTest (missing 'D')
- Make ReplayingDecoderByteBuf.reject() return an exception instead of
throwing it so that its callers don't need to add dummy return
statement
Result:
Derived buffers are now recycled when created via retainedSlice() and
retainedDuplicate() and derived from a pooled buffer
Motivation:
Sometimes it is useful to include more details in the IdleStateEvents that are produced by the IdleStateHandler. For this users should be able to create their own IdleStateEvents that encapsulate more informations.
Modifications:
- Make IdleStateEvent constructor protected and the class non-final
- Add protected method to IdleStateHandler that users can override and so create their own IdleStateEvents.
Result:
More flexible and customizable IdleStateEvents / IdleStateHandler
