Thank you Matt and Dave.
Matt,
Yes I agree that I should be calling EncryptInit and EncryptFinal only
once. That is one of the mistakes. The reason why I did that was, I am
exposing a encryption API to other functions in the project.
They dont care how encryption is done. They have a buffer ( usualLy a page
size of 4k) and repeatedly encrypt the whole buffer x number of times. The
intention is they should not see any of the openssl stuff.
So I consolidated everything into a single API. It actually worked when I
was using low level aes APIs:
AES_set_encrypt_key
AES_cbc_encrypt
These two could be called any number of times in the loop.
This is not the case with EVP APIs. I can separate the init and key
generation part and make it happen only once.
But calling EVP_EncryptUpdate only in the loop means, I have to expose it
outside.
Yes 32 is a very small chunk indeed. This was my test program. In the
project we do larger sizes.
I had the impression that the key and IV should be of 256 bits ( 32) since
its aes 256 mode. I might be wrong.
Will keep the IV to 16 only. The IV is very simple for now. But will make
it more difficult to predict.
I hash the key using SHA256. This is how my generatehash method looks like:
unsigned char* generateHash( unsigned char *dataToHash, int dataSize )
{
// setFIPSMode();
unsigned char *returnBuffer;
EVP_MD_CTX mdctx;
const EVP_MD *md;
unsigned int md_len, i;
OpenSSL_add_all_digests();
md = EVP_get_digestbyname("SHA256");
returnBuffer = (unsigned char *)calloc( EVP_MAX_MD_SIZE,
sizeof(unsigned char));
EVP_MD_CTX_init(&mdctx);
EVP_DigestInit_ex(&mdctx, md, NULL);
EVP_DigestUpdate(&mdctx, dataToHash, dataSize);
EVP_DigestFinal_ex(&mdctx, returnBuffer, &md_len);
EVP_MD_CTX_cleanup(&mdctx);
return returnBuffer;
}
As for the malloc errors are concerned, I guess they might be because of
calling EncryptInit and EncryptFinal in a loop.
Dave,
Passing enLength to EVP_EncryptUpdate is a mistake. That should correct
malloc errors.
The reason I am trying to expose a single API for encryption is to avoid a
lot of code changes for those who are consuming it now.
Is that possible with EVP API?
Thanks,
Tarani
On Thu, Apr 25, 2013 at 6:46 PM, Matt Caswell <fr...@baggins.org> wrote:
> 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
>
