Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bug in AES CMAC calculation when using multiple tc_cmac_update() calls #1

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");
}

…tover 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");
}
~~~
@KonsKr KonsKr changed the title If the tc_cmac_update() function is called and fills the internal lef… Bug in AES CMAC calculation when using multiple tc_cmac_update() calls Jul 14, 2022
@KonsKr
Copy link
Author

KonsKr commented Jul 14, 2022

See bug report in intel tinycrypt repository: intel/tinycrypt#51

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

Successfully merging this pull request may close these issues.

1 participant