Commit Graph

524 Commits

Author SHA1 Message Date
Norman Maurer
1b383c5de8 Correctly handle errors when using OpenSSL
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.
2015-06-21 21:07:12 +02:00
Norman Maurer
303cb53523 [#3883] OpenSSL SSLSession returns incorrect peer principal
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.
2015-06-17 06:35:58 +02:00
Norman Maurer
9a30b6860f [#3798] Extract dump method to ByteBufUtil
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.
2015-06-12 14:05:32 +02:00
Norman Maurer
657f6a8a72 Not skip first cert when using OpenSslClientContext
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.
2015-06-10 09:31:43 +02:00
Norman Maurer
74c0f4ce14 Guard against race when calling SslHandler.handshakeFuture().sync()
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.
2015-06-08 09:17:15 +02:00
Trustin Lee
263e6979a6 Replace SpdyOrHttpChooser and Http2OrHttpChooser with ApplicationProtocolNegotiationHandler
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
- Deprecate SpdyOrHttpChooser

Result:

- Less code duplication
- A user can perform dynamic pipeline configuration that follows ALPN/NPN for any protocols.
2015-06-05 14:51:21 +09:00
Trustin Lee
67e02dad0a Improve the API design of Http2OrHttpChooser and SpdyOrHttpChooser
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
2015-06-05 14:51:20 +09:00
Norman Maurer
9e125d8456 Expose SSL_CTX and SSL pointers
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.
2015-06-05 07:24:57 +02:00
Norman Maurer
7706d2309c Reduce calls to System.nanoTime() and object creation in IdleStateHandler. Related to [#3808]
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.
2015-05-27 13:55:01 +02:00
Norman Maurer
bb230579f8 Do not try to init TrustManagerFactory if trustCertChainFile is null.
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.
2015-05-27 13:45:34 +02:00
Scott Mitchell
08cb650b64 Consistent use of SSLHandshakeException for ALPN
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.
2015-05-26 16:10:43 -07:00
johnou
0decf2f647 Allow writing with void promise if IdleStateHandler is configured in pipeline.
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.
2015-05-25 21:12:51 +02:00
Norman Maurer
6703318010 Only call System.nanoTime() if no read batch is ongoing. Related to [#3808]
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.
2015-05-25 18:20:50 +02:00
nmittler
2ca6a70211 Upgrading Jetty alpn-api version
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.
2015-05-22 13:15:21 -07:00
Robert Varga
addc4413aa Do not call System.nanoTime() in ReadTimeoutHandler.channelRead()
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.
2015-05-21 07:13:39 +02:00
Robin Stocker
214e889937 Fix typo in FingerprintTrustManagerFactory docs 2015-05-18 08:31:00 +02:00
Norman Maurer
b02f52a680 [#3784] Support hostname verification when using OpenSSLEngine
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.
2015-05-18 08:01:38 +02:00
Eric Anderson
b44fe3fd68 Add missing SslContextBuilder.forServer(KeyManagerFactory)
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
2015-05-11 22:09:50 +02:00
Norman Maurer
d1c46ca987 [maven-release-plugin] prepare for next development iteration 2015-05-07 11:33:47 -04:00
Norman Maurer
005d4a42fc [maven-release-plugin] prepare release netty-4.0.28.Final 2015-05-07 11:33:09 -04:00
Norman Maurer
0f4d3a981e Revert "[maven-release-plugin] prepare for next development iteration"
This reverts commit 3c10ffab5e.
2015-05-07 11:02:03 -04:00
Norman Maurer
3c10ffab5e [maven-release-plugin] prepare for next development iteration 2015-05-07 09:09:23 -04:00
Norman Maurer
4581c441c9 Allow rejection of remote initiated renegotiation
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.
2015-05-07 14:41:00 +02:00
Norman Maurer
d1c235d454 Fix possible IllegalStateException caused by closeNotifyTimeout when using SslHandler
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.
2015-05-06 21:50:09 +02:00
Norman Maurer
0027d830d4 Only run OpenSslEngineTests if OpenSsl is installed. Related to [#3732] 2015-05-06 10:41:44 +02:00
Norman Maurer
9dc236054e Add support for mutual auth when using OpenSslEngine.
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
2015-05-06 09:07:18 +02:00
Eric Anderson
97eba4eb66 Fix SslContextBuilder swapping client and server
The 'forClient' boolean was swapped to 'forServer' in code review of #3531.
Not all locations were updated.
2015-04-20 17:24:43 -07:00
Norman Maurer
5cd541c537 Fix handling of non-auto read for ByteToMessageDecoder and SslHandler
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.
2015-04-20 09:02:47 +02:00
Norman Maurer
d376bf6a58 Reduce object allocation during wrap/unwrap while handshake is in progress
Motivation:

Unnecessary object allocation is currently done during wrap/unwrap while a handshake is still in progress.

Modifications:

Use static instances when possible.

Result:

Less object creations.
2015-04-20 06:47:18 +02:00
Norman Maurer
19179f9fee Allow to get version of available OpenSSL library
Motivation:

Sometimes it's useful to get informations about the available OpenSSL library that is used for the OpenSslEngine.

Modifications:

Add two new methods which allows to get the available OpenSSL version as either
an int or an String.

Result:

Easy to access details about OpenSSL version.
2015-04-18 20:56:15 +02:00
Eric Anderson
6668b3c40d The "null" ClassLoader is the bootstrap ClassLoader
Motivation:
Class.forName() documents that null will use bootstrap loader:
http://docs.oracle.com/javase/8/docs/api/java/lang/Class.html#forName-java.lang.String-boolean-java.lang.ClassLoader-

But the link between "null" and bootstrap loader is even more explicit
in ClassLoader's documentation:
http://docs.oracle.com/javase/8/docs/api/java/lang/ClassLoader.html#getParent--

The current code is trying to use the bootstrap loader but seems to have
not been aware of the meaning of null.

Modifications:
Use "null" as the class loader when we want to load classes in the
bootstrap loader.

Result:
More reliable ALPN/NPN loading and simpler code.
2015-04-16 17:25:18 +02:00
Norman Maurer
9e5dd21d23 Fix regression introduced by cherry-pick bd224286f5 2015-04-14 17:59:33 +02:00
Norman Maurer
e907ae57a2 Add support for EC Keys when using SslServerContext
Motivation:

Sometimes it's useful to use EC keys and not DSA or RSA. We should support it.

Modifications:

Support EC keys and share the code between JDK and Openssl impl.

Result:

It's possible to use EC keys now.
2015-04-14 08:44:38 +02:00
Eric Anderson
fdc396674e [#3531] Create SslContext.Builder
Motivation:

SslContext factory methods have gotten out of control; it's past time to
swap to a builder.

Modifications:

New Builder class. The existing factory methods must be left as-is for
backward compatibility.

Result:

Fixes #3531
2015-04-14 07:27:55 +02:00
Norman Maurer
97a3308c7e Add support for ALPN when using openssl + NPN client mode and support for CipherSuiteFilter
Motivation:

To support HTTP2 we need APLN support. This was not provided before when using OpenSslEngine, so SSLEngine (JDK one) was the only bet.
Beside this CipherSuiteFilter was not supported

Modifications:

- Upgrade netty-tcnative and make use of new features to support ALPN and NPN in server and client mode.
- Guard against segfaults after the ssl pointer is freed
- support correctly different failure behaviours
- add support for CipherSuiteFilter

Result:

Be able to use OpenSslEngine for ALPN / NPN for server and client.
2015-04-10 18:59:58 +02:00
Frederic Bregier
2ab2e2d6b7 Fix incorrect null value check in TrafficCounter
In TrafficCounter, a recent change makes the contract of the API (the
constructor) wrong and lead to issue with GlobalChannelTrafficCounter
where executor must be null.

Motivation:
TrafficCounter executor argument in constructor might be null, as
explained in the API, for some particular cases where no executor are
needed (relevant tasks being taken by the caller as in
GlobalChannelTrafficCounter).
A null pointer exception is raised while it should not since it is
legal.

Modifications:
Remove the 2 null checking for this particular attribute.
Note that when null, the attribute is not reached nor used (a null
checking condition later on is applied).

Result:
No more null exception raized while it should not.

This shall be made also to 4.0, 4.1 (present) and master. 3.10 is not
concerned.
2015-04-06 19:07:13 +02:00
Norman Maurer
f2fedbcdef [maven-release-plugin] prepare for next development iteration 2015-03-31 22:06:30 -04:00
Norman Maurer
054e7c5d17 [maven-release-plugin] prepare release netty-4.0.27.Final 2015-03-31 22:05:43 -04:00
Trustin Lee
4d8f824358 Fix unbounded expansion of cumulative buffer in SslHandler
Related: #3567

Motivation:

SslHandler.channelReadComplete() forgets to call
super.channelReadComplete(), which discards read bytes from the
cumulative buffer.  As a result, the cumulative buffer can expand its
capacity unboundedly.

Modifications:

Call super.channelReadComplete() instead of calling
ctx.fireChannelReadComplete()

Result:

Fixes #3567
2015-04-02 14:57:02 +09:00
Trustin Lee
28caa873b3 Don't trigger IOException at ChunkedStream.isEndOfInput()
Related: #3368

Motivation:

ChunkedWriteHandler checks if the return value of
ChunkedInput.isEndOfInput() after calling ChunkedInput.close().

This makes ChunkedStream.isEndOfInput() trigger an IOException, which is
originally triggered by PushBackInputStream.read().

By contract, ChunkedInput.isEndOfInput() should not raise an IOException
even when the underlying stream is closed.

Modifications:

Add a boolean flag that keeps track of whether the underlying stream has
been closed or not, so that ChunkedStream.isEndOfInput() does not
propagate the IOException from PushBackInputStream.

Result:

Fixes #3368
2015-03-31 11:31:54 +09:00
Norman Maurer
0c6ba4aab6 Add supported for X509ExtendedTrustManager when using OpenSslEngine
Motivation:

For some use cases X509ExtendedTrustManager is needed as it allows to also access the SslEngine during validation.

Modifications:

Add support for X509ExtendedTrustManager on java >= 7

Result:

It's now possible to use X509ExtendedTrustManager with OpenSslEngine
2015-03-30 09:05:09 +02:00
Leonardo Freitas Gomes
034e0bd8d2 Ensure server preference order in ALPN
Motivation:
With the current implementation the client protocol preference list
takes precedence over the one of the server, since the select method
will return the first item, in the client list, that matches any of the
protocols supported by the server. This violates the recommendation of
http://tools.ietf.org/html/rfc7301#section-3.2.

It will also fail with the current implementation of Chrome, which
sends back Extension application_layer_protocol_negotiation, protocols:
[http/1.1, spdy/3.1, h2-14]

Modifications:
Changed the protocol negotiator to prefer server’s list. Added a test
case that demonstrates the issue and that is fixed with the
modifications of this commit.

Result:
Server’s preference list is used.
2015-03-17 07:29:10 +01:00
Trustin Lee
54293003ef Add a new constructor without handler parameter to TrafficCounter
Related: #3476

Motivation:

Some users use TrafficCounter for other uses than we originally
intended, such as implementing their own traffic shaper.  In such a
case, a user does not want to specify an AbstractTrafficShapingHandler.

Modifications:

- Add a new constructor that does not require an
  AbstractTrafficShapingHandler, so that a user can use it without it.
- Simplify TrafficMonitoringTask
- Javadoc cleanup

Result:

We open the possibility of using TrafficCounter for other purposes than
just using it with AbstractTrafficShapingHandler.  Eventually, we could
generalize it a little bit more, so that we can potentially use it for
other uses.
2015-03-10 11:26:41 +09:00
Norman Maurer
37264bb72b [maven-release-plugin] prepare for next development iteration 2015-03-02 01:31:30 -05:00
Norman Maurer
0dbc96cffd [maven-release-plugin] prepare release netty-4.0.26.Final 2015-03-02 01:30:58 -05:00
Norman Maurer
e99d89c04d [maven-release-plugin] rollback the release of netty-4.0.26.Final 2015-02-28 21:28:06 +01:00
Norman Maurer
b86e2e6ac0 [maven-release-plugin] prepare release netty-4.0.26.Final 2015-02-28 13:55:01 -05:00
Norman Maurer
f7ccdd5d3b Various performance optimizations in OpenSslEngine
Motivation:

There are various places in OpenSslEngine wher we can do performance optimizations.

Modifications:

- Reduce JNI calls when possible
- Detect finished handshake as soon as possible
- Eliminate double calculations
- wrap multiple ByteBuffer if possible in a loop

Result:

Better performance
2015-02-09 06:20:09 +01:00
Norman Maurer
1c01dd1efc Log only on debug log level in OpenSslEngine
Motivation:

At the moment we log priming read and handshake errors via info log level and still throw a SSLException that contains the error. We should only log with debug level to generate less noise.

Modifications:

Change logging to debug level.

Result:

Less noise .
2015-02-07 06:01:41 +01:00
scottmitch
8e2f91b424 SonarQube issues OpenSslEngine
Motivation:
SonarQube (clinker.netty.io/sonar) reported a few 'critical' issues related to the OpenSslEngine.

Modifications:
- Remove potential for dereference of null variable.
- Remove duplicate null check and TODO cleanup.

Results:
Less potential for null dereference, cleaner code, and 1 less TODO.
2015-02-03 20:01:40 +01:00