1

I've been using the code that was posted here to encrypt and decrypt, written by @nerdybeardo. However, I'm getting the error "pad block corrupted" when trying to decrypt.

The Encryptor class looks like this, which implements encrypt then MAC:

/// <summary>
/// Encrypt/decrypt + HMAC using BouncyCastle (C# Java port)
/// </summary>
/// <typeparam name="TBlockCipher">The type of the block cipher.</typeparam>
/// <typeparam name="TDigest">The type of the digest.</typeparam>
/// <see cref="https://stackoverflow.com/a/13511671/119624"/>
public sealed class Encryptor<TBlockCipher, TDigest>
    where TBlockCipher : IBlockCipher, new()
    where TDigest : IDigest, new()
{
    private readonly Encoding encoding;

    private readonly byte[] key;

    private IBlockCipher blockCipher;

    private BufferedBlockCipher cipher;

    private HMac mac;

    /// <summary>
    /// Initializes a new instance of the <see cref="Encryptor{TBlockCipher, TDigest}"/> class.
    /// </summary>
    /// <param name="encoding">The encoding.</param>
    /// <param name="key">The key.</param>
    /// <param name="macKey">The mac key.</param>
    public Encryptor(Encoding encoding, byte[] key, byte[] macKey)
    {
        this.encoding = encoding;
        this.key = key;
        this.Init(key, macKey, new Pkcs7Padding());
    }

    /// <summary>
    /// Initializes a new instance of the <see cref="Encryptor{TBlockCipher, TDigest}"/> class.
    /// </summary>
    /// <param name="encoding">The encoding.</param>
    /// <param name="key">The key.</param>
    /// <param name="macKey">The mac key.</param>
    /// <param name="padding">The padding.</param>
    public Encryptor(Encoding encoding, byte[] key, byte[] macKey, IBlockCipherPadding padding)
    {
        this.encoding = encoding;
        this.key = key;
        this.Init(key, macKey, padding);
    }

    /// <summary>
    /// Encrypts the specified plain.
    /// </summary>
    /// <param name="plain">The plain.</param>
    /// <returns></returns>
    public string Encrypt(string plain)
    {
        return Convert.ToBase64String(EncryptBytes(plain));
    }

    /// <summary>
    /// Encrypts the bytes.
    /// </summary>
    /// <param name="plain">The plain.</param>
    /// <returns></returns>
    public byte[] EncryptBytes(string plain)
    {
        byte[] input = this.encoding.GetBytes(plain);

        var iv = this.GenerateInitializationVector();

        var cipher = this.BouncyCastleCrypto(true, input, new ParametersWithIV(new KeyParameter(key), iv));
        byte[] message = CombineArrays(iv, cipher);

        this.mac.Reset();
        this.mac.BlockUpdate(message, 0, message.Length);
        var digest = new byte[this.mac.GetUnderlyingDigest().GetDigestSize()];
        this.mac.DoFinal(digest, 0);

        var result = CombineArrays(digest, message);
        return result;
    }

    /// <summary>
    /// Decrypts the bytes.
    /// </summary>
    /// <param name="bytes">The bytes.</param>
    /// <returns></returns>
    /// <exception cref="CryptoException"></exception>
    public byte[] DecryptBytes(byte[] bytes)
    {
        // split the digest into component parts
        var digest = new byte[this.mac.GetUnderlyingDigest().GetDigestSize()];
        var message = new byte[bytes.Length - digest.Length];
        var iv = new byte[this.blockCipher.GetBlockSize()];
        var cipher = new byte[message.Length - iv.Length];

        Buffer.BlockCopy(bytes, 0, digest, 0, digest.Length);
        Buffer.BlockCopy(bytes, digest.Length, message, 0, message.Length);
        if (!IsValidHMac(digest, message))
        {
            throw new CryptoException();
        }

        Buffer.BlockCopy(message, 0, iv, 0, iv.Length);
        Buffer.BlockCopy(message, iv.Length, cipher, 0, cipher.Length);

        byte[] result = this.BouncyCastleCrypto(false, cipher, new ParametersWithIV(new KeyParameter(key), iv));
        return result;
    }

    /// <summary>
    /// Decrypts the specified bytes.
    /// </summary>
    /// <param name="bytes">The bytes.</param>
    /// <returns></returns>
    public string Decrypt(byte[] bytes)
    {
        return this.encoding.GetString(DecryptBytes(bytes));
    }

    /// <summary>
    /// Decrypts the specified cipher.
    /// </summary>
    /// <param name="cipher">The cipher.</param>
    /// <returns></returns>
    public string Decrypt(string cipher)
    {
        return this.Decrypt(Convert.FromBase64String(cipher));
    }

    /// <summary>
    /// Combines the arrays.
    /// </summary>
    /// <param name="source1">The source1.</param>
    /// <param name="source2">The source2.</param>
    /// <returns></returns>
    private static byte[] CombineArrays(byte[] source1, byte[] source2)
    {
        var result = new byte[source1.Length + source2.Length];
        Buffer.BlockCopy(source1, 0, result, 0, source1.Length);
        Buffer.BlockCopy(source2, 0, result, source1.Length, source2.Length);

        return result;
    }

    /// <summary>
    /// Ares the equal.
    /// </summary>
    /// <param name="digest">The digest.</param>
    /// <param name="computed">The computed.</param>
    /// <returns></returns>
    private static bool AreEqual(byte[] digest, byte[] computed)
    {
        if (digest.Length != computed.Length)
        {
            return false;
        }

        var result = 0;
        for (var i = 0; i < digest.Length; i++)
        {
            result |= digest[i] ^ computed[i];
        }

        return result == 0;
    }

    /// <summary>
    /// Initializes the specified key.
    /// </summary>
    /// <param name="key">The key.</param>
    /// <param name="macKey">The mac key.</param>
    /// <param name="padding">The padding.</param>
    private void Init(byte[] key, byte[] macKey, IBlockCipherPadding padding)
    {
        this.blockCipher = new CbcBlockCipher(new TBlockCipher());
        this.cipher = new PaddedBufferedBlockCipher(this.blockCipher, padding);
        this.mac = new HMac(new TDigest());
        this.mac.Init(new KeyParameter(macKey));
    }

    /// <summary>
    /// Determines whether [is valid h mac] [the specified digest].
    /// </summary>
    /// <param name="digest">The digest.</param>
    /// <param name="message">The message.</param>
    /// <returns></returns>
    private bool IsValidHMac(byte[] digest, byte[] message)
    {
        this.mac.Reset();
        this.mac.BlockUpdate(message, 0, message.Length);
        var computed = new byte[this.mac.GetUnderlyingDigest().GetDigestSize()];
        this.mac.DoFinal(computed, 0);

        return AreEqual(digest, computed);
    }

    /// <summary>
    /// Bouncy Castle Cryptography.
    /// </summary>
    /// <param name="forEncrypt">if set to <c>true</c> [for encrypt].</param>
    /// <param name="input">The input.</param>
    /// <param name="parameters">The parameters.</param>
    /// <returns></returns>
    private byte[] BouncyCastleCrypto(bool forEncrypt, byte[] input, ICipherParameters parameters)
    {
        try
        {
            cipher.Init(forEncrypt, parameters);

            return this.cipher.DoFinal(input);
        }
        catch (CryptoException)
        {
            throw;
        }
    }

    /// <summary>
    /// Generates the initialization vector.
    /// </summary>
    /// <returns></returns>
    private byte[] GenerateInitializationVector()
    {
        using (var provider = new RNGCryptoServiceProvider())
        {
            // 1st block
            var result = new byte[this.blockCipher.GetBlockSize()];
            provider.GetBytes(result);

            return result;
        }
    }
}

