From 80f766554a3dd117b2958fd8c55b8fab2b73f9f9 Mon Sep 17 00:00:00 2001 From: Demian Date: Thu, 11 Jun 2015 10:20:37 -0300 Subject: Implemented password hashing using a salt, following this code&guidelines: https://crackstation.net/hashing-security.htm --- src/org/traccar/database/DataManager.java | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) (limited to 'src/org/traccar/database') diff --git a/src/org/traccar/database/DataManager.java b/src/org/traccar/database/DataManager.java index ef9e2a31a..79de15998 100644 --- a/src/org/traccar/database/DataManager.java +++ b/src/org/traccar/database/DataManager.java @@ -166,8 +166,8 @@ public class DataManager { User admin = new User(); admin.setName("admin"); admin.setEmail("admin"); - admin.setPassword("admin"); admin.setAdmin(true); + admin.hashPassword("admin"); admin.setId(QueryBuilder.create(dataSource, properties.getProperty("database.insertUser")) .setObject(admin) .executeUpdate()); @@ -221,10 +221,10 @@ public class DataManager { } public User login(String email, String password) throws SQLException { - return QueryBuilder.create(dataSource, properties.getProperty("database.loginUser")) + User user = QueryBuilder.create(dataSource, properties.getProperty("database.loginUser")) .setString("email", email) - .setBytes("password", Hashing.sha256(password)) .executeQuerySingle(new User()); + return user != null && user.isPasswordValid(password) ? user : null; } public Collection getUsers() throws SQLException { @@ -232,19 +232,20 @@ public class DataManager { .executeQuery(new User()); } - public void addUser(User user) throws SQLException { + public void addUser(User user, String password) throws SQLException { + user.hashPassword(password); user.setId(QueryBuilder.create(dataSource, properties.getProperty("database.insertUser")) .setObject(user) .executeUpdate()); Context.getPermissionsManager().refresh(); } - public void updateUser(User user) throws SQLException { + public void updateUser(User user, String password) throws SQLException { QueryBuilder.create(dataSource, properties.getProperty("database.updateUser")) .setObject(user) .executeUpdate(); - - if(user.getPassword() != null) { + if(password != null) { + user.hashPassword(password); QueryBuilder.create(dataSource, properties.getProperty("database.updateUserPassword")) .setObject(user) .executeUpdate(); @@ -252,7 +253,7 @@ public class DataManager { Context.getPermissionsManager().refresh(); } - + public void removeUser(User user) throws SQLException { QueryBuilder.create(dataSource, properties.getProperty("database.deleteUser")) .setObject(user) -- cgit v1.2.3 From 92ac9aaa10fcf65a005c4e06245ce4a9427d5148 Mon Sep 17 00:00:00 2001 From: Demian Date: Tue, 16 Jun 2015 18:25:28 -0300 Subject: Separated the persisted password (hashedPassword) from the password sent from the web request. Improved JSON serialization so it doesnt send as a response the hashed password and salt. --- debug.xml | 8 +++--- src/org/traccar/database/DataManager.java | 10 +++----- src/org/traccar/helper/IgnoreOnSerialization.java | 12 +++++++++ src/org/traccar/http/JsonConverter.java | 5 ++++ src/org/traccar/http/MainServlet.java | 2 +- src/org/traccar/http/UserServlet.java | 4 +-- src/org/traccar/model/User.java | 30 ++++++++++++++++------- 7 files changed, 49 insertions(+), 22 deletions(-) create mode 100644 src/org/traccar/helper/IgnoreOnSerialization.java (limited to 'src/org/traccar/database') diff --git a/debug.xml b/debug.xml index 84587f293..01bb66d60 100644 --- a/debug.xml +++ b/debug.xml @@ -43,7 +43,7 @@ id INT PRIMARY KEY AUTO_INCREMENT, name VARCHAR(1024) NOT NULL, email VARCHAR(256) NOT NULL UNIQUE, - password VARCHAR(1024) NOT NULL, + hashedPassword VARCHAR(1024) NOT NULL, salt VARCHAR(1024) DEFAULT '' NOT NULL, readonly BOOLEAN DEFAULT false NOT NULL, admin BOOLEAN DEFAULT false NOT NULL, @@ -141,8 +141,8 @@ - INSERT INTO user (name, email, password, salt, admin) - VALUES (:name, :email, :password, :salt, :admin); + INSERT INTO user (name, email, hashedPassword, salt, admin) + VALUES (:name, :email, :hashedPassword, :salt, :admin); @@ -154,7 +154,7 @@ - UPDATE user SET password = :password, salt = :salt WHERE id = :id; + UPDATE user SET hashedPassword = :hashedPassword, salt = :salt WHERE id = :id; diff --git a/src/org/traccar/database/DataManager.java b/src/org/traccar/database/DataManager.java index 79de15998..1aae7da4e 100644 --- a/src/org/traccar/database/DataManager.java +++ b/src/org/traccar/database/DataManager.java @@ -167,7 +167,7 @@ public class DataManager { admin.setName("admin"); admin.setEmail("admin"); admin.setAdmin(true); - admin.hashPassword("admin"); + admin.setPassword("admin"); admin.setId(QueryBuilder.create(dataSource, properties.getProperty("database.insertUser")) .setObject(admin) .executeUpdate()); @@ -232,20 +232,18 @@ public class DataManager { .executeQuery(new User()); } - public void addUser(User user, String password) throws SQLException { - user.hashPassword(password); + public void addUser(User user) throws SQLException { user.setId(QueryBuilder.create(dataSource, properties.getProperty("database.insertUser")) .setObject(user) .executeUpdate()); Context.getPermissionsManager().refresh(); } - public void updateUser(User user, String password) throws SQLException { + public void updateUser(User user) throws SQLException { QueryBuilder.create(dataSource, properties.getProperty("database.updateUser")) .setObject(user) .executeUpdate(); - if(password != null) { - user.hashPassword(password); + if(user.getHashedPassword() != null) { QueryBuilder.create(dataSource, properties.getProperty("database.updateUserPassword")) .setObject(user) .executeUpdate(); diff --git a/src/org/traccar/helper/IgnoreOnSerialization.java b/src/org/traccar/helper/IgnoreOnSerialization.java new file mode 100644 index 000000000..22ec7ced8 --- /dev/null +++ b/src/org/traccar/helper/IgnoreOnSerialization.java @@ -0,0 +1,12 @@ +package org.traccar.helper; + +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; + +import static java.lang.annotation.ElementType.METHOD; + +@Retention(RetentionPolicy.RUNTIME) +@Target(value = {METHOD}) +public @interface IgnoreOnSerialization { +} diff --git a/src/org/traccar/http/JsonConverter.java b/src/org/traccar/http/JsonConverter.java index 6cdba5492..f18470d9d 100644 --- a/src/org/traccar/http/JsonConverter.java +++ b/src/org/traccar/http/JsonConverter.java @@ -30,6 +30,8 @@ import javax.json.JsonArrayBuilder; import javax.json.JsonObject; import javax.json.JsonObjectBuilder; import javax.json.JsonValue; + +import org.traccar.helper.IgnoreOnSerialization; import org.traccar.model.Factory; public class JsonConverter { @@ -88,6 +90,9 @@ public class JsonConverter { Method[] methods = object.getClass().getMethods(); for (Method method : methods) { + if(method.isAnnotationPresent(IgnoreOnSerialization.class)) { + continue; + } if (method.getName().startsWith("get") && method.getParameterTypes().length == 0) { String name = Introspector.decapitalize(method.getName().substring(3)); try { diff --git a/src/org/traccar/http/MainServlet.java b/src/org/traccar/http/MainServlet.java index cf6e81286..18430f0c3 100644 --- a/src/org/traccar/http/MainServlet.java +++ b/src/org/traccar/http/MainServlet.java @@ -67,7 +67,7 @@ public class MainServlet extends BaseServlet { private void register(HttpServletRequest req, HttpServletResponse resp) throws Exception { User user = JsonConverter.objectFromJson(req.getReader(), new User()); - Context.getDataManager().addUser(user, user.getPassword()); + Context.getDataManager().addUser(user); sendResponse(resp.getWriter(), true); } diff --git a/src/org/traccar/http/UserServlet.java b/src/org/traccar/http/UserServlet.java index 197ef0326..f388326b0 100644 --- a/src/org/traccar/http/UserServlet.java +++ b/src/org/traccar/http/UserServlet.java @@ -47,14 +47,14 @@ public class UserServlet extends BaseServlet { private void add(HttpServletRequest req, HttpServletResponse resp) throws Exception { User user = JsonConverter.objectFromJson(req.getReader(), new User()); Context.getPermissionsManager().checkUser(getUserId(req), user.getId()); - Context.getDataManager().addUser(user, user.getPassword()); + Context.getDataManager().addUser(user); sendResponse(resp.getWriter(), JsonConverter.objectToJson(user)); } private void update(HttpServletRequest req, HttpServletResponse resp) throws Exception { User user = JsonConverter.objectFromJson(req.getReader(), new User()); Context.getPermissionsManager().checkUser(getUserId(req), user.getId()); - Context.getDataManager().updateUser(user, user.getPassword()); + Context.getDataManager().updateUser(user); sendResponse(resp.getWriter(), true); } diff --git a/src/org/traccar/model/User.java b/src/org/traccar/model/User.java index fa09861ed..f7c55c0d6 100644 --- a/src/org/traccar/model/User.java +++ b/src/org/traccar/model/User.java @@ -15,6 +15,7 @@ */ package org.traccar.model; +import org.traccar.helper.IgnoreOnSerialization; import org.traccar.helper.PasswordHash; import org.traccar.helper.PasswordHash.HashingResult; @@ -36,14 +37,16 @@ public class User implements Factory { private String email; public String getEmail() { return email; } public void setEmail(String email) { this.email = email; } - - private String password; - public String getPassword() { return password; } - public void setPassword(String password) { - this.password = password; + + private String hashedPassword; + @IgnoreOnSerialization + public String getHashedPassword() { return hashedPassword; } + public void setHashedPassword(String hashedPassword) { + this.hashedPassword = hashedPassword; } - + private String salt; + @IgnoreOnSerialization public String getSalt() { return salt; } public void setSalt(String salt) { this.salt = salt; } private boolean readonly; @@ -65,14 +68,23 @@ public class User implements Factory { private double longitude; private int zoom; - + + private String password; + public String getPassword() { return password; } + public void setPassword(String password) { + this.password = password; + if(this.password != null && !this.password.trim().equals("")) { + this.hashPassword(password); + } + } + public boolean isPasswordValid(String inputPassword) { - return PasswordHash.validatePassword(inputPassword.toCharArray(), PasswordHash.PBKDF2_ITERATIONS, this.salt, this.password); + return PasswordHash.validatePassword(inputPassword.toCharArray(), PasswordHash.PBKDF2_ITERATIONS, this.salt, this.hashedPassword); } public void hashPassword(String password) { HashingResult hashingResult = PasswordHash.createHash(password); - this.password = hashingResult.hash; + this.hashedPassword = hashingResult.hash; this.salt = hashingResult.salt; } } -- cgit v1.2.3