Skip to content

Commit

Permalink
add constant time compare function to mitigate timing attacks
Browse files Browse the repository at this point in the history
  • Loading branch information
allenp authored and Paul Allen committed Oct 9, 2013
1 parent 8d107c8 commit de6aa4e
Show file tree
Hide file tree
Showing 4 changed files with 46 additions and 3 deletions.
3 changes: 2 additions & 1 deletion README.md
Expand Up @@ -33,7 +33,8 @@ Hash Password Example:
//validate user
//compare the password (this should be true since we are rehashing the same password and using the same generated salt)
bool isPasswordValid = cryptoService.Compute(password, salt) == hashedPassword;
string hashedPassword2 = cryptoService.Compute(password, salt);
bool isPasswordValid = cryptoService.Compare(hashedPassword, hashedPassword2);

Generate Random Password Example:

Expand Down
7 changes: 7 additions & 0 deletions src/Interfaces/ICryptoService.cs
Expand Up @@ -86,5 +86,12 @@ public interface ICryptoService
/// <param name="iteration"></param>
/// <returns></returns>
int GetElapsedTimeForIteration(int iteration);

/// <summary>
/// Compare the passwords for equality
/// <param name="passwordHash1">The first password hash to compare</param>
/// <param name="passwordHash2">The second password hash to compare</param>
/// <returns>true: indicating the password hashes are the same, false otherwise.</param>
bool Compare(string passwordHash1, string passwordHash2);
}
}
20 changes: 19 additions & 1 deletion src/PBKDF2.cs
Expand Up @@ -208,6 +208,24 @@ private void expandSalt()
}
}


/// <summary>
/// Compare password hashes for equality. Uses a constant time comparison method.
/// </summary>
/// <param name="passwordHash1"></param>
/// <param name="passwordHash2"></param>
/// <returns></returns>
public bool Compare(string passwordHash1, string passwordHash2)
{
if (passwordHash1 == null || passwordHash2 == null)
return false;

int min_length = Math.Min(passwordHash1.Length, passwordHash2.Length);
int result = 0;

for(int i = 0; i < min_length; i++)
result |= passwordHash1[i] ^ passwordHash2[i];

return 0 == result;
}
}
}
19 changes: 18 additions & 1 deletion tests/PBKDF2Tests.cs
@@ -1,4 +1,5 @@
using System;
using System;
using System.Diagnostics;
using NUnit.Framework;

namespace SimpleCrypto.Tests
Expand Down Expand Up @@ -164,5 +165,21 @@ public void Generate_Salt_Sets_Salt_Property()

Assert.AreEqual(crypto.Salt, salt, "The returned salt is not the salt set as parameter");
}

[Test]
public void Comparison_To_Same_Hash_Returns_True()
{
var crypto = CreateICryptoService();

var hash1 = crypto.Compute("password");
var salt = crypto.Salt;

var hash2 = crypto.Compute("password", salt);

Assert.IsTrue(crypto.Compare(hash1, hash2), "Hash comparison fails");

var hash3 = crypto.Compute("pasw1rd", salt);
Assert.IsFalse(crypto.Compare(hash1, hash3), "Hash comparison should fail but didn't");
}
}
}

2 comments on commit de6aa4e

@mikegoatly
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's a bug in the Compare method - it will return true for hashes that are different in length, but start with the same substring. E.g. comparing "abcde" with "abcdefghij" will return true...

@allenp
Copy link
Contributor Author

@allenp allenp commented on de6aa4e Jul 22, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could fix with

if (passwordHash1.Length != passwordHash2.Length)
  return false;

after the null check

Please sign in to comment.