Added XXTEA

[prochat]

Added xxtea as documented in wikipedia to have very simple block cipher with configurable block size.

This is very usefull for LoRa packet radio encryption (I'm also working on a fork of RadioHead Library that use BlockCipher)

[rweather]

An example with public test vectors would be useful.

The code looks reasonable, but I'm a bit hesitant about adding something that can be broken with 2^59 effort (according to wikipedia). I took SHA1 out of the library recently because it is broken with 2^60 effort.

I suppose if it's part of the LoRa standard then you are kind of stuck with it. I'm not necessarily saying no, and I'm happy for you to use other components of arduinolibs in your RadioHead fork, but I'll need some more convincing.

[prochat]

I get the point, you are right.

I just made this addition to have a very simple CipherBlock class to test with. Now it works with RadioHead (for the tests I’ve made), but maybe it would be wiser to choose another algo for a real use. So far, I Suggest I keep this fork for myself and my own development. I will continue to develop RH encrypted driver with XXTEA for the moment, and change it to another one available in Arduino libs for my final examples in RH. BTW, what would you recommend to use for LoRa messages with variable length between 0 and 251 ? I will follow your recommandation for my example as soon as I can finalize my RH encrypted driver.

And of course, if you think it’s worth to do it, I’ll implement test vectors.

Regards, Ph.R.

Le 6 juil. 2017 à 09:44, rweather notifications@github.com a écrit :

An example with public test vectors would be useful.

The code looks reasonable, but I'm a bit hesitant about adding something that can be broken with 2^59 effort (according to wikipedia). I took SHA1 out of the library recently because it is broken with 2^60 effort.

I suppose if it's part of the LoRa standard then you are kind of stuck with it. I'm not necessarily saying no, and I'm happy for you to use other components of arduinolibs in your RadioHead fork, but I'll need some more convincing.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/rweather/arduinolibs/pull/22#issuecomment-313321599, or mute the thread https://github.com/notifications/unsubscribe-auth/AADyPQkka4ym0FIxMqmKJruVZHUQ7_O-ks5sLJBCgaJpZM4OO67F.

[rweather]

I would recommend using ChaCha for encryption. You should use the packet sequence number or some other such identifier as the 64-bit "IV" to ensure that every packet is encrypted differently under the same key. It is very important not to reuse the same key/IV combination on different packets. If your packet sequence number is only 8-bit or 16-bit, then make sure to deal with rollover by incrementing the high bits of the 64-bit IV each time rollover happens.

I would also recommend looking into ChaChaPoly, which adds a 16-byte authentication tag to verify the contents of each packet. Sure, it increases the packet size but it prevents tampering. It is possible to truncate the tag to something like 12 or 8 bytes, but I wouldn't go any smaller than 8. If you already have a CRC value or something on the packet, then you can remove the CRC to save some bytes because the tag will be better at detecting corruption than the CRC anyway.

I'll demonstrate why tampering would be a problem with a radio protocol. ChaCha is a stream cipher, which means that an attacker can flip bits in the ciphertext and the corresponding bits in the plaintext will also flip. Say you had two commands in the underlying plaintext: 10 does something innocuous like get information about the other party, and 11 does something dangerous like "fire all torpedoes!" :-) .

If the attacker knows that the "get information" packet is always sent first in a session, they could flip the low bit of the command code and turn that 10 into an 11. The resulting data will decrypt successfully. This would be bad! Authenticated encryption is a must for ensuring that data has not been tampered with during transit.

If you prefer block ciphers to stream ciphers for some reason, then EAX-AES256 or EAX-Speck would be the authenticated ciphers of choice. ChaChaPoly is still my preference though.

Once you've gotten something working, I can take a look at the code if you'd like to check that there aren't any obvious tampering holes or other obvious weaknesses.

[prochat]

Hello RWeather,

Thank you for your time and detailed recommandation. The example for RadioHead must be kept as simple as possible. Interested people will dig into cryptography if the need to, and at the moment, since I used the abstract class BlockCipher, it needs to be a Block Ciphering algorithm. For simplicity and since it was strait forward, I changed XXTEA with Speck. It worked smoothly, and I propose to keep it that simple for the moment.

Mike McCauley asked me for a formal declaration to give my IP to the RadioHead project, which I did … thus it could probably appear in the next release. If you have some wishes about the reference for arduinolibs, please let me know, I’ll forward your desiderata. You can review my code on github, I’ll correct documentation following your recommandations (README.md will probably disappear).

Regards, Ph.R.

Le 7 juil. 2017 à 18:16, rweather notifications@github.com a écrit :

I would recommend using ChaCha for encryption. You should use the packet sequence number or some other such identifier as the 64-bit "IV" to ensure that every packet is encrypted differently under the same key. It is very important not to reuse the same key/IV combination on different packets. If your packet sequence number is only 8-bit or 16-bit, then make sure to deal with rollover by incrementing the high bits of the 64-bit IV each time rollover happens.

I would also recommend looking into ChaChaPoly, which adds a 16-byte authentication tag to verify the contents of each packet. Sure, it increases the packet size but it prevents tampering. It is possible to truncate the tag to something like 12 or 8 bytes, but I wouldn't go any smaller than 8. If you already have a CRC value or something on the packet, then you can remove the CRC to save some bytes because the tag will be better at detecting corruption than the CRC anyway.

I'll demonstrate why tampering would be a problem with a radio protocol. ChaCha is a stream cipher, which means that an attacker can flip bits in the ciphertext and the corresponding bits in the plaintext will also flip. Say you had two commands in the underlying plaintext: 10 does something innocuous like get information about the other party, and 11 does something dangerous like "fire all torpedoes!" :-) .

If the attacker knows that the "get information" packet is always sent first in a session, they could flip the low bit of the command code and turn that 10 into an 11. The resulting data will decrypt successfully. This would be bad! Authenticated encryption is a must for ensuring that data has not been tampered with during transit.

If you prefer block ciphers to stream ciphers for some reason, then EAX or EAX would be the authenticated ciphers of choice. ChaChaPoly is still my preference though.

Once you've gotten something working, I can take a look at the code if you'd like to check that there aren't any obvious tampering holes or other obvious weaknesses.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/rweather/arduinolibs/pull/22#issuecomment-313726925, or mute the thread https://github.com/notifications/unsubscribe-auth/AADyPZ19Vzq65h6bpl1LjZpb57Okx9neks5sLlnxgaJpZM4OO67F.

[rweather]

I had a quick look at RHEncryptedDriver. You are using the block cipher in ECB mode, which is not recommended for security. ECB mode is fairly easy to break (check out the discussion in the Wikipedia page for ECB). You should be using the block cipher with a different mode such as CBC, CTR, or CFB mode.

For your use case, I would suggest CBC or CFB mode. Prefix each packet with a 16-byte random IV value (you can use my RNG class for this) so that the same plaintext in different packets encrypts to different ciphertext. The main difference between CBC and CFB is that CFB doesn't require the plaintext to be padded to a 16-byte boundary.

Here is an example of what send() might look like (just a code sketch - some of the fine details and error handling have been omitted):

char *output = malloc(16 + len); RNG.rand(output, 16)); _cfb.setIV(output, 16); _cfb.encrypt(output + 16, buf, len); _driver.send(output, 16 + len); clean(output, 16 + len); free(output);

