kokke/tiny-AES-c

Interface change rationale

Closed this issue · 5 comments

ascheurer commented

A prior release had the key and iv passed to the function and the interface supported them as const so they were used as reads only - inputs.

The new interfaces using AES_ctx as a ptr and its not const so it could change and it does change. But I must now use AES_ctx_set_iv(..) before encrypt and decrypt - both of them. I can't init it once and use for any number of encrypts and decrypts. This is a bit counter-intuitive.

Is the reason to avoid a copy? Seems like init may not be the function name as init implies once and doing multiple inits for every encryption and another init for every decryption is not intuitively obvious based on the function naming of init

ascheurer commented

Old interface:
/* AES256 encrypt buffer */
AES_CBC_encrypt_buffer(encrypted_buf, in,
sizeof(encrypted_buf), key, iv);

/* compare apriori out buf vs encrypted - computed ahead of time */
rc = memcmp(out, encrypted_buf, sizeof(encrypted_buf));
printf("CBC encrypt: %s\n", rc ? "failure" : "success");

AES_CBC_decrypt_buffer(decrypted_buf, encrypted_buf,
                       sizeof(decrypted_buf), key, iv);

/* validate the in buffer above when put into an encrypt_buf results
   in the same decrypted buf
 */
rc = memcmp(decrypted_buf, in, sizeof(in));
printf("CBC decrypt: %s\n", rc ? "failure" : "success");

New Interface:

/* save a copy */
memcpy(working_buf, in, sizeof(in));

AES_init_ctx_iv(&ctx, key, iv);
ctx2 = ctx;     /* hack to get around multiple inits */
rc = memcmp(&ctx, &ctx2, sizeof(ctx2));
assert(rc == 0);

AES_CBC_encrypt_buffer(&ctx, working_buf, sizeof(working_buf));
rc = memcmp(&ctx, &ctx2, sizeof(ctx2)); /* not same anymore */
assert(rc != 0);    /* ctx is not only read but also modified */

/* compare apriori out buf vs encrypted - computed ahead of time */
rc = memcmp(out, working_buf, sizeof(working_buf));
printf("CBC encrypt: %s\n", rc ? "failure" : "success");

/* Note: This is counter-inutuitive Must call AES_init before
 * encrypt and decrypt. Evidently structure object changes
 */
/*AES_init_ctx_iv(&ctx, key, iv); works here also - must re-init */
/* workaround is to create a copy */
AES_CBC_decrypt_buffer(&ctx2, working_buf, sizeof(working_buf));

/* Why using ctx2 above?
 * ctx2 is updated by decrypt so calling key and iv init before both
 * encrypt and decrypt is almost unavoidable and thread safety an issue
 * if using pairs of ctx objects with copies after init, just avoids
 * function call overhead
 */

/* validate the in buffer against decrypted workingbuf */
rc = memcmp(working_buf, in, sizeof(in));
printf("CBC decrypt: %s\n", rc ? "failure" : "success");
tehKaiN commented

@ascheurer I'll try to explain using my use case. Right now I'm working on small embedded project, where RAM is scarce. Part of my program receives data from serial port, decrypts it and stores in flash for later use. Problem is that data sent is too big to be stored all at once in RAM. So the current interface allows you to do something like this:

AES_ctx_set_iv(&ctx, &iv);
for(i = 0 ; i < 10; ++i) {
  recvEncryptedDataPart(buf);
  AES_CBC_decrypt(&ctx, buf, len);
  writeToFlash(buf, i);
}

By having IV placed in arg, I'd have to copy myself last 16-byte block of encrypted part before it gets decrypted for being used as IV on next package. Things get a lot more messy that way. ;)

kokke commented

Hi @ascheurer

The API was changed to make the code thread safe.
Discussions about the new API can be found in #75.

EDIT:

The arguments by @tehKaiN are very good and I agree with them.
The current API is meant to facilitate a streaming operation, where you don't need to keep everything in RAM at once.

kokke commented

Hey Andrew, I'm closing this issue if it's okay with you. We can continue the discussion if you didn't get what you came for ;)?

ascheurer commented