On 25 April 2013 21:42, Taraniteja Vishwanatha <taranit...@gmail.com> wrote:
> Hey guys,
>
> I was using the low level aes APIs and now have switched to EVP ones. My

Good. That is (in most cases) the correct approach.


> string encryption and decryption always work fine. But when it comes to
> files, I am getting malloc errors: malloc: *** error for object : incorrect
> checksum for freed object - object was probably modified after being freed.
>
> Here is my code for encryption ( decryption is very similar). I am trying to
> call this in a loop whenever I have a file to encrypt 32 bytes of data each
> time. Is this teh correct way to do it?
> Please advice.

Well its a little unclear what you are trying to achieve, but probably
it is not the correct approach. If you are simply looking to encrypt
the whole file then you should not be calling EncryptInit and
EncryptFinal repeatedly. Call EncryptInit_ex once at the beginning,
and EncryptFinal_ex once at the end. Call EncryptUpdate_ex in a loop
if you need to (but 32 bytes at a time seems quite small you can
afford to do more than that in one go if you want to).

What you are actually doing is encrypting the file in lots of small
32-byte chunks - each one with its own IV (which will be another 16
bytes), and padding (which if the input is exactly 32 bytes, then the
padding will be exactly 16 bytes). i.e. you are having to store a
total of 64 bytes of output for 32 bytes of input. This is quite
inefficient! If you are intentionally doing it this way for some
reason (such as to allow random access to the file), then CBC is
probably not the correct mode choice.

>
> unsigned char* encryptBlockAES(unsigned char *plainText, int dataLength, int
> *outLength,const unsigned char* keyData, int pageNo)
> {
>
>     unsigned char key[AES_BLOCK_SIZE*2], iv[AES_BLOCK_SIZE*2];


This looks odd. The key length is independent of the block size - so
why define it in terms of that? It just so happens that for your
particular cipher the key length is twice the block size - but that's
just coincidence. The IV on the other hand only needs to be equal to
the block size - not double it.


>     EVP_CIPHER_CTX enCtx;
>     int enLength = 0;
>     unsigned char* encryptedString;
>     int outLen;
>     int tempLen;
>
>     enLength = dataLength + (AES_BLOCK_SIZE);
>     memset(iv, pageNo, AES_BLOCK_SIZE*2); //Hard coded for now. Will become
> a parameter soon

I don't know what pageNo represents but it suggests to me that you
might be using a predictable IV. If an attacker can predict what IV
you are going to use next then this can open yourself up to certain
types of attacks. When using CBC mode your IVs should generally not be
predictable. If you need to use a predictable IV, then CBC mode might
not be the right mode for you.

>     memset(key, 0, AES_BLOCK_SIZE*2);
>
>     unsigned char* tempKey = generateHash((unsigned char*)keyData,
> AES_BLOCK_SIZE*2);

I don't know where you have obtained your key data, or how
generateHash works. This looks reasonable if you have obtained you key
material from some key agreement protocol (e.g. DH or ECDH). However
if your key material is a password or similar then you should be using
some form of password based key derivation function. You've said above
that you are calling this in a loop. Assuming that the key is constant
throughout the whole loop, then you probably don't want to be
generating your key in this way every time (do it once up front).


>     if (tempKey == NULL)
>     {
>         fprintf(stderr, "Unable to generate key \n");
>         exit(-1);
>     }
>     memmove(key,tempKey, AES_BLOCK_SIZE*2);
>
>     // alloc encrypt_string
>     encryptedString = (unsigned char*)calloc(enLength, sizeof(unsigned
> char));
>     if (encryptedString == NULL)
>     {
>         fprintf(stderr, "Unable to allocate memory for encrypt_string\n");
>         exit(-1);
>     }
>
>     EVP_CIPHER_CTX_init(&enCtx);
>     EVP_EncryptInit_ex(&enCtx, EVP_aes_256_cbc(), NULL, key, iv);
>
>     EVP_EncryptUpdate(&enCtx, encryptedString, &outLen, plainText,
> enLength);
>     EVP_EncryptFinal_ex(&enCtx, encryptedString + outLen, &tempLen);
>     *outLength = outLen + tempLen;
>     EVP_CIPHER_CTX_cleanup(&enCtx);
>     return encryptedString;
> }

I can't see an obvious memory corruption issue in your code (maybe
others on this list can spot something). You haven't shared with us
the rest of your code where the encryptedString gets freed, or what
happens to it in between. I would go through your code line by line
looking for a memory corruption.

Matt
______________________________________________________________________
OpenSSL Project                                 http://www.openssl.org
User Support Mailing List                    openssl-users@openssl.org
Automated List Manager                           majord...@openssl.org

Reply via email to