Commit Graph

1306 Commits

Author SHA1 Message Date
Norman Maurer
00fd4ea264
Disable TLSv1 and TLSv1.1 by default (#11237)
Motivation:

TLSv1 and TLSv1.1 is considered insecure. Let's follow the JDK and disable these by default

Modifications:

- Disable TLSv1 and TLSv1.1 by default when using OpenSSL.
- Add unit tests

Result:

Use only strong TLS versions by default when using OpenSSL
2021-05-11 10:43:40 +02:00
Norman Maurer
57a70c5d83
Update conscrypt and add workaround for test failure (#11238)
Motivation:

Conscrypt not correctly filters out non support TLS versions which may lead to test failures.

Related to https://github.com/google/conscrypt/issues/1013

Modifications:

- Bump up to latest patch release
- Add workaround

Result:

No more test failures caused by conscrypt
2021-05-11 10:40:54 +02:00
Idel Pivnitskiy
3ecb32357c
Use PlatformDependent#normalizedOs() instead of reading os.name prop (#11239)
Motivation:

`PlatformDependent#normalizedOs()` already caches normalized variant of
the value of `os.name` system property. Instead of inconsistently
normalizing it in every case, use the utility method.

Modifications:

- `PlatformDependent`: `isWindows0()` and `isOsx0()` use `NORMALIZED_OS`;
- `PlatformDependent#normalizeOs(String)` define `darwin` as `osx`;
- `OpenSsl#loadTcNative()` does not require `equalsIgnoreCase` bcz `os`
is already normalized;
- Epoll and KQueue: `Native#loadNativeLibrary()` use `normalizedOs()`;
- Use consistent `Locale.US` for lower case conversion of `os.name`;
- `MacOSDnsServerAddressStreamProvider#loadNativeLibrary()` uses
`PlatformDependent.isOsx()`;

Result:

Consistent approach for `os.name` parsing.
2021-05-11 08:52:29 +02:00
skyguard1
bb7b05be01
Add explicit null checks in OpenSslX509KeyManagerFactory (#11230)
Motivation:

We should add explicit null checks so its easier for people to understand why it throws.

Modification:

Add explicit checkNotNull(...)

Result:

Easier to understand for users why it fails.

Signed-off-by: xingrufei <xingrufei@sogou-inc.com>

Co-authored-by: xingrufei <xingrufei@sogou-inc.com>
2021-05-07 11:22:27 +02:00
Chris Vest
03c2644c19
Bump initial timeouts in SSLEngineTest (#11221)
Motivation:
We've seen (very rare) flaky test failures due to timeouts.
They are too rare to analyse properly, but a theory is that on overloaded, small cloud CI instances, it can sometimes take a surprising amount of time to start a thread.
It could be that the event loop thread is getting an unlucky schedule, and takes seconds to start, causing the timeouts to elapse.

Modification:
Increase the initial timeouts in the SSLEngineTest, that could end up waiting for the event loop thread to start.
Also fix a few simple warnings from Intellij.

Result:
Hopefully we will not see these tests be flaky again.
2021-05-05 15:14:23 +02:00
Scott Mitchell
d0fea48604
ReferenceCountedOpenSslEngine unwrap handshake complete status fix (#11210)
Motivation:
ReferenceCountedOpenSslEngine may unwrap data and complete the handshake
in a single unwrap() call. However it may return HanshakeStatus of
HandshakeStatus of NEED_UNWRAP instead of FINISHED. This may result in
the SslHandler sending the unwrapped data up the pipeline before
notifying that the handshake has completed, and result in out-of-order
events.

Modifications:
- if ReferenceCountedOpenSslEngine handshake status is NEED_UNWRAP and
  produced data, or NEED_WRAP and consumed some data, we should call
  handshake() to get the current state.

Result:
ReferenceCountedOpenSslEngine correctly indicates when the handshake has
finished if at the same time data was produced or consumed.
2021-04-29 09:56:01 +02:00
Norman Maurer
216cd38ff2
Re-enable running openssl (shared) tests on CI (#11197)
Motivation:

It turned out we didnt run the openssl tests on the CI when we used the non-static version of netty-tcnative.

Modifications:

- Upgrade netty-tcnative to fix segfault when using shared openssl
- Adjust tests to only run session cache tests when openssl supports it
- Fix some more tests to only depend on KeyManager if the underlying openssl version supports it

Result:

Run all openssl test on the CI even when shared library is used
2021-04-27 13:38:08 +02:00
skyguard1
a98c60283b
Enable Tlsv1.3 when using BouncyCastle ALPN support (#11193)
Motivation:

In the latest version of BouncyCastle, BCJSSE:'TLSv1.3' is now a supported protocol for both client and server. So should consider enabling TLSv1.3 when TLSv1.3 is available

Modification:

This pr is to enable TLSv1.3 when using BouncyCastle ALPN support, please review this pr,thanks

Result:

Enable TLSv1.3 when using BouncyCastle ALPN support

Signed-off-by: xingrufei <xingrufei@sogou-inc.com>

Co-authored-by: xingrufei <xingrufei@sogou-inc.com>
2021-04-26 10:01:16 +02:00
Boris Unckel
2f4beae8ec
Utilize i.n.u.internal.ObjectUtil to assert Preconditions (handler) (#11170) (#11180)
Motivation:

NullChecks resulting in a NullPointerException or IllegalArgumentException, numeric ranges (>0, >=0) checks, not empty strings/arrays checks must never be anonymous but with the parameter or variable name which is checked. They must be specific and should not be done with an "OR-Logic" (if a == null || b == null) throw new NullPointerEx.

Modifications:

* import static relevant checks
* Replace manual checks with ObjectUtil methods

Result:

All checks needed are done with ObjectUtil, some exception texts are improved.

Fixes #11170
2021-04-22 12:51:23 +02:00
Karsten Ohme
56ef371676
BouncyCastle ALPN support (#11157)
Motivation:

Under Android it was not possible to load a specific web page. It might be related to the (missing?) ALPN of the internal TLS implementation. BouncyCastle as a replacement works but this was not supported so far by Netty.
BouncyCastle also has the benefit to be a pure Java solution, all the other providers (OpenSSL, Conscrypt) require native libraries which are not available under Android at least.

Modification:

BouncyCastleAlpnSslEngine.java and support classes have been added. It is relying on the JDK code, hence some support classes had to be opened to prevent code duplication.

Result:

BouncyCastle can be used as TLS provider.

Co-authored-by: Norman Maurer <norman_maurer@apple.com>
2021-04-21 16:18:34 +02:00
Chris Vest
1a5ebfb999
Log fewer stack traces from initialisation code (#11164)
Motivation:
We are increasingly running in environments where Unsafe, setAccessible, etc. are not available.
When debug logging is enabled, we log a complete stack trace every time one of these initialisations fail.
Seeing these stack traces can cause people unnecessary concern.
For instance, people might have alerts that are triggered by a stack trace showing up in logs, regardless of its log level.

Modification:
We continue to print debug log messages on the result of our initialisations, but now we only include the full stack trace is _trace_ logging (or FINEST, or equivalent in whatever logging framework is configured) is enabled.

Result:
We now only log these initialisation stack traces when the lowest possible log level is enabled.

Fixes #7817
2021-04-19 08:53:08 +02:00
Scott Mitchell
0ef2f05373
SslHandler LocalChannel read/unwrap reentry fix (#11156)
Motivation:
SslHandler invokes channel.read() during the handshake process. For some
channel implementations (e.g. LocalChannel) this may result in re-entry
conditions into unwrap. Unwrap currently defers updating the input
buffer indexes until the unwrap method returns to avoid intermediate
updates if not necessary, but this may result in unwrapping the same
contents multiple times which leads to handshake failures [1][2].

[1] ssl3_get_record:decryption failed or bad record mac
[2] ssl3_read_bytes:sslv3 alert bad record mac

Modifications:
- SslHandler#unwrap updates buffer indexes on each iteration so that if
  reentry scenario happens the correct indexes will be visible.

Result:
Fixes https://github.com/netty/netty/issues/11146
2021-04-16 08:21:40 -07:00
Scott Mitchell
16b40d8a37
SslHandler consolidate state to save memory (#11160)
Motivation:
SslHandler has many independent boolean member variables. They can be
collapsed into a single variable to save memory.

Modifications:
- SslHandler boolean state consolidated into a single short variable.

Result:
Savings of 8 bytes per SslHandler (which is per connection) observed on
OpenJDK.
2021-04-15 08:18:54 -07:00
Idel Pivnitskiy
c93419ba57
Fire SslHandshakeCompletionEvent after the last decoded data chunk (#11148)
Motivation:

`SslHandler#unwrap` may produce `SslHandshakeCompletionEvent` if it
receives `close_notify` alert. This alert indicates that the engine is
closed and no more data are expected in the pipeline. However, it fires
the event before the last data chunk. As the result, further handlers
may loose data if they handle `SslHandshakeCompletionEvent`.
This issue was not visible before #11133 because we did not write
`close_notify` alert reliably.

Modifications:

- Add tests to reproduce described behavior;
- Move `notifyClosePromise` after fire of the last `decodeOut`;

Result:

`SslHandshakeCompletionEvent` correctly indicates that the engine is
closed and no more data are expected on the pipeline.
2021-04-15 14:06:42 +02:00
Chris Vest
df53de5b68
Less blocking in ChunkedStream (#11150)
Motivation:
We should avoid blocking in the event loop as much as possible.
The InputStream.read() is a blocking method, and we don't need to call it if available() returns a positive number.

Modification:
Bypass calling InputStream.read() if available() returns a positive number.

Result:
Fewer blocking calls in the event loop, in general, when ChunkedStream is used.
2021-04-12 13:40:14 +02:00
Netty Project Bot
6724786dcc [maven-release-plugin] prepare for next development iteration 2021-04-01 10:50:23 +00:00
Netty Project Bot
b78d8f2abd [maven-release-plugin] prepare release netty-4.1.63.Final 2021-04-01 10:50:18 +00:00
Scott Mitchell
6b48e690fd
SslHandler wrap reentry bug fix (#11133)
Motivation:
SslHandler's wrap method notifies the handshakeFuture and sends a
SslHandshakeCompletionEvent user event down the pipeline before writing
the plaintext that has just been wrapped. It is possible the application
may write as a result of these events and re-enter into wrap to write
more data. This will result in out of sequence data and result in alerts
such as SSLV3_ALERT_BAD_RECORD_MAC.

Modifications:
- SslHandler wrap should write any pending data before notifying
  promises, generating user events, or anything else that may create a
  re-entry scenario.

Result:
SslHandler will wrap/write data in the same order.
2021-04-01 10:00:18 +02:00
Netty Project Bot
e4dd6ee532 [maven-release-plugin] prepare for next development iteration 2021-03-30 17:22:25 +00:00
Netty Project Bot
573d1f4bf5 [maven-release-plugin] prepare release netty-4.1.62.Final 2021-03-30 17:22:18 +00:00
Netty Project Bot
7192ab0f0a [maven-release-plugin] prepare for next development iteration 2021-03-30 07:42:06 +00:00
Netty Project Bot
ccb3ff388f [maven-release-plugin] prepare release netty-4.1.61.Final 2021-03-30 07:42:01 +00:00
Norman Maurer
36cacc201e
Don't bind to a specify port during SSLEngine tests as the port may also be used (#11102)
Motivation:

We should always bind to port 0 to ensure we not try to bind to a port that is already in use during our tests.
As we missed to do this in one test we did see test failures sometimes.

This showed up here:
https://pipelines.actions.githubusercontent.com/obCqqsCMwwGme5y2aRyYOiZvWeJK4O0EY5AYRUDMSELILdqEjV/_apis/pipelines/1/runs/1963/signedlogcontent/18?urlExpires=2021-03-19T12%3A41%3A21.4370902Z&urlSigningMethod=HMACV1&urlSignature=zL6O0msEkghT%2B0hOAL1lqLK66SR0Mp99QIjiau1yPe4%3D

Modifications:

- Use new InetSocketAddress(0)

Result:

Fixes possible test failures.
2021-03-19 21:09:46 +01:00
Scott Mitchell
0b0c234bed
SslHandler flushing with TCP Fast Open fix (#11077)
Motivation:
SslHandler owns the responsibility to flush non-application data
(e.g. handshake, renegotiation, etc.) to the socket. However when
TCP Fast Open is supported but the client_hello cannot be written
in the SYN the client_hello may not always be flushed. SslHandler
may not wrap/flush previously written/flushed data in the event
it was not able to be wrapped due to NEED_UNWRAP state being
encountered in wrap (e.g. peer initiated renegotiation).

Modifications:
- SslHandler to flush in channelActive() if TFO is enabled and
  the client_hello cannot be written in the SYN.
- SslHandler to wrap application data after non-application data
  wrap and handshake status is FINISHED.
- SocketSslEchoTest only flushes when writes are done, and waits
  for the handshake to complete before writing.

Result:
SslHandler flushes handshake data for TFO, and previously flushed
application data after peer initiated renegotiation finishes.
2021-03-14 14:18:27 +01:00
Netty Project Bot
e83132fcf2 [maven-release-plugin] prepare for next development iteration 2021-03-09 08:18:31 +00:00
Netty Project Bot
eef26e8bb5 [maven-release-plugin] prepare release netty-4.1.60.Final 2021-03-09 08:18:26 +00:00
Netty Project Bot
b8d843757b [maven-release-plugin] rollback the release of netty-4.1.60.Final 2021-03-09 07:39:13 +00:00
Netty Project Bot
4f099dd11f [maven-release-plugin] prepare for next development iteration 2021-03-09 07:21:55 +00:00
Netty Project Bot
44e392e851 [maven-release-plugin] prepare release netty-4.1.60.Final 2021-03-09 07:21:51 +00:00
Norman Maurer
7d4aaa268b
Support session cache for client and server when using native SSLEngine implementation (#10994)
Motivation:

At the moment we don't support session caching on the client side at all when using the native SSL implementation. We should at least allow to enable it.

Modification:

Allow to enable session cache for client side but disable ti by default due a JDK bug atm.

Result:

Be able to cache sessions on the client side when using native SSL implementation .
2021-03-07 19:15:39 +01:00
Norman Maurer
5d41611b79
Fix NPE that can happen in the WriteTimeoutHandler when multiple executors are used (#11056)
Motivation:

In WriteTimeoutHandler we did make the assumption that the executor which is used to schedule the timeout is the same that is backing the write promise. This may not be true which will cause concurrency issues

Modifications:

Ensure we are on the right thread when try to modify the doubly-linked-list and if not schedule it on the right thread.

Result:

Fixes https://github.com/netty/netty/issues/11053
2021-03-04 15:18:20 +01:00
Norman Maurer
18c66a9d70
Ensure removal from queue happens before writeAndFlush(...) is called (#11049)
Motivation:

We need to ensure that we call queue.remove() before we cal writeAndFlush() as this operation may cause an event that also touches the queue and remove from it. If we miss to do so we may see NoSuchElementExceptions.

Modifications:

- Call queue.remove() before calling writeAndFlush(...)
- Add unit test

Result:

Fixes https://github.com/netty/netty/issues/11046
2021-03-02 21:56:59 +01:00
Idel Pivnitskiy
080d69137f
Simplify flushAtEnd flag computation in SslHandler#handlerAdded (#11025)
Motivation:

The `!fastOpen` part of `active || !fastOpen` is always false.

Modification:

- Remove `!fastOpen` and keep only `active` as a `flushAtEnd` flag for
`startHandshakeProcessing`;
- Update comment;

Result:

Simplified `flushAtEnd` flag computation in `SslHandler#handlerAdded`.
2021-02-16 14:04:29 +01:00
Chris Vest
56adab2743
TCP Fast Open for clients (#11006)
Support TCP Fast Open for clients and make SslHandler take advantage

Motivation:
- TCP Fast Open allow us to send a small amount of data along side the initial SYN packet when establishing a TCP connection.
- The TLS Client Hello packet is small enough to fit in there, and is also idempotent (another requirement for using TCP Fast Open), so if we can save a round-trip when establishing TLS connections when using TFO.

Modification:
- Add support for client-side TCP Fast Open for Epoll, and also lowers the Linux kernel version requirements to 3.6.
- When adding the SslHandler to a pipeline, if TCP Fast Open is enabled for the channel (and the channel is not already active) then start the handshake early by writing it to the outbound buffer.
- An important detail to note here, is that the outbound buffer is not flushed at this point, like it would for normal handshakes. The flushing happens later as part of establishing the TCP connection.

Result:
- It is now possible for clients (on epoll) to open connections with TCP Fast Open.
- The SslHandler automatically detects when this is the case, and now send its Client Hello message as part of the initial data in the TCP Fast Open flow when available, saving a round-trip when establishing TLS connections.

Co-authored-by: Colin Godsey <crgodsey@gmail.com>
2021-02-15 13:13:44 +01:00
Norman Maurer
12b5ef545a
Disable flaky test (#11017)
Motivation:

The testGlobalWriteThrottle is flaky and failed our build multiple times now. Lets disable it for now until we had time to investigate

Modifications:

Disable flaky test

Result:

Less failures during build
2021-02-11 13:27:21 +01:00
Norman Maurer
ecd7dc3516
Enable stateless resumption for TLSv1.3 by default when using OpenSSL / BoringSSL (#10997)
Motivation:

At the moment we always set SSL_OP_NO_TICKET when building our context. The problem with this is that this also disables resumption for TLSv1.3 in BoringSSL as it only supports stateless resumption for TLSv1.3 which uses tickets.
We should better clear this option when TLSv1.3 is enabled to be able to resume sessions. This is also inline with the OpenJDK which enables this for TLSv1.3 by default as well.

Modifications:

Check for enabled protocols and if TLSv1.3 is set clear SSL_OP_NO_TICKET.

Result:

Be able to resume sessions for TLSv1.3 when using BoringSSL.
2021-02-08 20:55:02 +01:00
root
411f76d3ad [maven-release-plugin] prepare for next development iteration 2021-02-08 10:48:37 +00:00
root
97d044812d [maven-release-plugin] prepare release netty-4.1.59.Final 2021-02-08 10:47:46 +00:00
Norman Maurer
c735357bf2 Use Files.createTempFile(...) to ensure the file is created with proper permissions
Motivation:

File.createTempFile(String, String)` will create a temporary file in the system temporary directory if the 'java.io.tmpdir'. The permissions on that file utilize the umask. In a majority of cases, this means that the file that java creates has the permissions: `-rw-r--r--`, thus, any other local user on that system can read the contents of that file.
This can be a security concern if any sensitive data is stored in this file.

This was reported by Jonathan Leitschuh <jonathan.leitschuh@gmail.com> as a security problem.

Modifications:

Use Files.createTempFile(...) which will use safe-defaults when running on java 7 and later. If running on java 6 there isnt much we can do, which is fair enough as java 6 shouldnt be considered "safe" anyway.

Result:

Create temporary files with sane permissions by default.
2021-02-08 11:44:05 +01:00
Norman Maurer
ab8b8ae81b
Introduce SslContextOption which can be used for "optional" features … (#10981)
Motivation:

Some of the features we want to support can only be supported by some of the SslContext implementations. We should allow to configure these in a consistent way the same way as we do it with Channel / ChannelOption

Modifications:

- Add SslContextOption and add builder methods that take these
- Add OpenSslContextOption and define two options there which are specific to openssl

Result:

More flexible configuration and implementation of SslContext
2021-02-02 08:29:21 +01:00
Norman Maurer
a9a3e2cef1
Clarify who is responsible closing the InputStream once SslContext w… (#10983)
Motivation:

It was not 100% clear who is responsible calling close() on the InputStream.

Modifications:

Clarify javadocs.

Result:

Related to https://github.com/netty/netty/issues/10974

Co-authored-by: Chris Vest <christianvest_hansen@apple.com>
2021-02-01 11:50:26 +01:00
Norman Maurer
3353bc1152
Revert "Enable SSL_MODE_ENABLE_FALSE_START if jdkCompatibilityMode is false (#10407)" (#10980)
Motivation:

TLS_FALSE_START slightly changes the "flow" during handshake which may cause suprises for the end-user. We should better disable it by default again and later add a way to enable it for the user.

Modification:

This reverts commit 514d349e1f.

Result:

Restore "old flow" during TLS handshakes.
2021-01-29 19:55:56 +01:00
Norman Maurer
df10dfbb49
Correctly filter out TLSv1.3 ciphers if TLSv1.3 is not enabled (#10919)
Motivation:

We didnt correctly filter out TLSv1.3 ciphers when TLSv1.3 is not enabled.

Modifications:

- Filter out ciphers that are not supported due the selected TLS version
- Add unit test

Result:

Fixes https://github.com/netty/netty/issues/10911

Co-authored-by: Bryce Anderson <banderson@twitter.com>
2021-01-28 11:00:51 +01:00
Carl Mastrangelo
16f2a5e49f
Don't use Fixed ports for UDP test (#10961)
Motivation:
If the given port is already bound, the PcapWriteHandlerTest will sometimes fail.

Modification:
Use a dynamic port using `0`, which is more reliable

Result:
Less Flaky
2021-01-26 08:30:58 +01:00
Norman Maurer
41e79835f2
Move OpenSsl*X509Certificate to util package and rename it (#10955)
Motivation:

Creating certificates from a byte[] while lazy parse it is general useful and is also needed by https://github.com/netty/netty-incubator-codec-quic/pull/141

Modifications:

Move classes, rename these and make them public

Result:

Be able to reuse code
2021-01-21 11:12:54 +01:00
Norman Maurer
42ba433576
Override ALPN methods on ReferenceCountedOpenSslEngine (#10954)
Motivation:

We should override the get*ApplicationProtocol() methods in ReferenceCountedOpenSslEngine to make it easier for users to obtain the selected application protocol

Modifications:

Add missing overrides

Result:

Easier for the user to get the selected application protocol (if any)
2021-01-21 10:55:16 +01:00
Norman Maurer
ae3eebf11b
Mark some methods as protected to make it easier to write own SslContext implementations (#10953)
Motivation:

We should expose some methods as protected to make it easier to write custom SslContext implementations.
This will be reused by the code for https://github.com/netty/netty-incubator-codec-quic/issues/97

Modifications:

- Add protected to some static methods which are useful for sub-classes
- Remove some unused methods
- Move *Wrapper classes to util package and make these public

Result:

Easier to write custom SslContext implementations
2021-01-21 10:41:33 +01:00
root
a137ce2042 [maven-release-plugin] prepare for next development iteration 2021-01-13 10:28:54 +00:00
root
10b03e65f1 [maven-release-plugin] prepare release netty-4.1.58.Final 2021-01-13 10:27:17 +00:00
root
c6b894d03d [maven-release-plugin] prepare for next development iteration 2021-01-12 11:10:44 +00:00