Adding password salting

- When A UserConfig object is created, generate its 64 bit long salt
  and use it to salt the clear text password before hashing.

Change-Id: I2ddc5ffa061cdef59d01caa6d98cb6eb4b7be014
Signed-off-by: Alessandro Boch <aboch@cisco.com>

diff --git a/opendaylight/usermanager/api/src/main/java/org/opendaylight/controller/usermanager/UserConfig.java b/opendaylight/usermanager/api/src/main/java/org/opendaylight/controller/usermanager/UserConfig.java
index 0c14dea38a4a9ff69993d1e6a88503ceb5db0792..83532a1012f6a9431cd4f75a3f1b201e653ec395 100644 (file)
--- a/opendaylight/usermanager/api/src/main/java/org/opendaylight/controller/usermanager/UserConfig.java
+++ b/opendaylight/usermanager/api/src/main/java/org/opendaylight/controller/usermanager/UserConfig.java
@@ -9,10 +9,11 @@
 package org.opendaylight.controller.usermanager;
 
 import java.io.Serializable;
 package org.opendaylight.controller.usermanager;
 
 import java.io.Serializable;

-import java.nio.charset.Charset;

 import java.security.MessageDigest;
 import java.security.NoSuchAlgorithmException;
 import java.security.MessageDigest;
 import java.security.NoSuchAlgorithmException;

+import java.security.SecureRandom;

 import java.util.ArrayList;
 import java.util.ArrayList;

+import java.util.Collections;

 import java.util.Iterator;
 import java.util.List;
 import java.util.regex.Matcher;
 import java.util.Iterator;
 import java.util.List;
 import java.util.regex.Matcher;


@@ -24,6 +25,7 @@ import javax.xml.bind.annotation.XmlElement;
 import javax.xml.bind.annotation.XmlRootElement;
 
 import org.opendaylight.controller.sal.authorization.AuthResultEnum;
 import javax.xml.bind.annotation.XmlRootElement;
 
 import org.opendaylight.controller.sal.authorization.AuthResultEnum;

+import org.opendaylight.controller.sal.packet.BitBufferHelper;

 import org.opendaylight.controller.sal.utils.HexEncode;
 import org.opendaylight.controller.sal.utils.Status;
 import org.opendaylight.controller.sal.utils.StatusCode;
 import org.opendaylight.controller.sal.utils.HexEncode;
 import org.opendaylight.controller.sal.utils.Status;
 import org.opendaylight.controller.sal.utils.StatusCode;


