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 --- .gitignore | 2 + debug.xml | 8 +- src/org/traccar/database/DataManager.java | 17 +-- src/org/traccar/helper/PasswordHash.java | 231 ++++++++++++++++++++++++++++++ src/org/traccar/http/MainServlet.java | 2 +- src/org/traccar/http/UserServlet.java | 4 +- src/org/traccar/model/User.java | 24 +++- 7 files changed, 269 insertions(+), 19 deletions(-) create mode 100644 src/org/traccar/helper/PasswordHash.java diff --git a/.gitignore b/.gitignore index ce0c89d44..342b2965d 100644 --- a/.gitignore +++ b/.gitignore @@ -5,3 +5,5 @@ nbactions.xml .settings .idea web/.idea +.idea +traccar.iml diff --git a/debug.xml b/debug.xml index 799f6aa92..84587f293 100644 --- a/debug.xml +++ b/debug.xml @@ -133,7 +133,7 @@ SELECT * FROM user - WHERE email = :email AND password = :password; + WHERE email = :email; @@ -141,8 +141,8 @@ - INSERT INTO user (name, email, password, admin) - VALUES (:name, :email, :password, :admin); + INSERT INTO user (name, email, password, salt, admin) + VALUES (:name, :email, :password, :salt, :admin); @@ -154,7 +154,7 @@ - UPDATE user SET password = :password WHERE id = :id; + UPDATE user SET password = :password, salt = :salt WHERE id = :id; 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) diff --git a/src/org/traccar/helper/PasswordHash.java b/src/org/traccar/helper/PasswordHash.java new file mode 100644 index 000000000..98ded0988 --- /dev/null +++ b/src/org/traccar/helper/PasswordHash.java @@ -0,0 +1,231 @@ +package org.traccar.helper; + +/* + * Password Hashing With PBKDF2 (http://crackstation.net/hashing-security.htm). + * Copyright (c) 2013, Taylor Hornby + * All rights reserved. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions are met: + * + * 1. Redistributions of source code must retain the above copyright notice, + * this list of conditions and the following disclaimer. + * + * 2. Redistributions in binary form must reproduce the above copyright notice, + * this list of conditions and the following disclaimer in the documentation + * and/or other materials provided with the distribution. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" + * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE + * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE + * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR + * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF + * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS + * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN + * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) + * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE + * POSSIBILITY OF SUCH DAMAGE. + */ + +import java.security.SecureRandom; + +import javax.crypto.spec.PBEKeySpec; +import javax.crypto.SecretKeyFactory; + +import java.math.BigInteger; +import java.security.NoSuchAlgorithmException; +import java.security.spec.InvalidKeySpecException; + +/* + * PBKDF2 salted password hashing. + * Author: havoc AT defuse.ca + * www: http://crackstation.net/hashing-security.htm + */ +public class PasswordHash { + public static final String PBKDF2_ALGORITHM = "PBKDF2WithHmacSHA1"; + + // The following constants may be changed without breaking existing hashes. + public static final int SALT_BYTE_SIZE = 24; + public static final int HASH_BYTE_SIZE = 24; + public static final int PBKDF2_ITERATIONS = 1000; + + public static final int ITERATION_INDEX = 0; + public static final int SALT_INDEX = 1; + public static final int PBKDF2_INDEX = 2; + + public static class HashingResult { + + public final Integer iterations; + public final String hash; + public final String salt; + + public HashingResult(Integer iterations, String hash, String salt) { + this.hash = hash; + this.salt = salt; + this.iterations = iterations; + } + } + + /** + * Returns a salted PBKDF2 hash of the password. + * + * @param password + * the password to hash + * @return a salted PBKDF2 hash of the password + */ + public static HashingResult createHash(String password) { + return createHash(password.toCharArray()); + } + + /** + * Returns a salted PBKDF2 hash of the password. + * + * @param password + * the password to hash + * @return a salted PBKDF2 hash of the password + */ + public static HashingResult createHash(char[] password) { + // Generate a random salt + SecureRandom random = new SecureRandom(); + byte[] salt = new byte[SALT_BYTE_SIZE]; + random.nextBytes(salt); + + // Hash the password + byte[] hash = pbkdf2(password, salt, PBKDF2_ITERATIONS, HASH_BYTE_SIZE); + + return new HashingResult(PBKDF2_ITERATIONS, toHex(hash), toHex(salt)); + } + + /** + * Validates a password using a hash. + * + * @param password + * the password to check + * @param correctHash + * the hash of the valid password + * @return true if the password is correct, false if not + */ + public static boolean validatePassword(String password, String correctHash) + throws NoSuchAlgorithmException, InvalidKeySpecException { + return validatePassword(password.toCharArray(), correctHash); + } + + /** + * Validates a password using a hash. + * + * @param password + * the password to check + * @param correctHash + * the hash of the valid password + * @return true if the password is correct, false if not + */ + public static boolean validatePassword(char[] password, String correctHash) + throws NoSuchAlgorithmException, InvalidKeySpecException { + // Decode the hash into its parameters + String[] params = correctHash.split(":"); + int iterations = Integer.parseInt(params[ITERATION_INDEX]); + return validatePassword(password, iterations, params[SALT_INDEX], params[PBKDF2_INDEX]); + } + + public static boolean validatePassword(char[] password, int iterations, + String saltHex, String hashHex) { + byte[] salt = fromHex(saltHex); + byte[] hash = fromHex(hashHex); + // Compute the hash of the provided password, using the same salt, + // iteration count, and hash length + byte[] testHash = pbkdf2(password, salt, iterations, hash.length); + // Compare the hashes in constant time. The password is correct if + // both hashes match. + return slowEquals(hash, testHash); + } + + + + + public static boolean slowEquals(String hash1, String hash2) { + return slowEquals(fromHex(hash1), fromHex(hash2)); + } + + /** + * Compares two byte arrays in length-constant time. This comparison method + * is used so that password hashes cannot be extracted from an on-line + * system using a timing attack and then attacked off-line. + * + * @param a + * the first byte array + * @param b + * the second byte array + * @return true if both byte arrays are the same, false if not + */ + private static boolean slowEquals(byte[] a, byte[] b) { + int diff = a.length ^ b.length; + for (int i = 0; i < a.length && i < b.length; i++) + diff |= a[i] ^ b[i]; + return diff == 0; + } + + /** + * Computes the PBKDF2 hash of a password. + * + * @param password + * the password to hash. + * @param salt + * the salt + * @param iterations + * the iteration count (slowness factor) + * @param bytes + * the length of the hash to compute in bytes + * @return the PBDKF2 hash of the password + */ + private static byte[] pbkdf2(char[] password, byte[] salt, int iterations, + int bytes) { + try { + PBEKeySpec spec = new PBEKeySpec(password, salt, iterations, + bytes * 8); + SecretKeyFactory skf = SecretKeyFactory + .getInstance(PBKDF2_ALGORITHM); + return skf.generateSecret(spec).getEncoded(); + } catch (NoSuchAlgorithmException e) { + throw new RuntimeException(e); + } catch (InvalidKeySpecException e) { + throw new RuntimeException(e); + } + } + + /** + * Converts a string of hexadecimal characters into a byte array. + * + * @param hex + * the hex string + * @return the hex string decoded into a byte array + */ + private static byte[] fromHex(String hex) { + byte[] binary = new byte[hex.length() / 2]; + for (int i = 0; i < binary.length; i++) { + binary[i] = (byte) Integer.parseInt( + hex.substring(2 * i, 2 * i + 2), 16); + } + return binary; + } + + /** + * Converts a byte array into a hexadecimal string. + * + * @param array + * the byte array to convert + * @return a length*2 character string encoding the byte array + */ + private static String toHex(byte[] array) { + BigInteger bi = new BigInteger(1, array); + String hex = bi.toString(16); + int paddingLength = (array.length * 2) - hex.length(); + if (paddingLength > 0) + return String.format("%0" + paddingLength + "d", 0) + hex; + else + return hex; + } + + + +} \ No newline at end of file diff --git a/src/org/traccar/http/MainServlet.java b/src/org/traccar/http/MainServlet.java index 18430f0c3..cf6e81286 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); + Context.getDataManager().addUser(user, user.getPassword()); sendResponse(resp.getWriter(), true); } diff --git a/src/org/traccar/http/UserServlet.java b/src/org/traccar/http/UserServlet.java index f388326b0..197ef0326 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); + Context.getDataManager().addUser(user, user.getPassword()); 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); + Context.getDataManager().updateUser(user, user.getPassword()); sendResponse(resp.getWriter(), true); } diff --git a/src/org/traccar/model/User.java b/src/org/traccar/model/User.java index 410bc4d74..fa09861ed 100644 --- a/src/org/traccar/model/User.java +++ b/src/org/traccar/model/User.java @@ -15,7 +15,8 @@ */ package org.traccar.model; -import org.traccar.helper.Hashing; +import org.traccar.helper.PasswordHash; +import org.traccar.helper.PasswordHash.HashingResult; public class User implements Factory { @@ -36,10 +37,15 @@ public class User implements Factory { public String getEmail() { return email; } public void setEmail(String email) { this.email = email; } - private byte[] password; - public byte[] getPassword() { return password; } - public void setPassword(String password) { this.password = Hashing.sha256(password); } + private String password; + public String getPassword() { return password; } + public void setPassword(String password) { + this.password = password; + } + private String salt; + public String getSalt() { return salt; } + public void setSalt(String salt) { this.salt = salt; } private boolean readonly; private boolean admin; @@ -59,4 +65,14 @@ public class User implements Factory { private double longitude; private int zoom; + + public boolean isPasswordValid(String inputPassword) { + return PasswordHash.validatePassword(inputPassword.toCharArray(), PasswordHash.PBKDF2_ITERATIONS, this.salt, this.password); + } + + public void hashPassword(String password) { + HashingResult hashingResult = PasswordHash.createHash(password); + this.password = hashingResult.hash; + this.salt = hashingResult.salt; + } } -- 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 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