You could make it a little more efficient by using calloc/realloc on a common buffer like in your current code.

The recv() side:

_driver.recv(buf, len); if ((len) < 16) return false; // Not enough data for the IV. _cfb.setIV(buf, 16); (len) -= 16; _cfb.decrypt(buf, buf + 16, (*len));

I hope this helps. It is important to get this kind of thing right to avoid embarrassment later.

[prochat]

Hello Rhys Weatherly,

The main issue I’m facing at the moment is memory available on an Arduino UNO (2048 Bytes).

ECB mode is straight forward and would use less memory than would CBC mode for example (I would need a static array of the message length at least for xoring form one call to another).

Regarding a previous email you sent me: for streaming Ciphers, there is a CRC in the LoRa protocol, but I think it remains at the modem level and is not accessible. And using a Streaming class would cause a lot of problems to integrate in the current RHDriver of the RadioHead library, but I might look at that later).

Now on my project, as I’m piling layers (over the RHEncryptedDriver), I’m trying to add a serializer (msgpack) and my memory is getting drained, I had to fall back on XXTEA. Speck is siphoning 15% of memory, when XXTEA is using only 4%. That might be an argument for XXTEA: tiny but insecure ?

I will try later to find a way to optimise and get more memory … but at the moment I cannot afford it.

Regards, Ph.R.

Le 8 juil. 2017 à 06:52, rweather notifications@github.com a écrit :

