Refactor encryption implementation

* Simplify utility class and make it more independent from the vendor code
* Create only one cipher object for verification

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
+ * @see net.minecraft.server.MinecraftEncryption
- *
- * Remapped by: https://github.com/Techcable/MinecraftMappings/tree/master/1.8
  */
 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"
+     * Generate a random token. This is used to verify that we are communicating with the same player
-                , serverId.getBytes(Charsets.ISO_8859_1), secretKey.getEncoded(), publicKey.getEncoded());
+     * 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);
+            byte[] serverHash = getServerIdHash(sessionId, sharedSecret, publicKey);
-            Stream.of(content).forEach(messagedigest::update);
+            return (new BigInteger(serverHash)).toString(16);
-
+        } catch (NoSuchAlgorithmException e) {
-            return messagedigest.digest();
+            e.printStackTrace();
-        } 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();
         }
 
-        System.err.println("Cipher data failed!");
+        return "";
-        return null;
     }
 
-    private static Cipher createCipherInstance(int operationMode, String cipherName, Key key) {
+    /**
-        try {
+     * Decrypts the content and extracts the key spec.
-            Cipher cipher = Cipher.getInstance(cipherName);
+     *
-
+     * @param cipher decryption cipher
-            cipher.init(operationMode, key);
+     * @param privateKey private key of the server
-            return cipher;
+     * @param sharedKey the encrypted shared key
-        } catch (InvalidKeyException | NoSuchAlgorithmException | NoSuchPaddingException ex) {
+     * @return
-            ex.printStackTrace();
+     * @throws GeneralSecurityException
-        }
+     */
-
+    public static SecretKey decryptSharedKey(Cipher cipher, PrivateKey privateKey, byte[] sharedKey)
-        System.err.println("Cipher creation failed!");
+            throws GeneralSecurityException {
-        return null;
+        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

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);

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);
-
+        verifyToken = EncryptionUtil.generateVerifyToken(random);
-        //generate a random token which should be the same when we receive it from the client
-        random.nextBytes(verifyToken);
         
         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);
 

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.GeneralSecurityException;
-import java.security.Key;
 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);
+        Cipher cipher;
-        if (!checkVerifyToken(session, privateKey) || !encryptConnection(loginKey)) {
+        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();
-
+        String serverId = EncryptionUtil.getServerIdHashString(generatedId, loginKey, publicKey);
-        //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 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("error-kick", false, "Couldn't enable encryption", ex);
-            disconnect(plugin.getCore().getMessage("error-kick"), false, "Couldn't enable encryption");
             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"));
         }
     }
 }

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);
+    //     }
+    // }
+}

pom.xml

@@ -8,7 +8,7 @@
     <packaging>pom</packaging>
 
     <name>FastLogin</name>
-    <version>1.10</version>
+    <version>1.11</version>
     <inceptionYear>2015</inceptionYear>
     <url>https://www.spigotmc.org/resources/fastlogin.14153/</url>
     <description>