search

spaniakos / AES

AES for microcontrollers (Arduino & Raspberry pi)
http://spaniakos.github.io/AES/
GNU Affero General Public License v3.0
126 stars 55 forks source link

Last byte of plaintext should not be ignored. #16

Open Dzitu opened 7 years ago

Dzitu commented 7 years ago

Messages passed are byte arrays. There is no hint the array is NULL terminated string. Especially that length is given explicitly. Code should not assume final character is terminator in that case. Due to that substraction of 1 breaks the message (one byte of plaintext is ignored).

spaniakos commented 7 years ago

will review! thanks

spaniakos commented 6 years ago

the -1 have to be replaced in more than one function. will implement the changes , thanks for the hint.

palto42 commented 6 years ago

I checked the other functions in AES.cpp and don't think that there is any other function where the "-1" would need to be replaced. In my few this pull request can be accepted as is. Only other potential change is the description of this function in AES.h which currently states _"* @param psize the size of the byte array ex sizeof(plaintext)". It might be useful to add a note that the sizeof() function may be incorrect in case of strings. For example byte plain[] = "0123456789" will have sizeof(plain) = 11 since it was created from a string, but strlen(plain) = 10 would be correct.

spaniakos commented 6 years ago

i will test the code. you are correct about string. the problem with the strings are that the \0 is calculated as a character. but since the library is supposed to handle bytes and not strings, it is better to have a function that will handle byte arrays and a separate function that will handle strings. the seconds function should have the -1 automatically when a string is given and should be a able to calculate the size on its own.

now for the padding, i saw that you you have made the padding array 0x01 to 0x10 that is also correct. as by using this kind of padding 0x10 = 16 the algorithm will be pkcs7 compliant. ( as it should have been from the beginning).

nevertheless, i will test your code against the tests and afterwards will accept the pull request.

palto42 commented 6 years ago

Hi @spaniakos I've started to review the joint changes of #15 and #16 and plan to submit a pull in the next days. If you're interested, you can have a look at my branch "pad_fix" https://github.com/spaniakos/AES/compare/master...palto42:pad_fix (not tested yet).

spaniakos commented 6 years ago

will sure do!