Skip to content

[#728] Reject TCP self-connects in replication connect paths#729

Open
vharseko wants to merge 1 commit into
OpenIdentityPlatform:masterfrom
vharseko:fix/replication-tcp-self-connect
Open

[#728] Reject TCP self-connects in replication connect paths#729
vharseko wants to merge 1 commit into
OpenIdentityPlatform:masterfrom
vharseko:fix/replication-tcp-self-connect

Conversation

@vharseko

Copy link
Copy Markdown
Member

Fixes #728.

Problem

AssuredReplicationServerTest.testSafeDataLevelOne fails intermittently on
CI during setup (e.g. ubuntu-latest/25 on PR #727's run, unrelated to it):
createFakeReplicationDomain:321 expected [true] but found [false], with the
real cause right above in the server log:

msgID=6 Replication Server failed to start : could not bind to the listen port : 65531.
Error : Address already in use

Root cause

TCP self-connect (simultaneous open), the same bug class as
ZOOKEEPER-2101 / KAFKA-1836 / JDK-8067207:

  1. Test listen ports come from TestCaseUtils.findFreePorts() = bind(0),
    i.e. they live inside the OS ephemeral port range (the OS assigned
    65531 in the first place).
  2. When an RS is shut down between data-provider iterations, its former peers
    (ReplicationBrokers of fake domains, other RSs) keep reconnecting to the
    vacated port in a retry loop.
  3. Each connect() draws a local ephemeral port; on Linux, when the kernel
    picks the destination port itself, the connect succeeds via TCP
    simultaneous open — the socket is connected to itself and now occupies
    the very port the next RS is about to bind.
  4. ReplicationServer.initialize() fails to bind, only logs the error, and
    the RS runs without a listener — the fake domain then cannot connect.

TIME_WAIT was ruled out: the JDK enables SO_REUSEADDR on ServerSocket by
default on POSIX platforms (verified empirically), so rebinding over
TIME_WAIT remnants already works. macOS/BSD refuse self-connects (EINVAL),
consistent with the flake being ubuntu-only.

Fix

Detect self-connected sockets right after connect() in both outgoing
replication connect paths and fail the attempt with ConnectException:

  • StaticUtils.isSelfConnection(Socket) — local endpoint == remote endpoint.
    A legitimate established connection can never satisfy this, so false
    positives are impossible.
  • ReplicationBroker.performPhaseOneHandshake() — the existing error
    handling closes the socket (releasing the stolen port) and retries.
  • ReplicationServer.connect() — the existing failure path closes the socket
    and blacklists the address for a few connect iterations.

Tests

  • TestStaticUtils.testIsSelfConnectionFalseForNormalConnection — both ends
    of a normal loopback connection are not self-connections.
  • TestStaticUtils.testIsSelfConnectionTrueForSelfConnectedSocket — forces a
    deterministic self-connect (binds the local end to the very port being
    connected) and asserts detection; skipped on BSD-based stacks (macOS
    refuses explicit self-connects with EINVAL), runs on the Linux CI legs.
  • GenerationIdTest (4/4) and TestStaticUtils (1755, 1 skip on macOS) green
    locally with the guard in place — normal broker/RS connects unaffected.

Follow-ups tracked in #728: initialize() swallowing the bind failure, and
the isAddressInUse() dummy-connect probe being self-connect-prone itself.

Test listen ports for replication servers are drawn from the OS ephemeral
range (TestCaseUtils.findFreePorts binds port 0). When an RS is stopped, its
former peers keep reconnecting to the vacated port in a retry loop; on Linux
the kernel eventually assigns the destination port itself as the local port
of such a connect and TCP simultaneous open "establishes" the connection to
itself. The self-connected socket then occupies the very port the next RS is
about to bind: ReplicationServer.initialize() logs "could not bind to the
listen port ... Address already in use", swallows the error, and the RS runs
without a listener - the intermittent
AssuredReplicationServerTest.testSafeDataLevelOne setup failure on CI
(isConnected() expected [true] but found [false]).

Detect self-connected sockets right after connect() in both outgoing paths
(ReplicationBroker.performPhaseOneHandshake, ReplicationServer.connect) and
fail the attempt with ConnectException: the existing error handling closes
the socket - releasing the port - and retries or blacklists as usual. Same
fix as ZOOKEEPER-2101 / KAFKA-1836 for the identical bug. A legitimate
connection can never have identical local and remote endpoints, so no false
positives are possible.

TIME_WAIT was ruled out: the JDK enables SO_REUSEADDR on ServerSocket by
default on POSIX platforms, so rebinding over TIME_WAIT already works.
@vharseko vharseko added bug java Pull requests that update java code replication concurrency Thread-safety / race-condition bugs tests Test suites: fixing, enabling, un-disabling labels Jul 10, 2026
@vharseko vharseko requested a review from maximthomas July 10, 2026 19:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug concurrency Thread-safety / race-condition bugs java Pull requests that update java code replication tests Test suites: fixing, enabling, un-disabling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Replication port stolen by TCP self-connect: RS silently starts without listener (testSafeDataLevelOne CI flake)

2 participants