search

dhuertas / AES

AES algorithm implementation in C
MIT License
615 stars 323 forks source link

fix a bug #2

Closed Alfred1992 closed 8 years ago

Alfred1992 commented 8 years ago

hi, here is a fix, see if you agree

dhuertas commented 8 years ago

Thanks! Really looking forward to include them. Just a few comments though:

Commit 6f491b3be10f77034be02aa41d82f90da1d0c7d7 looks good (although AES specification forces 4 columns, it is a bug nonetheless). As a suggestion: being number 4 a common multiplier for the corresponding round key part I'd rather use state[Nb*k+c] = state[Nb*k+c]^w[4*(Nb*r+c)+k] instead for ease of reading (where k takes values 0,1,2 and 3 for the corresponding byte in the 32 bit word). Just my two cents ;)

Commit 5ad2275b35df366661e9896973c773c9d31d3294 looks kinda odd to me. For instance, taking "A.1 Expansion of a 128-bit Cipher Key" from FIPS 197 document as an example: for i = 36 and 40, Rcon takes i/Nk (where Nk = 4, so 9 and 10 respectively). Then bit shifting 0x01 << 8 and 0x01 << 9 both are 0x00 since it is not an exponentiation in GF(2^8) (should be 0x1b and 0x36).

Please, review and comment.

dhuertas commented 8 years ago

Closing this pull request, as the commit that solves the bug has already been merged.

Alfred1992 commented 7 years ago

You are right about the Commit 5ad2275