From dd9003190a29a2ed6f7978551dba240486575625 Mon Sep 17 00:00:00 2001 From: Carlos <28845529+mesacarlos@users.noreply.github.com> Date: Sat, 5 Dec 2020 13:26:04 +0100 Subject: [PATCH] Improved security --- client/scripts/WebConsole.js | 2 +- client/scripts/WebConsoleConnector.js | 10 +- client/scripts/WebConsoleManager.js | 34 +++++-- .../webconsole/auth/ConnectedUser.java | 14 ++- .../webconsole/auth/LoginManager.java | 16 +++- .../mesacarlos/webconsole/util/JsonUtils.java | 93 +++++++++++++++++++ .../webconsole/websocket/WSServer.java | 21 +++-- .../websocket/command/LogInCommand.java | 8 +- .../websocket/response/LoggedIn.java | 11 ++- 9 files changed, 181 insertions(+), 28 deletions(-) create mode 100644 src/es/mesacarlos/webconsole/util/JsonUtils.java diff --git a/client/scripts/WebConsole.js b/client/scripts/WebConsole.js index 0c6ac1c..389f8c3 100644 --- a/client/scripts/WebConsole.js +++ b/client/scripts/WebConsole.js @@ -24,7 +24,7 @@ function openServer(serverName){ //Change server name and related info $("#serverTitle").text(serverName); - $("#consoleTextArea").text("Connecting..."); + $("#consoleTextArea").text(""); $("#commandInput").prop("disabled", false); $("#sendCommandButton").prop("disabled", false); diff --git a/client/scripts/WebConsoleConnector.js b/client/scripts/WebConsoleConnector.js index 804b988..ccaa371 100644 --- a/client/scripts/WebConsoleConnector.js +++ b/client/scripts/WebConsoleConnector.js @@ -9,6 +9,7 @@ class WebConsoleConnector { constructor(serverName, serverURI) { this.serverName = serverName; this.serverURI = serverURI; + this.token; this.subscribers = []; //List of functions called when a new message arrive this.messages = []; //All messages retrieved since connection start this.commands = []; //EXEC Commands sent by user to this server @@ -32,7 +33,7 @@ class WebConsoleConnector { * Internal function */ onOpen(evt){ - //TODO Check que la version es correcta, y que es un WebSocket del plugin y no de otra cosa + //TODO Check version is correct, and this websocket server is a WebConsole WebSocket } /** @@ -48,6 +49,11 @@ class WebConsoleConnector { */ onMessage(evt){ var obj = JSON.parse(evt.data); + + + if(obj.status === 200) //If is a LoggedIn response, save our token + this.token = obj.token; + this.notify(obj); //Notify all subscribers this.messages.push(obj); } @@ -63,7 +69,7 @@ class WebConsoleConnector { * Sends a WebSocket command to Server */ sendToServer(message){ - this.websocket.send(message); + this.websocket.send(JSON.stringify(message)); } /** diff --git a/client/scripts/WebConsoleManager.js b/client/scripts/WebConsoleManager.js index 7aea278..a374f36 100644 --- a/client/scripts/WebConsoleManager.js +++ b/client/scripts/WebConsoleManager.js @@ -60,14 +60,22 @@ class WebConsoleManager { * Send password to server */ sendPassword(pwd){ - this.activeConnection.sendToServer("LOGIN " + pwd); + this.activeConnection.sendToServer({ + command: "LOGIN", + params: pwd + }); } /** * Send console command to server */ sendConsoleCmd(cmd){ - this.activeConnection.sendToServer("EXEC " + cmd); + this.activeConnection.sendToServer({ + command: "EXEC", + token: this.activeConnection.token, + params: cmd + }); + this.activeConnection.commands.push(cmd); } @@ -75,16 +83,30 @@ class WebConsoleManager { * Asks server for CPU, RAM and players info */ askForInfo(){ - this.activeConnection.sendToServer("PLAYERS"); - this.activeConnection.sendToServer("CPUUSAGE"); - this.activeConnection.sendToServer("RAMUSAGE"); + this.activeConnection.sendToServer({ + command: "PLAYERS", + token: this.activeConnection.token, + }); + + this.activeConnection.sendToServer({ + command: "CPUUSAGE", + token: this.activeConnection.token, + }); + + this.activeConnection.sendToServer({ + command: "RAMUSAGE", + token: this.activeConnection.token, + }); } /** * Asks server for full latest.log */ askForLogs(){ - this.activeConnection.sendToServer("READLOGFILE"); + this.activeConnection.sendToServer({ + command: "READLOGFILE", + token: this.activeConnection.token, + }); } } \ No newline at end of file diff --git a/src/es/mesacarlos/webconsole/auth/ConnectedUser.java b/src/es/mesacarlos/webconsole/auth/ConnectedUser.java index 6318679..1e4bcef 100644 --- a/src/es/mesacarlos/webconsole/auth/ConnectedUser.java +++ b/src/es/mesacarlos/webconsole/auth/ConnectedUser.java @@ -8,20 +8,26 @@ import es.mesacarlos.webconsole.util.Internationalization; public class ConnectedUser { private String username; private InetSocketAddress socketAddress; + private String token; private UserType userType; - public ConnectedUser(InetSocketAddress socketAddress, String username, UserType userType) { + public ConnectedUser(InetSocketAddress socketAddress, String username, String token, UserType userType) { this.socketAddress = socketAddress; this.username = username; + this.token = token; this.userType = userType; } + public String getUsername() { + return username; + } + public InetSocketAddress getSocketAddress() { return socketAddress; } - - public String getUsername() { - return username; + + public String getToken() { + return token; } public UserType getUserType() { diff --git a/src/es/mesacarlos/webconsole/auth/LoginManager.java b/src/es/mesacarlos/webconsole/auth/LoginManager.java index ecfc47a..d6d062f 100644 --- a/src/es/mesacarlos/webconsole/auth/LoginManager.java +++ b/src/es/mesacarlos/webconsole/auth/LoginManager.java @@ -46,11 +46,23 @@ public class LoginManager { } /** - * Check if user is logged in + * Check if user is logged in. It checks that both the socket adress and the user token corresponds to a logged in user. * @param address User to check * @return true if user is logged in, false otherwise */ - public boolean isLoggedIn(InetSocketAddress address) { + public boolean isLoggedIn(InetSocketAddress address, String token) { + for(ConnectedUser user : loggedInUsers) + if(user.getSocketAddress().equals(address) && user.getToken().equals(token)) + return true; + return false; + } + + /** + * Check if an user is logged in from a given socket address + * @param address User to check + * @return true if user is logged in, false otherwise + */ + public boolean isSocketConnected(InetSocketAddress address) { for(ConnectedUser user : loggedInUsers) if(user.getSocketAddress().equals(address)) return true; diff --git a/src/es/mesacarlos/webconsole/util/JsonUtils.java b/src/es/mesacarlos/webconsole/util/JsonUtils.java new file mode 100644 index 0000000..0c1a59e --- /dev/null +++ b/src/es/mesacarlos/webconsole/util/JsonUtils.java @@ -0,0 +1,93 @@ +package es.mesacarlos.webconsole.util; + +import com.google.gson.Gson; +import com.google.gson.JsonElement; +import com.google.gson.JsonObject; +import com.google.gson.JsonParser; + +public class JsonUtils { + /*{ + "command": "LOGIN", + "token": "aosduhasiudgasuidgasdgaspid", + "params": "" + }*/ + public final static String COMMAND_PROPERTY = "command"; + public final static String TOKEN_PROPERTY = "token"; + public final static String PARAMS_PROPERTY = "params"; + + /** + * Check that a given String is a valid JSON + * @param Json JSON to check + * @return true if it is a valid JSON, false otherwise + */ + public static boolean isValidJson(String Json) { + Gson gson = new Gson(); + try { + gson.fromJson(Json, Object.class); + Object jsonObjType = gson.fromJson(Json, Object.class).getClass(); + if(jsonObjType.equals(String.class)){ + return false; + } + return true; + } catch (com.google.gson.JsonSyntaxException ex) { + return false; + } + } + + /** + * Check that a given JSON contains some property + * @param JsonString JSON to check + * @param property property to check + * @return true if the JSON string contains that property, false otherwise + */ + public static boolean containsProperty(String JsonString, String property) { + if(!isValidJson(JsonString)) + return false; + + JsonParser parser = new JsonParser(); + JsonObject obj = parser.parse(JsonString).getAsJsonObject(); + JsonElement elem = obj.get(property); + if(elem == null) + return false; + return true; + } + + /** + * Check that a given JSON contains some property, and its type is a String + * @param JsonString JSON to check + * @param property property to check + * @returntrue if the JSON string contains that property and it is a String, false otherwise + */ + public static boolean containsStringProperty(String JsonString, String property) { + if(!isValidJson(JsonString)) + return false; + + JsonParser parser = new JsonParser(); + JsonObject obj = parser.parse(JsonString).getAsJsonObject(); + JsonElement elem = obj.get(property); + if(elem == null) + return false; + try { + elem.getAsString(); + return true; + }catch(Exception e) { + return false; + } + } + + /** + * Get a String property from a JSON + * @param JsonString JSON to check + * @param property property to extract from JSON string + * @return the value for that property. If the property is not set, an empty string will be returned + */ + public static String getStringProperty(String JsonString, String property) { + JsonParser parser = new JsonParser(); + JsonObject obj = parser.parse(JsonString).getAsJsonObject(); + JsonElement result = obj.get(property); + if(result != null) + return result.getAsString(); + return ""; + } + +} \ No newline at end of file diff --git a/src/es/mesacarlos/webconsole/websocket/WSServer.java b/src/es/mesacarlos/webconsole/websocket/WSServer.java index c466bd8..9ce9c46 100644 --- a/src/es/mesacarlos/webconsole/websocket/WSServer.java +++ b/src/es/mesacarlos/webconsole/websocket/WSServer.java @@ -13,6 +13,7 @@ import org.java_websocket.server.WebSocketServer; import es.mesacarlos.webconsole.auth.LoginManager; import es.mesacarlos.webconsole.util.DateTimeUtils; import es.mesacarlos.webconsole.util.Internationalization; +import es.mesacarlos.webconsole.util.JsonUtils; import es.mesacarlos.webconsole.websocket.command.WSCommandFactory; import es.mesacarlos.webconsole.websocket.command.WSCommand; import es.mesacarlos.webconsole.websocket.response.ConsoleOutput; @@ -30,7 +31,7 @@ public class WSServer extends WebSocketServer { @Override public void onOpen(WebSocket conn, ClientHandshake handshake) { - if (LoginManager.getInstance().isLoggedIn(conn.getRemoteSocketAddress())) { + if (LoginManager.getInstance().isSocketConnected(conn.getRemoteSocketAddress())) { sendToClient(conn, new LoggedIn(Internationalization.getPhrase("connection-resumed-message"))); Bukkit.getLogger().info(Internationalization.getPhrase("connection-resumed-console", conn.getRemoteSocketAddress())); } else { @@ -41,11 +42,15 @@ public class WSServer extends WebSocketServer { @Override public void onMessage(WebSocket conn, String message) { + if(!JsonUtils.containsStringProperty(message, "command") //Contains a command + || ( !JsonUtils.containsStringProperty(message, "token") && !JsonUtils.getStringProperty(message, JsonUtils.COMMAND_PROPERTY).equals("LOGIN")) //Contains a token or it is a login command + ) + return; + // Get command and params - String wsCommand = message.split(" ")[0]; - String wsCommandParams = ""; - if (message.contains(" ")) - wsCommandParams = message.split(" ", 2)[1]; + String wsCommand = JsonUtils.getStringProperty(message, JsonUtils.COMMAND_PROPERTY); + String wsToken = JsonUtils.getStringProperty(message, JsonUtils.TOKEN_PROPERTY); + String wsCommandParams = JsonUtils.getStringProperty(message, JsonUtils.PARAMS_PROPERTY); // Run command WSCommand cmd = commands.get(wsCommand); @@ -54,8 +59,8 @@ public class WSServer extends WebSocketServer { // Command does not exist sendToClient(conn, new UnknownCommand(Internationalization.getPhrase("unknown-command-message"), message)); Bukkit.getLogger().info(Internationalization.getPhrase("unknown-command-console", message)); - } else if (!LoginManager.getInstance().isLoggedIn(conn.getRemoteSocketAddress()) - && !wsCommand.equals("LOGIN")) { + } else if (!wsCommand.equals("LOGIN") + && !LoginManager.getInstance().isLoggedIn(conn.getRemoteSocketAddress(), wsToken)) { // User is not authorised. DO NOTHING, IMPORTANT! sendToClient(conn, new LoginRequired(Internationalization.getPhrase("forbidden-message"))); Bukkit.getLogger().warning(Internationalization.getPhrase("forbidden-console", conn.getRemoteSocketAddress(), message)); @@ -86,7 +91,7 @@ public class WSServer extends WebSocketServer { public void onNewConsoleLinePrinted(String line) { Collection connections = getConnections(); for (WebSocket connection : connections) { - if (LoginManager.getInstance().isLoggedIn(connection.getRemoteSocketAddress())) + if (LoginManager.getInstance().isSocketConnected(connection.getRemoteSocketAddress())) sendToClient(connection, new ConsoleOutput(line, DateTimeUtils.getTimeAsString())); } } diff --git a/src/es/mesacarlos/webconsole/websocket/command/LogInCommand.java b/src/es/mesacarlos/webconsole/websocket/command/LogInCommand.java index 343523d..202ff20 100644 --- a/src/es/mesacarlos/webconsole/websocket/command/LogInCommand.java +++ b/src/es/mesacarlos/webconsole/websocket/command/LogInCommand.java @@ -1,5 +1,7 @@ package es.mesacarlos.webconsole.websocket.command; +import java.util.UUID; + import org.bukkit.Bukkit; import org.java_websocket.WebSocket; @@ -17,16 +19,16 @@ public class LogInCommand implements WSCommand { @Override public void execute(WSServer wsServer, WebSocket conn, String password) { // If user is logged in, then return. - if (LoginManager.getInstance().isLoggedIn(conn.getRemoteSocketAddress())) + if (LoginManager.getInstance().isSocketConnected(conn.getRemoteSocketAddress())) return; //Check if user exists for(UserData ud : ConfigManager.getInstance().getAllUsers()) { if(ud.getPassword().equals(password)) { - ConnectedUser user = new ConnectedUser(conn.getRemoteSocketAddress(), ud.getUsername(), ud.getUserType()); + ConnectedUser user = new ConnectedUser(conn.getRemoteSocketAddress(), ud.getUsername(), UUID.randomUUID().toString(), ud.getUserType()); LoginManager.getInstance().logIn(user); - wsServer.sendToClient(conn, new LoggedIn(Internationalization.getPhrase("login-sucessful-message"), "LOGIN ********", user.getUsername(), user.getUserType())); + wsServer.sendToClient(conn, new LoggedIn(Internationalization.getPhrase("login-sucessful-message"), "LOGIN ********", user.getUsername(), user.getUserType(), user.getToken())); Bukkit.getLogger().info(Internationalization.getPhrase("login-sucessful-console", user.toString())); return; } diff --git a/src/es/mesacarlos/webconsole/websocket/response/LoggedIn.java b/src/es/mesacarlos/webconsole/websocket/response/LoggedIn.java index a2e1210..9cd87f9 100644 --- a/src/es/mesacarlos/webconsole/websocket/response/LoggedIn.java +++ b/src/es/mesacarlos/webconsole/websocket/response/LoggedIn.java @@ -9,16 +9,18 @@ public class LoggedIn implements JSONOutput{ private String respondsTo; private String username; private UserType as; + private String token; public LoggedIn(String message) { this.message = message; } - public LoggedIn(String message, String respondsTo, String username, UserType as) { + public LoggedIn(String message, String respondsTo, String username, UserType as, String token) { this.message = message; this.respondsTo = respondsTo; this.username = username; this.as = as; + this.token = token; } @Override @@ -52,7 +54,11 @@ public class LoggedIn implements JSONOutput{ return "VIEWER"; //This is not a security hole bc its just informative... } } - + + private String getToken() { + return token; + } + @Override public String toJSON() { JsonObject object = new JsonObject(); @@ -61,6 +67,7 @@ public class LoggedIn implements JSONOutput{ object.addProperty("respondsTo", getRespondsTo()); object.addProperty("username", getUsername()); object.addProperty("as", getAs()); + object.addProperty("token", getToken()); object.addProperty("message", getMessage()); return object.toString(); }