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.
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.
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.
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.
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.
Motivation:
We should use OneTimeTask where possible to reduce object creation.
Modifications:
Replace Runnable with OneTimeTask
Result:
Less object creation
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.
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.
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.
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
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
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.
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()
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.
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
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.
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
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.
Motivation:
The latest netty-tcnative fixes a bug in determining the version of the runtime openssl lib. It also publishes an artificact with the classifier linux-<arch>-fedora for fedora-based systems.
Modifications:
Modified the build files to use the "-fedora" classifier when appropriate for tcnative. Care is taken, however, to not change the classifier for the native epoll transport.
Result:
Netty is updated the the new shiny netty-tcnative.
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.
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.
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.
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
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.
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
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.
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.
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.
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.
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.
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.
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.
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.
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.
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.
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.
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.
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.
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.
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.
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
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.
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.
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.
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.
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.
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.
Norman Maurer
Alex Petrov
Scott Mitchell
nmittler
Michael Bildner
Vineet Garg
Trustin Lee
johnou