OF-2559: Introduce Netty for S2S & C2S #2220

viv · 2023-07-18T11:09:54Z

Initial attempt at introducing Netty for S2S and C2S connections. Still to do are:

Implement NettyConnectioHandler.reconfigure()
Properly handle the idle state
Consider impact on stats counter

Needs extensive testing but should now support TLS 1.3.

pom.xml

xmppserver/src/main/java/org/jivesoftware/admin/PluginFilter.java

xmppserver/src/main/java/org/jivesoftware/openfire/spi/MINAConnectionAcceptor.java

Replace CopyOnWriteMap from MINA; migrating to netty so replacing MINA utility with something similar. Remove MINA-specific stat collector; migrating to netty so removing MINA specific stat collector. For netty we might look to the following in the future to implement a netty-specific stats collector: - https://netty.io/4.0/api/io/netty/handler/ssl/OpenSslSessionStats.html - https://netty.io/4.0/api/io/netty/handler/traffic/package-summary.html

…incoming c2s and s2s connections

XMLLightweightParserTest now works with the newly refactored XMLLightweightParser (which had MINA specifics extracted).

MINA will wrap the non-mina exception further up the chain, we were not using the hexdump feature the MINA exception added.

…ected

…tionAcceptor.buildSocketAcceptor function

…implementation

Tested with non-netty outbound server to a netty-based inbound, so using TLS 1.2 as restricted by outbound capabilities until we pull netty through into outbound connection.

TLS 1.2 & 1.3 working with S2S

Increased time allowed dialback DNS lookup to timeout and enabled us to debug deeper into the flow.

We are moving to Netty.

Was part of an incomplete migration to Mina.

Explicitly show this by removing the Mina-based implementations from the ConnectionAcceptor.

… S2S connections Adds IdleStateHandler and NettyIdleStateKeepAliveHandler to the NettyServerInitializer pipeline code. If an inbound session idles Openfire will either send a ping to keep the connection alive, or close the connection.

Tested with non-netty outbound server to a netty-based inbound, so using TLS 1.2 as restricted by outbound capabilities until we pull netty through into outbound connection.

TLS 1.2 & 1.3 working with S2S

…rations

Rather than wait for the Netty-based session to timeout (default 5s) before attempting dialback auth this commit moves the fallback dialback code into Netty-land by listening for `SslHandshakeCompletionEvent`. There's more refactoring required, I dislike the state leaking through the stanza handler - there is perhaps a need for a connection/session that wraps the netty connection. This concept might already exist but can't quite get my head around it yet.

The Connection interface defines methods to read the configuration of TLS and compression policies, even though these are also defined by the ConnectionConfiguration instance that is used to create the connection. It is undesirable to have the configuration of a connection be defined in various places, or be modified after the original connection has been applied. This commit removes the duplication, and ensures that connection configuration is applied as soon as the instance is created. As a side-effect, this solves an issue with the new Netty code, that never explicitly sets the tlsPolicy on the connection. The single functional aspect of the separation of tlsPolicy between connection and configuration (prior to this change) was the following: the state of the connection-tlsPolicy was used to implicitly define if a session was initialized (this was used to close a connection that was sending unencrypted data, when its configuration required encryption). This commit replaces that implicit defintion by a new, explicit 'isInitialized` method on the Connection interface.

…nt connections

…lexer connections We can now deprecate (and remove) all NIO components that were built using the Apache MINA framework

We were seeing resource limit issues (too many open files) when running Outgoing S2S tests. This was caused by the outbound session initialisation failing to clean up its NioEventLoopGroup in many scenarios.

…ted implementations

When the identity store does not contain any certificates, inbound TLS will never be able to succeed. In such cases, lets not advertise the StartTLS feature.

By waiting for handshake to complete before attempting SASL

… LocalOutboundServerSessionTest pass. Also add generic typing for Connection.starttls return type, some tidy up of comments and WIP code.

…eption check

