Commit Graph

1137 Commits

Author SHA1 Message Date
Norman Maurer
ed61e5f543 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:42:53 +02:00
Norman Maurer
260a8a0e9e 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:25:03 +02:00
RoganDawes
0a1786c32c 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:55:17 +02:00
SplotyCode
d3a13a0d6a 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:51:20 +02:00
Norman Maurer
c06ca367ca 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:59:40 +02:00
Ilya Maykov
1cd9f5e17b [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:53 +02:00
Norman Maurer
795fa8aef1 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:56 +02:00
Norman Maurer
ee207f4bc6 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:37:31 +02:00
Norman Maurer
7c35781f4d
DefaultPromise may throw checked exceptions that are not advertised (#8995)
Motivation:

We should not throw check exceptions when the user calls sync*() but should better wrap it in a CompletionException to make it easier for people to reason about what happens.

Modifications:

- Change sync*() to throw CompletionException
- Adjust tests
- Add some more tests

Result:

Fixes https://github.com/netty/netty/issues/8521.
2019-04-10 07:15:31 +02:00
秦世成
fdb4b0e7af 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:27:09 +02:00
Norman Maurer
4eeaa4f956 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:33:42 +02:00
Norman Maurer
a9cca146d7 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:25:37 +02:00
Farid Zakaria
2935944426 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:09:32 +02:00
Norman Maurer
6c4485f53c 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:24:41 +02:00
Norman Maurer
c4c3acf6fb 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:56 +02:00
Norman Maurer
bace8a1cce
Remove code that accounts for changing EventExecutors in DefaultPromise (#8996)
Motivation:

DefaultPromise requires an EventExecutor which provides the thread to notify listeners on and this EventExecutor can never change. We can remove the code that supported the possibility of a changing the executor as this is not possible anymore.

Modifications:

- Remove constructor which allowed to construct a *Promise without an EventExecutor
- Remove extra state
- Adjusted SslHandler and ProxyHandler for new code

Result:

Fixes https://github.com/netty/netty/issues/8517.
2019-04-03 10:36:55 +02:00
Norman Maurer
6b8a0ed374 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:03:20 +02:00
Norman Maurer
07244a194f 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:31 +02:00
Norman Maurer
0f34345347
Merge ChannelInboundHandler and ChannelOutboundHandler into ChannelHa… (#8957)
Motivation:

In 42742e233f we already added default methods to Channel*Handler and deprecated the Adapter classes to simplify the class hierarchy. With this change we go even further and merge everything into just ChannelHandler. This simplifies things even more in terms of class-hierarchy.

Modifications:

- Merge ChannelInboundHandler | ChannelOutboundHandler into ChannelHandler
- Adjust code to just use ChannelHandler
- Deprecate old interfaces.

Result:

Cleaner and simpler code in terms of class-hierarchy.
2019-03-28 09:28:27 +00:00
Norman Maurer
231eb145f1 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:39:09 +01:00
Norman Maurer
1736b0e6a8 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:04:18 +01:00
Norman Maurer
a817e30d41 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:40:49 +01:00
Norman Maurer
d3535d31be Correctly detect exeception cause when using BoringSSL in SslErrorTest
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:31:35 +01:00
Norman Maurer
21cb040aef 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:51:35 +01:00
Norman Maurer
42742e233f
Deprecate ChannelInboundHandlerAdapter and ChannelOutboundHandlerAdapter (#8929)
Motivation:

As we now us java8 as minimum java version we can deprecate ChannelInboundHandlerAdapter / ChannelOutboundHandlerAdapter and just move the default implementations into the interfaces. This makes things a bit more flexible for the end-user and also simplifies the class-hierarchy.

Modifications:

- Mark ChannelInboundHandlerAdapter and ChannelOutboundHandlerAdapter as deprecated
- Add default implementations to ChannelInboundHandler / ChannelOutboundHandler
- Refactor our code to not use ChannelInboundHandlerAdapter / ChannelOutboundHandlerAdapter anymore

Result:

Cleanup class-hierarchy and make things a bit more flexible.
2019-03-13 09:46:10 +01:00
Norman Maurer
7e76d02fe7 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:54:45 +01:00
Norman Maurer
22128a85fe 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:37:21 +01:00
Norman Maurer
b2dc54c8c6 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:38:03 +01:00
Norman Maurer
71d8d057e6 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:30 +01:00
Konstantin Lutovich
94ffd28973 Close consumed inputs in ChunkedWriteHandler (#8876)
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.
2019-02-28 21:18:43 +01:00
Norman Maurer
89139aa3f8 Correctly resume wrap / unwrap when SslTask execution completes (#8899)
Motivation:

fa6a8cb09c 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.
2019-02-28 20:30:04 +01:00
Norman Maurer
0d37b06bc8 Update JDK12 and 13 to latest EA releases. (#8809)
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
2019-02-28 13:55:01 +01:00
Norman Maurer
b9d277dbcb Support using an Executor to offload blocking / long-running tasks wh… (#8847)
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.
2019-02-11 10:00:55 +01:00
Stephane Landelle
ee4e46e6e2 Drop SPDY support (#8845)
Motivation:

SPDY has been superseded by HTTP/2. Chrome has dropped support in 2016 and GFE no longer negociate it.

Modifications:

* drop codec
* drop examples
* drop constants from `ApplicationProtocolNames`

Result:

SPDY support dropped from Netty 5
2019-02-07 09:25:31 +01:00
田欧
e8efcd82a8 migrate java8: use requireNonNull (#8840)
Motivation:

We can just use Objects.requireNonNull(...) as a replacement for ObjectUtil.checkNotNull(....)

Modifications:

- Use Objects.requireNonNull(...)

Result:

Less code to maintain.
2019-02-04 10:32:25 +01:00
田欧
6222101924 migrate java8: use lambda and method reference (#8781)
Motivation:

We can use lambdas now as we use Java8.

Modification:

use lambda function for all package, #8751 only migrate transport package.

Result:

Code cleanup.
2019-01-29 14:06:05 +01:00
田欧
e941cbe27a remove unused import statement (#8792)
Motivation:
The code contained some unused import statements.

Modification:
Remove unused import statements.

Result:
Code cleanup
2019-01-28 16:50:15 +01:00
Stephane Landelle
e1c94363eb Remove deprecated SslContext constructors (#8785)
Motivation:

SslContext implementations have tons of contructors, most of them deprecated as we want to enforce builder usage in Netty 5.

Cleaning them up is a requirement prior to introducing new parameters such as hostname verification.

Modifications:

* Make SslContext implementations classes and constructors package private, users are supposed to use the SslContextBuilder.
* Drop all but one constructor. The exception for now is with Jdk(Client|Server)Context that still has an additional constructor that takes an ApplicationProtocolNegotiator parameter. ApplicationProtocolNegotiator usage is supposed to be dropped in favor of ApplicationProtocolConfig and this constructor is only used in tests, so I guess it will be dropped to in a follow up.

Result:

Deprecated code dropped. Path cleaned up for introducing new features with having to introduce yet another constructor.
2019-01-28 06:01:53 +01:00
田欧
934a07fbe2 migrate java8 (#8779)
Motivation:

We can omit argument types when using Java8.

Modification:

Omit arguments where possible.

Result:

Cleaner code.
2019-01-28 05:55:30 +01:00
kezhenxu94
7b6336f1fd Java 8 Migration: remove uneccessary if statement (#8755)
Motivation:

As netty 4.x supported Java 6 we had various if statements to check for java versions < 8. We can remove these now.

Modification:

Remove unnecessary if statements that check for java versions < 8.

Result:

Cleanup code.
2019-01-25 08:57:11 +01:00
Norman Maurer
310f31b392
Update to new checkstyle plugin (#8777)
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.
2019-01-24 16:24:19 +01:00
Dmitriy Dumanskiy
4a10357fd8 IDE warnings cleanup (#8768)
Motivation:

IDE shows some warnings

Modification:

Small IDE warnings cleanup in different places.

Result:

Less warnings
2019-01-23 14:01:48 +01:00
Norman Maurer
3d6e6136a9
Decouple EventLoop details from the IO handling for each transport to… (#8680)
* Decouble EventLoop details from the IO handling for each transport to allow easy re-use of code and customization

Motiviation:

As today extending EventLoop implementations to add custom logic / metrics / instrumentations is only possible in a very limited way if at all. This is due the fact that most implementations are final or even package-private. That said even if these would be public there are the ability to do something useful with these is very limited as the IO processing and task processing are very tightly coupled. All of the mentioned things are a big pain point in netty 4.x and need improvement.

Modifications:

This changeset decoubled the IO processing logic from the task processing logic for the main transport (NIO, Epoll, KQueue) by introducing the concept of an IoHandler. The IoHandler itself is responsible to wait for IO readiness and process these IO events. The execution of the IoHandler itself is done by the SingleThreadEventLoop as part of its EventLoop processing. This allows to use the same EventLoopGroup (MultiThreadEventLoupGroup) for all the mentioned transports by just specify a different IoHandlerFactory during construction.

Beside this core API change this changeset also allows to easily extend SingleThreadEventExecutor / SingleThreadEventLoop to add custom logic to it which then can be reused by all the transports. The ideas are very similar to what is provided by ScheduledThreadPoolExecutor (that is part of the JDK). This allows for example things like:

  * Adding instrumentation / metrics:
    * how many Channels are registered on an SingleThreadEventLoop
    * how many Channels were handled during the IO processing in an EventLoop run
    * how many task were handled during the last EventLoop / EventExecutor run
    * how many outstanding tasks we have
    ...
    ...
  * Implementing custom strategies for choosing the next EventExecutor / EventLoop to use based on these metrics.
  * Use different Promise / Future / ScheduledFuture implementations
  * decorate Runnable / Callables when submitted to the EventExecutor / EventLoop

As a lot of functionalities are folded into the MultiThreadEventLoopGroup and SingleThreadEventLoopGroup this changeset also removes:

  * AbstractEventLoop
  * AbstractEventLoopGroup
  * EventExecutorChooser
  * EventExecutorChooserFactory
  * DefaultEventLoopGroup
  * DefaultEventExecutor
  * DefaultEventExecutorGroup

Result:

Fixes https://github.com/netty/netty/issues/8514 .
2019-01-23 08:32:05 +01:00
Dmitriy Dumanskiy
7b92ff2500 Java 8 migration. Remove ThreadLocalProvider and inline java.util.concurrent.ThreadLocalRandom.current() where necessary. (#8762)
Motivation:

Custom Netty ThreadLocalRandom and ThreadLocalRandomProvider classes are no longer needed and can be removed.

Modification:

Remove own ThreadLocalRandom

Result:

Less code to maintain
2019-01-22 20:14:28 +01:00
Dmitriy Dumanskiy
32d96a7f79 Java 8 migration. Similar catch blocks joined (#8759)
Motivation:

Avoid IDE warnings, easier to read.

Modification:

Same catch blocks joined.

Result:

Cleanup
2019-01-22 18:00:10 +01:00
Dmitriy Dumanskiy
42376c052a Java 8 migration. Inline PlatformDependent.newConcurrentHashMap() (#8760)
Motivation:

PlatformDependent.newConcurrentHashMap() is no longer needed so it could be easily removed and new ConcurrentHashMap<>() inlined instead of invoking PlatformDependent.newConcurrentHashMap().

Modification:

Use ConcurrentHashMap provided by the JDK directly.

Result:

Less code to maintain.
2019-01-22 17:18:50 +01:00
田欧
9d62deeb6f Java 8 migration: Use diamond operator (#8749)
Motivation:

We can use the diamond operator these days.

Modification:

Use diamond operator whenever possible.

Result:

More modern code and less boiler-plate.
2019-01-22 16:07:26 +01:00
Dmitriy Dumanskiy
5fb515f4af Java 8 migration. Use string switch where possible (#8753)
Motivation:

Replace "if else" conditions with string switch. It is easier to read the code, for large "if else" constructions switch also could be faster.

Modification:

Replaced "if else" with a string switch.

Result:

Use new language features
2019-01-22 15:58:49 +01:00
Dmitriy Dumanskiy
45c0ea543f Java 8 migration. Replace netty ConcurrentSet with Java KeySet. (#8745)
Motivation:

The concurrent set is present in Java 8 and above so we can use it instead of own implementation.

Modification:

io.netty.utik.internal.ConcurrentSet replaced with ConcurrentHashMap.newKeySet().

Result:

Less code to maintain.
2019-01-22 10:40:40 +01:00
Norman Maurer
4a82d107d2
Require Java8 as minimum (#8739)
Motivation:

While we are not yet quite sure if we want to require Java11 as minimum we are at least sure we want to use java8 as minimum.

Modifications:

Change minimum version to java8 and update some tests which failed compilation after this change.

Result:

Use Java8 as minimum and be able to use Java8 features.
2019-01-22 08:48:18 +01:00
Norman Maurer
1fe931b6e2
Make it possible to use a wrapped EventLoop with a Channel (#8677)
Motiviation:

Because of how we implemented the registration / deregistration of an EventLoop it was not possible to wrap an EventLoop implementation and use it with a Channel.

Modification:

- Introduce EventLoop.Unsafe which is responsible for the actual registration.
- Move validation of EventLoop / Channel combo to the EventLoop
- Add unit test that verifies that wrapping works

Result:

Be able to wrap an EventLoop and so add some extra functionality.
2019-01-17 09:17:51 +01:00
Oleksii Kachaiev
f004b72662 Correctly propagate write failures from ChunkedWriteHandler (#8716)
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
2019-01-16 11:08:11 +01:00
Norman Maurer
c10ccc5dec
Tighten contract between Channel and EventLoop by require the EventLoop on Channel construction. (#8587)
Motivation:

At the moment it’s possible to have a Channel in Netty that is not registered / assigned to an EventLoop until register(...) is called. This is suboptimal as if the Channel is not registered it is also not possible to do anything useful with a ChannelFuture that belongs to the Channel. We should think about if we should have the EventLoop as a constructor argument of a Channel and have the register / deregister method only have the effect of add a Channel to KQueue/Epoll/... It is also currently possible to deregister a Channel from one EventLoop and register it with another EventLoop. This operation defeats the threading model assumptions that are wide spread in Netty, and requires careful user level coordination to pull off without any concurrency issues. It is not a commonly used feature in practice, may be better handled by other means (e.g. client side load balancing), and therefore we propose removing this feature.

Modifications:

- Change all Channel implementations to require an EventLoop for construction ( + an EventLoopGroup for all ServerChannel implementations)
- Remove all register(...) methods from EventLoopGroup
- Add ChannelOutboundInvoker.register(...) which now basically means we want to register on the EventLoop for IO.
- Change ChannelUnsafe.register(...) to not take an EventLoop as parameter (as the EventLoop is supplied on custruction).
- Change ChannelFactory to take an EventLoop to create new Channels and introduce ServerChannelFactory which takes an EventLoop and one EventLoopGroup to create new ServerChannel instances.
- Add ServerChannel.childEventLoopGroup()
- Ensure all operations on the accepted Channel is done in the EventLoop of the Channel in ServerBootstrap
- Change unit tests for new behaviour

Result:

A Channel always has an EventLoop assigned which will never change during its life-time. This ensures we are always be able to call any operation on the Channel once constructed (unit the EventLoop is shutdown). This also simplifies the logic in DefaultChannelPipeline a lot as we can always call handlerAdded / handlerRemoved directly without the need to wait for register() to happen.

Also note that its still possible to deregister a Channel and register it again. It's just not possible anymore to move from one EventLoop to another (which was not really safe anyway).

Fixes https://github.com/netty/netty/issues/8513.
2019-01-14 20:11:13 +01:00
kashike
c0aa1ea5c7 Fix minor spelling issues in javadocs (#8701)
Motivation:

Javadocs contained some spelling errors, we should fix these.

Modification:

Fix spelling

Result:

Javadoc cleanup.
2019-01-14 07:25:13 +01:00
Jon Chambers
400b3081b0 Publicize default explicitFlushAfterFlushes count. (#8683)
Motivation:

Users who want to construct a `FlushConsolidationHandler` with a default `explicitFlushAfterFlushes` but non-default `consolidateWhenNoReadInProgress` may benefit from having an easy way to get the default "flush after flushes" count.

Modifications:

- Moved default `explicitFlushAfterFlushes` value to a public constant.
- Adjusted Javadoc accordingly.

Result:

Default `explicitFlushAfterFlushes` is accessible to callers.
2018-12-25 22:36:23 +01:00
Norman Maurer
26e1411897 Add test for correctly handling SSLSessionBindingEvent when acting on th… (#8649)
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.
2018-12-19 12:56:14 +01:00
Norman Maurer
7f20adb5fc Skip tests that use KeyManagerFactory if not supported by OpenSSL version / flavor (#8662)
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.
2018-12-14 21:34:05 +01:00
Feri73
563793688f Adding support for whitespace in resource path in tests (#8606)
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
2018-12-12 10:29:19 +01:00
Norman Maurer
68f4c82d5a SSLSession.putValue / getValue / removeValue / getValueNames must be thread-safe. (#8648)
Motivation:

SSLSession.putValue / getValue / removeValue / getValueNames must be thread-safe as it may be called from multiple threads. This is also the case in the OpenJDK implementation.

Modifications:

Guard with synchronized (this) blocks to keep the memory overhead low as we do not expect to have these called frequently.

Result:

SSLSession implementation is thread-safe.
2018-12-12 07:41:31 +01:00
Norman Maurer
d9a6cf341c
Remove support for marking reader and writerIndex in ByteBuf to reduce overhead and complexity. (#8636)
Motivation:

ByteBuf supports “marker indexes”. The intended use case for these is if a speculative operation (e.g. decode) is in process the user can “mark” and interface and refer to it later if the operation isn’t successful (e.g. not enough data). However this is rarely used in practice,
requires extra memory to maintain, and introduces complexity in the state management for derived/pooled buffer initialization, resizing, and other operations which may modify reader/writer indexes.

Modifications:

Remove support for marking and adjust testcases / code.

Result:

Fixes https://github.com/netty/netty/issues/8535.
2018-12-11 14:00:49 +01:00
Paul Verest
9c594e5068 ReadTimeoutHandler - missing ) within JavaDoc example (#8645)
Motivation:

improve docs

Modification:

ReadTimeoutHandler - missing ) within JavaDoc example

No logic/unit tests affected
2018-12-10 20:50:43 +01:00
多巴胺
0288570b06 Fix concurrency problem in UniqueIpFilter (#8635)
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.
2018-12-07 13:50:44 +01:00
Feri73
5618229203 Correcting Maven Dependencies (#8622)
Motivation:

Most of the maven modules do not explicitly declare their
dependencies and rely on transitivity, which is not always correct.

Modifications:

For all maven modules, add all of their dependencies to pom.xml

Result:

All of the (essentially non-transitive) depepdencies of the modules are explicitly declared in pom.xml
2018-12-06 09:02:00 +01:00
Norman Maurer
2c78dde749 Update version number to start working on Netty 5 2018-11-20 15:49:57 +01:00
Norman Maurer
38524ec3e2
Fix test that assumed detection of peer supported algs is not supported in BoringSSL. (#8573)
Motivation:

0d2e38d5d6 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.
2018-11-19 12:13:05 +01:00
JStroom
ce02d5a184 Update SslHandler.java (#8564)
Swallow SSL Exception "closing inbound before receiving peer's close_notify" when running on Java 11 (#8463)

Motivation:

When closing a inbound SSL connection before the remote peer has send a close notify, the Java JDK is trigger happy to throw an exception. This exception can be ignored since the connection is about to be closed.
The exception wasn't printed in Java 8, based on filtering on the exception message. In Java 11 the exception message has been changed.

Modifications:
Update the if statement to also filter/swallow the message on Java 11.

Result:
On Java 11 the exception isn't printed with log levels set to debug. The old behaviour is maintained.
2018-11-16 10:32:34 +01:00
Norman Maurer
8d4d76d216
ReferenceCountedOpenSslEngine SSLSession.getLocalCertificates() / getLocalPrincipial() did not work when KeyManagerFactory was used. (#8560)
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.
2018-11-16 07:38:32 +01:00
Norman Maurer
20d4fda55e
Return the correct pointer from ReferenceCountedOpenSslContext.context() and sslCtxPointer() (#8562)
Motivation:

We did not return the pointer to SSL_CTX put to the internal datastructure of tcnative.

Modifications:

Return the correct pointer.

Result:

Methods work as documented in the javadocs.
2018-11-16 07:37:57 +01:00
Norman Maurer
0d2e38d5d6
Correctly convert supported signature algorithms when using BoringSSL (#8481)
* 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.
2018-11-14 19:23:11 +01:00
Norman Maurer
d1654484c1
Correctly convert between openssl / boringssl and java cipher names when using TLSv1.3 (#8485)
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.
2018-11-14 08:49:13 +01:00
Tim Brooks
11ec7d892e Cleanup SslHandler handshake/renegotiation (#8555)
Motivation:

The code for initiating a TLS handshake or renegotiation process is
currently difficult to reason about.

Modifications:

This commit introduces to clear paths for starting a handshake. The
first path is a normal handshake. The handshake is started and a timeout
is scheduled.

The second path is renegotiation. If the first handshake is incomplete,
the renegotiation promise is added as a listener to the handshake
promise. Otherwise, the renegotiation promise replaces the original
promsie. At that point the handshake is started again and a timeout is
scheduled.

Result:

Cleaner and easier to understand code.
2018-11-14 08:19:06 +01:00
Norman Maurer
4c73d24ea8
Handshake timeout may never be scheduled if handshake starts via a flush or starttls is used. (#8494)
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.
2018-11-13 19:22:38 +01:00
Norman Maurer
c0dfb568a2
SSLHandler may throw AssertionError if writes occur before channelAct… (#8486)
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.
2018-11-11 07:23:08 +01:00
Norman Maurer
8a24df88a4
Ensure we correctly call wrapEngine(...) during tests. (#8473)
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
2018-11-08 15:22:33 +01:00
Norman Maurer
fd57d971d1
Override and so delegate all methods in OpenSslX509Certificate (#8472)
Motivation:

We did not override all methods in OpenSslX509Certificate and delegate to the internal 509Certificate.

Modifications:

Add missing overrides.

Result:

More correct implementation
2018-11-07 12:16:04 +01:00
Norman Maurer
4760dc5c2d
Don't double release ByteBuf when parsing of the X509Certificate fails (#8457)
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.
2018-11-02 17:08:53 +01:00
Daniel Gartmann
9c70dc8ba5 Replaced obsolete cryptographic primitive with a modern/secure one. (#8450)
Motivation:

SHA1 is a broken hash function and shouldn't be used anymore (see: https://shattered.io/).
Security scanning tools will raise this as an issue and it will reflect badly on netty and I, therefore, recommend to use a SHA2 hash function which is secure and won't be flagged by such tools.

Modifications:

Replaced insecure SHA1 based signing scheme with SHA2.

Result:

Modern and thus secure cryptographic primitives will be in use and won't be flagged by security scanning tools.
2018-11-02 07:20:54 +01:00
Norman Maurer
52699bd6dd
Add test to verify that invalid ciphers are handled in all SSLEngine implementations correctly. (#8443)
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
2018-10-30 17:58:08 +01:00
Norman Maurer
46460de243
Correctly init X509Certificate array when testing if we need to wrap the KeyManager due of TLSv1.3 (#8435)
Motivation:

201e984cb3 added support to use native TLSv1.3 support even with Java versions prior to 11. For this we try to detect if we need to wrap the used KeyManager or not. This testing code did create an X509Certificate[1] but does not correctly also set the certficiate on index 0. While this should be harmless we should better do the right thing and set it.

Modifications:

Correctly init the array.

Result:

Cleaner and more correct code.
2018-10-30 08:17:31 +01:00
root
3e7ddb36c7 [maven-release-plugin] prepare for next development iteration 2018-10-29 15:38:51 +00:00
root
9e50739601 [maven-release-plugin] prepare release netty-4.1.31.Final 2018-10-29 15:37:47 +00:00
Norman Maurer
ce39773e04
Add support for boringssl and TLSv1.3 (#8412)
Motivation:

0ddc62cec0 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.
2018-10-26 15:29:49 -07:00
Norman Maurer
9e762e8816
Correctly detect if KeyManagerFactory is supported by OpenSSL even when sun.security.x509.* can not be accessed and bouncycastle is not on the classpath. (#8415)
Motivation:

OpenSsl used SelfSignedCertificate in its static init block to detect if KeyManagerFactory is supported. Unfortunally this only works when either sun.security.x509.* can be accessed or bouncycastle is on the classpath.
We should not depend on either of it.

This came up in https://github.com/netty/netty-tcnative/issues/404#issuecomment-431551890.

Modifications:

Just directly use the bytes to generate the X509Certificate and so not depend on sun.security.x509.* / bouncycastle.

Result:

Correctly be able to detect if KeyManagerFactory can be supported in all cases.
2018-10-23 17:08:23 +02:00
Norman Maurer
91201fb338
Remove workaround in tests for TLSv1.3 bug in Java11 as it was fixed in 11.0.1 (#8409)
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.
2018-10-19 17:21:04 +02:00
Norman Maurer
201e984cb3
Allow to use TLSv1.3 with netty-tcnative withe java versions prior to 11. (#8394)
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.
2018-10-18 13:50:12 +02:00
Norman Maurer
3543e17967
Ensure OpenSslEngine will not try to call SSL_free multiple times even when constructor throws. (#8399)
Motivation:

When the constructor of OpenSslEngine threw we could end up to self call SSL_free by ourself and then have the finalizer do the same which may lead to double free-ing and so SIGSEV.

Modifications:

Just call shutdown() when the constructor throws and so ensure SSL_free is guarded correctly in the finalizer.

Result:

No more SIGSEV possible.
2018-10-18 07:38:03 +02:00
Norman Maurer
0ddc62cec0
Add support for TLSv1.3 (#8293)
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.
2018-10-17 08:35:35 +02:00
Stephane Landelle
9eebe7ed74 Add full JdkSslContext public constructor, close #8384 (#8389)
Motivation:

JdkSslContext provides public constructors to wrap an existing `javax.net.ssl.SSLContext`.

Sadly, some options combinations are not possible with the existing constructors, eg:
*  protocols is not exposed and always forced to null, so default protocols are always enforced
* startTls is not exposed and always forced to false

Modification:

Add full constructor that take protocols and startTls parameters.

Result:

It's possible to create a JdkSslContext from an existing SSLContext and still have control over protocols and startTls
2018-10-17 08:28:39 +02:00
Norman Maurer
aae7cdca96
Prevent NPE when attempting to set client key material with no alias (#8378)
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.
2018-10-12 09:27:46 +02:00
Norman Maurer
59973e93dd
Ensure X509KeyManager methods are called on the correct time when using server-side and support more methods of ExtendedSSLSession. (#8283)
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.
2018-09-28 11:34:38 +02:00
Norman Maurer
73acac13f4
Check if hostname validation is supported before trying to use in test. (#8333)
Motivation:

a208f6dc7c 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.
2018-09-28 10:54:05 +02:00
Norman Maurer
a208f6dc7c
Do the same extended checks as the JDK when a X509TrustManager is used with the OpenSSL provider. (#8307)
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.
2018-09-28 09:19:58 +02:00
root
2d7cb47edd [maven-release-plugin] prepare for next development iteration 2018-09-27 19:00:45 +00:00
root
3a9ac829d5 [maven-release-plugin] prepare release netty-4.1.30.Final 2018-09-27 18:56:12 +00:00
Norman Maurer
652650e127
Fix leak in SniClientJava8TestUtil (#8326)
Motivation:

4d1458604a did fix some leaks in SniClientTest but missed the ones in SniClientJava8TestUtil.

Modifications:

Correctly release SslContext.

Result:

No more leaks in SNI tests.
2018-09-27 09:32:41 +02:00
Norman Maurer
4d1458604a
Fix leak in SniClientTest. (#8324)
Motivation:

We need to release the ReferenceCountedSslContext to eliminate resource leaks.

Reported in https://garage.netty.io/teamcity/viewLog.html?buildId=33353&buildTypeId=netty_build_oraclejdk8&tab=buildLog#_focus=157264.

Modifications:

Call release on the SslContext instances.

Result:

No more leaks in tests.
2018-09-26 19:59:00 +02:00
Norman Maurer
a80c49828f
Cleanup SSL test. (#8301)
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.
2018-09-21 08:04:24 -07:00
Norman Maurer
01db30a163
Correctly implement ExtendedSSLSession.getStatusResponses() for ReferenceCountedOpenSslEngine (#8297)
Motivation:

Java9 added getStatusResponses() to ExtendedSSLSession which we should correctly support when possible.

Modifications:

Implement the method correctly.

Result:

More complete and correct implementation.
2018-09-19 17:13:44 -07:00
Norman Maurer
34d52fcbfe
Implemented ExtendedOpenSslSession.getStatusResponses() so it not throws an UnsupportedOperationException. (#8290)
Motivation:

6ed7c6c75d added support for ExtendedOpenSslSession but we did not override getStatusResponses(). This lead to test failures on java9.

Modifications:

Implement ExtendedOpenSslSession.getStatusResponses() so it just returns an empty list.

Result:

Test pass again on Java9.
2018-09-14 20:33:09 +02:00
Norman Maurer
2b1514ec5a
Only use KeyManagerFactory in SniClientTest when supported by OpenSSL version. (#8289)
Motivation:

6ed7c6c75d 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.
2018-09-14 19:01:55 +02:00