Commit Graph

300 Commits

Author SHA1 Message Date
DhanaRaj Durairaj
eb27cd279c [#2494] Fix data curruption by ChannelTrafficShapingHandler
Motivation:
ChannelTrafficShapingHandler may corrupt inbound data stream by
scheduling the fireChannelRead event.

Modification:
Always call fireChannelRead(...) and only suspend reads after it

Result:
No more data corruption
2014-06-03 08:38:05 +02:00
Trustin Lee
2f8c13e85a Fix NPE 2014-05-22 10:29:24 +09:00
Trustin Lee
5ce410c69e Future compatibility with TLS ALPN
Motivation:

According to TLS ALPN draft-05, a client sends the list of the supported
protocols and a server responds with the selected protocol, which is
different from NPN.  Therefore, ApplicationProtocolSelector won't work
with ALPN

Modifications:

- Use Iterable<String> to list the supported protocols on the client
  side, rather than using ApplicationProtocolSelector
- Remove ApplicationProtocolSelector

Result:

Future compatibility with TLS ALPN
2014-05-22 10:03:02 +09:00
Trustin Lee
27b67e981e Fix NPE in OpenSslEngine 2014-05-21 20:02:40 +09:00
Trustin Lee
ac968fbf7b Escape a colon in protocol names 2014-05-21 17:45:18 +09:00
Trustin Lee
861ed1e7ad Add unified NextProtoNego extension support to SslContext
Motivation:

- OpenSslEngine and JDK SSLEngine (+ Jetty NPN) have different APIs to
  support NextProtoNego extension.
  - It is impossible to configure NPN with SslContext when the provider
    type is JDK.

Modification:

- Implement NextProtoNego extension by overriding the behavior of
  SSLSession.getProtocol() for both OpenSSLEngine and JDK SSLEngine.
  - SSLEngine.getProtocol() returns a string delimited by a colon (':')
    where the first component is the transport protosol (e.g. TLSv1.2)
    and the second component is the name of the application protocol
- Remove the direct reference of Jetty NPN classes from the examples
- Add SslContext.newApplicationProtocolSelector

Result:

- A user can now use both JDK SSLEngine and OpenSslEngine for NPN-based
  protocols such as HTTP2 and SPDY
2014-05-21 17:24:52 +09:00
Trustin Lee
4882377c27 Work around the JVM crash that occurs when cipher suite uses GCM
Motivation:

For an unknown reason, JVM of JDK8 crashes intermittently when
SslHandler feeds a direct buffer to SSLEngine.unwrap() *and* the current
cipher suite has GCM (Galois/Counter Mode) enabled.

Modifications:

Convert the inbound network buffer to a heap buffer when the current
cipher suite is using GCM.

Result:

JVM does not crash anymore.
2014-05-19 11:47:20 +09:00
Trustin Lee
ae61b12b9e Reduce memory usage of SslHandler when OpenSslEngine is in use
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.
2014-05-18 05:10:41 +09:00
Trustin Lee
28c390d86d Fix buffer leaks during PEM to KeyStore conversion 2014-05-18 04:29:50 +09:00
Trustin Lee
167a685a5f Fix JDK 8 compatibility issue with OpenJdkSelfSignedCertGenerator
- X509CertInfo.setSubject/setIssuer() requires X500Name instead of
  CertificateSubjectName/CertificateIssuerName.
2014-05-18 03:47:55 +09:00
Trustin Lee
9125060752 Optimize SslHandler in an OpenSslEngine-friendly way
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
2014-05-18 03:33:00 +09:00
Trustin Lee
b6c0c0c95f Add an OpenSslEngine and the universal API for enabling SSL
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.
2014-05-18 02:54:06 +09:00
Norman Maurer
0aea9eaed5 Correctly write pending data after ssl handshake completes. Related to [#2437]
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
2014-04-30 14:23:18 +02:00
Trustin Lee
0fceef8ab6 Undeprecate deregister() and chanelUnregistered()
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
2014-04-25 16:40:42 +09:00
Trustin Lee
d765f6b870 Synchronized between 4.1 and master (part 3)
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.
2014-04-25 16:17:59 +09:00
Trustin Lee
b9039eaa82 Synchronized between 4.1 and master again (part 2)
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.
2014-04-25 15:06:26 +09:00
Trustin Lee
db3709e652 Synchronized between 4.1 and 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.
2014-04-25 00:38:02 +09:00
Trustin Lee
af9ab8b370 Feed only a single SSL record to SSLEngine.unwrap()
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.
2014-04-20 17:33:04 +09:00
Trustin Lee
e9161147a5 Work around an Android SSLEngine issue
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
2014-04-18 17:59:48 +09:00
Norman Maurer
88481131be [#2353] Use a privileged block to get ClassLoader and System property if needed
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
2014-04-08 14:12:25 +02:00
Norman Maurer
3eec26b0a2 [#2358] SslHandler.safeClose(...) may not notify the ChannelPromise
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
2014-04-03 13:27:33 +02:00
Ian Barfield
cf9c1f946a Deleting redundant needsFlush boolean
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.
2014-03-29 20:21:19 +01:00
Norman Maurer
9856563144 Replace usage of System.currentTimeMillis() with System.nanoTime()
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
2014-03-18 16:06:16 +09:00
Trustin Lee
1e4c22453c Do not use finally to propagate events in AbstractRemoteAddressFilter
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.
2014-03-12 16:17:40 +09:00
Trustin Lee
cf275237c9 Overall clean-up of ipfilter package
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
2014-03-12 16:06:04 +09:00
Jakob Buchgraber
386cc2cb73 ipfilter implementation for netty 4/5 [#2129] 2014-03-10 20:43:39 +01:00
Norman Maurer
918dd54a72 [#2261] Correct javadoc of ChunkedInput 2014-03-03 07:04:11 +01:00
Trustin Lee
1884a5697c Avoid unnecessary IllegalStateException in ChunkedWriteHandler
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
2014-02-20 17:14:25 -08:00
Trustin Lee
abcb39b638 Do not use String.format() for log message generation
- It's slow.
2014-02-13 19:31:17 -08:00
Trustin Lee
df346a023b Change the return type of EmbeddedChannel.read*() from Object to an ad-hoc type parameter
.. so that there's no need to explicitly down-cast.

Fixes #2067
2014-02-13 17:19:26 -08:00
Trustin Lee
a8bc720977 Fixed buffer leaks in LoggingHandlerTest 2014-02-13 17:03:21 -08:00
Trustin Lee
3d874f0bd7 Improve documentation for the two-args formatter in LoggingHandler 2014-02-13 16:51:26 -08:00
Trustin Lee
fbed62249c Rename formatUserMessage to formatSimple and use it in two-args formatter in LoggingHandler 2014-02-13 16:51:15 -08:00
Trustin Lee
a9150ee7d6 Simplify two-args message formatter in LoggingHandler 2014-02-13 16:51:06 -08:00
Trustin Lee
2e58497160 Make LoggingHandler.appendHexDump(..) protected for the subclasses 2014-02-13 16:50:50 -08:00
Trustin Lee
835b4443f3 Optimize and clean up LoggingHandler
- 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);
        }
    }
2014-02-13 16:50:31 -08:00
Trustin Lee
499033d44f Add a shortcut method for collision-free naming 2014-02-13 15:17:09 -08:00
Trustin Lee
c4c71e6d28 Fix the potential copyright issue in SocksCommonUtils
- Add StringUtil.toHexString() methods which are based on LoggingHandler's lookup table implementation, and use it wherever possible
2014-02-06 15:00:06 -08:00
Norman Maurer
37e6588845 [#2159] Not fail the ChannelPromise with WriteTimeoutException to prevent warning 2014-01-30 07:02:06 +01:00
Trustin Lee
bc21443ea9 Fix a regression in SslHandler where delegated tasks run in a different executor makes the session hang
- Fixes #2098
- Deprecate specifying an alternative Executor for delegated tasks for SslHandler
2014-01-09 18:08:05 +09:00
Trustin Lee
f7a3881536 Fix a bug in SslHandler where a ClassCastException is raised when non-ByteBuf message is passed
- Fixes #1828
2013-12-16 16:30:41 +09:00
Norman Maurer
b3d8c81557 Fix all leaks reported during tests
- One notable leak is from WebSocketFrameAggregator
- All other leaks are from tests
2013-12-07 00:44:56 +09:00
Norman Maurer
7c7acdcaac [#2033] Correctly handle adding of IdleStateHandler after Channel was already active and registered 2013-12-03 13:56:43 +01:00
Alex Petrov
90309f9065 Improve doc of IdleStateHandler according to example given in UptimeClientHandler (L57) 2013-11-20 10:24:33 +01:00
Trustin Lee
b65b4199dc Fix regression introduced by 4c7fa950cc
- Some promises were not fulfilled when SSLEngine produces 0 bytes.
2013-11-14 15:09:20 +09:00
Trustin Lee
11f95c78e2 Optimize SslHandler
- 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
2013-11-08 17:41:16 +09:00
Trustin Lee
51ca4f3e91 Fix a bug where SslHandler doesn't sometimes handle renegotiation correctly
- Fixes #1964
2013-11-04 16:54:13 +09:00
Trustin Lee
1c2352e6a0 Replace constructor calls on UniqueName and its subtypes with valueOf() wherever possible 2013-10-25 20:58:53 +09:00
Norman Maurer
79562d5891 [#1936] Fix example in javadoc 2013-10-20 09:32:38 +02:00
Norman Maurer
bcdb3e88d8 [#1934] Correctly log handshake errors and not print them to STDERR 2013-10-18 17:39:04 +02:00