…penfire into OF-2559_mina-to-netty # Conflicts: # xmppserver/src/main/java/org/jivesoftware/openfire/net/VirtualConnection.java # xmppserver/src/main/java/org/jivesoftware/openfire/nio/NettyConnection.java

Prior to this commit handlers were being shared across all sessions. Now a new handler is instantiated per connection/session.

Prior to this commit SSL Handshake events were not making it down the netty pipeline to our client connection handler (aka business logic handler). This meant that inbound connections were never set to encrypted=true causing the session to be abandoned when TLS was required.

The old implementation (still used by ServerDialback) is unable to negotiate a TLS 1.3 connection. Netty-based connections can use TLS 1.3.

guusdk reviewed Jul 18, 2023

View reviewed changes

pom.xml Outdated Show resolved Hide resolved

guusdk reviewed Jul 18, 2023

View reviewed changes

xmppserver/src/main/java/org/jivesoftware/admin/PluginFilter.java Outdated Show resolved Hide resolved

guusdk reviewed Jul 18, 2023

View reviewed changes

xmppserver/src/main/java/org/jivesoftware/openfire/spi/MINAConnectionAcceptor.java Outdated Show resolved Hide resolved

AlexGidman force-pushed the OF-2559_mina-to-netty branch from 01098be to 880ef47 Compare July 18, 2023 13:21

guusdk mentioned this pull request Jul 19, 2023

OF-1112: Switch to MINA for inbound S2S #2159

Closed

viv force-pushed the OF-2559_mina-to-netty branch from b3afb6f to 36abd9a Compare July 25, 2023 07:43

viv and others added 24 commits July 25, 2023 10:04

feat: OF-2559 - add compression for inbound and outbound traffic for …

b0f8731

…incoming c2s and s2s connections

OF-2559 Fixed tests after refactor

2c91f91

XMLLightweightParserTest now works with the newly refactored XMLLightweightParser (which had MINA specifics extracted).

OF-2559 Removed last of MINA from XMLLightweightParser

00aa6fa

MINA will wrap the non-mina exception further up the chain, we were not using the hexdump feature the MINA exception added.

fix: OF-2559 - isIdle now returns true when channel open but not conn…

16a412e

…ected

feat: OF-2559 - add channel options that match settings in MinaConnec…

5af394b

…tionAcceptor.buildSocketAcceptor function

feat: OF-2559 - add WriteTimeoutHandler to handle stalled sessions

6d9c6f3

fix: OF-2559 - improve write timeout system property description and …

78f05a7

…implementation

OF-2559 Added TLS handler for inbound netty connections

1650fe5

Tested with non-netty outbound server to a netty-based inbound, so using TLS 1.2 as restricted by outbound capabilities until we pull netty through into outbound connection.

OF-2559 S2S outbound with Netty

48c9599

TLS 1.2 & 1.3 working with S2S

OF-2559 - Direct TLS on Netty inbound and outbound S2S

c8a2ba2

OF-2559 Refactor to remove duplication

c52100f

OF-2559 Remove temporary long timeout

ee5bd07

Increased time allowed dialback DNS lookup to timeout and enabled us to debug deeper into the flow.

OF-2559 Tidy up, removed unused properties

3f6241f

OF-2559 Tidy up comments

dec32a3

OF-2559 Refactor: extracted isDirectTLS method

cc177aa

OF-2559 Refactor: Log to application log

fe819fb

OF-2559 Deprecate Mina-based ClientConnectionHandler

e71831c

We are moving to Netty.

OF-2559 Improved comments

ad3c115

OF-2559 Removed unused connection handler

4ee6a39

Was part of an incomplete migration to Mina.

OF-2559 Netty is always used for S2S and C2S

04ecba7

Explicitly show this by removing the Mina-based implementations from the ConnectionAcceptor.

OF-2559 Refactor to remove unused code following Netty migration

31c1b23

feat: OF-2559 - surface session initialise timeout to a system property

e186bc5

viv and others added 6 commits July 25, 2023 10:34

