…#16876)

Motivation:
Clients might choose to omit including the max concurrent streams
setting in the SETTINGS frame they send to the server.
Or the client might open streams before sending the settings frame.
In which case the server should enforce its own limits regardless.

Modification:
In the AbstractHttp2ConnectionHandlerBuilder, make sure to configure the
max active streams on the remote endpoint as early as possible.

Add tests to prove that the enforcement works.

Result:
The max active streams setting is always enforced.

---------

Co-authored-by: Chris Vest <[email protected]>

…ory (netty#16866)

Motivation:

We need to verify that the contained length actually is valid before
retaining the buffer as otherwise we will leak memory.

Modifications:

- Verify length is valid
- Only retain buffer if we not throw exceptio
- Add unit test

Result:

Ensure we never leak memory when handling invalid messages

Motivation:

Netty's DNS resolver uses a predictable PRNG for generating DNS
transaction IDs and defaults to a static UDP source port. This
combination reduces the entropy of DNS queries, enabling DNS Cache
Poisoning (Kaminsky attack).

Modifications:

- Replace usage of ThreadLocalRandom with SecureRandom

Result:

query id is not predictible anymore

…ning queue (netty#15053) (netty#16837)

`FlowControlHandler` has two problems related to read completions:

1. Assume `autoRead == false` and `FlowControlHandler` has already
queued two messages from upstream. If a downstream handler calls
`read()`, `FlowControlHandler` serves that read with the first queued
message. Since the queue is still not empty, `FlowControlHandler` does
not fire `channelReadComplete`.

Because `allowRead == false`, no further messages are delivered
downstream until the downstream handler calls `read()` again. A
downstream handler that waits for `channelReadComplete` before issuing
the next `read()` may therefore stall.

2. Assume `FlowControlHandler` has just emptied its queue and correctly
fired `channelRead` followed by `channelReadComplete`. If
`FlowControlHandler` then receives one or more `channelReadComplete`
events from upstream, it currently relays all of them downstream,
resulting in duplicate `channelReadComplete` events. This is unexpected
because, from the perspective of downstream handlers, there are no
further read operations in progress.

To solve problem 1., we want to ensure that with `autoRead == false`,
`FlowControlHandler` fires `channelReadComplete` after `dequeue`ing a
message for a downstream `read()`, even if the queue is not empty. This,
however, needs to be done carefully as the `read()` can be reentered
after `fireChannelRead(...)`.

To solve problem 2., we want to ensure that `FlowControlHandler` does
not pass through upstream `channelReadComplete` events by default. The
only case where it needs to relay such an event is when there are one or
more downstream `read`s for which we did not fire a message yet.

`FlowControlHandler` now tracks unsatisfied downstream `read()` calls in
`activeReads`. A read is unsatisfied if it has not yet resulted in a
downstream `fireChannelRead(...)`.

Based on this state, `read()`, `channelRead(...)`, and
`channelReadComplete(...)` were adapted to match the target behavior.
`dequeue()` is no longer responsible for firing `channelReadComplete`.

- `FlowControlHandlerTest`: added tests covering the new read-completion
behavior and related edge cases.
- `HttpContentDecompressorTest`: added a reproducer for netty#15053.

`FlowControlHandler` now:

- fires `channelReadComplete` after `dequeue`ing one ore more messages
given there are no further unsatisfied `read()` calls, even if the queue
is not empty (solving Problem 1);
- refires an upstream `channelReadComplete` only for empty reads, i.e.
when upstream completes a read without producing a `channelRead`
(solving Problem 2, fixing netty#15053).

Motivation:
The RedisArrayAggregator was pre-allocating the aggregating ArrayList
instance based on just the array header message.

Modification:
Add a RedisArrayAggregator constructor that takes a max number of
elements as an argument. Deprecate the other constructor and make it use
1.000.000 as a default, defined by a new system property.

Why 1.000.000? It seemed big enough to not cause too many compatibility
problems from this change, yet small enough to somewhat constrain the
resource usage from nefarious peers.

Result:
Less exposure to excessive resource usage by adversarial peers.

Co-authored-by: Chris Vest <[email protected]>

Motivation:

We didn'T limit the maximum number of nested array and so we were in
risk of OOME

Modifications:

- Add constructor that allows to set a limit and use 1024 by default
- Add unit test

Result:

Limit maximum nested arrays

Motivation:
In SCTP, the test fails because the assertion doesn't match the current
code. It's possible the code was updated in response to review comments,
while the test wasn't updated to match.
In Redis, we had two PRs merged in incompatible ways where one broke the
tests of the other.

Modification:
- Correct the assertions to match that the handler now closes the
channel in exceptionCaught.
- Make sure that the RedisArrayAggregator releases its messages through
both the nested array failure path, as well as on handler removed.

Result:
The transport-sctp and codec-redis module tests now passes.

SCTP: Limit the number of inflight incomplete SCTP messages and the number of fragments
Motivation:

SctpMessageCompletionHandler did not inforce any limits and so could cause unbound memory usage.

Modifications:

- Add new constructor that takes a limit for the maximum incomplete SCTP messages in flight and the number of fragments
- Use sane defaults
- Add unit tests

Result:

No more unbound memory usage