Wrapping AES within functions with String input/output

Picking up from #7, here's the actual issue I am having.

Basically, I want to create encryption and decryption functions which take in both the data and key as Strings and similarly return the output as a String. This is done in order to keep things simple in my project code, since I will be sending/receiving the data in String format, which means I can abstract from the required type conversions . Here's my attempt:

#include <AES.h>

String key = "0123456789010123";
String plain = "Add NodeAdd NodeAdd NodeAdd NodeAdd Node";
String cipher, check;

String AES_encrypt(String plain, String key) {
  AES aes ;
  byte * plain_buf = (unsigned char*)plain.c_str();
  byte * key_buf = (unsigned char*)key.c_str();
  // add padding where appropriate
  int cipher_length = (plain.length()+1 < 8) ? 8 : plain.length()+1 + (8 - (plain.length()+1) % 8); 
  byte cipher_buf[cipher_length];
  aes.do_aes_encrypt(plain_buf, plain.length() + 1, cipher_buf, key_buf, 128);
  String cipher = String((char*)cipher_buf);
  return cipher;
}

String AES_decrypt(String cipher, String key) {
  AES aes ;
  byte * cipher_buf = (unsigned char*)cipher.c_str();
  byte * key_buf = (unsigned char*)key.c_str();
  int plain_length = cipher.length();
  byte plain_buf[plain_length];
  aes.do_aes_decrypt(cipher_buf, cipher.length(), plain_buf, key_buf, 128);
  String plain = String((char*)plain_buf);
  return plain;
}

void setup ()
{
  Serial.begin (57600) ;
  delay(500);
  printf("\n===testng mode\n") ;

  Serial.print("\nPLAIN: "); Serial.println(plain);
  cipher = AES_encrypt(plain, key);
  Serial.print("CIPHER: "); Serial.println(cipher);
  check = AES_decrypt(cipher, key);
  Serial.print("CHECK: "); Serial.println(check);
  Serial.println("============================================================");
  //  otfly_test () ;
  //  otfly_test256 () ;
}

void loop ()
{

}

However, here's the result.....

PLAIN: Add NodeAdd NodeAdd NodeAdd NodeAdd Node
CIPHER: x©W;
d“ÖƒßÇs¦uù®˜öOËép^|»î•kn†0Ù£±³
CHECK: Add NodeAdd NodeAdd NodeAdd Node|ôþ?pôþ?ˆôþ?½ @

============================================================

Not sure what I am doing wrong... I am guessing there is something wrong in my conversion from String to byte array and vice versa. Also, I still cannot grasp the solution for issue #7, which means there could be some relevance.

Any help would be much appreciated.

it seems that you are encrypting using 48+1 length buffer on 40 length string. can you try to change the line: int cipher_length = (plain.length()+1 < 8) ? 8 : plain.length()+1 + (8 - (plain.length()+1) % 8); to: int cipher_length = plain.length(); and tell me the results? The padding is handled from the functions.

Thanks for the response. Unfortunately, this didn't fix the issue and I am not too sure the above code should be replaced anyways. All I am basically doing is defining the size of the cipher byte array, such that the entire ciphered text (including padding) can fit inside it. Otherwise, the size of the array will be the same as the length of the plain text, which means that it will most probably not be a multiple of 8 bytes.

Now, I have added a function within my fork of your library (named printToString(byte[] output, bool p_pad)), which basically follows the structure of printArray(byte[] output, bool p_pad), but instead of printing to Serial, it simply appends to a String (see here).

Using the above, I have done some further tests and here's what I found:

First, an "iv" must be set (aes.set_IV(my_iv);) in order for the above to work. Even if the iv is not passed to the aes.do_aes_encrypt and aes.do_aes_decrypt functions. Since I am not too sure if AES is meant to work without an iv anyways, I assume that's something I've missed.
Second (and probably most important), if I use one global AES instance then everything works fine. If however, I create separate AES instances within each of the encryption and decryption functions (as I have done in the example a provided in my initial post) it does not.

Here's a working code example to contrast with the previous one I supplied that doesn't. In the example below, to recreate the problem one can simply comment the global AES aes; definition and uncomment the ones within each one of the functions.

#include <AES.h>

String key_0 = "0123456789010123";
String plain_0 = "Add NodeAdd NodeAdd NodeAdd NodeAdd Node";
String cipher_0, check_0;

/* Now iv has been added */
unsigned long long int my_iv = 36753562;

/* AES global instance (comment out to recreate problem)*/
AES aes ;

String AES_encrypt(String plain, String key) {
  /* Local AES instance (uncomment to recreate problem) */
  //AES aes ; 
  aes.set_IV(my_iv);
  byte * plain_buf = (unsigned char*)plain.c_str();
  byte * key_buf = (unsigned char*)key.c_str();
  // add padding where appropriate
  int cipher_length = (plain.length()+1 < 8) ? 8 : (plain.length()+1) + (8 - (plain.length()+1) % 8);
  byte cipher_buf[cipher_length];
  aes.do_aes_encrypt(plain_buf, plain.length() + 1, cipher_buf, key_buf, 128);
  String cipher = aes.printToString(cipher_buf, false);//String((char*)cipher_buf);
  return cipher;
}

String AES_decrypt(String cipher, String key) {
  /* Local AES instance (uncomment to recreate problem) */
  //AES aes ; 
  aes.set_IV(my_iv);
  byte * cipher_buf = (unsigned char*)cipher.c_str();
  byte * key_buf = (unsigned char*)key.c_str();
  int plain_length = cipher.length();
  byte plain_buf[plain_length];
  aes.do_aes_decrypt(cipher_buf, cipher.length(), plain_buf, key_buf, 128);
  String plain = aes.printToString(plain_buf, true);
  return plain;
}

