Commit Graph

1092 Commits

Author SHA1 Message Date
Norman Maurer
1b82474286
Fix NPE caused by re-entrance calls in FlowControlHandler (#9320)
Motivation:

2c99fc0f12 introduced a change that eagly recycles the queue. Unfortunally it did not correct protect against re-entrance which can cause a NPE.

Modifications:

- Correctly protect against re-entrance by adding null checks
- Add unit test

Result:

Fixes https://github.com/netty/netty/issues/9319.
2019-07-03 19:55:18 +02:00
Cory Benfield
14154074f2 Don't loop over TLS records for SNI (#7479)
Motivation:

The AbstractSniHandler previously was willing to tolerate up to three
non-handshake records before a ClientHello that contained an SNI
extension field. This is, so far as I can tell, completely
unnecessary: no TLS implementation will be sending alerts or change
cipher spec messages before ClientHello.

Given that it was not possible to determine why this loop is in
the code to begin with, it's probably just best to remove it.

Modifications:

Remove the for loop.

Result:

The AbstractSniHandler will more rapidly determine whether it should
pass the records on to the default SSL handler.

Co-authored-by: Norman Maurer <norman_maurer@apple.com>
2019-07-01 11:22:55 +02:00
jimin
ee8206cb26 optimize some code (#9289)
Motivation:

There is some manual coping of elements of Collections which can be replaced by Collections.addAll(...) and also some unnecessary semicolons.

Modifications:

- Simplify branches
- Use Collections.addAll
- Code cleanup

Result:

Code cleanup
2019-06-28 13:48:23 +02:00
Farid Zakaria
efe40ac17d Add a test for OpenSslEngine which decrypts traffic (#8699)
Motivation:
I've introduced netty/netty-tcnative#421 that introduced exposing OpenSSL master key & client/server
random values with the purpose of allowing someone to log them to debug the traffic via auxiliary tools like Wireshark (see also #8653)

Modification:
Augmented OpenSslEngineTest to include a test which manually decrypts the TLS ciphertext
after exposing the masterkey + client/server random. This acts as proof that the tc-native new methods work correctly!

Result:

More tests

Signed-off-by: Farid Zakaria <farid.m.zakaria@gmail.com>
2019-06-28 13:41:24 +02:00
root
5b58b8e6b5 [maven-release-plugin] prepare for next development iteration 2019-06-28 05:57:21 +00:00
root
35e0843376 [maven-release-plugin] prepare release netty-4.1.37.Final 2019-06-28 05:56:28 +00:00
jimin
856f1185e1 All override methods must be added @override (#9285)
Motivation:

Some methods that either override others or are implemented as part of implementation an interface did miss the `@Override` annotation

Modifications:

Add missing `@Override`s

Result:

Code cleanup
2019-06-27 13:51:26 +02:00
jimin
9621a5b981 remove unused imports (#9287)
Motivation:

Some imports are not used

Modification:

remove unused imports

Result:

Code cleanup
2019-06-26 21:08:31 +02:00
jimin
6bd8f0502d Call to ‘asList’ with only one argument could be replaced with ‘singletonList’ (#9288)
Motivation:

asList should only be used if there are multiple elements.

Modification:

Call to asList with only one argument could be replaced with singletonList

Result:

Cleaner code and a bit of memory savings
2019-06-26 21:06:48 +02:00
Stephane Landelle
039087ed47 Don't filter out TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (#9274)
Motivation:

TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 is supported since Java 8 (see https://docs.oracle.com/javase/8/docs/technotes/guides/security/SunProviders.html) and belongs to the recommended configurations in many references, eg SSLabs (https://github.com/ssllabs/research/wiki/SSL-and-TLS-Deployment-Best-Practices) or Google Cloud Platform Restricted Profile.

Modifications:

Add TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 to default ciphers list.

Result:

TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 is enabled by default.
2019-06-24 23:11:24 +02:00
Norman Maurer
2c99fc0f12
Recycle RecyclableArrayDeque as fast as possible in FlowControlHandler (#9263)
Motivation:

FlowControlHandler does use a recyclable ArrayDeque internally but only recycles it when the channel is closed. We should better recycle it once it is empty.

Modifications:

Recycle the deque as fast as possible

Result:

Less RecyclableArrayDeque instances.
2019-06-22 07:27:04 +02:00
Frédéric Brégier
b1fb40e42d Change Scheduled to FixedRate in Traffic Counter (#9245)
Motivation:

Traffic shaping needs more accurate execution than scheduled one. So the
use of FixedRate instead.
Moreover the current implementation tends to create as many threads as
channels use a ChannelTrafficShapingHandlern, which is unnecessary.

Modifications:

Change the executor.schedule to executor.scheduleAtFixedRate in the
start and remove the reschedule call from run monitor thread since it
will be restarted by the Fixed rate executor.
Also fix a minor bug where restart was only doing start() without stop()
before.

Result:

Threads are more stable in number of cached and precision of traffic
shaping is enhanced.
2019-06-18 09:34:48 +02:00
Scott Mitchell
96feca1d23 SslHandler to fail handshake and pending writes if non-application write fails (#9240)
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.
2019-06-16 07:38:33 +02:00
Nick Hill
8ce3d52c0e OpenSsl.USE_KEYMANAGER_FACTORY incorrectly set to false with BoringSSL (#9175)
Motivation

SSL unit tests started failing for me (RHEL 7.6) after #9162. It looks
like the intention was to prevent disable use of the
io.netty.handler.ssl.openssl.useKeyManagerFactory property when using
BoringSSL, but it now gets set to false in that case rather than the
prior/non-BoringSSL default of true.

Modification

Set useKeyManagerFactory to true rather than false in BoringSSL case
during static init of OpenSSl class.

Result

Tests pass again.
2019-05-23 07:09:55 +02:00
Norman Maurer
224d5fafaf
Correctly detect that KeyManagerFactory is not supported when using OpenSSL 1.1.0+ (#9170)
Motivation:

How we tried to detect if KeyManagerFactory is supported was not good enough for OpenSSL 1.1.0+ as it partly provided the API but not all of what is required.

This then lead to failures like:

[ERROR] Tests run: 1, Failures: 0, Errors: 1, Skipped: 0, Time elapsed: 1.102 s <<< FAILURE! - in io.netty.channel.epoll.EpollDomainSocketStartTlsTest
[ERROR] initializationError(io.netty.channel.epoll.EpollDomainSocketStartTlsTest)  Time elapsed: 0.016 s  <<< ERROR!
javax.net.ssl.SSLException: failed to set certificate and key
	at io.netty.handler.ssl.ReferenceCountedOpenSslServerContext.newSessionContext(ReferenceCountedOpenSslServerContext.java:130)
	at io.netty.handler.ssl.OpenSslServerContext.<init>(OpenSslServerContext.java:353)
	at io.netty.handler.ssl.OpenSslServerContext.<init>(OpenSslServerContext.java:334)
	at io.netty.handler.ssl.SslContext.newServerContextInternal(SslContext.java:468)
	at io.netty.handler.ssl.SslContextBuilder.build(SslContextBuilder.java:457)
	at io.netty.testsuite.transport.socket.SocketStartTlsTest.data(SocketStartTlsTest.java:93)
	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.runners.Parameterized.allParameters(Parameterized.java:280)
	at org.junit.runners.Parameterized.<init>(Parameterized.java:248)
	at sun.reflect.NativeConstructorAccessorImpl.newInstance0(Native Method)
	at sun.reflect.NativeConstructorAccessorImpl.newInstance(NativeConstructorAccessorImpl.java:62)
	at sun.reflect.DelegatingConstructorAccessorImpl.newInstance(DelegatingConstructorAccessorImpl.java:45)
	at java.lang.reflect.Constructor.newInstance(Constructor.java:423)
	at org.junit.internal.builders.AnnotatedBuilder.buildRunner(AnnotatedBuilder.java:104)
	at org.junit.internal.builders.AnnotatedBuilder.runnerForClass(AnnotatedBuilder.java:86)
	at org.junit.runners.model.RunnerBuilder.safeRunnerForClass(RunnerBuilder.java:59)
	at org.junit.internal.builders.AllDefaultPossibilitiesBuilder.runnerForClass(AllDefaultPossibilitiesBuilder.java:26)
	at org.junit.runners.model.RunnerBuilder.safeRunnerForClass(RunnerBuilder.java:59)
	at org.junit.internal.requests.ClassRequest.getRunner(ClassRequest.java:33)
	at org.apache.maven.surefire.junit4.JUnit4Provider.execute(JUnit4Provider.java:362)
	at org.apache.maven.surefire.junit4.JUnit4Provider.executeWithRerun(JUnit4Provider.java:273)
	at org.apache.maven.surefire.junit4.JUnit4Provider.executeTestSet(JUnit4Provider.java:238)
	at org.apache.maven.surefire.junit4.JUnit4Provider.invoke(JUnit4Provider.java:159)
	at org.apache.maven.surefire.booter.ForkedBooter.invokeProviderInSameClassLoader(ForkedBooter.java:384)
	at org.apache.maven.surefire.booter.ForkedBooter.runSuitesInProcess(ForkedBooter.java:345)
	at org.apache.maven.surefire.booter.ForkedBooter.execute(ForkedBooter.java:126)
	at org.apache.maven.surefire.booter.ForkedBooter.main(ForkedBooter.java:418)
Caused by: java.lang.Exception: Requires OpenSSL 1.0.2+
	at io.netty.internal.tcnative.SSLContext.setCertificateCallback(Native Method)
	at io.netty.handler.ssl.ReferenceCountedOpenSslServerContext.newSessionContext(ReferenceCountedOpenSslServerContext.java:126)
	... 32 more

Modifications:

Also try to set the certification callback and only if this works as well mark KeyManagerFactory support as enabled.

Result:

Also correctly work when OpenSSL 1.1.0 is used.
2019-05-22 19:07:19 +02:00
Norman Maurer
af98b62150
Log deprecation info message when using 'io.netty.handler.ssl.openssl.useKeyManagerFactory' and ignore it when using BoringSSL (#9162)
Motivation:

When we added support for KeyManagerFactory we also allowed to disable it to make the change less risky. This was done years ago and so there is really no need to use the property anyway.
Unfortunally due a change in netty-tcnative it is even not supported anymore when using BoringSSL.

Modifications:

- Log an info message to tell users that 'io.netty.handler.ssl.openssl.useKeyManagerFactory' is deprecated when it is used
- Ignore 'io.netty.handler.ssl.openssl.useKeyManagerFactory' when BoringSSL is used.

Result:

Fixes https://github.com/netty/netty/issues/9147.
2019-05-22 08:40:19 +02:00
Norman Maurer
f17bfd0f64
Only use static Exception instances when we can ensure addSuppressed … (#9152)
Motivation:

OOME is occurred by increasing suppressedExceptions because other libraries call Throwable#addSuppressed. As we have no control over what other libraries do we need to ensure this can not lead to OOME.

Modifications:

Only use static instances of the Exceptions if we can either dissable addSuppressed or we run on java6.

Result:

Not possible to OOME because of addSuppressed. Fixes https://github.com/netty/netty/issues/9151.
2019-05-17 22:23:02 +02:00
Norman Maurer
ccf56706f8
Add missing assume checks to skip tests if KeyManagerFactory can not be used (#9148)
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
2019-05-15 07:24:01 +02:00
RoganDawes
3221bf6854 Remove the Handler only after it has initialized the channel (#9132)
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
2019-05-13 13:49:17 +02:00
SplotyCode
5a27f2f78b Allow to specify KeyStore type in SslContext (#9003)
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.
2019-05-10 07:29:14 +02:00
root
ba06eafa1c [maven-release-plugin] prepare for next development iteration 2019-04-30 16:42:29 +00:00
root
49a451101c [maven-release-plugin] prepare release netty-4.1.36.Final 2019-04-30 16:41:28 +00:00
Norman Maurer
0c114dabed
Introduce DynamicAddressConnectHandler which can be used to dynamically change remoteAddress / localAddress when a connect is issued (#8982)
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.
2019-04-30 07:52:12 +02:00
Ilya Maykov
c8ff76ba91 [openssl] fix refcount bug in OpenSslPrivateKeyMaterial ctor
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.
2019-04-29 23:11:18 +02:00
Norman Maurer
3367a53d3b
Throw SignatureException if OpenSslPrivateKeyMethod.* return null to prevent segfault (#9100)
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.
2019-04-29 08:31:14 +02:00
root
baab215f66 [maven-release-plugin] prepare for next development iteration 2019-04-17 07:26:24 +00:00
root
dfe657e2d4 [maven-release-plugin] prepare release netty-4.1.35.Final 2019-04-17 07:25:40 +00:00
Norman Maurer
3ebd29f9c7
Only try to use OpenSslX509TrustManagerWrapper when using Java 7+ (#9065)
Motivation:

We should only try to use OpenSslX509TrustManagerWrapper when using Java 7+ as otherwise it fail to init in it's static block as X509ExtendedTrustManager was only introduced in Java7

Modifications:

Only call OpenSslX509TrustManagerWrapper if we use Java7+

Result:

Fixes https://github.com/netty/netty/issues/9064.
2019-04-17 08:16:55 +02:00
Norman Maurer
48edf40861
Make validation tools more happy by not have TrustManager impl just accept (#9041)
Motivation:

Seems like some analyzer / validation tools scan code to detect if it may produce some security risk because of just blindly accept certificates. Such a tool did tag our code because we have such an implementation (which then is actually never be used). We should just change the impl to not do this as it does not matter for us and it makes such tools happier.

Modifications:

Throw CertificateException

Result:

Fixes https://github.com/netty/netty/issues/9032
2019-04-12 21:36:57 +02:00
秦世成
51112e2b36 Avoid IdleStateHandler triggering unexpected idle events when flushing large entries to slow clients (#9020)
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 .
2019-04-09 16:26:27 +02:00
Norman Maurer
c3c05e8570 Fix NPE in OpenSslPrivateKeyMethodTest.destroy() when BoringSSL is not used
Motivation:

4079189f6b 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.
2019-04-09 08:31:39 +02:00
Norman Maurer
4079189f6b
Allow to offload / customize key signing operations when using BoringSSL. (#8943)
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.
2019-04-08 20:17:44 +02:00
Farid Zakaria
4373a1fba2 Increase default bits for SelfSignedCertificate (#9019)
Motivation:
During OpenSsl.java initialization, a SelfSignedCertificate is created
during the static initialization block to determine if OpenSsl
can be used.

The default key strength for SelfSignedCertificate was too low if FIPS
mode is used and BouncyCastle-FIPS is the only available provider
(necessary for compliance). A simple fix is to just augment the key
strength to the minimum required about by FIPS.

Modification:
Set default key bit length to 2048 but also allow it to be dynamically set via a system property for future proofing to more stricter security compliance.

Result:
Fixes #9018

Signed-off-by: Farid Zakaria <farid.m.zakaria@gmail.com>
2019-04-08 20:08:59 +02:00
Norman Maurer
4b83be1ceb
We should fail fast if the given PrivateKey or X509Certificate chain is not supported by the used SslProvider. (#9009)
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.
2019-04-08 15:20:14 +02:00
Norman Maurer
547a375737
Always include initial handshake exception when throwing SslHandshakeException (#9008)
Motivation:

A callback may already have stored a initial handshake exception in ReferenceCountedOpenSslEngine so we should include it when throwing a SslHandshakeException to ensure the user has all the infos when debugging.

Modifications:

Include initial handshake exception

Result:

Include all erros when throwing the SslHandshakeException.
2019-04-05 09:55:32 +02:00
Norman Maurer
f8c89e2e05
Remove call to SSL.setHostNameValidation(...) as it is done in the TrustManager (#8981)
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.
2019-04-01 21:02:36 +02:00
Norman Maurer
f7359aa742
Use SSL.setKeyMaterial(...) to test if the KeyManagerFactory is supported (#8985)
Motivation:

We use SSL.setKeyMaterial(...) in our implementation when using the KeyManagerFactory so we should also use it to detect if we can support KeyManagerFactory.

Modifications:

Use SSL.setKeyMaterial(...) as replacement for SSL.setCertificateBio(...)

Result:

Use the same method call to detect if KeyManagerFactory can be supported as we use in the real implementation.
2019-04-01 12:03:05 +02:00
Norman Maurer
86ecad517c
Consolidate creation of SslHandshakeException when caused by a callback that is used in the native SSL implementation. (#8979)
Motivation:

We have multiple places where we store the exception that was produced by a callback in ReferenceCountedOpenSslEngine, and so have a lot of code-duplication.

Modifications:

- Consolidate code into a package-private method that is called from the callbacks if needed

Result:

Less code-duplication and cleaner code.
2019-03-26 11:38:37 +01:00
Norman Maurer
41b0236815
Allow to offload certificate validation when using BoringSSL (#8974)
Motivation:

BoringSSL supports offloading certificate validation to a different thread. This is useful as it may need to do blocking operations and so may block the EventLoop.

Modification:

- Adjust ReferenceCountedOpenSslEngine to correctly handle offloaded certificate validation (just as we already have code for certificate selection).

Result:

Be able to offload certificate validation when using BoringSSL.
2019-03-24 20:03:30 +01:00
Norman Maurer
33e2f5609d Revert "Allow to offload certificate validation when using BoringSSL (#8908)"
This reverts commit 316dd98284.
2019-03-24 09:33:42 +01:00
Norman Maurer
316dd98284
Allow to offload certificate validation when using BoringSSL (#8908)
Motivation:

BoringSSL supports offloading certificate validation to a different thread. This is useful as it may need to do blocking operations and so may block the EventLoop.

Modification:

- Adjust ReferenceCountedOpenSslEngine to correctly handle offloaded certificate validation (just as we already have code for certificate selection).

Result:

Be able to offload certificate validation when using BoringSSL.
2019-03-24 09:03:27 +01:00
Norman Maurer
33128c85f8
Add SSLEngineTest to ensure Signature Algorithms are present during KeyManager calls. (#8965)
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.
2019-03-24 07:35:03 +01:00
Norman Maurer
1ca37a0edb
Correctly detect exeception cause when using BoringSSL in SslErrorTest (#8970)
Motivation:

e9ce5048df 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.
2019-03-22 16:30:53 +01:00
Norman Maurer
e9ce5048df
Correctly produce ssl alert when certificate validation fails on the client-side when using native SSL implementation. (#8949)
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.
2019-03-18 18:42:11 +01:00
root
92b19cfedd [maven-release-plugin] prepare for next development iteration 2019-03-08 08:55:45 +00:00
root
ff7a9fa091 [maven-release-plugin] prepare release netty-4.1.34.Final 2019-03-08 08:51:34 +00:00
Norman Maurer
3e24e9f6ff
ReferenceCountedOpenSslEngines SSLSession must provide local certific… (#8918)
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.
2019-03-08 06:47:28 +01:00
Norman Maurer
0de5402337
Add interopt tests between Conscrypt and OpenSSL SSLEngine implementations. (#8919)
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.
2019-03-07 09:36:59 +01:00
Norman Maurer
39fcdb3e0d
Support delegating task when using ReferenceCountedOpenSslEngine. (#8859)
Motivation:

SSLEngine API has a notion of tasks that may be expensive and offload these to another thread. We did not support this when using our native implementation but can now for various operations during the handshake.

Modifications:

- Support offloading tasks during the handshake when using our native SSLEngine implementation
- Correctly handle the case when NEED_TASK is returned and nothing was consumed / produced yet

Result:

Be able to offload long running tasks from the EventLoop when using SslHandler with our native SSLEngine.
2019-03-05 09:17:18 +01:00
Norman Maurer
6f507dfeed
Only remove ReferenceCountedOpenSslEngine from OpenSslEngineMap when engine is destroyed (#8905)
Motivation:

We must only remove ReferenceCountedOpenSslEngine from OpenSslEngineMap when engine is destroyed as the verifier / certificate callback may be called multiple times when the remote peer did initiate a renegotiation.
If we fail to do so we will cause an NPE like this:

```
13:16:36.750 [testsuite-oio-worker-5-18] DEBUG i.n.h.s.ReferenceCountedOpenSslServerContext - Failed to set the server-side key material
java.lang.NullPointerException: null
	at io.netty.handler.ssl.OpenSslKeyMaterialManager.setKeyMaterialServerSide(OpenSslKeyMaterialManager.java:69)
	at io.netty.handler.ssl.ReferenceCountedOpenSslServerContext$OpenSslServerCertificateCallback.handle(ReferenceCountedOpenSslServerContext.java:212)
	at io.netty.internal.tcnative.SSL.readFromSSL(Native Method)
	at io.netty.handler.ssl.ReferenceCountedOpenSslEngine.readPlaintextData(ReferenceCountedOpenSslEngine.java:575)
	at io.netty.handler.ssl.ReferenceCountedOpenSslEngine.unwrap(ReferenceCountedOpenSslEngine.java:1124)
	at io.netty.handler.ssl.ReferenceCountedOpenSslEngine.unwrap(ReferenceCountedOpenSslEngine.java:1236)
	at io.netty.handler.ssl.ReferenceCountedOpenSslEngine.unwrap(ReferenceCountedOpenSslEngine.java:1279)
	at io.netty.handler.ssl.SslHandler$SslEngineType$1.unwrap(SslHandler.java:217)
	at io.netty.handler.ssl.SslHandler.unwrap(SslHandler.java:1330)
	at io.netty.handler.ssl.SslHandler.decodeNonJdkCompatible(SslHandler.java:1237)
	at io.netty.handler.ssl.SslHandler.decode(SslHandler.java:1274)
	at io.netty.handler.codec.ByteToMessageDecoder.decodeRemovalReentryProtection(ByteToMessageDecoder.java:502)
	at io.netty.handler.codec.ByteToMessageDecoder.callDecode(ByteToMessageDecoder.java:441)
	at io.netty.handler.codec.ByteToMessageDecoder.channelRead(ByteToMessageDecoder.java:278)
	at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:359)
	at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:345)
	at io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:337)
	at io.netty.channel.DefaultChannelPipeline$HeadContext.channelRead(DefaultChannelPipeline.java:1408)
	at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:359)
	at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:345)
	at io.netty.channel.DefaultChannelPipeline.fireChannelRead(DefaultChannelPipeline.java:930)
	at io.netty.channel.oio.AbstractOioByteChannel.doRead(AbstractOioByteChannel.java:170)
	at io.netty.channel.oio.AbstractOioChannel$1.run(AbstractOioChannel.java:40)
	at io.netty.channel.ThreadPerChannelEventLoop.run(ThreadPerChannelEventLoop.java:69)
	at io.netty.util.concurrent.SingleThreadEventExecutor$5.run(SingleThreadEventExecutor.java:905)
	at io.netty.util.concurrent.FastThreadLocalRunnable.run(FastThreadLocalRunnable.java:30)
	at java.base/java.lang.Thread.run(Thread.java:834)
```

While the exception is kind of harmless (as we will reject the renegotiation at the end anyway) it produces some noise in the logs.

Modifications:

Don't remove engine from map after handshake is complete but wait for it to be removed until the engine is destroyed.

Result:

No more NPE and less noise in the logs.
2019-03-01 19:31:06 +01:00