Commit Graph

1277 Commits

Author SHA1 Message Date
Norman Maurer
07baabaac5
Remove Progressive*Promise / Progressive*Future (#11374)
Motivation:

This special case implementation of Promise / Future requires the implementations responsible for completing the promise to have knowledge of this class to provide value. It also requires that the implementations are able to provide intermediate status while the work is being done. Even throughout the core of Netty it is not really supported most of the times and so just brings more complexity without real gain.

Let's remove it completely which is better then only support it sometimes.

Modifications:

Remove Progressive* API

Result:

Code cleanup.... Fixes https://github.com/netty/netty/issues/8519
2021-06-09 08:32:38 +02:00
Norman Maurer
abdaa769de
Remove Void*Promise (#11348)
Motivation:

Sometime in the past we introduced the concept of Void*Promise. As it turned out this was not a good idea at all as basically each handler in the pipeline need to be very careful to correctly handle this. We should better just remove this "optimization".

Modifications:

- Remove Void*Promise and all the related APIs
- Remove tests which were related to Void*Promise

Result:

Less error-prone API
2021-06-08 14:22:16 +02:00
skyguard1
21c3de9c69 Fix IpSubnetFilterRule with IPv6 Default Route does not accept all IPv6 addresses (#11351)
Motivation:

In this issue(https://github.com/netty/netty/issues/11349 ),IpSubnetFilterRule needs to support ipv6 reserved addresses, such as 8000::, but the current implementation does not support

Modification:

Added support for default rule

Result:

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

Signed-off-by: xingrufei <xingrufei@sogou-inc.com>
2021-06-07 08:07:35 +02:00
Norman Maurer
7b3e28f96f
Disable two tests which currently fail in master (#11344)
Motivation:

We have currently two test-failures in master. Let's disable these and then open a PR with a fix once we know why. This way we can make progress in master

Modifications:

Disable the two failing tests

Result:

Master builds again
2021-06-01 13:41:20 +02:00
skyguard1
cfd64f7ced Add default block in IdleStateHandler (#11341)
Motivation:

We should have a default case in every switch block.

Modification:

Add default block in IdleStateHandler

Result:

Cleanup

Signed-off-by: xingrufei <xingrufei@sogou-inc.com>
2021-06-01 09:22:02 +02:00
Norman Maurer
f0c919768b Add license header to our scripts and workflows (#11282)
Motivation:

We should have license header whenever possible.

Modifications:

Add header to scripts and workflow config

Result:

More clear licensing
2021-05-19 14:07:00 +02:00
Norman Maurer
c56e5e6e4f Fail the build if we can't load the OpenSSL library (#11269)
Motivation:

We should better fail the build if we can't load the OpenSSL library to ensure we not introduce a regression at some point related to native library loading

Modifications:

Remove usages of assumeTrue and let the tests fail if we cant load the native lib

Result:

Ensure we not regress
2021-05-19 11:03:01 +02:00
Aayush Atharva
0d93c24547 Change asterisk to 'x' in FQDN of SelfSignedCertificate (#11245)
Motivation:

`SelfSignedCertificate` creates a certificate and private key files and store them in a temporary directory. However, if the certificate uses a wildcard hostname that uses asterisk *, e.g. `*.shieldblaze.com`, it'll throw an error because * is not a valid character in the file system.

Modification:
Replace the asterisk with 'x'

Result:
Fixes #11240
2021-05-12 19:33:05 +02:00
Norman Maurer
1602b96a25 Use tasks by default when using openssl based SSL provider (#11242)
Motivation:

We introduced the ability to offload certain operations to an executor that may take some time to complete. At the moment this is not enabled by default when using the openssl based SSL provider. Let's enable it by default as we have this support for some while now and didnt see any issues yet. This will also make things less confusing and more consistent with the JDK based provider.

Modifications:

Use true as default value for io.netty.handler.ssl.openssl.useTasks.

Result:

Offloading works with openssl based SSL provider as well by default
2021-05-12 15:01:22 +02:00
Norman Maurer
f546718df6 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:55 +02:00
Norman Maurer
e54aeea1da 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:41:17 +02:00
Idel Pivnitskiy
b9685a63de 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:53:47 +02:00
skyguard1
e10c1af314 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:47 +02:00
Chris Vest
584d674acd 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:15:17 +02:00
Scott Mitchell
382885538f 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 10:08:11 +02:00
Norman Maurer
c919b385e2 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:49:06 +02:00
skyguard1
f221e4d706 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:05:56 +02:00
Boris Unckel
83297ed2ba 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 13:00:33 +02:00
Karsten Ohme
d4b9001d1f 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-22 08:04:13 +02:00
Chris Vest
d1b896b701 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 09:17:32 +02:00
Scott Mitchell
59867fa0fd 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:33:13 -07:00
Scott Mitchell
3049eacc45 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 09:49:40 -07:00
Idel Pivnitskiy
01768f0a65 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:10:19 +02:00
Chris Vest
6d35db57bd 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:42:15 +02:00
Scott Mitchell
ad7372b112 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 11:00:54 +02:00
Norman Maurer
98b9158f66 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-20 12:46:08 +01:00
Scott Mitchell
e5ff6216ff 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:27:10 +01:00
Norman Maurer
8a9320161c 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:35:31 +01:00
Norman Maurer
d209eb0e18 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-03-07 19:30:17 +01:00
Norman Maurer
b1a8de0d7a 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-03-07 19:25:22 +01:00
Norman Maurer
2ce03e0a08 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:27:02 +01:00
Norman Maurer
bcb165bccd 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 22:02:27 +01:00
Chris Vest
d8ae9dfea3
Bring forward build automation changes (#11052)
This brings forward the build and release automation changes from 4.1 (#10879, #10883, #10884, #10886, #10888, #10889, #10893, #10900, #10933, #10945, #10966, #10968, #11002, and #11019) to 5.0.

Details are as follows:

* Use Github workflows for CI (#10879)

Motivation:

We should just use GitHub Actions for the CI

Modifications:

- Adjust docker / docker compose files
- Add different workflows and jobs to deploy and build the project

Result:

Don't depend on external CI services

* Fix non leak build condition

* Only use build and deploy workflows for 4.1 for now

* Add deploy job for cross compiled aarch64 (#10883)

Motivation:

We should also deploy snapshots for our cross compiled native jars.

Modifications:

- Add job and docker files for deploying cross compiled native jars
- Ensure we map the maven cache into our docker containers

Result:

Deploy aarch64 jars and re-use cache

* Use correct docker-compose file to deploy cross compiled artifacts

* Use correct docker-compose task to deploy for cross compiled artifacts

* Split pr and normal build (#10884)

Motivation:

We should better use seperate workflows for PR and normal builds

Modifications:

- Split workflows
- Better cache reuse

Result:

Cleanup

* Only deploy snapshots for one arch

Motivation:

We need to find a way to deploy SNAPSHOTS for different arch with the same timestamp. Otherwise it will cause problems.

See https://github.com/netty/netty/issues/10887

Modification:

Skip all other deploys then x86_64

Result:

Users are able to use SNAPSHOTS for x86_6

* Use maven cachen when running analyze job (#10888)

Motivation:

To prevent failures to problems while downloading dependencies we shoud cache these

Modifications:

Add maven cache

Result:

No more failures due problems while downloading dependencies

* Also include one PR job that uses boringssl (#10886)

Motivation:

When validating PRs we should also at least run one job that uses boringssl

Modifications:

- Add job that uses boringssl
- Cleanup docker compose files
- Fix buffer leak in test

Result:

Also run with boringssl when PRs are validated

* Use matrix for job configurations (#10889)

Motivation:

We can use the matrix feature to define our jobs. This reduces a lot of config

Modification:

Use job matrix

Result:

Easier to maintain

* Correctly deploy artifacts that are build on different archs (#10893)

Motivation:

We need to take special care when deploying snapshots as we need to generate the jars in multiple steps

Modifications:

- Use the nexus staging pluging to stage jars locally in multiple steps
- Add extra job that will merge these staged jars and deploy these

Result:

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

* Dont use cron for PRs

Motivation:

It doesnt make sense to use cron for PRs

Modifications:

Remove cron config

Result:

Cleanup

* We run all combinations when validate the PR, let's just use one type for normal push

Motivation:

Let us just only use one build config when building the 4.1 branch.

Modifications:

As we already do a full validation when doing the PR builds we can just only use one build config for pushes to the "main" branches

Result:

Faster build times

* Update action-docker-layer-caching (#10900)

Motivation:

We are three releases behind.

Modifications:

Update to latest version

Result:

Use up-to-date action-docker-layer-caching version

* Verify we can load native modules and add job that verifies on aarch64 as well (#10933)

Motivation:

As shown in the past we need to verify we actually can load the native as otherwise we may introduce regressions.

Modifications:

- Add new maven module which tests loading of native modules
- Add job that will also test loading on aarch64

Result:

Less likely to introduce regressions related to loading native code in the future

* Let script fail if one command fail (#10945)

Motivation:

We should use `set -e` to ensure we fail the script if one command fails.

Modifications:

Add set -e to script

Result:

Fail fast

* Use action to report unit test errors (#10966)

Motivation:

To make it easier to understand why the build fails lets use an action that will report which unit test failed

Modifications:

- Replace custom script with action-surefire-report

Result:

Easier to understand test failures

* Use custom script to check for build failures (#10968)

Motivation:

It turns out we can't use the action to check for build failures as it can't be used when a PR is done from a fork. Let's just use our simple script.

Modifications:

- Replace action with custom script

Result:

Builds for PRs that are done via forks work again.

* Publish test results after PR run (#11002)

Motivation:

To make it easier to understand why a build failed let us publish the rest results

Modifications:

Use a new workflow to be able to publish the test reports

Result:

Easier to understand why a PR did fail

* Fix test reports name

* Add workflow to cut releases (#11019)

Motivation:

Doing releases manually is error-prone, it would be better if we could do it via a workflow

Modification:

- Add workflow to cut releases
- Add related scripts

Result:

Be able to easily cut a release via a workflow

* Update build for master branch

Motivation:
The build changes were brought forward from 4.1, and contain many things specific to 4.1.

Modification:
Changed baseline Java version from 8 to 11, and changed branch references from "4.1" to "master".

Result:
Builds should now work for the master branch.

Co-authored-by: Norman Maurer <norman_maurer@apple.com>
2021-03-02 17:44:03 +01:00
Chris Vest
ec18aa8731
Introduce ByteBufConvertible interface (#11036)
Motivation:
To make it possible to experiment with alternative buffer implementations, we need a way to abstract away the concrete buffers used throughout most of the Netty pipelines, while still having a common currency for doing IO in the end.

Modification:
- Introduce an ByteBufConvertible interface, that allow arbitrary objects to convert themselves into ByteBuf objects.
- Every place in the code, where we did an instanceof check for ByteBuf, we now do an instanceof check for ByteBufConvertible.
- ByteBuf itself implements ByteBufConvertible, and returns itself from the asByteBuf method.

Result:
It is now possible to use Netty with alternative buffer implementations, as long as they can be converted to ByteBuf.
This has been verified elsewhere, with an alternative buffer implementation.
2021-02-26 15:03:58 +01:00
Idel Pivnitskiy
b29df0f87a 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:05:26 +01:00
Chris Vest
d60b1651fc 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 14:29:03 +01:00
Norman Maurer
b43b67553e 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:38 +01:00
Norman Maurer
42a2c9c831 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:56:42 +01:00
Norman Maurer
9c2de76add 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 18:17:31 +01:00
Norman Maurer
f25b12077a 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:55 +01:00
Norman Maurer
980ea9c351 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:56:12 +01:00
Norman Maurer
99fc0e486d 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:01:57 +01:00
Carl Mastrangelo
c68af5c8f4 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:31:22 +01:00
Norman Maurer
56d2635733 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 14:07:44 +01:00
Norman Maurer
6ae8cd6e44 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 14:07:28 +01:00
Norman Maurer
dc632e378f We need to ensure we always drain the error stack when a callback throws (#10920)
Motivation:

We need to ensure we always drain the error stack when a callback throws as otherwise we may pick up the error on a different SSL instance which uses the same thread.

Modifications:

- Correctly drain the error stack if native method throws
- Add a unit test which failed before the change

Result:

Always drain the error stack
2021-01-11 20:57:16 +01:00
Norman Maurer
4f7e6d4841 Workaround possible JDK bug in SSLEngineImpl when using TLSv1.3 that lead to multiple notifications (#10860)
Motivation:

When using the JDKs SSLEngineImpl with TLSv1.3 it sometimes returns HandshakeResult.FINISHED multiple times. This can lead to have SslHandshakeCompletionEvents to be fired multiple times.

Modifications:

- Keep track of if we notified before and if so not do so again if we use TLSv1.3
- Add unit test

Result:

Consistent usage of events
2020-12-15 08:07:19 +01:00
terrarier2111
38e06ef7ae Removed redundant assignment (#10853)
Motivation:

Found a redundant assignment.

Modification:

Removed the redundant assignment.

Result:

Minor performance improvement.
2020-12-10 10:29:49 +01:00
Norman Maurer
6c446d14fd OpenSsl.memoryAddress(...) should use internalNioBuffer(...) if it can't access the memoryAddress (#10818)
Motivation:

We can make use of internalNioBuffer(...) if we cant access the memoryAddress. This at least will reduce the object creations.

Modifications:

Use internalNioBuffer(...) and so reduce the GC

Result:

Less object creation if we can't access the memory address.
2020-11-25 10:32:43 +01:00
Norman Maurer
eeece4cfa5 Use http in xmlns URIs to make maven release plugin happy again (#10788)
Motivation:

https in xmlns URIs does not work and will let the maven release plugin fail:

```
[INFO] ------------------------------------------------------------------------
[INFO] BUILD FAILURE
[INFO] ------------------------------------------------------------------------
[INFO] Total time:  1.779 s
[INFO] Finished at: 2020-11-10T07:45:21Z
[INFO] ------------------------------------------------------------------------
[ERROR] Failed to execute goal org.apache.maven.plugins:maven-release-plugin:2.5.3:prepare (default-cli) on project netty-parent: Execution default-cli of goal org.apache.maven.plugins:maven-release-plugin:2.5.3:prepare failed: The namespace xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" could not be added as a namespace to "project": The namespace prefix "xsi" collides with an additional namespace declared by the element -> [Help 1]
[ERROR]
```

See also https://issues.apache.org/jira/browse/HBASE-24014.

Modifications:

Use http for xmlns

Result:

Be able to use maven release plugin
2020-11-10 10:51:05 +01:00