Skip to content
This repository has been archived by the owner on Mar 5, 2024. It is now read-only.

Resolve aes cmac token generation bug if input data is chunked #52

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

KonsKr
Copy link

@KonsKr KonsKr commented Jul 14, 2022

If the tc_cmac_update() function is called and fills the internal leftover cache completely, the following tc_cmac_final() call will produce a wrong token.

If the tc_cmac_update() is called with a data length which fills up the internal leftover cache completely, the leftover data will be processed instantly and is left empty. This is not the right behavior, because tc_cmac_final() requires that the last block is still in the leftover cache and not processed, because it need special treatment.

Bug reproduction code:

#include "tinycrypt/cmac_mode.h"
#include "tinycrypt/aes.h"
#include "tinycrypt/utils.h"
#include "tinycrypt/constants.h"
#include <stdlib.h>
#include <stdio.h>

#define ASSERT_TRUE(cond) if (!(cond)) {printf("Test failed!\n"); return -1;}

const uint8_t testData[16] =
{
    0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08, 0x09, 0x0A, 0x0B, 0x0C, 0x0D, 0x0E, 0x0F, 0x10
};

const uint8_t key[TC_AES_KEY_SIZE] =
{
    0xAA, 0xBB, 0xCC, 0xDD, 0xEE, 0xFF, 0x11, 0x22, 0x33, 0x44, 0x55, 0x66, 0x77, 0x88, 0x99, 0x12
};

uint8_t token1[TC_AES_BLOCK_SIZE];
uint8_t token2[TC_AES_BLOCK_SIZE];

int main()
{
    {
        struct tc_cmac_struct ctx;
        struct tc_aes_key_sched_struct sched;
        ASSERT_TRUE(tc_cmac_init(&ctx) == TC_CRYPTO_SUCCESS);
        ASSERT_TRUE(tc_cmac_setup(&ctx, key, &sched) == TC_CRYPTO_SUCCESS);
        ASSERT_TRUE(tc_cmac_update(&ctx, testData, sizeof(testData)) == TC_CRYPTO_SUCCESS);
        ASSERT_TRUE(tc_cmac_final(token1, &ctx) == TC_CRYPTO_SUCCESS);
    }

    {
        const size_t splitOffset = 8;
        struct tc_cmac_struct ctx;
        struct tc_aes_key_sched_struct sched;
        ASSERT_TRUE(tc_cmac_init(&ctx) == TC_CRYPTO_SUCCESS);
        ASSERT_TRUE(tc_cmac_setup(&ctx, key, &sched) == TC_CRYPTO_SUCCESS);
        ASSERT_TRUE(tc_cmac_update(&ctx, testData, splitOffset) == TC_CRYPTO_SUCCESS);
        ASSERT_TRUE(tc_cmac_update(&ctx, &testData[splitOffset] , TC_AES_BLOCK_SIZE - splitOffset) == TC_CRYPTO_SUCCESS);
        ASSERT_TRUE(tc_cmac_final(token2, &ctx) == TC_CRYPTO_SUCCESS);
    }

    ASSERT_TRUE(memcmp(token1, token2, TC_AES_BLOCK_SIZE) == 0); //will fail, tokens do not match

    printf("Test ok!\n");
}

If the tc_cmac_update() function is called and fills the internal leftover cache completely, the following tc_cmac_final() call will produce a wrong token.

If the tc_cmac_update() is called with a data length which fills up the internal leftover cache completely, the leftover data will be processed instantly and is left empty. This is not the right behavior, because tc_cmac_final() requires that the last block is still in the leftover cache and not processed, because it need special treatment.

Bug reproduction code:

~~~c
#include "tinycrypt/cmac_mode.h"
#include "tinycrypt/aes.h"
#include "tinycrypt/utils.h"
#include "tinycrypt/constants.h"
#include <stdlib.h>
#include <stdio.h>

#define ASSERT_TRUE(cond) if (!(cond)) {printf("Test failed!\n"); return -1;}

const uint8_t testData[16] =
{
    0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08, 0x09, 0x0A, 0x0B, 0x0C, 0x0D, 0x0E, 0x0F, 0x10
};

const uint8_t key[TC_AES_KEY_SIZE] =
{
    0xAA, 0xBB, 0xCC, 0xDD, 0xEE, 0xFF, 0x11, 0x22, 0x33, 0x44, 0x55, 0x66, 0x77, 0x88, 0x99, 0x12
};

uint8_t token1[TC_AES_BLOCK_SIZE];
uint8_t token2[TC_AES_BLOCK_SIZE];

int main()
{
    {
        struct tc_cmac_struct ctx;
        struct tc_aes_key_sched_struct sched;
        ASSERT_TRUE(tc_cmac_init(&ctx) == TC_CRYPTO_SUCCESS);
        ASSERT_TRUE(tc_cmac_setup(&ctx, key, &sched) == TC_CRYPTO_SUCCESS);
        ASSERT_TRUE(tc_cmac_update(&ctx, testData, sizeof(testData)) == TC_CRYPTO_SUCCESS);
        ASSERT_TRUE(tc_cmac_final(token1, &ctx) == TC_CRYPTO_SUCCESS);
    }

    {
        const size_t splitOffset = 8;
        struct tc_cmac_struct ctx;
        struct tc_aes_key_sched_struct sched;
        ASSERT_TRUE(tc_cmac_init(&ctx) == TC_CRYPTO_SUCCESS);
        ASSERT_TRUE(tc_cmac_setup(&ctx, key, &sched) == TC_CRYPTO_SUCCESS);
        ASSERT_TRUE(tc_cmac_update(&ctx, testData, splitOffset) == TC_CRYPTO_SUCCESS);
        ASSERT_TRUE(tc_cmac_update(&ctx, &testData[splitOffset] , TC_AES_BLOCK_SIZE - splitOffset) == TC_CRYPTO_SUCCESS);
        ASSERT_TRUE(tc_cmac_final(token2, &ctx) == TC_CRYPTO_SUCCESS);
    }

    ASSERT_TRUE(memcmp(token1, token2, TC_AES_BLOCK_SIZE) == 0); //will fail, tokens do not match

    printf("Test ok!\n");
}
~~~
@mdwood-intel
Copy link

I haven't had a chance to evaluate whether they are the same issue, but do you know if this is solve the same issue as #34?

@KonsKr
Copy link
Author

KonsKr commented Jul 15, 2022

I haven't had a chance to evaluate whether they are the same issue, but do you know if this is solve the same issue as #34?

Yes it solves the same issue. I could not get through their fix, but their added unit tests triggers the same problem and is also solved by this fix.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants