kokke/tiny-AES-c

AES CTR mode dont work if used to decrypt a stream

Closed this issue · 10 comments

Hi,
I have the following use case. I need to encrypt a file and transfer it to the embedded system where it needs to be decrypted and stored to flash. So the server encrypts a file and the lower network layer cuts the file in small chunks and sends the chunks over the network to the embedded device. Embedded system doesnt have enough ram to fully assemble the file, so it needs to decrypt every chunk received over the network on the fly and store it to the flash... So this means that AES_CTR_xcrypt_buffer function needs to be called every time new packet is received. The problem is that only the first packet received gets decrypted ok (first call to the decrypt function), all the other packets decrypt to some garbage. After looking at the code I think I found the problem, but it would be nice if someone here with at least some experience in crypto stuff can confirm this...

So the AES_CTR_xcrypt_buffer function allocates some local data at the beginning
uint8_t buffer[AES_BLOCKLEN] and int bi, but it looks like this data should be persistant between the calls, so they should be global variables or part of the struct AES_ctx structure. So I did the following change to get things working:

// global vars that were previous local
uint8_t buffer[AES_BLOCKLEN]; 
int bi = AES_BLOCKLEN; // TODO: There should be a way to reset this variable

void AES_CTR_xcrypt_buffer(struct AES_ctx* ctx, uint8_t* buf, uint32_t length)
{
  unsigned i;
  for (i = 0/*, bi = AES_BLOCKLEN*/; i < length; ++i, ++bi)
  {
    if (bi == AES_BLOCKLEN)
    {
...
kokke commented

Hi @IzidorM - which version of the code are you looking at? I think you're looking at an old version of the code. I can't find any reference of the TODO you quote, in https://github.com/kokke/tiny-AES-c/blob/master/aes.c

When using CTR-mode, the IV is the counter-variable, and this is incremented for each chunk you encrypt/decrypt. You should use AES_init_ctx_iv to set the IV/counter-variable accordingly.

Can you verify that you are using the same IV on both client- and server-side for each of the chunks?

You can check out https://en.wikipedia.org/wiki/Block_cipher_mode_of_operation#Counter_(CTR) to gain a better understanding of how the counter-mode works compared to CBC etc.

image

image

Hi,
I am using the latest code from the master branch. The code I posted, with the todo comment is my modified code to show what I modified to get it working.

So what I want to say is that you can encrypt the message with one call to the AES_CTR_xcrypt_buffer function but when you are decrypting message, you can call the same function multiple times on different sub parts of the message. Here is an example:

Lets say we have input_data[100] we want to encrypt. So we call AES_CTR_xcrypt_buffer(ctx, input_data, sizeof(input_data) and we how have the encrypted data in input_data[100]. Now we can decrypt data like this, first we decrypt first 10 bytes, so we call AES_CTR_xcrypt_buffer(ctx, input_data, 10), so now first 10 bytes in input_data are decrypted, then we want to decrypt next 20 bytes so we call AES_CTR_xcrypt_buffer(ctx, input_data + 10, 30) and so we get additional 20 bytes decrypted so now 10+20= 30 bytes in input_data are decrypted. We can than do this until we decrypt all the data in input_data.

So with your implementation this is not possible, because you initialize uint8_t buffer[AES_BLOCKLEN]; and int bi every time this function is called. So currently only way to decrypt the data is to call the AES_CTR_xcrypt_buffer on whole encrypted data like this:
AES_CTR_xcrypt_buffer(ctx, input_data, 100)

I hope I did a better job explaining what I mean this time :)

kokke commented

It is indeed possible, you just need to get the server and client to agree on how many times xcrypt is called so to speak, i.e. how many chunks to use when xcrypting.

You can't encrypt 100 bytes into 1 x 100 bytes and then send 10 x 10 bytes and expect that to work before collecting all 100 bytes.

If you want to send 10 bytes at a time, encrypt 10 bytes at a time and then decrypt 10 bytes at a time on the client side. Just get both server and client to agree on the protocol.

kokke commented

Closing, because it's a usage issue.

You need the client and server to agree on how many (and how big) chunks are used, if other sizes than 16 bytes are used. With 16 byte chunks, I think you can encrypt the buffer one time on one end, and decrypt it in 16 byte chunks on the other way.

Of course you can encrypt 100 bytes into 1 x 100 bytes and then send 10 x 10 bytes and expect that to work before collecting all 100 bytes. I am doing just that with the python aes library and it works like charm :) And it works also with your implementation with a little modification to the AES_CTR_xcrypt_buffer function...

Because CTR turns aes into a stream cipher. And stream cipher should work on every digit separately...

kokke commented

Yes you can do that, if you manually handle the Iv / Counter-variable using AES_init_ctx_iv accordingly, but this library does not do that automatically as your python code does - that was my intended meaning with that phrase :) I'm sorry for the confusion.

Your fix breaks thread safety and probably makes it impossible to have two concurrent sessions running, but is of course workable if you are only using one encrypted stream at a time.

However this issue is a usage-issue, not a bug in the code.

Thanks for the explanation. Now we understand each other. Well by moving the global variables to the struct AES_ct structure you get back the thread safety and multiple instances. I added the code just to make a point how simple it is to get this feature in to your code. But anyway, it is your code so you decide about it :D

kokke commented

There is no need to put the buffer in the ctx. I don't think you understand the purpose that buffer serves and how it works. Your first statement in the quote below seems to suggest it:

So the AES_CTR_xcrypt_buffer function allocates some local data at the beginning
uint8_t buffer[AES_BLOCKLEN] and int bi, but it looks like this data should be persistant between the calls, so they should be global variables or part of the struct AES_ctx structure. So I did the following change to get things working:

buffer gets filled with ctx->Iv on line 547: memcpy(buffer, ctx->Iv, AES_BLOCKLEN);

You see now, why I don't want to add buffer to the ctx-struct :)? It's sort-of already there. You can achieve the same thing, by manipulating the Iv through AES_init_ctx_iv.

By the way, I don't mean to come off sounding offending or offended :) My reason for rejecting your suggested additions, is that what your are raising in this issue, can be achieved by means of the existing code.

You could add a layer on top of this library that handles the counter-variable in-between sessions/calls, but it isn't strictly necessary and therefore is left out :) 16 bytes of RAM is much to ask for a nice-to-have feature for some users.

16 bytes of extra RAM for a rarely-used feature is indeed often too much for an embedded application. Usually means I have to cut out another feature to make space!

Rgds

Damon

If someone doesn't know how to increment, here is an example of the procedure for AES 128 CTR:

  1. I call AES_init_ctx_iv() and remember the iv.
  2. I use AES_CTR_xcrypt_buffer() to encrypt only the first 16 bytes (i.e. 128 bits for AES 128).
  3. I increment iv by one and call AES_ctx_set_iv().
  4. I encrypt the next 16 bytes using AES_CTR_xcrypt_buffer().
  5. I repeat steps 3 and 4 depending on the length of the input data. (The last block can be shorter than 16 bytes.)