Encrypted text - C# SHA256, implementation

Question

I have a C# application which needs to encrypt string and save the key as a hash. The key is saved as a hash to check the right key has been input before trying to decrypt the message.

First, I have these method to generate the hash string - hopefully "off the shelf" and ready to go

        public static byte[] GetHash(string inputString)
        {
            using (HashAlgorithm algorithm = SHA256.Create())
                return algorithm.ComputeHash(Encoding.UTF8.GetBytes(inputString));
        }

        public static string GetHashString(string inputString)
        {
            StringBuilder sb = new StringBuilder();
            foreach (byte b in GetHash(inputString))
                sb.Append(b.ToString("X2"));

            return sb.ToString();
        }

Then my encryption extension method:

private const int Keysize = 256;
private const int DerivationIterations = 1000;

public static string Encrypt(this string plainText, string passPhrase)
        {
            // Salt and IV is randomly generated each time, but is preprended to encrypted cipher text
            // so that the same Salt and IV values can be used when decrypting.  
            var saltStringBytes = Generate256BitsOfRandomEntropy();
            var ivStringBytes = Generate256BitsOfRandomEntropy();
            var plainTextBytes = Encoding.UTF8.GetBytes(plainText);
            using (var password = new Rfc2898DeriveBytes(passPhrase, saltStringBytes, DerivationIterations))
            {
                var keyBytes = password.GetBytes(Keysize / 8);
                var engine = new RijndaelEngine(256);
                var blockCipher = new CbcBlockCipher(engine);
                var cipher = new PaddedBufferedBlockCipher(blockCipher, new Pkcs7Padding());
                var keyParam = new KeyParameter(keyBytes);
                var keyParamWithIV = new ParametersWithIV(keyParam, ivStringBytes, 0, 32);

                cipher.Init(true, keyParamWithIV);
                var comparisonBytes = new byte[cipher.GetOutputSize(plainTextBytes.Length)];
                var length = cipher.ProcessBytes(plainTextBytes, comparisonBytes, 0);

                cipher.DoFinal(comparisonBytes, length);
                //                return Convert.ToBase64String(comparisonBytes);
                return Convert.ToBase64String(saltStringBytes.Concat(ivStringBytes).Concat(comparisonBytes).ToArray());
            }
        }


private static byte[] Generate256BitsOfRandomEntropy()
        {
            var randomBytes = new byte[32]; // 32 Bytes will give us 256 bits.
            using (var rngCsp = new RNGCryptoServiceProvider())
            {
                // Fill the array with cryptographically secure random bytes.
                rngCsp.GetBytes(randomBytes);
            }
            return randomBytes;
        }

Which is then called to a input string:

var encryptedString = inputString.Encrypt(_appSettings.EncryptionSecret + passPhrase);

The passphrase is a random set of characters, prepended with an application string.

My concern is that the passphrase is 4 sets of "diceware" words (like here: https://diceware.dmuth.org/) and therefore could be brute forced. Would adding more to the passphrase, such as as hash of DateTime.UtcNow.ToString("yyyyMMdd") or GetHashString(random-string) for example add any additional security or is it just theatre and the "diceware" passphrase is practicably unbreakable?

Answer 1

My concern is that the passphrase is 4 sets of "diceware" words (like here: https://diceware.dmuth.org/) and therefore could be brute forced.

The diceware password is about 51 bits in strength, so it should indeed be run through a password based key derivation function, especially if you use it to encrypt messages - because encryption can be broken offline (i.e. on the computer of an attacker).

Would adding more to the passphrase, such as as hash of DateTime.UtcNow.ToString("yyyyMMdd") or GetHashString(random-string) for example add any additional security or is it just theatre and the "diceware" passphrase is practicably unbreakable?

No, the passphrase is not big enough to be unbreakable. Adding more information could help, but only if it is not available to an attacker, which is very questionable for a date / time.

---

private const int Keysize = 256;

This is good because we generally use key sizes in bits (and "size" is better than "length" in my opinion).

private const int DerivationIterations = 1000;

This is not a good idea at all. First of all, you should use about a million as iteration count at the moment. It would also be a big boon if you could upgrade it to a higher count later on. The amount of operations that an attacker has to perform is the same as the number of iterations, and an attacker may use a very fast implementation. Currently it adds about log_2(1000) =~ 10 bits of security.

public static byte[] GetHash(string inputString)

Especially if this is a public method (why?) then the hash function and encoding performed on the input string should be documented. It is not clear why the input needs to be a string in the first place though.

public static string GetHashString(string inputString)

Same for the output encoding of course.

StringBuilder sb = new StringBuilder();
foreach (byte b in GetHash(inputString))
    sb.Append(b.ToString("X2"));

Reprogramming a hex encoder doesn't make sense, and why not use a generic hex encoding function? In the end you want to have something like: hex(hash(utf8(text))) in your code...

var saltStringBytes = Generate256BitsOfRandomEntropy();

No, that's 256 bits of randomness you generate, the entropy is gathered by the operating system in the end. Why not just call it a salt? A StringByte is not a structure I've ever heard of.

 using (var password = new Rfc2898DeriveBytes(passPhrase, saltStringBytes, DerivationIterations))

This badly named class of Microsoft implements PBKDF2, which is defined in the Password Based Encryption standard. So what about var pbkdf2 = because the returned object certainly is not a password.

Note that PBKDF2 is inefficient if you ask more than the configured hash output size. That defaults to SHA-1. If you'd use SHA-512 instead then you could retrieve a key, and IV and a check value all from a 512 bit / 64 byte output (32 bytes for the key, 16 bytes for the IV and check value each, for instance).

var engine = new RijndaelEngine(256);

Why would you use Bouncy Castle, a software only provider for bog-standard AES functionality? Do you hate the AES-NI instruction set?

var keyParamWithIV = new ParametersWithIV(keyParam, ivStringBytes, 0, 32);

I'm not sure, but I'm pretty confident that ivStringBytes is already the correct size, so the offset and length should not be needed.

var comparisonBytes = new byte[cipher.GetOutputSize(plainTextBytes.Length)];

Comparison to what? You have encrypted a message, right? If you just want to check if a password is correct then just use the password hash (PBKDF2).