Skip to main content
Code Review

Return to Answer

replaced http://security.stackexchange.com/ with https://security.stackexchange.com/
Source Link

Your code has few flaws:

  • You decrypting password - if someone gain access to your source code he can brake all your passwords with no effort. All you need to do is verify, not know it. Instead of decrypting it just check hash of provided password matches.
  • Have one salt for every password / user. It should be unique.
  • Doesn't have random seed.
  • Salt is too short - there is no reason for shortening it since nobody have to remember it.
  • Uses MD5 as hashing algorithm. You should move to PBKDF2 (see RFC2898)
  • Have only one iteration of hashing function - for example LastPass uses 100k rounds of hashing. (read more here read more here)

You need 3 methods:

  1. For computing hash.

  2. For generating salt.

  3. For checking if password hash matches.

    using System.Web.Security;
    using System.Security.Cryptography;
    namespace RijndaelEncDec{
    public static class PasswordHasher
    {
    // 24 = 192 bits
    private const int SaltByteSize = 24;
    private const int HashByteSize = 24;
    private const int HasingIterationsCount = 10101;
    public static byte[] ComputeHash(string password, byte[] salt, int iterations=HasingIterationsCount, int hashByteSize=HashByteSize)
    {
     Rfc2898DeriveBytes hashGenerator = new Rfc2898DeriveBytes(password, salt);
     hashGenerator.IterationCount = iterations;
     return hashGenerator.GetBytes(hashByteSize);
    }
    public static byte[] GenerateSalt(int saltByteSize=SaltByteSize)
    {
     RNGCryptoServiceProvider saltGenerator = new RNGCryptoServiceProvider();
     byte[] salt = new byte[saltByteSize];
     saltGenerator.GetBytes(salt);
     return salt;
    }
    public static bool VerifyPassword(String password, byte[] passwordSalt, byte[] passwordHash)
    {
     byte[] computedHash = ComputeHash(password, passwordSalt);
     return AreHashesEqual(computedHash, passwordHash);
    }
    //Length constant verification - prevents timing attack
    private static bool AreHashesEqual(byte[] firstHash, byte[] secondHash)
    { 
     int minHashLenght = firstHash.Length <= secondHash.Length ? firstHash.Length : secondHash.Length;
     var xor = firstHash.Length ^ secondHash.Length;
     for (int i = 0; i < minHashLenght; i++)
     xor |= firstHash[i] ^ secondHash[i];
     return 0 == xor;
    }
    } 
    }
    

Unfortunately you will have to change your interface a little...

Your code has few flaws:

  • You decrypting password - if someone gain access to your source code he can brake all your passwords with no effort. All you need to do is verify, not know it. Instead of decrypting it just check hash of provided password matches.
  • Have one salt for every password / user. It should be unique.
  • Doesn't have random seed.
  • Salt is too short - there is no reason for shortening it since nobody have to remember it.
  • Uses MD5 as hashing algorithm. You should move to PBKDF2 (see RFC2898)
  • Have only one iteration of hashing function - for example LastPass uses 100k rounds of hashing. (read more here)

You need 3 methods:

  1. For computing hash.

  2. For generating salt.

  3. For checking if password hash matches.

    using System.Web.Security;
    using System.Security.Cryptography;
    namespace RijndaelEncDec{
    public static class PasswordHasher
    {
    // 24 = 192 bits
    private const int SaltByteSize = 24;
    private const int HashByteSize = 24;
    private const int HasingIterationsCount = 10101;
    public static byte[] ComputeHash(string password, byte[] salt, int iterations=HasingIterationsCount, int hashByteSize=HashByteSize)
    {
     Rfc2898DeriveBytes hashGenerator = new Rfc2898DeriveBytes(password, salt);
     hashGenerator.IterationCount = iterations;
     return hashGenerator.GetBytes(hashByteSize);
    }
    public static byte[] GenerateSalt(int saltByteSize=SaltByteSize)
    {
     RNGCryptoServiceProvider saltGenerator = new RNGCryptoServiceProvider();
     byte[] salt = new byte[saltByteSize];
     saltGenerator.GetBytes(salt);
     return salt;
    }
    public static bool VerifyPassword(String password, byte[] passwordSalt, byte[] passwordHash)
    {
     byte[] computedHash = ComputeHash(password, passwordSalt);
     return AreHashesEqual(computedHash, passwordHash);
    }
    //Length constant verification - prevents timing attack
    private static bool AreHashesEqual(byte[] firstHash, byte[] secondHash)
    { 
     int minHashLenght = firstHash.Length <= secondHash.Length ? firstHash.Length : secondHash.Length;
     var xor = firstHash.Length ^ secondHash.Length;
     for (int i = 0; i < minHashLenght; i++)
     xor |= firstHash[i] ^ secondHash[i];
     return 0 == xor;
    }
    } 
    }
    

