Commit Graph

517 Commits

Author SHA1 Message Date
Norman Maurer
0f91ad841d Fix possible testfailure due not waiting on Channel.close() (introduced by e220c56823) 2016-02-05 12:28:11 +01:00
Norman Maurer
d9f938ca03 [#4828] OpenSslContext throws UnsupportedOperationException when Unsafe not available
Motivation:

OpenSslContext constructor fails with a UnsupportedOperationException if Unsafe is not present on the system.

Modifications:

Make OpenSslContext work also when Unsafe is not present by fallback to using JNI to get the memory address.

Result:

Using OpenSslContext also works on systems without Unsafe.
2016-02-05 09:25:18 +01:00
Norman Maurer
eb1d9da76c Enable SSL_MODE_ACCEPT_MOVING_WRITE_BUFFER when using OpenSslContext
Motivation:

We need to enable SSL_MODE_ACCEPT_MOVING_WRITE_BUFFER when using OpenSslContext as the memory address of the buffer that is passed to OpenSslEngine.wrap(...) may change during calls and retries. This is the case as
if the buffer is a heap-buffer we will need to copy it to a direct buffer to hand it over to the JNI layer. When not enable this mode we may see errors like: 'error:1409F07F:SSL routines:SSL3_WRITE_PENDING: bad write retry'.
Related to https://github.com/netty/netty-tcnative/issues/100.

Modifications:

Explitict set mode to SSL.SSL_MODE_RELEASE_BUFFERS | SSL.SSL_MODE_ACCEPT_MOVING_WRITE_BUFFER . (SSL.SSL_MODE_RELEASE_BUFFERS was used before implicitly).

Result:

No more 'error:1409F07F:SSL routines:SSL3_WRITE_PENDING: bad write retry' possible when writing heap buffers.
2016-02-03 11:29:07 +01:00
Norman Maurer
e220c56823 [#4746] Support SNI when using OpenSSL
Motivation:

When using SslProvider.OPENSSL we currently not handle SNI on the client side.

Modifications:

Correctly enable SNI when using clientMode and peerHost != null.

Result:

SNI works even with SslProvider.OPENSSL.
2016-02-03 10:46:10 +01:00
Xiaoyan Lin
b7415a3307 Add a reusable ArrayList to InternalThreadLocalMap
Motivation:

See #3411. A reusable ArrayList in InternalThreadLocalMap can avoid allocations in the following pattern:

```
List<...> list = new ArrayList<...>();

add something to list but never use InternalThreadLocalMap

return list.toArray(new ...[list.size()]);

```

Modifications:

Add a reusable ArrayList to InternalThreadLocalMap and update codes to use it.

Result:

Reuse a thread local ArrayList to avoid allocations.
2016-02-01 15:49:28 +01:00
Trustin Lee
c3e5604f59 Do not throw IndexOutOfBoundsException on an invalid SSL record
Motivation:

When an SSL record contains an invalid extension data, SniHandler
currently throws an IndexOutOfBoundsException, which is not optimal.

Modifications:

- Do strict index range checks

Result:

No more unnecessary instantiation of exceptions and their stack traces
2016-01-29 08:00:46 +01:00
Scott Mitchell
e1d34ef05d SslHandler should call beginHanshake once for the initial handshake
Motivation:
Not all SSLEngine implementations permit beginHandshake being called while a handshake is in progress during the initial handshake. We should ensure we only go through the initial handshake code once to prevent unexpected exceptions from being thrown.

Modifications:
- Only call beginHandshake if there is not currently a handshake in progress

Result:
SslHandler's handshake method is compatible with OpenSSLEngineImpl in Android 5.0+ and 6.0+.
Fixes https://github.com/netty/netty/issues/4718
2016-01-28 13:27:15 +01:00
Norman Maurer
ee2558bdf3 [#4722] Ensure the whole certificate chain is used when creating SslContext for client mode and SslProvider.OPENSSL is used
Motivation:

We incorrectly added the trustCertChain as certificate chain when OpenSslClientContext was created. We need to correctly add the keyCertChain.

Modifications:

Correctly add whole keyCertChain.

Result:

SSL client auth is working when usin OpenSslClientContext and more then one cert is contained in the certificate chain.
2016-01-28 08:55:28 +01:00
Norman Maurer
d4a1665941 Fix SSLEngineTest handshake method.
Motivation:

We used && in the handshake method of SSLEngineTest but it must be ||.

Modifications:

Changed && to ||

Result:

Correctly check condition
2016-01-27 08:58:45 +01:00
Brendt Lucas
7090d1331c Clear disabled SSL protocols before enabling provided SSL protocols
Motivation:

Attempts to enable SSL protocols which are currently disabled fail when using the OpenSslEngine. Related to https://github.com/netty/netty/issues/4736

Modifications:

Clear out all options that have disabled SSL protocols before attempting to enable any SSL protocol.

Result:

setEnabledProtocols works as expected.
2016-01-22 16:59:40 +01:00
Norman Maurer
ae4e9ddc2d Ensure we flush out all pending data on SslException. Related to [#3900]
Motivation:

We need to ensure we flush out all pending data when an SslException accours so the remote peer receives all alerts.

Modifications:

Ensure we call ctx.flush() when needed.

Result:

Correctly receive alerts in all cases on the remote peer.
2016-01-20 19:56:24 +01:00
Norman Maurer
38f27b06e7 [#4725] Ensure correct cause of handshake error is included in the SSLHandshakeException when using OpenSslEngine.
Motivation:

We need to ensure we add the correct handshake error to the SSLHandshakeException before throwing it when failing the
handshake.

Modifications:

Use the correct error string when creating the SSLHandshakeException.

Result:

Correct SSLHandshakeException message included.
2016-01-20 13:36:09 +01:00
Norman Maurer
7b51412c3c Allow to do async mappings in the SniHandler
Motivation:

Sometimes a user want to do async mappings in the SniHandler as it is not possible to populate a Mapping up front.

Modifications:

Add AsyncMapping interface and make SniHandler work with it.

Result:

It is possible to do async mappings for SNI
2016-01-18 21:02:13 +01:00
Norman Maurer
3c5abaa39a Correctly handle non handshake commands when using SniHandler
Motivation:

As we can only handle handshake commands to parse SNI we should try to skip alert and change cipher spec commands a few times before we fallback to use a default SslContext.

Modifications:

- Use default SslContext if no application data command was received
- Use default SslContext if after 4 commands we not received a handshake command
- Simplify code
- Eliminate multiple volatile fields
- Rename SslConstants to SslUtils
- Share code between SslHandler and SniHandler by moving stuff to SslUtils

Result:

Correct handling of non handshake commands and cleaner code.
2016-01-14 20:56:02 +01:00
Norman Maurer
da01b1daec Decryption failed or bad mac record in Android 5.0
Motivation:

Android 5.0 (API version 21) has a bug which not correctly set the bytesConsumed of SSLEngineResult when HandshakeStatus is FINISHED.  Because of this we need to special handle the status and so workaround the Android bug.

Modifications:

- Break the unwrap for (;;) loop when HandshakeStatus is FINISHED and bytesConsumed == 0 && bytesProduced == 0.

Result:

SslHandler works with all known version of Android.
2016-01-11 09:35:29 +01:00
Scott Mitchell
e578134b57 Unpooled and Wrapped Buffer Leak
Motivation:
There are a few buffer leaks related to how Unpooled.wrapped and Base64.encode is used.

Modifications:
- Fix usages of Bas64.encode to correct leaks
- Clarify interface of Unpooled.wrapped* to ensure reference count ownership is clearly defined.

Result:
Reference count code is more clearly defined and less leaks are possible.
2016-01-07 12:02:52 -08:00
Norman Maurer
a157528ec2 Ensure we only add OpenSslEngine to the OpenSslEngineMap when handshake is started
Motivation:

We need to ensure we only add the OpenSslEngine to the OpenSslEngineMap when the handshake is started as otherwise we may produce a memory leak when the OpenSslEngine is created but not actually used. This can for example happen if we encounter a connection refused from the remote peer. In this case we will never remove the OpenSslEngine from the OpenSslEngineMap and so it will never be collected (as we hold a reference). This has as affect that the finalizer will never be run as well.

Modifications:

- Lazy add the OpenSslEngine to the OpenSslEngineMap to elimate possible leak.
- Call OpenSslEngine.shutdown() when SslHandler is removed from the ChannelPipeline to free memory asap in all cases.

Result:

No more memory leak with OpenSslEngine if connection is refused.
2016-01-05 11:10:08 +01:00
Xiaoyan Lin
f90032933d javadoc fix and better cleanup for WriteTimeoutHandler
Motivation:

- Javadoc is not correct (#4353)
- WriteTimeoutHandler does not always cancel the timeout task (#2973)

Modifications:

Fix the javadoc and cleanup timeout task in handlerRemoved

Result:

WriteTimeoutHandler's javadoc describes the correct behavior and it will cancel timeout tasks when it's removed.
2015-12-30 18:31:55 +01:00
louxiu
6ee5341cdf Fix typo Motivation:
MessageReciever should be MessageReceiver

Modifications:

Refactor MessageReciever to MessageReceiver

Result:

No more typo
2015-12-29 18:56:29 +01:00
Norman Maurer
79bc90be32 Fix buffer leak introduced by 693633eeff
Motivation:

As we not used Unpooled anymore for allocate buffers in Base64.* methods we need to ensure we realease all the buffers.

Modifications:

Correctly release buffers

Result:

No more buffer leaks
2015-12-29 17:13:07 +01:00
Scott Mitchell
8732745264 OpenSslEngine skip ALPN tests if OpenSsl version doesn't support ALPN
Motivation:
OpenSslEngine now tests ALPN behavior. However it is possible that OpenSSL is present, but the version does not support ALPN. This will result in test failures instead of just skipping the test.

Modifications:
- Skip ALPN tests in OpenSslEngineTest if the version of OpenSSL does not support ALPN

Result:
Tests don't fail due to unsupported feature in OpenSSL.
2015-12-28 12:20:38 -08:00
Alex Petrov
ba22b0b944 Implement OpenSSL Engine tests for NPN / ALPN.
Motivation:

Currently there are no tests for OpenSSL Engine,
only for JdkSSL engine.

Modifications:

Common methods from `JdkSslEngine` test moved
to `SSLEngineTest`, JdkSslEngine now implements
NPN and ALPN tests.

Result:

OpenSSL Engine is now covered with unit tests.
2015-12-28 10:18:52 -08:00
Xiaoyan Lin
475d901131 Fix errors reported by javadoc
Motivation:

Javadoc reports errors about invalid docs.

Modifications:

Fix some errors reported by javadoc.

Result:

A lot of javadoc errors are fixed by this patch.
2015-12-27 08:36:45 +01:00
Xiaoyan Lin
a96d52fe66 Fix javadoc links and tags
Motivation:

There are some wrong links and tags in javadoc.

Modifications:

Fix the wrong links and tags in javadoc.

Result:

These links will work correctly in javadoc.
2015-12-26 08:34:31 +01:00
Scott Mitchell
fd5316ed6f ChunkedInput.readChunk parameter of type ByteBufAllocator
Motivation:
ChunkedInput.readChunk currently takes a ChannelHandlerContext object as a parameters. All current implementations of this interface only use this object to get the ByteBufAllocator object. Thus taking a ChannelHandlerContext as a parameter is more restrictive for users of this API than necessary.

Modifications:
- Add a new method readChunk(ByteBufAllocator)
- Deprecate readChunk(ChannelHandlerContext) and updates all implementations to call readChunk(ByteBufAllocator)

Result:
API that only requires ByteBufAllocator to use ChunkedInput.
2015-12-24 12:46:40 -08:00
Xiaoyan Lin
507feb5602 Close FileInputStream after consuming it in SelfSignedCertificate
Motivation:

FileInputStream opened by SelfSignedCertificate wasn't closed.

Modifications:

Use a try-finally to close the opened FileInputStream.

Result:

FileInputStream will be closed properly.
2015-12-24 07:51:09 +01:00
Trustin Lee
b62c5290ed Let SniHandler accept Mapping as well as DominaNameMapping
Related: #4470 #4473

Motivation:

A user might want to:

- implement dynamic mapping from hostname to SslContext
- server large number of domain names whose SslContext can be
  initialized lazily and destroyed when unused

Modifications:

- Let SniHandler accept Mapping<String, SslContext> as well as
  DomainNameMapping
- Make the default constructor of SslContext so that a user can create
  his or her own SslContext wrapper

Result:

Flexibility
2015-12-18 12:36:26 +09:00
Norman Maurer
dd9fc289fd Throw exception if KeyManagerFactory is used with OpenSslServerContext
Motivation:

We currently not supported using KeyManagerFactory with OpenSslServerContext and so should throw an exception if the user tries to do so. This will at least not give suprising and hard to debug problems later.

Modifications:

Throw exception if a user tries to construct a OpenSslServerContext with a KeyManagerFactory

Result:

Fail fast if the user tries to use something that is not supported.
2015-12-17 08:01:51 +01:00
Norman Maurer
253cd694ef Ensure we not leave data in the BIO when error happens.
Motivation:

We need to ensure we consume all pending data in the BIO on error to correctly send the close notify for the remote peer.

Modifications:

Correctly force the user to call wrap(...) if there is something left in the BIO.

Result:

close_notify is not lost.
2015-12-17 12:40:19 +09:00
Norman Maurer
eb577c5bd9 Respect ClientAuth set via OpenSslEngine constructor
Motivation:

When ClientAuth is set via SslContextBuilder we pass it into the OpenSslEngine constructor. Due a bug we missed to call the correct native methods and so never enabled ClientAuth in this case.

Modifications:

Correctly call setClientAuth(...) in the constructor if needed.

Result:

client auth also works when configured via the SslContextBuilder and OPENSSL is used.
2015-12-16 15:39:29 +01:00
Norman Maurer
088ee71222 Remove unused method in SslContext
Motivation:

We missed to remove a method in SslContext while refactored the implementation. We should remove the method to keep things clean.

Modifications:

Remove unused method.

Result:

Code cleanup.
2015-12-10 08:58:40 +01:00
Alex Petrov
43ebbc3fa0 Update JDK SSL Tests to use SSL Context Builder.
Motivation:

Use new / non-deprecated APIs for creating SSL Context
in tests, in order to be able to implement OpenSsl
tests with maximum code reuse.

Modifications:

Use `SslContextBuilder.(forServer|forClient)` instead
of deprecated `JdkSslServerContext` constructor.
Use `ApplicationProtocolConfig` instead of Protocol
Negotiator.
Use custom exception type for skipping tests to avoid
swallowing exceptions arising from tests.

Result:

Exceptions from tests aren't swallowed.
Using new APIs allows reusing same test code for
OpenSsl tests.
2015-12-04 11:08:57 -08:00
Norman Maurer
7bee318fc7 Use OneTimeTask where possible to reduce object creation
Motivation:

We should use OneTimeTask where possible to reduce object creation.

Modifications:

Replace Runnable with OneTimeTask

Result:

Less object creation
2015-11-20 14:39:06 -08:00
Scott Mitchell
21e27da410 ApplicationProtocolNegotiationHandler failure behavior
Motivation:
Child classes of ApplicationProtocolNegotiationHandler may want to override the behavior when a handshake failure is detected.

Modifications:
- Provide a method which can be overriden when a handshake failure is detected.

Result:
Child classes can override ApplicationProtocolNegotiationHandler handshake failure behavior.
2015-11-07 09:33:08 -08:00
Norman Maurer
85236d5446 [#4355] OpenSslServerContext reinitializes the provided TrustManagerFactory with the key cert chain.
Motivation:

OpenSslServerContext should not reinitialize the provided TrustManagerFactory with the key cert chain as the user should be able to pass a fully initialized TrustManagerFactory. This is also in line with how JdkSslServerContext works.

Modifications:

Not reinitialize the provided TrustManagerFactory with the key cert chain.

Result:

Correct and consistent behavior.
2015-10-25 10:59:50 +01:00
Norman Maurer
4aa19a09bd Implement SSLSession.invalidate() and isValid() for OpenSSLEngine.
Motivation:

The SSLSession allows to invalidate a SSLSession and so disallow resume of a session. We should support this for OpenSSLEngine as well.

Modifications:

- Correctly implement SSLSession.isValid() and invalidate() in OpenSSLEngine
- Add unit test.

Result:

Invalidate of SSL sessions is supported when using OpenSSL now.
2015-10-15 12:02:19 +02:00
Norman Maurer
66c3c58d3e Reduce object creation for for unwrap/wrap if no ByteBuffer[] is used.
Motivation:

Often unwrap(...), wrap(...) is used with a single ByteBuffer and not with a ByteBuffer[]. We should reduce the array creations in this case.

Modifications:

Reuse ByteBuffer[1] for dst/src ByteBuffer.

Result:

Less object creation and so less GC
2015-10-07 13:35:53 +02:00
Norman Maurer
dc6cb7545b Lazy compute SSLSession creation time.
Motivation:

As a SSL session may be created later at some time we should compute the creation time in a lazy fashion.

Modifications:

- Lazy compute creation time
- Add some unit test

Result:

More correct behavior
2015-10-03 10:42:00 +02:00
Norman Maurer
5deec9631f Add support for server-side renegotiation when using OpenSslEngine.
Motivation:

JDK SslEngine supports renegotion, so we should at least support it server-side with OpenSslEngine as well.

That said OpenSsl does not support sending messages asynchronly while the renegotiation is still in progress, so the application need to ensure there are not writes going on while the renegotiation takes place. See also https://rt.openssl.org/Ticket/Display.html?id=1019 .

Modifications:

- Add support for renegotiation when OpenSslEngine is used in server mode
- Add unit tests.
- Upgrade to netty-tcnative 1.1.33.Fork9

Result:

Better compatibility with the JDK SSLEngine implementation.
2015-10-02 11:24:51 +02:00
Norman Maurer
179cd9a4a1 Correctly update internal handshake state on beginHandshake()
Motivation:

We missed to correctly update the internal handshake state on beginHandshake() if we was able to finish the handshake directly. Also we not handled the case correctly when beginHandshake() was called after the first handshake was finished, which incorrectly throw an Error.

Modifications:

- Correctly set internal handshake state in all cases
- Correctly handle beginHandshake() once first handshake was finished.

Result:

Correctly handle OpenSslEngine.beginHandshake()
2015-10-01 17:41:46 +02:00
Norman Maurer
2766fc49e2 Expose new way of setting session keys
Motivation:

We should provide a better way to set session keys that not use the deprecated method of netty-tcnative.

Modifications:

- Add OpenSslSessionTicketKey
- Expose new method on OpenSslServerContext and deprecate the old method.

Result:

Easier to use and can remove the deprecated method later on.
2015-09-25 20:58:04 +02:00
Scott Mitchell
c47106587a Unused paramters introduced by https://github.com/netty/netty/pull/4257
Motivation:
PR https://github.com/netty/netty/pull/4257 introduced paramters and didn't use them.

Modifications:
- Use the new paramters

Result:
No warnings and correct behavior
2015-09-24 17:37:33 -07:00
Norman Maurer
6c6c369c68 [#4235] Ensure OpenSslEngine.unwrap(...) / wrap(...) correctly return HandshakeStatus.FINISHED
Motivation:

OpenSslEngine.unwrap(...) / wrap(...) must return HandhsakeStatus.FINISHED if an unwrap or wrap finishes a handshake to behave like descripted in the SSLEngine docs.

Modifications:

- Ensure we return HandshakeStatus.FINISHED

Result:

Behave correctly.
2015-09-24 14:58:31 +02:00
Scott Mitchell
c116c35ed0 SelfSignedCertificate configurable valid dates
Motivation:
Users may want to control the valid dates for SelfSignedCertificate.

Modifications:
- Allow NOT_BEFORE and NOT_AFTER to be controlled via java system properties.

Result:
Fixes https://github.com/netty/netty/issues/3978
2015-09-23 17:04:05 -07:00
nmittler
a1d0207ec5 Adding client auth to SslContextBuilder
Motivation:

To simplify the use of client auth, we need to add it to the SslContextBuilder.

Modifications:

Added a ClientAuth enum and plumbed it through the builder, down into the contexts/engines.

Result:

Client auth can be configured when building an SslContext.
2015-09-18 12:16:49 -07:00
Norman Maurer
30b30f77c6 Support SSLSession.getLocalCertificates() and getLocalPrincipal() when using OpenSSL
Motivation:

SSLSession.getLocalCertificates() and getLocalPrincipal() was not supported when using OpenSSL, which can produce problems when switch from JDK to OpenSSL impl.

Modifications:

Implement SSLSession.getLocalCertificates() and getLocalPrincipal() for OpenSslEngine.

Result:

More consistent behaving between JDK and OpenSSL based SSLEngine.
2015-09-15 12:22:00 +02:00
Norman Maurer
6e3acfeb06 Correctly throw SSLPeerUnverifiedException if peers identity has not been verified
Motivation:

As stated in the SSLSession javadocs getPeer* methods need to throw a SSLPeerUnverifiedException if peers identity has not be verified.

Modifications:

- Correctly throw SSLPeerUnverifiedException
- Add test for it.

Result:

Correctly behave like descripted in javadocs.
2015-09-15 09:57:45 +02:00
Michael Bildner
58dc7f7902 Do not bother closing SSL enging inbound if the outbound has already been closed.
Motivation:

Invoking the javax.net.ssl.SSLEngine.closeInbound() method will send a
fatal alert and invalidate the SSL session if a close_notify alert has
not been received.
From the javadoc:
If the application initiated the closing process by calling
closeOutbound(), under some circumstances it is not required that the
initiator wait for the peer's corresponding close message. (See section
7.2.1 of the TLS specification (RFC 2246) for more information on
waiting for closure alerts.) In such cases, this method need not be
called.
Always invoking the closeInbound() method without regard to whether or
not the closeOutbound() method has been invoked could lead to
invalidating perfectly valid SSL sessions.

Modifications:

Added an instance variable to track whether the
SSLEngine.closeOutbound() method has been invoked. When the instance
variable is true, the SSLEngine.closeInbound() method doesn't need to be
invoked.

Result:

SSL sessions will not be invalidated if the outbound side has been
closed but a close_notify alert hasn't been received.
2015-09-06 10:00:46 +02:00
Norman Maurer
407d5ccdcf Revert "Consistent naming style for enum"
This reverts commit 4feafc4a52.
2015-08-28 20:49:38 +02:00
Norman Maurer
4feafc4a52 Consistent naming style for enum
Motivation:

We should use camel-case for Enums.

Modifications:

Rename enums to use camel-case.

Result:

Consistent naming
2015-08-21 07:18:19 +02:00
Vineet Garg
052a171a52 Fixes infinite loop during handshake in SslHandler in Android devices
Motivation:

On Android devices with version less than Lollipop, HarmonyJSSE is used for SSL. After completion of handshake, handshake status is NOT_HANDSHAKING instead of FINISHED. Also encrypting empty buffer after handshake should cause underflow exception and produce 0 bytes, but here it happily encrypts it causing for loop to never break

Modification:

Since 0 bytes should only be consumed in handshake process. Added a condition to break loop when 0 bytes are consumed and handshake status is NOT_HANDSHAKING

Result:

Sucessful ssl handshake on Android devices, no infinite loop now
2015-08-19 22:12:52 +02:00
Scott Mitchell
a4261d481c Eclipse SPDY docs moved
Motivation:
We provide a hyperlink to the docs for SPDY if the runtime is not setup correctly to help users. These docs have moved.

Modifications:
- Update the hyperlink to point to the new doc location.

Result:
Users are able to find docs more easily.
2015-08-13 09:45:06 -07:00
Norman Maurer
5ac84760c4 Allow to create SslContext from existing PrivateKey / X509Certificate
Motivation:

Sometimes the user already has a PrivateKey / X509Certificate which should be used to create a new SslContext. At the moment we only allow to construct it via Files.

Modifications:

- Add new methods to the SslContextBuilder to allow creating a SslContext from PrivateKey / X509Certificate
- Mark all public constructors of *SslContext as @Deprecated, the user should use SslContextBuilder
- Update tests to us SslContextBuilder.

Result:

Creating of SslContext is possible with PrivateKay/X509Certificate
2015-08-12 15:05:58 +02:00
Norman Maurer
ecc01da9dd [#3968] Disallow pass-through of non ByteBufs in SslHandler
Motivation:

We pass-through non ByteBuf when SslHandler.write(...) is called which can lead to have unencrypted data to be send (like for example if a FileRegion is written).

Modifications:

- Fail ChannelPromise with UnsupportedMessageException if a non ByteBuf is written.

Result:

Only allow ByteBuf to be written when using SslHandler.
2015-07-22 13:31:33 +02:00
Norman Maurer
c3ab557f85 [#3987] Remove RC4 from default ciphers.
Motivation:

Remove RC4 from default ciphers as it is not known as secure anymore.

Modifications:

Remove RC4

Result:

Not use an insecure cipher as default.
2015-07-22 13:29:43 +02:00
Norman Maurer
9660e2f6a9 Better handling of BUFFER_OVERFLOW when unwrap data.
Motivation:

When we detect a BUFFER_OVERFLOW we should just forward the already produced data and allocate a new buffer and NOT do any extra memory copies while trying to expand the buffer.

Modifications:

When a BUFFER_OVERFLOW is returned and some data was produced just fire this data through the pipeline and allocate a new buffer to read again.

Result:

Less memorycopies and so better performance.
2015-07-08 09:39:12 +02:00
Norman Maurer
8d1c6ebf71 Only do priming read if there is no space in dsts buffers.
Motivation:

A SSL_read is needed to ensure the bio buffer is flushed, for this we did a priming read. This can be removed in many cases. Also ensure we always fill as much as possible in the destination buffers.

Modifications:

- Only do priming read if capacity of all dsts buffers is zero
- Always produce as must data as possible in the dsts buffers.

Result:

Faster code.
2015-07-08 08:41:18 +02:00
Norman Maurer
18356911ab Stop calling BIO_write once internal buffer is full.
Motivation:

Previous we called BIO_write until either everything was written into it or it returned an error, which meant that the buffer is full. This then needed a ERR_clear_error() call which is expensive.

Modifications:

Break out of writing loop once we detect that not everything was written and so the buffer is full.

Result:

Less overhead when writing more data then the internal buffer can take.
2015-07-08 08:39:21 +02:00
Norman Maurer
a9d2b5cef0 Skip empty buffers and not pass these to BIO_write
Motivation:

When BIO_write is called with an empty buffer it will return 0 for which we call ERR_clear_error(). This is not neccessary as we should just skip these buffers. This eliminates a lot of overhead.

Modifications:

Skip empty src buffers when call unwrap(...).

Result:

Less overhead for unwrap(...) when called with empty buffers.
2015-07-08 08:37:51 +02:00
Norman Maurer
0f95b85ec2 Ensure OpenSslSession informations can be retrieved even after shutdown
Motivation:

If a user tries to access various informations on the OpenSslSession after the SSLEngine was closed it will not work if these were not accessed before as we lazy init most of them.

Modifications:

Directly populate the whole OpenSslSession once the handshake is complete and before the user is notified about it.

Result:

OpenSslSession informations are avaible until it is GC'ed.
2015-07-07 09:49:52 +02:00
Norman Maurer
bad8e0d6ab Correctly handle errors when using OpenSSL
Motivation:

We used ERR_get_error() to detect errors and missed to handle different errors. Also we missed to clear the error queue for a thread before invoke SSL operations,
this could lead to detecting errors on different OpenSslEngines then the one in which the error actual happened.

Modifications:

Explicit handle errors via SSL.get_error and clear the error code before SSL operations.

Result:

Correctly handle errors and no false-positives in different OpenSslEngines then the one which detected an error.
2015-06-21 21:06:42 +02:00
Norman Maurer
29ac2ae3c2 [#3883] OpenSSL SSLSession returns incorrect peer principal
Motivation:

According to the javadocs of SSLSession.getPeerPrincipal should be returning the identity of the peer, while we return the identity of the issuer.

Modifications:

Return the correct indentity.

Result:

Behavior match the documentation.
2015-06-17 06:36:13 +02:00
Norman Maurer
d1b7f990f2 Not skip first cert when using OpenSslClientContext
Motivation:

Due a copy and paste error we incorrectly skipped the first cert in the keyCertChainFile when using OpenSslClientContext.

Modifications:

Correctly not skip the first cert.

Result:

The certificate chain is correctly setup when using OpenSslClientContext.
2015-06-10 09:01:31 +02:00
Norman Maurer
4570f30dd9 [#3798] Extract dump method to ByteBufUtil
Motivation:

Dumping the content of a ByteBuf in a hex format is very useful.

Modifications:

Move code into ByteBufUtil so its easy to reuse.

Result:

Easy to reuse dumping code.
2015-06-09 06:21:09 +02:00
Norman Maurer
cf54c04241 Correctly respect readerIndex of buffer when dumping.
Motivation:

The current dumping code does not respect the readerIndex and so logs incorrect.

Modifications:

Respect readerIndex of ByteBuf

Result:

Correctly log content of buffer.
2015-06-08 09:23:40 +02:00
Norman Maurer
a485ae68dc Guard against race when calling SslHandler.handshakeFuture().sync()
Motivation:

If the handlerAdded(...) callback was not called, the checkDeadLock() of the handshakeFuture will produce an IllegalStateException.
This was first reported at https://github.com/impossibl/pgjdbc-ng/issues/168 .

Modifications:

Pass deadlock check if ctx is null

Result:

No more race and so IllegalStateException.
2015-06-08 09:17:27 +02:00
Norman Maurer
2b0dfc4e80 Expose SSL_CTX and SSL pointers
Motivation:

For advanced use-cases it an be helpful to be able to directly access the SSL_CTX and SSL pointers of the underlying openssl objects. This for example allows to register custom C callbacks.

Modifications:

- Expose the SSL_CTX and SSL pointers
- Cleanup the shutdown code

Result:

It's now possible to obtain the c pointes and set native callbacks.
2015-06-05 07:25:06 +02:00
Trustin Lee
0775089496 Replace SpdyOrHttpChooser and Http2OrHttpChooser with ApplicationProtocolNegotiationHandler
Motivation:

SpdyOrHttpChooser and Http2OrHttpChooser duplicate fair amount code with each other.

Modification:

- Replace SpdyOrHttpChooser and Http2OrHttpChooser with ApplicationProtocolNegotiationHandler
- Add ApplicationProtocolNames to define the known application-level protocol names

Result:

- Less code duplication
- A user can perform dynamic pipeline configuration that follows ALPN/NPN for any protocols.
2015-06-05 11:58:20 +09:00
Trustin Lee
afb46b926f Improve the API design of Http2OrHttpChooser and SpdyOrHttpChooser
Related: #3641 and #3813

Motivation:

When setting up an HTTP/1 or HTTP/2 (or SPDY) pipeline, a user usually
ends up with adding arbitrary set of handlers.

Http2OrHttpChooser and SpdyOrHttpChooser have two abstract methods
(create*Handler()) that expect a user to return a single handler, and
also have add*Handlers() methods that add the handler returned by
create*Handler() to the pipeline as well as the pre-defined set of
handlers.

The problem is, some users (read: I) don't need all of them or the
user wants to add more than one handler. For example, take a look at
io.netty.example.http2.tiles.Http2OrHttpHandler, which works around
this issue by overriding addHttp2Handlers() and making
createHttp2RequestHandler() a no-op.

Modifications:

- Replace add*Handlers() and create*Handler() with configure*()
- Rename getProtocol() to selectProtocol() to make what it does clear
- Provide the default implementation of selectProtocol()
- Remove SelectedProtocol.UNKNOWN and use null instead, because
  'UNKNOWN' is not a protocol
- Proper exception handling in the *OrHttpChooser so that the
  exception is logged and the connection is closed when failed to
  select a protocol
- Make SpdyClient example always use SSL. It was always using SSL
  anyway.
- Implement SslHandshakeCompletionEvent.toString() for debuggability
- Remove an orphaned class: JettyNpnSslSession
- Add SslHandler.applicationProtocol() to get the name of the
  application protocol
  - SSLSession.getProtocol() now returns transport-layer protocol name
    only, so that it conforms to its contract.

Result:

- *OrHttpChooser have better API.
- *OrHttpChooser handle protocol selection failure properly.
- SSLSession.getProtocol() now conforms to its contract.
- SpdyClient example works with SpdyServer example out of the box
2015-06-05 11:58:19 +09:00
Norman Maurer
bac2e3a6d2 Reduce calls to System.nanoTime() and object creation in IdleStateHandler. Related to [#3808]
Motivation:

Calling System.nanoTime() for each channelRead(...) is very expensive. See [#3808] for more detailed description.
Also we always do extra work for each write and read even if read or write idle states should not be handled.

Modifications:

- Move System.nanoTime() call to channelReadComplete(...).
- Reuse ChannelFutureListener for writes
- Only add ChannelFutureListener to writes if write and all idle states should be handled.
- Only call System.nanoTime() for reads if idle state events for read and all states should be handled.

Result:

Less overhead when using the IdleStateHandler.
2015-05-27 14:07:39 +02:00
Norman Maurer
6fce3b79c3 Do not try to init TrustManagerFactory if trustCertChainFile is null.
Motivation:

We called TrustManagerFactory.init(...) even when the trustCertChainFile is null. This could lead to exceptions during the handshake.

Modifications:

Correctly only call TurstManagerFactory.init() if trustCertcChainFail is not null.

Result:

Correct behavior.
2015-05-27 13:45:57 +02:00
Scott Mitchell
d5f1dc66aa Consistent use of SSLHandshakeException for ALPN
Motiviation:
The OpenSSL engine uses SSLHandshakeException in the event of failures that occur during the handshake process. The alpn-boot project's getSSLException will also map the no_application_protocol to a SSLHandshakeException exception. We should be consistent and use SSLHandshakeException for handshake failure events.

Modifications:
-Update JdkAlpnSslEngine to propagate an SSLHandshakeException in the event of a failure.

Result:
Consistent usage of SSLHandshakeException during a handshake failure event.
2015-05-26 16:10:58 -07:00
johnou
ad7f033c06 Allow writing with void promise if IdleStateHandler is configured in pipeline.
Motivation:

Allow writing with void promise if IdleStateHandler is configured in the pipeline for read timeout events.

Modifications:

Better performance.

Result:

No more ChannelFutureListeners are created if IdleStateHandler is only configured for read timeouts allowing for writing to the channel with void promise.
2015-05-25 21:09:47 +02:00
Norman Maurer
9d675def81 Only call System.nanoTime() if no read batch is ongoing. Related to [#3808]
Motivation:

[#3808] introduced some improvements to reduce the calls to System.nanoTime() but missed one possible optimization.

Modifications:

Only call System.nanoTime() if no reading patch is in process.

Result:

Less System.nanoTime() calls.
2015-05-25 18:21:00 +02:00
nmittler
e4af176be7 Upgrading Jetty alpn-api version
Motivation:

Discussion is in https://github.com/jetty-project/jetty-alpn/issues/8. The new API allows protocol negotiation to properly throw SSLHandshakeException.

Modifications:

Updated the parent pom.xml with the new version.

Result:

Upgraded alpn-api now allows throwing SSLHandshakeException.
2015-05-22 13:13:14 -07:00
Robert Varga
f3dcad3230 Do not call System.nanoTime() in ReadTimeoutHandler.channelRead()
Motivation:

We mitigate callouts to System.nanoTime() in SingleThreadEventExecutor
as it is 'relatively expensive'. On a modern system, tak translates to
about 20ns per call. With channelReadComplete() we can side-step this in
channelRead().

Modifications:

Introduce a boolean flag, which indicates that a read batch is currently
on-going, which acts as a flush guard for lastReadTime. Update
lastReadTime in channelReadComplete() just before setting the flag to
false. We set the flag to true in channelRead().

The periodic task examines the flag, and if it observes it to be true,
it will reschedule the task for the full duration. If it observes as
false, it will read lastReadTime and adjust the delay accordingly.

Result:

ReadTimeoutHandler calls System.nanoTime() only once per read batch.
2015-05-21 07:14:08 +02:00
Robin Stocker
9bf636076a Fix typo in FingerprintTrustManagerFactory docs 2015-05-18 08:30:25 +02:00
Norman Maurer
b934257796 [#3784] Support hostname verification when using OpenSSLEngine
Motivation:

At the moment hostname verification is not supported with OpenSSLEngine.

Modifications:

- Allow to create OpenSslEngine with peerHost and peerPort informations.
- Respect endPointIdentificationAlgorithm and algorithmConstraints when set and get SSLParamaters.

Result:

hostname verification is supported now.
2015-05-18 08:16:49 +02:00
Eric Anderson
864f196c67 Add missing SslContextBuilder.forServer(KeyManagerFactory)
Motivation:

keyManager() is required on server-side, and so there is a forServer()
method for each override of keyManager(). However, one of the
forServer() overrides was missing, which meant that if you wanted to use
a KeyManagerFactory you were forced to provide garbage configuration
just to get past null checks.

Modifications:

Add missing override.

Result:

No hacks to use SslContextBuilder on server-side with KeyManagerFactory.
Resolves #3775
2015-05-11 22:10:34 +02:00
Norman Maurer
f963401d42 Allow rejection of remote initiated renegotiation
Motivation:

To prevent from DOS attacks it can be useful to disable remote initiated renegotiation.

Modifications:

Add new flag to OpenSslContext that can be used to disable it
Adding a testcase

Result:

Remote initiated renegotion requests can be disabled now.
2015-05-07 14:41:25 +02:00
Norman Maurer
e71e40057f Fix possible IllegalStateException caused by closeNotifyTimeout when using SslHandler
Motivation:

In the SslHandler we schedule a timeout at which we close the Channel if a timeout was detected during close_notify. Because this can race with notify the flushFuture we can see an IllegalStateException when the Channel is closed.

Modifications:

- Use a trySuccess() and tryFailure(...) to guard against race.

Result:

No more race.
2015-05-06 21:50:16 +02:00
Norman Maurer
868eb49cd2 Only run OpenSslEngineTests if OpenSsl is installed. Related to [#3732] 2015-05-06 10:42:00 +02:00
Norman Maurer
52eae1c9b3 Add support for mutual auth when using OpenSslEngine.
Motivation:

Currently mutual auth is not supported when using OpenSslEngine.

Modification:

- Add support to OpenSslClientContext
- Correctly throw SSLHandshakeException when an error during handshake is detected

Result:

Mutual auth can be used with OpenSslEngine
2015-05-06 09:08:05 +02:00
Eric Anderson
f467d695be Fix SslContextBuilder swapping client and server
The 'forClient' boolean was swapped to 'forServer' in code review of #3531.
Not all locations were updated.
2015-04-20 17:25:14 -07:00
Norman Maurer
62057f73d6 Fix handling of non-auto read for ByteToMessageDecoder and SslHandler
Motivation:

Our automatically handling of non-auto-read failed because it not detected the need of calling read again by itself if nothing was decoded. Beside this handling of non-auto-read never worked for SslHandler as it always triggered a read even if it decoded a message and auto-read was false.

This fixes [#3529] and [#3587].

Modifications:

- Implement handling of calling read when nothing was decoded (with non-auto-read) to ByteToMessageDecoder again
- Correctly respect non-auto-read by SslHandler

Result:

No more stales and correctly respecting of non-auto-read by SslHandler.
2015-04-20 09:11:02 +02:00
Norman Maurer
bdfdf3094d Reduce object allocation during wrap/unwrap while handshake is in progress
Motivation:

Unnecessary object allocation is currently done during wrap/unwrap while a handshake is still in progress.

Modifications:

Use static instances when possible.

Result:

Less object creations.
2015-04-20 06:48:09 +02:00
Norman Maurer
3850cff0fc Allow to get version of available OpenSSL library
Motivation:

Sometimes it's useful to get informations about the available OpenSSL library that is used for the OpenSslEngine.

Modifications:

Add two new methods which allows to get the available OpenSSL version as either
an int or an String.

Result:

Easy to access details about OpenSSL version.
2015-04-18 20:56:27 +02:00
Eric Anderson
4e70523edd The "null" ClassLoader is the bootstrap ClassLoader
Motivation:
Class.forName() documents that null will use bootstrap loader:
http://docs.oracle.com/javase/8/docs/api/java/lang/Class.html#forName-java.lang.String-boolean-java.lang.ClassLoader-

But the link between "null" and bootstrap loader is even more explicit
in ClassLoader's documentation:
http://docs.oracle.com/javase/8/docs/api/java/lang/ClassLoader.html#getParent--

The current code is trying to use the bootstrap loader but seems to have
not been aware of the meaning of null.

Modifications:
Use "null" as the class loader when we want to load classes in the
bootstrap loader.

Result:
More reliable ALPN/NPN loading and simpler code.
2015-04-16 17:26:09 +02:00
Norman Maurer
05498ee938 Fix regression introduced by cherry-pick bd224286f5 2015-04-14 09:36:48 +02:00
Norman Maurer
6c3f5ab34d Add support for EC Keys when using SslServerContext
Motivation:

Sometimes it's useful to use EC keys and not DSA or RSA. We should support it.

Modifications:

Support EC keys and share the code between JDK and Openssl impl.

Result:

It's possible to use EC keys now.
2015-04-14 08:45:22 +02:00
Eric Anderson
bd224286f5 [#3531] Create SslContext.Builder
Motivation:

SslContext factory methods have gotten out of control; it's past time to
swap to a builder.

Modifications:

New Builder class. The existing factory methods must be left as-is for
backward compatibility.

Result:

Fixes #3531
2015-04-14 07:28:34 +02:00
Norman Maurer
aebbb862ac Add support for ALPN when using openssl + NPN client mode and support for CipherSuiteFilter
Motivation:

To support HTTP2 we need APLN support. This was not provided before when using OpenSslEngine, so SSLEngine (JDK one) was the only bet.
Beside this CipherSuiteFilter was not supported

Modifications:

- Upgrade netty-tcnative and make use of new features to support ALPN and NPN in server and client mode.
- Guard against segfaults after the ssl pointer is freed
- support correctly different failure behaviours
- add support for CipherSuiteFilter

Result:

Be able to use OpenSslEngine for ALPN / NPN for server and client.
2015-04-10 18:52:34 +02:00
Frederic Bregier
190cbf55e4 Fix incorrect null value check in TrafficCounter
In TrafficCounter, a recent change makes the contract of the API (the
constructor) wrong and lead to issue with GlobalChannelTrafficCounter
where executor must be null.

Motivation:
TrafficCounter executor argument in constructor might be null, as
explained in the API, for some particular cases where no executor are
needed (relevant tasks being taken by the caller as in
GlobalChannelTrafficCounter).
A null pointer exception is raised while it should not since it is
legal.

Modifications:
Remove the 2 null checking for this particular attribute.
Note that when null, the attribute is not reached nor used (a null
checking condition later on is applied).

Result:
No more null exception raized while it should not.

This shall be made also to 4.0, 4.1 (present) and master. 3.10 is not
concerned.
2015-04-06 18:27:56 +02:00
Trustin Lee
2e509f7bb7 Fix unbounded expansion of cumulative buffer in SslHandler
Related: #3567

Motivation:

SslHandler.channelReadComplete() forgets to call
super.channelReadComplete(), which discards read bytes from the
cumulative buffer.  As a result, the cumulative buffer can expand its
capacity unboundedly.

Modifications:

Call super.channelReadComplete() instead of calling
ctx.fireChannelReadComplete()

Result:

Fixes #3567
2015-04-02 14:54:29 +09:00
Trustin Lee
44eeb5f6b4 Fix intermittent test failure in LoggingHandlerTest
Motivation:

LoggingHandlerTest sometimes failure due to unexpected log messages
logged due to the automatic reclaimation of thread-local objects.

  Expectation failure on verify:
    Appender.doAppend([DEBUG] Freed 3 thread-local buffer(s) from thread: nioEventLoopGroup-23-0): expected: 1, actual: 0
    Appender.doAppend([DEBUG] Freed 9 thread-local buffer(s) from thread: nioEventLoopGroup-23-1): expected: 1, actual: 0
    Appender.doAppend([DEBUG] Freed 2 thread-local buffer(s) from thread: nioEventLoopGroup-23-2): expected: 1, actual: 0
    Appender.doAppend([DEBUG] Freed 4 thread-local buffer(s) from thread: nioEventLoopGroup-26-0): expected: 1, actual: 0
    Appender.doAppend(matchesLog(expected: ".+CLOSE$", got: "[id: 0xembedded, embedded => embedded] CLOSE")): expected: 1, actual: 0

Modifications:

Add the mock appender to the related logger only

Result:

No more intermittent test failures
2015-03-31 15:08:52 +09:00
Trustin Lee
f4e527c64d Don't trigger IOException at ChunkedStream.isEndOfInput()
Related: #3368

Motivation:

ChunkedWriteHandler checks if the return value of
ChunkedInput.isEndOfInput() after calling ChunkedInput.close().

This makes ChunkedStream.isEndOfInput() trigger an IOException, which is
originally triggered by PushBackInputStream.read().

By contract, ChunkedInput.isEndOfInput() should not raise an IOException
even when the underlying stream is closed.

Modifications:

Add a boolean flag that keeps track of whether the underlying stream has
been closed or not, so that ChunkedStream.isEndOfInput() does not
propagate the IOException from PushBackInputStream.

Result:

Fixes #3368
2015-03-31 11:38:25 +09:00
Norman Maurer
a2428c7e47 Add supported for X509ExtendedTrustManager when using OpenSslEngine
Motivation:

For some use cases X509ExtendedTrustManager is needed as it allows to also access the SslEngine during validation.

Modifications:

Add support for X509ExtendedTrustManager on java >= 7

Result:

It's now possible to use X509ExtendedTrustManager with OpenSslEngine
2015-03-30 09:05:18 +02:00
nmittler
0fe67cfba5 Using public LogLevel for HTTP/2 frame logging.
Motivation:

The Http2FrameLogger is currently using the internal logging classes. We should change this so that it's using the public classes and then converts internally.

Modifications:

Modified Http2FrameLogger and the examples to use the public LogLevel class.

Result:

Fixes #2512
2015-03-17 15:10:35 -07:00
Leonardo Freitas Gomes
a97e413a65 Ensure server preference order in ALPN
Motivation:
With the current implementation the client protocol preference list
takes precedence over the one of the server, since the select method
will return the first item, in the client list, that matches any of the
protocols supported by the server. This violates the recommendation of
http://tools.ietf.org/html/rfc7301#section-3.2.

It will also fail with the current implementation of Chrome, which
sends back Extension application_layer_protocol_negotiation, protocols:
[http/1.1, spdy/3.1, h2-14]

Modifications:
Changed the protocol negotiator to prefer server’s list. Added a test
case that demonstrates the issue and that is fixed with the
modifications of this commit.

Result:
Server’s preference list is used.
2015-03-17 07:28:53 +01:00
Trustin Lee
8c135cdd55 Add a new constructor without handler parameter to TrafficCounter
Related: #3476

Motivation:

Some users use TrafficCounter for other uses than we originally
intended, such as implementing their own traffic shaper.  In such a
case, a user does not want to specify an AbstractTrafficShapingHandler.

Modifications:

- Add a new constructor that does not require an
  AbstractTrafficShapingHandler, so that a user can use it without it.
- Simplify TrafficMonitoringTask
- Javadoc cleanup

Result:

We open the possibility of using TrafficCounter for other purposes than
just using it with AbstractTrafficShapingHandler.  Eventually, we could
generalize it a little bit more, so that we can potentially use it for
other uses.
2015-03-10 11:28:11 +09:00