Commit Graph

1282 Commits

Author SHA1 Message Date
Roger
5aaa16b24c An unit test to ensure that cipher suites don't break/disappear between releases. (#8225)
Motivation

Ensure classes of cipher suites continue working between releases. Adding just a DHE check for now as it caused #8165 but this test can be expaned to other suites.

Modifications

Adding an unit test that checks for the presence of a cipher suite.

Result

Prevent #8165 from happening in the future.
2018-08-29 14:14:26 +02:00
root
a580dc7585 [maven-release-plugin] prepare for next development iteration 2018-08-24 06:36:33 +00:00
root
3fc789e83f [maven-release-plugin] prepare release netty-4.1.29.Final 2018-08-24 06:36:06 +00:00
Norman Maurer
df00539fa2
Allow to load PrivateKey via OpenSSL Engine (#8200)
Motivation:

OpenSSL itself has an abstraction which allows you to customize some things. For example it is possible to load the PrivateKey from the engine. We should support this.

Modifications:

Add two new static methods to OpenSslX509KeyManagerFactory which allow to create an OpenSslX509KeyManagerFactory that loads its PrivateKey via the OpenSSL Engine directly.

Result:

More flexible usage of OpenSSL possible
2018-08-18 07:20:44 +02:00
Norman Maurer
bbe2e4d224
We should try to load netty-tcnative before using it in OpenSslCertificateException. (#8202)
Motivation:

In OpenSslCertificateException we should ensure we try to load netty-tcnative before trying to use any class from it as otherwise it may throw an error due missing linking of the native libs.

Modifications:

- Ensure we call OpenSsl.isAvailable() before we try to use netty-tcnative for validation
- Add testcase.

Result:

No more errors causing by not loading native libs before trying to use these.
2018-08-18 06:26:45 +02:00
Norman Maurer
8255f85f24
Rename SslHandler.close(...) to closeOutbound(...) as it is still useful and delegate to the methods. (#8193)
* Rename SslHandler.close(...) to closeOutbound(...) as it is still useful and delegate to the methods.

Motivation:

Sometimes the user may want to send a close_notify without closing the underlying Channel. For this we offered the SslHandler.close(...) methods which were marked as deeprecated. We should offer an way to still do this without the user calling deprecated methods.

See https://stackoverflow.com/questions/51710231/using-nettys-sslhandlerclosechannelhandlercontext-channelpromise/51753742#comment90555949_51753742 .

Modifications:

- Remove deprecation of the SslHandler.close(...) method that exactly allows this and rename these to closeOutbound(...) as this is more clear.
- Add close(...) methods that delegate to these and mark these as deprecated.

Result:

Be able to send close_notify without closing the Channel.
2018-08-15 20:07:56 +02:00
Lai Jiang
dbc9ec1ab2 Set SNI servernames in OpenSSL engine when created in client mode (#8178)
Motivation:

When using the JDK SSL provider in client mode, the SNI host names (called serverNames in SslEngineImpl) is set to the peerHost (if available) that is used to initialize the SSL Engine:

http://hg.openjdk.java.net/jdk8/jdk8/jdk/file/687fd7c7986d/src/share/classes/sun/security/ssl/SSLEngineImpl.java#l377

This allows one to call SslEngine.getSSLParameters() and inspect what is the SNI name to be sent. The same should be done in the OpenSSL provider as well. Currently even though the the SNI name is sent by the OpenSSL provider during handshake when the peerHost is specified, it is missing from the parameters.

Modification:

Set the sniHostNames field when SNI is to be used. Also verifies the peer is actually a hostname before setting it as the SNI name, which is consistent with JDK SSL provider's behavior.

Result:

SslEngine using the OpenSSL provider created in client mode with peerHost will initialize sniHostNames with the peerHost.

Calling SslEngine.getSSLParameters().getServerNames() will return a list that contains that name.
2018-08-09 21:30:02 +02:00
Norman Maurer
44d3753c48
Fix NPE exception when using invalid cipher during building SslContext. (#8171)
Motivation:

We missed to do a null check before trying to destroy the OpenSslSessionContext, which could lead to a NPE.

Modifications:

Add null check and tests.

Result:

Fix https://github.com/netty/netty/issues/8170.
2018-08-02 21:42:21 +02:00
Norman Maurer
fe14bad69c
Adjust SSL related tests to be more correct and so pass in the next EA release of java11. (#8162)
Motivation:

In some of our tests we not correctly init the SSLEngine before trying to perform a handshake which can cause an IllegalStateException. While this not happened in previous java releases it does now on Java11 (which is "ok" as its even mentioned in the api docs). Beside this how we selected the ciphersuite to test renegotation was not 100 % safe.

Modifications:

- Correctly init SSLEngine before using it
- Correctly select ciphersuite before testing for renegotation.

Result:

More correct tests and also pass on next java11 EA release.
2018-08-01 06:37:53 +02:00
root
fcb19cb589 [maven-release-plugin] prepare for next development iteration 2018-07-27 04:59:28 +00:00
root
ff785fbe39 [maven-release-plugin] prepare release netty-4.1.28.Final 2018-07-27 04:59:06 +00:00
Norman Maurer
620dad0c26
Allow to validate sni hostname with underscore (#8150)
Motivation:

We should allow to also validate sni hostname which contains for example underscore when using our native SSL impl. The JDK implementation does this as well.

Modifications:

- Construct the SNIHostName via byte[] and not String.
- Add unit test

Result:

Fixes https://github.com/netty/netty/issues/8144.
2018-07-27 01:56:32 +08:00
root
b4dbdc2036 [maven-release-plugin] prepare for next development iteration 2018-07-11 15:37:40 +00:00
root
1c16519ac8 [maven-release-plugin] prepare release netty-4.1.27.Final 2018-07-11 15:37:21 +00:00
Norman Maurer
df08467d7c
Fix possible NPE introduced by a137291ad1 when using SslProvider.OPENSSL and init via files or OpenSslX509KeyManagerFactory (#8126)
Motivation:

a137291ad1 introduced a way to get the most speed out of OpenSSL by not only caching keymaterial but pre-compute these. The problem was we missed to check for null before doing an instanceof check and then a cast which could lead to a NPE as we tried to cast null to Exception and throw it.

Modifications:

Add null check and unit test.

Result:

No more NPE when keymaterial was not found for requested alias.
2018-07-11 15:19:37 +01:00
root
7bb9e7eafe [maven-release-plugin] prepare for next development iteration 2018-07-10 05:21:24 +00:00
root
8ca5421bd2 [maven-release-plugin] prepare release netty-4.1.26.Final 2018-07-10 05:18:13 +00:00
Norman Maurer
a137291ad1
Add OpenSslX509KeyManagerFactory which makes it even easier for peopl… (#8084)
* Add OpenSslX509KeyManagerFactory which makes it even easier for people to get the maximum performance when using OpenSSL / LibreSSL / BoringSSL  with netty.

Motivation:

To make it even easier for people to get the maximum performance when using native SSL we should provide our own KeyManagerFactory implementation that people can just use to configure their key material.

Modifications:

- Add OpenSslX509KeyManagerFactory which users can use for maximum performance with native SSL
- Refactor some internal code to re-use logic and not duplicate it.

Result:

Easier to get the max performance out of native SSL implementation.
2018-07-10 00:42:37 -04:00
Norman Maurer
83710cb2e1
Replace toArray(new T[size]) with toArray(new T[0]) to eliminate zero-out and allow the VM to optimize. (#8075)
Motivation:

Using toArray(new T[0]) is usually the faster aproach these days. We should use it.

See also https://shipilev.net/blog/2016/arrays-wisdom-ancients/#_conclusion.

Modifications:

Replace toArray(new T[size]) with toArray(new T[0]).

Result:

Faster code.
2018-06-29 07:56:04 +02:00
Norman Maurer
ecc238bea5
Only try to call SSL.setHostnameValidation(...) if needed. (#8074)
Motivation:

As the used OpenSSL version may not support hostname validation we should only really call SSL.setHostNameValidation(...) if we detect that its needed.

Modifications:

Only call SSL.setHostNameValidation if it was disabled before and now it needs to be enabled or if it was enabled before and it should be disabled now.

Result:

Less risk of an exception when using an OpenSSL version that does not support hostname validation.
2018-06-28 11:07:13 +02:00
Norman Maurer
2818730092
OpenSSL (and so netty-tcnative) should allow to use custom engine. (#8050)
Motivation:

OpenSSL allows to use a custom engine for its cryptographic operations. We should allow the user to make use of it if needed.

See also: https://www.openssl.org/docs/man1.0.2/crypto/engine.html.

Modifications:

Add new system property which can be used to specify the engine to use (null is the default and will use the build in default impl).

Result:

More flexible way of using OpenSSL.
2018-06-28 08:13:52 +02:00
Norman Maurer
0337ecdcc8 Allow to cache keymaterial when using OpenSSL
Motiviation:

During profiling it showed that a lot of time during the handshake is spent by parsing the key / chain over and over again. We should cache these parsed structures if possible to reduce the overhead during handshake.

Modification:

- Use new APIs provided by https://github.com/netty/netty-tcnative/pull/360.
- Introduce OpensslStaticX509KeyManagerFactory which allows to wrap another KeyManagerFactory and caches the key material provided by it.

Result:

In benchmarks handshake times have improved by 30 %.
2018-06-24 07:36:27 +02:00
Norman Maurer
3fb1b992ef
Remove some cipher protocol combos for tests due removal in more recent versions of OpenSSL (#8033)
Motivation:

Some of the cipher protocol combos that were used are no longer included in more recent OpenSSL releases.

Modifications:

Remove some combos that were used for testing.

Result:

Tests also pass in more recent OpenSSL versions (1.1.0+).
2018-06-19 08:12:02 +02:00
Roger
3e3e5155b9 Check if Log level is enabled before creating log statements (#8022)
Motivation

There is a cost to concatenating strings and calling methods that will be wasted if the Logger's level is not enabled.

Modifications

Check if Log level is enabled before producing log statement. These are just a few cases found by RegEx'ing in the code.

Result

Tiny bit more efficient code.
2018-06-13 23:21:53 -07:00
Norman Maurer
a4393831f0
Fix race in SslHandlerTest that could lead to NPE. (#7989)
Motivation:

SslHandlerTest tried to get access to the SslHandler in the pipeline via pipeline.get(...) which may return null if the channel was already closed and so the pipeline was teared down.

This showed up in a test run as:

```
-------------------------------------------------------------------------------
Test set: io.netty.handler.ssl.SslHandlerTest
-------------------------------------------------------------------------------
Tests run: 17, Failures: 0, Errors: 1, Skipped: 1, Time elapsed: 0.802 sec <<< FAILURE! - in io.netty.handler.ssl.SslHandlerTest
testCloseOnHandshakeFailure(io.netty.handler.ssl.SslHandlerTest)  Time elapsed: 0.188 sec  <<< ERROR!
java.lang.NullPointerException
        at io.netty.handler.ssl.SslHandlerTest.testCloseOnHandshakeFailure(SslHandlerTest.java:640)
        at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
        at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
        at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
        at java.base/java.lang.reflect.Method.invoke(Method.java:564)
        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.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)
        at org.junit.internal.runners.statements.FailOnTimeout$CallableStatement.call(FailOnTimeout.java:298)
        at org.junit.internal.runners.statements.FailOnTimeout$CallableStatement.call(FailOnTimeout.java:292)
        at java.base/java.util.concurrent.FutureTask.run(FutureTask.java:264)
        at java.base/java.lang.Thread.run(Thread.java:844)
```

Modifications:

Use an AtomicReference to propagate the SslHandler instance to the outer scope.

Result:

No more NPE.
2018-05-30 22:07:42 +02:00
Norman Maurer
987c443888
Use ByteBufAllocator used by the ReferenceCountedOpenSslEngine when build key-material. (#7952)
Motivation:

When we build the key-material we should use the ByteBufAllocator used by the ReferenceCountedOpenSslEngine when possible.

Modifications:

Whenever we have access to the ReferenceCountedOpenSslEngine we use its allocator.

Result:

Use correct allocator
2018-05-18 19:36:57 +02:00
Norman Maurer
7727649b2c
Add tests for the Conscrypt based SSLEngine. (#7950)
Motivation:

We currently have only interopt tests for Conscrypt, we should also have non-interopt tests.

Modifications:

Add ConscryptSslEngineTest

Result:

More tests
2018-05-18 19:36:40 +02:00
Norman Maurer
546ddd2c28
Correctly calculate and respect if we can correctly fullfil wrap for alerts. (#7945)
Motivation:

We previously did not correctly take into account when we could not wrap (and so produce) the full SSL record with an alert when the SSLEngine was closed.

There are two problems here:

- If we call wrap(...) with an empty dst buffer after closeOutbound() was called we will not notify the user if we could not store the whole SSLRecord into the dst buffer and so we may produce incomplete SSLRecords

Modifications:

Add unit test which failed before.

Result:

Correctly handle the case when the dst buffer is not big enough and and alert needs to be produced.
2018-05-16 20:08:05 +02:00
Norman Maurer
47985c11c1 Add missing parameter when delegate to SSLEngine.
Motivation:

https://github.com/netty/netty/pull/7943 had a bug which caused to not have the argument passed to the delegating method.

Modifications:

Add argument to release call.

Result:

Correctly delegate method.
2018-05-16 20:01:58 +02:00
Norman Maurer
0bce0450c0
Adjust tests to also pass when using BoringSSL (#7946)
Motivation:

Some of the tests failed when using BoringSSL as some protocol / cipher combinations are not supported and it uses a different alert when the cert is not valid yet.

Modification:

- Remove protocol / cipher combos that are not supported by BoringSSL
- Test for different alert when using BoringSSL

Result:

Not test failures when using BoringSSL.
2018-05-16 18:58:27 +02:00
Norman Maurer
932d77b83e
Verify error stack is empty after each operation when using ReferenceCountedOpenSslEngine. (#7943)
Motivation:

https://github.com/netty/netty/pull/7941 proved that its easy to not correctly clear the error stack sometimes. We should do carefully test this.

Modifications:

Add a new SSLEngine wrapper that is used during tests, which verifies that the error stack is empty after each method call.

Result:

Better testing.
2018-05-16 13:50:37 +02:00
Norman Maurer
69c644bb98
Correctly detect if protocol is enabled when using netty-tcnative (#7940)
Motivation:

We sometimes did not correctly detect when a protocol is not enabled when using netty-tcnative as we did not take into account when the option flag is 0 (as for example in BoringSSL for SSLv2).

Modifications:

- Correctly take an option flag with 0 into account.
- Add unit tests.

Result:

Fixes https://github.com/netty/netty/issues/7935.
2018-05-16 07:23:55 +02:00
Norman Maurer
ed54ab034d
Correctly clear the error stack in all cases when using ReferenceCountedOpenSslEngine. (#7941)
Motivation:

We missed to correctly clear the error stack in one case when using the ReferenceCountedOpenSslEngine. Because of this it was possible to pick up an error on an unrelated operation.

Modifications:

- Correctly clear the stack
- Add verification of empty error stack to tests.

Result:

Not possible to observe unrelated errors.
2018-05-15 19:44:02 +02:00
Norman Maurer
64bb279f47 [maven-release-plugin] prepare for next development iteration 2018-05-14 11:11:45 +00:00
Norman Maurer
c67a3b0507 [maven-release-plugin] prepare release netty-4.1.25.Final 2018-05-14 11:11:24 +00:00
Norman Maurer
54c3de0f8a
Add test to validate SSLEngine does not zero out src buffer on wrap. (#7914)
Motivation:

We had a bug-report that claimed the src buffer used by OpenSslEngine will be zero out.

Modifications:

Add testcase to ensure correct behaviour

Result:

Testcase for https://github.com/netty/netty/issues/7753
2018-05-07 20:10:06 +02:00
Norman Maurer
358249e5c9
Allow to disable native transport and native ssl support via system property. (#7903)
Motivation:

Sometimes it's useful to disable native transports / native ssl to debug a problem. We should allow to do so with a system property so people not need to adjust code for this.

Modifications:

Add system properties which allow to disable native transport and native ssl.

Result:

Easier to disable native code usage without code changes.
2018-05-04 14:44:44 +02:00
Norman Maurer
b75f44db9a [maven-release-plugin] prepare for next development iteration 2018-04-19 11:56:07 +00:00
Norman Maurer
04fac00c8c [maven-release-plugin] prepare release netty-4.1.24.Final 2018-04-19 11:55:47 +00:00
Nikolay Fedorovskikh
401b196623 Extract common parts from if statements (#7831)
Motivation:
Some `if` statements contains common parts that can be extracted.

Modifications:
Extract common parts from `if` statements.

Result:
Less code and bytecode. The code is simpler and more clear.
2018-04-11 14:36:56 +02:00
root
0a61f055f5 [maven-release-plugin] prepare for next development iteration 2018-04-04 10:44:46 +00:00
root
8c549bad38 [maven-release-plugin] prepare release netty-4.1.23.Final 2018-04-04 10:44:15 +00:00
Nicolae Mihalache
8d78893a76 Avoid writing two times the same message in case channelWritabilityChanged event is called during a write.
Motivation:

ChunkedWriteHandler.doFlush is called twice from the same write if the channelWritabilityChanged event is invoked during the write. The buffer is already written so no extra data is sent on the socket but it causes the "promise already done" exception to be thrown.
This error happens only when the message is not ChunkedInput.

Modification:
Clear out the currentWrite reference before the ctx.write call, such that next time when the method is invoked the same object is not used twice.

Result:

Fixes #7819
2018-03-30 19:32:08 +02:00
Norman Maurer
2e92a2f5cd Ensure we not schedule multiple timeouts for close notify
Motivation:

We should only schedule one timeout to wait for the close notify to be done.

Modifications:

Keep track of if we already scheduled a timeout for close notify and if so not schedule another one.

Result:

No duplicated timeouts.
2018-03-27 09:43:46 +02:00
Norman Maurer
40af10b782 Skip NPN tests when libressl 2.6.1+ is used.
Motivation:

LibreSSL removed support for NPN in its 2.6.1+ releases.

Modifications:

Skip NPN tests in libressl 2.6.1+

Result:

Be able to run netty tests against libressl 2.6.1+ as well.
2018-03-22 08:30:49 +01:00
Norman Maurer
352e36a179 Remove code duplication in ChunkedWriteHandler
Motivation:

We had some code duplication in ChunkedWriteHandler.

Modifications:

Factor out duplicated code into private methods and reuse it.

Result:

Less code duplication.
2018-03-19 09:14:07 +01:00
Norman Maurer
d1055e0665 Add testcase for c11b23bbc1
Motivation:

c11b23bbc1 added a fix for closing the SSLEngine otbound but no test was provided.

Modifications:

Add testcase.

Result:

More tests.
2018-03-06 14:33:54 +09:00
Carl Mastrangelo
c11b23bbc1 Close SSLEngine when connection fails.
Motivation:
When using the JdkSslEngine, the ALPN class is used keep a reference
to the engine.   In the event that the TCP connection fails, the
SSLEngine is not removed from the map, creating a memory leak.

Modification:
Always close the SSLEngine regardless of if the channel became
active.  Also, record the SSLEngine was closed in all places.

Result:
Fixes: https://github.com/grpc/grpc-java/issues/3080
2018-03-04 06:55:51 -08:00
Norman Maurer
bf8cac4939 Workaround SSLEngine.unwrap(...) bug in Android 5.0
Motivation:

Android 5.0 sometimes not correctly update the bytesConsumed of the SSLEngineResult when consuming data from the input ByteBuffer. This will lead to handshake failures.

Modifications:

Add a workaround for Android 5.0

Result:

Be able to use netty on Android 5.0 by fixing https://github.com/netty/netty/issues/7758 .
2018-03-03 15:01:39 -08:00
Norman Maurer
69582c0b6c [maven-release-plugin] prepare for next development iteration 2018-02-21 12:52:33 +00:00
Norman Maurer
786f35c6c9 [maven-release-plugin] prepare release netty-4.1.22.Final 2018-02-21 12:52:19 +00:00
Norman Maurer
268b901844 SSL connection not closed properly after handshake failure
Motivation:

When SSL handshake fails, the connection should be closed. This is not true anymore after 978a46c.

Modifications:

- Ensure we always flush and close the channel on handshake failure.
- Add testcase.

Result:

Fixes [#7724].
2018-02-16 08:25:47 -08:00
Carl Mastrangelo
e00f24961a Get memory address from Unsafe for OpenSSL
Motivation:
Profiling tcnative SSL code showed a non trivial percentage (1%)
of time spent in JNI code for InstaceOf.   This turned out to be
from `Buffer.address` which makes a JNI call, which safely checks
on each call that The ByteBuffer is direct.

Modification:
Prefer using the address field of the pojo rather than looking it
up with JNI.  This is the same approach taken by the `OpenSsl`
class.

Result:
Less JNI overhead
2018-02-16 07:57:43 +01:00
Eric Anderson
8b273983f0 Load Conscrypt method via reflection only once
Motivation:

The code did reflection every method call which made the code slower and
harder to read with additional cases to consider.

Modifications:

Instead of loading the method and then throwing it away, save the Method
reference instead of the Class reference. Then also use more precise
exception handling for the method invocation.

Result:

Simpler, speedier code.
2018-02-09 21:13:42 +01:00
Stephane Landelle
40ae4fefc7 Introduce an alternative IdentityCipherSuiteFilter that defaults to supportedCiphers, close #7655
Motivation:

Sometimes, it would be convenient to be able to easily enable all
supported cipher suites, regardless of security.

Currently, the only way it to retrieve all supported ciphers and pass
them explicitly.

Modification:

Introduce a new IdentityCipherSuiteFilter singleton that defaults to
supportedCiphers instead of defaultCiphers when ciphers are null.

Result:

Convenient way to enabled all supported cipher suites.
2018-02-07 13:37:25 +01:00
Norman Maurer
e71fa1e7b6 [maven-release-plugin] prepare for next development iteration 2018-02-05 12:02:35 +00:00
Norman Maurer
41ebb5fcca [maven-release-plugin] prepare release netty-4.1.21.Final 2018-02-05 12:02:19 +00:00
Eric Anderson
2923f33530 Adapt to API changes in Conscrypt 1.0.0.RC11
Motivation:

In google/conscrypt#313 the Conscrypt.Engines class was removed in favor
of methods directly on Conscrypt and overloading. The Conscrypt-using
code in Netty used reflection to access the old API, that doesn't exist
anymore. And thus recent versions of Conscrypt fail to enable things
like ALPN with Netty.

Modifications:

Instead of calling Conscrypt.Engines.isConscrypt, call
Conscrypt.isConscrypt.

Result:

Conscrypt detected properly at runtime.
2018-02-02 07:25:59 +01:00
Scott Mitchell
3f3d309a28
JdkSslContext supported cipher suites incorrect
Motivation:
JdkSslContext builds the list of supported cipher suites, but assumes that ciphers prefixed with SSL_ and TLS_ will be interchangeable. However this is not the case and only applies to a small subset of ciphers. This results in the JdkSslContext attempting to use unsupported ciphers.

Modifications:
- When building the list of ciphers in JdkSslContext we should first check if the engine supports the TLS_ prefix cipher.

Result:
Fixes https://github.com/netty/netty/issues/7673
2018-02-01 09:34:44 -08:00
Scott Mitchell
978a46cc0a SslHandler unwrap out of order promise/event notificaiton
Motivation:
SslHandler#decode methods catch any exceptions and attempt to wrap
before shutting down the engine. The intention is to write any alerts
which the engine may have pending. However the wrap process may also
attempt to write user data, and may also complete the associated
promises. If this is the case, and a promise listener closes the channel
then SslHandler may later propagate a SslHandshakeCompletionEvent user
event through the pipeline. Since the channel has already been closed
the user may no longer be paying attention to user events.

Modifications:
- Sslhandler#decode should first fail the associated handshake promise
and propagate the SslHandshakeCompletionEvent before attempting to wrap

Result:
Fixes https://github.com/netty/netty/issues/7639
2018-02-01 09:25:40 -08:00
Norman Maurer
ea58dc7ac7 [maven-release-plugin] prepare for next development iteration 2018-01-21 12:53:51 +00:00
Norman Maurer
96c7132dee [maven-release-plugin] prepare release netty-4.1.20.Final 2018-01-21 12:53:34 +00:00
Dmitriy Dumanskiy
e6c9ac968d Cleanup: replaced deprecated ctx.attr() and ctx.hasAttr() methods usage with ch.attr() and ch.hasAttr().
Motivation:

Will allow easy removal of deprecated methods in future.

Modification:

Replaced ctx.attr(), ctx.hasAttr() with ctx.channel().attr(), ctx.channel().hasAttr().

Result:

No deprecated ctx.attr(), ctx.hasAttr() methods usage.
2018-01-18 15:00:41 +00:00
Scott Mitchell
062d926912 Remove remote initiated renegotiation support
Motivation:
We recently removed support for renegotiation, but there are still some hooks to attempt to allow remote initiated renegotiation to succeed. The remote initated renegotiation can be even more problematic from a security stand point and should also be removed.

Modifications:
- Remove state related to remote iniated renegotiation from OpenSslEngine

Result:
More renegotiation code removed from the OpenSslEngine code path.
2018-01-15 10:16:08 +01:00
Norman Maurer
ab9f0a0fda Remove direct usage of JKS and SunX509
Motivation:

When using netty on android or with for example a IBM JVM it may not be able to build a SslContext as we hardcoded the use of JKS and SunX509 (which both may not be present).

Modifications:

- Use the default algorithm / type which can be override via a System property
- Remove System property check as its redundant with KeyManagerFactory.getDefaultAlgorithm()

Result:

More portable code. Fixes [#7546].
2018-01-03 18:32:18 -08:00
Chris West
e24e06bfcb OpenSslEngine: Remove renegotiation support
Motivation:

SSL.setState() has gone from openssl 1.1. Calling it is, and probably
always has been, incorrect. Doing renogitation in this manner is
potentially insecure. There have been at least two insecure
renegotiation vulnerabilities in users of the OpenSSL library.

Renegotiation is not necessary for correct operation of the TLS protocol.

BoringSSL has already eliminated this functionality, and the tests
(now deleted) were not running on BoringSSL.

Modifications:

If the connection setup has completed, always return that
negotiation is not supported. Previously this was done only if we were
the client.

Remove the tests for this functionality.

Fixes #6320.
2018-01-02 13:00:11 -05:00
Norman Maurer
264a5daa41 [maven-release-plugin] prepare for next development iteration 2017-12-15 13:10:54 +00:00
Norman Maurer
0786c4c8d9 [maven-release-plugin] prepare release netty-4.1.19.Final 2017-12-15 13:09:30 +00:00
Norman Maurer
b2bc6407ab [maven-release-plugin] prepare for next development iteration 2017-12-08 09:26:15 +00:00
Norman Maurer
96732f47d8 [maven-release-plugin] prepare release netty-4.1.18.Final 2017-12-08 09:25:56 +00:00
Norman Maurer
1453f8d18b Ensure we not try to call select when the AbstractSniHandler was already removed from the pipeline.
Motivation:

We tried to call `select` after we closed the channel (and so removed all the handlers from the pipeline) when we detected a non SSL record. This would cause an exception like this:

```
Caused by: java.util.NoSuchElementException: io.netty.handler.ssl.SniHandler
	at io.netty.channel.DefaultChannelPipeline.getContextOrDie(DefaultChannelPipeline.java:1098)
	at io.netty.channel.DefaultChannelPipeline.replace(DefaultChannelPipeline.java:506)
	at io.netty.handler.ssl.SniHandler.replaceHandler(SniHandler.java:133)
	at io.netty.handler.ssl.SniHandler.onLookupComplete(SniHandler.java:113)
	at io.netty.handler.ssl.AbstractSniHandler.select(AbstractSniHandler.java:225)
	at io.netty.handler.ssl.AbstractSniHandler.decode(AbstractSniHandler.java:218)
	at io.netty.handler.codec.ByteToMessageDecoder.decodeRemovalReentryProtection(ByteToMessageDecoder.java:489)
	at io.netty.handler.codec.ByteToMessageDecoder.callDecode(ByteToMessageDecoder.java:428)
	... 40 more
```

Modifications:

- Ensure we rethrow the NotSslRecordException when detecting it (and closing the channel). This will also ensure we not call `select(...)`
- Not catch `Throwable` but only `Exception`
- Add test case.

Result:

Correctly handle the case of an non SSL record.
2017-12-08 07:42:15 +01:00
louxiu
805ac002e6 FIX: force a read operation for peer instead of self (#7454)
* FIX: force a read operation for peer instead of self

Motivation:
When A is in `writeInProgress` and call self close, A should
`finishPeerRead` for B(A' peer).

Modifications:
Call `finishPeerRead` with peer in `LocalChannel#doClose`

Result:
Clear confuse of code logic

* FIX: preserves order of close after write in same event loop

Motivation:
If client and server(client's peer channel) are in same event loop, client writes data to
server in `ChannelActive`. Server receives the data and write it
back. The client's read can't be triggered becasue client's
`ChannelActive` is not finished at this point and its `readInProgress`
is false. Then server closes itself, it will also close the client's
channel. And client has no chance to receive the data.

Modifications:
1. Add a test case to demonstrate the problem
2. When `doClose` peer, we always call
`peer.eventLoop().execute()` and `registerInProgress` is not needed.
3. Remove test case
`testClosePeerInWritePromiseCompleteSameEventLoopPreservesOrder`. This
test case can't pass becasue of this commit. IMHO, I think it is OK,
becasue it is reasonable that the client flushes the data to socket,
then server close the channel without received the data.
4. For mismatch test in SniClientTest, the client should receive server's alert before closed(caused by server's close)

Result:
The problem is gone.
2017-12-07 17:05:57 -08:00
Norman Maurer
aabb73a9d2 Add SniCompletionEvent which allows to easily retrieve the hostname that was used to select the SslContext.
Motivation:

At the moment its a bit "hacky" to retrieve the hostname that was used during SNI as you need to hold a reference to SniHandler and then call hostname() once the selection is done. It would be better to fire an event to let the user know we did the selection.

Modifications:

Add a SniCompletionEvent that can be used to get the hostname that was used to do the selection and was included in the SNI extension.

Result:

Easier usage of SNI.
2017-12-06 14:09:11 +01:00
Norman Maurer
ca1e1fcddf Only try to match SSLException message when debug logging is enabled.
Motivation:

We only want to log for the particular case when debug logging is enabled so we not need to try to match the message if this is not the case.

Modifications:

Guard with logger.isDebugEnabled()

Result:

Less overhead when debug logging is not enabled.
2017-12-05 20:57:08 +01:00
Scott Mitchell
d7c977dd71 SslHandler aggregation prefer copy over CompositeByteBuf
Motivation:
SslHandler will do aggregation of writes by default in an attempt to improve goodput and reduce the number of discrete buffers which must be accumulated. However if aggregation is not possible then a CompositeByteBuf is used to accumulate multiple buffers. Using a CompositeByteBuf doesn't provide any of the benefits of better goodput and in the case of small + large writes (e.g. http/2 frame header + data) this can reduce the amount of data that can be passed to writev by about half. This has the impact of increasing latency as well as reducing goodput.

Modifications:
- SslHandler should prefer copying instead of using a CompositeByteBuf

Result:
Better goodput (and potentially improved latency) at the cost of copy operations.
2017-12-04 11:02:33 -08:00
Silvio Giebl
3c8f4b81d7 Fixed default OpenSsl cipher suites
Motivation:

The default enabled cipher suites of the OpenSsl engine are not set to
SslUtils#DEFAULT_CIPHER_SUITES. Instead all available cipher suites are
enabled. This should happen only as a fallback.

Modifications:

Moved the line in the static initializer in OpenSsl which adds the
SslUtils#DEFAULT_CIPHER_SUITES to the default enabled cipher suites up
before the fallback.

Result:

The default enabled cipher suites of the OpenSsl engine are set to the
available ones of the SslUtils#DEFAULT_CIPHER_SUITES.
The default enabled cipher suites of the OpenSsl engine are only set to
all available cipher suites if no one of the
SslUtils#DEFAULT_CIPHER_SUITES is supported.
2017-11-29 12:12:43 +01:00
Tomasz Jędrzejewski
e8540c2b7a Adding stable JDK9 module names that follow reverse-DNS style
Automatic-Module-Name entry provides a stable JDK9 module name, when Netty is used in a modular JDK9 applications. More info: http://blog.joda.org/2017/05/java-se-9-jpms-automatic-modules.html

When Netty migrates to JDK9 in the future, the entry can be replaced by actual module-info descriptor.

Modification:

The POM-s are configured to put the correct module names to the manifest.

Result:

Fixes #7218.
2017-11-29 11:50:24 +01:00
Norman Maurer
c921742a42 Dont fire an SslHandshakeEvent if the handshake was not started at all.
Motivation:

We should not fire a SslHandshakeEvent if the channel is closed but the handshake was not started.

Modifications:

- Add a variable to SslHandler which tracks if an handshake was started yet or not and depending on this fire the event.
- Add a unit test

Result:

Fixes [#7262].
2017-11-16 19:57:04 +01:00
Scott Mitchell
3648ab0355 Add comments for ApplicationProtocolConfig
Motivation:
The behavior for SelectorFailureBehavior and SelectedListenerFailureBehavior enum values are not clear. Additional comments would clarify the expected behavior.

Modifications:
- Add comments for each value in SelectedListenerFailureBehavior and SelectorFailureBehavior which clarify the expected behavior

Result:
The behavior of SelectedListenerFailureBehavior and SelectorFailureBehavior are more clearly communicated.
2017-11-15 08:01:47 +01:00
Norman Maurer
2adb8bd80f Use Parameterized to run SslHandler tests with different SslProviders.
Motivation:

At the moment use loops to run SslHandler tests with different SslProviders which is error-prone and also make it difficult to understand with which provider these failed.

Modifications:

- Move unit tests that should run with multiple SslProviders to extra class.
- Use junit Parameterized to run with different SslProvider combinations

Result:

Easier to understand which SslProvider produced test failures
2017-11-10 07:20:35 -08:00
Norman Maurer
188ea59c9d [maven-release-plugin] prepare for next development iteration 2017-11-08 22:36:53 +00:00
Norman Maurer
812354cf1f [maven-release-plugin] prepare release netty-4.1.17.Final 2017-11-08 22:36:33 +00:00
Scott Mitchell
8c5eeb581e SslHandler promise completion incorrect if write doesn't immediately
complete

Motivation:
SslHandler removes a Buffer/Promise pair from
AbstractCoalescingBufferQueue when wrapping data. However it is possible
the SSLEngine will not consume the entire buffer. In this case
SslHandler adds the Buffer back to the queue, but doesn't add the
Promise back to the queue. This may result in the promise completing
immediately in finishFlush, and generally not correlating to the
completion of writing the corresponding Buffer

Modifications:
- AbstractCoalescingBufferQueue#addFirst should also support adding the
ChannelPromise
- In the event of a handshake timeout we should immediately fail pending
writes immediately to get a more accurate exception

Result:
Fixes https://github.com/netty/netty/issues/7378.
2017-11-07 09:24:40 -08:00
Scott Mitchell
570d96d8c2 SslHandler leak
Motivation:
SslHandler only supports ByteBuf objects, but will not release objects of other types. SslHandler will also not release objects if its internal state is not correctly setup.

Modifications:
- Release non-ByteBuf objects in write
- Release all objects if the SslHandler queue is not setup

Result:
Less leaks in SslHandler.
2017-11-06 15:41:42 -08:00
Scott Mitchell
35b0cd58fb HTTP/2 write of released buffer should not write and should fail the promise
Motivation:
HTTP/2 allows writes of 0 length data frames. However in some cases EMPTY_BUFFER is used instead of the actual buffer that was written. This may mask writes of released buffers or otherwise invalid buffer objects. It is also possible that if the buffer is invalid AbstractCoalescingBufferQueue will not release the aggregated buffer nor fail the associated promise.

Modifications:
- DefaultHttp2FrameCodec should take care to fail the promise, even if releasing the data throws
- AbstractCoalescingBufferQueue should release any aggregated data and fail the associated promise if something goes wrong during aggregation

Result:
More correct handling of invalid buffers in HTTP/2 code.
2017-11-06 14:38:58 -08:00
Scott Mitchell
fbe0e3506e OpenSslEngine support unwrap plaintext greater than 2^14 and avoid
infinite loop

Motivation:
If SslHandler sets jdkCompatibilityMode to false and ReferenceCountedOpenSslEngine sets jdkCompatibilityMode to true there is a chance we will get stuck in an infinite loop if the peer sends a TLS packet with length greater than 2^14 (the maximum length allowed in the TLS 1.2 RFC [1]). However there are legacy implementations which actually send larger TLS payloads than 2^14 (e.g. OpenJDK's SSLSessionImpl [2]) and in this case ReferenceCountedOpenSslEngine will return BUFFER_OVERFLOW in an attempt to notify that a larger buffer is to be used, but if the buffer is already at max size this process will repeat indefinitely.

[1] https://tools.ietf.org/html/rfc5246#section-6.2.1
[2] http://hg.openjdk.java.net/jdk8u/jdk8u/jdk/file/d5a00b1e8f78/src/share/classes/sun/security/ssl/SSLSessionImpl.java#l793

Modifications:
- Support TLS payload sizes greater than 2^14 in ReferenceCountedOpenSslEngine
- ReferenceCountedOpenSslEngine should throw an exception if a
BUFFER_OVERFLOW is impossible to rectify

Result:
No more infinite loop in ReferenceCountedOpenSslEngine due to
BUFFER_OVERFLOW and large TLS payload lengths.
2017-11-02 11:42:38 -07:00
Norman Maurer
7321418eb5 Fix possible NPE in ReferenceCountedOpenSslEngine.rejectRemoteInitiatedRenegotiation()
Motivation:

ReferenceCountedOpenSslEngine.rejectRemoteInitiatedRenegotiation() is called in a finally block to ensure we always check for renegotiation. The problem here is that sometimes we will already shutdown the engine before we call the method which will lead to an NPE in this case as the ssl pointer was already destroyed.

Modifications:

Check that the engine is not destroyed yet before calling SSL.getHandshakeCount(...)

Result:

Fixes [#7353].
2017-11-02 14:28:59 +01:00
Scott Mitchell
fa584c146f SslHandler dervies jdkCompatibilityMode from SSLEngine
Motivation:
Some SSLEngine implementations (e.g. ReferenceCountedOpenSslContext) support unwrapping/wrapping multiple packets at a time. The SslHandler behaves differently if the SSLEngine supports this feature, but currently requires that the constructor argument between the SSLEngine creation and SslHandler are coordinated. This can be difficult, or require package private access, if extending the SslHandler.

Modifications:
- The SslHandler should inspect the SSLEngine to see if it supports jdkCompatibilityMode instead of relying on getting an extra constructor argument which maybe out of synch with the SSLEngine

Result:
Easier to override SslHandler and have consistent jdkCompatibilityMode between SSLEngine and SslHandler.
2017-11-01 12:00:51 -07:00
Norman Maurer
ad1f0d46b3 Take the architecture into account when loading netty-tcnative
Motivation:

We should ensure we only try to load the netty-tcnative version that was compiled for the architecture we are using.

Modifications:

Include architecture into native lib name.

Result:

Only load native lib if the architecture is supported.
2017-11-01 08:52:09 +01:00
Norman Maurer
92b786e2f3 Fix possible leak in SslHandler if wrap(...) throws.
Motivation:

We can end up with a buffer leak if SSLEngine.wrap(...) throws.

Modifications:

Correctly release the ByteBuf if SSLEngine.wrap(...) throws.

Result:

Fixes [#7337].
2017-10-24 18:52:20 +02:00
Norman Maurer
55b501d0d4 Correctly update Channel writability when queueing data in SslHandler.
Motivation:

A regression was introduced in 86e653e which had the effect that the writability was not updated for a Channel while queueing data in the SslHandler.

Modifications:

- Factor out code that will increment / decrement pending bytes and use it in AbstractCoalescingBufferQueue and PendingWriteQueue
- Add test-case

Result:

Channel writability changes are triggered again.
2017-10-24 09:13:15 +02:00
Nikolay Fedorovskikh
dcc39e5b21 Fixes a LoggingHandler#format method with two arguments
Motivation:
Bug in capacity calculation: occurs auto convert to string instead of sum up.

Modifications:
Use `eventName.length()` in sum.

Result:
Less trash in logs.
2017-10-24 06:56:54 +02:00
Norman Maurer
521e87984d SslHandler.setHandshakeTimeout*(...) should also been enforced on the server side.
Motivation:

We should also enforce the handshake timeout on the server-side to allow closing connections which will not finish the handshake in an expected amount of time.

Modifications:

- Enforce the timeout on the server and client side
- Add unit test.

Result:

Fixes [#7230].
2017-10-23 19:33:07 +02:00
Idel Pivnitskiy
50a067a8f7 Make methods 'static' where it possible
Motivation:

Even if it's a super micro-optimization (most JVM could optimize such
 cases in runtime), in theory (and according to some perf tests) it
 may help a bit. It also makes a code more clear and allows you to
 access such methods in the test scope directly, without instance of
 the class.

Modifications:

Add 'static' modifier for all methods, where it possible. Mostly in
test scope.

Result:

Cleaner code with proper 'static' modifiers.
2017-10-21 14:59:26 +02:00
Idel Pivnitskiy
558097449c Add missed 'serialVersionUID' field for Serializable classes
Motivation:

Without a 'serialVersionUID' field, any change to a class will make
previously serialized versions unreadable.

Modifications:

Add missed 'serialVersionUID' field for all Serializable
classes.

Result:

Proper deserialization of previously serialized objects.
2017-10-21 14:41:18 +02:00
Jason Tedor
3fe1f71511 Do not treat errors as decoder exception (redux)
Motivation: Today when Netty encounters a general error while decoding
it treats this as a decoder exception. However, for fatal causes this
should not be treated as such, instead the fatal error should be carried
up the stack without the callee having to unwind causes. This was
probably done for byte to byte message decoder but is now done for all
decoders.

Modifications: Instead of translating any error to a decoder exception,
we let those unwind out the stack (note that finally blocks still
execute) except in places where an event needs to fire where we fire
with the error instead of wrapping in a decoder exception.

Result: Fatal errors will not be treated as innocent decoder exceptions.
2017-10-07 18:17:00 +02:00
Norman Maurer
bb1833f22c Fix Java9SslEngine implementation of ApplicationProtocolAccessor and so fix ApplicationProtocolNegationHandler
Motivation:

Java9SslEngine did not correctly implement ApplicationProtocolAccessor and so returned an empty String when no extension was used while the interface contract is to return null. This lead to the situation that ApplicationProtocolNegationHandler did not correct work.

Modifications:

- Rename ApplicationProtocolAccessor.getApplicationProtocol() to getNegotiatedApplicationProtocol() which resolves the clash with the method exposed by Java9s SSLEngine.
- Correctly implement getNegotiatedApplicationProtocol() for Java9Sslengine
- Add delegate in Java9Sslengine.getApplicationProtocol() which is provided by Java9
- Adjust tests to test the correct behaviour.

Result:

Fixes [#7251].
2017-10-02 08:27:10 +02:00
nmittler
5a6ee27cee Upgrade Conscrypt to 1.0.0.RC11
Motivation:

Getting the latest Conscrypt goodies.

Modifications:

A few API changes have occurred, specifically in the Conscrypt
class.

Result:

Netty now builds and tests against Conscrypt 1.0.0.RC11
2017-09-26 20:15:54 +02:00
Norman Maurer
625a7426cd [maven-release-plugin] prepare for next development iteration 2017-09-25 06:12:32 +02:00
Norman Maurer
f57d8f00e1 [maven-release-plugin] prepare release netty-4.1.16.Final 2017-09-25 06:12:16 +02:00
Lionel Li
28d2560679 Update SSLEngineTest certificate validity
Motivation:
A bunch of unit tests are failing due to certificates having expired.
This has to be replaced with a newly generated certificate that has a
longer validity.

Modifications:
- generated a certificate with validity of 100 years from now

Results:
Unit tests are passing again
2017-09-20 21:47:38 -07:00
Norman Maurer
0fffc844d6 Only load native transport if running architecture match the compiled library architecture.
Motivation:

We should only try to load the native artifacts if the architecture we are currently running on is the same as the one the native libraries were compiled for.

Modifications:

Include architecture in native lib name and append the current arch when trying to load these. This will fail then if its not the same as the arch of the compiled arch.

Result:

Fixes [#7150].
2017-09-04 13:34:55 +02:00
Paul Gross
0b0de76adc Fix typo in error message
Motivation:

Fix typo in error message.

Modification:

could not fine -> could not find
2017-08-28 09:05:08 +02:00
Norman Maurer
b967805f32 [maven-release-plugin] prepare for next development iteration 2017-08-24 15:38:22 +02:00
Norman Maurer
da8e010a42 [maven-release-plugin] prepare release netty-4.1.15.Final 2017-08-24 15:37:59 +02:00
Norman Maurer
1065e0f26e Support JDK9-native ALPN
Motivation:

Netty is unable to use Java9s ALPN support atm.

Modifications:

When running on Java9+ we invoke the correct methods that are exposed on the Java9+ implementation of SSLEngine and so be able to support ALPN.
This patch is based on the work of @rschmitt and so https://github.com/netty/netty/pull/6992.

Result:

Fixes #6933.
2017-08-24 08:16:51 +02:00
Scott Mitchell
c93e58c453 OpenSsl should use _ instead of -
Motivation:
netty-tcnative recently change the name of the native libraries from using - to _.

Modifications:
- OpenSsl should use _ instead of - even for the classifiers to be consistent with netty-tcnative

Result:
Loading netty-tcnative works.
2017-08-23 22:40:47 -07:00
Norman Maurer
6e859469ca Deprecate ApplicationProtocolNegotiator and its implementation as people should use ApplicationProtocolConfig
Motivation:

We should deprecate ApplicationProtocolNegotiator as the users should use ApplicationProtocolConfig these days.

Modifications:

Add deprecation annotations and javadocs.

Result:

Be able to make package-private in next major release.
2017-08-23 20:18:58 +02:00
Scott Mitchell
cbce95eae1 SslHandlerTest#testCompositeBufSizeEstimationGuaranteesSynchronousWrite print SslProvider on failure
Motivation:
When SslHandlerTest#testCompositeBufSizeEstimationGuaranteesSynchronousWrite fails it would be useful to know the SslProvider type

Modifications:
- Print the sever and client SslProvider upon failure
- Increase test timeout to 8 minutes to allow more time to run

Result:
Failures include more info to help diagnose issues.
2017-08-22 22:20:54 -07:00
Norman Maurer
2beb5fc8ee DelegatingSslContext should also be able to configure the SslHandler
Motivation:

DelegatingSslContext at the moment intercept newEngine calls and allow to init the SslEngine after it is created. The problem here is that this may not work the SSLEngine that is wrapped in the SslHandler when calling newHandler(...). This is because some SslContext implementations not delegate to newEngine(...) when creating the SslHandler to allow some optimizations. For this we should also allow to init the SslHandler after its creation and by default just delegate to initEngine(...).

Modifications:

Allow the user to also init the SslHandler after creation while by default init its SSLEngine after creation.

Result:

More flexible and correct code.
2017-08-21 20:13:48 +02:00
Norman Maurer
123e07ca80 Revert "Only call ctx.fireChannelReadComplete() if ByteToMessageDecoder decoded at least one message."
This reverts commit d63bb4811e as this not covered correctly all cases and so could lead to missing fireChannelReadComplete() calls. We will re-evalute d63bb4811e and resbumit a pr once we are sure all is handled correctly
2017-08-18 09:06:37 +02:00
Norman Maurer
5de38051c9 Update netty-tcnative native library names to use underscores.
Motivation:

We recently changed netty-tcnative to use underscores in its native library names.

Modifications:

Update code to use underscores when loading native library.

Result:

More consistent code.
2017-08-17 10:08:32 +02:00
Scott Mitchell
053e6184f2 Increase visibility for SslHandlerTest#testCompositeBufSizeEstimationGuaranteesSynchronousWrite
Motivation:
SslHandlerTest#testCompositeBufSizeEstimationGuaranteesSynchronousWrite has been observed to fail on CI servers, but it is not clear why.

Modifications:
- Add more visibility into what the state was and what the condition that caused the failure was.

Result:
More visibility when the test fails.
2017-08-16 09:53:00 -07:00
Norman Maurer
5d9a5d3e8d Revert SslEngineWrapperFactory api breakage introduced by 4448b8f42f.
Motivation:

Commit 4448b8f42f introduced some API breakage which we need to revert before we release.

Modifications:

- Introduce an AllocatorAwareSslEngineWrapperFactory which expose an extra method that takes a ByteBufAllocator as well.
- Revert API changes to SslEngineWrapperFactory.

Result:

API breakage reverted.
2017-08-16 08:33:46 +02:00
Norman Maurer
d63bb4811e Only call ctx.fireChannelReadComplete() if ByteToMessageDecoder decoded at least one message.
Motivation:

Its wasteful and also confusing that channelReadComplete() is called even if there was no message forwarded to the next handler.

Modifications:

- Only call ctx.fireChannelReadComplete() if at least one message was decoded
- Add unit test

Result:

Less confusing behavior. Fixes [#4312].
2017-08-04 10:54:56 +02:00
Norman Maurer
32f497760f Ensure no java.lang.UnsupportedClassVersionError are thrown if running on Java7 and try to check if conscrypt is available.
Motivation:

We need to ensure we not try to load any conscrypt classes directly (which means without using reflection) in the same class that is used to check if conscrypt is available. This is needed as otherwise we will have the following problem when try to use netty on java7:

java.lang.UnsupportedClassVersionError: org/conscrypt/BufferAllocator : Unsupported major.minor version 52.0
	at io.netty.handler.ssl.ConscryptJdkSslEngineInteropTest.checkConscrypt(ConscryptJdkSslEngineInteropTest.java:49)

This regression was introduced by 4448b8f42f and detected on the CI when using:

mvn clean package -DtestJavaHome=$JAVA7_HOME

Modifications:

Move the detection code in an extra class and use it.

Result:

Works correctly also when using Java7.
2017-08-04 10:48:50 +02:00
Nathan Mittler
4448b8f42f Upgrading to Conscrypt 1.0.0.RC9. (#7044)
Motivation:

Starting with 1.0.0.RC9, conscrypt supports a buffer allocator.

Modifications:

- Updated the creation process for the engine to pass through the
ByteBufAllocator.
- Wrap a ByteBufAllocator with an adapter for conscrypt.
- Added a property to optionally control whether conscrypt uses
Netty's buffer allocator.

Result:

Netty+conscrypt will support using Netty's ByteBufAllocator.
2017-08-03 14:21:32 -07:00
Norman Maurer
52f384b37f [maven-release-plugin] prepare for next development iteration 2017-08-02 12:55:10 +00:00
Norman Maurer
8cc1071881 [maven-release-plugin] prepare release netty-4.1.14.Final 2017-08-02 12:54:51 +00:00
Norman Maurer
d9d3d65716 Add comment why the ResourceLeak creation is happening as last in the constructor. Followup of c5b5d36360 2017-07-30 06:55:22 +02:00
Norman Maurer
c5b5d36360 Fix false-positive leak detection report when ReferenceCountedOpenSslEngine constructor throws.
Motivation:

We need to ensure we only create the ResourceLeak when the constructor not throws.

Modifications:

Ensure ResourceLeakDetector.track(...) is only called if the constructor of ReferenceCoundedOpenSslEngine not throws.

Result:

No more false-positves.
2017-07-29 22:01:56 +02:00
Scott Mitchell
452fd36240 ByteBufs which are not resizable should not throw in ensureWritable(int,boolean)
Motivation:
ByteBuf#ensureWritable(int,boolean) returns an int indicating the status of the resize operation. For buffers that are unmodifiable or cannot be resized this method shouldn't throw but just return 1.
ByteBuf#ensureWriteable(int) should throw unmodifiable buffers.

Modifications:
- ReadOnlyByteBuf should be updated as described above.
- Add a unit test to SslHandler which verifies the read only buffer can be tolerated in the aggregation algorithm.

Result:
Fixes https://github.com/netty/netty/issues/7002.
2017-07-22 08:44:48 -07:00
Eric Anderson
2fbce6d470 Delete temporary self-signed certs in SSLEngineTest-based tests
Motivation:

Lots of usages of SelfSignedCertificates were not deleting the certs at
the end of the test. This includes setupHandlers() which is used by
extending classes. Although these files will be deleted at JVM exit and
deleting them early does not free the JVM from trying to delete them at
shutdown, it's good practice to delete eagerly and since users sometimes
use tests as a form of documentation, it'd be good for them to see the
explicit deletes.

Modifications:

Add missing delete() calls to ½ of the SelfSignedCertificates-using
tests.

Result:

Tests that more clearly communicates which resources are created and
may accumulate without early delete.
2017-07-22 08:13:57 +02:00
Eric Anderson
8a25c35939 Filter user-provided ciphers using RFC cipher names
Motivation:

Previously filterCipherSuites was being passed the OpenSSL-formatted
cipher names. Commit 43ae974 introduced a regression as it swapped to the
RFC/JDK format, except that user-provided ciphers were not converted and
remained in the OpenSSL format.

This mis-match would cause all user-provided to be thrown away, leading
to failure trying to set zero ciphers:
Exception in thread "main" javax.net.ssl.SSLException: failed to set cipher suite: []
	at io.netty.handler.ssl.ReferenceCountedOpenSslContext.<init>(ReferenceCountedOpenSslContext.java:299)
	at io.netty.handler.ssl.OpenSslContext.<init>(OpenSslContext.java:43)
	at io.netty.handler.ssl.OpenSslServerContext.<init>(OpenSslServerContext.java:347)
	at io.netty.handler.ssl.OpenSslServerContext.<init>(OpenSslServerContext.java:335)
	at io.netty.handler.ssl.SslContext.newServerContextInternal(SslContext.java:421)
	at io.netty.handler.ssl.SslContextBuilder.build(SslContextBuilder.java:441)
Caused by: java.lang.Exception: Unable to configure permitted SSL ciphers (error:100000b1:SSL routines:OPENSSL_internal:NO_CIPHER_MATCH)
	at io.netty.internal.tcnative.SSLContext.setCipherSuite(Native Method)
	at io.netty.handler.ssl.ReferenceCountedOpenSslContext.<init>(ReferenceCountedOpenSslContext.java:295)
	... 7 more

Modifications:

Remove the reformatting of user-provided ciphers, as they are already in
the RFC/JDK format.

Result:

No regression, and the internals stay sane using the RFC/JDK format.
2017-07-21 19:20:07 -07:00
Norman Maurer
ef22e65b57 Allow to delay registration when creating a EmbeddedChannel
Motivation:

Some ChannelOptions must be set before the Channel is really registered to have the desired effect.

Modifications:

Add another constructor argument which allows to not register the EmbeddedChannel to its EventLoop until the user calls register().

Result:

More flexible usage of EmbeddedChannel. Also Fixes [#6968].
2017-07-19 19:35:03 +02:00
Norman Maurer
64a3e6c69c SSLEngineTest should not depend on OpenSsl* class.
Motivation:

6152990073 introduced a test-case in SSLEngineTest which used OpenSsl.* which should not be done as this is am abstract bass class that is also used for non OpenSsl tests.

Modifications:

Move the protocol definations into SslUtils.

Result:

Cleaner code.
2017-07-18 13:21:27 +02:00
Norman Maurer
d4b9f3e4aa Use array initializer expression
Motivation:

Code introduced in 6152990073 can be cleaned up and use array initializer expressions.

Modifications:

Use array initializer expressions.

Result:

Cleaner code.
2017-07-18 07:22:41 +02:00
Scott Mitchell
6152990073 OpenSslEngine protocol selection must be contiguous
Motivation:
TLS doesn't support a way to advertise non-contiguous versions from the client's perspective, and the client just advertises the max supported version. The TLS protocol also doesn't support all different combinations of discrete protocols, and instead assumes contiguous ranges. OpenSSL has some unexpected behavior (e.g. handshake failures) if non-contiguous protocols are used even where there is a compatible set of protocols and ciphers. For these reasons this method will determine the minimum protocol and the maximum protocol and enabled a contiguous range from [min protocol, max protocol] in OpenSSL.

Modifications:
- ReferenceCountedOpenSslEngine#setEnabledProtocols should determine the min/max protocol versions and enable a contiguous range

Result:
OpenSslEngine is more consistent with the JDK's SslEngineImpl and no more unexpected handshake failures due to protocol selection quirks.
2017-07-13 08:17:38 -07:00
Scott Mitchell
43ae9748d0 Unify default cipher suites betweek JDK and OpenSSL
Motivation:
Currently the default cipher suites are set independently between JDK and OpenSSL. We should use a common approach to setting the default ciphers. Also the OpenSsl default ciphers are expressed in terms of the OpenSSL cipher name conventions, which is not correct and may be exposed to the end user. OpenSSL should also use the RFC cipher names like the JDK defaults.

Modifications:
- Move the default cipher definition to a common location and use it in JDK and OpenSSL initialization
- OpenSSL should not expose OpenSSL cipher names externally

Result:
Common initialization and OpenSSL doesn't expose custom cipher names.
2017-07-12 18:11:56 -07:00
Carl Mastrangelo
e5455d31b3 Fix Race in ReferenceCountedOpenSslEngine
Motivation:
ReferenceCountedOpenSslEngine is careful to lock access to `ssl`
almost everywhere (manually verified) *except* in the constructor.
Since `ssl` is non-final, it does not enjoy automatic thread safety
of the code that uses it.  Specifically, that means netty tcnative
code is not thread safe.

Modifications:

Ensure that all ssl engine intialization and variables related to
it are properly synchronized  by adding in the constructor.

Result:
Less noisy race detector.

Notes:
The specific racing threads are:
```
  Read of size 8 at 0x7b5400019ff8 by thread T52 (mutexes: write M215300):
    #0 ssl_do_info_callback .../src/ssl/ssl_lib.c:2602:24 (f077793ecd812aeebb37296c987f655c+0x23c6834)
    #1 ssl_process_alert .../src/ssl/tls_record.c:473:3 (f077793ecd812aeebb37296c987f655c+0x23a5346)
    #2 tls_open_record .../src/ssl/tls_record.c:338:12 (f077793ecd812aeebb37296c987f655c+0x23a5289)
    #3 ssl3_get_record .../src/ssl/s3_pkt.c:146:7 (f077793ecd812aeebb37296c987f655c+0x23a3da0)
    #4 ssl3_read_app_data .../src/ssl/s3_pkt.c:388:17 (f077793ecd812aeebb37296c987f655c+0x23a368f)
    #5 ssl_read_impl .../src/ssl/ssl_lib.c:722:15 (f077793ecd812aeebb37296c987f655c+0x23c0895)
    #6 SSL_read .../src/ssl/ssl_lib.c:743:10 (f077793ecd812aeebb37296c987f655c+0x23c075b)
    #7 netty_internal_tcnative_SSL_readFromSSL .../netty_tcnative/openssl-dynamic/src/main/c/ssl.c:946:12 (f077793ecd812aeebb37296c987f655c+0x23827f7)
    #8 <null> <null> (0x7fc0760193be)
    #9 io.netty.handler.ssl.ReferenceCountedOpenSslEngine.readPlaintextData(Ljava/nio/ByteBuffer;)I (ReferenceCountedOpenSslEngine.java:449)
    #10 io.netty.handler.ssl.ReferenceCountedOpenSslEngine.unwrap([Ljava/nio/ByteBuffer;II[Ljava/nio/ByteBuffer;II)Ljavax/net/ssl/SSLEngineResult; (ReferenceCountedOpenSslEngine.java:882)
    #11 io.netty.handler.ssl.ReferenceCountedOpenSslEngine.unwrap([Ljava/nio/ByteBuffer;[Ljava/nio/ByteBuffer;)Ljavax/net/ssl/SSLEngineResult; (ReferenceCountedOpenSslEngine.java:985)
    #12 io.netty.handler.ssl.ReferenceCountedOpenSslEngine.unwrap(Ljava/nio/ByteBuffer;Ljava/nio/ByteBuffer;)Ljavax/net/ssl/SSLEngineResult; (ReferenceCountedOpenSslEngine.java:1028)
    #13 io.netty.handler.ssl.SslHandler$SslEngineType$1.unwrap(Lio/netty/handler/ssl/SslHandler;Lio/netty/buffer/ByteBuf;IILio/netty/buffer/ByteBuf;)Ljavax/net/ssl/SSLEngineResult; (SslHandler.java:206)
    #14 io.netty.handler.ssl.SslHandler.unwrap(Lio/netty/channel/ChannelHandlerContext;Lio/netty/buffer/ByteBuf;II)Z (SslHandler.java:1162)
    #15 io.netty.handler.ssl.SslHandler.decode(Lio/netty/channel/ChannelHandlerContext;Lio/netty/buffer/ByteBuf;Ljava/util/List;)V (SslHandler.java:1084)
    #16 io.netty.handler.codec.ByteToMessageDecoder.decodeRemovalReentryProtection(Lio/netty/channel/ChannelHandlerContext;Lio/netty/buffer/ByteBuf;Ljava/util/List;)V (ByteToMessageDecoder.java:489)
    #17 io.netty.handler.codec.ByteToMessageDecoder.callDecode(Lio/netty/channel/ChannelHandlerContext;Lio/netty/buffer/ByteBuf;Ljava/util/List;)V (ByteToMessageDecoder.java:428)
    #18 io.netty.handler.codec.ByteToMessageDecoder.channelRead(Lio/netty/channel/ChannelHandlerContext;Ljava/lang/Object;)V (ByteToMessageDecoder.java:265)
    #19 io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(Ljava/lang/Object;)V (AbstractChannelHandlerContext.java:362)
    #20 io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(Lio/netty/channel/AbstractChannelHandlerContext;Ljava/lang/Object;)V (AbstractChannelHandlerContext.java:348)
    #21 io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(Ljava/lang/Object;)Lio/netty/channel/ChannelHandlerContext; (AbstractChannelHandlerContext.java:340)
    #22 io.netty.channel.DefaultChannelPipeline$HeadContext.channelRead(Lio/netty/channel/ChannelHandlerContext;Ljava/lang/Object;)V (DefaultChannelPipeline.java:1334)
    #23 io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(Ljava/lang/Object;)V (AbstractChannelHandlerContext.java:362)
    #24 io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(Lio/netty/channel/AbstractChannelHandlerContext;Ljava/lang/Object;)V (AbstractChannelHandlerContext.java:348)
    #25 io.netty.channel.DefaultChannelPipeline.fireChannelRead(Ljava/lang/Object;)Lio/netty/channel/ChannelPipeline; (DefaultChannelPipeline.java:926)
    #26 io.netty.channel.nio.AbstractNioByteChannel$NioByteUnsafe.read()V (AbstractNioByteChannel.java:134)
    #27 io.netty.channel.nio.NioEventLoop.processSelectedKey(Ljava/nio/channels/SelectionKey;Lio/netty/channel/nio/AbstractNioChannel;)V (NioEventLoop.java:644)
    #28 io.netty.channel.nio.NioEventLoop.processSelectedKeysOptimized()V (NioEventLoop.java:579)
    #29 io.netty.channel.nio.NioEventLoop.processSelectedKeys()V (NioEventLoop.java:496)
    #30 io.netty.channel.nio.NioEventLoop.run()V (NioEventLoop.java:458)
    #31 io.netty.util.concurrent.SingleThreadEventExecutor$5.run()V (SingleThreadEventExecutor.java:858)
    #32 io.netty.util.concurrent.DefaultThreadFactory$DefaultRunnableDecorator.run()V (DefaultThreadFactory.java:138)
    #33 java.lang.Thread.run()V (Thread.java:745)
    #34 (Generated Stub)

  Previous write of size 8 at 0x7b5400019ff8 by thread T97:
    #0 SSL_CTX_set_info_callback .../ssl/ssl_session.c:1136:22 (f077793ecd812aeebb37296c987f655c+0x23bd621)
    #1 netty_internal_tcnative_SSL_newSSL .../netty_tcnative/openssl-dynamic/src/main/c/ssl.c:830:5 (f077793ecd812aeebb37296c987f655c+0x2382306)
    #2 <null> <null> (0x7fc0760193be)
    #3 io.netty.handler.ssl.ReferenceCountedOpenSslEngine.<init>(Lio/netty/handler/ssl/ReferenceCountedOpenSslContext;Lio/netty/buffer/ByteBufAllocator;Ljava/lang/String;IZ)V (ReferenceCountedOpenSslEngine.java:237)
    #4 io.netty.handler.ssl.OpenSslEngine.<init>(Lio/netty/handler/ssl/OpenSslContext;Lio/netty/buffer/ByteBufAllocator;Ljava/lang/String;I)V (OpenSslEngine.java:31)
    #5 io.netty.handler.ssl.OpenSslContext.newEngine0(Lio/netty/buffer/ByteBufAllocator;Ljava/lang/String;I)Ljavax/net/ssl/SSLEngine; (OpenSslContext.java:49)
    #6 io.netty.handler.ssl.ReferenceCountedOpenSslContext.newEngine(Lio/netty/buffer/ByteBufAllocator;Ljava/lang/String;I)Ljavax/net/ssl/SSLEngine; (ReferenceCountedOpenSslContext.java:409)
    #7 io.netty.handler.ssl.ReferenceCountedOpenSslContext.newEngine(Lio/netty/buffer/ByteBufAllocator;)Ljavax/net/ssl/SSLEngine; (ReferenceCountedOpenSslContext.java:423)
    #8 io.grpc.netty.ProtocolNegotiators$ServerTlsHandler.handlerAdded(Lio/netty/channel/ChannelHandlerContext;)V (ProtocolNegotiators.java:133)
    #9 io.netty.channel.DefaultChannelPipeline.callHandlerAdded0(Lio/netty/channel/AbstractChannelHandlerContext;)V (DefaultChannelPipeline.java:597)
    #10 io.netty.channel.DefaultChannelPipeline.addLast(Lio/netty/util/concurrent/EventExecutorGroup;Ljava/lang/String;Lio/netty/channel/ChannelHandler;)Lio/netty/channel/ChannelPipeline; (DefaultChannelPipeline.java:226)
    #11 io.netty.channel.DefaultChannelPipeline.addLast(Lio/netty/util/concurrent/EventExecutorGroup;[Lio/netty/channel/ChannelHandler;)Lio/netty/channel/ChannelPipeline; (DefaultChannelPipeline.java:392)
    #12 io.netty.channel.DefaultChannelPipeline.addLast([Lio/netty/channel/ChannelHandler;)Lio/netty/channel/ChannelPipeline; (DefaultChannelPipeline.java:379)
    #13 io.grpc.netty.NettyServerTransport.start(Lio/grpc/internal/ServerTransportListener;)V (NettyServerTransport.java:99)
    #14 io.grpc.netty.NettyServer$1.initChannel(Lio/netty/channel/Channel;)V (NettyServer.java:164)
    #15 io.netty.channel.ChannelInitializer.initChannel(Lio/netty/channel/ChannelHandlerContext;)Z (ChannelInitializer.java:113)
    #16 io.netty.channel.ChannelInitializer.handlerAdded(Lio/netty/channel/ChannelHandlerContext;)V (ChannelInitializer.java:105)
    #17 io.netty.channel.DefaultChannelPipeline.callHandlerAdded0(Lio/netty/channel/AbstractChannelHandlerContext;)V (DefaultChannelPipeline.java:597)
    #18 io.netty.channel.DefaultChannelPipeline.access$000(Lio/netty/channel/DefaultChannelPipeline;Lio/netty/channel/AbstractChannelHandlerContext;)V (DefaultChannelPipeline.java:44)
    #19 io.netty.channel.DefaultChannelPipeline$PendingHandlerAddedTask.execute()V (DefaultChannelPipeline.java:1387)
    #20 io.netty.channel.DefaultChannelPipeline.callHandlerAddedForAllHandlers()V (DefaultChannelPipeline.java:1122)
    #21 io.netty.channel.DefaultChannelPipeline.invokeHandlerAddedIfNeeded()V (DefaultChannelPipeline.java:647)
    #22 io.netty.channel.AbstractChannel$AbstractUnsafe.register0(Lio/netty/channel/ChannelPromise;)V (AbstractChannel.java:506)
    #23 io.netty.channel.AbstractChannel$AbstractUnsafe.access$200(Lio/netty/channel/AbstractChannel$AbstractUnsafe;Lio/netty/channel/ChannelPromise;)V (AbstractChannel.java:419)
    #24 io.netty.channel.AbstractChannel$AbstractUnsafe$1.run()V (AbstractChannel.java:478)
    #25 io.netty.util.concurrent.AbstractEventExecutor.safeExecute(Ljava/lang/Runnable;)V (AbstractEventExecutor.java:163)
    #26 io.netty.util.concurrent.SingleThreadEventExecutor.runAllTasks(J)Z (SingleThreadEventExecutor.java:403)
    #27 io.netty.channel.nio.NioEventLoop.run()V (NioEventLoop.java:462)
    #28 io.netty.util.concurrent.SingleThreadEventExecutor$5.run()V (SingleThreadEventExecutor.java:858)
    #29 io.netty.util.concurrent.DefaultThreadFactory$DefaultRunnableDecorator.run()V (DefaultThreadFactory.java:138)
    #30 java.lang.Thread.run()V (Thread.java:745)
    #31 (Generated Stub)

```
2017-07-12 09:52:57 -07:00
Scott Mitchell
74140dbf53 Correct merge error from f7b3caeddc 2017-07-11 18:15:25 -07:00
Scott Mitchell
81fb2eede8 Revert "Revert "SslHandler avoid calling wrap/unwrap when unnecessary""
Motivation:
PR https://github.com/netty/netty/pull/6803 corrected an error in the return status of the OpenSslEngine. We should now be able to restore the SslHandler optimization.

Modifications:
- This reverts commit 7f3b75a509.

Result:
SslHandler optimization is restored.
2017-07-10 12:38:03 -07:00
Scott Mitchell
24263c2bd8 Fix merge issue from 86e653e04f 2017-07-10 12:31:16 -07:00
Scott Mitchell
86e653e04f SslHandler aggregation of plaintext data on write
Motivation:
Each call to SSL_write may introduce about ~100 bytes of overhead. The OpenSslEngine (based upon OpenSSL) is not able to do gathering writes so this means each wrap operation will incur the ~100 byte overhead. This commit attempts to increase goodput by aggregating the plaintext in chunks of <a href="https://tools.ietf.org/html/rfc5246#section-6.2">2^14</a>. If many small chunks are written this can increase goodput, decrease the amount of calls to SSL_write, and decrease overall encryption operations.

Modifications:
- Introduce SslHandlerCoalescingBufferQueue in SslHandler which will aggregate up to 2^14 chunks of plaintext by default
- Introduce SslHandler#setWrapDataSize to control how much data should be aggregated for each write. Aggregation can be disabled by setting this value to <= 0.

Result:
Better goodput when using SslHandler and the OpenSslEngine.
2017-07-10 12:22:08 -07:00
Scott Mitchell
f7b3caeddc OpenSslEngine option to wrap/unwrap multiple packets per call
Motivation:
The JDK SSLEngine documentation says that a call to wrap/unwrap "will attempt to consume one complete SSL/TLS network packet" [1]. This limitation can result in thrashing in the pipeline to decode and encode data that may be spread amongst multiple SSL/TLS network packets.
ReferenceCountedOpenSslEngine also does not correct account for the overhead introduced by each individual SSL_write call if there are multiple ByteBuffers passed to the wrap() method.

Modifications:
- OpenSslEngine and SslHandler supports a mode to not comply with the limitation to only deal with a single SSL/TLS network packet per call
- ReferenceCountedOpenSslEngine correctly accounts for the overhead of each call to SSL_write
- SslHandler shouldn't cache maxPacketBufferSize as aggressively because this value may change before/after the handshake.

Result:
OpenSslEngine and SslHanadler can handle multiple SSL/TLS network packet per call.

[1] https://docs.oracle.com/javase/7/docs/api/javax/net/ssl/SSLEngine.html
2017-07-10 12:15:02 -07:00
Norman Maurer
2a376eeb1b [maven-release-plugin] prepare for next development iteration 2017-07-06 13:24:06 +02:00
Norman Maurer
c7f8168324 [maven-release-plugin] prepare release netty-4.1.13.Final 2017-07-06 13:23:51 +02:00
Scott Mitchell
4d51eca218 SslHandlerTest#testCompositeBufSizeEstimationGuaranteesSynchronousWrite debug info
Motivation:
SslHandlerTest#testCompositeBufSizeEstimationGuaranteesSynchronousWrite has been observed to fail on CI servers. Knowing how many bytes were seen by the client would be helpful.

Modifications:
- Add bytesSeen to the exception if the client closes early.

Result:
More debug info available.
2017-07-05 14:06:02 -04:00
Scott Mitchell
449befa003 Workaround IBM's J9 JVM getSupportedCipherSuites() returning SSL_ prefix cipher names
Motivation:
IBM's J9 JVM utilizes a custom cipher naming scheme with SSL_ prefix [1] instead of the TLS_ prefix defined by TLS RFCs and the JSSE cihper suite names [2]. IBM's documentation says that the SSL_ prefix are "interchangeable" with cipher names with the TLS_ prefix [1]. To work around this issue we parse the supported cipher list and see an SSL_ prefix we can also add the same cipher with the TLS_ prefix. For more details see a discussion on IBM's forums [3] and IBM's issue tracker [4].

[1] https://www.ibm.com/support/knowledgecenter/en/SSYKE2_8.0.0/com.ibm.java.security.component.80.doc/security-component/jsse2Docs/ciphersuites.html
[2] http://docs.oracle.com/javase/8/docs/technotes/guides/security/StandardNames.html#ciphersuites
[3] https://www.ibm.com/developerworks/community/forums/html/topic?id=9b5a56a9-fa46-4031-b33b-df91e28d77c2
[4] https://www.ibm.com/developerworks/rfe/execute?use_case=viewRfe&CR_ID=71770

Modifications:
- When parsing the supported cipher list to get the supported ciphers and we encounter a SSL_ prefix we should also add a TLS_ prefix cipher.
- Remove SSL_ prefix ciphers from Http2SecurityUtil.

Result:
Work around for IBM JVM's custom naming scheme covers more cases for supported cipher suites.
2017-07-05 09:05:42 -04:00
Carl Mastrangelo
322fe8ec85 Exhaust non-exhaustive switch cases
Motivation:
ErrorProne warns about missing cases in switch statements that
appear as an oversight.

Modifcation:
Add the last case to statement to ensure all cases are covered.

Result:
Able to enable Error Prone static analysis
2017-06-27 07:56:41 +02:00
Carl Mastrangelo
83de77fbe5 Make Native loading work better with Java 8
Motivation:
Enable static linking for Java 8.  These commits are the same as those introduced to netty tcnative.  The goal is to allow lots of JNI libraries to be statically linked together without having conflict `JNI_OnLoad` methods.

Modification:
* add JNI_OnLoad suffixes to enable static linking
* Add static names to the list of libraries that try to be loaded
* Enable compiling with JNI 1.8
* Sort includes

Result:
Enable statically linked JNI code.
2017-06-23 19:42:13 +02:00
Nikolay Fedorovskikh
01eb428b39 Move methods for decode hex dump into StringUtil
Motivation:

PR #6811 introduced a public utility methods to decode hex dump and its parts, but they are not visible from netty-common.

Modifications:

1. Move the `decodeHexByte`, `decodeHexDump` and `decodeHexNibble` methods into `StringUtils`.
2. Apply these methods where applicable.
3. Remove similar methods from other locations (e.g. `HpackHex` test class).

Result:

Less code duplication.
2017-06-23 18:52:42 +02:00
Carl Mastrangelo
cbde38f6e9 Add cause to thrown exception in SelfSignedCert
Motivation:
Exceptions with causes are easier to debug

Modification:
Add the cause when generating a SelfSignedCert

Results:
More debugging context
2017-06-23 07:24:11 +02:00
Scott Mitchell
6d029ad3ac OpenSSL CHACHA20 CipherSuiteConverter updates
Motivation:
For historical reasons OpenSSL's internal naming convention for CHACHA20 based cipher suites does not include the HMAC algorithm in the cipher name. This will prevent the CHACHA20 cipher suites from being used if the RFC cipher names are specified.

Modifications:
- Add a special case for CHACHA20 cipher name conversions in CipherSuiteConverter
- Update OPENSSL_CIPHERSUITE_PATTERN to accommodate the new naming scheme for CHACHA20 cipher suites

Result:
CipherSuiteConverter now works with CHACHA20 cipher suites.
2017-06-21 06:50:43 +02:00
Roger Kapsi
7460d90a67 Add listener to returned Future rather than passed in Promise
Motivation

It's cleaner to add listeners to returned Futures rather than provided Promises because the latter can have strange side effects in terms of listeners firing and called methods returning. Adding listeners preemtively may yield also to more OPS than necessary when there's an Exception in the to be called method.

Modifications

Add listener to returned ChannelFuture rather than given ChannelPromise

Result

Cleaner completion and exception handling
2017-06-16 13:21:19 +01:00
Norman Maurer
e597756a56 Remove synchronized (ReferenceCountedOpenSslContext.class) blocks
Motivation:

We had some useless synchronized (ReferenceCountedOpenSslContext.class) blocks in our code which could slow down concurrent collecting and creating of ReferenceCountedOpenSslContext instances. Beside this we missed a few guards.

Modifications:

Use ReadWriteLock to correctly guard. A ReadWriteLock was choosen as SSL.newSSL(...) will be called from multiple threads all the time so using synchronized would be worse and there would be no way for the JIT to optimize it away

Result:

Faster concurrent creating and collecting of ReferenceCountedOpenSslContext instances and correctly guard in all cases.
2017-06-15 06:29:14 +02:00
Norman Maurer
fd67a2354d [maven-release-plugin] prepare for next development iteration 2017-06-08 21:06:24 +02:00
Norman Maurer
3acd5c68ea [maven-release-plugin] prepare release netty-4.1.12.Final 2017-06-08 21:06:01 +02:00
Scott Mitchell
24f801c7d1 OpenSslEngine return NEED_WRAP if the destination buffered filled
Motivation:
If the destination buffer is completely filled during a call to OpenSslEngine#wrap(..) we may return NEED_UNWRAP because there is no data pending in the SSL buffers. However during a handshake if the SSL buffers were just drained, and filled up the destination buffer it is possible OpenSSL may produce more data on the next call to SSL_write. This means we should keep trying to call SSL_write as long as the destination buffer is filled and only return NEED_UNWRAP when the destination buffer is not full and there is no data pending in OpenSSL's buffers.

Modifications:
- If the handshake produces data in OpenSslEngine#wrap(..) we should return NEED_WRAP if the destination buffer is completely filled

Result:
OpenSslEngine returns the correct handshake status from wrap().
Fixes https://github.com/netty/netty/issues/6796.
2017-06-02 07:47:35 -07:00
Roger Kapsi
2db4f2557d The SNI extension value is ASCII encoded but Netty uses UTF-8.
Motivation

RFC 6066 (https://tools.ietf.org/html/rfc6066#page-6) says that the hostname in the SNI extension is ASCII encoded but Netty decodes it using UTF-8.

Modifications

Use ASCII instead of UTF-8

Result

Fixes #6717
2017-05-19 15:12:06 -07:00
Roger Kapsi
915bf5f5b7 SslHandler#handlerRemoved0() shouldn't care about the SSLEngine being a specific type but only if it's ReferenceCounted
Motivation

SslHandler should release any type of SSLEngine if it implements the ReferenceCounted interface

Modifications

Change condition to check for ReferenceCounted interface

Result

Better use of interfaces
2017-05-19 19:31:51 +02:00
Norman Maurer
732948b3c4 Ensure SslUtils and so SslHandler works when using with Little-Endian buffers.
Motivation:

We not correctly handle LE buffers when try to read the packet length out of the buffer and just assume it always is a BE buffer.

Modifications:

Correctly account for the endianess of the buffer when reading the packet lenght.

Result:

Fixes [#6709].
2017-05-18 07:08:37 +02:00
Scott Mitchell
f20063d26b SslHandler#wrapNonAppData return early
Motivation:

SslHandler#wrapNonAppData may be able to return early if it is called from a unwrap method and the status is NEED_UNWRAP. This has been observed to occur while using the OpenSslEngine and can avoid allocation of an extra ByteBuf of size 2048.

Modifications:
- Return early from SslHandler#wrapNonAppData if NEED_UNWRAP and we are called from an unwrap method

Result:
Less buffer allocations and early return from SslHandler#wrapNonAppData.
2017-05-17 13:53:42 -07:00
Norman Maurer
0db2901f4d [maven-release-plugin] prepare for next development iteration 2017-05-11 16:00:55 +02:00
Norman Maurer
f7a19d330c [maven-release-plugin] prepare release netty-4.1.11.Final 2017-05-11 16:00:16 +02:00
Norman Maurer
dd837fe803 Remove some dead-code and cleanup 2017-05-10 20:48:47 +02:00
Norman Maurer
c053c5144d Correctly detect if Ocsp is supported
Motivation:

We only used the openssl version to detect if Ocsp is supported or not which is not good enough as even the version is correct it may be compiled without support for OCSP (like for example on ubuntu).

Modifications:

Try to enable OCSP while static init OpenSsl and based on if this works return true or false when calling OpenSsl.isOcspSupported().

Result:

Correctly detect if OSCP is supported.
2017-05-10 08:42:28 +02:00
Norman Maurer
ec935c5a7b Correctly delete SelfSignedCertificate once done with it.
Motivation:

In OpenSsl init code we create a SelfSignedCertificate which we not explicitly delete. This can lead to have the deletion delayed.

Modifications:

Delete the SelfSignedCertificate once done with it.

Result:

Fixes [#6716]
2017-05-09 11:28:20 +02:00
Scott Mitchell
141089998f OpenSslEngine wrap may generate bad data if multiple src buffers
Motivation:
SSL_write requires a fixed amount of bytes for overhead related to the encryption process for each call. OpenSslEngine#wrap(..) will attempt to encrypt multiple input buffers until MAX_PLAINTEXT_LENGTH are consumed, but the size estimation provided by calculateOutNetBufSize may not leave enough room for each call to SSL_write. If SSL_write is not able to completely write results to the destination buffer it will keep state and attempt to write it later. Netty doesn't account for SSL_write keeping state and assumes all writes will complete synchronously (by attempting to allocate enough space to account for the overhead) and feeds the same data to SSL_write again later which results in corrupted data being generated.

Modifications:
- OpenSslEngine#wrap should only produce a single TLS packet according to the SSLEngine API specificaiton [1].
[1] https://docs.oracle.com/javase/8/docs/api/javax/net/ssl/SSLEngine.html#wrap-java.nio.ByteBuffer:A-int-int-java.nio.ByteBuffer-
- OpenSslEngine#wrap should only consider a single buffer when determining if there is enough space to write, because only a single buffer will ever be consumed.

Result:
OpenSslEngine#wrap will no longer produce corrupted data due to incorrect accounting of space required in the destination buffers.
2017-05-08 14:43:36 -07:00
Norman Maurer
aab89b058e Ensure Netty is usable on Java7
Motivation:

When adding SNIMatcher support we missed to use static delegating methods and so may try to load classes that not exists in Java7. Which will lead to errors.

Modifications:

- Correctly only try to load classes when running on java8+
- Ensure Java8+ related tests only run when using java8+

Result:

Fixes [#6700]
2017-05-04 14:10:53 -07:00
Norman Maurer
6915ec3bb9 [maven-release-plugin] prepare for next development iteration 2017-04-29 14:10:00 +02:00
Norman Maurer
f30f242fee [maven-release-plugin] prepare release netty-4.1.10.Final 2017-04-29 14:09:32 +02:00
Norman Maurer
3ba4da8a79 Mark conscrypt as optional
Motivation:

Conscrypt is not needed when using the handler module, so it should be marked as optional

Modifications:

Mark conscrypt as optional

Result:

Be able to use handler module without conscrypt
2017-04-28 17:36:31 +02:00
Roger Kapsi
57d3393527 Ability to extend SniHandler and configure it with arbitrary runtime data
Motivation

SniHandler is "hardcoded" to use hostname -> SslContext mappings but there are use-cases where it's desireable and necessary to return more information than a SslContext. The only option so far has been to use a delegation pattern

Modifications

Extract parts of the existing SniHandler into an abstract base class and extend SniHandler from it. Users can do the same by extending the new abstract base class and implement custom behavior that is possibly very different from the common/default SniHandler.

Touches

- f97866dbc6
- b604a22395

Result

Fixes #6603
2017-04-26 19:05:16 -07:00
Norman Maurer
7f3b75a509 Revert "SslHandler avoid calling wrap/unwrap when unnecessary"
This reverts commit 6353c229fd to "fix" [#6578].
2017-04-23 21:08:03 +02:00
Dmitriy Dumanskiy
e78ccd6d52 #6657 do not throw ClassCastException when rule subnet version doesn't match remote IP version 2017-04-23 08:30:02 +02:00
Norman Maurer
7214740c06 Add support for SNIMatcher when using SslProvider.OPENSSL* and Java8+
Motivation:

Java8 adds support for SNIMatcher to reject SNI when the hostname not matches what is expected. We not supported doing this when using SslProvider.OPENSSL*.

Modifications:

- Add support for SNIMatcher when using SslProvider.OPENSSL*
- Add unit tests

Result:

SNIMatcher now support with our own SSLEngine as well.
2017-04-21 10:23:47 +02:00
Norman Maurer
4dd6c14ba2 Only use test SslProviders that are supported in SslHandlerTest.testCompositeBufSizeEstimationGuaranteesSynchronousWrite().
Motivation:

We need to ensure we only try to to test with the SslProviders that are supported when running the SslHandlerTest.testCompositeBufSizeEstimationGuaranteesSynchronousWrite test.

Modifications:

Skip SslProvider.OPENSSL* if not supported.

Result:

No more test-failures if openssl is not installed on the system.
2017-04-20 19:29:52 +02:00
Nikolay Fedorovskikh
0692bf1b6a fix the typos 2017-04-20 04:56:09 +02:00
Norman Maurer
1dfd852dff Revert "Add support for SNIMatcher when using SslProvider.OPENSSL* and Java8+"
This reverts commit cc5d1d0a7e.
2017-04-18 13:43:03 +02:00
Norman Maurer
cc5d1d0a7e Add support for SNIMatcher when using SslProvider.OPENSSL* and Java8+
Motivation:

Java8 adds support for SNIMatcher to reject SNI when the hostname not matches what is expected. We not supported doing this when using SslProvider.OPENSSL*.

Modifications:

- Add support for SNIMatcher when using SslProvider.OPENSSL*
- Add unit tests

Result:

SNIMatcher now support with our own SSLEngine as well.
2017-04-18 08:16:33 +02:00
Roger Kapsi
077a1988b9 OCSP stapling support for Netty using netty-tcnative.
https://github.com/netty/netty-tcnative/pull/215

Motivation

OCSP stapling (formally known as TLS Certificate Status Request extension) is alternative approach for checking the revocation status of X.509 Certificates. Servers can preemptively fetch the OCSP response from the CA's responder, cache it for some period of time, and pass it along during (a.k.a. staple) the TLS handshake. The client no longer has to reach out on its own to the CA to check the validity of a cetitficate. Some of the key benefits are:

1) Speed. The client doesn't have to crosscheck the certificate.
2) Efficiency. The Internet is no longer DDoS'ing the CA's OCSP responder servers.
3) Safety. Less operational dependence on the CA. Certificate owners can sustain short CA outages.
4) Privacy. The CA can lo longer track the users of a certificate.

https://en.wikipedia.org/wiki/OCSP_stapling
https://letsencrypt.org/2016/10/24/squarespace-ocsp-impl.html

Modifications

https://www.openssl.org/docs/man1.0.2/ssl/SSL_set_tlsext_status_type.html

Result

High-level API to enable OCSP stapling
2017-04-03 11:56:53 -07:00
Kevin Oliver
34e0007f07 LoggingHandler does not override channelReadComplete or channelWritabilityChanged
Motivation:

`io.netty.handler.logging.LoggingHandler` does not log when these
events happen.

Modifiations:

Add overrides with logging to these methods.

Result:

Logging now happens for these two events.
2017-04-03 11:46:01 -07:00
Norman Maurer
4bcfa07a7d Fix OpenSslCertificateException error code validation
Motivation:

In OpenSslCertificateException we tried to validate the supplied error code but did not correctly account for all different valid error codes and so threw an IllegalArgumentException.

Modifications:

- Fix validation by updating to latest netty-tcnative and use CertificateVerifier.isValid
- Add unit tests

Result:

Validation of error code works as expected.
2017-04-03 11:12:15 -07:00
Norman Maurer
5163869439 Only try to load conscrypt class in tests when supported.
Motivation:

1419f5b601 added support for conscrypt but the CI started to fail when running tests with java7 as conscrypt is compiled with java8. This was partly fixed in c4832cd9d9 but we also need to ensure we not try to even load the classes.

Modifications:

Only try to load conscrypt classes when on java8+-

Result:

CI not fails anymore.
2017-04-01 22:51:51 +02:00
Norman Maurer
c4832cd9d9 Only support using Conscrypt on Java8+
Motivation:

1419f5b601 added support for conscrypt but the CI started to fail when running tests with java7 as conscrypt is compiled with java8.

Modifications:

Only support conscrypt on Java8+

Result:

CI not fails anymore.
2017-04-01 20:38:33 +02:00
Nathan Mittler
1419f5b601 Adding support for Conscrypt (#6271)
Motivation:

Conscrypt is a Java Security provider that wraps OpenSSL (specifically BoringSSL). It's a possible alternative to Netty-tcnative that we should explore. So this commit is just to enable us to further investigate its use.

Modifications:

Modifying the SslContext creation path to support the Conscrypt provider.

Result:

Netty will support OpenSSL with conscrypt.
2017-03-31 13:55:59 -07:00
Norman Maurer
ed1071d327 Limit the maximum size of the allocated outbound buffer to MAX_ENCRYPTED_PACKET_LENGTH
Motivation:

We should limit the size of the allocated outbound buffer to MAX_ENCRYPTED_PACKET_LENGTH to ensure we not cause an OOME when the user tries to encrypt a very big buffer.

Modifications:

Limit the size of the allocated outbound buffer to MAX_ENCRYPTED_PACKET_LENGTH

Result:

Fixes [#6564]
2017-03-31 07:53:50 +02:00
R Kaja Mohideen
13cd69c5ec Recognizing TLS Extension "HeartBeat" as Valid TLS Packet
Motivation:

The widely used SSL Implementation, OpenSSL, already supports Heartbeat Extension; both sending and responding to Heartbeat Messages. But, since Netty is not recognizing that extension as valid packet, peers won't be able to use this extension.

Modification:

Update SslUtils.java to recognize Heartbeat Extension as valid tls packet.

Result:

With this change, softwares using Netty + OpenSSL will be able to respond for TLS Heartbeat requests (actually taken care by OpenSSL - no need of any extra implementation from Clients)
2017-03-28 11:43:26 +02:00
Michael O'Brien
61f53c4d07 ChunkedWriteHandler flushes too often
Motivation:

ChunkedWriteHandler queues written messages and actually writes them
when flush is called. In its doFlush method, it needs to flush after
each chunk is written to preserve memory. However, non-chunked messages
(those that aren't of type ChunkedInput) are treated in the same way,
which means that flush is called after each message is written.

Modifications:

Moved the call to flush() inside the if block that tests if the message
is an instance of ChunkedInput. To ensure flush is called at least once,
the existing boolean flushed is checked at the end of doFlush. This
check was previously in ChunkedWriteHandler.flush(), but wasn't checked in
other invocations of doFlush, e.g. in channelInactive.

Result:

When this handler is present in a pipeline, writing a series
of non-chunked messages will be flushed as the developer intended.
2017-03-18 08:05:59 -07:00
Norman Maurer
2b8c8e0805 [maven-release-plugin] prepare for next development iteration 2017-03-10 07:46:17 +01:00
Norman Maurer
1db58ea980 [maven-release-plugin] prepare release netty-4.1.9.Final 2017-03-10 07:45:28 +01:00
Johno Crawford
cfebaa36c0 Support for handling SSL and non-SSL in pipeline
Motivation:

Some pipelines require support for both SSL and non-SSL messaging.

Modifications:

Add utility decoder to support both SSL and non-SSL handlers based on the initial message.

Result:

Less boilerplate code to write for developers.
2017-03-09 01:00:43 -08:00
Scott Mitchell
10d9f82f14 Remove duplicate call to SSLContext.setVerify from ReferenceCountedOpenSslServerContext
Motivation:
5e64985089 introduced support for the KeyManagerFactory while using OpenSSL. This same commit also introduced 2 calls to SSLContext.setVerify when 1 should be sufficient.

Modifications:
- Remove the duplicate call to SSLContext.setVerify

Result:
Less duplicate code in ReferenceCountedOpenSslServerContext.
2017-03-08 13:56:12 -08:00
Scott Mitchell
a2304287a1 SslContext to support TLS/SSL protocols
Motivation:
SslContext and SslContextBuilder do not support a way to specify the desired TLS protocols. This currently requires that the user extracts the SSLEngine once a context is built and manually call SSLEngine#setEnabledProtocols(String[]). Something this critical should be supported at the SslContext level.

Modifications:
- SslContextBuilder should accept a list of protocols to configure for each SslEngine

Result:
SslContext consistently sets the supported TLS/SSL protocols.
2017-03-08 09:24:59 -08:00
Scott Mitchell
6bb661302f OpenSsl tests incomplete check for supporting key manager
Motivaiton:
It is possible that if the OpenSSL library supports the interfaces required to use the KeyManagerFactory, but we fail to get the io.netty.handler.ssl.openssl.useKeyManagerFactory system property (or this property is set to false) that SSLEngineTest based unit tests which use a KeyManagerFactory will fail.

Modifications:
- We should check if the OpenSSL library supports the KeyManagerFactory interfaces and if the system property allows them to be used in OpenSslEngineTests

Result:
Unit tests which use OpenSSL and KeyManagerFactory will be skipped instead of failing.
2017-03-07 08:24:06 +01:00
Scott Mitchell
53fc693901 SslHandler and OpenSslEngine miscalculation of wrap destination buffer size
Motivation:
When we do a wrap operation we calculate the maximum size of the destination buffer ahead of time, and return a BUFFER_OVERFLOW exception if the destination buffer is not big enough. However if there is a CompositeByteBuf the wrap operation may consist of multiple ByteBuffers and each incurs its own overhead during the encryption. We currently don't account for the overhead required for encryption if there are multiple ByteBuffers and we assume the overhead will only apply once to the entire input size. If there is not enough room to write an entire encrypted packed into the BIO SSL_write will return -1 despite having actually written content to the BIO. We then attempt to retry the write with a bigger buffer, but because SSL_write is stateful the remaining bytes from the previous operation are put into the BIO. This results in sending the second half of the encrypted data being sent to the peer which is not of proper format and the peer will be confused and ultimately not get the expected data (which may result in a fatal error). In this case because SSL_write returns -1 we have no way to know how many bytes were actually consumed and so the best we can do is ensure that we always allocate a destination buffer with enough space so we are guaranteed to complete the write operation synchronously.

Modifications:
- SslHandler#allocateNetBuf should take into account how many ByteBuffers will be wrapped and apply the encryption overhead for each
- Include the TLS header length in the overhead computation

Result:
Fixes https://github.com/netty/netty/issues/6481
2017-03-06 08:15:13 -08:00
Scott Mitchell
2cff918044 Correct usages of internalNioBuffer
Motivation:
There are numerous usages of internalNioBuffer which hard code 0 for the index when the intention was to use the readerIndex().

Modifications:
- Remove hard coded 0 for the index and use readerIndex()

Result:
We are less susceptible to using the wrong index, and don't make assumptions about the ByteBufAllocator.
2017-03-02 12:51:22 -08:00
Scott Mitchell
1f6782894a OpenSslEngine wrap with heap buffer bug
Motivation:
ReferenceCountedOpenSslEngine#wrap must have a direct buffer for a destination to interact with JNI. If the user doesn't supply a direct buffer we internally allocate one to write the results of wrap into. After this operation completes we copy the contents of the direct buffer into the heap buffer and use internalNioBuffer to get the content. However we pass in the end index but the internalNioBuffer expects a length.

Modifications:
- pass the length instead of end index to internalNioBuffer

Result:
ReferenceCountedOpenSslEngine#wrap will copy the correct amount of data into the destination buffer when heap buffers are wrapped.
2017-03-02 12:49:35 -08:00
Norman Maurer
7e7e10fb1e Correct SslContextBuilder javadocs
Motivation:

SslContextBuilder sill state the KeyManagerFactory and TrustManagerFactory are only supported when SslProvider.JDK is used. This is not correct anymore.

Modifications:

Fix javadocs.

Result:

Correct javadocs.
2017-03-02 09:36:45 +01:00
Stephane Landelle
79e24d1a17 Add javadoc warning on SslContext#newHandler client-side
Motivation:

SslContext#newHandler currently creates underlying SSLEngine without
enabling HTTPS endpointIdentificationAlgorithm. This behavior in
unsecured when used on the client side.

We can’t harden the behavior for now, as it would break existing
behavior, for example tests using self signed certificates.

Proper hardening will happen in a future major version when we can
break behavior.

Modifications:

Add javadoc warnings with code snippets.

Result:

Existing unsafe behavior and workaround documented.
2017-03-01 10:44:44 +01:00
Norman Maurer
ca2c349a4a Correctly have exceptions thrown from decode(...) method be wrapped with DecodingException
Motivation:

Normally if a decoder produces an exception its wrapped with DecodingException. This is not the cause for NotSslRecordException in SslHandler and SniHandler.

Modifications:

Just throw the NotSslRecordException exception for decode(...) and so ensure its correctly wrapped in a DecodingException before its passed through the pipeline.

Result:

Consist behavior.
2017-03-01 06:41:30 +01:00
Scott Mitchell
70bcb40855 OpenSslEngine may use networkBIO after calling shutdown
Motivation:
To ensure that all bytes queued in OpenSSL/tcnative internal buffers we invoke SSL_shutdown again to stimulate OpenSSL to write any pending bytes. If this call fails we may call SSL_free and the associated shutdown method to free resources. At this time we may attempt to use the networkBIO which has already been freed and get a NPE.

Modifications:
- Don't call bioLengthByteBuffer(networkBIO) if we have called shutdown() in ReferenceCountedOpenSslEngine

Result:
Fixes https://github.com/netty/netty/issues/6466
2017-02-28 12:01:12 -08:00
Nikolay Fedorovskikh
c0cd3db5b1 Fix class references of its subclass issue
Motivation:

Realization of `AbstractTrafficShapingHandler.userDefinedWritabilityIndex()` has references to subclasses.
In addition, one of the subclasses overriding it, but the other does not.

Modifications:

Add overriding to the second subclass. Remove references to subclasses from parent class.

Result:

More consistent and clean code (OOP-stylish).
2017-02-27 07:48:51 +01:00
Norman Maurer
22ccf6c7b1 Fix test-failures introduced 325cc84a2e on Java7
Motivation:

325cc84a2e introduced new tests which uses classes only provided by Java8+. We need to ensure we only try to load classes needed for these when we run the tests on Java8+ so we still can run the testsuite with Java7.

Modifications:

Add extra class which only gets loaded when Java8+ is used and move code there.

Result:

No more class-loader issue when running tests with Java7.
2017-02-24 10:36:30 +01:00
Norman Maurer
325cc84a2e Throw if SSLParameters contains settings that are not supported by ReferenceCountedOpenSslEngine
Motivation:

We not support all SSLParameters settings so we should better throw if a user try to use them.

Modifications:

- Check for unsupported parameters
- Add unit test

Result:

Less surprising behavior.
2017-02-23 20:00:40 +01:00
Nikolay Fedorovskikh
0623c6c533 Fix javadoc issues
Motivation:

Invalid javadoc in project

Modifications:

Fix it

Result:

More correct javadoc
2017-02-22 07:31:07 +01:00
Scott Mitchell
e08a3b1f35 Fix SSLException check for JDK work around missed by 2dffc2f9fb 2017-02-20 21:38:04 -08:00
Scott Mitchell
2dffc2f9fb SSLEngineTest issue introduced by d8e6fbb9c3
Motivation:
d8e6fbb9c3 attempted to account for the JDK not throwing the expected SSLHandshakeException by allowing a SSLException to also pass the test. However in some situations the SSLException will not be the top level exception and the Throwable must be unwrapped to see if the root cause is an SSLException.

Modifications:
- Unwrap exceptions thrown by the JDK's SSLEngine to check for SSLException.

Result:
SSLEngineTest (and derived classes) are more reliable.
2017-02-20 12:19:41 -08:00
Norman Maurer
c57a1bdb2d Log used native library by netty-tcnative
Motivation:

As netty-tcnative can be build against different native libraries and versions we should log the used one.

Modifications:

Log the used native library after netty-tcnative was loaded.

Result:

Easier to understand what native SSL library was used.
2017-02-20 20:52:22 +01:00
Scott Mitchell
d8e6fbb9c3 OpenSslEngine should respect hostname verification
Motivation:
OpenSSL doesn't automatically verify hostnames and requires extract method calls to enable this feature [1]. We should allow this to be configured.

Modifications:
- SSLParamaters#getEndpointIdentificationAlgorithm() should be respected and configured via tcnative interfaces.

Result:
OpenSslEngine respects hostname verification.

[1] https://wiki.openssl.org/index.php/Hostname_validation
2017-02-17 13:21:29 -08:00
Scott Mitchell
5de4029b43 Checkstyle fix from 56694eb 2017-02-16 22:28:56 -08:00
Scott Mitchell
56694ebc0f Cleanup from fbf0e5f4dd
Motivation:
ThreadLocalInsecureRandom still referenced ThreadLocalRandom directly, but shouldn't.

Modifications:
ThreadLocalInsecureRandom should reference PlatformDependent#threadLocalRandom() in comments

Result:
Less usage of internal.ThreadLocalRandom.
2017-02-16 15:56:23 -08:00
Norman Maurer
fbf0e5f4dd Prefer JDK ThreadLocalRandom implementation over ours.
Motivation:

We have our own ThreadLocalRandom implementation to support older JDKs . That said we should prefer the JDK provided when running on JDK >= 7

Modification:

Using ThreadLocalRandom implementation of the JDK when possible.

Result:

Make use of JDK implementations when possible.
2017-02-16 15:44:00 -08:00
Norman Maurer
b2f7e8648e Fix ReferenceCountedOpenSslEngine.getEnabledProtocols() when using boringssl
Motivation:

Commit cd3bf3df58 made netty observe the latest version of netty-tcnative which changed the way how static fields are computed for various SSL.* values. This lead to have SSL_OP_NO_SSLv2 become 0 when using boringssl as boringssl not supports SSLv2 at all. In the logic of ReferenceCountedOpenSslEngine.getEnabledProtocols() we not expect to have a zero value and so our logic fails.

Modifications:

Check we actual support the protocol before return it as enabled.

Result:

SSLEngineTest.testEnablingAnAlreadyDisabledSslProtocol passes again with boringssl
2017-02-16 07:47:06 +01:00
Scott Mitchell
4431ad894d OpenSslEngine may lose data if the non-application buffer is small/full
Motivation:
If an event occurs which generates non-application data (shutdown, handshake failure, alert generation, etc...) and the non-application buffer in the ByteBuffer BIO is full (or sufficiently small) we may not propagate all data to our peer before tearing down the socket.

Modifications:
- when wrap() detects the outbound is closed, but there is more data pending in the non-application buffers, we must also check if OpenSSL will generate more data from calling SSL_shutdown again
- when wrap() detects a handshakeExcpetion during failure we should check if OpenSSL has any pending data (in addition to the non-application buff) before throwing the handshake exception

Result:
OpenSslEngine more reliably transmits data to the peer before closing the socket.
2017-02-15 16:14:14 -08:00
Norman Maurer
847359fd36 Fix incorrect delegate in overriden method in JdkOpenSslEngineInteroptTest
Motivation:

JdkOpenSslEngineInteroptTest.mySetupMutualAuthServerIsValidClientException(...) delegated to the wrong super method.

Modifications:

Fix delegate

Result:

Correct test-code.
2017-02-15 19:20:06 +01:00
Norman Maurer
43a2315372 Skip SSLEngineTests that depend on KeyManagerFactory when this is not supported by the openssl version.
Motivation:

Some version of openssl dont support the needed APIs to use a KeyManagerFactory. In this case we should skip the tests.

Modifications:

- Use assumeTrue(...) to skip tests that need a KeyManagerFactory and its not supported.

Result:

Tests pass on all openssl versions we support.
2017-02-15 19:17:43 +01:00
Norman Maurer
b7acae03f2 Update tcnative package names
Motivation:

tcnative was moved into an internal package.

Modifications:

Update package for tcnative imports.

Result:

Use correct package names for tcnative.
2017-02-15 13:51:41 +01:00
Norman Maurer
5728e0eb2c Use the correct arguments when run with jdk9
Motivation:

We need to pass special arguments to run with jdk9 as otherwise some tests will not be able to run.

Modifications:

Allow to define extra arguments when running with jdk9

Result:

Tests pass with jdk9
2017-02-15 10:15:00 +01:00
Scott Mitchell
d60e37cb3d OpenSslEngine wrap may not consume all data
Motivation:
If the OpenSslEngine has bytes pending in the non-application buffer and also generates wrapped data during the handshake then the handshake data will be missed. This will lead to a handshake stall and eventually timeout. This can occur if the non-application buffer becomes full due to a large certificate/hello message.

Modification:
- ReferenceCountedOpenSslEngine should not assume if no data is flushed from the non-application buffer that no data will be produced by the handshake.

Result:
New unit tests with larger certificate chains don't fail.
2017-02-15 09:29:32 +01:00
Scott Mitchell
c521c72178 SSLEngineTest cleanup
Motivation:
OpenSslEngineTest has unused imports and SSLEngineTest uses a fixed port which was used for debugging.

Modifications:
- Remove unused imports
- Use ephemeral port

Result:
Cleaner test code.
2017-02-14 17:28:13 -08:00
Scott Mitchell
84ebb4c315 Fix checkstyle issues introduced by fdcad3150e 2017-02-14 14:49:22 -08:00
Scott Mitchell
fdcad3150e Use tcnative's new setVerify modes
Modifications:
tcnative made some fixes and API changes related to setVerify. We should absorb these changes in Netty.

Modifications:
- Use tcnatives updated APIs
- Add unit tests to demonstrate correct behavior

Result:
Updated to latest tcnative code and more unit tests to verify expected behavior.
2017-02-14 12:14:58 -08:00
Scott Mitchell
cd3bf3df58 Consume tcnative options update
Motivation:
tcnative has updated how constants are defined and removed some constants which are either obsolete or now set directly in tcnative.

Modifications:
- update to compile against tcnative changes.

Result:
Netty compiles with tcnative options changes.
2017-02-14 12:09:10 -08:00
Norman Maurer
591293bfb4 Change minimum JDK version for compilation to 1.8
Motivation:

We previously changed netty to always compile with java7 as otherwise source compatibility was broken.

This was reported in [#3548] but was fixed in the meantime:
- https://bugs.openjdk.java.net/browse/JDK-8029240
- https://bugs.openjdk.java.net/browse/JDK-8030855

Modifications:

Change minimum JDK version for compilation to 1.8

Result:

Easier to maintain code.
2017-02-14 19:06:59 +01:00
Norman Maurer
adcde84253 Allow to unwrap ByteBuffer > MAX_ENCRYPTED_PACKET_LENGTH
Motivation:

We should remove the restriction to only allow to call unwrap with a ByteBuffer[] whose cumulative length exceeds MAX_ENCRYPTED_PACKET_LENGTH.

Modifications:

Remove guard.

Result:

Fixes [#6335].
2017-02-14 08:30:35 +01:00
Norman Maurer
f7c8cf9cb9 Cleanup code in ssl package.
Motivation:

There were some warnings for the code in the ssl package.

Modifications:

- Remove not needed else blocks
- Use correctly base class for static usage
- Replace String.length() == 0 with String.isEmpty()
- Remove unused code

Result:

Less warnings and cleaner code.
2017-02-14 08:23:04 +01:00
Scott Mitchell
6765e9f99d CipherSuiteConverter NPE
Motivation:
CipherSuiteConverter may throw a NPE if a cipher suite from OpenSSL does not match the precomputed regular expression for OpenSSL ciphers. This method shouldn't throw and instead just return null.

Modifications:
- if cacheFromOpenSsl(..) fails the conversion toJava should return null

Result:
Fixes https://github.com/netty/netty/issues/6336.
2017-02-13 15:02:09 -08:00
Scott Mitchell
d06990f434 OpenSSL ByteBuffer BIO
Motivation:
Currently Netty utilizes BIO_new_bio_pair so we can control all FD lifetime and event notification but delegates to OpenSSL for encryption/decryption. The current implementation sets up a pair of BIO buffers to read/write encrypted/plaintext data. This approach requires copying of data from Java ByteBuffers to native memory BIO buffers, and also requires both BIO buffers to be sufficiently large to hold application data. If direct ByteBuffers are used we can avoid coyping to/from the intermediate BIO buffer and just read/write directly from the direct ByteBuffer memory. We still need an internal buffer because OpenSSL may generate write data as a result of read calls (e.g. handshake, alerts, renegotiation, etc..), but this buffer doesn't have to be be large enough to hold application data.

Modifications:
- Take advantage of the new ByteBuffer based BIO provided by netty-tcnative instead of using BIO_read and BIO_write.

Result:
Less copying and lower memory footprint requirement per TLS connection.
2017-02-09 09:50:55 -08:00
Scott Mitchell
6353c229fd SslHandler avoid calling wrap/unwrap when unnecessary
Motivation:
The SSLEngine wrap and unwrap methods can be called in a way that has no side effects, but this could involve costly validation and allocation. The SslHandler should avoid calling into these methods if possible.

Modifications:
- wrapNonAppData should provide additional status which can be used by wrap to breakout early if possible

Result:
SslHandler invokes the SSLEngine less.
2017-02-07 00:12:31 -08:00
Norman Maurer
a7c0ff665c Only use Mockito for mocking.
Motivation:

We used various mocking frameworks. We should only use one...

Modifications:

Make usage of mocking framework consistent by only using Mockito.

Result:

Less dependencies and more consistent mocking usage.
2017-02-07 08:47:22 +01:00
Dmitriy Dumanskiy
b9abd3c9fc Cleanup : for loops for arrays to make code easier to read and removed unnecessary toLowerCase() 2017-02-06 07:47:59 +01:00
Norman Maurer
1a05463c56 More strict testing of handshake behaviour
Motiviation:

We should ensure we not need any extra wrap / unwrap calls during handshake once the handshake was signaled as finished

Modifications:

More strict testing

Result:

Better testing of handshake behaviour
2017-02-03 09:45:09 +01:00
Roger Kapsi
d688e35e70 Fixing argument names
Motivation

Misleading argument names

Modifications

Stripping xMillis suffix from arguments because there's a TimeUnit

Result

Less confusion
2017-02-03 08:39:25 +01:00
Norman Maurer
1d128c7a65 Switch to netty-tcnative 2.0.0 which uses different package names
Motivation:

Previous versions of netty-tcnative used the org.apache.tomcat namespace which could lead to problems when a user tried to use tomcat and netty in the same app.

Modifications:

Use netty-tcnative which now uses a different namespace and adjust code to some API changes.

Result:

Its now possible to use netty-tcnative even when running together with tomcat.
2017-02-02 10:44:38 +01:00
Norman Maurer
735d6dd636 [maven-release-plugin] prepare for next development iteration 2017-01-30 15:14:02 +01:00
Norman Maurer
76e22e63f3 [maven-release-plugin] prepare release netty-4.1.8.Final 2017-01-30 15:12:36 +01:00
Norman Maurer
7736534b34 Ensure tests added in 91f050d2ef work with different openssl installations
Motivation:

Tests were added in 91f050d2ef to run with different protocols / ciphers. These may fail currently when openssl was compiled without support for the protocol / ciphers.

Modifications:

- Refactor tests to easier understand for which protocol / cipher it failed
- Not fail the test if the protocol is not supported with the used openssl version.

Result:

More robust testing.
2017-01-30 13:21:56 +01:00
Norman Maurer
7a39afd031 Correctly detect which protocols are supported when using OpenSSL
Motivation:

We failed to properly test if a protocol is supported on an OpenSSL installation and just always returned all protocols.

Modifications:

- Detect which protocols are supported on a platform.
- Skip protocols in tests when not supported. This fixes a build error on some platforms introduced by [#6276].

Result:

Correctly return only the supported protocols
2017-01-27 23:37:10 +01:00
Norman Maurer
91f050d2ef More precise calculate the maximum record size when using SslProvider.OPENSSL* and so decrease mem usage.
Motivation:

We used ca 2k as maximum overhead for encrypted packets which is a lot more then what is needed in reality by OpenSSL. This could lead to the need of more memory.

Modification:

- Use a lower overhead of 86 bytes as defined by the spec and openssl itself
- Fix unit test to use the correct session to calculate needed buffer size

Result:

Less memory usage.
2017-01-27 19:51:45 +01:00
Norman Maurer
5cd8133477 Add unit test that shows we correctly return BUFFER_UNDERFLOW
Motivation:

We should test that we correctly return BUFFER_UNDERFLOW if the src buffer not contains enough data to unwrap it.

Modification:

Add unit test to verify behaviour.

Result:

Better test coverrage of SSLEngine implementations.
2017-01-26 15:04:10 +01:00
Norman Maurer
640ef615be Allow to configure SslHandler to wait for close_notify response before closing the Channel and fix racy flush close_notify timeout scheduling.
Motivation:

SslHandler closed the channel as soon as it was able to write out the close_notify message. This may not be what the user want as it may make sense to only close it after the actual response to the close_notify was received in order to guarantee a clean-shutdown of the connection in all cases.

Beside this closeNotifyFlushTimeoutMillis is volatile so may change between two reads. We need to cache it in a local variable to ensure it not change int between. Beside this we also need to check if the flush promise was complete the schedule timeout as this may happened but we were not able to cancel the timeout yet. Otherwise we will produce an missleading log message.

Modifications:

- Add new setter / getter to SslHandler which allows to specify the behavior (old behavior is preserved as default)
- Added unit test.
- Cache volatile closeNotifyTimeoutMillis.
- Correctly check if flush promise was complete before we try to forcibly close the Channel and log a warning.
- Add missing javadocs.

Result:

More clean shutdown of connection possible when using SSL and fix racy way of schedule close_notify flush timeouts and javadocs.
2017-01-24 10:51:16 +01:00
Norman Maurer
e9fa40d770 Ensure calling ReferenceCountedOpenSslEngine.wrap(...) after closeOutbound() was called will not throw an SSLException
Motivation:

PR [#6238] added guards to be able to call wrap(...) / unwrap(...) after the engine was shutdown. Unfortunally one case was missed which is when closeOutbound() was called and produced some data while closeInbound() was not called yet.

Modifications:

Correctly guard against SSLException when closeOutbound() was called, produced data and someone calls wrap(...) after it.

Result:

No more SSLException. Fixes [#6260].
2017-01-21 07:36:05 +01:00
Norman Maurer
1b31397249 Deprecate methods on SslHandler that have other replacements
Motivation:

SslHandler has multiple methods which have better replacements now or are obsolete. We should mark these as `@Deprecated`.

Modifications:

Mark methods as deprecated.

Result:

API cleanup preparation.
2017-01-19 21:34:22 +01:00
Norman Maurer
ac5f701a5c Use less memory during writes when using SslHandler with SslProvider.OpenSsl
Motivation:

In commit fc3c9c9523 I changes the way how we calculate the capacity of the needed ByteBuf for wrap operations that happen during writes when the SslHandler is used. This had the effect that the same capacity for ByteBufs is needed for the JDK implementation of SSLEngine but also for our SSLEngine implementation that uses OpenSSL / BoringSSL / LibreSSL. Unfortunally this had the side-effect that applications that used our SSLEngine implementation now need a lot more memory as bascially the JDK implementation always needs a 16kb buffer for each wrap while we can do a lot better for our SSLEngine implementation.

Modification:

- Resurrect code that calculate a better ByteBuf capacity when using our SSLEngine implementation and so be able to safe a lot of memory
- Add test-case to ensure it works as expected and is not removed again later on.

Result:

Memory footprint of applications that uses our SSLEngine implementation based on OpenSSL / BoringSSL / LibreSSL is back to the same amount of before commit fc3c9c9523.
2017-01-19 21:24:39 +01:00
Tim Brooks
3344cd21ac Wrap operations requiring SocketPermission with doPrivileged blocks
Motivation:

Currently Netty does not wrap socket connect, bind, or accept
operations in doPrivileged blocks. Nor does it wrap cases where a dns
lookup might happen.

This prevents an application utilizing the SecurityManager from
isolating SocketPermissions to Netty.

Modifications:

I have introduced a class (SocketUtils) that wraps operations
requiring SocketPermissions in doPrivileged blocks.

Result:

A user of Netty can grant SocketPermissions explicitly to the Netty
jar, without granting it to the rest of their application.
2017-01-19 21:12:52 +01:00
Norman Maurer
cf8e07f62f Run all tests in SSLEngineTest with heap, direct and mixed buffers
Motivation:

As we use different execution path in our SSLEngine implementation depending on if heap, direct or mixed buffers are used we should run the tests with all of them.

Modification:

Ensure we run all tests with different buffer types.

Result:

Better test-coverage
2017-01-19 19:18:31 +01:00
Norman Maurer
d55c321306 Add SslCloseCompletionEvent that is fired once a close_notify was received
Motivation:

For the completion of a handshake we already fire a SslHandshakeCompletionEvent which the user can intercept. We should do the same for the receiving of close_notify.

Modifications:

Add SslCloseCompletionEvent and test-case.

Result:

More consistent API.
2017-01-19 19:15:24 +01:00
Scott Mitchell
9a4aa617f4 PlatformDependent#getClassLoader fails in restrictive classloader environment
Motivation:
https://github.com/netty/netty/pull/6042 only addressed PlatformDependent#getSystemClassLoader but getClassLoader is also called in an optional manner in some common code paths but fails to catch a general enough exception to continue working.

Modifications:
- Calls to getClassLoader which can continue if results fail should catch Throwable

Result:
More resilient code in the presense of restrictive class loaders.
Fixes https://github.com/netty/netty/issues/6246.
2017-01-19 09:01:36 -08:00
Norman Maurer
d37702aa69 Ensure SslHandler.sslCloseFuture() is notified in all cases.
Motivation:

The SslHandler.sslCloseFuture() may not be notified when the Channel is closed before a closify_notify is received.

Modifications:

Ensure we try to fail the sslCloseFuture() when the Channel is closed.

Result:

Correctly notify the ssl close future.
2017-01-19 07:53:08 +01:00
Norman Maurer
91951a51ad Ensure calling ReferenceCountedSslEngine.unwrap(...) / wrap(...) can be called after it was closed
Motivation:

The JDK implementation of SSLEngine allows to have unwrap(...) / wrap(...) called even after closeInbound() and closeOutbound() were called. We need to support the same in ReferenceCountedSslEngine.

Modification:

- Allow calling ReferenceCountedSslEngine.unwrap(...) / wrap(...) after the engine was closed
- Modify unit test to ensure correct behaviour.

Result:

Implementation works as expected.
2017-01-19 07:51:27 +01:00
Norman Maurer
821501717b Fix possible IOOBE when calling ReferenceCountedSslEngine.unwrap(...) with heap buffers.
Motivation:

fc3c9c9523 introduced a bug which will have ReferenceCountedSslEngine.unwrap(...) produce an IOOBE when be called with an BÅ·teBuffer as src that contains multiple SSLRecords and has a position != 0.

Modification:

- Correctly set the limit on the ByteBuffer and so fix the IOOBE.
- Add test-case to verify the fix

Result:

Correctly handle heap buffers as well.
2017-01-19 07:47:12 +01:00
Norman Maurer
7f01da8d0f [maven-release-plugin] prepare for next development iteration 2017-01-12 11:36:51 +01:00
Norman Maurer
7a21eb1178 [maven-release-plugin] prepare release netty-4.1.7.Final 2017-01-12 11:35:58 +01:00
Norman Maurer
be8c16cd0c [#6141] OpenSSLContext Mutual Auth does not announce acceptable CAs
Motivation:

Openssl provider should behave same as JDK provider when mutual authentication is required and a specific set of trusted Certificate Authorities are specified. The SSL handshake should return back to the connected peer the same list of configured Certificate Authorities.

Modifications:

Correctly set the CA list.

Result:

Correct and same behaviour as the JDK implementation.
2017-01-12 08:05:15 +01:00
Norman Maurer
dd055c01c7 Ensure ReferenceCountedOpenSslEngine not swallow the close_notify
Motivation:

We need to ensure we not swallow the close_notify that should be send back to the remote peer. See [#6167]

Modifications:

- Only call shutdown() in closeInbound() if there is nothing pending that should be send back to the remote peer.
- Return the correct HandshakeStatus when the close_notify was received.
- Only shutdown() when close_notify was received after closeOutbound() was called.

Result:

close_notify is correctly send back to the remote peer and handled when received.
2017-01-12 07:57:37 +01:00
Roger Kapsi
68a941c091 Detecting actual Channel write idleness vs. slowness
Motivation

The IdleStateHandler tracks write() idleness on message granularity but does not take into consideration that the client may be just slow and has managed to consume a subset of the message's bytes in the configured period of time.

Modifications

Adding an optional configuration parameter to IdleStateHandler which tells it to observe ChannelOutboundBuffer's state.

Result

Fixes https://github.com/netty/netty/issues/6150
2016-12-30 17:22:07 -08:00
Norman Maurer
89cb50aefa Explicit disable support of SSL / TLS Compression
Motivation:

Our ReferenceCountedOpenSslEngine does not support compression so we should explicit disable it.
This is related to #3722.

Modifications:

Set SSL_OP_NO_COMPRESSION option.

Result:

Not use compression.
2016-12-16 08:25:38 +00:00
Norman Maurer
89e93968ac Remove usage of own Atomic*FieldUpdater in favor of JDKs
Motivation:

In later Java8 versions our Atomic*FieldUpdater are slower then the JDK implementations so we should not use ours anymore. Even worse the JDK implementations provide for example an optimized version of addAndGet(...) using intrinsics which makes it a lot faster for this use-case.

Modifications:

- Remove methods that return our own Atomic*FieldUpdaters.
- Use the JDK implementations everywhere.

Result:

Faster code.
2016-12-15 08:09:06 +00:00
Norman Maurer
2055f4cf12 Correctly handle the case when BUFFER_OVERFLOW happens during unwrap but the readable bytes are bigger then the expected applicationBufferSize
Motivation:

We need to ensure we handle the case when BUFFER_OVERFLOW happens during unwrap but the readable bytes are bigger then the expected applicationBufferSize. Otherwise we may produce an IllegalArgumentException as we will try to allocate a buffer with capacity < 0.

Modifications:

- Guard against this case.
- Ensure we not double release buffer on exception when doing unwrap.

Result:

No more exception when running testsuite with java 9.
2016-12-08 08:22:47 +01:00
Norman Maurer
41ea9fa3b6 Ensure SSLErrorTest also works with boringssl
Motivation:

boringssl uses different messages for the ssl alerts which are all uppercase. As we try to match case as well this fails in SSLErrorTest as we expect lower-case.

This test was introduced by 9b7fb2f362.

Modifications:

Ensure we first translate everything to lower-case before doing the assert.

Result:

SSLErrorTest also pass when boringssl is used.
2016-12-07 16:09:12 +01:00
Norman Maurer
c2f4daa739 Fix false-positives when using ResourceLeakDetector.
Motivation:

We need to ensure the tracked object can not be GC'ed before ResourceLeak.close() is called as otherwise we may get false-positives reported by the ResourceLeakDetector. This can happen as the JIT / GC may be able to figure out that we do not need the tracked object anymore and so already enqueue it for collection before we actually get a chance to close the enclosing ResourceLeak.

Modifications:

- Add ResourceLeakTracker and deprecate the old ResourceLeak
- Fix some javadocs to correctly release buffers.
- Add a unit test for ResourceLeakDetector that shows that ResourceLeakTracker has not the problems.

Result:

No more false-positives reported by ResourceLeakDetector when ResourceLeakDetector.track(...) is used.
2016-12-04 09:01:39 +01:00
Norman Maurer
7ce0f35b69 Correctly not try to call handshake() when engine is already closed.
Motivation:

We need to ensure we not call handshake() when the engine is already closed. Beside this our implementation of isOutboundDone() was not correct as it not took the pending data in the outbound buffer into acount (which may be also generated as part of an ssl alert). Beside this we also called SSL_shutdown(...) while we were still in init state which will produce an error and so noise in the log with openssl later versions.

This is also in some extend related to #5931 .

Modifications:

- Ensure we not call handshake() when already closed
- Correctly implement isOutboundDone()
- Not call SSL_shutdown(...) when still in init state
- Added test-cases

Result:

More correct behaviour of our openssl SSLEngine implementation.
2016-12-04 08:59:00 +01:00
Norman Maurer
0ca2c3016b Correct guard against non SSL data in ReferenceCountedOpenSslEngine
Motivation:

When non SSL data is passed into SSLEngine.unwrap(...) we need to throw an SSLException. This was not done at the moment. Even worse we threw an IllegalArgumentException as we tried to allocate a direct buffer with capacity of -1.

Modifications:

- Guard against non SSL data and added an unit test.
- Make code more consistent

Result:

Correct behaving SSLEngine implementation.
2016-12-04 08:48:52 +01:00
Norman Maurer
f332a00e1a Support compiling netty with Java9
Motivation:

Java9 will be released soon so we should ensure we can compile netty with Java9 and run all our tests. This will help to make sure Netty will be usable with Java9.

Modification:

- Add some workarounds to be able to compile with Java9, note that the full profile is not supported with Java9 atm.
- Remove some usage of internal APIs to be able to compile on java9
- Not support Alpn / Npn and so not run the tests when using Java9 for now. We will do a follow up PR to add support.

Result:

Its possible to build netty and run its testsuite with Java9.
2016-12-03 20:12:56 +01:00
Norman Maurer
6dbec4181c Ensure we not complete the same promise that may be failed because of outbound handler exception.
Motivation:

It's important that we do not pass in the original ChannelPromise to safeClose(...) as when flush(...) will throw an Exception it will be propagated to the AbstractChannelHandlerContext which will try to fail the promise because of this. This will then fail as it was already completed by safeClose(...).

Modifications:

Create a new ChannelPromise and pass it to safeClose(...).

Result:

No more confusing logs because of failing to fail the promise.
2016-12-01 21:20:03 +01:00
Scott Mitchell
55c291ae5b SslHandlerTest ByteBuf leak
Motivation:
We are now more careful to flush alerts that are generated when errors occur. We should also be more careful in unit tests to release any buffers that may be queued due to potential errors wich result in alerts.

Modifications:
- When SslHandlerTest uses EmbeddedChannel we should always call finishAndReleaseAll

Results:
Fixes https://github.com/netty/netty/issues/6057
2016-12-01 06:50:44 +01:00
Norman Maurer
f289ebf7fa Ensure alert is send when SSLException happens during calling SslHandler.unwrap(...)
Motivation:

When the SslHandler.unwrap(...) (which is called via decode(...)) method did produce an SSLException it was possible that the produced alert was not send to the remote peer. This could lead to staling connections if the remote peer did wait for such an alert and the connection was not closed.

Modifications:

- Ensure we try to flush any pending data when a SSLException is thrown during unwrapping.
- Fix SniHandlerTest to correct test this
- Add explicit new test in SslHandlerTest to verify behaviour with all SslProviders.

Result:

The alert is correctly send to the remote peer in all cases.
2016-11-21 20:36:44 +01:00
Norman Maurer
9b7fb2f362 Use the correct alert depending on the CertificateException when using OpenSslEngine
Motivation:

We tried to detect the correct alert to use depending on the CertificateException that is thrown by the TrustManager. This not worked all the time as depending on the TrustManager implementation it may also wrap a CertPathValidatorException.

Modification:

- Try to unwrap the CertificateException if needed and detect the right alert via the CertPathValidatorException.
- Add unit to verify

Result:

Send the correct alert depending on the CertificateException when using OpenSslEngine.
2016-11-21 07:51:04 +01:00
Norman Maurer
0bc30a123e Eliminate usage of releaseLater(...) to reduce memory usage during tests
Motiviation:

We used ReferenceCountUtil.releaseLater(...) in our tests which simplifies a bit the releasing of ReferenceCounted objects. The problem with this is that while it simplifies stuff it increase memory usage a lot as memory may not be freed up in a timely manner.

Modifications:

- Deprecate releaseLater(...)
- Remove usage of releaseLater(...) in tests.

Result:

Less memory needed to build netty while running the tests.
2016-11-18 09:34:11 +01:00
nmittler
9725a4d004 Restructuring SslHandler to support new engines
Motivation:

In preparation for support of Conscrypt, I'm consolidating all of the engine-specific details so that it's easier to add new engine types that affect the behavior of SslHandler.

Modifications:

Added an enum SslEngineType that provides SSL engine-specific details.

Result:

SslHandler is more extensible for other engine types.
2016-11-17 20:12:40 +01:00
Norman Maurer
86bbf242b4 [#5874] [#5971] Ensure SniHandlerTest.testServerNameParsing not fails with SslProvider.JDK
Motivation:

The SniHandlerTest.testServerNameParsing did fail when SslProvider.JDK was used as it the JDK SSLEngineImpl does not send an alert.

Modifications:

Ensure tests pass with JDK and OPENSSL ssl implementations.

Result:

SniHandlerTest will run with all SslProvider and not fail when SslProvider.JDK is used.
2016-11-16 07:59:30 +01:00
Norman Maurer
c8575c42d0 [#5976] Ensure we only consume as much data on wrap(...) as we can handle.
Motiviation:

We need to ensure we only consume as much da as we can maximal put in one ssl record to not produce a BUFFER_OVERFLOW when calling wrap(...).

Modification:

- Limit the amount of data that we consume based on the maximal plain text size that can be put in one ssl record
- Add testcase to verify the fix
- Tighten up testcases to ensure the amount of produced and consumed data in SslEngineResult matches the buffers. If not the tests will fail now.

Result:

Correct and conform behavior of OpenSslEngine.wrap(...) and better test coverage during handshaking in general.
2016-11-15 09:39:03 +01:00
Scott Mitchell
c1932a8537 ByteBuf Input Stream Reference Count Ownership
Motivation:
Netty provides a adaptor from ByteBuf to Java's InputStream interface. The JDK Stream interfaces have an explicit lifetime because they implement the Closable interface. This lifetime may be differnt than the ByteBuf which is wrapped, and controlled by the interface which accepts the JDK Stream. However Netty's ByteBufInputStream currently does not take reference count ownership of the underlying ByteBuf. There may be no way for existing classes which only accept the InputStream interface to communicate when they are done with the stream, other than calling close(). This means that when the stream is closed it may be appropriate to release the underlying ByteBuf, as the ownership of the underlying ByteBuf resource may be transferred to the Java Stream.

Motivation:
- ByteBufInputStream.close() supports taking reference count ownership of the underyling ByteBuf

Result:
ByteBufInputStream can assume reference count ownership so the underlying ByteBuf can be cleaned up when the stream is closed.
2016-11-14 16:29:55 -08:00
Norman Maurer
fc3c9c9523 Let OpenSslEngine.wrap(...) / OpenSslEngine.unwrap(...) behave like stated in the javadocs.
Motivation:

OpenSslEngine.wrap(...) and OpenSslEngie.unwrap(...) may consume bytes even if an BUFFER_OVERFLOW / BUFFER_UNDERFLOW is detected. This is not correct as it should only consume bytes if it can process them without storing data between unwrap(...) / wrap(...) calls. Beside this it also should only process one record at a time.

Modifications:

- Correctly detect BUFFER_OVERFLOW / BUFFER_UNDERFLOW and only consume bytes if non of them is detected.
- Only process one record per call.

Result:

OpenSslEngine behaves like stated in the javadocs of SSLEngine.
2016-11-11 20:21:40 +01:00
Norman Maurer
705e3f629a Not use InternalThreadLocalMap where access may be done from outside the EventLoop.
Motivation:

We should not use the InternalThreadLocalMap where access may be done from outside the EventLoop as this may create a lot of memory usage while not be reused anyway.

Modifications:

Not use InternalThreadLocalMap in places where the code-path will likely be executed from outside the EventLoop.

Result:

Less memory bloat.
2016-11-10 14:37:16 +01:00
Evgeny Slutsky
4ee361e302 OpenSslSession#initPeerCerts creates too long almost empty arrays.
Motivation:

https://github.com/netty/netty/issues/5945

Modifications:

Refactored initialization of arrays. Fixed arrays length

Result:

Cert arrays have proper length. Testing added
2016-11-03 12:10:39 +01:00
Norman Maurer
5f533b7358 [maven-release-plugin] prepare for next development iteration 2016-10-14 13:20:41 +02:00
Norman Maurer
35fb0babe2 [maven-release-plugin] prepare release netty-4.1.6.Final 2016-10-14 12:47:19 +02:00
Trustin Lee
0b7bf49c16 Convert X509TrustManager into X509ExtendedTrustManager for Java 7+
Motivation:

Since Java 7, X509TrustManager implementation is wrapped by a JDK class
called AbstractTrustManagerWrapper, which performs an additional
certificate validation for Socket or SSLEngine-backed connections.

This makes the TrustManager implementations provided by
InsecureTrustManagerFactory and FingerprintTrustManagerFactory not
insecure enough, where their certificate validation fails even when it
should pass.

Modifications:

- Add X509TrustManagerWrapper which adapts an X509TrustManager into an
  X509ExtendedTrustManager
- Make SimpleTrustManagerFactory wrap an X509TrustManager with
  X509TrustManagerWrapper is the provided TrustManager does not extend
  X509ExtendedTrustManager

Result:

- InsecureTrustManagerFactory and FingerprintTrustManagerFactory are now
  insecure as expected.
- Fixes #5910
2016-10-12 13:46:02 +02:00
Scott Mitchell
8ba5b5f740 Update Default Cipher List
Motivation:
Our default cipher list has not been updated in a while. We current support some older ciphers not commonly in use and we don't support some newer ciphers which are more commonly used.

Modifications:
- Update the default list of ciphers for JDK and OpenSSL.

Result:
Default cipher list is more likely to connect to peers.
Fixes https://github.com/netty/netty/issues/5859
2016-10-11 07:50:17 -07:00
Norman Maurer
39997814bf [#5860] Ensure removal of SslHandler not produce IllegalReferenceCountException
Motivation:

If the user removes the SslHandler while still in the processing loop we will produce an IllegalReferenceCountException. We should stop looping when the handlerwas removed.

Modifications:

Ensure we stop looping when the handler is removed.

Result:

No more IllegalReferenceCountException.
2016-10-10 11:06:19 +02:00
Norman Maurer
1c588ce7e9 [#5841] Add test-case for mutual auth when using certificate chain
Motivation:

Add test-case for doing mutal auth with a certificate chain that holds more then one certificate.

Modifications:

Add test case

Result:

more tests.
2016-09-30 10:46:11 +02:00
radai-rosenblatt
15ac6c4a1f Clean-up unused imports
Motivation:

the build doesnt seem to enforce this, so they piled up

Modifications:

removed unused import lines

Result:

less unused imports

Signed-off-by: radai-rosenblatt <radai.rosenblatt@gmail.com>
2016-09-30 09:08:50 +02:00
olim7t
3a8b8c9219 Consolidate flushes even when no read in progress
Motivation:

Currently FlushConsolidationHandler only consolidates if a read loop is
active for a Channel, otherwise each writeAndFlush(...) call will still
be flushed individually. When these calls are close enough, it can be
beneficial to consolidate them even outside of a read loop.

Modifications:

When we allow a flush to "go through", don't perform it immediately, but
submit it on the channel's executor. Under high pressure, this gives
other writes a chance to enqueue before the task gets executed, and so
we flush multiple writes at once.

Result:

Lower CPU usage and less context switching.
2016-09-23 15:27:03 -07:00
Roger Kapsi
9a18159bfe Catch+fire Exceptions thrown from SniHandler's select() method
Motivation

Give the user the ability to back out from SNI negoations.

Modifications

Put a try-catch around the select() call and re-fire any caught Exceptions.

Result

Fixes #5787
2016-09-19 16:10:24 -07:00
Scott Mitchell
0b5e75a614 IdleStateHandler volatile member variables
Motivation:
IdleStateHandler has a number of volatile member variables which are only accessed from the EventLoop thread. These do not have to be volatile. The accessibility of these member variables are not consistent between private and package private. The state variable can also use a byte instead of an int.

Modifications:
- Remove volatile from member variables
- Change access to private for member variables
- Change state from int to byte

Result:
IdleStateHandler member variables cleaned up.
2016-09-15 10:27:29 -07:00
Norman Maurer
4a18a143d2 Only set lastReadTime if an read actually happened before in IdleStateHandler.
Motivation:

IdleStateHandler and ReadTimeoutHandler could mistakely not fire an event even if no channelRead(...) call happened.

Modifications:

Only set lastReadTime if a read happened before.

Result:

More correct IdleStateHandler / ReadTimeoutHandler.
2016-09-13 16:57:29 -07:00
Victor Noël
8566fd1019 Add startTls parameter to SslContextBuilder
Motivation:

There is an incoherence in terms of API when one wants to use
startTls: without startTls one can use the SslContextBuilder's
method newHandler, but with startTls, the developper is forced
to call directly the SslHandler constructor.

Modifications:

Introduce startTls as a SslContextBuilder parameter as well as a
member in SslContext (and thus Jdk and OpenSsl implementations!).
Always use this information to call the SslHandler constructor.
Use false by default, in particular in deprecated constructors of
the SSL implementations.
The client Context use false by default

Results:

Fixes #5170 and more generally homogenise the API so that
everything can be done via SslContextBuilder.
2016-09-06 11:29:10 +02:00
Roger Kapsi
b604a22395 Expose the ChannelHandlerContext from SniHandler's select() step to the user.
Motivation

I'm looking to harden our SSL impl. a little bit and add some guards agaist certain types of abuse. One can think of invalid hostname strings in the SNI extenstion or invalid SNI handshakes altogether. This will require measuring, velocity tracking and other things.

Modifications

Adding a protected `lookup(ctx, hostname)` method that is called from SniHandler's `select(...)` method which users can override and implement custom behaviour. The default implementation will simply call the AsyncMapper.

Result

It's possible to get a hold onto the ChannelHandlerContext. Users can override that method and do something with it right there or they can delegate it to something else. SniHandler is happy as long as a `Future<SslContext>` is being returned.
2016-09-06 07:29:12 +02:00
Norman Maurer
bd7806dd4f Ensure SSLEngineTest works on different jvm versions.
Motivation:

af632278d2 introduced a test which only worked on some jvm versions and specific os'es.

Modifications:

Fix test to work on different java versions and os'es

Result:

No flacky test.
2016-09-05 21:07:48 +02:00
Norman Maurer
af632278d2 Ensure we not sent duplicate certificates when using OpenSslEngine
Motivation:

We need to ensure we not set duplicated certificates when using OpenSslEngine.

Modifications:

- Skip first cert in chain when set the chain itself and so not send duplicated certificates
- Add interopt unit tests to ensure no duplicates are send.

Result:

No more duplicates.
2016-09-05 15:05:17 +02:00
Norman Maurer
147d427adc [#5712] Allow clients to override userDefinedWritabilityIndex from AbstractTrafficShapingHandler
Motivation:

AbstractTrafficShapingHandler has a package-private method called "userDefinedWritabilityIndex()" which a user may need to override if two sub-classes wants to be used in the ChannelPipeline.

Modifications:

Mark method protected.

Result:

Easier to extend AbstractTrafficShapingHandler.
2016-09-05 12:29:40 +02:00
Norman Maurer
54b1a100f4 [maven-release-plugin] prepare for next development iteration 2016-08-26 10:06:32 +02:00
Norman Maurer
31cbb85073 Cleanup and simplify SslHandler
Motivation:

SslHandler can be cleaned up a bit in terms of naming and duplicated code.

Modifications:

- Fix naming of arguments
- Not schedule timeout event if not really needed
- share some code and simplify

Result:

Cleaner code.
2016-09-01 08:32:35 +02:00
Norman Maurer
3051df8961 Ensure accessing System property is done via AccessController.
Motivation:

When a SecurityManager is in place it may dissallow accessing the property which will lead to not be able to load the application.

Modifications:

Use AccessController.doPrivileged(...)

Result:

No more problems with SecurityManager.
2016-08-31 14:01:05 +02:00
Roger Kapsi
f97866dbc6 Expose SniHandler's replaceHandler() so that users can implement custom behavior.
Motivation

The SniHandler is currently hiding its replaceHandler() method and everything that comes with it. The user has no easy way of getting a hold onto the SslContext for the purpose of reference counting for example. The SniHandler does have getter methods for the SslContext and hostname but they're not very practical or useful. For one the SniHandler will remove itself from the pipeline and we'd have to track a reference of it externally and as we saw in #5745 it'll possibly leave its internal "selection" object with the "EMPTY_SELECTION" value (i.e. we've just lost track of the SslContext).

Modifications

Expose replaceHandler() and allow the user to override it and get a hold onto the hostname, SslContext and SslHandler that will replace the SniHandler.

Result

It's possible to get a hold onto the SslContext, the hostname and the SslHandler that is about to replace the SniHandler. Users can add additional behavior.
2016-08-29 13:18:54 -07:00
Norman Maurer
1208b90f57 [maven-release-plugin] prepare release netty-4.1.5.Final 2016-08-26 04:59:35 +02:00
Norman Maurer
1fef3872fb Update CertificateRequestCallback usage to match new tcnative
Motiviation:

Previously the way how CertificateRequestCallback was working had some issues which could cause memory leaks and segfaults. Due of this tcnative code was updated to change the signature of the method provided by the interface.

Modifications:

Update CertificateRequestCallback implementations to match new interface signature.

Result:

No more segfaults / memory leaks when using boringssl or openssl >= 1.1.0
2016-08-28 20:26:15 +02:00
Roger Kapsi
3a6157e2aa Unit Test for SslHandler's handlerRemoved()
Motivation

SslHandler's handlerRemoved() is supposed to release the SSLEngine (which it does) but there is no Test for it to make sure it really happens and doesn't unexpectedly change in the future.

Modifications

Add a Unit Test that makes sure that SslHandler releases the SSLEngine when the Channel gets closed.

Result

Assurance that SslHandler will not leak (ReferenceCounted) SSLEngines.
2016-08-27 20:50:35 +02:00
Scott Mitchell
acb40a87c3 SniHandler reference count leak if pipeline replace fails
Motivation:
The SniHandler attempts to generate a new SslHandler from the selected SslContext in a and insert that SslHandler into the pipeline. However if the underlying channel has been closed or the pipeline has been modified the pipeline.replace(..) operation may fail. Creating the SslHandler may also create a SSLEngine which is of type ReferenceCounted. The SslHandler states that if it is not inserted into a pipeline that it will not take reference count ownership of the SSLEngine. Under these conditions we will leak the SSLEngine if it is reference counted.

Modifications:
- If the pipeline.replace(..) operation fails we should release the SSLEngine object.

Result:
Fixes https://github.com/netty/netty/issues/5678
2016-08-25 13:08:59 -07:00
Norman Maurer
2d111e0980 Ensure SslHandler.close(...) will not throw exception if flush of pending messages fails
Motivation:

When SslHandler.close(...) is called (as part of Channel.close()). it will also try to flush pending messages. This may fail for various reasons, but we still should propergate the close operation

Modifications:

- Ensure flush(...) itself will not throw an Exception if we was able to at least fail one pending promise (which should always be the case).
- If flush(...) fails as part of close ensure we still close the channel and then rethrow.

Result:

No more lost close operations possible if an exception is thrown during close
2016-08-24 07:33:25 +02:00
Scott Mitchell
9bc3e56647 ReferenceCountedOpenSslEngineTest cleanup sequencing bug
Motivation:
ReferenceCountedOpenSslEngine depends upon the the SslContext to cleanup JNI resources. If we don't wait until the ReferenceCountedOpenSslEngine is done with cleanup before cleaning up the SslContext we may crash the JVM.

Modifications:
- Wait for the channels to close (and thus the ReferenceCountedOpenSslEngine to be cleaned up) before cleaning up the associated SslContext.

Result:
Cleanup sequencing is correct and no more JVM crash.
Fixes https://github.com/netty/netty/issues/5692
2016-08-18 21:03:39 +02:00
Norman Maurer
382f8eacaf Correctly fail all promises SSLENGINE_CLOSED once the engine is closed
Motivation:

We should fail all promises with the correct SSLENGINE_CLOSED exception one the engine is closed. We did not fail the current promise with this exception if the ByteBuf was not readable.

Modifications:

Correctly fail promises.

Result:

More correct handling of promises if the SSLEngine is closed.
2016-08-17 21:47:00 +02:00
Norman Maurer
fb3dc84e5b Only run PemEncodedTest if OpenSsl.useKeyManagerFactory() returns false.
Motivation:

Commit b963595988 added a unit that will not work when KeyManagerFactory is used.

Modifications:

Only run the test if OpenSsl.useKeyManagerFactory() returns false.

Result:

Builds with boringssl
2016-08-16 13:42:09 +02:00
Roger Kapsi
b963595988 Fix handling of non direct backed PemEncoded.
Motivation:

The private key and certificate that are passed into #serKeyMaterial() could be PemEncoded in which case the #toPEM() methods return the identity of the value.

That in turn will fail in the #toBIO() step because the underlying ByteBuf is not necessarily direct.

Modifications:

- Use toBIO(...) which also works with non direct PemEncoded values
- Add unit test.

Result:

Correct handling of PemEncoded.
2016-08-16 09:04:38 +02:00
Norman Maurer
00deb2efd5 Remove missleading javadoc
Motivation:

Its completely fine to start writing before the handshake completes when using SslHandler. The writes will be just queued.

Modifications:

Remove the missleading and incorrect javadoc.

Result:

Correct javadoc.
2016-08-14 13:21:34 +02:00
Norman Maurer
c1d5066929 Ensure BIO is only used one time
Motivation:

Its not safe to reuse a BIO, we need to ensure we only use it once.

Modifications:

Only use the BIO once.

Result:

Correctly usage of BIO
2016-08-12 14:41:13 +02:00
Norman Maurer
47d55339c9 [#5648] Detect if netty-tcnative is in classpath or just tcnative
Motivation:

If netty is used in a tomcat container tomcat itself may ship tcnative. Because of this we will try to use OpenSsl in netty and fail because it is different to netty-tcnative.

Modifications:

Ensure if we find tcnative it is really netty-tcnative before using it.

Result:

No more problems when using netty in a tomcat container that also has tcnative installed.
2016-08-11 14:13:05 +02:00
Norman Maurer
cb5f71782e Ensure we only call ReferenceCountUtil.safeRelease(...) in finalize() if the refCnt() > 0
Motivation:

We need to ensure we only call ReferenceCountUtil.safeRelease(...) in finalize() if the refCnt() > 0 as otherwise we will log a message about IllegalReferenceCountException.

Modification:

Check for a refCnt() > 0 before try to release

Result:

No more IllegalReferenceCountException produced when run finalize() on OpenSsl* objects that where explicit released before.
2016-08-11 08:54:19 +02:00
Scott Mitchell
fef2940f32 Update for netty-tcnative API changes
Motivation:
netty-tcnative API has changed to remove a feature that contributed to a memory leak.

Modifications:
- Update to use the modified netty-tcnative API

Result:
Netty can use the latest netty-tcnative.
2016-08-10 21:56:15 -07:00
Norman Maurer
129aee8a92 Remove unused imports and not needed throws declarations.
Motivation:

In latest refeactoring we failed to cleanup imports and also there are some throws declarations which are not needed.

Modifications:

Cleanup imports and throws declarations

Result:

Cleaner code.
2016-08-10 11:46:59 +02:00
Scott Mitchell
7d774584c8 OpenSslEngine with no finalizer
Motivation:
OpenSslEngine and OpenSslContext currently rely on finalizers to ensure that native resources are cleaned up. Finalizers require the GC to do extra work, and this extra work can be avoided if the user instead takes responsibility of releasing the native resources.

Modifications:
- Make a base class for OpenSslENgine and OpenSslContext which does not have a finalizer but instead implements ReferenceCounted. If this engine is inserted into the pipeline it will be released by the SslHandler
- Add a new SslProvider which can be used to enable this new feature

Result:
Users can opt-in to a finalizer free OpenSslEngine and OpenSslContext.
Fixes https://github.com/netty/netty/issues/4958
2016-08-05 00:57:37 -07:00
Norman Maurer
e5b45f120a Allow to explicit disable usage of KeyManagerFactory when using OpenSsl
Motivation:

Sometimes it may be useful to explicit disable the usage of the KeyManagerFactory when using OpenSsl.

Modifications:

Add io.netty.handler.ssl.openssl.useKeyManagerFactory which can be used to explicit disable KeyManagerFactory usage.

Result:

More flexible usage.
2016-08-05 07:15:37 +02:00
Norman Maurer
5513514d08 Take readerIndex into account when write to BIO.
Motivation:

We should take the readerIndex into account whe write into the BIO. Its currently not a problem as we slice before and so the readerIndex is always 0 but we should better not depend on this as this will break easily if we ever refactor the code and not slice anymore.

Modifications:

Take readerIndex into acount.

Result:

More safe and correct use.
2016-08-05 07:14:16 +02:00
Norman Maurer
6bd810210d Servers should not send duplicate intermediate certificates.
Motivation:
Servers sometimes send duplicate intermediate certificates.

Modifications:
OpenSslKeyMaterialManager.setKeyMaterial() dedups aliases before calling SSL.setCertificateChainBio().

Result:
Servers no longer send duplicate itermediate certificates.
2016-08-01 10:52:46 +02:00
Norman Maurer
f585806a74 [#5598] Ensure SslHandler not log false-positives when try to close the channel due timeout.
Motivation:

When we try to close the Channel due a timeout we need to ensure we not log if the notification of the promise fails as it may be completed in the meantime.

Modifications:

Add another constructor to ChannelPromiseNotifier and PromiseNotifier which allows to log on notification failure.

Result:

No more miss-leading logs.
2016-07-30 21:15:09 +02:00
Norman Maurer
cb7cf4491c [maven-release-plugin] prepare for next development iteration 2016-07-27 13:29:56 +02:00
Norman Maurer
9466b32d05 [maven-release-plugin] prepare release netty-4.1.4.Final 2016-07-27 13:16:59 +02:00
Scott Mitchell
cebf255951 FlushConsolidationHandler remove conditional
Motivation:
FlushConsolidationHandler#flushIfNeeded has a conditional which is fixed based upon code path. This conditional can be removed and instead just manually set in each fixed code path.

Modifications:
- Remove boolean parameter on FlushConsolidationHandler#flushIfNeeded and set readInprogess to false manually when necessary

Result:
Less conditionals in FlushConsolidationHandler
2016-07-27 07:11:47 +02:00
Ian Haken
2ce1d29d4d Elliminated some buggy behavior when using a KeyManagerFactory with OpenSslServerContext.
Motivation:

PR #5493 added support for KeyManagerFactories when using the OpenSsl context. This commit corrects a bug causing a NullPointerException that occurs when using a KeyManagerFactory without a certificate chain and private key.

Modifications:

Removes assertNotNull() assertions which were causing a certificate chain and private key to be required even when using a KeyManagerFactory. Also removed a redundant call to buildKeyManagerFactory() which was also causing a exception when a KeyManagerFactory is provided but a certificate chain and private key is not.

Result:

A KeyManagerFactory can now be used in the OpenSslServerContext without an independent certificate chain and private key.
2016-07-22 09:14:54 +02:00
Norman Maurer
047f6aed28 [maven-release-plugin] prepare for next development iteration 2016-07-15 09:09:13 +02:00
Norman Maurer
b2adea87a0 [maven-release-plugin] prepare release netty-4.1.3.Final 2016-07-15 09:08:53 +02:00
Norman Maurer
e3c8a92499 Add FlushConsolidationHandler which consolidates flush operations as these are expensive
Motivation:

Calling flush() and writeAndFlush(...) are expensive operations in the sense as both will produce a write(...) or writev(...) system call if there are any pending writes in the ChannelOutboundBuffer. Often we can consolidate multiple flush operations into one if currently a read loop is active for a Channel, as we can just flush when channelReadComplete is triggered. Consolidating flushes can give a huge performance win depending on how often is flush is called. The only "downside" may be a bit higher latency in the case of where only one flush is triggered by the user.

Modifications:

Add a FlushConsolidationHandler which will consolidate flushes and so improve the throughput.

Result:

Better performance (throughput). This is especially true for protocols that use some sort of PIPELINING.
2016-07-07 06:48:23 +02:00
Norman Maurer
987ebb90ec Share code between ReadTimeoutHandler and IdleStateHandler
Motivation:

ReadTimeoutHandler and IdleStateHandler have duplicated code, we should share whatever possible.

Modifications:

Let ReadTimeoutHandler extend IdleStateHandler.

Result:

Remove code duplication.
2016-07-05 20:12:37 +02:00
Scott Mitchell
5c124ae8e2 OpenSslEngine writePlaintextData WANT_READ with no data in BIO buffer unit test
Motivation:
Unit test for the OpenSslEngine "OpenSslEngine writePlaintextData WANT_READ with no data in BIO buffer" issue.

Modifications:
- Update SslEngine test to include renegotiation

Result:
More test coverage in OpenSslEngine.
2016-07-01 11:58:08 -07:00
Norman Maurer
4676a2271c [maven-release-plugin] prepare for next development iteration 2016-07-01 10:33:32 +02:00
Norman Maurer
ad270c02b9 [maven-release-plugin] prepare release netty-4.1.2.Final 2016-07-01 09:07:40 +02:00
buchgr
f3dc483c99 Fix NPE in OpenSslEngine
Motivation:

The gRPC interop tests fail due to a NPE in OpenSslEngine.

Caused by: java.lang.NullPointerException
at io.netty.handler.ssl.OpenSslEngine.setSSLParameters(OpenSslEngine.java:1473)

Modifications:

Add a null check

Result:

No more NPE exceptions :-)
2016-06-30 11:12:55 +02:00
Norman Maurer
5e64985089 Add support for KeyManagerFactory when using SslProvider.OpenSsl.
Motivation:

To be able to use SslProvider.OpenSsl with existing java apps that use the JDK SSL API we need to also provide a way to use it with an existing KeyManagerFactory.

Modification:

Make use of new tcnative apis and so hook in KeyManagerFactory.

Result:

SslProvider.OpenSsl can be used with KeyManagerFactory as well.
2016-06-28 09:34:01 +02:00
Norman Maurer
3a69adfefb [#5401] Support -Djdk.tls.ephemeralDHKeySize=num when using OpenSslContext
Motivation:

Java8+ adds support set a DH key size via a System property (jdk.tls.ephemeralDHKeySize). We should respect this when using OpenSSL.

Modifications:

Respect system property.

Result:

More consistent SSL implementation.
2016-06-28 07:07:42 +02:00
Norman Maurer
2546d99864 Expose session ticket statistics.
Motivation:

We recently added support for session ticket statistics which we can expose now.

Modifications:

Expose the statistics.

Result:

Be able to obtain session ticket statistics.
2016-06-27 19:36:45 +02:00
Norman Maurer
77c6d7a672 Correctly implement SSLSession.getLastAccessedTime() for OpenSSLEngine
Motivation:

We need to return a correct time for SSLSession.getLastAccessedTime() so it reflect when the handshake was done when the session was reused.

Modifications:

Correctly reflect handshake time in getLastAccessedTime().

Result:

More conform SSLSession implementation.
2016-06-23 11:30:28 +02:00
Norman Maurer
d495792c48 Allow to wrap another SslContext implementation and do extra init steps on the SSLEngine.
Motivation:

Sometimes its needed to customize the SSLEngine (like setting protocols etc). For this it would be useful if the user could wrap an SslContext and do init steps on the SSLEngine.

Modifications:

Add new SslContext implementation which can wrap another one and allow to customize the SSLEngine

Result:

More flexible usage of SslContext.
2016-06-23 11:28:52 +02:00
Norman Maurer
7c1374dd54 OpenSslEngine.getSupportedCipherSuites() must return java names as well.
Motivation:

At the moment OpenSslEngine.getSupportedCipherSuites() only return the original openssl cipher names and not the java names. We need also include the java names.

Modifications:

Correctly return the java names as well.

Result:

Correct implementation of OpenSslEngine.getSupportedCipherSuites()
2016-06-23 11:27:23 +02:00
Tim Brooks
d964bf6f18 Remove usages of deprecated methods group() and childGroup().
Motivation:

These methods were recently deprecated. However, they remained in use in several locations in Netty's codebase.

Modifications:

Netty's code will now access the bootstrap config to get the group or child group.

Result:

No impact on functionality.
2016-06-21 14:06:57 +02:00
Norman Maurer
9687d77b5a Move validation of arguments out of synchronized block
Motivation:

There is no need already use synchronized when validate the args of the methods.

Modifications:

First validate arguments and then use synchronized

Result:

Less code executed in synchronized block.
2016-06-20 14:23:47 +02:00
Norman Maurer
e845670043 Set some StackTraceElement on pre-instantiated static exceptions
Motivation:

We use pre-instantiated exceptions in various places for performance reasons. These exceptions don't include a stacktrace which makes it hard to know where the exception was thrown. This is especially true as we use the same exception type (for example ChannelClosedException) in different places. Setting some StackTraceElements will provide more context as to where these exceptions original and make debugging easier.

Modifications:

Set a generated StackTraceElement on these pre-instantiated exceptions which at least contains the origin class and method name. The filename and linenumber are specified as unkown (as stated in the javadocs of StackTraceElement).

Result:

Easier to find the origin of a pre-instantiated exception.
2016-06-20 11:33:05 +02:00
Norman Maurer
f2efd68c39 Correctly support SSLSession.getId() when using OpenSslEngine
Motivation:

At the moment SSLSession.getId() may always return an empty byte array when OpenSSLEngine is used. This is as we not set SSL_OP_NO_TICKET on the SSLContext and so SSL_SESSION_get_id(...) will return an session id with length of 0 if tickets are not used.

Modifications:

- Set SSL_OP_NO_TICKET by default and only clear it if the user requests the usage of session tickets.
- Add unit test

Result:

Ensure consistent behavior between different SSLEngine implementations.
2016-06-20 09:34:54 +02:00
Norman Maurer
e4d21abfc1 Add support for SSLParameters.setCipherSuiteOrder() when using Java8+
Motivation:

When using java8+ we should support SSLParameters.setCipherSuiteOrder()

Modifications:

Add support of SLParameters.setCipherSuiteOrder() by using reflection, so we can compile with java7 but still support it.

Result:

Users that use java8+ can use SSLParameters.setCipherSuiteOrder()
2016-06-20 09:33:49 +02:00
Norman Maurer
e14a385a88 Add support for SNIHostName when using Java8+
Motivation:

Java8 added support for using SNIHostName with SSLParameters. We currently ignore it in OpenSslEngine.

Modifications:

Use reflection to support SNIHostName.

Result:

People using Java8 can use SNIHostName even when OpenSslEngine is used.
2016-06-20 09:25:12 +02:00
Norman Maurer
87d9ecc2c9 Correctly skip OpenSsl* tests if OpenSsl.isAvailable() is false.
Motivation:

We missed to skip some tests for OpenSsl when OpenSsl.isAvailable() is false.

Modifications:

- Correctly skip tests when OpenSsl.isAvailable() is false.
- Simplify some code by using @BeforeClass.

Result:

Be able to compile netty even when OpenSsl is not present on the system.
2016-06-17 08:35:57 +02:00
Norman Maurer
65d1fb474d Guard against possible segfault when OpenSslContext is gc'ed and user still hold reference to OpenSslSessionContext / OpenSslSessionStats
Motivation:

When the OpenSslContext is gc'ed and the user still hold a reference to OpenSslSessionContext / OpenSslSessionStats it is possible to produce a segfault when calling
a method on any of these that tries to pass down the ctx pointer to the native methods. This is because the OpenSslContext finalizer will free the native pointer.

Modifications:

Change OpenSslSessionContext / OpenSslSessionContext to store a reference to OpenSslContext and so prevent the GC to collect it as long as the user has a reference to OpenSslSessionContext / OpenSslSessionContext.

Result:

No more sefault possible.
2016-06-17 08:33:09 +02:00
Roger Kapsi
fe569ea7a3 Fix for a newly intrduced bug in #5377
Motivation

This bug was introduced with #5377 and affects only users who'd like to share/cache/re-use `PemPrivateKey` and `PemX509Certificate` instances.

Modifications

Use `ByteBuf#writeBytes(src, readerIndex, length)` so that the src's readerIndex doesn't change and can consequently be used more than once.

Result

It's possible to share/cache/re-use `PemPrivateKey` and `PemX509Certificate` instances as long as their refCnt remains >= 1.
2016-06-13 20:28:17 +02:00
Scott Mitchell
a7496ed83d FlowControlHandlerTest synchronization issues
Motivation:
2b65258568 only partially addressed the synchronization issues that are present in FlowControlHandlerTest. A few tests are attempting to validate state changes made across an EventLoop thread and the JUnit thread but are not properly synchronized.

Modifications:
- Ensure that conditions which verify expectations set in another thread have synchronization gates to ensure the event has actually occurred.
- Remove the message counter verification in favor of using individual CountDownLatch objects

Result:
FLowControlHanderTest has less race conditions which may lead to test failures.
2016-06-13 14:13:40 +02:00
Roger Kapsi
cc580e3ba1 Let OpenSslContext take pre-encoded pkcs#8 private key/cert bytes
Motivation

OpenSslContext is expecting Java's PrivateKey and X509Certificate objects as input
(for JdkSslContext API compatibility reasons) but doesn't really use them beyond
turning them into PEM/PKCS#8 strings.

This conversion can be entirely skipped if the user can pass in private keys and
certificates in a format that Netty's OpenSSL code can digest.

Modifications

Two new classes have been added that act as a wrapper around the pre-encoded byte[]
and also retain API compatibility to JdkSslContext.

Result

It's possible to pass PEM encoded bytes straight into OpenSSL without having to
parse them (e.g. File to Java's PrivateKey) and then encode them (i.e. PrivateKey
into PEM/PKCS#8).

File pemPrivateKeyFile;
byte[] pemBytes = readBytes(pemPrivateKeyFile);
PemPrivateKey pemPrivateKey = PemPrivateKey.valueOf(pemBytes);

SslContextBuilder.forServer(pemPrivateKey)
    .sslProvider(SslProvider.OPENSSL)
2016-06-10 18:07:40 +02:00
Guido Medina
c3abb9146e Use shaded dependency on JCTools instead of copy and paste
Motivation:
JCTools supports both non-unsafe, unsafe versions of queues and JDK6 which allows us to shade the library in netty-common allowing it to stay "zero dependency".

Modifications:
- Remove copy paste JCTools code and shade the library (dependencies that are shaded should be removed from the <dependencies> section of the generated POM).
- Remove usage of OneTimeTask and remove it all together.

Result:
Less code to maintain and easier to update JCTools and less GC pressure as the queue implementation nt creates so much garbage
2016-06-10 13:19:45 +02:00
Norman Maurer
88dbd96376 [#5372] Ensure OpenSslClientContext / OpenSslServerContext can be garbage collected
Motivation:

OpenSslClientContext / OpenSslServerContext can never be garbage collected as both are part of a reference to a callback that is stored as global reference in jni code.

Modifications:

Ensure the callbacks are static and so not hold the reference.

Result:

No more leak due not collectable OpenSslClientContext / OpenSslServerContext
2016-06-09 22:37:13 +02:00
Scott Mitchell
783567420f OpenSslEngine encrypt more data per wrap call
Motivation:
OpenSslEngine.wrap will only encrypt at most 1 buffer per call. We may be able to encrypt multiple buffers per call.

Modifications:
- OpensslEngine.wrap should continue encrypting data until there is an error, no more data, or until the destination buffer would be overflowed.

Result:
More encryption is done per OpenSslEngine.wrap call
2016-06-08 12:23:19 -07:00
Norman Maurer
4dec7f11b7 [maven-release-plugin] prepare for next development iteration 2016-06-07 18:52:34 +02:00
Norman Maurer
cf670fab75 [maven-release-plugin] prepare release netty-4.1.1.Final 2016-06-07 18:52:22 +02:00
Scott Mitchell
9e2c400f89 OpenSslEngine writePlaintextData WANT_READ with no data in BIO buffer
Motivation:
CVE-2016-4970

OpenSslEngine.wrap calls SSL_write which may return SSL_ERROR_WANT_READ, and if in this condition there is nothing to read from the BIO the OpenSslEngine and SslHandler will enter an infinite loop.

Modifications:
- Use the error code provided by OpenSSL and go back to the EventLoop selector to detect if the socket is closed

Result:
OpenSslEngine correctly handles the return codes from OpenSSL and does not enter an infinite loop.
2016-06-07 08:59:13 -07:00
Scott Mitchell
f7cf00cb5f OpenSslEngine remove unecessary rejectRemoteInitiatedRenegation call
Motivation:
OpenSslEngine calls rejectRemoteInitiatedRenegation in a scenario where the number of handshakes has not been observed to change. The number of handshakes has only been observed to change after readPlaintextData is called.

Modifications:
- Remove the call to rejectRemoteInitiatedRenegation before calls to readPlaintextData

Result:
Less code.
2016-06-03 13:02:01 -07:00
floragunn
528b83e277 Set the session id context properly to make client authentication work with open ssl provider.
Motivation:

When netty is used with open ssl provider and client authentication the following errors can occur:
error:140D9115:SSL routines:ssl_get_prev_session:session id context uninitialized
error:140A1175:SSL routines:ssl_bytes_to_cipher_list:inappropriate fallback
error:140760FC:SSL routines:SSL23_GET_CLIENT_HELLO:unknown protocol

Modifications:

Set the session id context in OpenSslServerContext so that sessions which use client authentication
which are cached have the same context id value.

Result:

Client authentication now works with open ssl provider.
2016-05-28 21:29:40 +02:00
Norman Maurer
6ca49d1336 [maven-release-plugin] prepare for next development iteration 2016-05-25 19:16:44 +02:00
Norman Maurer
446b38db52 [maven-release-plugin] prepare release netty-4.1.0.Final 2016-05-25 19:14:15 +02:00
Trustin Lee
f11a35af32 Replace DomainMappingBuilder with DomainNameMappingBuilder
Motivation:

DomainMappingBuilder should have been named as DomainNameMappingBuilder
because it builds a DomainNameMapping.

Modifications:

- Add DomainNameMappingBuilder that does the same job with
  DomainMappingBuilder
- Deprecate DomainMappingBuilder and delegate its logic to
  DomainNameMappingBuilder
- Remove the references to the deprecated methods and classes related
  with domain name mapping
- Miscellaneous:
  - Fix Javadoc of DomainNameMapping.asMap()
  - Pre-create the unmodifiable map in DomainNameMapping

Result:

- Consistent naming
- Less use of deprecated API
2016-05-18 12:03:14 +02:00
Trustin Lee
3a9f472161 Make retained derived buffers recyclable
Related: #4333 #4421 #5128

Motivation:

slice(), duplicate() and readSlice() currently create a non-recyclable
derived buffer instance. Under heavy load, an application that creates a
lot of derived buffers can put the garbage collector under pressure.

Modifications:

- Add the following methods which creates a non-recyclable derived buffer
  - retainedSlice()
  - retainedDuplicate()
  - readRetainedSlice()
- Add the new recyclable derived buffer implementations, which has its
  own reference count value
- Add ByteBufHolder.retainedDuplicate()
- Add ByteBufHolder.replace(ByteBuf) so that..
  - a user can replace the content of the holder in a consistent way
  - copy/duplicate/retainedDuplicate() can delegate the holder
    construction to replace(ByteBuf)
- Use retainedDuplicate() and retainedSlice() wherever possible
- Miscellaneous:
  - Rename DuplicateByteBufTest to DuplicatedByteBufTest (missing 'D')
  - Make ReplayingDecoderByteBuf.reject() return an exception instead of
    throwing it so that its callers don't need to add dummy return
    statement

Result:

Derived buffers are now recycled when created via retainedSlice() and
retainedDuplicate() and derived from a pooled buffer
2016-05-17 11:16:13 +02:00
Norman Maurer
89eae94542 Allow to extend IdleStateHandler and so provide more details for IdleStateEvents
Motivation:

Sometimes it is useful to include more details in the IdleStateEvents that are produced by the IdleStateHandler. For this users should be able to create their own IdleStateEvents that encapsulate more informations.

Modifications:

- Make IdleStateEvent constructor protected and the class non-final
- Add protected method to IdleStateHandler that users can override and so create their own IdleStateEvents.

Result:

More flexible and customizable IdleStateEvents / IdleStateHandler
2016-05-14 07:19:08 +02:00