void setup ()
{
  Serial.begin (57600) ;
  delay(500);
  printf("\n===testng mode\n") ;

  Serial.print("\nPLAIN: "); Serial.println(plain_0);
  cipher_0 = AES_encrypt(plain_0, key_0);
  Serial.print("CIPHER: "); Serial.println(cipher_0);
  check_0 = AES_decrypt(cipher_0, key_0);

  Serial.print("CHECK: "); Serial.println(check_0);
  Serial.println("============================================================");
  //  otfly_test () ;
  //  otfly_test256 () ;
}

void loop ()
{

}

Results using global instance:

PLAIN: Add NodeAdd NodeAdd NodeAdd NodeAdd Node
CIPHER: ÜÒ23zòÙùrµÑÀõÓÀÍ‹D™Ûlµ¹ÕºjåN€ÙÚí0ÔÒ%ã€ë �
CHECK: Add NodeAdd NodeAdd NodeAdd NodeAdd Node
============================================================

Results using local instances:

PLAIN: Add NodeAdd NodeAdd NodeAdd NodeAdd Node
CIPHER: ÜÒ23zòÙùrµÑÀõÓÀÍ‹D™Ûlµ¹ÕºjåN€ÙÚí0ÔÒ%ã€ë �
CHECK: Add NodeAdd NodeAdd NodeAdd NodeAdd Node
0123456789010123öñ7Â'Çú÷1Ê/Åá´†C#“ACÙ�¶r¢spß;×>ü¨–}%% 6‡SÀÖ;<~“F[³I/Üà6V7.jI“hs !\ÎÀý�ðd—ÄcäÖC-¸ƒ:ap‡¥csP)ÛkÓï±h¢µNœÐº��Y~Áhû`tc*.è³�³gV4û¬
============================================================

Are there any internal variables stored within the instance between encryption and decryption that could have an effect?

Using the same instance to make things work does not solve the problem, as encryption and decryption should be done by different devices.

as i see in your results:

PLAIN: Add NodeAdd NodeAdd NodeAdd NodeAdd Node
CIPHER: �ÜÒ23zòÙùrµÑÀ�õÓ�ÀÍ‹D™Û�lµ¹Õºj�åN€ÙÚí�0ÔÒ%ã�€ë �
CHECK: Add NodeAdd NodeAdd NodeAdd NodeAdd Node��������
0123456789010123ö�ñ7Â'Çú�÷1Ê/Å�á´†C#“ACÙ�¶r�¢spß;×>ü¨–}%% �6‡S�ÀÖ�;<~“F�[³I/Üà6V7.jI“hs� !\ÎÀ�ý�ðd—ÄcäÖC-¸�ƒ:�ap�‡¥��csP)ÛkÓ�ï�±h¢�µ�NœÐº��Y~Áhû`tc*.è³�³gV4û¬
============================================================

the check has all the unencrypted string followed by random characters. this happens as Serial.println(check_0); prints all the data in the buffer until it finds a terminating character ( \0 ) for strings. Thats why you need to print using the provided function.

The encryption methods are not ment to use string but buffers. a string is just an array of characters followed by the terminating character \0 The only thing you miss here is the terminating character.

When you transmit information you always have to transfer :

The cyphered string
The IV
The size of the string

There is a rule is cryptography that says ::

Everything is known except the key. The only secret is the key

Thats why the function is set up following the standards.

I have a framework on going the follows the exact same specifications and is tested using Rpi / arduino / intel galileo that encrypts and decrypts correctly using the pre-defined function. I hope i will release it in the near future so all this issues can be resolved.

Until then, if you want to decrypt Strings i suggest that you either print using the predefined function and transmitting all the required information or that you add a \0 after the end of the string. This happens cause the function are using bit by bit encryption and low level copy function that does not copy a string to an other string but rather manipulating the bytes in the memory.

For example

String plain = "Add Node"
in memory: Add Node\0<random data>

char* plain = "Add Node" 
in memory: Add Node<random data>

as you see you use byte * cipher_buf therefore it does not hold the \0 in the end. Therefore, if you try to print it as a string it will print All the data in the memory sequence until it find an \0 terminating character.

I hope i am helping you with this information.

there is a fork that work perfectly in ESP8266: https://github.com/shaneapowell/AES

i will soon add this commit to the library.

Ok, I think I get it now! Thanks much for all the explanation.

I solved this one by simply appending the length of the original message to the cipher text before transmitting the data.

I also identified another problem related to converting from byte arrays to Strings (and also to what you mentioned: "the function are using bit by bit encryption and low level copy function that does not copy a string to an other string but rather manipulating the bytes in the memory"). The problem lies in the fact that Strings do not store 0 values (i.e. the byte 0x00 or NULL or \0), but rather see it as a terminating character. Since there always exists the possibility that the cipher might contain this character, any attempt to convert it to a String will be prone to error.

The way I managed to go around this issue was by converting the value of each byte in the array to it's corresponding String hexadecimal representation (using String(byte, HEX) and appending a leading zero for values less than 0x10) and transmitting the data in this form. Not the most efficient way in terms of bandwidth, but it does the job for me. If you (or anyone reading this) know a better way, please do let me know.

spaniakos / AES

Wrapping AES within functions with String input/output #11