-
Notifications
You must be signed in to change notification settings - Fork 262
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
ELY-2435; ELY-2434; ELY-2433; ELY-2432 #1775
Conversation
…al to avoid a potential timing attack
…Digest#isEqual to avoid a potential timing attack
…isEqual to avoid a potential timing attack
…t#isEqual to avoid a potential timing attack
@@ -56,6 +57,7 @@ public boolean equals(final Object obj) { | |||
return false; | |||
} | |||
RawSaltedSimpleDigestPassword other = (RawSaltedSimpleDigestPassword) obj; | |||
return getAlgorithm().equals(other.getAlgorithm()) && Arrays.equals(digest, other.digest) && Arrays.equals(salt, other.salt); | |||
return getAlgorithm().equals(other.getAlgorithm()) && MessageDigest.isEqual(digest, other.digest) | |||
&& MessageDigest.isEqual(salt, other.salt); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a very small comment, we can continue to use Arrays.equals
for the salt comparison.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cool.. will update 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For reference: reason to not change this is since salt is not user input, it is not susceptible to timing attack
} | ||
|
||
public boolean equals(final Object obj) { | ||
if (! (obj instanceof RawScramDigestPassword)) { | ||
return false; | ||
} | ||
RawScramDigestPassword other = (RawScramDigestPassword) obj; | ||
return iterationCount == other.iterationCount && getAlgorithm().equals(other.getAlgorithm()) && Arrays.equals(digest, other.digest) && Arrays.equals(salt, other.salt); | ||
return iterationCount == other.iterationCount && getAlgorithm().equals(other.getAlgorithm()) | ||
&& MessageDigest.isEqual(digest, other.digest) && MessageDigest.isEqual(salt, other.salt); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a very small comment, we can continue to use Arrays.equals for the salt comparison.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @keyasarkar!
Refer issues below:
ELY-2435
ELY-2434
ELY-2433
ELY-2432