OF-2559 Added TLS handler for inbound netty connections

e56b34c

Tested with non-netty outbound server to a netty-based inbound, so using TLS 1.2 as restricted by outbound capabilities until we pull netty through into outbound connection.

OF-2559 S2S outbound with Netty

545fc8c

TLS 1.2 & 1.3 working with S2S

OF-2559 - Direct TLS on Netty inbound and outbound S2S

394c090

OF-2559 Tidy up comments

d28136c

OF-2559 Improved comments

2066e82

OF-2559 Remove HTML tags in comments

987eb52

viv force-pushed the OF-2559_mina-to-netty branch from 50b87a2 to 987eb52 Compare July 25, 2023 09:45

AlexGidman and others added 12 commits July 25, 2023 14:13

feat: OF-2599 - add stats updates on Netty channel read and write ope…

3f4b065

…rations

fix: OF-2599 - move NettyConnectionHandler creation to a factory method

b43d30b

feat: OF-2599 - add NettyComponentConnectionHandler to handle Compone…

c89095a

…nt connections

feat: OF-2599 - add NettyMultiplexerConnectionHandler to handle Mutip…

c095508

…lexer connections We can now deprecate (and remove) all NIO components that were built using the Apache MINA framework

OF-2559 Ensure NioEventLoopGroup is closed for outbound S2S

b25f6ba

We were seeing resource limit issues (too many open files) when running Outgoing S2S tests. This was caused by the outbound session initialisation failing to clean up its NioEventLoopGroup in many scenarios.

feat: OF-2599 - Delete all MINA dependencies, references, and depreca…

ce54590

…ted implementations

OF-2632: Do not offer StartTLS when the identity store is empty

2b95a74

When the identity store does not contain any certificates, inbound TLS will never be able to succeed. In such cases, lets not advertise the StartTLS feature.

OF-2559 WIP Fixed TLS negotiation

b3832d9

By waiting for handshake to complete before attempting SASL

fix: OF-2559 - re-implement strictCertificateValidation logic so that…

681c496

… LocalOutboundServerSessionTest pass. Also add generic typing for Connection.starttls return type, some tidy up of comments and WIP code.

chore: OF-2559 - remove unnecessary System.out statements

f591cc9

AlexGidman force-pushed the OF-2559_mina-to-netty branch from 05ab969 to f591cc9 Compare July 31, 2023 11:03

AlexGidman and others added 8 commits July 31, 2023 14:21

fix: OF-2599 - switch SSLHandshakeException check to a CertificateExc…

edec92b

…eption check

OF-2559 WIP startTLS does not need to return

8d8dd7f

fix: OF-2599 - remove generic type on connection

e4dd9df

Merge branch 'OF-2559_mina-to-netty' of https://github.com/surevine/O…

1d10a99

…penfire into OF-2559_mina-to-netty # Conflicts: # xmppserver/src/main/java/org/jivesoftware/openfire/net/VirtualConnection.java # xmppserver/src/main/java/org/jivesoftware/openfire/nio/NettyConnection.java

OF-2559: Remove MINA artifacts from pom.xml

381286d

OF-2559 Create new business logic handler for each session

7c9f90d

Prior to this commit handlers were being shared across all sessions. Now a new handler is instantiated per connection/session.

OF-2559 Prevent old ssl engine from using TLS 1.3

5e06b1e

The old implementation (still used by ServerDialback) is unable to negotiate a TLS 1.3 connection. Netty-based connections can use TLS 1.3.

viv marked this pull request as ready for review August 1, 2023 12:45

guusdk merged commit 5f135aa into igniterealtime:main Aug 1, 2023
15 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

OF-2559: Introduce Netty for S2S & C2S #2220

OF-2559: Introduce Netty for S2S & C2S #2220

viv commented Jul 18, 2023

OF-2559: Introduce Netty for S2S & C2S #2220

OF-2559: Introduce Netty for S2S & C2S #2220

Conversation

viv commented Jul 18, 2023