I had a quick look at RHEncryptedDriver. You are using the block cipher in ECB mode, which is not recommended for security. ECB mode is fairly easy to break (check out the discussion in the Wikipedia page for ECB). You should be using the block cipher with a different mode such as CBC, CTR, or CFB mode.

For your use case, I would suggest CBC or CFB mode. Prefix each packet with a 16-byte random IV value (you can use my RNG class for this) so that the same plaintext in different packets encrypts to different ciphertext. The main difference between CBC and CFB is that CFB doesn't require the plaintext to be padded to a 16-byte boundary.

Here is an example of what send() might look like (just a code sketch - some of the fine details and error handling have been omitted):

char *output = malloc(16 + len); RNG.rand(output, 16)); _cfb.setIV(output, 16); _cfb.encrypt(output + 16, buf, len); _driver.send(output, 16 + len); clean(output, 16 + len); free(output);

You could make it a little more efficient by using calloc/realloc on a common buffer like in your current code.

The recv() side:

_driver.recv(buf, len); if ((len) < 16) return false; // Not enough data for the IV. _cfb.setIV(buf, 16); (len) -= 16; _cfb.decrypt(buf, buf + 16, (*len));

I hope this helps. It is important to get this kind of thing right to avoid embarrassment later.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/rweather/arduinolibs/pull/22#issuecomment-313833718, or mute the thread https://github.com/notifications/unsubscribe-auth/AADyPVa7iozWOmIOKiHc7NjhrbQOF3nDks5sLwsHgaJpZM4OO67F.

[rweather]

If you use SpeckSmall instead of Speck, you can save quite a bit of memory - 67 bytes for the key schedule data instead of 275. SpeckSmall is a little slower because it expands the key schedule on the fly but the 208 bytes of memory savings are worth it. CFB mode would add about 20 or so extra bytes.

[prochat]

Hello Mike,

Maybe one last note about the encryption level I’m using and the security. I had a chat on that aspect with Rhys Weatherly, here in CC (a very nice exchange and he took a lot of time to send me very detailed and smart answers), and he very accurately pointed out that my implementation is somehow lazy and definitely not very strong. But I would argue that I wanted to keep the code a little bit minimalist regarding memory use. Since it’s a library I want to leave as much as possible space available for the core program. A point is on the fact that we should use ECB mode ((check out the discussion in the Wikipedia page for ECB)) … but changing this to CBC would require some more buffers with static variables to store the xored accumulated result (and for how long, and we have to keep this registered with a - per connexion - array, etc. For the moment I choosed to leave this on the side, since I would like to develop now the rest of my application, and I have no real pressure for security.

But if you consider that somehow, any sooner of later, the security is an issue, then this would have to be seriously reconsidered and it might be interesting to have Rhis with his strong expertise in the loop. (N.B: I do not have Rhis real email address, that’s why I’m using this github address).

Regards, Ph.R.

Le 13 juil. 2017 à 00:28, Philippe Rochat philippe.rochat@gymorges.ch a écrit :

Hello Mike,

I added: #ifdef RH_ENABLE_ENCRYPTION_MODULE and the definition is at the bottom of RadioHead.h It’s not now uncommented, but you should probably distribute a commented version, that will by default disable the encryption module. I also completed as much as possible the RHEncryptionDriver to describe the how to enable that.

The content length is now by default strict. (it could be changed by someone who would like to use all possible length … ?). I’ve tested it and it works now fine for me. It’s clean and does support now msgpack node.js module without any patch (which is much better).

I attached below my 2 programs (LoraSimpleClient does sent some fake values through msgpack, and LoraGW receive it and write it back to Serial for node red).

Regarding OSI networks layers, I have now an implementation from Layer 0 to Layer 6. (Implementing a REST program would then go to layer 7 … but probably my arduino memory won’t support a full implementation of ZeroRPC)

I wont disclose my EEPRON.h since it describes how my encryption key is stored … but it could be easily adapted. Keep it for your eyes please (it’s running for rl in my home ;-) ) I also presume it’s not that much interesting "as is" since it includes also other libraries like BufferedStreams and Msgpack. (all are available on my github account). But maybe we could think of another example ?

I hope this is now ok for you.

Regards, Ph.R.

/* Loca Simple Hello World Client with encrypted communications

• Philippe.Rochat'at'gmail.com
• 06.07.2017 */

  include

  include

  include

  include

  include "msgpck.h"

  include "EEPROM_Lora.h"

  include "BufferedOutputStream.h"

  include "BufferedInputStream.h"

