[GitHub] [shiro] bdemers commented on a change in pull request #12: Implement SaltStyle.CRYPT

[GitBox <gi...@apache.org>]

bdemers commented on a change in pull request #12:
URL: https://github.com/apache/shiro/pull/12#discussion_r657190549



##########
File path: core/src/main/java/org/apache/shiro/realm/jdbc/JdbcRealm.java
##########
@@ -227,9 +241,72 @@ protected AuthenticationInfo doGetAuthenticationInfo(AuthenticationToken token)
                 password = getPasswordForUser(conn, username)[0];
                 break;
             case CRYPT:
-                // TODO: separate password and hash from getPasswordForUser[0]
-                throw new ConfigurationException("Not implemented yet");
-                //break;
+                /*
+                http://www.slashroot.in/how-are-passwords-stored-linux-understanding-hashing-shadow-utils
+                
+                Example: $1$Etg2ExUZ$F9NTP7omafhKIlqaBMqng1
+
+                The above shown encoded hash value can be further classified into three different fields as below.
+                1. The first field is a numerical number that tell's you the hashing algorithm that's being used.
+
+                $1 = MD5 hashing algorithm.
+                $2 =Blowfish Algorithm is in use.
+                $2a=eksblowfish Algorithm
+                $5 =SHA-256 Algorithm
+                $6 =SHA-512 Algorithm
+
+                2. The second field is the salt value
+                Salt value is nothing but a random data that's generated to combine with the original password, inorder to increase the strength of the hash..
+
+                3.The last field is the hash value of salt+user password (we will be discussing this shortly).
+                
+                */
+                
+                String[] crypt=getPasswordForUser(conn, username)[0].split("\\$");
+                CredentialsMatcher credentialsMatcher = getCredentialsMatcher();
+                if (credentialsMatcher instanceof HashedCredentialsMatcher) {
+                    HashedCredentialsMatcher hashedCredentialsMatcher=(HashedCredentialsMatcher) credentialsMatcher;
+                            
+                    switch (crypt.length) {
+                        // hash algorithm is not set
+                        case 3:
+                            
+                            // Hex decoding is ugly and should not be used really    
+                            salt=hashedCredentialsMatcher.isStoredCredentialsHexEncoded()
+                                    ? new String(Hex.decode(crypt[1]))
+                                    : Base64.decodeToString(crypt[1]);
+                            password=crypt[2];
+                            break;
+                        
+                        // hash algorithm is set
+                        case 4:
+                            String hashAlgorithm=crypt[1];
+                                if (hashAlgorithm.equals("6")) 
+                                    hashedCredentialsMatcher.setHashAlgorithmName(Sha512Hash.ALGORITHM_NAME);
+                                else if (hashAlgorithm.equals("5")) 
+                                    hashedCredentialsMatcher.setHashAlgorithmName(Sha256Hash.ALGORITHM_NAME);
+                                else if (hashAlgorithm.equals("1")) 
+                                    hashedCredentialsMatcher.setHashAlgorithmName(Md5Hash.ALGORITHM_NAME);
+                                else if (hashAlgorithm.equals("2")) 
+                                    throw new AuthenticationException("Requested 'Blowfish' algorithm is not supported. Can not validate the token.");
+                                else if (hashAlgorithm.equals("2a")) 
+                                    throw new AuthenticationException("Requested 'eksblowfish' algorithm is not supported. Can not validate the token.");
+
+                                setCredentialsMatcher(credentialsMatcher);

Review comment:
       Hey @manticore-projects, thanks again 😉 
   
   The gist is that this method `doGetAuthenticationInfo` will be called from multiple threads, so updating the credentials matcher could affect other threads (e.g. two threads from two different user requests could both update the `credentialsMatcher`). 
   
   Ideally, the `credentialsMatcher` would be immutable, but Shiro allows for lazy initialization so we need to watch out for these types of issues.
   
   Does that help?




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org