Padding is invalid and cannot be removed - AES yet another

Question

The code below is the latest iteration in an attempt to build a simple encryption/decryption utility for an in-house project. I'm hoping it's something very silly that I'm simply too far into a trench to see.

It's in .Net 4.7 and I've tried to organise it to make it impossible to accidentally have different keys or options occurring during the process. The code should on Linqpad.

Some things I've tried are listed in the TODOs though I've not document all the daft variations I found today.

If anyone else can make it run on Linqpad or Visual studio 2017, I'd be very keen to learn what I am missing here.

Thanks for any advice!

'Linqpad version - should offer to import System.Security.Cryptography
'I'm on .net 4.7 in my main project hence the Tuple setup below.

Dim Testphrase = "Whatever is happening, nothing I google works!"

Sub Main
    Dim encrypted = EncryptString(Testphrase, "Password")
    Dim Decrypted = DecryptString(encrypted, "Password")
    Decrypted.Dump
End Sub

 Private Shared AesManaged As AesManaged 
 Private Shared password As  Rfc2898DeriveBytes

'''Makes sure that I use the same parameters both sides of the equation

Private Shared Function GetCryptBits(passphrase As String) As (passwords As Rfc2898DeriveBytes, AesManaged As AesManaged)
    If AesManaged Isnot Nothing Then Return (password, AesManaged) 'TODO: Don't rebuild this if you've already got it.
    password =  New Rfc2898DeriveBytes(passphrase, Encoding.UTF8.GetBytes("SaltBytes")) 
    AesManaged =  New AesManaged() With {.Mode = CipherMode.CBC, 
                                                           .Padding = PaddingMode.PKCS7,
                                                           .BlockSize = 128, .KeySize = 256} 'TODO: I've tried all the padding choices here.TODO: Have tried RijndaelManaged
    AesManaged.IV = password.GetBytes(AesManaged.BlockSize / 8) 
    AesManaged.Key = password.GetBytes(AesManaged.KeySize / 8)
    Return (password, AesManaged)
End Function

'Encrypt
Public Shared Function EncryptString(plainText As String, passPhrase As String) As String
    Dim B = GetCryptBits(passPhrase)
    With B
        Dim plainTextBytes As Byte() = Encoding.UTF8.GetBytes(plainText) 'TODO: Where you see UTF8 I've also tried with Unicode
        Dim encryptor As ICryptoTransform = B.AesManaged.CreateEncryptor(B.AesManaged.Key, B.AesManaged.IV)
        Using MemoryStream As New MemoryStream()
            Using CryptoStream As New CryptoStream(MemoryStream, encryptor, CryptoStreamMode.Write)
                CryptoStream.Write(plainTextBytes, 0, plainTextBytes.Length)
                CryptoStream.FlushFinalBlock()
                Dim cipherTextBytes As Byte() = MemoryStream.ToArray()
                AesManaged.clear
                Return Convert.ToBase64String(cipherTextBytes)
                MemoryStream.Close()
                CryptoStream.Close()
            End Using
        End Using
    End With
End Function

'Decrypt
Public Shared Function DecryptString(cipherText As String, passPhrase As String) As String
    Dim B = GetCryptBits(passPhrase)
    With B
        Dim cipherTextBytes As Byte() = Convert.FromBase64String(cipherText) 
        Dim decryptor As ICryptoTransform = B.AesManaged.CreateDecryptor(B.AesManaged.Key, B.AesManaged.IV)
        Using MemoryStream As New MemoryStream

                Using CryptoStream As New CryptoStream(MemoryStream, decryptor, CryptoStreamMode.Read)
                    Dim plainTextBytes As Byte() = New Byte(cipherTextBytes.Length - 1) {}
                    Dim decryptedByteCount As Integer = CryptoStream.Read(plainTextBytes, 0, plainTextBytes.Length) 'TODO: Here I'm getting "Padding is invalid and cannot be removed."
                    Return Encoding.UTF8.GetString(plainTextBytes, 0, decryptedByteCount)
                End Using
        End Using
    End With
End Function

While that error can have to do with the padding, most often it seems to simply mean the decryption failed. One thing is that it is generally not a good idea to derive the IV from the password, but a set of cryptographically random bytes. PBKDF mitigates that a little, but they are still determinative. — Ňɏssa Pøngjǣrdenlarp, Nov 24 '17 at 17:17
The decryption code forgets to initialize the memory stream, MemoryStream As New MemoryStream(cipherTextBytes) to fix. — Hans Passant, Nov 24 '17 at 17:36

score 3 · Accepted Answer · answered Nov 24 '17 at 17:39

Below is a corrected version of your code. I am not a VB.Net programmer and so I am not saying this is pretty or best style, etc, but it is working. Your issue was related to two things. One, you are clearing the AesManaged object in your encrypt function and then trying to use it again in your Decrypt function. The other issue was how you did the decrypt. If you look back at my answer in which you lead me here :) you will see the difference between your decrypt and mine. In yours, you were assuming the decrypted bytes would be equal to the length of the encrypted bytes (this line of code here Dim plainTextBytes As Byte() = New Byte(cipherTextBytes.Length - 1) {}), but what you need to do is decrypt in a loop, reading a block of bytes and appending it to your output until there are no more bytes to read.

Edit And as Hans Passant pointed out (and I forgot to mention) you need to initialize the decryption stream with the actual data.

Dim Testphrase = "Whatever is happening, nothing I google works!"

Sub Main
    Dim encrypted = EncryptString(Testphrase, "Password")
    encrypted.Dump
    Dim Decrypted = DecryptString(encrypted, "Password")
    Decrypted.Dump
End Sub

Private Shared AesManaged As AesManaged
Private Shared password As Rfc2898DeriveBytes

'''Makes sure that I use the same parameters both sides of the equation

