From b34d7aef50e6b650d75e53b0df8f09ca49678b9a Mon Sep 17 00:00:00 2001 From: Namnarak Date: Tue, 13 Oct 2026 01:26:32 +0700 Subject: [PATCH] fix(auth): bind active pre-login channel before native handler is ready --- CHANGELOG.md | 7 ++++ pom.xml | 2 +- .../netty/manager/NetworkManagerInjector.java | 9 +++-- .../temporary/TemporaryPlayerFactory.java | 25 ++++++++++-- src/main/resources/plugin.yml | 2 +- .../manager/PreLoginInjectorBindingTest.java | 38 +++++++++++++++++++ 6 files changed, 75 insertions(+), 8 deletions(-) create mode 100644 src/test/java/com/comphenix/protocol/injector/netty/manager/PreLoginInjectorBindingTest.java diff --git a/CHANGELOG.md b/CHANGELOG.md index 1eb1879..b13f176 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,10 @@ +## 1.0.10 — Late-binding race during Java Premium login + +- Fix the v1.0.9 startup race where `NetworkManagerInjector.getTemporaryInjector` refuses to associate a valid active PacketEvents pre-login Channel with the generated `TemporaryPlayer` if native `packet_handler` has not appeared yet. FastLoginPlus thus loses its Channel for the whole verification session and aborts as disconnected. +- Bind **active Channel first**, without requiring the native Minecraft Connection to exist yet; then refresh/resolve the genuine NMS Connection lazily every time FastLoginPlus calls `TemporaryPlayerFactory.getInjectorFromPlayer`. A non-active or non-Netty channel remains rejected. +- Do not change the authentication policy or inject encryption into incomplete channels. The native Connection is still required when the AES setter is called; missing Connection cannot silently allow a cracked bypass. +- Add regression coverage for active pre-login channels before native `packet_handler` is present and late handler attachment. + ## 1.0.9 — FastLoginPlus native login channel association - Associate the ProtocolLib `TemporaryPlayer` generated during PacketEvents pre-login callbacks with its actual Netty injector instead of leaving `TemporaryPlayerFactory.getInjectorFromPlayer()` null. FastLoginPlus uses that accessor during Mojang session verification. diff --git a/pom.xml b/pom.xml index 024f859..10ebda7 100644 --- a/pom.xml +++ b/pom.xml @@ -5,7 +5,7 @@ com.comphenix.protocol ProtocolLib2PacketEvents - 1.0.9 + 1.0.10 jar ProtocolLib2PacketEvents diff --git a/src/main/java/com/comphenix/protocol/injector/netty/manager/NetworkManagerInjector.java b/src/main/java/com/comphenix/protocol/injector/netty/manager/NetworkManagerInjector.java index d096e97..b700f5d 100644 --- a/src/main/java/com/comphenix/protocol/injector/netty/manager/NetworkManagerInjector.java +++ b/src/main/java/com/comphenix/protocol/injector/netty/manager/NetworkManagerInjector.java @@ -57,9 +57,12 @@ public Injector getTemporaryInjector(com.github.retrooper.packetevents.protocol. return null; } netty.refreshNativeConnection(); - // A stale/partial channel cannot authenticate a login. Fail closed: - // don't give FastLoginPlus an injector whose networkManager is null. - if (!netty.hasNativeConnection()) return null; + // The PacketEvents LOGIN_START callback can precede the native + // packet_handler's installation. Channel access is independently safe: + // bind now so FastLoginPlus can track a still-connected pre-login user. + // The native Connection is resolved lazily when FastLoginPlus asks for + // the injector again during premium verification/AES initialization. + // No new encryption or authentication decision is made here. netty.setPlayer(temporary); return netty; } diff --git a/src/main/java/com/comphenix/protocol/injector/temporary/TemporaryPlayerFactory.java b/src/main/java/com/comphenix/protocol/injector/temporary/TemporaryPlayerFactory.java index 0dc2e26..660a566 100644 --- a/src/main/java/com/comphenix/protocol/injector/temporary/TemporaryPlayerFactory.java +++ b/src/main/java/com/comphenix/protocol/injector/temporary/TemporaryPlayerFactory.java @@ -35,9 +35,28 @@ private TemporaryPlayerFactory() { } public static Injector getInjectorFromPlayer(Player player) { - return player instanceof TemporaryPlayer - ? ((TemporaryPlayer) player).getInjector() - : null; + if (!(player instanceof TemporaryPlayer temporary)) return null; + Injector injector = temporary.getInjector(); + // FastLoginPlus retrieves this adapter at multiple asynchronous login + // phases. When LOGIN_START ran before the PacketEvents/native pipeline + // was fully initialized, retry on the same live User+Channel rather + // than permanently treating a legitimate connection as disconnected. + com.github.retrooper.packetevents.protocol.player.User user = + TemporaryPlayerAdapter.getUser(player); + if (user != null) { + org.bukkit.plugin.Plugin plugin = com.comphenix.protocol.ProtocolLibrary.getPlugin(); + if (plugin instanceof com.comphenix.protocol.ProtocolLib bridge) { + Injector resolved = bridge.getTemporaryInjector(user, player); + if (resolved != null) { + if (resolved != injector) temporary.setInjector(resolved); + injector = resolved; + } + } + } + if (injector instanceof com.comphenix.protocol.injector.netty.channel.NettyChannelInjector netty) { + netty.refreshNativeConnection(); + } + return injector; } public static void setInjectorForPlayer(Player player, Injector injector) { diff --git a/src/main/resources/plugin.yml b/src/main/resources/plugin.yml index b9def08..1bccbdf 100644 --- a/src/main/resources/plugin.yml +++ b/src/main/resources/plugin.yml @@ -10,7 +10,7 @@ main: com.comphenix.protocol.ProtocolLib version: 5.4.0 author: CyoriaSMP Team description: A drop-in ProtocolLib compatibility layer powered by PacketEvents. -p2p-version: 1.0.9 +p2p-version: 1.0.10 protocol-lib-api-version: 5.4.0 depend: [packetevents] provides: [ProtocolLib] diff --git a/src/test/java/com/comphenix/protocol/injector/netty/manager/PreLoginInjectorBindingTest.java b/src/test/java/com/comphenix/protocol/injector/netty/manager/PreLoginInjectorBindingTest.java new file mode 100644 index 0000000..980e445 --- /dev/null +++ b/src/test/java/com/comphenix/protocol/injector/netty/manager/PreLoginInjectorBindingTest.java @@ -0,0 +1,38 @@ +package com.comphenix.protocol.injector.netty.manager; + +import com.comphenix.protocol.injector.netty.channel.NettyChannelInjector; +import com.comphenix.protocol.injector.temporary.TemporaryPlayerFactory; +import com.comphenix.protocol.reflect.ObjectAllocator; +import com.github.retrooper.packetevents.protocol.player.User; +import io.netty.channel.embedded.EmbeddedChannel; +import java.lang.reflect.Field; +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.*; + +class PreLoginInjectorBindingTest { + @Test + void activeChannelBindsBeforeNativeConnectionAndCanRefreshLater() throws Exception { + EmbeddedChannel channel = new EmbeddedChannel(); + User user = ObjectAllocator.allocate(User.class); + Field field = User.class.getDeclaredField("channel"); + field.setAccessible(true); + field.set(user, channel); + var player = TemporaryPlayerFactory.createTemporaryPlayer(); + try (NetworkManagerInjector manager = new NetworkManagerInjector(null, null, null)) { + var early = manager.getTemporaryInjector(user, player); + assertInstanceOf(NettyChannelInjector.class, early, + "valid channel must bind before native Connection exists"); + assertFalse(((NettyChannelInjector) early).hasNativeConnection()); + TemporaryPlayerFactory.setInjectorForPlayer(player, early); + assertSame(early, TemporaryPlayerFactory.getInjectorFromPlayer(player)); + channel.pipeline().addLast("packet_handler", new net.minecraft.network.Connection()); + var late = manager.getTemporaryInjector(user, player); + assertSame(early, late, "must not replace channel injector mid-login"); + assertTrue(((NettyChannelInjector) late).hasNativeConnection()); + assertSame(late, TemporaryPlayerFactory.getInjectorFromPlayer(player)); + } finally { + channel.finishAndReleaseAll(); + } + } +}