Question

I needed some simple string encryption, so I wrote the following code (with a great deal of "inspiration" from here):

    // create and initialize a crypto algorithm
    private static SymmetricAlgorithm getAlgorithm(string password) {
        SymmetricAlgorithm algorithm = Rijndael.Create();
        Rfc2898DeriveBytes rdb = new Rfc2898DeriveBytes(
            password, new byte[] {
            0x53,0x6f,0x64,0x69,0x75,0x6d,0x20,             // salty goodness
            0x43,0x68,0x6c,0x6f,0x72,0x69,0x64,0x65
        }
        );
        algorithm.Padding = PaddingMode.ISO10126;
        algorithm.Key = rdb.GetBytes(32);
        algorithm.IV = rdb.GetBytes(16);
        return algorithm;
    }

    /* 
     * encryptString
     * provides simple encryption of a string, with a given password
     */
    public static string encryptString(string clearText, string password) {
        SymmetricAlgorithm algorithm = getAlgorithm(password);
        byte[] clearBytes = System.Text.Encoding.Unicode.GetBytes(clearText);
        MemoryStream ms = new MemoryStream();
        CryptoStream cs = new CryptoStream(ms, algorithm.CreateEncryptor(), CryptoStreamMode.Write);
        cs.Write(clearBytes, 0, clearBytes.Length);
        cs.Close();
        return Convert.ToBase64String(ms.ToArray());
    }

    /*
     * decryptString
     * provides simple decryption of a string, with a given password
     */
    public static string decryptString(string cipherText, string password) {
        SymmetricAlgorithm algorithm = getAlgorithm(password);
        byte[] cipherBytes = Convert.FromBase64String(cipherText);
        MemoryStream ms = new MemoryStream();
        CryptoStream cs = new CryptoStream(ms, algorithm.CreateDecryptor(), CryptoStreamMode.Write);
        cs.Write(cipherBytes, 0, cipherBytes.Length);
        cs.Close();            
        return System.Text.Encoding.Unicode.GetString(ms.ToArray());
    }

The code appears to work fine, except that when decrypting data with an incorrect key, I get a CryptographicException - "Padding is invalid and cannot be removed" - on the cs.Close() line in decryptString.

example code:

    string password1 = "password";
    string password2 = "letmein";
    string startClearText = "The quick brown fox jumps over the lazy dog";
    string cipherText = encryptString(startClearText, password1);
    string endClearText = decryptString(cipherText, password2);     // exception thrown

My question is, is this to be expected? I would have thought that decrypting with the wrong password would just result in nonsense output, rather than an exception.

Was it helpful?

Solution

Although this have been already answered I think it would be a good idea to explain why it is to be expected.

A padding scheme is usually applied because most cryptographic filters are not semantically secure and to prevent some forms of cryptoatacks. For example, usually in RSA the OAEP padding scheme is used which prevents some sorts of attacks (such as a chosen plaintext attack or blinding).

A padding scheme appends some (usually) random garbage to the message m before the message is sent. In the OAEP method, for example, two Oracles are used (this is a simplistic explanation):

  1. Given the size of the modulus you padd k1 bits with 0 and k0 bits with a random number.
  2. Then by applying some transformation to the message you obtain the padded message wich is encrypted and sent.

That provides you with a randomization for the messages and with a way to test if the message is garbage or not. As the padding scheme is reversible, when you decrypt the message whereas you can't say anything about the integrity of the message itself you can, in fact, make some assertion about the padding and thus you can know if the message has been correctly decrypted or you're doing something wrong (i.e someone has tampered with the message or you're using the wrong key)

OTHER TIPS

I experienced a similar "Padding is invalid and cannot be removed." exception, but in my case the key IV and padding were correct.

It turned out that flushing the crypto stream is all that was missing.

Like this:

            MemoryStream msr3 = new MemoryStream();
            CryptoStream encStream = new CryptoStream(msr3, RijndaelAlg.CreateEncryptor(), CryptoStreamMode.Write);
            encStream.Write(bar2, 0, bar2.Length);
            // unless we flush the stream we would get "Padding is invalid and cannot be removed." exception when decoding
            encStream.FlushFinalBlock();
            byte[] bar3 = msr3.ToArray();

If you want your usage to be correct, you should add authentication to your ciphertext so that you can verify that it is the correct pasword or that the ciphertext hasn't been modified. The padding you are using ISO10126 will only throw an exception if the last byte doesn't decrypt as one of 16 valid values for padding (0x01-0x10). So you have a 1/16 chance of it NOT throwing the exception with the wrong password, where if you authenticate it you have a deterministic way to tell if your decryption is valid.

Using crypto api's while seemingly easy, actually is rather is easy to make mistakes. For example you use a fixed salt for for you key and iv derivation, that means every ciphertext encrypted with the same password will reuse it's IV with that key, that breaks semantic security with CBC mode, the IV needs to be both unpredictable and unique for a given key.

For that reason of easy to make mistakes, I have a code snippet, that I try to keep reviewed and up to date (comments, issues welcome):

Modern Examples of Symmetric Authenticated Encryption of a string C#.

If you use it's AESThenHMAC.AesSimpleDecryptWithPassword(ciphertext, password) when the wrong password is used, null is returned, if the ciphertext or iv has been modified post encryption null is returned, you will never get junk data back, or a padding exception.

If you've ruled out key-mismatch, then besides FlushFinalBlock() (see Yaniv's answer), calling Close() on the CryptoStream will also suffice.

If you are cleaning up resources strictly with using blocks, be sure to nest the block for the CryptoStream itself:

using (MemoryStream ms = new MemoryStream())
using (var enc = RijndaelAlg.CreateEncryptor())
{
  using (CryptoStream encStream = new CryptoStream(ms, enc, CryptoStreamMode.Write))
  {
    encStream.Write(bar2, 0, bar2.Length);
  } // implicit close
  byte[] encArray = ms.ToArray();
}

I've been bitten by this (or similar):

using (MemoryStream ms = new MemoryStream())
using (var enc = RijndaelAlg.CreateEncryptor())
using (CryptoStream encStream = new CryptoStream(ms, enc, CryptoStreamMode.Write))
{
  encStream.Write(bar2, 0, bar2.Length);
  byte[] encArray = ms.ToArray();
} // implicit close -- too late!

Yes, this is to be expected, or at least, its exactly what happens when our crypto routines get non-decryptable data

Another reason of the exception might be a race condition between several threads using decryption logic - native implementations of ICryptoTransform are not thread-safe (e.g. SymmetricAlgorithm), so it should be put to exclusive section, e.g. using lock. Please refer here for more details: http://www.make-awesome.com/2011/07/system-security-cryptography-and-thread-safety/

There may be some unread bytes in the CryptoStream. Closing before reading the stream completely was causing the error in my program.

I had a similar problem, the issue in decrypt method was initializing an empty memory stream. when it worked when I initialized it with the cipher text byte array like this:

MemoryStream ms = new MemoryStream(cipherText)

The answer updated by the user "atconway" worked for me.

The problem was not with the padding but the key which was different during encryption and decryption. The key and iv should be same during encypting and decrypting the same value.

Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top