4
\$\begingroup\$

I am writing a helper class for encrypting string and other data types. My goal is to make it as easy as possible to use, while still keeping a substantial amount of security.

One issue I struggled with was simplifying all the different key and IV sizes. I've settled on having the caller provider a salt, and then generating my own key and IV data from the salt.

I assume this compromises the security very slightly, but not that much.

I would appreciate hearing other people's thoughts, especially those who have more experience with cryptography than I. Am I compromising my security too much?

public virtual byte[] EncryptBytes(byte[] data)
{
    using (SymmetricAlgorithm algorithm = TripleDES.Create())
    using (ICryptoTransform encryptor = algorithm.CreateEncryptor(GetKey(algorithm), GetIV(algorithm)))
    {
        MemoryStream ms = new MemoryStream();
        using (Stream cs = new CryptoStream(ms, encryptor, CryptoStreamMode.Write))
        {
            cs.Write(data, 0, data.Length);
        }
        return ms.ToArray();
    }
}

public byte[] GetKey(SymmetricAlgorithm algorithm)
{
    return DeriveBytes(algorithm.KeySize / 8);
}

public byte[] GetIV(SymmetricAlgorithm algorithm)
{
    return DeriveBytes(algorithm.BlockSize / 8);
}

protected byte[] DeriveBytes(int bytes)
{
    Rfc2898DeriveBytes derivedBytes = new Rfc2898DeriveBytes(Password, Salt, 1000);
    return derivedBytes.GetBytes(bytes);
}
\$\endgroup\$
  • \$\begingroup\$ I have rolled back the last edit. Please see what you may and may not do after receiving answers. \$\endgroup\$ – Heslacher Jul 13 '15 at 15:58
  • \$\begingroup\$ Please link the full code for context. While this code has some clear mistakes (IV==Key) it's not possible to review it properly without the missing parts. In particular the salt management and if you add additional layers on top of your encryption that prevent active attacks. \$\endgroup\$ – CodesInChaos Jul 15 '15 at 7:48
  • \$\begingroup\$ What mistakes? The technique I was trying to have reviewed is there, and in fact I've received some good information about it already. \$\endgroup\$ – Jonathan Wood Jul 15 '15 at 13:35
  • \$\begingroup\$ @JonathanWood Depending on how the salt is handled deriving the IV from password and salt may or may not be secure. But you left out the salt handling. You also left out the derivation of the MAC key and the computation of the MAC. The clear mistakes are computing the same expensive KDF twice, a waste of resources, and using the same value of key and IV which might even lead to a key recovery attack. The function isn't easy to use either, since it leaves salt handling, which is rather tricky, to the caller. If you want a proper answer, please include the rest of the encryption code. \$\endgroup\$ – CodesInChaos Jul 21 '15 at 22:19
  • \$\begingroup\$ I understood a couple of things you said there. What handling are you referring to? The salt would be provided by the caller to ensure it could be the same value when the encrypted data needs to be decrypted. I was thinking it could be random bytes. \$\endgroup\$ – Jonathan Wood Jul 21 '15 at 22:31
2
\$\begingroup\$

The purpose of an initialization vector is to prevent differential cryptanalysis based on two similar plaintext messages that get encrypted to similar ciphertexts. The IV does not need to be secret, but you must not reuse one IV for encrypting two messages. To derive a unique IV for every message, you can use a counter, a fine-resolution timestamp, or a cryptographic-quality random number generator.

On the other hand, your GetIV() is a deterministic function of the Password and Salt. The Password, of course, rarely changes. The Salt doesn't look like it changes either. Therefore, your GetIV() fails to return a different IV for every message.

\$\endgroup\$
  • \$\begingroup\$ How would a timestamp work? When I tried to decrypt the data, the timestamp would be different and so I'd generate a different IV. And so it wouldn't work. What am I missing? \$\endgroup\$ – Jonathan Wood Jul 13 '15 at 17:38
  • 1
    \$\begingroup\$ Let us continue this discussion in chat. \$\endgroup\$ – 200_success Jul 13 '15 at 18:03
  • 1
    \$\begingroup\$ For CBC mode a counter or timestamp is not a good idea, since the IV should be unpredictable (unless you're using the variant that encrypts the IV). \$\endgroup\$ – CodesInChaos Jul 14 '15 at 20:48
  • \$\begingroup\$ @CodesInChaos I based my answer on Bruce Schneier's Applied Cryptography, second edition, Sec. 9.3 (CBC Mode): "A timestamp often makes a good IV." and "The IV need not be secret; it can be transmitted in the clear with the ciphertext." \$\endgroup\$ – 200_success Jul 15 '15 at 21:15
  • 2
    \$\begingroup\$ IV's don't need to be secret, but they need to be unpredictable. The BEAST attack was based on predictable IVs in CBC mode. \$\endgroup\$ – CodesInChaos Jul 15 '15 at 21:30
7
\$\begingroup\$

Why are you using 3DES instead of AES? Why is the algorithm hard-coded in EncryptBytes, but parameterized in GetKey() and GetIV()?

The last line looks suspicious — did you mean bits / 8?

protected byte[] DeriveBytes(int bits)
{
    Rfc2898DeriveBytes derivedBytes = new Rfc2898DeriveBytes(Password, Salt, 1000);
    return derivedBytes.GetBytes(bits / 8);
}

If so, then shouldn't it be…?

public byte[] GetKey(SymmetricAlgorithm algorithm)
{
    return DeriveBytes(algorithm.KeySize);
}
\$\endgroup\$
  • \$\begingroup\$ Yes, I misplaced one divide by 8 in my post. My real code is working reliably, and it supports many different encryption algorithms. (It's hard coded in my sample so I don't have to post additional routines.) My question here is about trying to determine how much security I lose by generating all my key and IV bytes from a password and salt value. \$\endgroup\$ – Jonathan Wood Jul 13 '15 at 16:00
2
\$\begingroup\$

Do not use 3DES, use AES 256 with CBC mode. Never use the same IV for multiple encryptions. You do not need to hide your IV, so you can always create a new IV and send it with your encrypted message.

You can use the Managed AES256 class in .NET: https://msdn.microsoft.com/de-de/library/system.security.cryptography.aesmanaged(v=vs.110).aspx

I would set the CBC mode, block and key sizes explicitly to make clear that you want to use them.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.