[jira] [Commented] (SHIRO-458) Possible leaked timing information from DefaultPasswordService

["ASF GitHub Bot (JIRA)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/SHIRO-458?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16028815#comment-16028815 ] 

ASF GitHub Bot commented on SHIRO-458:
--------------------------------------

GitHub user ddold opened a pull request:

    https://github.com/apache/shiro/pull/65

    Replaced string equals with internal method that does not leak time

    Followed the steps as described in the ticket https://issues.apache.org/jira/browse/SHIRO-458
    Replaced the string equals with an internal equals method.
    I understand why the need to make this change but I am not sure of the implementation of the steps done within the for loop. I ran the unit tests and they have all passed

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/ddold/shiro shiro-458

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/shiro/pull/65.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #65
    
----
commit d7214d8b9cee3e0386ddbcd7f2afeb8112057af2
Author: Dan Dold <do...@gmail.com>
Date:   2017-05-30T07:39:13Z

    Replaced string equals with internal method that does not leak time

----


> Possible leaked timing information from DefaultPasswordService
> --------------------------------------------------------------
>
>                 Key: SHIRO-458
>                 URL: https://issues.apache.org/jira/browse/SHIRO-458
>             Project: Shiro
>          Issue Type: Bug
>          Components: Cryptography & Hashing
>    Affects Versions: 1.2.2
>         Environment: Mac OS X 10.8.3, Java 1.6.0_51
>            Reporter: Stuart Broad
>            Priority: Trivial
>
> Use of the String equals comparison for the password hash comparison could leak timing information since it returns false as soon a character does not match.
> DefaultPasswordService>>passwordsMatch(Object submittedPlaintext, String saved)
> Last line is:
> return saved.equals(formatted); //saved and formatted are strings
> A possible constant time equals could be:
>     private boolean constantEquals(String s1, String s2)
>     {
>         /*
>          * Alternative option (simpler but I'm not sure about the intern 'cost'):
>          * s1.intern();
>          * s2.intern();
>          * s1 == s2
>          */
>         int result = 0;
>         byte[] a = s1.getBytes();
>         byte[] b = s2.getBytes();
>         // Also leaks timing information but probably ok...
>         if (a.length != b.length) {
>             return false;
>         }
>         /*
>          * XOR each byte.  If each byte is the
>          * same the XOR will result in 0.
>          */
>         for (int i = 0; i < a.length; i++) {
>             result |= a[i] ^ b[i];
>         }
>         return result == 0;
>     }



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)