Commit Graph

1111 Commits

Author SHA1 Message Date
Norman Maurer
4079189f6b
Allow to offload / customize key signing operations when using BoringSSL. (#8943)
Motivation:

BoringSSL allows to customize the way how key signing is done an even offload it from the IO thread. We should provide a way to plugin an own implementation when BoringSSL is used.

Modifications:

- Introduce OpenSslPrivateKeyMethod that can be used by the user to implement custom signing by using ReferenceCountedOpenSslContext.setPrivateKeyMethod(...)
- Introduce static methods to OpenSslKeyManagerFactory which allows to create a KeyManagerFactory which supports to do keyless operations by let the use handle everything in OpenSslPrivateKeyMethod.
- Add testcase which verifies that everything works as expected

Result:

A user is able to customize the way how keys are signed.
2019-04-08 20:17:44 +02:00
Farid Zakaria
4373a1fba2 Increase default bits for SelfSignedCertificate (#9019)
Motivation:
During OpenSsl.java initialization, a SelfSignedCertificate is created
during the static initialization block to determine if OpenSsl
can be used.

The default key strength for SelfSignedCertificate was too low if FIPS
mode is used and BouncyCastle-FIPS is the only available provider
(necessary for compliance). A simple fix is to just augment the key
strength to the minimum required about by FIPS.

Modification:
Set default key bit length to 2048 but also allow it to be dynamically set via a system property for future proofing to more stricter security compliance.

Result:
Fixes #9018

Signed-off-by: Farid Zakaria <farid.m.zakaria@gmail.com>
2019-04-08 20:08:59 +02:00
Norman Maurer
4b83be1ceb
We should fail fast if the given PrivateKey or X509Certificate chain is not supported by the used SslProvider. (#9009)
Motivation:

Some SslProvider do support different types of keys and chains. We should fail fast if we can not support the type.

Related to https://github.com/netty/netty-tcnative/issues/455.

Modifications:

- Try to parse key / chain first and if if this fails throw and SslException
- Add tests.

Result:

Fail fast.
2019-04-08 15:20:14 +02:00
Norman Maurer
547a375737
Always include initial handshake exception when throwing SslHandshakeException (#9008)
Motivation:

A callback may already have stored a initial handshake exception in ReferenceCountedOpenSslEngine so we should include it when throwing a SslHandshakeException to ensure the user has all the infos when debugging.

Modifications:

Include initial handshake exception

Result:

Include all erros when throwing the SslHandshakeException.
2019-04-05 09:55:32 +02:00
Norman Maurer
f8c89e2e05
Remove call to SSL.setHostNameValidation(...) as it is done in the TrustManager (#8981)
Motivation:

We do not need to call SSL.setHostNameValidation(...) as it should be done as part of the TrustManager implementation. This is consistent with the JDK implementation of SSLEngine.

Modifications:

Remove call to SSL.setHostNameValidation(...)

Result:

More consistent behaviour between our SSLEngine implementation and the one that comes with the JDK.
2019-04-01 21:02:36 +02:00
Norman Maurer
f7359aa742
Use SSL.setKeyMaterial(...) to test if the KeyManagerFactory is supported (#8985)
Motivation:

We use SSL.setKeyMaterial(...) in our implementation when using the KeyManagerFactory so we should also use it to detect if we can support KeyManagerFactory.

Modifications:

Use SSL.setKeyMaterial(...) as replacement for SSL.setCertificateBio(...)

Result:

Use the same method call to detect if KeyManagerFactory can be supported as we use in the real implementation.
2019-04-01 12:03:05 +02:00
Norman Maurer
86ecad517c
Consolidate creation of SslHandshakeException when caused by a callback that is used in the native SSL implementation. (#8979)
Motivation:

We have multiple places where we store the exception that was produced by a callback in ReferenceCountedOpenSslEngine, and so have a lot of code-duplication.

Modifications:

- Consolidate code into a package-private method that is called from the callbacks if needed

Result:

Less code-duplication and cleaner code.
2019-03-26 11:38:37 +01:00
Norman Maurer
41b0236815
Allow to offload certificate validation when using BoringSSL (#8974)
Motivation:

BoringSSL supports offloading certificate validation to a different thread. This is useful as it may need to do blocking operations and so may block the EventLoop.

Modification:

- Adjust ReferenceCountedOpenSslEngine to correctly handle offloaded certificate validation (just as we already have code for certificate selection).

Result:

Be able to offload certificate validation when using BoringSSL.
2019-03-24 20:03:30 +01:00
Norman Maurer
33e2f5609d Revert "Allow to offload certificate validation when using BoringSSL (#8908)"
This reverts commit 316dd98284.
2019-03-24 09:33:42 +01:00
Norman Maurer
316dd98284
Allow to offload certificate validation when using BoringSSL (#8908)
Motivation:

BoringSSL supports offloading certificate validation to a different thread. This is useful as it may need to do blocking operations and so may block the EventLoop.

Modification:

- Adjust ReferenceCountedOpenSslEngine to correctly handle offloaded certificate validation (just as we already have code for certificate selection).

Result:

Be able to offload certificate validation when using BoringSSL.
2019-03-24 09:03:27 +01:00
Norman Maurer
33128c85f8
Add SSLEngineTest to ensure Signature Algorithms are present during KeyManager calls. (#8965)
Motivation:

We had a bug which could case ExtendedSSLSession.getPeerSupportedSignatureAlgorithms() return an empty array when using BoringSSL. This testcase verifies we correctly return algorithms after the fix in https://github.com/netty/netty-tcnative/pull/449.

Modifications:

Add testcase to verify behaviour.

Result:

Ensure we correctly retuen the algorithms.
2019-03-24 07:35:03 +01:00
Norman Maurer
1ca37a0edb
Correctly detect exeception cause when using BoringSSL in SslErrorTest (#8970)
Motivation:

e9ce5048df added a testcase to ensure we correctly send the alert in all cases but did use a too strict message matching which did not work for BoringSSL as it not uses whitespaces but underscores.

Modifications:

Make the message matching less strict.

Result:

Test pass also when using BoringSSL.
2019-03-22 16:30:53 +01:00
Norman Maurer
e9ce5048df
Correctly produce ssl alert when certificate validation fails on the client-side when using native SSL implementation. (#8949)
Motivation:

When the verification of the server cert fails because of the used TrustManager on the client-side we need to ensure we produce the correct alert and send it to the remote peer before closing the connection.

Modifications:

- Use the correct verification mode on the client-side by default.
- Update tests

Result:

Fixes https://github.com/netty/netty/issues/8942.
2019-03-18 18:42:11 +01:00
root
92b19cfedd [maven-release-plugin] prepare for next development iteration 2019-03-08 08:55:45 +00:00
root
ff7a9fa091 [maven-release-plugin] prepare release netty-4.1.34.Final 2019-03-08 08:51:34 +00:00
Norman Maurer
3e24e9f6ff
ReferenceCountedOpenSslEngines SSLSession must provide local certific… (#8918)
Motivation:

The SSLSession that is returned by SSLEngine.getHandshakeSession() must be able to provide the local certificates when the TrustManager is invoked on the server-side.

Modifications:

- Correctly return the local certificates
- Add unit test

Result:

Be able to obtain local certificates from handshake SSLSession during verification on the server side.
2019-03-08 06:47:28 +01:00
Norman Maurer
0de5402337
Add interopt tests between Conscrypt and OpenSSL SSLEngine implementations. (#8919)
Motivation:

In the past we found a lot of SSL related bugs because of the interopt tests we have in place between different SSLEngine implementations. We should have as many of these interopt tests as possible for this reason.

Modifications:

- Add interopt tests between Conscrypt and OpenSSL SSLEngine implementations

Result:

More tests for SSL.
2019-03-07 09:36:59 +01:00
Norman Maurer
39fcdb3e0d
Support delegating task when using ReferenceCountedOpenSslEngine. (#8859)
Motivation:

SSLEngine API has a notion of tasks that may be expensive and offload these to another thread. We did not support this when using our native implementation but can now for various operations during the handshake.

Modifications:

- Support offloading tasks during the handshake when using our native SSLEngine implementation
- Correctly handle the case when NEED_TASK is returned and nothing was consumed / produced yet

Result:

Be able to offload long running tasks from the EventLoop when using SslHandler with our native SSLEngine.
2019-03-05 09:17:18 +01:00
Norman Maurer
6f507dfeed
Only remove ReferenceCountedOpenSslEngine from OpenSslEngineMap when engine is destroyed (#8905)
Motivation:

We must only remove ReferenceCountedOpenSslEngine from OpenSslEngineMap when engine is destroyed as the verifier / certificate callback may be called multiple times when the remote peer did initiate a renegotiation.
If we fail to do so we will cause an NPE like this:

```
13:16:36.750 [testsuite-oio-worker-5-18] DEBUG i.n.h.s.ReferenceCountedOpenSslServerContext - Failed to set the server-side key material
java.lang.NullPointerException: null
	at io.netty.handler.ssl.OpenSslKeyMaterialManager.setKeyMaterialServerSide(OpenSslKeyMaterialManager.java:69)
	at io.netty.handler.ssl.ReferenceCountedOpenSslServerContext$OpenSslServerCertificateCallback.handle(ReferenceCountedOpenSslServerContext.java:212)
	at io.netty.internal.tcnative.SSL.readFromSSL(Native Method)
	at io.netty.handler.ssl.ReferenceCountedOpenSslEngine.readPlaintextData(ReferenceCountedOpenSslEngine.java:575)
	at io.netty.handler.ssl.ReferenceCountedOpenSslEngine.unwrap(ReferenceCountedOpenSslEngine.java:1124)
	at io.netty.handler.ssl.ReferenceCountedOpenSslEngine.unwrap(ReferenceCountedOpenSslEngine.java:1236)
	at io.netty.handler.ssl.ReferenceCountedOpenSslEngine.unwrap(ReferenceCountedOpenSslEngine.java:1279)
	at io.netty.handler.ssl.SslHandler$SslEngineType$1.unwrap(SslHandler.java:217)
	at io.netty.handler.ssl.SslHandler.unwrap(SslHandler.java:1330)
	at io.netty.handler.ssl.SslHandler.decodeNonJdkCompatible(SslHandler.java:1237)
	at io.netty.handler.ssl.SslHandler.decode(SslHandler.java:1274)
	at io.netty.handler.codec.ByteToMessageDecoder.decodeRemovalReentryProtection(ByteToMessageDecoder.java:502)
	at io.netty.handler.codec.ByteToMessageDecoder.callDecode(ByteToMessageDecoder.java:441)
	at io.netty.handler.codec.ByteToMessageDecoder.channelRead(ByteToMessageDecoder.java:278)
	at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:359)
	at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:345)
	at io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:337)
	at io.netty.channel.DefaultChannelPipeline$HeadContext.channelRead(DefaultChannelPipeline.java:1408)
	at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:359)
	at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:345)
	at io.netty.channel.DefaultChannelPipeline.fireChannelRead(DefaultChannelPipeline.java:930)
	at io.netty.channel.oio.AbstractOioByteChannel.doRead(AbstractOioByteChannel.java:170)
	at io.netty.channel.oio.AbstractOioChannel$1.run(AbstractOioChannel.java:40)
	at io.netty.channel.ThreadPerChannelEventLoop.run(ThreadPerChannelEventLoop.java:69)
	at io.netty.util.concurrent.SingleThreadEventExecutor$5.run(SingleThreadEventExecutor.java:905)
	at io.netty.util.concurrent.FastThreadLocalRunnable.run(FastThreadLocalRunnable.java:30)
	at java.base/java.lang.Thread.run(Thread.java:834)
```

While the exception is kind of harmless (as we will reject the renegotiation at the end anyway) it produces some noise in the logs.

Modifications:

Don't remove engine from map after handshake is complete but wait for it to be removed until the engine is destroyed.

Result:

No more NPE and less noise in the logs.
2019-03-01 19:31:06 +01:00
Konstantin Lutovich
e609b5eeb7 Close consumed inputs in ChunkedWriteHandler (#8876)
Motivation:

ChunkedWriteHandler needs to close both successful and failed
ChunkInputs. It used to never close successful ones.

Modifications:

* ChunkedWriteHandler always closes ChunkInput before completing
the write promise. 
* Ensure only ChunkInput#close() is invoked
on a failed input.
* Ensure no methods are invoked on a closed input.

Result:

Fixes https://github.com/netty/netty/issues/8875.
2019-02-28 21:13:56 +01:00
Norman Maurer
c6d3792df0
Correctly resume wrap / unwrap when SslTask execution completes (#8899)
Motivation:

fa6a8cb09c introduced correct dispatching of delegated tasks for SSLEngine but did not correctly handle some cases for resuming wrap / unwrap after the task was executed. This could lead to stales, which showed up during tests when running with Java11 and BoringSSL.

Modifications:

- Correctly resume wrap / unwrap in all cases.
- Fix timeout value which was changed in previous commit by mistake.

Result:

No more stales after task execution.
2019-02-28 20:29:40 +01:00
Norman Maurer
d3d0b6478b
Update JDK12 and 13 to latest EA releases. (#8809)
Motivation:

We use outdated EA releases when building and testing with JDK 12 and 13.

Modifications:

- Update versions.
- Add workaround for possible JDK12+ bug.

Result:

Use latest releases
2019-02-28 13:54:04 +01:00
Norman Maurer
fa6a8cb09c
Support using an Executor to offload blocking / long-running tasks wh… (#8847)
Motivation:

The SSLEngine does provide a way to signal to the caller that it may need to execute a blocking / long-running task which then can be offloaded to an Executor to ensure the I/O thread is not blocked. Currently how we handle this in SslHandler is not really optimal as while we offload to the Executor we still block the I/O Thread.

Modifications:

- Correctly support offloading the task to the Executor while suspending processing of SSL in the I/O Thread
- Add new methods to SslContext to specify the Executor when creating a SslHandler
- Remove @deprecated annotations from SslHandler constructor that takes an Executor
- Adjust tests to also run with the Executor to ensure all works as expected.

Result:

Be able to offload long running tasks to an Executor when using SslHandler. Partly fixes https://github.com/netty/netty/issues/7862 and https://github.com/netty/netty/issues/7020.
2019-02-11 09:47:44 +01:00
Norman Maurer
cd3254df88
Update to new checkstyle plugin (#8777) (#8780)
Motivation:

We need to update to a new checkstyle plugin to allow the usage of lambdas.

Modifications:

- Update to new plugin version.
- Fix checkstyle problems.

Result:

Be able to use checkstyle plugin which supports new Java syntax.
2019-01-25 11:58:42 +01:00
root
cf03ed0478 [maven-release-plugin] prepare for next development iteration 2019-01-21 12:26:44 +00:00
root
37484635cb [maven-release-plugin] prepare release netty-4.1.33.Final 2019-01-21 12:26:12 +00:00
Oleksii Kachaiev
7988cfec0a Correctly propagate write failures from ChunkedWriteHandler (#8716)
Motivation:

ChunkedWriteHandler should report write operation as failed
in case *any* chunked was not written. Right now this is not
true for the last chunk.

Modifications:

* Check if the appropriate write operation was succesfull when
  reporting the last chunk

* Skip writing chunks if the write operation was already marked
  as "done"

* Test cases to cover write failures when dealing with chunked input

Result:

Fix https://github.com/netty/netty/issues/8700
2019-01-16 11:07:59 +01:00
kashike
6fdd7fcddb Fix minor spelling issues in javadocs (#8701)
Motivation:

Javadocs contained some spelling errors, we should fix these.

Modification:

Fix spelling

Result:

Javadoc cleanup.
2019-01-14 07:24:34 +01:00
Jon Chambers
66ccd1483a Publicize default explicitFlushAfterFlushes count. (#8683)
Motivation:

Users who want to construct a `FlushConsolidationHandler` with a default `explicitFlushAfterFlushes` but non-default `consolidateWhenNoReadInProgress` may benefit from having an easy way to get the default "flush after flushes" count.

Modifications:

- Moved default `explicitFlushAfterFlushes` value to a public constant.
- Adjusted Javadoc accordingly.

Result:

Default `explicitFlushAfterFlushes` is accessible to callers.
2018-12-25 22:35:58 +01:00
Norman Maurer
9947df4a74
Add test for correctly handling SSLSessionBindingEvent when acting on th… (#8649)
Motivation:

During some other work I noticed we do not have any tests to ensure we correctly use SSLSessionBindingEvent. We should add some testing.

Modifications:

- Added unit test to verify we correctly implement it.
- Ignore the test when using Conscrypt as it not correctly implements it.

Result:

More tests for custom SSL impl.
2018-12-19 12:55:48 +01:00
Norman Maurer
b6d6d98404
Skip tests that use KeyManagerFactory if not supported by OpenSSL version / flavor (#8662)
Motivation:

We missed to skip a few tests that depend on the KeyManagerFactory if the used OpenSSL version / flavor not support it.

Modifications:

Add missing overrides.

Result:

Testsuite also passes for example when using LibreSSL.
2018-12-14 21:33:38 +01:00
Feri73
d17bd5e160 Adding support for whitespace in resource path in tests (#8606)
Motivation:

In windows if the project is in a path that contains whitespace,
resources cannot be accessed and tests fail.

Modifications:

Adds ResourcesUtil.java in netty-common. Tests use ResourcesUtil.java to access a resource.

Result:

Being able to build netty in a path containing whitespace
2018-12-12 10:29:02 +01:00
Norman Maurer
1dacd37989
SSLSession.putValue / getValue / removeValue / getValueNames must be thread-safe. (#8648)
Motivation:

SSLSession.putValue / getValue / removeValue / getValueNames must be thread-safe as it may be called from multiple threads. This is also the case in the OpenJDK implementation.

Modifications:

Guard with synchronized (this) blocks to keep the memory overhead low as we do not expect to have these called frequently.

Result:

SSLSession implementation is thread-safe.
2018-12-12 07:41:26 +01:00
Paul Verest
25216be118 ReadTimeoutHandler - missing ) within JavaDoc example (#8645)
Motivation:

improve docs

Modification:

ReadTimeoutHandler - missing ) within JavaDoc example

No logic/unit tests affected
2018-12-10 20:50:38 +01:00
多巴胺
22b2c4c3b8 Fix concurrency problem in UniqueIpFilter (#8635)
Motivation:

If two requests from the same IP are reached at the same time, `connected.contains(remoteIp)` may return false in both threads.

Modifications:

Check if there is already a connection with the same IP using return values.

Result:

Become thread safe.
2018-12-07 13:50:00 +01:00
Feri73
5df235c083 Correcting Maven Dependencies (#8622)
Motivation:

Most of the maven modules do not explicitly declare their
dependencies and rely on transitivity, which is not always correct.

Modifications:

For all maven modules, add all of their dependencies to pom.xml

Result:

All of the (essentially non-transitive) depepdencies of the modules are explicitly declared in pom.xml
2018-12-06 09:01:14 +01:00
root
8eb313072e [maven-release-plugin] prepare for next development iteration 2018-11-29 11:15:09 +00:00
root
afcb4a37d3 [maven-release-plugin] prepare release netty-4.1.32.Final 2018-11-29 11:14:20 +00:00
Norman Maurer
38524ec3e2
Fix test that assumed detection of peer supported algs is not supported in BoringSSL. (#8573)
Motivation:

0d2e38d5d6 added supported for detection of peer supported algorithms but we missed to fix the testcase.

Modifications:

Fix test-case.

Result:

No more failing tests with BoringSSL.
2018-11-19 12:13:05 +01:00
JStroom
ce02d5a184 Update SslHandler.java (#8564)
Swallow SSL Exception "closing inbound before receiving peer's close_notify" when running on Java 11 (#8463)

Motivation:

When closing a inbound SSL connection before the remote peer has send a close notify, the Java JDK is trigger happy to throw an exception. This exception can be ignored since the connection is about to be closed.
The exception wasn't printed in Java 8, based on filtering on the exception message. In Java 11 the exception message has been changed.

Modifications:
Update the if statement to also filter/swallow the message on Java 11.

Result:
On Java 11 the exception isn't printed with log levels set to debug. The old behaviour is maintained.
2018-11-16 10:32:34 +01:00
Norman Maurer
8d4d76d216
ReferenceCountedOpenSslEngine SSLSession.getLocalCertificates() / getLocalPrincipial() did not work when KeyManagerFactory was used. (#8560)
Motivation:

The SSLSession.getLocalCertificates() / getLocalPrincipial() methods did not correctly return the local configured certificate / principal if a KeyManagerFactory was used when configure the SslContext.

Modifications:

- Correctly update the local certificates / principial when the key material is selected.
- Add test case that verifies the SSLSession after the handshake to ensure we correctly return all values.

Result:

SSLSession returns correct values also when KeyManagerFactory is used with the OpenSSL provider.
2018-11-16 07:38:32 +01:00
Norman Maurer
20d4fda55e
Return the correct pointer from ReferenceCountedOpenSslContext.context() and sslCtxPointer() (#8562)
Motivation:

We did not return the pointer to SSL_CTX put to the internal datastructure of tcnative.

Modifications:

Return the correct pointer.

Result:

Methods work as documented in the javadocs.
2018-11-16 07:37:57 +01:00
Norman Maurer
0d2e38d5d6
Correctly convert supported signature algorithms when using BoringSSL (#8481)
* Correctly convert supported signature algorithms when using BoringSSL

Motivation:

BoringSSL uses different naming schemes for the signature algorithms so we need to adjust the regex to also handle these.

Modifications:

- Adjust SignatureAlgorithmConverter to handle BoringSSL naming scheme
- Ensure we do not include duplicates
- Add unit tests.

Result:

Correctly convert boringssl signature algorithm names.
2018-11-14 19:23:11 +01:00
Norman Maurer
d1654484c1
Correctly convert between openssl / boringssl and java cipher names when using TLSv1.3 (#8485)
Motivation:

We did not correctly convert between openssl / boringssl and java ciphers when using TLV1.3 which had different effects when either using openssl or boringssl.
 - When using openssl and TLSv1.3 we always returned SSL_NULL_WITH_NULL_NULL as cipher name
 - When using boringssl with TLSv1.3 we always returned an incorrect constructed cipher name which does not match what is defined by Java.

Modifications:

 - Add correct mappings in CipherSuiteConverter for both openssl and boringssl
 - Add unit tests for CipherSuiteConvert
 - Add unit in SSLEngine which checks that we do not return SSL_NULL_WITH_NULL_NULL ever and that server and client returns the same cipher name.

Result:

Fixes https://github.com/netty/netty/issues/8477.
2018-11-14 08:49:13 +01:00
Tim Brooks
11ec7d892e Cleanup SslHandler handshake/renegotiation (#8555)
Motivation:

The code for initiating a TLS handshake or renegotiation process is
currently difficult to reason about.

Modifications:

This commit introduces to clear paths for starting a handshake. The
first path is a normal handshake. The handshake is started and a timeout
is scheduled.

The second path is renegotiation. If the first handshake is incomplete,
the renegotiation promise is added as a listener to the handshake
promise. Otherwise, the renegotiation promise replaces the original
promsie. At that point the handshake is started again and a timeout is
scheduled.

Result:

Cleaner and easier to understand code.
2018-11-14 08:19:06 +01:00
Norman Maurer
4c73d24ea8
Handshake timeout may never be scheduled if handshake starts via a flush or starttls is used. (#8494)
Motivation:

We did not correctly schedule the handshake timeout if the handshake was either started by a flush(...) or if starttls was used.

Modifications:

- Correctly setup timeout in all cases
- Add unit tests.

Result:

Fixes https://github.com/netty/netty/issues/8493.
2018-11-13 19:22:38 +01:00
Norman Maurer
c0dfb568a2
SSLHandler may throw AssertionError if writes occur before channelAct… (#8486)
Motivation:

If you attempt to write to a channel with an SslHandler prior to channelActive being called you can hit an assertion. In particular - if you write to a channel it forces some handshaking (through flush calls) to occur.

The AssertionError only happens on Java11+.

Modifications:

- Replace assert by an "early return" in case of the handshake be done already.
- Add unit test that verifies we do not hit the AssertionError anymore and that the future is correctly failed.

Result:

Fixes https://github.com/netty/netty/issues/8479.
2018-11-11 07:23:08 +01:00
Norman Maurer
8a24df88a4
Ensure we correctly call wrapEngine(...) during tests. (#8473)
Motivation:

We should call wrapEngine(...) in our SSLEngineTest to correctly detect all errors in case of the OpenSSLEngine.

Modifications:

Add missing wrapEngine(...) calls.

Result:

More correct tests
2018-11-08 15:22:33 +01:00
Norman Maurer
fd57d971d1
Override and so delegate all methods in OpenSslX509Certificate (#8472)
Motivation:

We did not override all methods in OpenSslX509Certificate and delegate to the internal 509Certificate.

Modifications:

Add missing overrides.

Result:

More correct implementation
2018-11-07 12:16:04 +01:00
Norman Maurer
4760dc5c2d
Don't double release ByteBuf when parsing of the X509Certificate fails (#8457)
Motivation:

Due a bug in our implementation we tried to release the same ByteBuf two times when we failed to parse the X509Certificate as closing the ByteBufInputStream already closed it.

Modifications:

- Don't close the ByteBuf when closing the ByteBufInputStream
- Explicit release all ByteBufs after we are done parsing in a finally block.
- Add testcase.

Result:

Do not produce an IllegalReferenceCountException and throw the correct CertificateException.
2018-11-02 17:08:53 +01:00
Daniel Gartmann
9c70dc8ba5 Replaced obsolete cryptographic primitive with a modern/secure one. (#8450)
Motivation:

SHA1 is a broken hash function and shouldn't be used anymore (see: https://shattered.io/).
Security scanning tools will raise this as an issue and it will reflect badly on netty and I, therefore, recommend to use a SHA2 hash function which is secure and won't be flagged by such tools.

Modifications:

Replaced insecure SHA1 based signing scheme with SHA2.

Result:

Modern and thus secure cryptographic primitives will be in use and won't be flagged by security scanning tools.
2018-11-02 07:20:54 +01:00
Norman Maurer
52699bd6dd
Add test to verify that invalid ciphers are handled in all SSLEngine implementations correctly. (#8443)
Motivation:

https://github.com/netty/netty/issues/8442 reported that we fail to build a SslContext when an invalid cipher is used with netty-tcnative-boringssl-static, while it worked before. This test verifies that this is now consistent with all other SSLEngine implementations.

Modifications:

Add test-case to verify consistent behaviour

Result:

More tests to assert consistent behaviour across SSLEngine implementations
2018-10-30 17:58:08 +01:00
Norman Maurer
46460de243
Correctly init X509Certificate array when testing if we need to wrap the KeyManager due of TLSv1.3 (#8435)
Motivation:

201e984cb3 added support to use native TLSv1.3 support even with Java versions prior to 11. For this we try to detect if we need to wrap the used KeyManager or not. This testing code did create an X509Certificate[1] but does not correctly also set the certficiate on index 0. While this should be harmless we should better do the right thing and set it.

Modifications:

Correctly init the array.

Result:

Cleaner and more correct code.
2018-10-30 08:17:31 +01:00
root
3e7ddb36c7 [maven-release-plugin] prepare for next development iteration 2018-10-29 15:38:51 +00:00
root
9e50739601 [maven-release-plugin] prepare release netty-4.1.31.Final 2018-10-29 15:37:47 +00:00
Norman Maurer
ce39773e04
Add support for boringssl and TLSv1.3 (#8412)
Motivation:

0ddc62cec0 added support for TLSv1.3 when using openssl 1.1.1. Now that BoringSSL chromium-stable branch supports it as well we can also support it with netty-tcnative-boringssl-static.
During this some unit tests failed with BoringSSL which was caused by not correctly handling flush() while the handshake is still in progress.

Modification:

- Upgrade netty-tcnative version which also supports TLSv1.3 when using BoringSSL
- Correctly handle flush() when done while the handshake is still in progress in all cases.

Result:

Easier for people to enable TLSv1.3 when using native SSL impl.
Ensure flush() while handshake is in progress will always be honored.
2018-10-26 15:29:49 -07:00
Norman Maurer
9e762e8816
Correctly detect if KeyManagerFactory is supported by OpenSSL even when sun.security.x509.* can not be accessed and bouncycastle is not on the classpath. (#8415)
Motivation:

OpenSsl used SelfSignedCertificate in its static init block to detect if KeyManagerFactory is supported. Unfortunally this only works when either sun.security.x509.* can be accessed or bouncycastle is on the classpath.
We should not depend on either of it.

This came up in https://github.com/netty/netty-tcnative/issues/404#issuecomment-431551890.

Modifications:

Just directly use the bytes to generate the X509Certificate and so not depend on sun.security.x509.* / bouncycastle.

Result:

Correctly be able to detect if KeyManagerFactory can be supported in all cases.
2018-10-23 17:08:23 +02:00
Norman Maurer
91201fb338
Remove workaround in tests for TLSv1.3 bug in Java11 as it was fixed in 11.0.1 (#8409)
Motivation:

We had put some workaround in our tests due a bug in the Java11 implementation of TLSv1.3. This was now fixes as part of 11.0.1.

See https://bugs.openjdk.java.net/browse/JDK-8211067.

Modifications:

Remove workaround in SSL tests.

Result:

Run all tests with supported TLS version.
2018-10-19 17:21:04 +02:00
Norman Maurer
201e984cb3
Allow to use TLSv1.3 with netty-tcnative withe java versions prior to 11. (#8394)
Motivation:

At the moment it's only possible to use TLSv1.3 with netty-tcnative if Java 11 is used. It should be possible to do so even with Java 8, 9 and 10.

Modification:

Add a workaround to be able to use TLSv1.3 also when using Java version prior to Java 11 and the default X509ExtendedTrustManager is used.

Result:

Be able to use TLSv1.3 also with past versions of Java.
2018-10-18 13:50:12 +02:00
Norman Maurer
3543e17967
Ensure OpenSslEngine will not try to call SSL_free multiple times even when constructor throws. (#8399)
Motivation:

When the constructor of OpenSslEngine threw we could end up to self call SSL_free by ourself and then have the finalizer do the same which may lead to double free-ing and so SIGSEV.

Modifications:

Just call shutdown() when the constructor throws and so ensure SSL_free is guarded correctly in the finalizer.

Result:

No more SIGSEV possible.
2018-10-18 07:38:03 +02:00
Norman Maurer
0ddc62cec0
Add support for TLSv1.3 (#8293)
Motivation:

TLSv1.3 support is included in java11 and is also supported by OpenSSL 1.1.1, so we should support when possible.

Modifications:
- Add support for TLSv1.3 using either the JDK implementation or the native implementation provided by netty-tcnative when compiled against openssl 1.1.1
- Adjust unit tests for semantics provided by TLSv1.3
- Correctly handle custom Provider implementations that not support TLSv1.3

Result:

Be able to use TLSv1.3 with netty.
2018-10-17 08:35:35 +02:00
Stephane Landelle
9eebe7ed74 Add full JdkSslContext public constructor, close #8384 (#8389)
Motivation:

JdkSslContext provides public constructors to wrap an existing `javax.net.ssl.SSLContext`.

Sadly, some options combinations are not possible with the existing constructors, eg:
*  protocols is not exposed and always forced to null, so default protocols are always enforced
* startTls is not exposed and always forced to false

Modification:

Add full constructor that take protocols and startTls parameters.

Result:

It's possible to create a JdkSslContext from an existing SSLContext and still have control over protocols and startTls
2018-10-17 08:28:39 +02:00
Norman Maurer
aae7cdca96
Prevent NPE when attempting to set client key material with no alias (#8378)
Motivation:

It is possible that a client is unable to locate a certificate alias given the list of issuers and key types. In this case the X509KeyManager
will return a null which when past to the OpenSslKeyMaterialProvider implementation may produce a NPE. If no matching alias could be found we should not
call OpenSslKeyMaterialProvider at all which is also consistent what OpenJDK does.

Modifications:

- Add null check before calling OpenSslKeyMaterialProvider
- Add unit test.

Result:

No more NPE caused by passing null as client alias.
2018-10-12 09:27:46 +02:00
Norman Maurer
59973e93dd
Ensure X509KeyManager methods are called on the correct time when using server-side and support more methods of ExtendedSSLSession. (#8283)
Motivation:

Before when on server-side we just called the X509KeyManager methods when handshake() was called the first time which is not quite correct as we may not have received the full SSL hello / handshake and so could not extra for example the SNI hostname that was requested.
OpenSSL exposes the SSL_CTX_set_cert_cb function which allows to set a callback which is executed at the correct moment, so we should use it. This also allows us to support more methods of ExtendedSSLSession easily.

Modifications:

- Make use of new methods exposed by netty-tcnative since https://github.com/netty/netty-tcnative/pull/388 to ensure we select the key material at the correct time.
- Implement more methods of ExtendedOpenSslSession
- Add unit tests to ensure we are able to retrieve various things on server-side in the X509KeyManager and so verify it is called at the correct time.
- Simplify code by using new netty-tcnative methods.

Result:

More correct implementation for server-side usage and more complete implemented of ExtendedSSLSession.
2018-09-28 11:34:38 +02:00
Norman Maurer
73acac13f4
Check if hostname validation is supported before trying to use in test. (#8333)
Motivation:

a208f6dc7c added a testcase which uses hostname validation which may not be supported by OpenSSL depending on the version that is used. We should check first before we try to use it.

Modifications:

Add assumeTrue(...) check to ensure hostname validation is supported before trying to run the test.

Result:

No more test-failures on OpenSSL versions < 1.0.2.
2018-09-28 10:54:05 +02:00
Norman Maurer
a208f6dc7c
Do the same extended checks as the JDK when a X509TrustManager is used with the OpenSSL provider. (#8307)
Motivation:

When a X509TrustManager is used while configure the SslContext the JDK automatically does some extra checks during validation of provided certs by the remote peer. We should do the same when our native implementation is used.

Modification:

- Automatically wrap a X509TrustManager and so do the same validations as the JDK does.
- Add unit tests.

Result:

More consistent behaviour. Fixes https://github.com/netty/netty/issues/6664.
2018-09-28 09:19:58 +02:00
root
2d7cb47edd [maven-release-plugin] prepare for next development iteration 2018-09-27 19:00:45 +00:00
root
3a9ac829d5 [maven-release-plugin] prepare release netty-4.1.30.Final 2018-09-27 18:56:12 +00:00
Norman Maurer
652650e127
Fix leak in SniClientJava8TestUtil (#8326)
Motivation:

4d1458604a did fix some leaks in SniClientTest but missed the ones in SniClientJava8TestUtil.

Modifications:

Correctly release SslContext.

Result:

No more leaks in SNI tests.
2018-09-27 09:32:41 +02:00
Norman Maurer
4d1458604a
Fix leak in SniClientTest. (#8324)
Motivation:

We need to release the ReferenceCountedSslContext to eliminate resource leaks.

Reported in https://garage.netty.io/teamcity/viewLog.html?buildId=33353&buildTypeId=netty_build_oraclejdk8&tab=buildLog#_focus=157264.

Modifications:

Call release on the SslContext instances.

Result:

No more leaks in tests.
2018-09-26 19:59:00 +02:00
Norman Maurer
a80c49828f
Cleanup SSL test. (#8301)
Motivation:

I noticed that we had some errors showing up in a test (which did not fail it tho) because we tried to full-fill the promise multiples times.

Modifications:

Use trySuccess(...) as we may produce multiple exceptions.

Result:

Less errors during test-run.
2018-09-21 08:04:24 -07:00
Norman Maurer
01db30a163
Correctly implement ExtendedSSLSession.getStatusResponses() for ReferenceCountedOpenSslEngine (#8297)
Motivation:

Java9 added getStatusResponses() to ExtendedSSLSession which we should correctly support when possible.

Modifications:

Implement the method correctly.

Result:

More complete and correct implementation.
2018-09-19 17:13:44 -07:00
Norman Maurer
34d52fcbfe
Implemented ExtendedOpenSslSession.getStatusResponses() so it not throws an UnsupportedOperationException. (#8290)
Motivation:

6ed7c6c75d added support for ExtendedOpenSslSession but we did not override getStatusResponses(). This lead to test failures on java9.

Modifications:

Implement ExtendedOpenSslSession.getStatusResponses() so it just returns an empty list.

Result:

Test pass again on Java9.
2018-09-14 20:33:09 +02:00
Norman Maurer
2b1514ec5a
Only use KeyManagerFactory in SniClientTest when supported by OpenSSL version. (#8289)
Motivation:

6ed7c6c75d added a test which blindly assumed we can use a KeyManagerFactory all the time. This is only true if have OpenSSL 1.0.2 or later, which may not be the case.

Modifications:

Only use KeyManagerFactory in test if the OpenSSL version does support it.

Result:

More robust tests.
2018-09-14 19:01:55 +02:00
Norman Maurer
6ed7c6c75d
Return an ExtendSSLSession whenever possible to allow more strict checking when using OpenSSL (#8281)
Motivation:

When an ExtendedSSLSession is used its possible to do more strict checking of the keys during handshake. We should do this whenever possible.

Modification:

- Return an ExtendedSSLSession when using client-mode and Java7+
- Add unit test
- Simplify unit tests

Result:

More consistent behaviour.
2018-09-14 14:33:11 +02:00
Norman Maurer
5ff6b57940
PemPrivateKey.toPem(...) should throw IllegalArgumentException when P… (#8253)
* PemPrivateKey.toPem(...) should throw IllegalArgumentException when PrivateKey which does not support encoding is used.

Motivation:

At the moment when a PrivateKey is used that does not support encoding we throw a NPE when trying to convert the key. We should better throw an IllegalArgumentException with the details about what key we tried to encode.

Modifications:

- Check if PrivateKey.getEncoded() returns null and if so throw an IllegalArgumentException
- Add unit test.

Result:

Better handling of non-supported PrivateKey implementations.
2018-09-05 20:33:40 +02:00
Norman Maurer
f4bafd4fe0
Correctly check if cipher is supported for each SslProvider before trying to run test and fix buffer leaks in test. (#8247)
Motivation:

5aaa16b24c introduced a testcase for specific ciphersuites and checked if these are supported by our native implementation before running it. Unfortunally this is not good enough as even on the JDK it may not be supported on various JDK versions (like Java7). Beside this the test leaked buffers.

Modifications:

- Correctly check if ciphersuite is supported on each SslProvider before trying to run test.
- Fix buffer leaks.

Result:

Testsuite pass again on Java7 and others when -Pleak is used.
2018-09-01 08:11:13 +02:00
Norman Maurer
a644563625
Add more debug informations when log SSL errors. (#8241)
Motivation:

ea626ef8c3 added more debug logging but we can even include a bit more.

Modifications:

Always log the error number as well.

Result:

More informations for debugging SSL errors.
2018-08-30 20:44:47 +02:00
Norman Maurer
ea626ef8c3
Log more details when shutdown SSL because of an error. (#8236)
Motivation:

We should log a bit more details about why we shutdown the SSL.

Modifications:

Add the return value of SSL_get_error(...) as well in debug mode.

Result:

More logging to make it easier to understand why an SSL error happened.
2018-08-29 21:52:26 +02:00
Roger
5aaa16b24c An unit test to ensure that cipher suites don't break/disappear between releases. (#8225)
Motivation

Ensure classes of cipher suites continue working between releases. Adding just a DHE check for now as it caused #8165 but this test can be expaned to other suites.

Modifications

Adding an unit test that checks for the presence of a cipher suite.

Result

Prevent #8165 from happening in the future.
2018-08-29 14:14:26 +02:00
root
a580dc7585 [maven-release-plugin] prepare for next development iteration 2018-08-24 06:36:33 +00:00
root
3fc789e83f [maven-release-plugin] prepare release netty-4.1.29.Final 2018-08-24 06:36:06 +00:00
Norman Maurer
df00539fa2
Allow to load PrivateKey via OpenSSL Engine (#8200)
Motivation:

OpenSSL itself has an abstraction which allows you to customize some things. For example it is possible to load the PrivateKey from the engine. We should support this.

Modifications:

Add two new static methods to OpenSslX509KeyManagerFactory which allow to create an OpenSslX509KeyManagerFactory that loads its PrivateKey via the OpenSSL Engine directly.

Result:

More flexible usage of OpenSSL possible
2018-08-18 07:20:44 +02:00
Norman Maurer
bbe2e4d224
We should try to load netty-tcnative before using it in OpenSslCertificateException. (#8202)
Motivation:

In OpenSslCertificateException we should ensure we try to load netty-tcnative before trying to use any class from it as otherwise it may throw an error due missing linking of the native libs.

Modifications:

- Ensure we call OpenSsl.isAvailable() before we try to use netty-tcnative for validation
- Add testcase.

Result:

No more errors causing by not loading native libs before trying to use these.
2018-08-18 06:26:45 +02:00
Norman Maurer
8255f85f24
Rename SslHandler.close(...) to closeOutbound(...) as it is still useful and delegate to the methods. (#8193)
* Rename SslHandler.close(...) to closeOutbound(...) as it is still useful and delegate to the methods.

Motivation:

Sometimes the user may want to send a close_notify without closing the underlying Channel. For this we offered the SslHandler.close(...) methods which were marked as deeprecated. We should offer an way to still do this without the user calling deprecated methods.

See https://stackoverflow.com/questions/51710231/using-nettys-sslhandlerclosechannelhandlercontext-channelpromise/51753742#comment90555949_51753742 .

Modifications:

- Remove deprecation of the SslHandler.close(...) method that exactly allows this and rename these to closeOutbound(...) as this is more clear.
- Add close(...) methods that delegate to these and mark these as deprecated.

Result:

Be able to send close_notify without closing the Channel.
2018-08-15 20:07:56 +02:00
Lai Jiang
dbc9ec1ab2 Set SNI servernames in OpenSSL engine when created in client mode (#8178)
Motivation:

When using the JDK SSL provider in client mode, the SNI host names (called serverNames in SslEngineImpl) is set to the peerHost (if available) that is used to initialize the SSL Engine:

http://hg.openjdk.java.net/jdk8/jdk8/jdk/file/687fd7c7986d/src/share/classes/sun/security/ssl/SSLEngineImpl.java#l377

This allows one to call SslEngine.getSSLParameters() and inspect what is the SNI name to be sent. The same should be done in the OpenSSL provider as well. Currently even though the the SNI name is sent by the OpenSSL provider during handshake when the peerHost is specified, it is missing from the parameters.

Modification:

Set the sniHostNames field when SNI is to be used. Also verifies the peer is actually a hostname before setting it as the SNI name, which is consistent with JDK SSL provider's behavior.

Result:

SslEngine using the OpenSSL provider created in client mode with peerHost will initialize sniHostNames with the peerHost.

Calling SslEngine.getSSLParameters().getServerNames() will return a list that contains that name.
2018-08-09 21:30:02 +02:00
Norman Maurer
44d3753c48
Fix NPE exception when using invalid cipher during building SslContext. (#8171)
Motivation:

We missed to do a null check before trying to destroy the OpenSslSessionContext, which could lead to a NPE.

Modifications:

Add null check and tests.

Result:

Fix https://github.com/netty/netty/issues/8170.
2018-08-02 21:42:21 +02:00
Norman Maurer
fe14bad69c
Adjust SSL related tests to be more correct and so pass in the next EA release of java11. (#8162)
Motivation:

In some of our tests we not correctly init the SSLEngine before trying to perform a handshake which can cause an IllegalStateException. While this not happened in previous java releases it does now on Java11 (which is "ok" as its even mentioned in the api docs). Beside this how we selected the ciphersuite to test renegotation was not 100 % safe.

Modifications:

- Correctly init SSLEngine before using it
- Correctly select ciphersuite before testing for renegotation.

Result:

More correct tests and also pass on next java11 EA release.
2018-08-01 06:37:53 +02:00
root
fcb19cb589 [maven-release-plugin] prepare for next development iteration 2018-07-27 04:59:28 +00:00
root
ff785fbe39 [maven-release-plugin] prepare release netty-4.1.28.Final 2018-07-27 04:59:06 +00:00
Norman Maurer
620dad0c26
Allow to validate sni hostname with underscore (#8150)
Motivation:

We should allow to also validate sni hostname which contains for example underscore when using our native SSL impl. The JDK implementation does this as well.

Modifications:

- Construct the SNIHostName via byte[] and not String.
- Add unit test

Result:

Fixes https://github.com/netty/netty/issues/8144.
2018-07-27 01:56:32 +08:00
root
b4dbdc2036 [maven-release-plugin] prepare for next development iteration 2018-07-11 15:37:40 +00:00
root
1c16519ac8 [maven-release-plugin] prepare release netty-4.1.27.Final 2018-07-11 15:37:21 +00:00
Norman Maurer
df08467d7c
Fix possible NPE introduced by a137291ad1 when using SslProvider.OPENSSL and init via files or OpenSslX509KeyManagerFactory (#8126)
Motivation:

a137291ad1 introduced a way to get the most speed out of OpenSSL by not only caching keymaterial but pre-compute these. The problem was we missed to check for null before doing an instanceof check and then a cast which could lead to a NPE as we tried to cast null to Exception and throw it.

Modifications:

Add null check and unit test.

Result:

No more NPE when keymaterial was not found for requested alias.
2018-07-11 15:19:37 +01:00
root
7bb9e7eafe [maven-release-plugin] prepare for next development iteration 2018-07-10 05:21:24 +00:00
root
8ca5421bd2 [maven-release-plugin] prepare release netty-4.1.26.Final 2018-07-10 05:18:13 +00:00
Norman Maurer
a137291ad1
Add OpenSslX509KeyManagerFactory which makes it even easier for peopl… (#8084)
* Add OpenSslX509KeyManagerFactory which makes it even easier for people to get the maximum performance when using OpenSSL / LibreSSL / BoringSSL  with netty.

Motivation:

To make it even easier for people to get the maximum performance when using native SSL we should provide our own KeyManagerFactory implementation that people can just use to configure their key material.

Modifications:

- Add OpenSslX509KeyManagerFactory which users can use for maximum performance with native SSL
- Refactor some internal code to re-use logic and not duplicate it.

Result:

Easier to get the max performance out of native SSL implementation.
2018-07-10 00:42:37 -04:00
Norman Maurer
83710cb2e1
Replace toArray(new T[size]) with toArray(new T[0]) to eliminate zero-out and allow the VM to optimize. (#8075)
Motivation:

Using toArray(new T[0]) is usually the faster aproach these days. We should use it.

See also https://shipilev.net/blog/2016/arrays-wisdom-ancients/#_conclusion.

Modifications:

Replace toArray(new T[size]) with toArray(new T[0]).

Result:

Faster code.
2018-06-29 07:56:04 +02:00
Norman Maurer
ecc238bea5
Only try to call SSL.setHostnameValidation(...) if needed. (#8074)
Motivation:

As the used OpenSSL version may not support hostname validation we should only really call SSL.setHostNameValidation(...) if we detect that its needed.

Modifications:

Only call SSL.setHostNameValidation if it was disabled before and now it needs to be enabled or if it was enabled before and it should be disabled now.

Result:

Less risk of an exception when using an OpenSSL version that does not support hostname validation.
2018-06-28 11:07:13 +02:00
Norman Maurer
2818730092
OpenSSL (and so netty-tcnative) should allow to use custom engine. (#8050)
Motivation:

OpenSSL allows to use a custom engine for its cryptographic operations. We should allow the user to make use of it if needed.

See also: https://www.openssl.org/docs/man1.0.2/crypto/engine.html.

Modifications:

Add new system property which can be used to specify the engine to use (null is the default and will use the build in default impl).

Result:

More flexible way of using OpenSSL.
2018-06-28 08:13:52 +02:00