From 44d30829583f6f7daebdf5689e6a963f4199f993 Mon Sep 17 00:00:00 2001 From: delvh Date: Thu, 22 Oct 2020 23:05:51 +0200 Subject: [PATCH] Fix bug allowing unauthorized access to a client Additionally token authentication is now used whenever the client is online --- .../main/java/envoy/client/data/LocalDB.java | 9 ++- .../main/java/envoy/client/net/Client.java | 6 +- .../client/ui/controller/LoginScene.java | 12 ++-- .../java/envoy/data/AuthenticatedRequest.java | 70 ++++++++++++++++++ .../java/envoy/data/LoginCredentials.java | 28 +++----- .../server/net/ObjectMessageProcessor.java | 72 +++++++++++++++---- .../processors/IssueProposalProcessor.java | 3 + .../processors/LoginCredentialProcessor.java | 30 ++++---- .../server/processors/ObjectProcessor.java | 6 ++ 9 files changed, 179 insertions(+), 57 deletions(-) create mode 100644 common/src/main/java/envoy/data/AuthenticatedRequest.java diff --git a/client/src/main/java/envoy/client/data/LocalDB.java b/client/src/main/java/envoy/client/data/LocalDB.java index e3c4ac7..1edede8 100644 --- a/client/src/main/java/envoy/client/data/LocalDB.java +++ b/client/src/main/java/envoy/client/data/LocalDB.java @@ -44,6 +44,7 @@ public final class LocalDB implements EventListener { private IDGenerator idGenerator; private CacheMap cacheMap = new CacheMap(); private String authToken; + private boolean saveToken; private boolean contactsChanged; // Auto save timer @@ -260,7 +261,7 @@ public final class LocalDB implements EventListener { Context.getInstance().getClient().isOnline() ? Instant.now() : lastSync); // Save last login information - if (authToken != null) + if (saveToken && authToken != null) SerializationUtils.write(lastLoginFile, user, authToken); // Save ID generator @@ -488,4 +489,10 @@ public final class LocalDB implements EventListener { * @since Envoy Client v0.2-beta */ public String getAuthToken() { return authToken; } + + /** + * @param saveToken whether the token will be persisted or deleted on shutdown + * @since Envoy Client v0.3-beta + */ + public void setSaveToken(boolean saveToken) { this.saveToken = saveToken; } } diff --git a/client/src/main/java/envoy/client/net/Client.java b/client/src/main/java/envoy/client/net/Client.java index fe1c9e2..8c8e682 100644 --- a/client/src/main/java/envoy/client/net/Client.java +++ b/client/src/main/java/envoy/client/net/Client.java @@ -151,7 +151,11 @@ public final class Client implements EventListener, Closeable { checkOnline(); logger.log(Level.FINE, "Sending " + obj); try { - SerializationUtils.writeBytesWithLength(obj, socket.getOutputStream()); + SerializationUtils.writeBytesWithLength( + new AuthenticatedRequest<>(obj, + Context.getInstance().getLocalDB().getUser().getID(), + Context.getInstance().getLocalDB().getAuthToken()), + socket.getOutputStream()); } catch (final IOException e) { throw new RuntimeException(e); } diff --git a/client/src/main/java/envoy/client/ui/controller/LoginScene.java b/client/src/main/java/envoy/client/ui/controller/LoginScene.java index f9d062b..fc9a71f 100644 --- a/client/src/main/java/envoy/client/ui/controller/LoginScene.java +++ b/client/src/main/java/envoy/client/ui/controller/LoginScene.java @@ -16,7 +16,7 @@ import envoy.data.LoginCredentials; import envoy.event.HandshakeRejection; import envoy.util.*; -import envoy.client.data.ClientConfig; +import envoy.client.data.*; import envoy.client.ui.Startup; import envoy.client.util.IconUtil; @@ -79,9 +79,11 @@ public final class LoginScene implements EventListener { @FXML private void loginButtonPressed() { - final String user = userTextField.getText(), pass = passwordField.getText(), + final String user = userTextField.getText(), pass = passwordField.getText(), repeatPass = repeatPasswordField.getText(); - final boolean requestToken = cbStaySignedIn.isSelected(); + + // Choose whether to persist the token or not + Context.getInstance().getLocalDB().setSaveToken(cbStaySignedIn.isSelected()); // Prevent registration with unequal passwords if (registration && !pass.equals(repeatPass)) { @@ -96,8 +98,8 @@ public final class LoginScene implements EventListener { } else { Instant lastSync = Startup.loadLastSync(userTextField.getText()); Startup.performHandshake(registration - ? LoginCredentials.registration(user, pass, requestToken, Startup.VERSION, lastSync) - : LoginCredentials.login(user, pass, requestToken, Startup.VERSION, lastSync)); + ? LoginCredentials.registration(user, pass, Startup.VERSION, lastSync) + : LoginCredentials.login(user, pass, Startup.VERSION, lastSync)); } } diff --git a/common/src/main/java/envoy/data/AuthenticatedRequest.java b/common/src/main/java/envoy/data/AuthenticatedRequest.java new file mode 100644 index 0000000..fa2f6ce --- /dev/null +++ b/common/src/main/java/envoy/data/AuthenticatedRequest.java @@ -0,0 +1,70 @@ +package envoy.data; + +import java.io.Serializable; +import java.util.Objects; + +/** + * Wraps any request sent to the server in the authentication details of a user. + * + * @author Leon Hofmeister + * @param the type of object to be sent + * @since Envoy Common v0.3-beta + */ +public final class AuthenticatedRequest implements Serializable { + + private final T request; + private final String authentication; + private final long userID; + + private static final long serialVersionUID = 1L; + + /** + * @param request the actual object that should be sent + * @param userID the ID of the currently logged in user + * @param authentication the authentication of the currently logged in user + * @since Envoy Common v0.3-beta + */ + public AuthenticatedRequest(T request, long userID, String authentication) { + this.request = Objects.requireNonNull(request); + this.userID = userID; + this.authentication = authentication == null ? "" : authentication; + } + + /** + * @return the authentication token of the currently logged in user + * @since Envoy Common v0.3-beta + */ + public String getAuthentication() { return authentication; } + + /** + * @return the request + * @since Envoy Common v0.3-beta + */ + public T getRequest() { return request; } + + /** + * @return the userID + * @since Envoy Common v0.3-beta + */ + public long getUserID() { return userID; } + + @Override + public int hashCode() { + return Objects.hash(request, userID); + } + + @Override + public boolean equals(Object obj) { + if (this == obj) + return true; + if (!(obj instanceof AuthenticatedRequest)) + return false; + AuthenticatedRequest other = (AuthenticatedRequest) obj; + return userID == other.userID; + } + + @Override + public String toString() { + return "AuthenticatedRequest [request=" + request + ", userID=" + userID + "]"; + } +} diff --git a/common/src/main/java/envoy/data/LoginCredentials.java b/common/src/main/java/envoy/data/LoginCredentials.java index 292c2e2..f121005 100644 --- a/common/src/main/java/envoy/data/LoginCredentials.java +++ b/common/src/main/java/envoy/data/LoginCredentials.java @@ -14,19 +14,18 @@ import java.time.Instant; public final class LoginCredentials implements Serializable { private final String identifier, password, clientVersion; - private final boolean registration, token, requestToken; + private final boolean registration, token; private final Instant lastSync; private static final long serialVersionUID = 4; private LoginCredentials(String identifier, String password, boolean registration, - boolean token, boolean requestToken, String clientVersion, + boolean token, String clientVersion, Instant lastSync) { this.identifier = identifier; this.password = password; this.registration = registration; this.token = token; - this.requestToken = requestToken; this.clientVersion = clientVersion; this.lastSync = lastSync; } @@ -36,15 +35,14 @@ public final class LoginCredentials implements Serializable { * * @param identifier the identifier of the user * @param password the password of the user - * @param requestToken requests the server to generate an authentication token * @param clientVersion the version of the client sending these credentials * @param lastSync the timestamp of the last synchronization * @return the created login credentials * @since Envoy Common v0.2-beta */ - public static LoginCredentials login(String identifier, String password, boolean requestToken, + public static LoginCredentials login(String identifier, String password, String clientVersion, Instant lastSync) { - return new LoginCredentials(identifier, password, false, false, requestToken, clientVersion, + return new LoginCredentials(identifier, password, false, false, clientVersion, lastSync); } @@ -60,7 +58,7 @@ public final class LoginCredentials implements Serializable { */ public static LoginCredentials loginWithToken(String identifier, String token, String clientVersion, Instant lastSync) { - return new LoginCredentials(identifier, token, false, true, false, clientVersion, lastSync); + return new LoginCredentials(identifier, token, false, true, clientVersion, lastSync); } /** @@ -68,26 +66,24 @@ public final class LoginCredentials implements Serializable { * * @param identifier the identifier of the user * @param password the password of the user - * @param requestToken requests the server to generate an authentication token * @param clientVersion the version of the client sending these credentials * @param lastSync the timestamp of the last synchronization * @return the created login credentials * @since Envoy Common v0.2-beta */ public static LoginCredentials registration(String identifier, String password, - boolean requestToken, String clientVersion, Instant lastSync) { - return new LoginCredentials(identifier, password, true, false, requestToken, clientVersion, + String clientVersion, Instant lastSync) { + return new LoginCredentials(identifier, password, true, false, clientVersion, lastSync); } @Override public String toString() { return String.format( - "LoginCredentials[identifier=%s,registration=%b,token=%b,requestToken=%b,clientVersion=%s,lastSync=%s]", + "LoginCredentials[identifier=%s,registration=%b,token=%b,clientVersion=%s,lastSync=%s]", identifier, registration, token, - requestToken, clientVersion, lastSync); } @@ -119,14 +115,6 @@ public final class LoginCredentials implements Serializable { return token; } - /** - * @return {@code true} if the server should generate a new authentication token - * @since Envoy Common v0.2-beta - */ - public boolean requestToken() { - return requestToken; - } - /** * @return the version of the client sending these credentials * @since Envoy Common v0.1-beta diff --git a/server/src/main/java/envoy/server/net/ObjectMessageProcessor.java b/server/src/main/java/envoy/server/net/ObjectMessageProcessor.java index 0148be7..6bbde84 100755 --- a/server/src/main/java/envoy/server/net/ObjectMessageProcessor.java +++ b/server/src/main/java/envoy/server/net/ObjectMessageProcessor.java @@ -6,9 +6,12 @@ import java.util.Set; import java.util.logging.*; import com.jenkov.nioserver.*; +import com.jenkov.nioserver.Message; +import envoy.data.AuthenticatedRequest; import envoy.util.EnvoyLog; +import envoy.server.data.*; import envoy.server.processors.ObjectProcessor; /** @@ -33,7 +36,6 @@ public final class ObjectMessageProcessor implements IMessageProcessor { this.processors = processors; } - @SuppressWarnings("unchecked") @Override public void process(Message message, WriteProxy writeProxy) { try (ObjectInputStream in = @@ -45,23 +47,63 @@ public final class ObjectMessageProcessor implements IMessageProcessor { return; } - logger.fine("Received " + obj); + // authenticate requests if necessary + boolean authenticated = false; - // Get processor and input class and process object - for (@SuppressWarnings("rawtypes") - ObjectProcessor p : processors) { - Class c = (Class) ((ParameterizedType) p.getClass().getGenericInterfaces()[0]) - .getActualTypeArguments()[0]; - if (c.equals(obj.getClass())) - try { - p.process(c.cast(obj), message.socketId, new ObjectWriteProxy(writeProxy)); - break; - } catch (IOException e) { - logger.log(Level.SEVERE, "Exception during processor execution: ", e); - } - } + if (obj instanceof AuthenticatedRequest) { + Contact contact = PersistenceManager.getInstance() + .getContactByID(((AuthenticatedRequest) obj).getUserID()); + + // Validating the authenticity of the request + if (contact == null || contact instanceof Group + || !((AuthenticatedRequest) obj).getAuthentication() + .equals(((User) contact).getAuthToken())) { + + // Invalid request + logger.log(Level.INFO, + "A user tried to perform an authenticated request but could not identify himself. Discarding request."); + return; + } + + // Valid request + logger.log(Level.INFO, "A user successfully authenticated a request for " + obj); + authenticated = true; + obj = ((AuthenticatedRequest) obj).getRequest(); + } else + logger.log(Level.FINE, "Received unauthenticated " + obj); + + refer(message.socketId, writeProxy, obj, authenticated); } catch (IOException | ClassNotFoundException e) { e.printStackTrace(); } } + + /** + * Executes the appropriate {@link ObjectProcessor} for the given input ({@code obj}), if any is + * present. + */ + @SuppressWarnings("unchecked") + private void refer(long socketID, WriteProxy writeProxy, Object obj, boolean authenticated) { + + // Get processor and input class and process object + for (@SuppressWarnings("rawtypes") + ObjectProcessor p : processors) { + Class c = (Class) ((ParameterizedType) p.getClass().getGenericInterfaces()[0]) + .getActualTypeArguments()[0]; + if (c.equals(obj.getClass())) { + if (!authenticated && p.isAuthenticationRequired()) { + logger.log(Level.INFO, + "Discarding request as no authentication has been provided"); + return; + } + + try { + p.process(c.cast(obj), socketID, new ObjectWriteProxy(writeProxy)); + break; + } catch (IOException e) { + logger.log(Level.SEVERE, "Exception during processor execution: ", e); + } + } + } + } } diff --git a/server/src/main/java/envoy/server/processors/IssueProposalProcessor.java b/server/src/main/java/envoy/server/processors/IssueProposalProcessor.java index cd3f857..01e9b3b 100644 --- a/server/src/main/java/envoy/server/processors/IssueProposalProcessor.java +++ b/server/src/main/java/envoy/server/processors/IssueProposalProcessor.java @@ -22,6 +22,9 @@ public final class IssueProposalProcessor implements ObjectProcessor { * @since Envoy Server Standalone v0.1-alpha */ void process(T input, long socketID, ObjectWriteProxy writeProxy) throws IOException; + + /** + * @return whether authentication is required for the given processor. Defaults to {@code true}. + * @since Envoy Server v0.3-beta + */ + default boolean isAuthenticationRequired() { return true; } }