Motivation:
JDK's SSLEngine.wrap() requires the output buffer to be always as large as MAX_ENCRYPTED_PACKET_LENGTH even if the input buffer contains small number of bytes. Our OpenSslEngine implementation does not have such wasteful behaviot.
Modifications:
If the current SSLEngine is OpenSslEngine, allocate as much as only needed.
Result:
Less peak memory usage.
Motivation:
Previous fix for the OpenSslEngine compatibility issue (#2216 and
18b0e95659) was to feed SSL records one by
one to OpenSslEngine.unwrap(). It is not optimal because it will result
in more JNI calls.
Modifications:
- Do not feed SSL records one by one.
- Feed as many records as possible up to MAX_ENCRYPTED_PACKET_LENGTH
- Deduplicate MAX_ENCRYPTED_PACKET_LENGTH definitions
Result:
- No allocation of intemediary arrays
- Reduced number of calls to SSLEngine and thus its underlying JNI calls
- A tad bit increase in throughput, probably reverting the tiny drop
caused by 18b0e95659
Motivation:
Some users already use an SSLEngine implementation in finagle-native. It
wraps OpenSSL to get higher SSL performance. However, to take advantage
of it, finagle-native must be compiled manually, and it means we cannot
pull it in as a dependency and thus we cannot test our SslHandler
against the OpenSSL-based SSLEngine. For an instance, we had #2216.
Because the construction procedures of JDK SSLEngine and OpenSslEngine
are very different from each other, we also need to provide a universal
way to enable SSL in a Netty application.
Modifications:
- Pull netty-tcnative in as an optional dependency.
http://netty.io/wiki/forked-tomcat-native.html
- Backport NativeLibraryLoader from 4.0
- Move OpenSSL-based SSLEngine implementation into our code base.
- Copied from finagle-native; originally written by @jpinner et al.
- Overall cleanup by @trustin.
- Run all SslHandler tests with both default SSLEngine and OpenSslEngine
- Add a unified API for creating an SSL context
- SslContext allows you to create a new SSLEngine or a new SslHandler
with your PKCS#8 key and X.509 certificate chain.
- Add JdkSslContext and its subclasses
- Add OpenSslServerContext
- Add ApplicationProtocolSelector to ensure the future support for NPN
(NextProtoNego) and ALPN (Application Layer Protocol Negotiation) on
the client-side.
- Add SimpleTrustManagerFactory to help a user write a
TrustManagerFactory easily, which should be useful for those who need
to write an alternative verification mechanism. For example, we can
use it to implement an unsafe TrustManagerFactory that accepts
self-signed certificates for testing purposes.
- Add InsecureTrustManagerFactory and FingerprintTrustManager for quick
and dirty testing
- Add SelfSignedCertificate class which generates a self-signed X.509
certificate very easily.
- Update all our examples to use SslContext.newClient/ServerContext()
- SslHandler now logs the chosen cipher suite when handshake is
finished.
Result:
- Cleaner unified API for configuring an SSL client and an SSL server
regardless of its internal implementation.
- When native libraries are available, OpenSSL-based SSLEngine
implementation is selected automatically to take advantage of its
performance benefit.
- Examples take advantage of this modification and thus are cleaner.
Motivation:
When writing data from a server before the ssl handshake completes may not be written at all to the remote peer
if nothing else is written after the handshake was done.
Modification:
Correctly try to write pending data after the handshake was complete
Result:
Correctly write out all pending data
Motivation:
As discussed in #2250, it will become much less complicated to implement
deregistration and reregistration of a channel once #2250 is resolved.
Therefore, there's no need to deprecate deregister() and
channelUnregistered().
Modification:
- Undeprecate deregister() and channelUnregistered()
- Remove SuppressWarnings annotations where applicable
Result:
We (including @jakobbuchgraber) are now ready to play with #2250 at
master
Motivation:
4 and 5 were diverged long time ago and we recently reverted some of the
early commits in master. We must make sure 4.1 and master are not very
different now.
Modification:
Fix found differences
Result:
4.1 and master got closer.
Motivation:
4 and 5 were diverged long time ago and we recently reverted some of the
early commits in master. We must make sure 4.1 and master are not very
different now.
Modification:
Remove ChannelHandlerInvoker.writeAndFlush(...) and the related
implementations.
Result:
4.1 and master got closer.
Motivation:
4 and 5 were diverged long time ago and we recently reverted some of the
early commits in master. We must make sure 4.1 and master are not very
different now.
Modification:
Fix found differences
Result:
4.1 and master got closer.
Motivation:
Some SSLEngine implementations violate the contract and raises an
exception when SslHandler feeds an input buffer that contains multiple
SSL records to SSLEngine.unwrap(), while the expected behavior is to
decode the first record and return.
Modification:
- Modify SslHandler.decode() to keep the lengths of each record and feed
SSLEngine.unwrap() record by record to work around the forementioned
issue.
- Rename unwrap() to unwrapMultiple() and unwrapNonApp()
- Rename unwrap0() to unwrapSingle()
Result:
SslHandler now works OpenSSLEngine from finagle-native. Performance
impact remains unnoticeable. Slightly better readability. Fixes#2116.
Motivation:
Some Android SSLEngine implementations skip FINISHED handshake status
and go straightly into NOT_HANDSHAKING. This behavior blocks SslHandler
from notifying its handshakeFuture, because we do the notification when
SSLEngine enters the FINISHED state.
Modification:
When the current handshake state is NOT_HANDSHAKING and the
handshakeFuture is not fulfilled yet, treat NOT_HANDSHAKING as FINISHED.
Result:
Better Android compatibility - fixes#1823
Motivation:
When using System.getProperty(...) and various methods to get a ClassLoader it will fail when a SecurityManager is in place.
Modifications:
Use a priveled block if needed. This work is based in the PR #2353 done by @anilsaldhana .
Result:
Code works also when SecurityManager is present
Motivation:
In SslHandler.safeClose(...) we attach a ChannelFutureListener to the flushFuture and will notify the ChannelPromise which was used for close(...) in it. The problem here is that we only call ChannelHandlerContext.close(ChannelPromise) if Channel.isActive() is true and otherwise not notify it at all. We should just call ChannelHandlerContext.close(ChannelPromise) in all cases.
Modifications:
Always call ChannelHandlerContext.close(ChannelPromise) in the ChannelFutureListeiner
Result:
ChannelPromise used for close the Channel is notified in all cases
Motivation:
In ChunkedWriteHandler, there is a redundant variable that servers
no purpose. It implies that under some conditions you might not want
to flush.
Modifications:
Removed the variable and the if condition that read it. The boolean
was always true so just removing the if statement was fine.
Result:
Slightly less misleading code.
Motivation:
Currently we use System.currentTimeMillis() in our timeout handlers this is bad
for various reasons like when the clock adjusts etc.
Modifications:
Replace System.currentTimeMillis() with System.nanoTime()
Result:
More robust timeout handling
Motivation:
We don't really need to propagate an event when handling the event fails.
Modifications:
Do not use finally block in AbstractRemoteAddressFilter
Result:
AbstractRemoteaddressFilter does not forward an event in case of failure.
Motivation:
Recently merged ipfilter package has the following problems:
* AbstractIpFilterHandler could be improved to support any SocketAddress types rather than only InetSocketAddress.
* AbstractIpFilterHandler can be removed immediately after decision is made rather than keeping the outcome of the decision as an attribute.
* AbstractIpFilterHandler doesn't have a hook for the accepted addresses.
* The hook method (reject()) needs to be named in line with other handler methods (i.e. channelRejected())
* IpFilterRuleHandler should allow accepting zero rules - it's particularly useful for machine-configured setup (i.e. specifying zero rules disables ipfilter).
* IpFilterRuleType.ALLOW/DENY should be ACCEPT/REJECT for consistency.
Modifications:
* AbstractIpFilterHandler has been renamed to AbstractRemoteAddressFilter and now uses type parameter.
* Added channelAccepted() and renamed reject() to channelRejected()
* Added ChannelHandlerContext as a parameter of accept() so that accept() can add a listener to the closeFuture() of the channel. This way, UniqueIpFilter continue working even if we remove the filtering handler early.
* Various renames
* IpFilterRuleHandler -> RuleBasedIpFilter
* UniqueIpFilterHandler -> UniqueIpFilter
Result:
* Much cleaner API with more extensibility
Motivation:
ChunkedWriteHandler can sometimes fail to write the last chunk of a ChunkedInput due to an I/O error. Subsequently, the ChunkedInput's associated promise is marked as failure and the connection is closed. When the connection is closed, ChunkedWriteHandler attempts to clean up its message queue and to mark their promises as success or failure. However, because the promise of the ChunkedInput, which was consumed completely yet failed to be written, is already marked as failure, the attempt to mark it as success fails, leading a WARN level log.
Modification:
Use trySuccess() instead of setSuccess() so that the attempt to mark a ChunkedInput as success does not raise an exception even if the promise is already done.
Result:
Fixes#2249
- Use ': ' instead of '(...)' for simpler string concatenation and prettier presentation
- Optimize the overall performance of format*() methods
- All format*() methods are now expected to encode the channel information by themselves so that StringBuilder instances are created less often.
- Use a look-up table for generating per-row prefixes
- Hid formatByteBuf(), formatByteBufHolder(), and formatNonByteBuf() from user because a user can always override format(ctx, eventName, arg). For example, to disable hexdump:
protected void format(ChannelHandlerContext ctx, String eventName, Object arg) {
if (arg instanceof ByteBuf) {
super.format(ctx, eventName, arg.toString());
} else {
super.format(ctx, eventName, arg);
}
}
- Fixes#1905
- Call ctx.flush() only when necessary
- Improve the estimation of application and packet buffer sizes
- decode() method now tries to call unwrap() with as many SSL records as
possible to reduce the number of events triggered
The old implementation was broken and could lead to pending message never be picked up again until the user either explicit called flush or
resumeTransfer().