RH_RF95 rf95; // Instanciate a LoRa driver //Speck myCipher; // Instanciate a Speck block ciphering XXTEA myCipher; RHEncryptedDriver myDriver(rf95, myCipher); // Instantiate the driver with those two LoraConfig config; unsigned char encryptkey[16]={0}; // The very secret key uint8_t msg_size = 0; uint8_t MSGBuffer; BufferedOutputStream outBuf; BufferedInputStream *inBuf;

void setup() { // Init of serial stuff Serial.begin(9600); while (!Serial) ; // Wait for serial port to be available Serial.println("LoRa Simple_Encrypted Client"); // LoRa init if (!rf95.init()) Serial.println("LoRa init failed"); rf95.setFrequency(config.getFreq()); // Setup ISM frequency Serial.print("Frequency set to: ");Serial.println(config.getFreq()); rf95.setTxPower(config.getPower()); // Setup Power,dBm Serial.print("Power set to: ");Serial.println(config.getPower()); // Encryption init config.getKey(encryptkey); myCipher.setKey(encryptkey, 16); Serial.println("Waiting for radio to setup"); delay(1000); // Message buffer init msg_size=myDriver.maxMessageLength(); MSGBuffer = calloc(msg_size, sizeof(uint8_t)); outBuf = new BufferedOutputStream(MSGBuffer, msg_size); inBuf = new BufferedInputStream(MSGBuffer, msg_size); Serial.println("Setup completed"); }

uint8_t makeMsg() { int len; outBuf->flush(); msgpck_write_map_header(outBuf, 3); //Map containing two pair of element msgpck_write_string(outBuf, "cnt", 3); //key: String "cnt" msgpck_write_string(outBuf, "Hello", 5); //Value: String "val" msgpck_write_string(outBuf, "tmp", 3); //key: String "cnt" msgpck_write_float(outBuf, 12.56); msgpck_write_string(outBuf, "pum", 3); //key: String "cnt" msgpck_write_bool(outBuf, true);

len = outBuf->current_length(); return len; }

void loop() { int l=makeMsg(); myDriver.send(MSGBuffer, l); Serial.print("Sent: "); Serial.println((char *)MSGBuffer); Serial.println(l); delay(4000); }

/////////// /* Loca simple server with encrypted communications

• Philippe.Rochat'at'gmail.com
• 06.07.2017 */

include

include

include

include

include "EEPROM_Lora.h"

RH_RF95 rf95; // Instanciate a LoRa driver //Speck myCipher; // Instanciate a Speck block ciphering XXTEA myCipher; RHEncryptedDriver myDriver(rf95, myCipher); // Instantiate the driver with those two

LoraConfig config;

unsigned char encryptkey[16]={0}; // The very secret key

void setup() { Serial.begin(9600); while (!Serial) ; // Wait for serial port to be available Serial.println(">> LoRa Simple_Encrypted Server"); if (!rf95.init()) Serial.println(">> LoRa init failed"); // Setup ISM frequency rf95.setFrequency(config.getFreq()); // Setup Power,dBm rf95.setTxPower(config.getPower()); config.getKey(encryptkey); myCipher.setKey(encryptkey, 16); delay(4000); Serial.println(">> Setup completed"); }

void printMsgArray(uint8_t *buf, uint8_t size) { Serial.print("["); for (int i=0; i< size; i++) { Serial.print(buf[i], DEC); if(i<size-1) Serial.print(":"); else Serial.print("]"); } }

void loop() { if (myDriver.available()) { // Should be a message for us now
uint8_t buf[myDriver.maxMessageLength()] = {0}; uint8_t size = sizeof(buf); if (myDriver.recv(buf, &size)) { Serial.print(">> Msg Received (");Serial.print(size); Serial.println(")"); Serial.print(">> Msg content: "); printMsgArray(buf, size); Serial.println(""); Serial.write(buf, size); Serial.println(""); } else { Serial.println(">> recv failed"); } } }

[rweather]

My e-mail address is in the README.md file for arduinolibs, down the bottom. Feel free to CC me into any discussions on LoRa security and I'll try to provide advice on good practices.

https://github.com/rweather/arduinolibs/blob/master/README.md

[rweather]

Closing this pull request - I'm not planning to add XXTEA to the library at this time.