Unfortunately you will have to change your interface a little...

Your code has few flaws:

  • You decrypting password - if someone gain access to your source code he can brake all your passwords with no effort. All you need to do is verify, not know it. Instead of decrypting it just check hash of provided password matches.
  • Have one salt for every password / user. It should be unique.
  • Doesn't have random seed.
  • Salt is too short - there is no reason for shortening it since nobody have to remember it.
  • Uses MD5 as hashing algorithm. You should move to PBKDF2 (see RFC2898)
  • Have only one iteration of hashing function - for example LastPass uses 100k rounds of hashing. (read more here)

You need 3 methods:

  1. For computing hash.

  2. For generating salt.

  3. For checking if password hash matches.

    using System.Web.Security;
    using System.Security.Cryptography;
    namespace RijndaelEncDec{
    public static class PasswordHasher
    {
    // 24 = 192 bits
    private const int SaltByteSize = 24;
    private const int HashByteSize = 24;
    private const int HasingIterationsCount = 10101;
    public static byte[] ComputeHash(string password, byte[] salt, int iterations=HasingIterationsCount, int hashByteSize=HashByteSize)
    {
     Rfc2898DeriveBytes hashGenerator = new Rfc2898DeriveBytes(password, salt);
     hashGenerator.IterationCount = iterations;
     return hashGenerator.GetBytes(hashByteSize);
    }
    public static byte[] GenerateSalt(int saltByteSize=SaltByteSize)
    {
     RNGCryptoServiceProvider saltGenerator = new RNGCryptoServiceProvider();
     byte[] salt = new byte[saltByteSize];
     saltGenerator.GetBytes(salt);
     return salt;
    }
    public static bool VerifyPassword(String password, byte[] passwordSalt, byte[] passwordHash)
    {
     byte[] computedHash = ComputeHash(password, passwordSalt);
     return AreHashesEqual(computedHash, passwordHash);
    }
    //Length constant verification - prevents timing attack
    private static bool AreHashesEqual(byte[] firstHash, byte[] secondHash)
    { 
     int minHashLenght = firstHash.Length <= secondHash.Length ? firstHash.Length : secondHash.Length;
     var xor = firstHash.Length ^ secondHash.Length;
     for (int i = 0; i < minHashLenght; i++)
     xor |= firstHash[i] ^ secondHash[i];
     return 0 == xor;
    }
    } 
    }
    

Unfortunately you will have to change your interface a little...

deleted 5 characters in body
Source Link

Your code has few flaws:

  • You decrypting password - if someone gain access to your source code he can brake all your passwords with no effort. All you need to do is verify, not know it. Instead of decrypting it just check hash of provided password matches.
  • Have one salt for every password / user. It should be unique.
  • Doesn't have random seed.
  • Salt is too short - there is no reason for shortening it since user nobody have to remember it.
  • Uses MD5 as hashing algorithm. You should move to PBKDF2 (see RFC2898)
  • Have only one iteration of hashing function - for example LastPass uses 100k rounds of hashing. (read more here)

