Motivation:
SslHandler must generate control data as part of the TLS protocol, for example
to do handshakes. SslHandler doesn't capture the status of the future
corresponding to the writes when writing this control (aka non-application
data). If there is another handler before the SslHandler that wants to fail
these writes the SslHandler will not detect the failure and we must wait until
the handshake timeout to detect a failure.
Modifications:
- SslHandler should detect if non application writes fail, tear down the
channel, and clean up any pending state.
Result:
SslHandler detects non application write failures and cleans up immediately.
Motivation:
Depending on what OpenSSL library version we use / system property that is set we need to skip tests that use KeyManagerFactory.
Modifications:
Add missing assume checks for tests that use KeyManagerFactory.
Result:
All tests pass even if KeyManagerFactory is not supported
Motivation:
Previously, any 'relative' pipeline operations, such as
ctx.pipeline().replace(), .addBefore(), addAfter(), etc
would fail as the handler was not present in the pipeline.
Modification:
Used the pattern from ChannelInitializer when invoking configurePipeline().
Result:
Fixes#9131
Motivation:
As brought up in https://github.com/netty/netty/issues/8998, JKS can be substantially faster than pkcs12, JDK's new default. Without an option to set the KeyStore type you must change the configuration of the entire JVM which is impractical.
Modification:
- Allow to specify KeyStore type
- Add test case
Result:
Fixes https://github.com/netty/netty/issues/8998.
Motivation:
Bootstrap allows you to set a localAddress for outbound TCP connections, either via the Bootstrap.localAddress(localAddress) or Bootstrap.connect(remoteAddress, localAddress) methods. This works well if you want to bind to just one IP address on an interface. Sometimes you want to bind to a specific address based on the resolved remote address which should be possible.
Modifications:
Add DynamicAddressConnectHandler and tests
Result:
Fixes https://github.com/netty/netty/issues/8940.
Motivation:
Subclasses of `OpenSslKeyMaterial` implement `ReferenceCounted`. This means that a new object should have an initial refcount of 1. An `OpenSslPrivateKey.OpenSslPrivateKeyMaterial` object shares its refcount with the enclosing `OpenSslPrivateKey` object. This means the enclosing object's refcount must be incremented by 1 when an instance of `OpenSslPrivateKey.OpenSslPrivateKeyMaterial` is created. Otherwise, when the key material object is `release()`-ed, the refcount on the enclosing object will drop to 0 while it is still in use.
Modification:
- Increment the refcount in the constructor of `OpenSslPrivateKey.OpenSslPrivateKeyMaterial`
- Ensure we also always release the native certificates as well.
Result:
Refcount is now correct.
Motivation:
While OpenSslPrivateKeyMethod.* should never return null we should still guard against it to prevent any possible segfault.
Modifications:
- Throw SignatureException if null is returned
- Add unit test
Result:
No segfault when user returns null.
Motivation:
IdleStateHandler may trigger unexpected idle events when flushing large entries to slow clients.
Modification:
In netty design, we check the identity hash code and total pending write bytes of the current flush entry to determine whether there is a change in output. But if a large entry has been flushing slowly (for some reason, the network speed is slow, or the client processing speed is too slow to cause the TCP sliding window to be zero), the total pending write bytes size and identity hash code would remain unchanged.
Avoid this issue by adding checks for the current entry flush progress.
Result:
Fixes#8912 .
Motivation:
4079189f6bd3e2c26ec443f24a204ffe144f1ada introduced OpenSslPrivateKeyMethodTest which will only be run when BoringSSL is used. As the assumeTrue(...) also guards the init of the static fields we need to ensure we only try to destroy these if BoringSSL is used as otherwise it will produce a NPE.
Modifications:
Check if BoringSSL is used before trying to destroy the resources.
Result:
No more NPE when BoringSSL is not used.
Motivation:
BoringSSL allows to customize the way how key signing is done an even offload it from the IO thread. We should provide a way to plugin an own implementation when BoringSSL is used.
Modifications:
- Introduce OpenSslPrivateKeyMethod that can be used by the user to implement custom signing by using ReferenceCountedOpenSslContext.setPrivateKeyMethod(...)
- Introduce static methods to OpenSslKeyManagerFactory which allows to create a KeyManagerFactory which supports to do keyless operations by let the use handle everything in OpenSslPrivateKeyMethod.
- Add testcase which verifies that everything works as expected
Result:
A user is able to customize the way how keys are signed.
Motivation:
Some SslProvider do support different types of keys and chains. We should fail fast if we can not support the type.
Related to https://github.com/netty/netty-tcnative/issues/455.
Modifications:
- Try to parse key / chain first and if if this fails throw and SslException
- Add tests.
Result:
Fail fast.
Motivation:
We do not need to call SSL.setHostNameValidation(...) as it should be done as part of the TrustManager implementation. This is consistent with the JDK implementation of SSLEngine.
Modifications:
Remove call to SSL.setHostNameValidation(...)
Result:
More consistent behaviour between our SSLEngine implementation and the one that comes with the JDK.
Motivation:
We had a bug which could case ExtendedSSLSession.getPeerSupportedSignatureAlgorithms() return an empty array when using BoringSSL. This testcase verifies we correctly return algorithms after the fix in https://github.com/netty/netty-tcnative/pull/449.
Modifications:
Add testcase to verify behaviour.
Result:
Ensure we correctly retuen the algorithms.
Motivation:
e9ce5048dfdfed32df28e46fae6e766d8fd6d734 added a testcase to ensure we correctly send the alert in all cases but did use a too strict message matching which did not work for BoringSSL as it not uses whitespaces but underscores.
Modifications:
Make the message matching less strict.
Result:
Test pass also when using BoringSSL.
Motivation:
When the verification of the server cert fails because of the used TrustManager on the client-side we need to ensure we produce the correct alert and send it to the remote peer before closing the connection.
Modifications:
- Use the correct verification mode on the client-side by default.
- Update tests
Result:
Fixes https://github.com/netty/netty/issues/8942.
Motivation:
The SSLSession that is returned by SSLEngine.getHandshakeSession() must be able to provide the local certificates when the TrustManager is invoked on the server-side.
Modifications:
- Correctly return the local certificates
- Add unit test
Result:
Be able to obtain local certificates from handshake SSLSession during verification on the server side.
Motivation:
In the past we found a lot of SSL related bugs because of the interopt tests we have in place between different SSLEngine implementations. We should have as many of these interopt tests as possible for this reason.
Modifications:
- Add interopt tests between Conscrypt and OpenSSL SSLEngine implementations
Result:
More tests for SSL.
Motivation:
ChunkedWriteHandler needs to close both successful and failed
ChunkInputs. It used to never close successful ones.
Modifications:
* ChunkedWriteHandler always closes ChunkInput before completing
the write promise.
* Ensure only ChunkInput#close() is invoked
on a failed input.
* Ensure no methods are invoked on a closed input.
Result:
Fixes https://github.com/netty/netty/issues/8875.
Motivation:
fa6a8cb09c9679468a6c2d912ddfbbe885ee0c08 introduced correct dispatching of delegated tasks for SSLEngine but did not correctly handle some cases for resuming wrap / unwrap after the task was executed. This could lead to stales, which showed up during tests when running with Java11 and BoringSSL.
Modifications:
- Correctly resume wrap / unwrap in all cases.
- Fix timeout value which was changed in previous commit by mistake.
Result:
No more stales after task execution.
Motivation:
We use outdated EA releases when building and testing with JDK 12 and 13.
Modifications:
- Update versions.
- Add workaround for possible JDK12+ bug.
Result:
Use latest releases
Motivation:
The SSLEngine does provide a way to signal to the caller that it may need to execute a blocking / long-running task which then can be offloaded to an Executor to ensure the I/O thread is not blocked. Currently how we handle this in SslHandler is not really optimal as while we offload to the Executor we still block the I/O Thread.
Modifications:
- Correctly support offloading the task to the Executor while suspending processing of SSL in the I/O Thread
- Add new methods to SslContext to specify the Executor when creating a SslHandler
- Remove @deprecated annotations from SslHandler constructor that takes an Executor
- Adjust tests to also run with the Executor to ensure all works as expected.
Result:
Be able to offload long running tasks to an Executor when using SslHandler. Partly fixes https://github.com/netty/netty/issues/7862 and https://github.com/netty/netty/issues/7020.
Motivation:
We need to update to a new checkstyle plugin to allow the usage of lambdas.
Modifications:
- Update to new plugin version.
- Fix checkstyle problems.
Result:
Be able to use checkstyle plugin which supports new Java syntax.
Motivation:
ChunkedWriteHandler should report write operation as failed
in case *any* chunked was not written. Right now this is not
true for the last chunk.
Modifications:
* Check if the appropriate write operation was succesfull when
reporting the last chunk
* Skip writing chunks if the write operation was already marked
as "done"
* Test cases to cover write failures when dealing with chunked input
Result:
Fix https://github.com/netty/netty/issues/8700
Motivation:
During some other work I noticed we do not have any tests to ensure we correctly use SSLSessionBindingEvent. We should add some testing.
Modifications:
- Added unit test to verify we correctly implement it.
- Ignore the test when using Conscrypt as it not correctly implements it.
Result:
More tests for custom SSL impl.
Motivation:
We missed to skip a few tests that depend on the KeyManagerFactory if the used OpenSSL version / flavor not support it.
Modifications:
Add missing overrides.
Result:
Testsuite also passes for example when using LibreSSL.
Motivation:
In windows if the project is in a path that contains whitespace,
resources cannot be accessed and tests fail.
Modifications:
Adds ResourcesUtil.java in netty-common. Tests use ResourcesUtil.java to access a resource.
Result:
Being able to build netty in a path containing whitespace
Motivation:
If two requests from the same IP are reached at the same time, `connected.contains(remoteIp)` may return false in both threads.
Modifications:
Check if there is already a connection with the same IP using return values.
Result:
Become thread safe.
Motivation:
0d2e38d5d6db6ed1abdf901230fa524c0f8db14c added supported for detection of peer supported algorithms but we missed to fix the testcase.
Modifications:
Fix test-case.
Result:
No more failing tests with BoringSSL.
Motivation:
The SSLSession.getLocalCertificates() / getLocalPrincipial() methods did not correctly return the local configured certificate / principal if a KeyManagerFactory was used when configure the SslContext.
Modifications:
- Correctly update the local certificates / principial when the key material is selected.
- Add test case that verifies the SSLSession after the handshake to ensure we correctly return all values.
Result:
SSLSession returns correct values also when KeyManagerFactory is used with the OpenSSL provider.
* Correctly convert supported signature algorithms when using BoringSSL
Motivation:
BoringSSL uses different naming schemes for the signature algorithms so we need to adjust the regex to also handle these.
Modifications:
- Adjust SignatureAlgorithmConverter to handle BoringSSL naming scheme
- Ensure we do not include duplicates
- Add unit tests.
Result:
Correctly convert boringssl signature algorithm names.
Motivation:
We did not correctly convert between openssl / boringssl and java ciphers when using TLV1.3 which had different effects when either using openssl or boringssl.
- When using openssl and TLSv1.3 we always returned SSL_NULL_WITH_NULL_NULL as cipher name
- When using boringssl with TLSv1.3 we always returned an incorrect constructed cipher name which does not match what is defined by Java.
Modifications:
- Add correct mappings in CipherSuiteConverter for both openssl and boringssl
- Add unit tests for CipherSuiteConvert
- Add unit in SSLEngine which checks that we do not return SSL_NULL_WITH_NULL_NULL ever and that server and client returns the same cipher name.
Result:
Fixes https://github.com/netty/netty/issues/8477.
Motivation:
We did not correctly schedule the handshake timeout if the handshake was either started by a flush(...) or if starttls was used.
Modifications:
- Correctly setup timeout in all cases
- Add unit tests.
Result:
Fixes https://github.com/netty/netty/issues/8493.
Motivation:
If you attempt to write to a channel with an SslHandler prior to channelActive being called you can hit an assertion. In particular - if you write to a channel it forces some handshaking (through flush calls) to occur.
The AssertionError only happens on Java11+.
Modifications:
- Replace assert by an "early return" in case of the handshake be done already.
- Add unit test that verifies we do not hit the AssertionError anymore and that the future is correctly failed.
Result:
Fixes https://github.com/netty/netty/issues/8479.
Motivation:
We should call wrapEngine(...) in our SSLEngineTest to correctly detect all errors in case of the OpenSSLEngine.
Modifications:
Add missing wrapEngine(...) calls.
Result:
More correct tests
Motivation:
Due a bug in our implementation we tried to release the same ByteBuf two times when we failed to parse the X509Certificate as closing the ByteBufInputStream already closed it.
Modifications:
- Don't close the ByteBuf when closing the ByteBufInputStream
- Explicit release all ByteBufs after we are done parsing in a finally block.
- Add testcase.
Result:
Do not produce an IllegalReferenceCountException and throw the correct CertificateException.
Motivation:
https://github.com/netty/netty/issues/8442 reported that we fail to build a SslContext when an invalid cipher is used with netty-tcnative-boringssl-static, while it worked before. This test verifies that this is now consistent with all other SSLEngine implementations.
Modifications:
Add test-case to verify consistent behaviour
Result:
More tests to assert consistent behaviour across SSLEngine implementations
Motivation:
0ddc62cec0b4715ae37cef0e6a9f8c79d42d74e9 added support for TLSv1.3 when using openssl 1.1.1. Now that BoringSSL chromium-stable branch supports it as well we can also support it with netty-tcnative-boringssl-static.
During this some unit tests failed with BoringSSL which was caused by not correctly handling flush() while the handshake is still in progress.
Modification:
- Upgrade netty-tcnative version which also supports TLSv1.3 when using BoringSSL
- Correctly handle flush() when done while the handshake is still in progress in all cases.
Result:
Easier for people to enable TLSv1.3 when using native SSL impl.
Ensure flush() while handshake is in progress will always be honored.
Motivation:
We had put some workaround in our tests due a bug in the Java11 implementation of TLSv1.3. This was now fixes as part of 11.0.1.
See https://bugs.openjdk.java.net/browse/JDK-8211067.
Modifications:
Remove workaround in SSL tests.
Result:
Run all tests with supported TLS version.
Motivation:
At the moment it's only possible to use TLSv1.3 with netty-tcnative if Java 11 is used. It should be possible to do so even with Java 8, 9 and 10.
Modification:
Add a workaround to be able to use TLSv1.3 also when using Java version prior to Java 11 and the default X509ExtendedTrustManager is used.
Result:
Be able to use TLSv1.3 also with past versions of Java.
Motivation:
TLSv1.3 support is included in java11 and is also supported by OpenSSL 1.1.1, so we should support when possible.
Modifications:
- Add support for TLSv1.3 using either the JDK implementation or the native implementation provided by netty-tcnative when compiled against openssl 1.1.1
- Adjust unit tests for semantics provided by TLSv1.3
- Correctly handle custom Provider implementations that not support TLSv1.3
Result:
Be able to use TLSv1.3 with netty.
Motivation:
It is possible that a client is unable to locate a certificate alias given the list of issuers and key types. In this case the X509KeyManager
will return a null which when past to the OpenSslKeyMaterialProvider implementation may produce a NPE. If no matching alias could be found we should not
call OpenSslKeyMaterialProvider at all which is also consistent what OpenJDK does.
Modifications:
- Add null check before calling OpenSslKeyMaterialProvider
- Add unit test.
Result:
No more NPE caused by passing null as client alias.
Motivation:
Before when on server-side we just called the X509KeyManager methods when handshake() was called the first time which is not quite correct as we may not have received the full SSL hello / handshake and so could not extra for example the SNI hostname that was requested.
OpenSSL exposes the SSL_CTX_set_cert_cb function which allows to set a callback which is executed at the correct moment, so we should use it. This also allows us to support more methods of ExtendedSSLSession easily.
Modifications:
- Make use of new methods exposed by netty-tcnative since https://github.com/netty/netty-tcnative/pull/388 to ensure we select the key material at the correct time.
- Implement more methods of ExtendedOpenSslSession
- Add unit tests to ensure we are able to retrieve various things on server-side in the X509KeyManager and so verify it is called at the correct time.
- Simplify code by using new netty-tcnative methods.
Result:
More correct implementation for server-side usage and more complete implemented of ExtendedSSLSession.
Motivation:
a208f6dc7c775ce8d7934f252cd6fa7527643f76 added a testcase which uses hostname validation which may not be supported by OpenSSL depending on the version that is used. We should check first before we try to use it.
Modifications:
Add assumeTrue(...) check to ensure hostname validation is supported before trying to run the test.
Result:
No more test-failures on OpenSSL versions < 1.0.2.
Motivation:
When a X509TrustManager is used while configure the SslContext the JDK automatically does some extra checks during validation of provided certs by the remote peer. We should do the same when our native implementation is used.
Modification:
- Automatically wrap a X509TrustManager and so do the same validations as the JDK does.
- Add unit tests.
Result:
More consistent behaviour. Fixes https://github.com/netty/netty/issues/6664.
Motivation:
4d1458604ab3d378192c6240dd12ea85d1223838 did fix some leaks in SniClientTest but missed the ones in SniClientJava8TestUtil.
Modifications:
Correctly release SslContext.
Result:
No more leaks in SNI tests.
Motivation:
I noticed that we had some errors showing up in a test (which did not fail it tho) because we tried to full-fill the promise multiples times.
Modifications:
Use trySuccess(...) as we may produce multiple exceptions.
Result:
Less errors during test-run.
Motivation:
6ed7c6c75d458047adc37470697f215e9d7436ea added a test which blindly assumed we can use a KeyManagerFactory all the time. This is only true if have OpenSSL 1.0.2 or later, which may not be the case.
Modifications:
Only use KeyManagerFactory in test if the OpenSSL version does support it.
Result:
More robust tests.
Motivation:
When an ExtendedSSLSession is used its possible to do more strict checking of the keys during handshake. We should do this whenever possible.
Modification:
- Return an ExtendedSSLSession when using client-mode and Java7+
- Add unit test
- Simplify unit tests
Result:
More consistent behaviour.
* PemPrivateKey.toPem(...) should throw IllegalArgumentException when PrivateKey which does not support encoding is used.
Motivation:
At the moment when a PrivateKey is used that does not support encoding we throw a NPE when trying to convert the key. We should better throw an IllegalArgumentException with the details about what key we tried to encode.
Modifications:
- Check if PrivateKey.getEncoded() returns null and if so throw an IllegalArgumentException
- Add unit test.
Result:
Better handling of non-supported PrivateKey implementations.
