Commit Graph

826 Commits

Author SHA1 Message Date
Norman Maurer
0e4c073bcf
Remove the intermediate List from ByteToMessageDecoder (and sub-class… (#8626)
Motivation:

ByteToMessageDecoder requires using an intermediate List to put results into. This intermediate list adds overhead (memory/CPU) which grows as the number of objects increases. This overhead can be avoided by directly propagating events through the ChannelPipeline via ctx.fireChannelRead(...). This also makes the semantics more clear and allows us to keep track if we need to call ctx.read() in all cases.

Modifications:

- Remove List from the method signature of ByteToMessageDecoder.decode(...) and decodeLast(...)
- Adjust all sub-classes
- Adjust unit tests
- Fix javadocs.

Result:

Adjust ByteToMessageDecoder as noted in https://github.com/netty/netty/issues/8525.
2019-12-16 21:00:32 +01:00
Norman Maurer
ff8846f1b5
Replace ObjectUtil.checkNonNull(...) with Objects.requireNonNull(...) (#9864)
Motivation:

We should use Objects.requireNonNull(...) as we require java8

Modifications:

Replace ObjectUtil.checkNonNull(...) with Objects.requireNonNull(...)

Result:

Code cleanup
2019-12-10 11:27:32 +01:00
Norman Maurer
29c471ec52 Correctly handle fragmented Handshake message when trying to detect SNI (#9806)
Motivation:

At the moment our AbstractSniHandler makes the assemption that Handshake messages are not fragmented. This is incorrect as it is completely valid to split these across multiple TLSPlaintext records.

Thanks to @sskrobotov for bringing this to my attentation and to @Lukasa for the help.

Modifications:

- Adjust logic in AbstractSniHandler to handle fragmentation
- Add unit tests

Result:

Correctly handle fragmented Handshake message in AbstractSniHandler (and so SniHandler).
2019-11-29 09:53:29 +01:00
Norman Maurer
b7ba807b30 Fix compile error introduced by 2c3d263e23 2019-11-27 09:08:20 +01:00
ZhenLian
2c3d263e23 Support Passing KeyManager and TrustManager into SslContextBuilder (#9805) (#9786)
Motivation:

This is a PR to solve the problem described here: https://github.com/netty/netty/issues/9767
Basically this PR is to add two more APIs in SslContextBuilder, for users to directly specify
the KeyManager or TrustManager they want to use when building SslContext. This is very helpful
when users want to pass in some customized implementation of KeyManager or TrustManager.

Modification:

This PR takes the first approach in here:
https://github.com/netty/netty/issues/9767#issuecomment-551927994 (comment)
which is to immediately convert the managers into factories and let factories continue to pass
through Netty.

1. Add in SslContextBuilder the two APIs mentioned above
2. Create a KeyManagerFactoryWrapper and a TrustManagerFactoryWrapper, which take a KeyManager
and a TrustManager respectively. These are two simple wrappers that do the conversion from
XXXManager class to XXXManagerFactory class
3.Create a SimpleKeyManagerFactory class(and internally X509KeyManagerWrapper for compatibility),
which hides the unnecessary details such as KeyManagerFactorySpi. This serves the similar
functionalities with SimpleTrustManagerFactory, which was already inside Netty.

Result:

Easier usage.
2019-11-26 14:26:04 +01:00
stroller
2e449a6769 Improve java doc for MINIMAL_WAIT (#9779)
Motivation:

MINIMAL_WAIT is the key constant. Thus, When we see the constant, we must read more code logic to see if it is ms or ns. So improving java doc will be better.

Modifications:
Improve java doc by add "10ms" such as DEFAULT_CHECK_INTERVAL with "1s".

Result:

Easy to know it is ms and keep same java doc style with other constants such as DEFAULT_CHECK_INTERVAL.
2019-11-17 17:42:10 +01:00
Norman Maurer
f8b05b1c84 Don't cache key material if sun.security.ssl.X509KeyManagerImpl is used (#9762)
Motivation:

sun.security.ssl.X509KeyManagerImpl will not use "stable" aliases and so aliases may be changed during invocations. This means caching is useless. Because of this we should disable the cache if its used.

Modifications:

- Disable caching if sun.security.ssl.X509KeyManagerImpl is used
- Add tests

Result:

More protection against https://github.com/netty/netty/issues/9747.
2019-11-07 15:28:00 +01:00
Norman Maurer
38dd3b6bd1 At the moment the cache provided by OpenSslCachingKeyMaterialProvider… (#9759)
Motivation:

At the moment te cache is not bound and so lead to huge memory consumpation. We should ensure its bound by default.

Modifications:

Ensure cache is bound

Result:

Fixes https://github.com/netty/netty/issues/9747.
2019-11-07 15:27:42 +01:00
Johno Crawford
6ad59d14d7 Complete todo in SelfSignedCertificate (#9720)
Motivation:

Easier to debug SelfSignedCertificate failures.

Modifications:

Add first throwable as suppressed to thrown exception.

Result:

Less technical debt.
2019-10-28 14:52:14 +01:00
Norman Maurer
4be554a21f Hide Recycler implemention to allow experimenting with different implementions of an Object pool (#9715)
Motivation:

At the moment we directly extend the Recycler base class in our code which makes it hard to experiment with different Object pool implementation. It would be nice to be able to switch from one to another by using a system property in the future. This would also allow to more easily test things like https://github.com/netty/netty/pull/8052.

Modifications:

- Introduce ObjectPool class with static method that we now use internally to obtain an ObjectPool implementation.
- Wrap the Recycler into an ObjectPool and return it for now

Result:

Preparation for different ObjectPool implementations
2019-10-26 09:43:21 +02:00
Anuraag Agrawal
e4d400fa4a Accept Iterable as argument to SslContextBuilder methods. (#9711)
Motivation:

It is common, especially in frameworks, for the parameters to `SslContextBuilder` methods to be built up as a `List` or similar `Iterable`. It is currently difficult to use `SslContextBuilder` in this case because it requires a conversion to array.

Modification:

Add overloads for methods that accept varargs to also accept `Iterable`, delegating by copying into an array.

Result:

Fixes #9293
2019-10-25 15:02:43 +02:00
Zhao Yang
b002f1ffc1 Changed Netty JDK SSL to use default protocols instead of hardcoded supported (#9707)
Motivation:

Netty should respect JVM flags to control SSL protocols, eg. `-Djdk.tls.client.protocols`

Modification:

Changed `JdkSslContext` to use `SSLContext.getDefaultSSLParameters().getProtocols()` instead of `engine.getSupportedProtocols()` which is hardcoded as `SSLv2Hello, SSLv3, TLSv1, TLSv1.1, TLSv1.2`.

Result:

Without `-Djdk.tls.client.protocols`, `SSLContext.getDefaultSSLParameters().getProtocols()` returns `TLSv1, TLSv1.1, TLSv1.2`.

With `-Djdk.tls.client.protocols=TLSv1.2`, `SSLContext.getDefaultSSLParameters().getProtocols()` returns `TLSv1.2`.

Fixes #9706
2019-10-25 08:43:39 +02:00
Carl Mastrangelo
05480c190f Make only default IdleStateEvents cached string representation (#9705)
Motivation:

In PR https://github.com/netty/netty/pull/9695   IdleStateEvents
were made to cache their string representation.   The reason for this
was to avoid creating garbage as these values would be used frequently.
However, these objects may be used on multiple event loops and this
may cause an unexpected race to occur.

Modification:
Only make the events that Netty creates cache their toString representation.

Result:
No races.
2019-10-24 08:35:59 +02:00
Norman Maurer
7150b42a56 Refactor SslHandler internals to always use heap buffers for JDK SSLE… (#9696)
Motivation:

We should aim to always use heap buffers when using the JDK SSLEngine for now as it wants to operate on byte[] and so will do internal memory copies if a non heap buffer is used. Beside this it will always return BUFFER_OVERFLOW when a smaller buffer then 16kb is used when calling wrap(...) (even if a very small amount of bytes should be encrypted). This can lead to excercive direct memory usage and pressure for no good reason.

Modifications:

Refactor internals of SslHandler to ensure we use heap buffers for the JDK SSLEngine impelementation

Result:

Less direct memory usage when JDK SSLEngine implementation is used
2019-10-23 14:29:01 +02:00
ursa
67e3ddb568 Bugfix #9667: FlowControllerHandler swallows read-complete event when auto-read is disabled (#9691)
FlowControllerHandler currently may swell read-complete events in some situations.

* Fire read-complete event from flow controller, when it previously was swallowed
* New unit test to cover this case

Fixes #9667: FlowControllerHandler swallows read-complete event when auto-read is disabled
2019-10-23 11:02:05 +02:00
ursa
aa78d49de1 Add 'toString' method into IdleStateEvent (#9695)
IdleStateEvent is very convenient and frequently used type of events. However both in runtime (logs) and in debug you need some manual steps to see their actual content. Default implementation generates worthless trash like this:

    io.netty.handler.timeout.IdleStateEvent@27f674d

There are examples already, where event has convenient and useful toString implementation:

* io.netty.handler.proxy.ProxyConnectionEvent
* io.netty.handler.ssl.SslCompletionEvent

* Implement 'IdleStateEvent.toString' method.
* Unit test.

More useful String representation of IdleStateEvent
2019-10-23 10:48:25 +02:00
Idel Pivnitskiy
c9c290019e Add a utility that checks if the a SslProvider supports ALPN (#9693)
Motivation:

We have a public utility `OpenSsl.isAlpnSupported()` that helps users to
check if ALPN is available for `SslProvider.OPENSSL`. However, we do not
provide a similar utility for `SslProvider.JDK`. Therefore, users who
configured ALPN with `SslProvider.JDK` will get a runtime exception at
the time when a new connection will be created.

Modifications:

- Add public `SslProvider.isAlpnSupported(SslProvider)` utility method
that returns `true` if the `SslProvider` supports ALPN;
- Deprecate `OpenSsl.isAlpnSupported()`;

Result:

Users can verify if their environment supports ALPN with
`SslProvider` upfront (at bootstrap), instead of failing with
runtime exception when a new connection will be created.
2019-10-23 08:58:32 +02:00
Norman Maurer
7d767d08f5 Add ability to set attributes on a SslContext (#9654)
Motivation:

Sometimes it is useful to be able to set attributes on a SslContext.

Modifications:

Add new method that will return a AttributeMap that is tied to a SslContext instance

Result:

Fixes https://github.com/netty/netty/issues/6542.
2019-10-22 15:40:03 +02:00
Norman Maurer
69575cf5a6 Remove usage of AtomicIntegerFieldUpdater in ReferenceCountedOpenSslE… (#9653)
Motivation:

There is not need to use a CAS as everything is synchronized anyway. We can simplify the code a bit by not using it.

Modifications:

- Just remove the CAS operation
- Change from int to boolean

Result:

Code cleanup
2019-10-13 11:03:08 +02:00
Tim Brooks
678983f2a7 Do not mandate direct bytes in SslHandler queue (#9656)
Motivation:

Currently when the SslHandler coalesces outbound bytes it always
allocates a direct byte buffer. This does not make sense if the JDK
engine is being used as the bytes will have to be copied back to heap
bytes for the engine to operate on them.

Modifications:

Inspect engine type when coalescing outbound bytes and allocate heap
buffer if heap bytes are preferred by the engine.

Result:

Improved performance for JDK engine. Better performance in environments
without direct buffer pooling.
2019-10-12 20:12:37 +02:00
康智冬
1c69448e2e Fix typos in javadocs (#9527)
Motivation:

We should have correct docs without typos

Modification:

Fix typos and spelling

Result:

More correct docs
2019-10-09 15:25:41 +02:00
Bryce Anderson
3f7a7949db Reference-counted SslEngines retain a reference to their parent SslContext (#9626)
Motivation:
With the Netty ref-counted OpenSSL implementation the parent SslContext
maintains state necessary for the SslEngine's it produces. However, it's
possible for the parent context to be closed and release those resources
before the child engines are finished which causes problems.

Modification:
Spawned ReferenceCountedOpenSslEngine's retain a reference to their
parent ReferenceCountedOpenSslContext.

Result:
The lifetime of the shared data is extended to include the lifetime of
the dependents.
2019-10-07 08:13:05 +02:00
Francesco Nigro
e5eb94668c ChunkedNioFile can use absolute FileChannel::read to read chunks (#9592)
Motivation:

Users can reuse the same FileChannel for different ChunkedNioFile
instances without being worried that FileChannel::position will be
changed concurrently by them.
In addition, FileChannel::read with absolute position allows to
use on *nix pread that is more efficient then fread.

Modifications:

Always use absolute FileChannel::read ops

Result:

Faster and more flexible uses of FileChannel for ChunkedNioFile
2019-09-24 07:17:26 +02:00
Norman Maurer
fafde4aeec No need to explicit use the AccessController when SystemPropertyUtil is used (#9577)
Motivation:

SystemPropertyUtil already uses the AccessController internally so not need to wrap its usage with AccessController as well.

Modifications:

Remove explicit AccessController usage when SystemPropertyUtil is used.

Result:

Code cleanup
2019-09-19 08:50:36 +02:00
Norman Maurer
2ba99b4996 Correctly handle task offloading when using BoringSSL / OpenSSL (#9575)
Motivation:

We did not correctly handle taskoffloading when using BoringSSL / OpenSSL. This could lead to the situation that we did not write the SSL alert out for the remote peer before closing the connection.

Modifications:

- Correctly handle exceptions when we resume processing on the EventLoop after the task was offloadded
- Ensure we call SSL.doHandshake(...) to flush the alert out to the outboundbuffer when an handshake exception was detected
- Correctly signal back the need to call WRAP again when a handshake exception is pending. This will ensure we flush out the alert in all cases.

Result:

No more failures when task offloading is used.
2019-09-19 08:17:45 +02:00
Norman Maurer
cda8ee95b3 Correctly synchronize before trying to set key material to fix possible native crash (#9566)
Motivation:

When using io.netty.handler.ssl.openssl.useTasks=true we may call ReferenceCountedOpenSslEngine.setKeyMaterial(...) from another thread and so need to synchronize and also check if the engine was destroyed in the meantime to eliminate of the possibility of a native crash.
The same is try when trying to access the authentication methods.

Modification:

- Add synchronized and isDestroyed() checks where missing
- Add null checks for the case when a callback is executed by another thread after the engine was destroyed already
- Move code for master key extraction to ReferenceCountedOpenSslEngine to ensure there can be no races.

Result:

No native crash possible anymore when using io.netty.handler.ssl.openssl.useTasks=true
2019-09-16 11:15:06 +02:00
Norman Maurer
027aec23b8 Allow to build on powerpc
Motivation:

At the moment it is not possible to build netty on a power 8 systems.

Modifications:

- Improve detection of the possibility of using Conscrypt
- Skip testsuite-shading when not on x86_64 as this is the only platform for which we build tcnative atm
- Only include classifier if on x86_64 for tcnative as dependency as this is the only platform for which we build tcnative atm
- Better detect if UDT test can be run

Result:

Fixes https://github.com/netty/netty/issues/9479
2019-09-13 22:21:36 +02:00
stroller
38e5983463 Fix WriteTimeoutException java doc description (#9554)
Motivation:

The java doc doesn't match the real case: The exception only happen when a write operation
 cannot finish in a certain period of time instead of write idle happen.

Modification:

Correct java doc

Result:
java doc matched the real case
2019-09-09 13:59:06 +02:00
Xiaoqin Fu
88aa12cc1a Remove extra checks to fix #9456 (#9523)
Motivation:

There are some extra log level checks (logger.isWarnEnabled()).

Modification:

Remove log level checks (logger.isWarnEnabled()) from io.netty.channel.epoll.AbstractEpollStreamChannel, io.netty.channel.DefaultFileRegion, io.netty.channel.nio.AbstractNioChannel, io.netty.util.HashedWheelTimer, io.netty.handler.stream.ChunkedWriteHandler and io.netty.channel.udt.nio.NioUdtMessageConnectorChannel

Result:

Fixes #9456
2019-08-30 10:40:04 +02:00
Codrut Stancu
de126fdf65 Update GraalVM Native Image configuration. (#9515)
Motivation:

The Netty classes are initialized at build time by default for GraalVM Native Image compilation. This is configured via the `--initialize-at-build-time=io.netty` option. While this reduces start-up time it can lead to some problems:

 - The class initializer of `io.netty.buffer.PooledByteBufAllocator` looks at the maximum memory size to compute the size of internal buffers. If the class initializer runs during image generation, then the buffers are sized according to the very large heap size that the image generator uses, and Netty allocates several arrays that are 16 MByte. The fix is to initialize the following 3 classes at run time: `io.netty.buffer.PooledByteBufAllocator,io.netty.buffer.ByteBufAllocator,io.netty.buffer.ByteBufUtil`. This fix was dependent on a GraalVM Native Image fix that was included in 19.2.0.

 - The class initializer of `io.netty.handler.ssl.util.ThreadLocalInsecureRandom` needs to be initialized at runtime to ensure that the generated values are trully random and not fixed for each generated image.

 - The class initializers of `io.netty.buffer.AbstractReferenceCountedByteBuf` and `io.netty.util.AbstractReferenceCounted` compute field offsets. While the field offset recomputation is necessary for correct execution as a native image these initializers also have logic that depends on the presence/absence of `sun.misc.Unsafe`, e.g., via the `-Dio.netty.noUnsafe=true` flag. The fix is to push these initializers to runtime so that the field offset lookups (and the logic depending on them) run at run time. This way no manual substitutions are necessary either.
 
Modifications:

Add `META-INF/native-image` configuration files that correctly trigger the inialization of the above classes at run time via `--initialize-at-run-time=...` flags.
 
Result:

Fixes the initialisation issues described above for Netty executables built with GraalVM.
2019-08-30 09:21:33 +02:00
Norman Maurer
6f616bb3cf Avoid creating FileInputStream and FileOutputStream for obtaining Fil… (#8110)
Motivation:

If all we need is the FileChannel we should better use RandomAccessFile as FileInputStream and FileOutputStream use a finalizer.

Modifications:

Replace FileInputStream and FileOutputStream with RandomAccessFile when possible.

Result:

Fixes https://github.com/netty/netty/issues/8078.
2019-08-17 09:52:16 +02:00
Norman Maurer
16e290796d Always wrap X509ExtendedTrustManager when using OpenSSL and JDK < 11 (#9443)
Motivation:

When using OpenSSL and JDK < 11 is used we need to wrap the user provided X509ExtendedTrustManager to be able to support TLS1.3. We had a check in place that first tried to see if wrapping is needed at all which could lead to missleading calls of the user provided trustmanager. We should remove these calls and just always wrap if needed.

Modifications:

Always wrap if OpenSSL + JDK < 11 and TLS1.3 is supported

Result:

Less missleading calls to user provided trustmanager
2019-08-13 10:26:56 +02:00
Nico Kruber
d285623925 Try to load native linux libraries with matching classifier first (#9411)
Motivation:

Users' runtime systems may have incompatible dynamic libraries to the ones our
tcnative wrappers link to. Unfortunately, we cannot determine and catch these
scenarios (in which the JVM crashes) but we can make a more educated guess on
what library to load and try to find one that works better before crashing.

Modifications:

1) Build dynamically linked openSSL builds for more OSs (netty-tcnative)
2) Load native linux libraries with matching classifier (first)

Result:

More developers / users can use the dynamically-linked native libraries.
2019-08-12 08:48:58 +02:00
Farid Zakaria
794bb6c7b6 Introduce SslMasterKeyHandler (#8653)
Motivation

Debugging SSL/TLS connections through wireshark is a pain -- if the cipher used involves Diffie-Hellman then it is essentially impossible unless you can have the client dump out the master key [1]

This is a work-in-progress change (tests & comments to come!) that introduces a new handler you can set on the SslContext to receive the master key & session id. I'm hoping to get feedback if a change in this vein would be welcomed.

An implementation that conforms to Wireshark's NSS key log[2] file is also included.

Depending on feedback on the PR going forward I am planning to "clean it up" by adding documentation, example server & tests. Implementation will need to be finished as well for retrieving the master key from the OpenSSL context.

[1] https://jimshaver.net/2015/02/11/decrypting-tls-browser-traffic-with-wireshark-the-easy-way/
[2] https://developer.mozilla.org/en-US/docs/Mozilla/Projects/NSS/Key_Log_Format

Modification

- Added SslMasterKeyHandler
- An implementation of the handler that conforms to Wireshark's key log format is included.

Result:

Be able to debug SSL / TLS connections more easily.

Signed-off-by: Farid Zakaria <farid.m.zakaria@gmail.com>
2019-07-10 12:16:56 +02:00
jingene
af614e4d6e Change the netty.io homepage scheme(http -> https) (#9344)
Motivation:

Netty homepage(netty.io) serves both "http" and "https".
It's recommended to use https than http.
Modification:

I changed from "http://netty.io" to "https://netty.io"
Result:

No effects.
2019-07-09 21:10:14 +02:00
Norman Maurer
f7e8603d60 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:39 +02:00
Cory Benfield
e0094b2f89 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:23:31 +02:00
Farid Zakaria
cc1528bdad 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:45:05 +02:00
jimin
78adeb5408 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:52:06 +02:00
Stephane Landelle
e6adf1a590 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:47 +02:00
Norman Maurer
9b37be9550 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:29 +02:00
Frédéric Brégier
1a487a0ff9 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:35:10 +02:00
Scott Mitchell
bf7f41a993 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:45:25 +02:00
Norman Maurer
9c51a8c6d4 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:08:40 +02:00
Norman Maurer
bbb397ac5c Remove usage of io.netty.handler.ssl.openssl.useKeyManagerFactory system property
Motivation:

Usafe of io.netty.handler.ssl.openssl.useKeyManagerFactory system property was deprecated in 4.1 so let us remove it.

Modifications:

Remove io.netty.handler.ssl.openssl.useKeyManagerFactory usage.

Result:

Remove support of deprecated system property
2019-05-22 09:09:32 +02:00
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
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