diff --git a/megamek/src/megamek/server/ServerLobbyHelper.java b/megamek/src/megamek/server/ServerLobbyHelper.java index 8d0f94419a5..e2a6aea2584 100644 --- a/megamek/src/megamek/server/ServerLobbyHelper.java +++ b/megamek/src/megamek/server/ServerLobbyHelper.java @@ -42,6 +42,7 @@ import java.util.Set; import megamek.common.Player; +import megamek.common.annotations.Nullable; import megamek.common.force.Force; import megamek.common.force.Forces; import megamek.common.game.Game; @@ -52,6 +53,7 @@ import megamek.common.options.OptionsConstants; import megamek.common.units.Entity; import megamek.logging.MMLogger; +import megamek.server.UnitOwnershipRules.OwnershipVerdict; import megamek.server.totalWarfare.TWGameManager; public class ServerLobbyHelper { @@ -266,7 +268,32 @@ public static void receiveEntitiesAssign( // Get the local (server) entities var serverEntities = new HashSet(); entityList.stream().map(e -> game.getEntity(e.getId())).forEach(serverEntities::add); + + // The owner arrives in the payload, so it is checked here rather than trusted (issue #8860). Each unit is + // judged on its own: a sender may own some of a mixed selection and not the rest. + Player sender = game.getPlayer(connId); + List permitted = new ArrayList<>(); for (Entity entity : serverEntities) { + if (entity == null) { + continue; + } + OwnershipVerdict verdict = UnitOwnershipRules.verdictFor(sender, newOwner); + if (verdict.isAllowed() && !mayActFor(sender, entity.getOwner())) { + // Handing over someone else's unit is a different question from who may receive it. + verdict = OwnershipVerdict.NOT_PERMITTED; + } + UnitOwnershipRules.logDecision("reassign", verdict, sender, newOwner, entity.getShortNameRaw()); + if (verdict.isAllowed()) { + permitted.add(entity); + } else { + gameManager.sendServerChat( + UnitOwnershipRules.refusalMessage("reassign", sender, newOwner, entity.getShortNameRaw())); + } + } + if (permitted.isEmpty()) { + return; + } + for (Entity entity : permitted) { entity.setOwner(newOwner); } game.getForces().correct(); @@ -279,6 +306,37 @@ public static void receiveEntitiesAssign( * Handles a force assign full packet, changing the owner of forces and everything in them. This method is intended * for use in the lobby! */ + /** + * Whether the sender may act for a unit's current owner: their own unit, one of their bots, or anything at all if + * they hold Gamemaster. Giving a unit away is a separate question from who may receive it, so both are checked. + * + * @param sender the player on the sending connection, or {@code null} + * @param currentOwner the unit's owner before the change, or {@code null} + * + * @return {@code true} if the sender may hand this unit over + */ + private static boolean mayActFor(@Nullable Player sender, @Nullable Player currentOwner) { + return UnitOwnershipRules.verdictFor(sender, currentOwner).isAllowed(); + } + + /** + * Whether the sender may act for every unit in a force. A force moves whole, so one unit they may not give is + * enough to refuse it. + * + * @param sender the player on the sending connection, or {@code null} + * @param entities the units in the force + * + * @return {@code true} if every unit is the sender's to hand over + */ + private static boolean mayActForAll(@Nullable Player sender, Collection entities) { + for (Entity entity : entities) { + if (!mayActFor(sender, entity.getOwner())) { + return false; + } + } + return true; + } + public static void receiveForceAssignFull( Packet packet, int connId, Game game, TWGameManager gameManager ) throws InvalidPacketDataException { @@ -299,8 +357,23 @@ public static void receiveForceAssignFull( serverForces.stream().map(forces::getFullSubForces).forEach(allSubForces::addAll); serverForces.removeIf(allSubForces::contains); + Player forceSender = game.getPlayer(connId); for (Force force : serverForces) { Collection entities = ForceAssignable.filterToEntityList(forces.getFullEntities(force)); + + // Same rule as a unit reassignment (issue #8860). A force moves whole or not at all, so if any unit in + // it is not the sender's to give, the force is refused rather than partly moved. + OwnershipVerdict verdict = UnitOwnershipRules.verdictFor(forceSender, newOwner); + if (verdict.isAllowed() && !mayActForAll(forceSender, entities)) { + verdict = OwnershipVerdict.NOT_PERMITTED; + } + UnitOwnershipRules.logDecision("reassign force", verdict, forceSender, newOwner, force.getName()); + if (!verdict.isAllowed()) { + gameManager.sendServerChat( + UnitOwnershipRules.refusalMessage("reassign force", forceSender, newOwner, force.getName())); + continue; + } + forces.assignFullForces(force, newOwner); for (Entity entity : entities) { entity.setOwner(newOwner); diff --git a/megamek/src/megamek/server/UnitOwnershipRules.java b/megamek/src/megamek/server/UnitOwnershipRules.java new file mode 100644 index 00000000000..3c9e8b67665 --- /dev/null +++ b/megamek/src/megamek/server/UnitOwnershipRules.java @@ -0,0 +1,156 @@ +/* + * Copyright (C) 2026 The MegaMek Team. All Rights Reserved. + * + * This file is part of MegaMek. + * + * MegaMek is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License (GPL), + * version 3 or (at your option) any later version, + * as published by the Free Software Foundation. + * + * MegaMek is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty + * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. + * See the GNU General Public License for more details. + * + * A copy of the GPL should have been included with this project; + * if not, see . + * + * NOTICE: The MegaMek organization is a non-profit group of volunteers + * creating free software for the BattleTech community. + * + * MechWarrior, BattleMech, `Mech and AeroTech are registered trademarks + * of The Topps Company, Inc. All Rights Reserved. + * + * Catalyst Game Labs and the Catalyst Game Labs logo are trademarks of + * InMediaRes Productions, LLC. + * + * MechWarrior Copyright Microsoft Corporation. MegaMek was created under + * Microsoft's "Game Content Usage Rules" + * and it is not endorsed by or + * affiliated with Microsoft. + */ +package megamek.server; + +import megamek.common.Player; +import megamek.common.annotations.Nullable; +import megamek.logging.MMLogger; + +/** + * Who a client may hand units to, and why. + * + *

Three packets change who owns a unit, and all three used to take the new owner straight from the payload: + * adding units, reassigning units, and reassigning a whole force. The client only ever offers legal recipients, but a + * rule enforced on one side only is a rule the other side cannot rely on - two unrelated client changes once combined + * to hand every connecting player's units to the host for several days (issue #8860).

+ * + *

The rule lives here so all three doors enforce the same one, and so every decision is recorded in the same + * shape. Every outcome is logged, not only refusals: a permitted hand-off that leaves no trace cannot be audited + * afterwards or verified in testing.

+ */ +public final class UnitOwnershipRules { + + private static final MMLogger LOGGER = MMLogger.create(UnitOwnershipRules.class); + + private UnitOwnershipRules() {} + + /** + * The reason a hand-off was allowed or refused. Kept as distinct values rather than a boolean so the log says + * which clause applied, which is what makes a permitted path auditable. + */ + public enum OwnershipVerdict { + /** The ordinary case: a player acting on their own units. */ + SENDER_OWNS_THEM(true), + /** A gamemaster is meant to be able to set anybody up. */ + SENDER_IS_GAMEMASTER(true), + /** + * The known weak point, kept deliberately. Since #8808 a unit for your own Princess travels over your + * connection carrying the bot's owner id, and the server holds no record of which human runs which bot, so it + * cannot tell your bot from anyone else's. + */ + RECIPIENT_IS_A_BOT(true), + /** The packet arrived on a connection that belongs to no player. */ + NO_SUCH_SENDER(false), + /** The claimed recipient is not in the game. */ + NO_SUCH_RECIPIENT(false), + /** A player trying to hand units to another human who is not theirs to act for. */ + NOT_PERMITTED(false); + + private final boolean allowed; + + OwnershipVerdict(boolean allowed) { + this.allowed = allowed; + } + + public boolean isAllowed() { + return allowed; + } + } + + /** + * Decides whether the sender may give units to the recipient. + * + * @param sender the player on the sending connection, or {@code null} if that connection has no player + * @param recipient the player the units are claimed for, or {@code null} if no such player is in the game + * + * @return the verdict, carrying which clause applied + */ + public static OwnershipVerdict verdictFor(@Nullable Player sender, @Nullable Player recipient) { + if (sender == null) { + return OwnershipVerdict.NO_SUCH_SENDER; + } + if (recipient == null) { + return OwnershipVerdict.NO_SUCH_RECIPIENT; + } + if (recipient.getId() == sender.getId()) { + return OwnershipVerdict.SENDER_OWNS_THEM; + } + if (sender.isGameMaster()) { + return OwnershipVerdict.SENDER_IS_GAMEMASTER; + } + if (recipient.isBot()) { + return OwnershipVerdict.RECIPIENT_IS_A_BOT; + } + return OwnershipVerdict.NOT_PERMITTED; + } + + /** + * Records one ownership decision. Called for every outcome so the permitted paths leave a trail too. + * + * @param action what was being attempted, for example {@code "add"} or {@code "reassign"} + * @param verdict the decision + * @param sender the player on the sending connection, or {@code null} + * @param recipient the claimed recipient, or {@code null} + * @param subject what was being handed over, for the log line + */ + public static void logDecision(String action, OwnershipVerdict verdict, @Nullable Player sender, + @Nullable Player recipient, String subject) { + String senderName = (sender == null) ? "an unknown connection" : sender.getName(); + String recipientName = (recipient == null) ? "an unknown player" : recipient.getName(); + if (verdict.isAllowed()) { + LOGGER.info("[Ownership] {} by {} to {}: {} ({})", action, senderName, recipientName, subject, verdict); + } else { + LOGGER.warn("[Ownership] REFUSED {} by {} to {}: {} ({})", + action, senderName, recipientName, subject, verdict); + } + } + + /** + * The chat line shown when a hand-off is refused, matching how an illegal design is already reported. + * + * @param action what was being attempted + * @param sender the player on the sending connection, or {@code null} + * @param recipient the claimed recipient, or {@code null} + * @param subject what was being handed over + * + * @return the message to send to all players + */ + public static String refusalMessage(String action, @Nullable Player sender, @Nullable Player recipient, + String subject) { + return String.format("Player %s attempted to %s %s for %s; it was rejected.", + (sender == null) ? "on an unknown connection" : sender.getName(), + action, + subject, + (recipient == null) ? "a player who is not in the game" : recipient.getName()); + } +} diff --git a/megamek/src/megamek/server/totalWarfare/TWGameManager.java b/megamek/src/megamek/server/totalWarfare/TWGameManager.java index bd758fe49cf..b335604406f 100644 --- a/megamek/src/megamek/server/totalWarfare/TWGameManager.java +++ b/megamek/src/megamek/server/totalWarfare/TWGameManager.java @@ -145,6 +145,8 @@ import megamek.common.weapons.infantry.InfantryWeapon; import megamek.logging.MMLogger; import megamek.server.*; +import megamek.server.UnitOwnershipRules; +import megamek.server.UnitOwnershipRules.OwnershipVerdict; import megamek.server.commands.*; import megamek.server.props.OrbitalBombardment; import megamek.server.victory.VictoryResult; @@ -26345,6 +26347,34 @@ void updateVisibilityIndicator(Map losCache) { } } + /** + * Whether the client on this connection may add a unit owned by the unit's stated owner. + * + *

The owner travels in the payload and used to be taken on trust. The client offers only legal recipients, + * but a rule enforced on one side only is a rule the other side cannot rely on: two unrelated client changes + * once combined to hand every connecting player's units to the host for several days (issue #8860).

+ * + *

Three cases are allowed. Adding to yourself, which is the ordinary one. Adding as a gamemaster, who is + * meant to be able to set up anybody. And adding to a bot, because since #8808 a unit for your own Princess + * travels over your connection carrying the bot's owner id, and the server holds no record of which human runs + * which bot. That last case is the known weak point, so it is logged rather than passed over in silence.

+ * + * @param entity the unit being added, carrying the owner the client claims for it + * @param connIndex the connection the packet arrived on + * + * @return {@code true} if the unit may be added + */ + private boolean mayAddUnitFor(Entity entity, int connIndex) { + Player sender = game.getPlayer(connIndex); + Player owner = game.getPlayer(entity.getOwnerId()); + OwnershipVerdict verdict = UnitOwnershipRules.verdictFor(sender, owner); + UnitOwnershipRules.logDecision("add", verdict, sender, owner, entity.getShortNameRaw()); + if (!verdict.isAllowed()) { + sendServerChat(UnitOwnershipRules.refusalMessage("add", sender, owner, entity.getShortNameRaw())); + } + return verdict.isAllowed(); + } + /** * Checks if an entity added by the client is valid and if so, adds it to the list * @@ -26363,6 +26393,11 @@ private void receiveEntityAdd(Packet packet, int connIndex) throws InvalidPacket // when removing // illegal entities for (final Entity entity : new ArrayList<>(entities)) { + if (!mayAddUnitFor(entity, connIndex)) { + entities.remove(entity); + continue; + } + // Create a TestEntity instance for supported unit types TestEntity testEntity = TestEntity.getEntityVerifier(entity); entity.restore(); diff --git a/megamek/unittests/megamek/server/totalWarfare/EntityAddOwnershipTest.java b/megamek/unittests/megamek/server/totalWarfare/EntityAddOwnershipTest.java new file mode 100644 index 00000000000..84007c5118c --- /dev/null +++ b/megamek/unittests/megamek/server/totalWarfare/EntityAddOwnershipTest.java @@ -0,0 +1,218 @@ +/* + * Copyright (C) 2026 The MegaMek Team. All Rights Reserved. + * + * This file is part of MegaMek. + * + * MegaMek is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License (GPL), + * version 3 or (at your option) any later version, + * as published by the Free Software Foundation. + * + * MegaMek is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty + * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. + * See the GNU General Public License for more details. + * + * A copy of the GPL should have been included with this project; + * if not, see . + * + * NOTICE: The MegaMek organization is a non-profit group of volunteers + * creating free software for the BattleTech community. + * + * MechWarrior, BattleMech, `Mech and AeroTech are registered trademarks + * of The Topps Company, Inc. All Rights Reserved. + * + * Catalyst Game Labs and the Catalyst Game Labs logo are trademarks of + * InMediaRes Productions, LLC. + * + * MechWarrior Copyright Microsoft Corporation. MegaMek was created under + * Microsoft's "Game Content Usage Rules" + * and it is not endorsed by or + * affiliated with Microsoft. + */ +package megamek.server.totalWarfare; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.fail; + +import java.io.File; +import java.util.List; + +import megamek.common.Player; +import megamek.common.equipment.EquipmentType; +import megamek.common.game.Game; +import megamek.common.loaders.MekFileParser; +import megamek.common.net.enums.PacketCommand; +import megamek.common.net.packets.Packet; +import megamek.common.units.Crew; +import megamek.common.units.CrewType; +import megamek.common.units.Entity; +import megamek.server.Server; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +/** + * Covers who the server lets a client add units for (GitHub issue #8860). The owner travels in the ENTITY_ADD payload + * and used to be taken on trust, so a client could hand its units to anybody. Drives the real packet path. + */ +class EntityAddOwnershipTest { + + private static final int SENDER_CONNECTION = 0; + private static final int OTHER_HUMAN_CONNECTION = 1; + private static final int BOT_CONNECTION = 2; + private static final String TEST_UNIT = "Atlas AS7-C"; + + private Game game; + private TWGameManager gameManager; + private Server server; + + @BeforeAll + static void initializeEquipment() { + EquipmentType.initializeTypes(); + } + + @BeforeEach + void setUp() throws Exception { + game = new Game(); + game.addPlayer(SENDER_CONNECTION, playerNamed(SENDER_CONNECTION, "Sender")); + game.addPlayer(OTHER_HUMAN_CONNECTION, playerNamed(OTHER_HUMAN_CONNECTION, "Someone Else")); + Player bot = playerNamed(BOT_CONNECTION, "Princess"); + bot.setBot(true); + game.addPlayer(BOT_CONNECTION, bot); + + gameManager = new TWGameManager(); + gameManager.setGame(game); + // Port 0 binds an ephemeral port; the server is only needed so that packet sends have a target + server = new Server(null, 0, gameManager); + } + + @AfterEach + void tearDown() { + if (server != null) { + server.die(); + } + } + + private static Player playerNamed(int id, String name) { + Player player = new Player(id, name); + player.setTeam(1); + return player; + } + + /** Sends one unit owned by the given player over the given connection, as the lobby does. */ + private void sendUnitFor(int ownerConnection, int overConnection) { + Entity entity; + try { + entity = new MekFileParser(new File("testresources/data/mekfiles/" + TEST_UNIT + ".mtf")).getEntity(); + } catch (Exception ex) { + fail("Failed to load " + TEST_UNIT + ": " + ex.getMessage()); + return; + } + entity.setGame(game); + entity.setCrew(new Crew(CrewType.SINGLE)); + entity.setOwner(game.getPlayer(ownerConnection)); + gameManager.handlePacket(overConnection, new Packet(PacketCommand.ENTITY_ADD, List.of(entity))); + } + + @Test + void aPlayerMayAddUnitsForThemselves() { + sendUnitFor(SENDER_CONNECTION, SENDER_CONNECTION); + assertEquals(1, game.getEntitiesVector().size(), "the ordinary case must still work"); + } + + @Test + void aPlayerMayNotAddUnitsForAnotherHuman() { + sendUnitFor(OTHER_HUMAN_CONNECTION, SENDER_CONNECTION); + assertEquals(0, game.getEntitiesVector().size(), "handing your units to another player must be refused"); + } + + @Test + void aGameMasterMayAddUnitsForAnyone() { + game.getPlayer(SENDER_CONNECTION).setGameMaster(true); + sendUnitFor(OTHER_HUMAN_CONNECTION, SENDER_CONNECTION); + assertEquals(1, game.getEntitiesVector().size(), "a gamemaster is meant to be able to set anybody up"); + } + + @Test + void anyoneMayAddUnitsForABot() { + // The known hole, kept deliberately: a unit for your own Princess arrives over your connection carrying the + // bot's owner id, and the server holds no record of which human runs which bot. + sendUnitFor(BOT_CONNECTION, SENDER_CONNECTION); + assertEquals(1, game.getEntitiesVector().size(), "bots must still be stockable by the client running them"); + } + + @Test + void aUnitOwnedByNobodyIsRefused() { + Entity entity; + try { + entity = new MekFileParser(new File("testresources/data/mekfiles/" + TEST_UNIT + ".mtf")).getEntity(); + } catch (Exception ex) { + fail("Failed to load " + TEST_UNIT + ": " + ex.getMessage()); + return; + } + entity.setGame(game); + entity.setCrew(new Crew(CrewType.SINGLE)); + // A player object the server never saw, which is what an id for a departed or invented player looks like. + entity.setOwner(playerNamed(99, "Ghost")); + gameManager.handlePacket(SENDER_CONNECTION, new Packet(PacketCommand.ENTITY_ADD, List.of(entity))); + assertEquals(0, game.getEntitiesVector().size(), "an owner who is not in the game must be refused"); + } + + /** Reassigning an existing unit, which is how a player actually gives one to their bot. */ + private void reassign(int unitOwnerConnection, int newOwnerConnection, int overConnection) { + Entity entity = addedUnitFor(unitOwnerConnection); + gameManager.handlePacket(overConnection, + new Packet(PacketCommand.ENTITY_ASSIGN, List.of(entity), newOwnerConnection)); + } + + /** Puts one unit straight into the game owned by the given player, bypassing the add packet. */ + private Entity addedUnitFor(int ownerConnection) { + Entity entity; + try { + entity = new MekFileParser(new File("testresources/data/mekfiles/" + TEST_UNIT + ".mtf")).getEntity(); + } catch (Exception ex) { + fail("Failed to load " + TEST_UNIT + ": " + ex.getMessage()); + return null; + } + entity.setGame(game); + entity.setCrew(new Crew(CrewType.SINGLE)); + entity.setOwner(game.getPlayer(ownerConnection)); + entity.setId(game.getNextEntityId()); + game.addEntity(entity); + return entity; + } + + private int ownerOfTheOnlyUnit() { + return game.getEntitiesVector().get(0).getOwnerId(); + } + + @Test + void aPlayerMayGiveTheirOwnUnitToTheirBot() { + // The ordinary way to stock a Princess, and what the add-packet guard never sees. + reassign(SENDER_CONNECTION, BOT_CONNECTION, SENDER_CONNECTION); + assertEquals(BOT_CONNECTION, ownerOfTheOnlyUnit(), "giving your own unit to a bot must keep working"); + } + + @Test + void aPlayerMayNotGiveAwayAUnitTheyDoNotOwn() { + reassign(OTHER_HUMAN_CONNECTION, BOT_CONNECTION, SENDER_CONNECTION); + assertEquals(OTHER_HUMAN_CONNECTION, ownerOfTheOnlyUnit(), + "handing away another player's unit must be refused"); + } + + @Test + void aPlayerMayNotPushTheirUnitOntoAnotherHuman() { + reassign(SENDER_CONNECTION, OTHER_HUMAN_CONNECTION, SENDER_CONNECTION); + assertEquals(SENDER_CONNECTION, ownerOfTheOnlyUnit(), + "pushing your unit onto another player must be refused"); + } + + @Test + void aGameMasterMayReassignBetweenOtherPlayers() { + game.getPlayer(SENDER_CONNECTION).setGameMaster(true); + reassign(OTHER_HUMAN_CONNECTION, BOT_CONNECTION, SENDER_CONNECTION); + assertEquals(BOT_CONNECTION, ownerOfTheOnlyUnit(), "a gamemaster may move anybody's units"); + } +}