Closed GoogleCodeExporter closed 8 years ago
[deleted comment]
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
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
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
/*
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
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
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:
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
I committed r39 which should fix this
Original comment by pfngu...@gmail.com
on 8 Mar 2011 at 4:36
Original issue reported on code.google.com by
abraham....@gmail.com
on 14 Oct 2009 at 4:09Attachments: