Feature Request: Change AES Bit Rate without Modifying the Header

[shinn5112]

Would it be possible to add a mechanism for changing the AES bit rate from the default 128 without modifying the header? I have your library listed as a dependency for mine on Platformio, but I need 256bit encryption. My current solution is to just bundle this library inside of mine with the modified header, but I would rather leave this library on as a dependency so I do not need to repackage it in my own when an update comes out.

I would be more than happy to help if you would be interested!

[kokke]

Hi Patrick,

If I understand you correctly, it should be trivially possible.

I think adding a #ifndef-guard around the #define AES{128|192|256} in aes.h should make it possible for you to #define whichever bit-rate you want at #include-time.

Am I making sense ?

[shinn5112]

That makes sense. This is what I came up with so far, but it is giving me issues.

#define AES128 1

// this replaced the commented out defines for AES192/256
#ifdef bitrate256
  #define AES256 1
  #undef AES128
#endif
#ifdef bitrate192
  #define AES192 1
  #undef AES128
#endif

I've got some printlns that are conditionally compiled based on wether AES265/192/128 are defined. Based on those print statements, this works. However, I get issues when decrypting still. I am still pretty new to C++ and the preprocessor, so there is probably a rookie mistake in here somewhere.

[kokke]

I don't know the specifics of your project, but if you want to define at #include-time, which key-size you want to use, we can modify aes.h to look something like this:

#if   defined(AESBITRATE) && (AESBITRATE == 128)
  #define AES128 1
#elif defined(AESBITRATE) && (AESBITRATE == 192)
  #define AES192 1
#elif defined(AESBITRATE) && (AESBITRATE == 256)
  #define AES256 1
#else
  #define AES128 1
  //#define AES192 1
  //#define AES256 1
#endif

#define AES_BLOCKLEN 16 //Block length in bytes AES is 128b block only
#define Nb 4

#if defined(AES256) && (AES256 == 1)
    #define AES_KEYLEN      32
    #define AES_keyExpSize  240
    #define Nk              8
    #define Nr              14
#elif defined(AES192) && (AES192 == 1)
    #define AES_KEYLEN      24
    #define AES_keyExpSize  208
    #define Nk              6
    #define Nr              12
#else
    #define AES_KEYLEN      16   // Key length in bytes
    #define AES_keyExpSize  176
    #define Nk              4    // The number of 32 bit words in a key.
    #define Nr              10   // The number of rounds in AES Cipher.
#endif

Then, if you just did #include "aes.h", it would default to AES128.
You could also override the default keysize, by defining AESBITRATE to 128|192|256 and include the header, like this:

#define AESBITRATE 192
#include "aes.h"

That would compile the library using 192 bit keys.

This approach doesn't work with the current test.c because that file also depends on the AES128|192|256-macro.

Refactoring test.c so it uses AESBITRATE (or what we could choose to call a new macro), should make it work again. That would also give me a good excuse to close #142

When I get home tonight, I will try and see if I can test the proposed changes, and commit them if they work out as I expect them to.

If the above made any sense to you, please feel free to test the proposed changes, and let me know how it works out for you.

[shinn5112]

I will mess around with it here in a bit, thank you for helping me out! Also, as part of my pipeline, I run static code analysis on my library. When I packaged your code in with mine, I got a few hits on some of your code. It was nothing major, just some editorial stuff. You can view the results here: https://app.codacy.com/gh/Firmware-Forge/ESP8266-Updater/issues/ignored?bid=15961858

Since the issues deal with potential scope reductions, I don't think they are really worth fixing, but I figured that you would like to know.

I will test out your recommended changes and get back to you. Thanks again!

[shinn5112]

After testing this out, for some reason, it still gives me issues. I agree that this should work, but for some reason, it is not working with my current implementation unless I explicitly define AES256 as 1 outside of any preprocessor logic. I am printing out the values AES_KEYLEN and AES_keyExpSize as part of my code, and the values match what they should given the preprocessor that AESBITRATE is 256, however, the value I am decrypting is not decrypted properly.

I doubt that this has anything to do with it, but my implementation is taking a hash that is encrypted using a Python library (pycrytodome) and sending it to an ESP8266, which decrypts the hash and compares it to one that it has calculated locally. The source for that project can be viewed here, specifically the renew fingerprint method:
https://github.com/Firmware-Forge/ESP8266-Updater/blob/master/src/FFUpdates.cpp

I will mess around with it some more tomorrow to see if I can figure it out. The values determined using the preprocessor logic are set properly when defining AESBITRATE before including aes.hpp, but for some reason decryption still fails unless #define AES256 1 is hardcoded.