You need 3 methods:

  1. For computing hash.

  2. For generating salt.

  3. For checking if password hash matches.

    using System.Web.Security;
    using System.Security.Cryptography;
    namespace RijndaelEncDec{
    public static class PasswordHasher
    {
    // 24 = 192 bits
    private const int SaltByteSize = 24;
    private const int HashByteSize = 24;
    private const int HasingIterationsCount = 10101;
    public static byte[] ComputeHash(string password, byte[] salt, int iterations=HasingIterationsCount, int hashByteSize=HashByteSize)
    {
     Rfc2898DeriveBytes hashGenerator = new Rfc2898DeriveBytes(password, salt);
     hashGenerator.IterationCount = iterations;
     return hashGenerator.GetBytes(hashByteSize);
    }
    public static byte[] GenerateSalt(int saltByteSize=SaltByteSize)
    {
     RNGCryptoServiceProvider saltGenerator = new RNGCryptoServiceProvider();
     byte[] salt = new byte[saltByteSize];
     saltGenerator.GetBytes(salt);
     return salt;
    }
    public static bool VerifyPassword(String password, byte[] passwordSalt, byte[] passwordHash)
    {
     byte[] computedHash = ComputeHash(password, passwordSalt);
     return AreHashesEqual(computedHash, passwordHash);
    }
    //Length constant verification - prevents timing attack
    private static bool AreHashesEqual(byte[] firstHash, byte[] secondHash)
    { 
     int minHashLenght = firstHash.Length <= secondHash.Length ? firstHash.Length : secondHash.Length;
     var xor = firstHash.Length ^ secondHash.Length;
     for (int i = 0; i < minHashLenght; i++)
     xor |= firstHash[i] ^ secondHash[i];
     return 0 == xor;
    }
    } 
    }
    

Unfortunately you will have to change your interface a little...

Your code has few flaws:

  • You decrypting password - if someone gain access to your source code he can brake all your passwords with no effort. All you need to do is verify, not know it. Instead of decrypting it just check hash of provided password matches.
  • Have one salt for every password / user. It should be unique.
  • Doesn't have random seed.
  • Salt is too short - there is no reason for shortening it since user nobody have to remember it.
  • Uses MD5 as hashing algorithm. You should move to PBKDF2 (see RFC2898)
  • Have only one iteration of hashing function - for example LastPass uses 100k rounds of hashing. (read more here)

You need 3 methods:

  1. For computing hash.

  2. For generating salt.

  3. For checking if password hash matches.

    using System.Web.Security;
    using System.Security.Cryptography;
    namespace RijndaelEncDec{
    public static class PasswordHasher
    {
    // 24 = 192 bits
    private const int SaltByteSize = 24;
    private const int HashByteSize = 24;
    private const int HasingIterationsCount = 10101;
    public static byte[] ComputeHash(string password, byte[] salt, int iterations=HasingIterationsCount, int hashByteSize=HashByteSize)
    {
     Rfc2898DeriveBytes hashGenerator = new Rfc2898DeriveBytes(password, salt);
     hashGenerator.IterationCount = iterations;
     return hashGenerator.GetBytes(hashByteSize);
    }
    public static byte[] GenerateSalt(int saltByteSize=SaltByteSize)
    {
     RNGCryptoServiceProvider saltGenerator = new RNGCryptoServiceProvider();
     byte[] salt = new byte[saltByteSize];
     saltGenerator.GetBytes(salt);
     return salt;
    }
    public static bool VerifyPassword(String password, byte[] passwordSalt, byte[] passwordHash)
    {
     byte[] computedHash = ComputeHash(password, passwordSalt);
     return AreHashesEqual(computedHash, passwordHash);
    }
    //Length constant verification - prevents timing attack
    private static bool AreHashesEqual(byte[] firstHash, byte[] secondHash)
    { 
     int minHashLenght = firstHash.Length <= secondHash.Length ? firstHash.Length : secondHash.Length;
     var xor = firstHash.Length ^ secondHash.Length;
     for (int i = 0; i < minHashLenght; i++)
     xor |= firstHash[i] ^ secondHash[i];
     return 0 == xor;
    }
    } 
    }
    

Unfortunately you will have to change your interface a little...

Your code has few flaws:

  • You decrypting password - if someone gain access to your source code he can brake all your passwords with no effort. All you need to do is verify, not know it. Instead of decrypting it just check hash of provided password matches.
  • Have one salt for every password / user. It should be unique.
  • Doesn't have random seed.
  • Salt is too short - there is no reason for shortening it since nobody have to remember it.
  • Uses MD5 as hashing algorithm. You should move to PBKDF2 (see RFC2898)
  • Have only one iteration of hashing function - for example LastPass uses 100k rounds of hashing. (read more here)

