Fixed NPE on invalid sessions + Improved security for premium logins

CHANGELOG.md

@@ -1,3 +1,9 @@
+######0.2.4
+
+* Fixed NPE on invalid sessions
+* Improved security by generating a randomized serverId
+* Removed /premium [player] because it's safer for premium players who join without registration
+
 ######0.2.3
 
 * Remove useless AuthMe forcelogin code

README.md

@@ -1,5 +1,7 @@
 # FastLogin
 
+[![Build Status](https://travis-ci.org/games647/FastLogin.svg?branch=master)](https://travis-ci.org/games647/FastLogin)
+
 Checks if a minecraft player has a paid account (premium). If so, they can skip offline authentication (auth plugins).
 So they don't need to enter passwords. This is also called auto login.
 
@@ -8,16 +10,15 @@ So they don't need to enter passwords. This is also called auto login.
 * Automatically login paid accounts (premium)
 * Support various of auth plugins
 * Experimental Cauldron support
+* No client modifications needed
 
 ***
 
 ###Commands:
 * /premium Label the invoker as paid account
-* /premium [playername] Label specified player as a paid account
 
 ###Permissions:
 * fastlogin.command.premium
-* fastlogin.command.premium.others
 
 ###Requirements:
 * Plugin: [ProtocolLib](http://www.spigotmc.org/resources/protocollib.1997/)
@@ -32,7 +33,7 @@ So they don't need to enter passwords. This is also called auto login.
 
 ###Downloads
 
-https://github.com/games647/FastLogin/releases
+https://www.spigotmc.org/resources/fastlogin.14153/history
 
 ***
 
@@ -97,11 +98,14 @@ Yes, indeed. Therefore the command for toggling the fast login method exists.
 
 ####Why do players have to invoke a command?
 1. It's a secure way to make sure a person with a paid account cannot steal the account
-of a cracked player that has the same username.
+of a cracked player that has the same username. The player have to proof first that it's his own account.
 2. We only receive the username from the player on login. We could check if that username is associated
 to a paid account but if we request a online mode login from a cracked player (who uses a username from
 a paid account), the player will disconnect with the reason bad login. There is no way to change that message
 on the server side (without client modifications), because it's a connection between the Client and the Sessionserver.
+3. If a premium player would skip registration too, a player of a cracked account could later still register the
+account and would claim and steal the account from the premium player. Because commands cannot be invoked unless the
+player has a account or is logged in, protects this method also premium players
 
 ###What happens if a paid account joins with a used username?
 The player on the server have to activate the feature of this plugin by command. If a person buys the username

pom.xml

@@ -8,7 +8,7 @@
     <packaging>jar</packaging>
 
     <name>FastLogin</name>
-    <version>0.2.3</version>
+    <version>0.2.4</version>
     <inceptionYear>2015</inceptionYear>
     <url>https://github.com/games647/FastLogin</url>
     <description>

src/main/java/com/github/games647/fastlogin/PlayerSession.java

@@ -7,15 +7,30 @@ package com.github.games647.fastlogin;
  */
 public class PlayerSession {
 
-    private final byte[] verifyToken;
     private final String username;
+    private final String serverId;
+    private final byte[] verifyToken;
     private boolean verified;
 
-    public PlayerSession(byte[] verifyToken, String username) {
+    public PlayerSession(String username, String serverId, byte[] verifyToken) {
         this.username = username;
+        this.serverId = serverId;
         this.verifyToken = verifyToken;
     }
 
+    /**
+     * Gets the random generated server id. This makes sure the request
+     * sent from the client is just for this server.
+     *
+     * See this for details
+     * http://www.sk89q.com/2011/09/minecraft-name-spoofing-exploit/
+     *
+     * @return random generated server id
+     */
+    public String getServerId() {
+        return serverId;
+    }
+
     /**
      * Gets the verify token the server sent to the client.
      *

src/main/java/com/github/games647/fastlogin/PremiumCommand.java

@@ -34,17 +34,6 @@ public class PremiumCommand implements CommandExecutor {
             return true;
         }
 
-        if (sender.hasPermission(plugin.getName() + ".command." + command.getName() + ".others")) {
-            String playerName = args[0];
-            //todo check if valid username
-            plugin.getEnabledPremium().add(playerName);
-            sender.sendMessage(ChatColor.DARK_GREEN + "Added "
-                    + ChatColor.DARK_BLUE + ChatColor.BOLD + playerName
-                    + ChatColor.RESET + ChatColor.DARK_GREEN + " to the list of premium players");
-        } else {
-            sender.sendMessage(ChatColor.DARK_RED + "Not enough permissions");
-        }
-
         return true;
     }
 }

src/main/java/com/github/games647/fastlogin/listener/EncryptionPacketListener.java

@@ -100,9 +100,13 @@ public class EncryptionPacketListener extends PacketAdapter {
             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("", plugin.getServerKey().getPublic(), loginKey);
+        byte[] serverIdHash = EncryptionUtil.getServerIdHash(generatedId, plugin.getServerKey().getPublic(), loginKey);
         String serverId = (new BigInteger(serverIdHash)).toString(16);
 
         String username = session.getUsername();
@@ -206,7 +210,7 @@ public class EncryptionPacketListener extends PacketAdapter {
 
             BufferedReader reader = new BufferedReader(new InputStreamReader(conn.getInputStream()));
             String line = reader.readLine();
-            if (!"null".equals(line)) {
+            if (line != null && !line.equals("null")) {
                 //validate parsing
                 //http://wiki.vg/Protocol_Encryption#Server
                 JSONObject userData = (JSONObject) JSONValue.parseWithException(line);
@@ -222,7 +226,7 @@ public class EncryptionPacketListener extends PacketAdapter {
             }
         } catch (Exception ex) {
             //catch not only ioexceptions also parse and NPE on unexpected json format
-            plugin.getLogger().log(Level.WARNING, "Failed to verify if session is valid", ex);
+            plugin.getLogger().log(Level.WARNING, "Failed to verify session", ex);
         }
 
         //this connection doesn't need to be closed. So can make use of keep alive in java

src/main/java/com/github/games647/fastlogin/listener/StartPacketListener.java

@@ -119,6 +119,10 @@ public class StartPacketListener extends PacketAdapter {
              */
             PacketContainer newPacket = protocolManager.createPacket(PacketType.Login.Server.ENCRYPTION_BEGIN, true);
 
+            //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/
+            String serverId = Long.toString(random.nextLong(), 16);
+            newPacket.getStrings().write(0, serverId);
             newPacket.getSpecificModifier(PublicKey.class).write(0, plugin.getServerKey().getPublic());
             //generate a random token which should be the same when we receive it from the client
             byte[] verifyToken = new byte[VERIFY_TOKEN_LENGTH];
@@ -129,7 +133,7 @@ public class StartPacketListener extends PacketAdapter {
             protocolManager.sendServerPacket(player, newPacket);
 
             //cancel only if the player has a paid account otherwise login as normal offline player
-            plugin.getSessions().put(sessionKey, new PlayerSession(verifyToken, username));
+            plugin.getSessions().put(sessionKey, new PlayerSession(username, serverId, verifyToken));
             packetEvent.setCancelled(true);
         } catch (InvocationTargetException ex) {
             plugin.getLogger().log(Level.SEVERE, "Cannot send encryption packet. Falling back to normal login", ex);

src/main/resources/plugin.yml

@@ -30,6 +30,4 @@ commands:
 permissions:
     ${project.artifactId}.command.premium:
         description: 'Label themselves as premium using a command'
-        default: true
+        default: true
-    ${project.artifactId}.command.premium.others:
-        description: 'Label other people as premium'