Private Shared Function GetCryptBits(passphrase As String) As (passwords As Rfc2898DeriveBytes, AesManaged As AesManaged)
    If AesManaged Isnot Nothing Then Return (password, AesManaged) 'TODO: Don't rebuild this if you've already got it.
    password = New Rfc2898DeriveBytes(passphrase, Encoding.UTF8.GetBytes("SaltBytes"))
    AesManaged = New AesManaged() With {.Mode = CipherMode.CBC,
                                                           .Padding = PaddingMode.PKCS7,
                                                           .KeySize = 256} 'TODO: I've tried all the padding choices here.TODO: Have tried RijndaelManaged
    Dim iv As Byte() = password.GetBytes(AesManaged.BlockSize / 8)
    Dim key As Byte() = password.GetBytes(AesManaged.KeySize / 8)
    AesManaged.IV = iv
    AesManaged.Key = key
    Return (password, AesManaged)
End Function

'Encrypt
Public Shared Function EncryptString(plainText As String, passPhrase As String) As String
    Dim B = GetCryptBits(passPhrase)
    With B
        Dim plainTextBytes As Byte() = Encoding.UTF8.GetBytes(plainText) 'TODO: Where you see UTF8 I've also tried with Unicode
        Dim encryptor As ICryptoTransform = B.AesManaged.CreateEncryptor(B.AesManaged.Key, B.AesManaged.IV)
        Using MemoryStream As New MemoryStream()
            Using CryptoStream As New CryptoStream(MemoryStream, encryptor, CryptoStreamMode.Write)
                CryptoStream.Write(plainTextBytes, 0, plainTextBytes.Length)
                CryptoStream.FlushFinalBlock()
                Dim cipherTextBytes As Byte() = MemoryStream.ToArray()
                Return Convert.ToBase64String(cipherTextBytes)
                MemoryStream.Close()
                CryptoStream.Close()
            End Using
        End Using
    End With
End Function

'Decrypt
Public Shared Function DecryptString(cipherText As String, passPhrase As String) As String
    Dim B = GetCryptBits(passPhrase)
    With B
        Dim cipherTextBytes As Byte() = Convert.FromBase64String(cipherText)
        Dim decryptor As ICryptoTransform = B.AesManaged.CreateDecryptor(B.AesManaged.Key, B.AesManaged.IV)
        Using MemoryStream As New MemoryStream(cipherTextBytes)
            Using OutputStream As New MemoryStream()
                Using CryptoStream As New CryptoStream(MemoryStream, decryptor, CryptoStreamMode.Read)
                    Dim plainTextBytes As Byte() = New Byte(1024) {}
                    Dim read As Integer = CryptoStream.Read(plainTextBytes, 0, plainTextBytes.Length)
                    While read > 0
                        OutputStream.Write(plainTextBytes,0, read)
                        read = CryptoStream.Read(plainTextBytes, 0, plainTextBytes.Length)
                    End While
                    Return Encoding.UTF8.GetString(OutputStream.ToArray())
                End Using                
            End Using
        End Using
    End With
End Function

that's the answer - sweet thank you very much for both the answer and explanation :). And it works in the live code! — Richard Griffiths, Nov 24 '17 at 17:45
I might add, for a not VB programmer the only edits I made were to slot into my project-it worked as is in linq pad, so thank you. — Richard Griffiths, Nov 25 '17 at 06:42
The as managed.clear actually came from another another somewhere where the user was told that needed to be done. It was another attempt. I simply haven't used encryption much in. Net, yet. — Richard Griffiths, Nov 25 '17 at 06:43
@RichardGriffiths - In general, you put those objects in a `Using` block or make a `Dispose` call on them so that the resources are cleaned up. In the case of your sample though, since you are reusing it in the decrypt call, you don't want to clear it. — pstrjds, Nov 25 '17 at 08:10
This version I posted here had a few modifications from my first shots at it. I'd not had premature dispose in the earlier ones, simply put it in due to casting about online for answers. — Richard Griffiths, Nov 26 '17 at 16:20

Padding is invalid and cannot be removed - AES yet another

1 Answers1

Linked