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.
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
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.
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
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.
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.
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.
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
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.
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.
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
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
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].
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.
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.
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]
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.
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]
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
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
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.
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.
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.
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_staplinghttps://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
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.
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.
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.
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.
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.
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]
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)
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.
