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:
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.
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.
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.
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
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.
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.
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
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.