@@ -46,6 +48,7 @@ public class UserConfig implements Serializable {
     protected static final String PASSWORD_REGEX = "(?=.*[^a-zA-Z0-9])(?=.*\\d)(?=.*[a-z])(?=.*[A-Z]).{8,256}$";
     private static final Pattern INVALID_USERNAME_CHARACTERS = Pattern.compile("([/\\s\\.\\?#%;\\\\]+)");
     private static MessageDigest oneWayFunction;
     protected static final String PASSWORD_REGEX = "(?=.*[^a-zA-Z0-9])(?=.*\\d)(?=.*[a-z])(?=.*[A-Z]).{8,256}$";
     private static final Pattern INVALID_USERNAME_CHARACTERS = Pattern.compile("([/\\s\\.\\?#%;\\\\]+)");
     private static MessageDigest oneWayFunction;

+    private static SecureRandom randomGenerator;

 
     static {
         try {
 
     static {
         try {


@@ -54,6 +57,7 @@ public class UserConfig implements Serializable {
             log.error(String.format("Implementation of %s digest algorithm not found: %s", DIGEST_ALGORITHM,
                     e.getMessage()));
         }
             log.error(String.format("Implementation of %s digest algorithm not found: %s", DIGEST_ALGORITHM,
                     e.getMessage()));
         }

+        UserConfig.randomGenerator = new SecureRandom(BitBufferHelper.toByteArray(System.currentTimeMillis()));

     }
 
     /**
     }
 
     /**


@@ -81,6 +85,8 @@ public class UserConfig implements Serializable {
     @XmlElement
     private String password;
 
     @XmlElement
     private String password;
 

+    private byte[] salt;
+

 
 
     public UserConfig() {
 
 
     public UserConfig() {


@@ -102,11 +108,19 @@ public class UserConfig implements Serializable {
         /*
          * Password validation to be done on clear text password. If fails, mark
          * the password with a well known label, so that object validation can
         /*
          * Password validation to be done on clear text password. If fails, mark
          * the password with a well known label, so that object validation can

-         * report the proper error. Only if password is a valid one, hash it.
+         * report the proper error. Only if password is a valid one, generate
+         * salt, concatenate it with clear text password and hash the
+         * resulting string. Hash result is going to be our stored password.

          */
          */

-        this.password = (validatePassword(password).isSuccess()) ? hash(password) : BAD_PASSWORD;
+        if (validateClearTextPassword(password).isSuccess()) {
+            this.salt = BitBufferHelper.toByteArray(randomGenerator.nextLong());
+            this.password = hash(salt, password);
+        } else {
+            this.salt = null;
+            this.password = BAD_PASSWORD;
+        }

 
 

-        this.roles = (roles == null) ? new ArrayList<String>() : new ArrayList<String>(roles);
+        this.roles = (roles == null) ? Collections.<String>emptyList() : new ArrayList<String>(roles);

     }
 
     public String getUser() {
     }
 
     public String getUser() {


@@ -176,6 +190,7 @@ public class UserConfig implements Serializable {
     public Status validate() {
         Status validCheck = validateUsername();
         if (validCheck.isSuccess()) {
     public Status validate() {
         Status validCheck = validateUsername();
         if (validCheck.isSuccess()) {

+            // Password validation was run at object construction time

             validCheck = (!password.equals(BAD_PASSWORD)) ? new Status(StatusCode.SUCCESS) : new Status(
                     StatusCode.BADREQUEST,
                     "Password should be 8 to 256 characters long, contain both upper and lower case letters, "
             validCheck = (!password.equals(BAD_PASSWORD)) ? new Status(StatusCode.SUCCESS) : new Status(
                     StatusCode.BADREQUEST,
                     "Password should be 8 to 256 characters long, contain both upper and lower case letters, "


@@ -203,7 +218,7 @@ public class UserConfig implements Serializable {
         return new Status(StatusCode.SUCCESS);
     }
 
         return new Status(StatusCode.SUCCESS);
     }
 

-    private Status validatePassword(String password) {
+    private Status validateClearTextPassword(String password) {

         if (password == null || password.isEmpty()) {
             return new Status(StatusCode.BADREQUEST, "Password cannot be empty");
         }
         if (password == null || password.isEmpty()) {
             return new Status(StatusCode.BADREQUEST, "Password cannot be empty");
         }


@@ -227,14 +242,14 @@ public class UserConfig implements Serializable {
 
         // To make any changes to a user configured profile, current password
         // must always be provided
 
         // To make any changes to a user configured profile, current password
         // must always be provided

-        if (!this.password.equals(hash(currentPassword))) {
+        if (!this.password.equals(hash(this.salt, currentPassword))) {

             return new Status(StatusCode.BADREQUEST, "Current password is incorrect");
         }
 
         // Create a new object with the proposed modifications
         UserConfig proposed = new UserConfig();
         proposed.user = this.user;
             return new Status(StatusCode.BADREQUEST, "Current password is incorrect");
         }
 
         // Create a new object with the proposed modifications
         UserConfig proposed = new UserConfig();
         proposed.user = this.user;

-        proposed.password = (newPassword == null)? this.password : hash(newPassword);
+        proposed.password = (newPassword == null)? this.password : hash(this.salt, newPassword);

         proposed.roles = (newRoles == null)? this.roles : newRoles;
 
         // Validate it
         proposed.roles = (newRoles == null)? this.roles : newRoles;
 
         // Validate it


@@ -251,9 +266,9 @@ public class UserConfig implements Serializable {
         return status;
     }
 
         return status;
     }
 

-    public AuthResponse authenticate(String clearTextPass) {
+    public AuthResponse authenticate(String clearTextPassword) {

         AuthResponse locResponse = new AuthResponse();
         AuthResponse locResponse = new AuthResponse();

-        if (password.equals(hash(clearTextPass))) {
+        if (password.equals(hash(this.salt, clearTextPassword))) {

             locResponse.setStatus(AuthResultEnum.AUTH_ACCEPT_LOC);
             locResponse.addData(getRolesString());
         } else {
             locResponse.setStatus(AuthResultEnum.AUTH_ACCEPT_LOC);
             locResponse.addData(getRolesString());
         } else {


@@ -275,12 +290,32 @@ public class UserConfig implements Serializable {
         return buffer.toString();
     }
 
         return buffer.toString();
     }
 

-    public static String hash(String message) {
+    private static byte[] concatenate(byte[] salt, String password) {
+        byte[] messageArray = password.getBytes();
+        byte[] concatenation = new byte[salt.length + password.length()];
+        System.arraycopy(salt, 0, concatenation, 0, salt.length);
+        System.arraycopy(messageArray, 0, concatenation, salt.length, messageArray.length);
+        return concatenation;
+    }
+
+    private static String hash(byte[] salt, String message) {

         if (message == null) {
         if (message == null) {

+            log.warn("Password hash requested but empty or no password provided");

             return message;
         }
             return message;
         }

+        if (salt == null || salt.length == 0) {
+            log.warn("Password hash requested but empty or no salt provided");
+            return message;
+        }
+
+        // Concatenate salt and password
+        byte[] messageArray = message.getBytes();
+        byte[] concatenation = new byte[salt.length + message.length()];
+        System.arraycopy(salt, 0, concatenation, 0, salt.length);
+        System.arraycopy(messageArray, 0, concatenation, salt.length, messageArray.length);
+

         UserConfig.oneWayFunction.reset();
         UserConfig.oneWayFunction.reset();

-        return HexEncode.bytesToHexString(UserConfig.oneWayFunction.digest(message.getBytes(Charset.defaultCharset())));
+        return HexEncode.bytesToHexString(UserConfig.oneWayFunction.digest(concatenate(salt, message)));

     }
 
     /**
     }
 
     /**


@@ -299,7 +334,8 @@ public class UserConfig implements Serializable {
     public static UserConfig getUncheckedUserConfig(String userName, String password, List<String> roles) {
         UserConfig config = new UserConfig();
         config.user = userName;
     public static UserConfig getUncheckedUserConfig(String userName, String password, List<String> roles) {
         UserConfig config = new UserConfig();
         config.user = userName;

-        config.password = hash(password);
+        config.salt = BitBufferHelper.toByteArray(randomGenerator.nextLong());
+        config.password = hash(config.salt, password);

         config.roles = roles;
         return config;
     }
         config.roles = roles;
         return config;
     }

diff --git a/opendaylight/usermanager/api/src/test/java/org/opendaylight/controller/usermanager/AuthorizationUserConfigTest.java b/opendaylight/usermanager/api/src/test/java/org/opendaylight/controller/usermanager/AuthorizationUserConfigTest.java
index d225e5c76014bfc2e647ceeb0fbff756de4065ba..8fc7f07df1db2fc3283057c1f55caed85799d604 100644 (file)
--- a/opendaylight/usermanager/api/src/test/java/org/opendaylight/controller/usermanager/AuthorizationUserConfigTest.java
+++ b/opendaylight/usermanager/api/src/test/java/org/opendaylight/controller/usermanager/AuthorizationUserConfigTest.java
@@ -7,7 +7,6 @@
  */
 package org.opendaylight.controller.usermanager;
 
  */
 package org.opendaylight.controller.usermanager;
 

-import static org.junit.Assert.assertEquals;

 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
 
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
 


@@ -71,12 +70,12 @@ public class AuthorizationUserConfigTest {
                 .isSuccess());
 
         // New Password = null, No change in password
                 .isSuccess());
 
         // New Password = null, No change in password

-        assertTrue(userConfig.getPassword().equals(UserConfig.hash("ciscocisco")));
+        assertTrue(userConfig.authenticate("ciscocisco").getStatus().equals(AuthResultEnum.AUTH_ACCEPT_LOC));

 
         // Password changed successfully, no change in user role
         assertTrue(userConfig.update("ciscocisco", "cisco123", roles)
                 .isSuccess());
 
         // Password changed successfully, no change in user role
         assertTrue(userConfig.update("ciscocisco", "cisco123", roles)
                 .isSuccess());

-        assertTrue(userConfig.getPassword().equals(UserConfig.hash("cisco123")));
+        assertTrue(userConfig.authenticate("cisco123").getStatus().equals(AuthResultEnum.AUTH_ACCEPT_LOC));

         assertTrue(userConfig.getRoles().get(0).equals(
                 UserLevel.NETWORKOPERATOR.toString()));
 
         assertTrue(userConfig.getRoles().get(0).equals(
                 UserLevel.NETWORKOPERATOR.toString()));
 


@@ -85,14 +84,14 @@ public class AuthorizationUserConfigTest {
         roles.add(UserLevel.SYSTEMADMIN.toString());
         assertTrue(userConfig.update("cisco123", "cisco123", roles)
                 .isSuccess());
         roles.add(UserLevel.SYSTEMADMIN.toString());
         assertTrue(userConfig.update("cisco123", "cisco123", roles)
                 .isSuccess());

-        assertTrue(userConfig.getPassword().equals(UserConfig.hash("cisco123")));
+        assertTrue(userConfig.authenticate("cisco123").getStatus().equals(AuthResultEnum.AUTH_ACCEPT_LOC));

         assertTrue(userConfig.getRoles().get(0)
                 .equals(UserLevel.SYSTEMADMIN.toString()));
 
         // Password and role changed successfully
         assertTrue(userConfig.update("cisco123", "ciscocisco", roles)
                 .isSuccess());
         assertTrue(userConfig.getRoles().get(0)
                 .equals(UserLevel.SYSTEMADMIN.toString()));
 
         // Password and role changed successfully
         assertTrue(userConfig.update("cisco123", "ciscocisco", roles)
                 .isSuccess());

-        assertTrue(userConfig.getPassword().equals(UserConfig.hash("ciscocisco")));
+        assertTrue(userConfig.authenticate("ciscocisco").getStatus().equals(AuthResultEnum.AUTH_ACCEPT_LOC));

         assertTrue(userConfig.getRoles().get(0)
                 .equals(UserLevel.SYSTEMADMIN.toString()));
 
         assertTrue(userConfig.getRoles().get(0)
                 .equals(UserLevel.SYSTEMADMIN.toString()));
 


@@ -104,14 +103,6 @@ public class AuthorizationUserConfigTest {
         assertTrue(authresp.getStatus().equals(AuthResultEnum.AUTH_ACCEPT_LOC));
         authresp = userConfig.authenticate("wrongPassword");
         assertTrue(authresp.getStatus().equals(AuthResultEnum.AUTH_REJECT_LOC));
         assertTrue(authresp.getStatus().equals(AuthResultEnum.AUTH_ACCEPT_LOC));
         authresp = userConfig.authenticate("wrongPassword");
         assertTrue(authresp.getStatus().equals(AuthResultEnum.AUTH_REJECT_LOC));

-
-        // test equals()
-        roles.clear();
-        roles.add(UserLevel.NETWORKOPERATOR.toString());
-        userConfig = new UserConfig("uname", "ciscocisco", roles);
-        assertEquals(userConfig, userConfig);
-        UserConfig userConfig2 = new UserConfig("uname", "ciscocisco", roles);
-        assertEquals(userConfig, userConfig2);

     }
 
     @Test
     }
 
     @Test