I have a simple wrapper for the AES engine. It looks like this:

public class AesSha256Encryptor
{
    private readonly Encryptor<AesEngine, Sha256Digest> provider;

    /// <summary>
    /// Initializes a new instance of the <see cref="AesSha256Encryptor"/> class.
    /// </summary>
    /// <param name="key">The key.</param>
    /// <param name="hmacKey">The HMAC key.</param>
    public AesSha256Encryptor(byte[] key, byte[] hmacKey)
    {
        provider = new Encryptor<AesEngine, Sha256Digest>(Encoding.UTF8, key, hmacKey);
    }

    /// <summary>
    /// Encrypts the specified plain.
    /// </summary>
    /// <param name="plain">The plain.</param>
    /// <returns></returns>
    public string Encrypt(string plain)
    {
        return provider.Encrypt(plain);
    }

    /// <summary>
    /// Decrypts the specified cipher.
    /// </summary>
    /// <param name="cipher">The cipher.</param>
    /// <returns></returns>
    public string Decrypt(string cipher)
    {
        return provider.Decrypt(cipher);
    }
}

I wanted to be able to have a different salt per database row, so I have a key manager that works like this:

public static class EncryptionKeyManager
{
    /// <summary>
    /// The salt length limit
    /// </summary>
    private const int SaltLengthLimit = 32;

