Skip to content

Commit

Permalink
Finally solve detection of the plugin messaging channel (#237)
Browse files Browse the repository at this point in the history
Channel detection is now re-fixed. After hours of testing,
  hopefully this will be the last time.
  • Loading branch information
A248 committed Mar 8, 2024
1 parent e2fdeec commit dc18139
Show file tree
Hide file tree
Showing 6 changed files with 46 additions and 42 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,8 @@

import net.kyori.adventure.text.Component;
import net.kyori.adventure.text.ComponentLike;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import space.arim.api.env.AudienceRepresenter;
import space.arim.libertybans.core.config.InternalFormatter;
import space.arim.libertybans.core.env.message.PluginMessage;
import space.arim.omnibus.util.ThisClass;
import space.arim.omnibus.util.concurrent.CentralisedFuture;
import space.arim.omnibus.util.concurrent.FactoryOfTheFuture;

Expand All @@ -42,8 +38,6 @@ public abstract class AbstractEnvEnforcer<P> implements EnvEnforcer<P> {
private final Interlocutor interlocutor;
private final AudienceRepresenter<? super P> audienceRepresenter;

private static final Logger logger = LoggerFactory.getLogger(ThisClass.get());

protected AbstractEnvEnforcer(FactoryOfTheFuture futuresFactory, InternalFormatter formatter,
Interlocutor interlocutor, AudienceRepresenter<? super P> audienceRepresenter) {
this.futuresFactory = Objects.requireNonNull(futuresFactory, "futuresFactory");
Expand Down Expand Up @@ -94,28 +88,6 @@ private CentralisedFuture<Void> sendToThoseWithPermissionNoPrefix(String permiss
return doForAllPlayers((players) -> players.forEach(callback));
}

@Override
public final <D> void sendPluginMessage(P player, PluginMessage<D, ?> pluginMessage, D data) {
if (!sendPluginMessageIfListening(player, pluginMessage, data)) {
logger.error(
"Attempted to send plugin message to {}, but it could not be sent. " +
"This suggests you enabled 'use-plugin-messaging' but are not using a network. " +
"Please address this critical security flaw immediately. " +
"It leaves your server vulnerable to clients spoofing the plugin messaging channel.",
player
);
}
}

/**
* Sends a plugin message to the given player if it is accepted by their client
*
* @param player the player to whom to send the message
* @param pluginMessage the plugin message
* @return true if sent, false if unsupported
*/
public abstract <D> boolean sendPluginMessageIfListening(P player, PluginMessage<D, ?> pluginMessage, D data);

@Override
public final void sendMessageNoPrefix(P player, ComponentLike message) {
audienceRepresenter.toAudience(player).sendMessage(message);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,12 +79,14 @@ public interface EnvEnforcer<@PlatformPlayer P> {
void kickPlayer(P player, Component message);

/**
* Sends a plugin message to the given player. Must be used only for proxies.
* Sends a plugin message to the given player if it is accepted by their client. Only to be used for backend
* servers connected by a network.
*
* @param player the player to whom to send the message
* @param pluginMessage the plugin message
* @return true if sent, false if unsupported
*/
<D> void sendPluginMessage(P player, PluginMessage<D, ?> pluginMessage, D data);
<D> boolean sendPluginMessageIfListening(P player, PluginMessage<D, ?> pluginMessage, D data);

/**
* Sends a message to the given player. Does not include a prefix. <br>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -246,12 +246,20 @@ private Consumer<P> enforcementCallback(Punishment punishment, Component message
case BAN, KICK -> (player) -> {
if (instanceType == InstanceType.GAME_SERVER
&& configs.getMainConfig().platforms().gameServers().usePluginMessaging()) {
envEnforcer.sendPluginMessage(
// Try to kick by plugin message
if (envEnforcer.sendPluginMessageIfListening(
player, new KickPlayer(), new KickPlayer.Data(envEnforcer.getNameFor(player), message)
);
} else {
envEnforcer.kickPlayer(player, message);
)) {
// Kicked by plugin message
return;
}
// This scenario is a remote possibility if the player joins and is very quickly kicked.
// That is, the plugin messaging channel is activated a small period after the join event.
// Using the existing APIs on Bukkit and Sponge, there is no solution to this problem without
// sacrificing the following important warning message for misconfigured setups.
logger.warn("Attempted to send plugin message to {}, but it could not be sent.", player);
}
envEnforcer.kickPlayer(player, message);
};
case MUTE -> (player) -> {
/*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,29 +21,38 @@

import jakarta.inject.Inject;
import jakarta.inject.Singleton;
import org.slf4j.LoggerFactory;
import space.arim.api.env.annote.PlatformPlayer;
import space.arim.libertybans.core.config.Configs;
import space.arim.libertybans.core.env.EnvEnforcer;
import space.arim.libertybans.core.env.EnvMessageChannel;
import space.arim.libertybans.core.env.InstanceType;
import space.arim.libertybans.core.env.PlatformListener;
import space.arim.libertybans.core.env.message.GetServer;
import space.arim.omnibus.util.concurrent.EnhancedExecutor;
import space.arim.omnibus.util.concurrent.FactoryOfTheFuture;

import java.time.Duration;
import java.util.function.Consumer;

@Singleton
public final class ServerNameListenerBase<@PlatformPlayer P, H> implements PlatformListener {

private final Configs configs;
private final FactoryOfTheFuture futuresFactory;
private final EnhancedExecutor enhancedExecutor;
private final InternalScopeManager scopeManager;
private final EnvEnforcer<P> envEnforcer;
private final EnvMessageChannel<H> envMessageChannel;

private final H handler;

@Inject
public ServerNameListenerBase(InstanceType instanceType, Configs configs, InternalScopeManager scopeManager,
public ServerNameListenerBase(InstanceType instanceType, Configs configs, FactoryOfTheFuture futuresFactory,
EnhancedExecutor enhancedExecutor, InternalScopeManager scopeManager,
EnvEnforcer<P> envEnforcer, EnvMessageChannel<H> envMessageChannel) {
this.futuresFactory = futuresFactory;
this.enhancedExecutor = enhancedExecutor;
if (instanceType != InstanceType.GAME_SERVER) {
throw new IllegalStateException("Cannot use server name listener except for backend game servers");
}
Expand Down Expand Up @@ -71,7 +80,23 @@ public void onJoin(P player) {
if (configs.getMainConfig().platforms().gameServers().usePluginMessaging()
&& configs.getScopeConfig().serverName().autoDetect()
&& scopeManager.serverNameUndetected()) {
envEnforcer.sendPluginMessage(player, new GetServer(), null);

// After the join event, sending a plugin message needs to happen on a delay
enhancedExecutor.scheduleOnce(
() -> {
// Recheck some conditions
if (scopeManager.serverNameUndetected()) {
futuresFactory.runSync(() -> {
envEnforcer.sendPluginMessageIfListening(player, new GetServer(), null);
}).exceptionally((ex) -> {
LoggerFactory.getLogger(getClass()).warn("Exception requesting backend server name", ex);
return null;
});
}
},
// 4 seconds is sufficient. If no response after 4 seconds, something is wrong with the proxy.
Duration.ofSeconds(4L)
);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,12 +55,9 @@ <D> boolean sendPluginMessage(Player player, PluginMessage<D, ?> pluginMessage,
boolean listened = player.getListeningPluginChannels().contains(BUNGEE_CHANNEL);
//
// 1. The backend server must NOT be in online mode
// 2. The channel must NOT be listened on, strangely enough
// Explanation: getListeningPluginChannels() should never return BungeeCord, because it is a special channel
// Therefore, if this channel IS listened on, that suggests the client is trying to spoof it.
// The anti-spoof check is used for pre-1.13 clients which may register legacy channel names.
// 2. The channel must be listened on
//
boolean canSend = !plugin.getServer().getOnlineMode() && !listened;
boolean canSend = !plugin.getServer().getOnlineMode() && listened;
if (canSend) {
player.sendPluginMessage(
plugin, BUNGEE_CHANNEL,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ public void kickPlayer(Void player, Component message) {
}

@Override
public <D> void sendPluginMessage(Void player, PluginMessage<D, ?> pluginMessage, D data) {
public <D> boolean sendPluginMessageIfListening(Void player, PluginMessage<D, ?> pluginMessage, D data) {
throw new UnsupportedOperationException();
}

Expand Down

0 comments on commit dc18139

Please sign in to comment.