search

kokke / tiny-AES-c

Small portable AES128/192/256 in C
The Unlicense
4.3k stars 1.3k forks source link

AES256 is broken. #185

Closed kooscode closed 3 years ago

kooscode commented 3 years ago

It seems there is code in the KeyExpansion function that is broken for AES256?

if AES256 is defined, the Nk value will be 8.. so this code will never run??

https://github.com/kokke/tiny-AES-c/blob/3f69a5899e58e2e398e8c32ce7b3a954dd593ed4/aes.c#L205

ali-tafakkori commented 3 years ago

+1

zoffixznet commented 3 years ago

I see differently: the i in that loop goes up from Nk (8) upto Nb * (Nr + 1) (84), so the code you pointed out will run 10 times.

kokke commented 3 years ago

Okay so let's recap here @kooscode @zoffixznet and @ali-tafakkori

// The number of columns comprising a state in AES. This is a constant in AES. Value=4
#define Nb 4

#if defined(AES256) && (AES256 == 1)
    #define Nk 8
    #define Nr 14
#elif defined(AES192) && (AES192 == 1)
    #define Nk 6
    #define Nr 12
#else
    #define Nk 4        // The number of 32 bit words in a key.
    #define Nr 10       // The number of rounds in AES Cipher.
#endif

Nb is always 4 Nk is 4, 6 or 8 and Nr is 10, 12 or 14 depending on whether you're running AES in 128, 192 or 256 bit mode.

The for-loop is defined like this:

  for (i = Nk; i < Nb * (Nr + 1); ++i)

So i runs from AES128: 4 -> 4 (10 + 1) -1 = 43 AES192: 6 -> 4 (12 + 1) -1 = 51 AES256: 8 -> 4 * (14 + 1) -1 = 59 ... with increments of 1 (++i).

This if-statement is only defined for AES256, where Nk is 8, so this if-statement

 if (i % Nk == 4)

will be true when i is 12, 20, 28, 36, 44 and 52.

So no, AES256 is not broken and yes the if-statement gets executed lots of times.