    /// <summary>
    /// Gets the key record.
    /// </summary>
    /// <returns></returns>
    public static KeyRecord GetKeyRecord()
    {
        // get the shared passphrasefrom appsettings
        var sharedPassphrase = GetSharedPassphrase();

        // get the client passphrase from config db to sign
        var clientPassphrase = GetClientPassphrase();

        // generate secure random salt
        var salt = GetSalt();

        // get both the encryption key and hmac key
        // these will be used for Encrypt-then-Mac
        var key = GetKeyFromPassphrase(sharedPassphrase, salt);
        var hmacKey = GetKeyFromPassphrase(clientPassphrase, salt);

        return new KeyRecord
        {
            SharedKey = key,
            HmacKey = hmacKey,
            Salt = salt
        };
    }

    /// <summary>
    /// Gets the client salt.
    /// </summary>
    /// <returns></returns>
    private static string GetClientPassphrase()
    {
        var settingsService = ServiceLocator.Current.GetInstance<ISettingService>();
        return settingsService.GetSetting(ConstantConfigSettings.EncryptionSettings.ClientPassphrase, defaultValue: "<removed>");
    }

    /// <summary>
    /// Gets the shared passphrase.
    /// </summary>
    /// <returns></returns>
    private static string GetSharedPassphrase()
    {
        return ConfigurationManager.AppSettings[ConstantConfigSettings.EncryptionSettings.SharedPassphrase] ?? "<removed>";
    }

    /// <summary>
    /// Gets the key from passphrase.
    /// </summary>
    /// <param name="passphrase">The passphrase.</param>
    /// <param name="salt">The salt.</param>
    /// <returns></returns>
    private static byte[] GetKeyFromPassphrase(string passphrase, string salt)
    {
        var saltArray = Encoding.UTF8.GetBytes(salt);
        var rfcKey = new Rfc2898DeriveBytes(passphrase, saltArray, 10000);

        return rfcKey.GetBytes(32); // for a 256-bit key (32*8=128)
    }

    /// <summary>
    /// Gets the salt from a secure random generator..
    /// </summary>
    /// <param name="maximumSaltLength">Maximum length of the salt.</param>
    /// <returns></returns>
    private static string GetSalt(int maximumSaltLength = SaltLengthLimit)
    {
        var salt = new byte[maximumSaltLength];
        using (var random = new RNGCryptoServiceProvider())
        {
            random.GetNonZeroBytes(salt);
        }

        return Convert.ToBase64String(salt);
    }
}

It all gets used like this to encrypt:

// get key and salt from 
var keyRecord = EncryptionKeyManager.GetKeyRecord();
var aesSha256Encryptor = new AesSha256Encryptor(keyRecord.SharedKey, keyRecord.HmacKey);

// now encrypt and store, include salt
entity.AccountNumber = aesSha256Encryptor.Encrypt(accountNumber);
entity.SortCode = aesSha256Encryptor.Encrypt(sortCode);
entity.Salt = keyRecord.Salt;

When I want to decrypt, I do the following:

public static class KeyManager
{
    /// <summary>
    /// Gets the key from passphrase.
    /// </summary>
    /// <param name="passphrase">The passphrase.</param>
    /// <param name="salt">The salt.</param>
    /// <returns>A byte array.</returns>
    public static byte[] GetKeyFromPassphrase(string passphrase, string salt)
    {
        var saltArray = Encoding.UTF8.GetBytes(salt);
        var rfcKey = new Rfc2898DeriveBytes(passphrase, saltArray, 10000);

        return rfcKey.GetBytes(32); // for a 256-bit key (32*8=128)
    }
}

var passphraseKey = KeyManager.GetKeyFromPassphrase(this.Passphrase, this.Salt);
var hmacKey = KeyManager.GetKeyFromPassphrase(this.ClientPassphrase, this.Salt);

var aesSha256Encryptor = new AesSha256Encryptor(passphraseKey, hmacKey);
var plaintext = aesSha256Encryptor.Decrypt(this.CipherText);

This is for a SAAS application. My basic idea was to have a passphrase that is core to the SAAS application that is used to encrypt/decrypt, but also have a specific client passphrase that is used to MAC. The reason for this was to spread the keys between endpoints (one in a database and one in a config setting). The salt gets saved to the database so that it can be used to decrypt using the same salt.

Can anyone see what I am doing wrong? Why am I getting the pad block error?

FYI: The passphrases are of the XKCD variety "horse-battery-stapler-correct" style, so they have hyphens in. I'm not sure if that is a red herring though.