You need 3 methods:

  1. For computing hash.

  2. For generating salt.

  3. For checking if password hash matches.

    using System.Web.Security;
    using System.Security.Cryptography;
    namespace RijndaelEncDec{
    public static class PasswordHasher
    {
    // 24 = 192 bits
    private const int SaltByteSize = 24;
    private const int HashByteSize = 24;
    private const int HasingIterationsCount = 10101;
    public static byte[] ComputeHash(string password, byte[] salt, int iterations=HasingIterationsCount, int hashByteSize=HashByteSize)
    {
     Rfc2898DeriveBytes hashGenerator = new Rfc2898DeriveBytes(password, salt);
     hashGenerator.IterationCount = iterations;
     return hashGenerator.GetBytes(hashByteSize);
    }
    public static byte[] GenerateSalt(int saltByteSize=SaltByteSize)
    {
     RNGCryptoServiceProvider saltGenerator = new RNGCryptoServiceProvider();
     byte[] salt = new byte[saltByteSize];
     saltGenerator.GetBytes(salt);
     return salt;
    }
    public static bool VerifyPassword(String password, byte[] passwordSalt, byte[] passwordHash)
    {
     byte[] computedHash = ComputeHash(password, passwordSalt);
     return AreHashesEqual(computedHash, passwordHash);
    }
    //Length constant verification - prevents timing attack
    private static bool AreHashesEqual(byte[] firstHash, byte[] secondHash)
    { 
     int minHashLenght = firstHash.Length <= secondHash.Length ? firstHash.Length : secondHash.Length;
     var xor = firstHash.Length ^ secondHash.Length;
     for (int i = 0; i < minHashLenght; i++)
     xor |= firstHash[i] ^ secondHash[i];
     return 0 == xor;
    }
    } 
    }
    

Unfortunately you will have to change your interface a little...

Code formating
Source Link
  1. For computing hash.

  2. For generating salt.

  3. For checking if password hash matches.

    using System.Web.Security; using System.Security.Cryptography;

    using System.Web.Security;
    using System.Security.Cryptography;
    namespace RijndaelEncDec{
    public static class PasswordHasher
    {
    // 24 = 192 bits
    private const int SaltByteSize = 24;
    private const int HashByteSize = 24;
    private const int HasingIterationsCount = 10101;
    public static byte[] ComputeHash(string password, byte[] salt, int iterations=HasingIterationsCount, int hashByteSize=HashByteSize)
    {
     Rfc2898DeriveBytes hashGenerator = new Rfc2898DeriveBytes(password, salt);
     hashGenerator.IterationCount = iterations;
     return hashGenerator.GetBytes(hashByteSize);
    }
    public static byte[] GenerateSalt(int saltByteSize=SaltByteSize)
    {
     RNGCryptoServiceProvider saltGenerator = new RNGCryptoServiceProvider();
     byte[] salt = new byte[saltByteSize];
     saltGenerator.GetBytes(salt);
     return salt;
    }
    public static bool VerifyPassword(String password, byte[] passwordSalt, byte[] passwordHash)
    {
     byte[] computedHash = ComputeHash(password, passwordSalt);
     return AreHashesEqual(computedHash, passwordHash);
    }
    //Length constant verification - prevents timing attack
    private static bool AreHashesEqual(byte[] firstHash, byte[] secondHash)
    { 
     int minHashLenght = firstHash.Length <= secondHash.Length ? firstHash.Length : secondHash.Length;
     var xor = firstHash.Length ^ secondHash.Length;
     for (int i = 0; i < minHashLenght; i++)
     xor |= firstHash[i] ^ secondHash[i];
     return 0 == xor;
    }
    } 
    }
    

namespace RijndaelEncDec

