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?

• Mr R: If you add a date into the pass-phrase - how does the other end know what it is ??
• Mr R: The point of a SALT is to not include it - thus reducing the likelihood of BFing with pre-computed hashes aka. rainbow attack
• RemarkLima: @MrR The date would be in the stored row as well, so could be retrieved as part of the record, and hashed and added back to the key to decrypt. This would then mean an attacker has the basis of that salt so not sure?
• RemarkLima: @MrR the rainbow attack is what I'm looking to mitigate - however Microsoft themselves include the salt in the password hashes stored in the same column in the ASP.NET identity framework. So you have the password password, which you hash and then get a load of password hashes. Adding the salt randomstuff+password and saving as randomstuff + hash(randomstuff+password) mitigates the rainbow attack right?
• Mr R: And I'd be willing to guess a Date is not good for a salt - because it is not really a random number - if you were concerned go for a wider salt.

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).

• RemarkLima: Quick question before I digest the rest of the review (thanks BTW) - the 51 bits for a diceware, is that 4 times diceware? i.e. fish-sock-cheese-river as 4 separate rolls to create the 4 words? Or is it 51 bits of entropy per roll? And so 51 bits of entropy multiplied by 4?
• Maarten Bodewes: Heh, no, it is the total of 4 rolls. I simply asked for a 4 word roll on the site, got the answer back + the number of possibilities. Then it is simply a question of taking the $\log_2$ of that number. Or counting the number of digits, and divide by 3 and multiply by 10 to get an approximation (and you're welcome :) ).
• RemarkLima: From 4 rolls, and your sums I get: (3,656,158,440,062,976 / 3) * 10 = 1.2187195e+16 ... Is that right? 4 rolls gives over 3.5 trillion (??? Quadrillion??) combinations right? Or are my maths way off? On my phone, hence a bit gammy with the numbers!
• RemarkLima: Hi @maarten-bodewes, I've put a few 4 word passwords into KeePass and always get over 110 bits of entropy? Not sure how to get an absolute on this
• Maarten Bodewes: @RemarkLima No, the calculations are over the number of digits. So if e+16 then you have slightly over 16 digits, then (16 / 3) x 10 = 53 bits and cryptographers do use caution so we guess to the low end. KeePass doesn't know that they are diceware rolls so it's guess is way too high, this kind of knowledge should be included. A key size of 32 bytes is 256 bit, which gives you enough protection even against quantum computers. And as you may notice, the 51 bit password is what you need to protect - not the 256 bit key.