I'm also not sure if the unique salt per row is required, or whether I could just hard code the salt? Is that overkill?

Update For anyone who finds this, the error was simply that the passphrase that I thought was being used was incorrect. The padding error was the result.

Community
  • 1
  • 1
Rebecca
  • 13,914
  • 10
  • 95
  • 136

2 Answers2

1

It is not clear what code exactly causes your problem (I mean there is no minimal example I could just run and see what is wrong), but I built an example which decrypts correctly without errors, based on your code, so you can look at it and probably spot your error. I made EncryptionKeyManager.GetSharedPassphrase() public, and it returns fixed string horse-battery-stapler-correct. I made EncryptionKeyManager.GetClientPassphrase() also public and it returns fixed horse-battery.

class Program {
    static void Main(string[] args) {
        // get key and salt from 
        var keyRecord = EncryptionKeyManager.GetKeyRecord();
        var aesSha256Encryptor = new AesSha256Encryptor(keyRecord.SharedKey, keyRecord.HmacKey);
        string targetData = "4343423343";

        var encrypted =  aesSha256Encryptor.Encrypt(targetData);
        var salt = keyRecord.Salt;
        var decrypted = Decrypt(encrypted, salt);
        Debug.Assert(targetData == decrypted);
        Console.WriteLine(decrypted);
        Console.ReadKey();

    }

    private static string Decrypt(string data, string salt) {
        var passphraseKey = KeyManager.GetKeyFromPassphrase(EncryptionKeyManager.GetSharedPassphrase(), salt);            
        var hmacKey = KeyManager.GetKeyFromPassphrase(EncryptionKeyManager.GetClientPassphrase(), salt);
        var aesSha256Encryptor = new AesSha256Encryptor(passphraseKey, hmacKey);
        var plaintext = aesSha256Encryptor.Decrypt(data);
        return plaintext;
    }
}
Evk
  • 98,527
  • 8
  • 141
  • 191
  • Thanks. I've put this into a small console app and it seems to work fine, but when I grab the actual data out of the database it doesn't. I'm starting to wonder if the problem lies with a corruption when the data is written to the database and then read back. The data type is current 'nvarchar(max)', where 'varchar(max)' would probably suffice for base64 encoded strings (although I'm not sure why that would make a difference using 'nvarchar'). – Rebecca Apr 11 '16 at 08:46
  • Well if you are storing binary data - use varbinary column, why convert to\from base64 and store as string? As for your problem, first check if your encrypt correctly. Grab exact encrypted data and salt from database and feed to your console app. If that is fine - then encrypt part is fine - add more logging to your decrypt process to check where exactly encrypted data becomes corrupt. – Evk Apr 11 '16 at 08:53
  • to be honest I don't trust the ORM I have to use, but I can give it a try. I've just run the application locally (SQL Server Express) and I've compared the base64 strings before they get written to the database and then after. The results are equal. Is there anything concerning database collation that could cause corrupted strings? – Rebecca Apr 11 '16 at 09:36
  • Well who knows, that is why I suggest to use binary column for binary data... But, does this base64 string got decrypted correctly in test console app? – Evk Apr 11 '16 at 09:41
  • Ok, I've found the error. The issue was that the passphrase I was expecting, was not correct. The padding error was the result. I bit of a red-herring. Your test code helped, so I'll mark your answer as the right one. – Rebecca Apr 11 '16 at 14:24
0

It's a bit disconcerting that you are getting a padding error, that means the mac validated or wasn't checked before decryption. I guess since you are using two different passphrases if you mac passphrase is correct and your encryption is off that could make sense.

The concern with padding errors being leaked are for chosen ciphertext attacks, so just scanning the overly complex class you borrowed it looks like the mac should be checked, however, I'd at least double check if you put in the incorrect mac passphrase that it won't give you a padding error, because if it does that means there is an issue with that class you are using.

Salt per row is the minimum if you are using passphrases, and not overkill. I prefer salt per encryption as you can see in my example.

The other thing you mentioned was spreading the keys by putting one in the database and one in the config. Splitting the storage of the encryption key from the mac key, is not a good way to do this if that is your the goal, but I'm a bit unclear to your goal. If your app was compromised in the database with an sql injection lets say, only the encryption key is needed to decrypt, the mac is there to just validate that the ciphertext hasn't been tampered with. It's much better to encrypt your ciphertext with decrypted keys stored in the database that were decrypted with keys stored in the config, if you just merely want to spread the key storage a little more.

Community
  • 1
  • 1
jbtule
  • 31,383
  • 12
  • 95
  • 128