From bb240d3aa0f7c177126d7d590dda27431a47d6ff Mon Sep 17 00:00:00 2001 From: games647 Date: Mon, 28 Aug 2017 12:17:47 +0200 Subject: [PATCH] Refactor encryption implementation * Simplify utility class and make it more independent from the vendor code * Create only one cipher object for verification --- .../fastlogin/bukkit/EncryptionUtil.java | 152 +++++++++--------- .../protocollib/ProtocolLibListener.java | 12 +- .../protocollib/ProtocolLibLoginSource.java | 20 +-- .../protocollib/VerifyResponseTask.java | 71 ++++---- .../fastlogin/bukkit/EncryptionUtilTest.java | 40 +++++ pom.xml | 2 +- 6 files changed, 178 insertions(+), 119 deletions(-) create mode 100644 bukkit/src/test/java/com/github/games647/fastlogin/bukkit/EncryptionUtilTest.java diff --git a/bukkit/src/main/java/com/github/games647/fastlogin/bukkit/EncryptionUtil.java b/bukkit/src/main/java/com/github/games647/fastlogin/bukkit/EncryptionUtil.java index 06df00fa..bd3e4604 100644 --- a/bukkit/src/main/java/com/github/games647/fastlogin/bukkit/EncryptionUtil.java +++ b/bukkit/src/main/java/com/github/games647/fastlogin/bukkit/EncryptionUtil.java @@ -2,18 +2,18 @@ package com.github.games647.fastlogin.bukkit; import com.google.common.base.Charsets; -import java.security.InvalidKeyException; +import java.math.BigInteger; +import java.security.GeneralSecurityException; import java.security.Key; import java.security.KeyPair; import java.security.KeyPairGenerator; import java.security.MessageDigest; import java.security.NoSuchAlgorithmException; -import java.util.stream.Stream; +import java.security.PrivateKey; +import java.security.PublicKey; +import java.util.Random; -import javax.crypto.BadPaddingException; import javax.crypto.Cipher; -import javax.crypto.IllegalBlockSizeException; -import javax.crypto.NoSuchPaddingException; import javax.crypto.SecretKey; import javax.crypto.spec.SecretKeySpec; @@ -21,15 +21,21 @@ import javax.crypto.spec.SecretKeySpec; * Encryption and decryption minecraft util for connection between servers * and paid minecraft account clients. * - * Source: https://github.com/bergerkiller/CraftSource/blob/master/net.minecraft.server/MinecraftEncryption.java - * - * Remapped by: https://github.com/Techcable/MinecraftMappings/tree/master/1.8 + * @see net.minecraft.server.MinecraftEncryption */ public class EncryptionUtil { + public static final int VERIFY_TOKEN_LENGTH = 4; + public static final String KEY_PAIR_ALGORITHM = "RSA"; + + /** + * Generate a RSA key pair + * + * @return The RSA key pair. + */ public static KeyPair generateKeyPair() { try { - KeyPairGenerator keyPairGenerator = KeyPairGenerator.getInstance("RSA"); + KeyPairGenerator keyPairGenerator = KeyPairGenerator.getInstance(KEY_PAIR_ALGORITHM); keyPairGenerator.initialize(1_024); return keyPairGenerator.generateKeyPair(); @@ -39,80 +45,76 @@ public class EncryptionUtil { } } - public static byte[] getServerIdHash(String serverId, Key publicKey, Key secretKey) { - return digestOperation("SHA-1" - , serverId.getBytes(Charsets.ISO_8859_1), secretKey.getEncoded(), publicKey.getEncoded()); + /** + * Generate a random token. This is used to verify that we are communicating with the same player + * in a login session. + * + * @param random random generator + * @return an error with 4 bytes long + */ + public static byte[] generateVerifyToken(Random random) { + byte[] token = new byte[VERIFY_TOKEN_LENGTH]; + random.nextBytes(token); + return token; } - private static byte[] digestOperation(String algorithm, byte[]... content) { + /** + * Generate the server id based on client and server data. + * + * @param sessionId session for the current login attempt + * @param sharedSecret shared secret between the client and the server + * @param publicKey public key of the server + * @return the server id formatted as a hexadecimal string. + */ + public static String getServerIdHashString(String sessionId, Key sharedSecret, PublicKey publicKey) { try { - MessageDigest messagedigest = MessageDigest.getInstance(algorithm); - Stream.of(content).forEach(messagedigest::update); - - return messagedigest.digest(); - } catch (NoSuchAlgorithmException nosuchalgorithmexception) { - nosuchalgorithmexception.printStackTrace(); - return null; - } - } - -// public static PublicKey decodePublicKey(byte[] encodedKey) { -// try { -// KeyFactory keyfactory = KeyFactory.getInstance("RSA"); -// -// X509EncodedKeySpec x509encodedkeyspec = new X509EncodedKeySpec(encodedKey); -// return keyfactory.generatePublic(x509encodedkeyspec); -// } catch (NoSuchAlgorithmException | InvalidKeySpecException nosuchalgorithmexception) { -// //ignore -// } -// -// System.err.println("Public key reconstitute failed!"); -// return null; -// } - - public static SecretKey decryptSharedKey(Key privateKey, byte[] encryptedSharedKey) { - return new SecretKeySpec(decryptData(privateKey, encryptedSharedKey), "AES"); - } - - public static byte[] decryptData(Key key, byte[] data) { - return cipherOperation(Cipher.DECRYPT_MODE, key, data); - } - - private static byte[] cipherOperation(int operationMode, Key key, byte[] data) { - try { - return createCipherInstance(operationMode, key.getAlgorithm(), key).doFinal(data); - } catch (IllegalBlockSizeException | BadPaddingException ex) { - ex.printStackTrace(); + byte[] serverHash = getServerIdHash(sessionId, sharedSecret, publicKey); + return (new BigInteger(serverHash)).toString(16); + } catch (NoSuchAlgorithmException e) { + e.printStackTrace(); } - System.err.println("Cipher data failed!"); - return null; + return ""; } - private static Cipher createCipherInstance(int operationMode, String cipherName, Key key) { - try { - Cipher cipher = Cipher.getInstance(cipherName); - - cipher.init(operationMode, key); - return cipher; - } catch (InvalidKeyException | NoSuchAlgorithmException | NoSuchPaddingException ex) { - ex.printStackTrace(); - } - - System.err.println("Cipher creation failed!"); - return null; + /** + * Decrypts the content and extracts the key spec. + * + * @param cipher decryption cipher + * @param privateKey private key of the server + * @param sharedKey the encrypted shared key + * @return + * @throws GeneralSecurityException + */ + public static SecretKey decryptSharedKey(Cipher cipher, PrivateKey privateKey, byte[] sharedKey) + throws GeneralSecurityException { + return new SecretKeySpec(decrypt(cipher, privateKey, sharedKey), "AES"); + } + + /** + * Decrypted the given data using the cipher. + * + * @param cipher decryption cypher + * @param key server private key + * @param data the encrypted data + * @return clear text data + * @throws GeneralSecurityException if it fails to initialize and decrypt the data + */ + public static byte[] decrypt(Cipher cipher, PrivateKey key, byte[] data) throws GeneralSecurityException { + cipher.init(Cipher.DECRYPT_MODE, key); + return cipher.doFinal(data); + } + + private static byte[] getServerIdHash(String sessionId, Key sharedSecret, PublicKey publicKey) + throws NoSuchAlgorithmException { + MessageDigest digest = MessageDigest.getInstance("SHA-1"); + + digest.update(sessionId.getBytes(Charsets.ISO_8859_1)); + digest.update(sharedSecret.getEncoded()); + digest.update(publicKey.getEncoded()); + + return digest.digest(); } -// -// public static Cipher createBufferedBlockCipher(int operationMode, Key key) { -// try { -// Cipher cipher = Cipher.getInstance("AES/CFB8/NoPadding"); -// -// cipher.init(operationMode, key, new IvParameterSpec(key.getEncoded())); -// return cipher; -// } catch (GeneralSecurityException generalsecurityexception) { -// throw new RuntimeException(generalsecurityexception); -// } -// } private EncryptionUtil() { //utility diff --git a/bukkit/src/main/java/com/github/games647/fastlogin/bukkit/listener/protocollib/ProtocolLibListener.java b/bukkit/src/main/java/com/github/games647/fastlogin/bukkit/listener/protocollib/ProtocolLibListener.java index 8ad93d2d..059fa260 100644 --- a/bukkit/src/main/java/com/github/games647/fastlogin/bukkit/listener/protocollib/ProtocolLibListener.java +++ b/bukkit/src/main/java/com/github/games647/fastlogin/bukkit/listener/protocollib/ProtocolLibListener.java @@ -1,31 +1,33 @@ package com.github.games647.fastlogin.bukkit.listener.protocollib; import com.comphenix.protocol.PacketType; -import com.comphenix.protocol.PacketType.Login.Client; import com.comphenix.protocol.ProtocolLibrary; import com.comphenix.protocol.events.PacketAdapter; import com.comphenix.protocol.events.PacketContainer; import com.comphenix.protocol.events.PacketEvent; import com.github.games647.fastlogin.bukkit.FastLoginBukkit; -import java.util.Random; +import java.security.SecureRandom; import java.util.logging.Level; import org.bukkit.Bukkit; import org.bukkit.entity.Player; +import static com.comphenix.protocol.PacketType.Login.Client.ENCRYPTION_BEGIN; +import static com.comphenix.protocol.PacketType.Login.Client.START; + public class ProtocolLibListener extends PacketAdapter { private static final int WORKER_THREADS = 3; private final FastLoginBukkit plugin; //just create a new once on plugin enable. This used for verify token generation - private final Random random = new Random(); + private final SecureRandom random = new SecureRandom(); public ProtocolLibListener(FastLoginBukkit plugin) { //run async in order to not block the server, because we are making api calls to Mojang super(params().plugin(plugin) - .types(PacketType.Login.Client.START, PacketType.Login.Client.ENCRYPTION_BEGIN) + .types(START, ENCRYPTION_BEGIN) .optionAsync()); this.plugin = plugin; @@ -48,7 +50,7 @@ public class ProtocolLibListener extends PacketAdapter { Player sender = packetEvent.getPlayer(); PacketType packetType = packetEvent.getPacketType(); - if (packetType == Client.START) { + if (packetType == START) { onLogin(packetEvent, sender); } else { onEncryptionBegin(packetEvent, sender); diff --git a/bukkit/src/main/java/com/github/games647/fastlogin/bukkit/listener/protocollib/ProtocolLibLoginSource.java b/bukkit/src/main/java/com/github/games647/fastlogin/bukkit/listener/protocollib/ProtocolLibLoginSource.java index 911cfbb2..8483b470 100644 --- a/bukkit/src/main/java/com/github/games647/fastlogin/bukkit/listener/protocollib/ProtocolLibLoginSource.java +++ b/bukkit/src/main/java/com/github/games647/fastlogin/bukkit/listener/protocollib/ProtocolLibLoginSource.java @@ -1,11 +1,11 @@ package com.github.games647.fastlogin.bukkit.listener.protocollib; -import com.comphenix.protocol.PacketType; import com.comphenix.protocol.ProtocolLibrary; import com.comphenix.protocol.ProtocolManager; import com.comphenix.protocol.events.PacketContainer; import com.comphenix.protocol.events.PacketEvent; import com.comphenix.protocol.wrappers.WrappedChatComponent; +import com.github.games647.fastlogin.bukkit.EncryptionUtil; import com.github.games647.fastlogin.bukkit.FastLoginBukkit; import com.github.games647.fastlogin.core.shared.LoginSource; @@ -16,9 +16,10 @@ import java.util.Random; import org.bukkit.entity.Player; -public class ProtocolLibLoginSource implements LoginSource { +import static com.comphenix.protocol.PacketType.Login.Server.DISCONNECT; +import static com.comphenix.protocol.PacketType.Login.Server.ENCRYPTION_BEGIN; - private static final int VERIFY_TOKEN_LENGTH = 4; +public class ProtocolLibLoginSource implements LoginSource { private final FastLoginBukkit plugin; @@ -28,7 +29,7 @@ public class ProtocolLibLoginSource implements LoginSource { private final Random random; private String serverId; - private final byte[] verifyToken = new byte[VERIFY_TOKEN_LENGTH]; + private byte[] verifyToken; public ProtocolLibLoginSource(FastLoginBukkit plugin, PacketEvent packetEvent, Player player, Random random) { this.plugin = plugin; @@ -42,9 +43,7 @@ public class ProtocolLibLoginSource implements LoginSource { //randomized server id to make sure the request is for our server //this could be relevant http://www.sk89q.com/2011/09/minecraft-name-spoofing-exploit/ serverId = Long.toString(random.nextLong(), 16); - - //generate a random token which should be the same when we receive it from the client - random.nextBytes(verifyToken); + verifyToken = EncryptionUtil.generateVerifyToken(random); sentEncryptionRequest(); } @@ -53,7 +52,7 @@ public class ProtocolLibLoginSource implements LoginSource { public void kick(String message) throws Exception { ProtocolManager protocolManager = ProtocolLibrary.getProtocolManager(); - PacketContainer kickPacket = protocolManager.createPacket(PacketType.Login.Server.DISCONNECT); + PacketContainer kickPacket = protocolManager.createPacket(DISCONNECT); kickPacket.getChatComponents().write(0, WrappedChatComponent.fromText(message)); try { @@ -78,10 +77,11 @@ public class ProtocolLibLoginSource implements LoginSource { * * ServerID="" (String) key=public server key verifyToken=random 4 byte array */ - PacketContainer newPacket = protocolManager.createPacket(PacketType.Login.Server.ENCRYPTION_BEGIN); + PacketContainer newPacket = protocolManager.createPacket(ENCRYPTION_BEGIN); newPacket.getStrings().write(0, serverId); - newPacket.getSpecificModifier(PublicKey.class).write(0, plugin.getServerKey().getPublic()); + PublicKey publicKey = plugin.getServerKey().getPublic(); + newPacket.getSpecificModifier(PublicKey.class).write(0, publicKey); newPacket.getByteArrays().write(0, verifyToken); diff --git a/bukkit/src/main/java/com/github/games647/fastlogin/bukkit/listener/protocollib/VerifyResponseTask.java b/bukkit/src/main/java/com/github/games647/fastlogin/bukkit/listener/protocollib/VerifyResponseTask.java index 737533e0..a584e6aa 100644 --- a/bukkit/src/main/java/com/github/games647/fastlogin/bukkit/listener/protocollib/VerifyResponseTask.java +++ b/bukkit/src/main/java/com/github/games647/fastlogin/bukkit/listener/protocollib/VerifyResponseTask.java @@ -1,6 +1,5 @@ package com.github.games647.fastlogin.bukkit.listener.protocollib; -import com.comphenix.protocol.PacketType; import com.comphenix.protocol.ProtocolLibrary; import com.comphenix.protocol.ProtocolManager; import com.comphenix.protocol.events.PacketContainer; @@ -16,40 +15,44 @@ import com.github.games647.fastlogin.bukkit.FastLoginBukkit; import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; -import java.math.BigInteger; -import java.security.Key; +import java.security.GeneralSecurityException; import java.security.PrivateKey; +import java.security.PublicKey; import java.util.Arrays; import java.util.UUID; import java.util.logging.Level; +import javax.crypto.Cipher; import javax.crypto.SecretKey; import org.bukkit.entity.Player; +import static com.comphenix.protocol.PacketType.Login.Client.START; +import static com.comphenix.protocol.PacketType.Login.Server.DISCONNECT; + public class VerifyResponseTask implements Runnable { private final FastLoginBukkit plugin; private final PacketEvent packetEvent; - private final Player fromPlayer; + private final Player player; private final byte[] sharedSecret; - public VerifyResponseTask(FastLoginBukkit plugin, PacketEvent packetEvent, Player fromPlayer, byte[] sharedSecret) { + public VerifyResponseTask(FastLoginBukkit plugin, PacketEvent packetEvent, Player player, byte[] sharedSecret) { this.plugin = plugin; this.packetEvent = packetEvent; - this.fromPlayer = fromPlayer; + this.player = player; this.sharedSecret = sharedSecret; } @Override public void run() { try { - BukkitLoginSession session = plugin.getLoginSessions().get(fromPlayer.getAddress().toString()); + BukkitLoginSession session = plugin.getLoginSessions().get(player.getAddress().toString()); if (session == null) { disconnect(plugin.getCore().getMessage("invalid-request"), true - , "Player {0} tried to send encryption response at invalid state", fromPlayer.getAddress()); + , "Player {0} tried to send encryption response at invalid state", player.getAddress()); } else { verifyResponse(session); } @@ -64,24 +67,36 @@ public class VerifyResponseTask implements Runnable { } private void verifyResponse(BukkitLoginSession session) { + PublicKey publicKey = plugin.getServerKey().getPublic(); PrivateKey privateKey = plugin.getServerKey().getPrivate(); - SecretKey loginKey = EncryptionUtil.decryptSharedKey(privateKey, sharedSecret); - if (!checkVerifyToken(session, privateKey) || !encryptConnection(loginKey)) { + Cipher cipher; + SecretKey loginKey; + try { + cipher = Cipher.getInstance(privateKey.getAlgorithm()); + + loginKey = EncryptionUtil.decryptSharedKey(cipher, privateKey, sharedSecret); + } catch (GeneralSecurityException securityEx) { + disconnect("error-kick", false, "Cannot decrypt received contents", securityEx); + return; + } + + try { + if (!checkVerifyToken(session, cipher, privateKey) || !encryptConnection(loginKey)) { + return; + } + } catch (Exception ex) { + disconnect("error-kick", false, "Cannot decrypt received contents", ex); return; } //this makes sure the request from the client is for us //this might be relevant http://www.sk89q.com/2011/09/minecraft-name-spoofing-exploit/ String generatedId = session.getServerId(); - - //https://github.com/bergerkiller/CraftSource/blob/master/net.minecraft.server/LoginListener.java#L193 - //generate the server id based on client and server data - byte[] serverIdHash = EncryptionUtil.getServerIdHash(generatedId, plugin.getServerKey().getPublic(), loginKey); - String serverId = (new BigInteger(serverIdHash)).toString(16); + String serverId = EncryptionUtil.getServerIdHashString(generatedId, loginKey, publicKey); String username = session.getUsername(); - if (plugin.getCore().getApiConnector().hasJoinedServer(session, serverId, fromPlayer.getAddress())) { + if (plugin.getCore().getApiConnector().hasJoinedServer(session, serverId, player.getAddress())) { plugin.getLogger().log(Level.INFO, "Player {0} has a verified premium account", username); session.setVerified(true); @@ -91,7 +106,7 @@ public class VerifyResponseTask implements Runnable { //user tried to fake a authentication disconnect(plugin.getCore().getMessage("invalid-session"), true , "Player {0} ({1}) tried to log in with an invalid session ServerId: {2}" - , session.getUsername(), fromPlayer.getAddress(), serverId); + , session.getUsername(), player.getAddress(), serverId); } } @@ -107,13 +122,14 @@ public class VerifyResponseTask implements Runnable { } } - private boolean checkVerifyToken(BukkitLoginSession session, Key privateKey) { + private boolean checkVerifyToken(BukkitLoginSession session, Cipher cipher, PrivateKey privateKey) + throws GeneralSecurityException { byte[] requestVerify = session.getVerifyToken(); //encrypted verify token byte[] responseVerify = packetEvent.getPacket().getByteArrays().read(1); //https://github.com/bergerkiller/CraftSource/blob/master/net.minecraft.server/LoginListener.java#L182 - if (!Arrays.equals(requestVerify, EncryptionUtil.decryptData(privateKey, responseVerify))) { + if (!Arrays.equals(requestVerify, EncryptionUtil.decrypt(cipher, privateKey, responseVerify))) { //check if the verify token are equal to the server sent one disconnect(plugin.getCore().getMessage("invalid-verify-token"), true , "Player {0} ({1}) tried to login with an invalid verify token. Server: {2} Client: {3}" @@ -126,7 +142,7 @@ public class VerifyResponseTask implements Runnable { //try to get the networkManager from ProtocolLib private Object getNetworkManager() throws IllegalAccessException, NoSuchFieldException, ClassNotFoundException { - Object injectorContainer = TemporaryPlayerFactory.getInjectorFromPlayer(fromPlayer); + Object injectorContainer = TemporaryPlayerFactory.getInjectorFromPlayer(player); //ChannelInjector Class injectorClass = Class.forName("com.comphenix.protocol.injector.netty.Injector"); @@ -147,8 +163,7 @@ public class VerifyResponseTask implements Runnable { //the client expects this behaviour encryptMethod.invoke(networkManager, loginKey); } catch (Exception ex) { - plugin.getLogger().log(Level.SEVERE, "Couldn't enable encryption", ex); - disconnect(plugin.getCore().getMessage("error-kick"), false, "Couldn't enable encryption"); + disconnect("error-kick", false, "Couldn't enable encryption", ex); return false; } @@ -162,13 +177,13 @@ public class VerifyResponseTask implements Runnable { plugin.getLogger().log(Level.SEVERE, logMessage, arguments); } - kickPlayer(packetEvent.getPlayer(), kickReason); + kickPlayer(plugin.getCore().getMessage(kickReason)); } - private void kickPlayer(Player player, String reason) { + private void kickPlayer(String reason) { ProtocolManager protocolManager = ProtocolLibrary.getProtocolManager(); - PacketContainer kickPacket = protocolManager.createPacket(PacketType.Login.Server.DISCONNECT); + PacketContainer kickPacket = protocolManager.createPacket(DISCONNECT); kickPacket.getChatComponents().write(0, WrappedChatComponent.fromText(reason)); try { //send kick packet at login state @@ -186,18 +201,18 @@ public class VerifyResponseTask implements Runnable { ProtocolManager protocolManager = ProtocolLibrary.getProtocolManager(); //see StartPacketListener for packet information - PacketContainer startPacket = protocolManager.createPacket(PacketType.Login.Client.START); + PacketContainer startPacket = protocolManager.createPacket(START); //uuid is ignored by the packet definition WrappedGameProfile fakeProfile = new WrappedGameProfile(UUID.randomUUID(), username); startPacket.getGameProfiles().write(0, fakeProfile); try { //we don't want to handle our own packets so ignore filters - protocolManager.recieveClientPacket(fromPlayer, startPacket, false); + protocolManager.recieveClientPacket(player, startPacket, false); } catch (InvocationTargetException | IllegalAccessException ex) { plugin.getLogger().log(Level.WARNING, "Failed to fake a new start packet", ex); //cancel the event in order to prevent the server receiving an invalid packet - kickPlayer(fromPlayer, plugin.getCore().getMessage("error-kick")); + kickPlayer(plugin.getCore().getMessage("error-kick")); } } } diff --git a/bukkit/src/test/java/com/github/games647/fastlogin/bukkit/EncryptionUtilTest.java b/bukkit/src/test/java/com/github/games647/fastlogin/bukkit/EncryptionUtilTest.java new file mode 100644 index 00000000..543477cf --- /dev/null +++ b/bukkit/src/test/java/com/github/games647/fastlogin/bukkit/EncryptionUtilTest.java @@ -0,0 +1,40 @@ +package com.github.games647.fastlogin.bukkit; + +import java.security.SecureRandom; + +import org.junit.Test; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; + +public class EncryptionUtilTest { + + @Test + public void testVerifyToken() throws Exception { + SecureRandom random = new SecureRandom(); + byte[] token = EncryptionUtil.generateVerifyToken(random); + + assertNotNull(token); + assertEquals(token.length, 4); + } + + // @Test + // public void testDecryptSharedSecret() throws Exception { + // + // } + // + // @Test + // public void testDecryptData() throws Exception { + // + // } + + // private static SecretKey createNewSharedKey() { + // try { + // KeyGenerator keygenerator = KeyGenerator.getInstance("AES"); + // keygenerator.init(128); + // return keygenerator.generateKey(); + // } catch (NoSuchAlgorithmException nosuchalgorithmexception) { + // throw new Error(nosuchalgorithmexception); + // } + // } +} diff --git a/pom.xml b/pom.xml index 5fcca273..0078ca0b 100644 --- a/pom.xml +++ b/pom.xml @@ -8,7 +8,7 @@ pom FastLogin - 1.10 + 1.11 2015 https://www.spigotmc.org/resources/fastlogin.14153/