{ public static class PasswordHasher { // 24 = 192 bits private const int SaltByteSize = 24; private const int HashByteSize = 24; private const int HasingIterationsCount = 11311;

 public static byte[] ComputeHash(string password, byte[] salt, int iterations=HasingIterationsCount, int hashByteSize=HashByteSize)
 {
 Rfc2898DeriveBytes hashGenerator = new Rfc2898DeriveBytes(password, salt);
 hashGenerator.IterationCount = iterations;
 return hashGenerator.GetBytes(hashByteSize);
 }
 
 public static byte[] GenerateSalt(int saltByteSize=SaltByteSize)
 {
 RNGCryptoServiceProvider saltGenerator = new RNGCryptoServiceProvider();
 byte[] salt = new byte[saltByteSize];
 saltGenerator.GetBytes(salt);
 return salt;
 }
 
 public static bool VerifyPassword(String password, byte[] passwordSalt, byte[] passwordHash)
 {
 byte[] computedHash = ComputeHash(password, passwordSalt);
 return AreHashesEqual(computedHash, passwordHash);
 }
 //Length constant verification - prevents timing attack
 private static bool AreHashesEqual(byte[] firstHash, byte[] secondHash)
 { 
 int minHashLenght = firstHash.Length <= secondHash.Length ? firstHash.Length : secondHash.Length;
 var xor = firstHash.Length ^ secondHash.Length;
 for (int i = 0; i < minHashLenght; i++)
 xor |= firstHash[i] ^ secondHash[i];
 return 0 == xor;
 }
} 
}
  1. For computing hash.

  2. For generating salt.

  3. For checking if password hash matches.

    using System.Web.Security; using System.Security.Cryptography;

namespace RijndaelEncDec

{ public static class PasswordHasher { // 24 = 192 bits private const int SaltByteSize = 24; private const int HashByteSize = 24; private const int HasingIterationsCount = 11311;

 public static byte[] ComputeHash(string password, byte[] salt, int iterations=HasingIterationsCount, int hashByteSize=HashByteSize)
 {
 Rfc2898DeriveBytes hashGenerator = new Rfc2898DeriveBytes(password, salt);
 hashGenerator.IterationCount = iterations;
 return hashGenerator.GetBytes(hashByteSize);
 }
 
 public static byte[] GenerateSalt(int saltByteSize=SaltByteSize)
 {
 RNGCryptoServiceProvider saltGenerator = new RNGCryptoServiceProvider();
 byte[] salt = new byte[saltByteSize];
 saltGenerator.GetBytes(salt);
 return salt;
 }
 
 public static bool VerifyPassword(String password, byte[] passwordSalt, byte[] passwordHash)
 {
 byte[] computedHash = ComputeHash(password, passwordSalt);
 return AreHashesEqual(computedHash, passwordHash);
 }
 //Length constant verification - prevents timing attack
 private static bool AreHashesEqual(byte[] firstHash, byte[] secondHash)
 { 
 int minHashLenght = firstHash.Length <= secondHash.Length ? firstHash.Length : secondHash.Length;
 var xor = firstHash.Length ^ secondHash.Length;
 for (int i = 0; i < minHashLenght; i++)
 xor |= firstHash[i] ^ secondHash[i];
 return 0 == xor;
 }
} 
}
  1. For computing hash.

  2. For generating salt.

  3. For checking if password hash matches.

    using System.Web.Security;
    using System.Security.Cryptography;
    namespace RijndaelEncDec{
    public static class PasswordHasher
    {
    // 24 = 192 bits
    private const int SaltByteSize = 24;
    private const int HashByteSize = 24;
    private const int HasingIterationsCount = 10101;
    public static byte[] ComputeHash(string password, byte[] salt, int iterations=HasingIterationsCount, int hashByteSize=HashByteSize)
    {
     Rfc2898DeriveBytes hashGenerator = new Rfc2898DeriveBytes(password, salt);
     hashGenerator.IterationCount = iterations;
     return hashGenerator.GetBytes(hashByteSize);
    }
    public static byte[] GenerateSalt(int saltByteSize=SaltByteSize)
    {
     RNGCryptoServiceProvider saltGenerator = new RNGCryptoServiceProvider();
     byte[] salt = new byte[saltByteSize];
     saltGenerator.GetBytes(salt);
     return salt;
    }
    public static bool VerifyPassword(String password, byte[] passwordSalt, byte[] passwordHash)
    {
     byte[] computedHash = ComputeHash(password, passwordSalt);
     return AreHashesEqual(computedHash, passwordHash);
    }
    //Length constant verification - prevents timing attack
    private static bool AreHashesEqual(byte[] firstHash, byte[] secondHash)
    { 
     int minHashLenght = firstHash.Length <= secondHash.Length ? firstHash.Length : secondHash.Length;
     var xor = firstHash.Length ^ secondHash.Length;
     for (int i = 0; i < minHashLenght; i++)
     xor |= firstHash[i] ^ secondHash[i];
     return 0 == xor;
    }
    } 
    }
    
Source Link
Loading
lang-cs

AltStyle によって変換されたページ (->オリジナル) /