[BLEEDING] Make use of the 'confirm teleport' packet. Cancel WAITING.

If the teleport confirm packet is available, flying packets with
AckResolution.WAITING will be cancelled.

This is real bleeding edge and might need other adjustments not to
freeze players for to be discovered edge cases. The TeleportQueue
already does contain a timeout mechanism and should return
AckResolution.IDLE after some time.
This commit is contained in:
asofold 2017-04-27 12:53:47 +02:00
parent 6ea30131ca
commit 1655a90b2b
5 changed files with 267 additions and 17 deletions

View File

@ -17,6 +17,7 @@ package fr.neatmonster.nocheatplus.checks.net.protocollib;
import java.util.Arrays;
import java.util.HashSet;
import java.util.LinkedHashSet;
import java.util.LinkedList;
import java.util.List;
import org.bukkit.Bukkit;
@ -24,9 +25,12 @@ import org.bukkit.entity.Player;
import org.bukkit.plugin.Plugin;
import com.comphenix.protocol.PacketType;
import com.comphenix.protocol.PacketType.Protocol;
import com.comphenix.protocol.PacketType.Sender;
import com.comphenix.protocol.events.ListenerPriority;
import com.comphenix.protocol.events.PacketContainer;
import com.comphenix.protocol.events.PacketEvent;
import com.comphenix.protocol.reflect.StructureModifier;
import fr.neatmonster.nocheatplus.NCPAPIProvider;
import fr.neatmonster.nocheatplus.checks.net.FlyingFrequency;
@ -34,8 +38,11 @@ import fr.neatmonster.nocheatplus.checks.net.NetConfig;
import fr.neatmonster.nocheatplus.checks.net.NetData;
import fr.neatmonster.nocheatplus.checks.net.model.DataPacketFlying;
import fr.neatmonster.nocheatplus.checks.net.model.DataPacketFlying.PACKET_CONTENT;
import fr.neatmonster.nocheatplus.checks.net.model.TeleportQueue.AckReference;
import fr.neatmonster.nocheatplus.compat.AlmostBoolean;
import fr.neatmonster.nocheatplus.config.ConfPaths;
import fr.neatmonster.nocheatplus.config.ConfigManager;
import fr.neatmonster.nocheatplus.logging.StaticLog;
import fr.neatmonster.nocheatplus.logging.Streams;
import fr.neatmonster.nocheatplus.time.monotonic.Monotonic;
import fr.neatmonster.nocheatplus.utilities.CheckUtils;
@ -63,6 +70,30 @@ public class MovingFlying extends BaseAdapter {
public static final int indexYaw = 0;
public static final int indexPitch = 1;
private static PacketType[] initPacketTypes() {
final List<PacketType> types = new LinkedList<PacketType>(Arrays.asList(
PacketType.Play.Client.FLYING,
PacketType.Play.Client.LOOK,
PacketType.Play.Client.POSITION,
PacketType.Play.Client.POSITION_LOOK
));
// Add confirm teleport.
try {
PacketType confirmType;
// PacketPlayInTeleportAccept
confirmType = PacketType.findCurrent(Protocol.PLAY, Sender.CLIENT, "PacketPlayInTeleportAccept");
if (confirmType != null) {
StaticLog.logInfo("Confirm teleport packet available (via name): " + confirmType);
types.add(confirmType);
}
}
catch (Throwable t) {
// uh
}
return types.toArray(new PacketType[types.size()]);
}
/** Frequency check for flying packets. */
private final FlyingFrequency flyingFrequency = new FlyingFrequency();
@ -77,15 +108,12 @@ public class MovingFlying extends BaseAdapter {
private long packetMismatchLogFrequency = 60000; // Every minute max, good for updating :).
private final HashSet<PACKET_CONTENT> validContent = new LinkedHashSet<PACKET_CONTENT>();
private final PacketType confirmTeleportType = PacketType.findCurrent(Protocol.PLAY, Sender.CLIENT, "PacketPlayInTeleportAccept");
private boolean acceptConfirmTeleportPackets = confirmTeleportType != null;
public MovingFlying(Plugin plugin) {
// PacketPlayInFlying[3, legacy: 10]
super(plugin, ListenerPriority.LOW, new PacketType[] {
PacketType.Play.Client.FLYING,
PacketType.Play.Client.LOOK,
PacketType.Play.Client.POSITION,
PacketType.Play.Client.POSITION_LOOK
});
super(plugin, ListenerPriority.LOW, initPacketTypes());
// Add feature tags for checks.
if (ConfigManager.isTrueForAnyConfig(ConfPaths.NET_FLYINGFREQUENCY_ACTIVE)) {
@ -96,6 +124,58 @@ public class MovingFlying extends BaseAdapter {
@Override
public void onPacketReceiving(final PacketEvent event) {
if (event.getPacketType().equals(confirmTeleportType)) {
if (acceptConfirmTeleportPackets) {
onConfirmTeleportPacket(event);
}
}
else {
onFlyingPacket(event);
}
}
private void onConfirmTeleportPacket(final PacketEvent event) {
try {
processConfirmTeleport(event);
}
catch (Throwable t) {
noConfirmTeleportPacket();
}
}
private void processConfirmTeleport(final PacketEvent event) {
final PacketContainer packet = event.getPacket();
final StructureModifier<Integer> integers = packet.getIntegers();
if (integers.size() != 1) {
noConfirmTeleportPacket();
return;
}
// TODO: Cross check legacy types (if they even had an integer).
Integer teleportId = integers.read(0);
if (teleportId == null) {
// TODO: Not sure ...
return;
}
final Player player = event.getPlayer();
final NetData data = dataFactory.getData(player);
final AlmostBoolean matched = data.teleportQueue.processAck(teleportId);
if (matched.decideOptimistically()) {
CheckUtils.subtract(System.currentTimeMillis(), 1, data.flyingFrequencyAll);
}
if (data.debug) {
debug(player, "Confirm teleport packet" + (matched.decideOptimistically() ? (" (matched=" + matched + ")") : "") + ": " + teleportId);
}
}
private void noConfirmTeleportPacket() {
acceptConfirmTeleportPackets = false;
// TODO: Attempt to unregister.
NCPAPIProvider.getNoCheatPlusAPI().getLogManager().info(Streams.STATUS, "Confirm teleport packet not available.");
}
private void onFlyingPacket(final PacketEvent event) {
final boolean primaryThread = Bukkit.isPrimaryThread(); // TODO: Code review protocol plugin :p.
counters.add(idFlying, 1, primaryThread);
if (event.isAsync() == primaryThread) {
@ -119,10 +199,11 @@ public class MovingFlying extends BaseAdapter {
// Always update last received time.
final NetData data = dataFactory.getData(player);
data.lastKeepAliveTime = time; // Update without much of a contract.
// TODO: Leniency options too (packet order inversion).
// TODO: Leniency options too (packet order inversion). -> current: flyingQueue is fetched.
if (!cc.flyingFrequencyActive) {
return;
}
boolean cancel = false;
// Interpret the packet content.
final DataPacketFlying packetData = interpretPacket(event, time);
@ -144,6 +225,16 @@ public class MovingFlying extends BaseAdapter {
if (data.debug) {
debug(player, "Incoming packet, still waiting for ACK on outgoing position.");
}
// Don't add to the flying queue for now (assumed invalid).
final AckReference ackReference = data.teleportQueue.getLastAckReference();
if (ackReference.lastOutgoingId != Integer.MIN_VALUE
&& ackReference.lastOutgoingId != ackReference.maxConfirmedId) {
// Still waiting for a 'confirm teleport' packet. More or less safe to cancel this out.
// TODO: Timeout -> either skip cancel or schedule a set back (to last valid pos or other).
// TODO: Config?
cancel = true;
}
break;
}
case ACK: {
// Skip processing ACK packets, no cancel.
@ -165,8 +256,8 @@ public class MovingFlying extends BaseAdapter {
// Actual packet frequency check.
// TODO: Consider using the NetStatic check.
boolean cancel = false;
if (!skipFlyingFrequency && flyingFrequency.check(player, packetData, time, data, cc)) {
if (!cancel && !skipFlyingFrequency
&& flyingFrequency.check(player, packetData, time, data, cc)) {
cancel = true;
}
@ -185,9 +276,10 @@ public class MovingFlying extends BaseAdapter {
if (data.debug) {
debug(player, (packetData == null ? "(Incompatible data)" : packetData.toString()) + (event.isCancelled() ? " CANCEL" : ""));
}
}
private boolean isInvalidContent(final DataPacketFlying packetData) {
if (packetData.hasPos && LocUtil.isBadCoordinate(packetData.getX(), packetData.getY(), packetData.getZ())) {
return true;

View File

@ -23,8 +23,10 @@ import com.comphenix.protocol.events.PacketContainer;
import com.comphenix.protocol.events.PacketEvent;
import com.comphenix.protocol.reflect.StructureModifier;
import fr.neatmonster.nocheatplus.NCPAPIProvider;
import fr.neatmonster.nocheatplus.checks.net.NetData;
import fr.neatmonster.nocheatplus.checks.net.model.CountableLocation;
import fr.neatmonster.nocheatplus.logging.Streams;
public class OutgoingPosition extends BaseAdapter {
@ -35,6 +37,8 @@ public class OutgoingPosition extends BaseAdapter {
public static final int indexPitch = 1;
private final Integer ID_OUTGOING_POSITION_UNTRACKED = counters.registerKey("packet.outgoing_position.untracked");
private boolean hasTeleportId = true;
public OutgoingPosition(Plugin plugin) {
// PacketPlayInFlying[3, legacy: 10]
@ -73,8 +77,33 @@ public class OutgoingPosition extends BaseAdapter {
final double z = doubles.read(indexZ);
final float yaw = floats.read(indexYaw);
final float pitch = floats.read(indexPitch);
Integer teleportId = Integer.MIN_VALUE;
if (hasTeleportId) {
try {
final StructureModifier<Integer> integers = packet.getIntegers();
if (integers.size() == 1) {
// Accept as id.
teleportId = integers.read(0);
if (teleportId == null) {
teleportId = Integer.MIN_VALUE;
}
if (teleportId != Integer.MIN_VALUE && data.debug) {
debug(player, "Outgoing confirm teleport id: " + teleportId);
}
}
else {
hasTeleportId = false;
NCPAPIProvider.getNoCheatPlusAPI().getLogManager().info(Streams.STATUS, "PacketPlayOutPosition: Teleport confirm id not available, field mismatch: " + integers.size());
}
}
catch (Throwable t) {
hasTeleportId = false;
NCPAPIProvider.getNoCheatPlusAPI().getLogManager().info(Streams.STATUS, "PacketPlayOutPosition: Teleport confirm id not available.");
}
}
final CountableLocation packetData = data.teleportQueue.onOutgoingTeleport(x, y, z, yaw, pitch);
final CountableLocation packetData = data.teleportQueue.onOutgoingTeleport(x, y, z, yaw, pitch, teleportId);
if (packetData == null) {
// Add counter for untracked (by Bukkit API) outgoing teleport.
// TODO: There may be other cases which are indicated by Bukkit API events.

View File

@ -473,6 +473,7 @@ public class MovingUtil {
* potential for abuse.
*/
// (CANCEL + UPDATE_FROM mean a certain teleport to the set back, still could be repeated tp.)
// TODO: Better method, full sync reference?
final CountableLocation cl = ((NetData) CheckType.NET.getDataFactory().getData(player)).teleportQueue.getLastAck();
if (data.isTeleportedPosition(cl)) {
if (data.debug) {

View File

@ -25,11 +25,19 @@ public class CountableLocation extends DataLocation {
public int count;
public long time;
/** Confirm teleport id: INTEGER.MIN_VALUE means it's not been provided. */
public int teleportId;
public CountableLocation(double x, double y, double z, float yaw, float pitch, int count, long time) {
this(x, y, z, yaw, pitch, count, time, Integer.MIN_VALUE);
}
public CountableLocation(double x, double y, double z, float yaw, float pitch,
int count, long time, int teleportId) {
super(x, y, z, yaw, pitch);
this.time = time;
this.count = count;
this.teleportId = teleportId;
}
@Override
@ -48,6 +56,10 @@ public class CountableLocation extends DataLocation {
builder.append(getYaw());
builder.append(",count=");
builder.append(count);
if (teleportId != Integer.MIN_VALUE) {
builder.append(",tpid=");
builder.append(teleportId);
}
// Skip time for now.
builder.append(")");
return builder.toString();

View File

@ -19,6 +19,8 @@ import java.util.LinkedList;
import java.util.concurrent.locks.Lock;
import java.util.concurrent.locks.ReentrantLock;
import fr.neatmonster.nocheatplus.compat.AlmostBoolean;
/**
* Queue outgoing teleport locations (DataPacketFlying), in order to cancel
* other moving until the client acknowledges previous teleport locations. In
@ -38,6 +40,19 @@ public class TeleportQueue {
IDLE
}
public static class AckReference {
public int lastOutgoingId = Integer.MIN_VALUE;
/**
* The maximum of confirmed ids. Once lastOutgoingId is reached, we
* assume all teleports are done. Until then this id may get adjusted if
* lower than lastOutgoingId. Note that the id will be circling within 0
* to Integer.MAX_VALUE - 1, systematically increasing until back to 0,
* thus this value can be greater than lastOutGoingId, which should be
* treated as 'bad luck', resetting this to Integer.MIN_VALUE.
*/
public int maxConfirmedId = Integer.MIN_VALUE;
}
// TODO: Consider passing a reentrant lock in the constructor (e.g. one lock per NetData).
private final Lock lock = new ReentrantLock();
@ -49,8 +64,14 @@ public class TeleportQueue {
private long maxAge = 4000; // TODO: configurable
private int maxQueueSize = 60; // TODO: configurable
/**
* Queried from the primary thread (read only), reset with outgoing
* teleport.
*/
private CountableLocation lastAck = null;
private AckReference lastAckReference = new AckReference();
/**
* Maximum age in milliseconds, older entries expire.
* @return
@ -58,11 +79,29 @@ public class TeleportQueue {
public long getMaxAge() {
return maxAge;
}
/**
* The last confirmed teleport location (read-only by primary thread),
* resets with outgoing teleport.
*
* @return
*/
public CountableLocation getLastAck() {
return lastAck;
}
/**
* Get the reference of the last ack, not resetting, but updating with
* incoming acks. This always is the same object, it might only by updated
* on a successful ACK, depending on what's needed (current use is to cancel
* packets on processAck returning WAITING).
*
* @return
*/
public AckReference getLastAckReference() {
return lastAckReference;
}
/**
* Call for Bukkit events (expect this packet to be sent).<br>
* TODO: The method name is misleading, as this also should be called with
@ -78,13 +117,27 @@ public class TeleportQueue {
}
/**
* Call for outgoing teleport packets. Lazily checks for expiration and max queue size.
* @param packetData
* Call for outgoing teleport packets. Lazily checks for expiration and max
* queue size. Update teleportId (Integer.MIN_VALUE means that it isn't
* provided).
*
* @param x
* @param y
* @param z
* @param yaw
* @param pitch
* @param teleportId
* @return
*/
public CountableLocation onOutgoingTeleport(final double x, final double y, final double z, final float yaw, final float pitch) {
public CountableLocation onOutgoingTeleport(final double x, final double y, final double z,
final float yaw, final float pitch, final int teleportId) {
CountableLocation res = null;
final long time = System.currentTimeMillis();
lock.lock();
lastAckReference.lastOutgoingId = teleportId;
if (lastAckReference.maxConfirmedId > lastAckReference.lastOutgoingId) {
lastAckReference.maxConfirmedId = Integer.MIN_VALUE; // Some data loss accepted here.
}
// Only register this location, if it matches the location from a Bukkit event.
if (expectOutgoing != null) {
if (expectOutgoing.isSameLocation(x, y, z, yaw, pitch)) {
@ -109,13 +162,14 @@ public class TeleportQueue {
if (last.isSameLocation(x, y, z, yaw, pitch)) {
last.time = time;
last.count ++;
last.teleportId = teleportId;
res = last;
}
}
}
// Add a new entry, if not merged with last.
if (res == null) {
res = new CountableLocation(x, y, z, yaw, pitch, 1, time);
res = new CountableLocation(x, y, z, yaw, pitch, 1, time, teleportId);
expectIncoming.addLast(res);
// Don't exceed maxQueueSize.
if (expectIncoming.size() > maxQueueSize) {
@ -130,6 +184,67 @@ public class TeleportQueue {
return res;
}
/**
* Process ack on receiving a 'confirm teleport' packet.
*
* @param teleportId
* @return YES, if this teleport id is explicitly matched (not necessarily
* the latest one), MAYBE if it's a unique id smaller than than
* maxConfirmedId. NO, if it's not a valid ack - it could be some
* other special condition, like Integer overflow, or timeout.
*/
public AlmostBoolean processAck(final int teleportId) {
/*
* Return values are subject to change, e.g.: (ACK_LATEST_POS),
* ACK_LATEST_ID, ACK_OUTDATED_POS, ACK_OUTDATED_ID, UNKNOWN
*/
if (teleportId == Integer.MIN_VALUE) {
// Could consider to return MAYBE for not knowing, needs elaborating on context and use cases.
return AlmostBoolean.NO;
}
lock.lock();
if (teleportId == lastAckReference.lastOutgoingId) {
lastAckReference.maxConfirmedId = teleportId;
// Abort here for efficiency.
expectIncoming.clear();
lock.unlock();
return AlmostBoolean.YES;
}
AlmostBoolean ackState = AlmostBoolean.NO;
final Iterator<CountableLocation> it = expectIncoming.iterator();
while (it.hasNext()) {
final CountableLocation ref = it.next();
// No expiration checks here.
if (ref.teleportId == teleportId) {
// Match an outdated id.
// Remove all preceding older entries and this one.
while (ref != expectIncoming.getFirst()) {
expectIncoming.removeFirst();
}
expectIncoming.removeFirst();
// The count doesn't count anymore.
ref.count = 0;
ackState = AlmostBoolean.YES;
break;
}
}
// Update lastAckReference only if within the safe area.
if (teleportId < lastAckReference.lastOutgoingId
&& teleportId > lastAckReference.maxConfirmedId) {
// Allow update.
lastAckReference.maxConfirmedId = teleportId;
if (ackState == AlmostBoolean.NO) {
// Adjust to maybe, as long as the id is increasing within unique range.
ackState = AlmostBoolean.MAYBE;
}
}
else {
lastAckReference.maxConfirmedId = Integer.MIN_VALUE;
}
lock.unlock();
return ackState;
}
/**
* Test if the move is an ACK move (or no ACK is expected), adjust internals
* if necessary.
@ -142,6 +257,7 @@ public class TeleportQueue {
public AckResolution processAck(final DataPacketFlying packetData) {
// Check packet.
if (!packetData.hasPos || !packetData.hasLook) {
// Applies, if we don't have a teleport-confirm event.
return AckResolution.IDLE;
}
// Check queue.
@ -160,7 +276,7 @@ public class TeleportQueue {
/**
* Check queue (lock is handled outside of this method). Does check for
* expiration of entries.
* expiration of entries. Must only be called under lock.
*
* @param packetData
* @return