3
\$\begingroup\$

I need to encrypt/decrypt 2D arrays (double pointers) using AES-128/CTR using Intel TinyCrypt (written in C). The following are two helper methods to simplify the library usage. Any comments would be highly appreciated. Thanks in advance.

#include <tinycrypt/constants.h>
#include <tinycrypt/ctr_mode.h>
#include <tinycrypt/aes.h>

#define AES_128_KEY_LENGTH 16
#define AES_128_CTR_LENGTH 16

typedef struct aes_128_ctr_params_t{
  byte key[AES_128_KEY_LENGTH];
  byte ctr[AES_128_CTR_LENGTH];
} aes_128_ctr_params_t;

//---------------------------------------------------------------------------
inline int32_t encrypt(uint8_t const * const * const plaintext,
                       uint8_t * const * const cihpertext,
                       size_t const height,
                       size_t const width,
                       aes_128_ctr_params_t params) {
  //TODO: Do some validation here!

  struct tc_aes_key_sched_struct sched;
  uint32_t result = TC_CRYPTO_SUCCESS;
  result = tc_aes128_set_encrypt_key(&sched, params.key);
  if (result != TC_CRYPTO_SUCCESS)
    return result;

  size_t const row_size_in_bytes = sizeof(uint8_t) * width;
  for (size_t row_index = 0; row_index < height; ++row_index) {
    result = tc_ctr_mode(cihpertext[row_index], row_size_in_bytes,
      plaintext[row_index], row_size_in_bytes, params.ctr, &sched);

    if (result != TC_CRYPTO_SUCCESS)
      return result;
  }
}

//---------------------------------------------------------------------------
inline int32_t decrypt(uint8_t const * const * const cihpertext,
                       uint8_t * const * const plaintext,
                       size_t const height,
                       size_t const width,
                       aes_128_ctr_params_t params) {
  //TODO: Do some validation here!

  struct tc_aes_key_sched_struct sched;
  uint32_t result = TC_CRYPTO_SUCCESS;
  result = tc_aes128_set_encrypt_key(&sched, params.key);
  if (result != TC_CRYPTO_SUCCESS)
    return result;

  size_t const row_size_in_bytes = sizeof(uint8_t) * width;
  for (size_t row_index = 0; row_index < height; ++row_index) {
    result = tc_ctr_mode(plaintext[row_index], row_size_in_bytes,
      cihpertext[row_index], row_size_in_bytes, params.ctr, &sched);

    if (result != TC_CRYPTO_SUCCESS)
      return result;
  }
}
\$\endgroup\$
1
\$\begingroup\$

I find your code a bit dense and hard to read overall.

Your identifier names are compound words but all lower case.

plaintext

I prefer plainText others would prefer plain_text (and a lot of the code uses this second C like style). But either is preferable to your current style.

This seems redundant.

uint32_t result = TC_CRYPTO_SUCCESS;
result = tc_aes128_set_encrypt_key(&sched, params.key);

Just use one line:

uint32_t result = tc_aes128_set_encrypt_key(&sched, params.key);

Technically both functions exhibit undefined behavior (in C++ not sure about C). There is no return on successful completion.

    if (result != TC_CRYPTO_SUCCESS)
      return result;
  }
  // Add the following line
  return result;
}
| improve this answer | |
\$\endgroup\$
1
\$\begingroup\$

I'm not particularly excited about the way you've expressed the logic of your functions. At least IMO, it's written kind of like a collection of special cases instead of applying general rules. I think I'd prefer something closer to this:

inline int32_t decrypt(uint8_t const * const * const cihpertext,
    uint8_t * const * const plaintext,
    size_t const height,
    size_t const width,
    aes_128_ctr_params_t params) 
{
    struct tc_aes_key_sched_struct sched;
    auto const good = TC_CRYPTO_SUCCESS;

    uint32_t result = tc_aes128_set_encrypt_key(&sched, params.key);

    size_t const row_size_in_bytes = sizeof(uint8_t) * width;

    for (size_t row_index = 0; row_index < height && result == good; ++row_index) {
        result = tc_ctr_mode(plaintext[row_index], row_size_in_bytes,
            cihpertext[row_index], row_size_in_bytes, params.ctr, &sched);
    }
    return result;
}
| improve this answer | |
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.