ThealeMW / slowaes

Slowaes
0 stars 0 forks source link

shouldn't need originalsize when decrypting #6

Closed GoogleCodeExporter closed 8 years ago

GoogleCodeExporter commented 8 years ago
See attachment for a modified version of decrypt that doesn't require 
originalsize as a parameter.

Original issue reported on code.google.com by abraham....@gmail.com on 14 Oct 2009 at 4:09

Attachments:

GoogleCodeExporter commented 8 years ago
[deleted comment]
GoogleCodeExporter commented 8 years ago
This fix did not work correctly with the following test vectors:

PT as Hex: 00112233445566778899aabbccddeeff
Key as Hex (128 Bit): 0f1571c947d9e8590cb7add6af7f6798
IV as Hex: d1671e68ea1f0f231918309301d36a49
Mode: CBC

Encrypted: 83242e768abc01dc0be13f58adfc545e
Decrypted: [Empty]

ByteArray returned by Decrypt() after applying this version is empty [].

Original comment by cougar...@gmail.com on 12 Feb 2010 at 5:35

GoogleCodeExporter commented 8 years ago
Perhaps I'm missing something due to my lack of knowledge in the area, but if 
you pad it you don't need to know the size and can pass in None for it?

Most messages could be padded I would say without a big performance hit; but 
then I was thinking about padding files and that they would double in size.

Then I thought to myself, if you read in chunks of 16, when you get one that is 
less than 16 you know it is the end of the file and pad it.  Then during 
decryption you can just divide the total size by 16 and on the last one try to 
un-pad it, if it fails catch the exception and leave it alone because that 
should mean the original file was a multiple of 16 and wasn't padded at all.

But maybe I'm missing something.  And yes I know this was over a year ago.  
Just writing this in case someone else stumbles upon it.

Original comment by nom...@gmail.com on 28 Dec 2010 at 6:43

GoogleCodeExporter commented 8 years ago
You can certainly decrypt AES messages without knowing the original size of the 
message.  Implementations in other libraries do not require it as a parameter.  
I don't know why my change fails with the given test vectors... I never looked 
into it.  I no longer work for the company that was using this library, and I 
don't even know whether they are still using it or not either.  If someone else 
wants to try to fix up the method to work without originalSize, go ahead.

Original comment by abraham....@gmail.com on 28 Dec 2010 at 8:12

GoogleCodeExporter commented 8 years ago
/*
This is a pretty naive change that I haven't tested yet, it appears like it 
might work, this assumes an 16byte block size.
*/
else if(mode == this.modeOfOperation.CBC)
{
        output = this.aes.decrypt(ciphertext, key, size);
        for (i = 0; i < 16; i++)
                byteArray[i] = ((firstRound) ? iv[i] : input[i]) ^ output[i];
        firstRound = false;
        for(var k = 0;k < end-start;k++)
                bytesOut.push(byteArray[k]);
        var padByte = -1;
        var padCount = 0;
        for (var k = bytesOut.length - 1; k > bytesOut.length - 8; k--) {
                if (bytesOut[k] < 16) {
                        if (padByte == -1)
                                padByte = bytesOut[k];
                        if (bytesOut[k] != padByte) {
                                padCount = 0;
                                break;
                        }
                        padCount++;
                } else {
                        break;
                }
                if (padCount == padByte)
                        break;
        }
        if (padCount > 0)
            bytesOut.length = bytesOut.length - padCount;
        input = ciphertext;

Original comment by pfngu...@gmail.com on 4 Mar 2011 at 2:44

GoogleCodeExporter commented 8 years ago
Hmm, that doesn't look correct, need to move the padding check out of the for 
loop... but the concept is sound...

Original comment by pfngu...@gmail.com on 4 Mar 2011 at 2:49

GoogleCodeExporter commented 8 years ago
Ok, I made a proper diff against the current trunk version.

I also removed the required keysize argument.

Is there any reason to return all the input arguments from encrypt() and not 
just the cipherOut?  Seems pretty silly.

Original comment by pfngu...@gmail.com on 4 Mar 2011 at 4:20

Attachments:

GoogleCodeExporter commented 8 years ago
Wow, it's even more broken than I thought... padding needs to occur at the end 
of bytesIn, not on every block (if bytesIn % 16 == 0, then other decryptors may 
fail [e.g. c#, java])--if anyone is interested, I'll post up a new diff, but 
this project seems somewhat dead.

Original comment by pfngu...@gmail.com on 7 Mar 2011 at 7:52

GoogleCodeExporter commented 8 years ago
I committed r39 which should fix this

Original comment by pfngu...@gmail.com on 8 Mar 2011 at 4:36