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

Support the negative option MBEDTLS_BLOCK_CIPHER_NO_DECRYPT #8124

Merged
merged 65 commits into from
Nov 23, 2023
Merged
Show file tree
Hide file tree
Changes from 54 commits
Commits
Show all changes
65 commits
Select commit Hold shift + click to select a range
422a77f
aes.c: clean up and fix wrong comment in #endif
Jul 7, 2023
4274247
aes.c: provide finer guard for RSb
Jul 7, 2023
78ee0c9
aes.c: add config option to support cipher_encrypt_only
May 15, 2023
590c9b7
AESCE: add macro guard of CIPHER_ENCRYPT_ONLY
Aug 28, 2023
380be5a
AESNI: add macro guard of CIPHER_ENCRYPT_ONLY
Aug 28, 2023
67208fd
PSA: auto-enable CIPHER_ENCRYPT_ONLY if cipher-decrypt is not needed
May 15, 2023
a8ac23a
all.sh: add test case for CIPHER_ENCRYPT_ONLY
May 15, 2023
9141ad1
aria/camellia/des: guard setkey_dec by CIPHER_ENCRYPT_ONLY
Aug 24, 2023
db9b309
cipher_wrap: remove *setkey_dec_func in CIPHER_ENCRYPT_ONLY
Aug 24, 2023
d7058b0
dh_client: removed under CIPHER_ENCRYPT_ONLY
Aug 24, 2023
85c3023
AES-ECB: add CIPHER_ENCRYPT_ONLY dependency for DECRYPT test cases
May 16, 2023
702c220
aria: add CIPHER_ENCRYPT_ONLY dependency for DECRYPT test cases
Aug 28, 2023
ba473b1
camellia: add CIPHER_ENCRYPT_ONLY dependency for DECRYPT test cases
Aug 28, 2023
3c56527
des: add CIPHER_ENCRYPT_ONLY dependency for test cases
Aug 28, 2023
72d7bb4
check_config.h: add checks for CIPHER_ENCRYPT_ONLY
Aug 30, 2023
dbcc0c6
aes: define internal macro to simplify #if Directive
Aug 30, 2023
207c991
all.sh: ciper_encrypt_only: cover AESNI and C Implementation
Aug 31, 2023
bf66ef9
all.sh: ciper_encrypt_only: cover baremetal build for AESCE
Aug 31, 2023
7821904
all.sh: ciper_encrypt_only: cover VIA PADLOCK
Aug 31, 2023
a675776
Add ChangeLog entry for MBEDTLS_CIPHER_ENCRYPT_ONLY
Sep 1, 2023
9b81165
Merge remote-tracking branch 'origin/development' into support_cipher…
Sep 7, 2023
4f4822c
Revert "des: add CIPHER_ENCRYPT_ONLY dependency for test cases"
Sep 7, 2023
56e27b9
des: don't consider DES for CIPHER_ENCRYPT_ONLY
Sep 7, 2023
c5944d4
all.sh: fix a typo
Sep 7, 2023
3caaf0c
Enable CIPHER_ENCRYPT_ONLY when DES is disabled
Sep 7, 2023
ef1b04d
all.sh: make sure CIPHER_ENCRYPT_ONLY is enabled in tests
Sep 8, 2023
bc7716c
all.sh: run make clean before make lib in armc6_build_test
Sep 8, 2023
aa01ee3
Merge remote-tracking branch 'origin/development' into support_cipher…
Oct 16, 2023
4b6595a
Merge remote-tracking branch 'origin/development' into support_cipher…
Oct 17, 2023
b67b474
Rename MBEDTLS_CIPHER_ENCRYPT_ONLY as MBEDTLS_BLOCK_CIPHER_NO_DECRYPT
Oct 31, 2023
e367e47
mbedtls_config: add new config option MBEDTLS_BLOCK_CIPHER_NO_DECRYPT
Oct 31, 2023
b799eea
check_config: add checks for MBEDTLS_BLOCK_CIPHER_NO_DECRYPT
Oct 31, 2023
6611139
all.sh: modify components to test BLOCK_CIPHER_NO_DECRYPT
Oct 31, 2023
eefd269
test_suite_psa_crypto: add dependency for decrypt test cases
Oct 31, 2023
6b190d4
psa_information.py: generate dep for AES/ARIA/CAMELLIA ECB test case
Nov 1, 2023
be08908
config.py: exclude MBEDTLS_BLOCK_CIPHER_NO_DECRYPT from full
Nov 1, 2023
f24bbd9
dh_client.c: modify prompt message
Nov 1, 2023
de0e259
cipher_wrap.c: remove unnecessary NO_DECRYPT guard for DES
Nov 1, 2023
f149640
all.sh: add components to test BLOCK_CIPHER_NO_DECRYPT with PSA
Nov 1, 2023
956aa00
check_config: add checks for MBEDTLS_BLOCK_CIPHER_NO_DECRYPT with PSA
Nov 1, 2023
0d76b6e
Return an error if asking for decrypt under BLOCK_CIPHER_NO_DECRYPT
Nov 2, 2023
5347957
ChangeLog: rewrite ChangeLog for block-cipher-no-decrypt
Nov 2, 2023
bc29aef
all.sh: test BLOCK_CIPHER_NO_DECRYPT in build_aes_variations
Nov 6, 2023
4995e0c
cipher.c: return error for ECB-decrypt under BLOCK_CIPHER_NO_DECRYPT
Nov 7, 2023
004a60c
aes.c: remove non-functional code
Nov 8, 2023
d137da5
check_config: make error message in BLOCK_CIPHER_NO_DECRYPT clearer
Nov 8, 2023
f03b491
aes.c: guard RSb and RTx properly
Nov 9, 2023
70743b0
psa_information: compile a regex instead of using string directly
Nov 9, 2023
9938554
BLOCK_CIPHER_NO_DECRYPT: rephrase ChangeLog
Nov 9, 2023
49cd4b5
all.sh: refine and simplify component for block_cipher_no_decrypt
Nov 10, 2023
4cd1b16
all.sh: check additional symbols in asece for block_cipher_no_decrypt
Nov 10, 2023
799bd84
all.sh: resue support_build_armcc for *_armcc test
Nov 10, 2023
111159b
BLOCK_CIPHER_NO_DECRYPT: call encrypt direction unconditionally
Nov 10, 2023
cd25d22
cipher.c: remove checks for CBC,XTS,KW,KWP in cipher_setkey
Nov 10, 2023
0287b9d
padlock.c: guard mbedtls_padlock_xcryptcbc by CIPHER_MODE_CBC
Nov 10, 2023
85b7465
all.sh: block_cipher_no_decrypt: fix various issues
Nov 13, 2023
b2d6e52
all.sh: block_cipher_no_decrypt: simplify code
Nov 13, 2023
07e663d
all.sh: block_cipher_no_decrypt: clean up cflags
Nov 13, 2023
3ae1199
all.sh: add config_block_cipher_no_decrypt to simplify code
Nov 13, 2023
19583e4
psa_information: improve code readability
Nov 13, 2023
c434791
aesce: fix unused parameter
Nov 14, 2023
690ee81
Merge remote-tracking branch 'origin/development' into support_cipher…
Nov 23, 2023
70642ec
all.sh: check_test_dependencies: add one more option
Nov 23, 2023
42be1ba
block_cipher_no_decrypt: improve comment
Nov 23, 2023
18040ed
all.sh: export LC_COLLATE=C for sorting in ASCII order
Nov 23, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions ChangeLog.d/add-block-cipher-no-decrypt.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
Features
* Enable the new option MBEDTLS_BLOCK_CIPHER_NO_DECRYPT to omit
the decryption direction of block ciphers (AES, ARIA, Camellia).
This affects both the low-level modules and the high-level APIs
(the cipher and PSA interfaces). This option is incompatible with modes
that use the decryption direction (ECB in PSA, CBC, XTS, KW) and with DES.
4 changes: 4 additions & 0 deletions include/mbedtls/aes.h
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,7 @@ MBEDTLS_CHECK_RETURN_TYPICAL
int mbedtls_aes_setkey_enc(mbedtls_aes_context *ctx, const unsigned char *key,
unsigned int keybits);

#if !defined(MBEDTLS_BLOCK_CIPHER_NO_DECRYPT)
/**
* \brief This function sets the decryption key.
*
Expand All @@ -185,6 +186,7 @@ int mbedtls_aes_setkey_enc(mbedtls_aes_context *ctx, const unsigned char *key,
MBEDTLS_CHECK_RETURN_TYPICAL
int mbedtls_aes_setkey_dec(mbedtls_aes_context *ctx, const unsigned char *key,
unsigned int keybits);
#endif /* !MBEDTLS_BLOCK_CIPHER_NO_DECRYPT */

#if defined(MBEDTLS_CIPHER_MODE_XTS)
/**
Expand Down Expand Up @@ -604,6 +606,7 @@ int mbedtls_internal_aes_encrypt(mbedtls_aes_context *ctx,
const unsigned char input[16],
unsigned char output[16]);

#if !defined(MBEDTLS_BLOCK_CIPHER_NO_DECRYPT)
/**
* \brief Internal AES block decryption function. This is only
* exposed to allow overriding it using see
Expand All @@ -619,6 +622,7 @@ MBEDTLS_CHECK_RETURN_TYPICAL
int mbedtls_internal_aes_decrypt(mbedtls_aes_context *ctx,
const unsigned char input[16],
unsigned char output[16]);
#endif /* !MBEDTLS_BLOCK_CIPHER_NO_DECRYPT */

#if defined(MBEDTLS_SELF_TEST)
/**
Expand Down
2 changes: 2 additions & 0 deletions include/mbedtls/aria.h
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,7 @@ int mbedtls_aria_setkey_enc(mbedtls_aria_context *ctx,
const unsigned char *key,
unsigned int keybits);

#if !defined(MBEDTLS_BLOCK_CIPHER_NO_DECRYPT)
/**
* \brief This function sets the decryption key.
*
Expand All @@ -128,6 +129,7 @@ int mbedtls_aria_setkey_enc(mbedtls_aria_context *ctx,
int mbedtls_aria_setkey_dec(mbedtls_aria_context *ctx,
const unsigned char *key,
unsigned int keybits);
#endif /* !MBEDTLS_BLOCK_CIPHER_NO_DECRYPT */

/**
* \brief This function performs an ARIA single-block encryption or
Expand Down
2 changes: 2 additions & 0 deletions include/mbedtls/camellia.h
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,7 @@ int mbedtls_camellia_setkey_enc(mbedtls_camellia_context *ctx,
const unsigned char *key,
unsigned int keybits);

#if !defined(MBEDTLS_BLOCK_CIPHER_NO_DECRYPT)
/**
* \brief Perform a CAMELLIA key schedule operation for decryption.
*
Expand All @@ -108,6 +109,7 @@ int mbedtls_camellia_setkey_enc(mbedtls_camellia_context *ctx,
int mbedtls_camellia_setkey_dec(mbedtls_camellia_context *ctx,
const unsigned char *key,
unsigned int keybits);
#endif /* !MBEDTLS_BLOCK_CIPHER_NO_DECRYPT */

/**
* \brief Perform a CAMELLIA-ECB block encryption/decryption operation.
Expand Down
30 changes: 30 additions & 0 deletions include/mbedtls/check_config.h
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,36 @@
#error "MBEDTLS_NIST_KW_C defined, but not all prerequisites"
#endif

#if defined(MBEDTLS_BLOCK_CIPHER_NO_DECRYPT) && defined(MBEDTLS_PSA_CRYPTO_CONFIG)
#if defined(PSA_WANT_ALG_CBC_NO_PADDING)
#error "MBEDTLS_BLOCK_CIPHER_NO_DECRYPT and PSA_WANT_ALG_CBC_NO_PADDING cannot be defined simultaneously"
#endif
#if defined(PSA_WANT_ALG_CBC_PKCS7)
#error "MBEDTLS_BLOCK_CIPHER_NO_DECRYPT and PSA_WANT_ALG_CBC_PKCS7 cannot be defined simultaneously"
#endif
#if defined(PSA_WANT_ALG_ECB_NO_PADDING)
#error "MBEDTLS_BLOCK_CIPHER_NO_DECRYPT and PSA_WANT_ALG_ECB_NO_PADDING cannot be defined simultaneously"
#endif
#if defined(PSA_WANT_KEY_TYPE_DES)
#error "MBEDTLS_BLOCK_CIPHER_NO_DECRYPT and PSA_WANT_KEY_TYPE_DES cannot be defined simultaneously"
#endif
#endif

#if defined(MBEDTLS_BLOCK_CIPHER_NO_DECRYPT)
#if defined(MBEDTLS_CIPHER_MODE_CBC)
#error "MBEDTLS_BLOCK_CIPHER_NO_DECRYPT and MBEDTLS_CIPHER_MODE_CBC cannot be defined simultaneously"
#endif
#if defined(MBEDTLS_CIPHER_MODE_XTS)
#error "MBEDTLS_BLOCK_CIPHER_NO_DECRYPT and MBEDTLS_CIPHER_MODE_XTS cannot be defined simultaneously"
#endif
#if defined(MBEDTLS_DES_C)
#error "MBEDTLS_BLOCK_CIPHER_NO_DECRYPT and MBEDTLS_DES_C cannot be defined simultaneously"
#endif
#if defined(MBEDTLS_NIST_KW_C)
#error "MBEDTLS_BLOCK_CIPHER_NO_DECRYPT and MBEDTLS_NIST_KW_C cannot be defined simultaneously"
#endif
#endif

#if defined(MBEDTLS_ECDH_C) && !defined(MBEDTLS_ECP_C)
#error "MBEDTLS_ECDH_C defined, but not all prerequisites"
#endif
Expand Down
19 changes: 19 additions & 0 deletions include/mbedtls/mbedtls_config.h
Original file line number Diff line number Diff line change
Expand Up @@ -2383,6 +2383,25 @@
*/
#define MBEDTLS_BASE64_C

/**
* \def MBEDTLS_BLOCK_CIPHER_NO_DECRYPT
*
* Remove decryption operation for AES, ARIA and Camellia block cipher.
*
* \note This feature is incompatible with insecure block cipher,
* MBEDTLS_DES_C, and cipher modes which always require decryption
* operation, MBEDTLS_CIPHER_MODE_CBC, MBEDTLS_CIPHER_MODE_XTS and
* MBEDTLS_NIST_KW_C.
yanrayw marked this conversation as resolved.
Show resolved Hide resolved
*
* Module: library/aes.c
* library/aesce.c
* library/aesni.c
* library/aria.c
* library/camellia.c
* library/cipher.c
*/
//#define MBEDTLS_BLOCK_CIPHER_NO_DECRYPT

/**
* \def MBEDTLS_BIGNUM_C
*
Expand Down
57 changes: 42 additions & 15 deletions library/aes.c
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,16 @@

#include "mbedtls/platform.h"

/*
* This is a convenience shorthand macro to check if we need reverse S-box and
* reverse tables. It's private and only defined in this file.
*/
#if (!defined(MBEDTLS_AES_DECRYPT_ALT) || \
(!defined(MBEDTLS_AES_SETKEY_DEC_ALT) && !defined(MBEDTLS_AES_USE_HARDWARE_ONLY))) && \
!defined(MBEDTLS_BLOCK_CIPHER_NO_DECRYPT)
#define MBEDTLS_AES_NEED_REVERSE_TABLES
#endif

#if !defined(MBEDTLS_AES_ALT)

#if defined(MBEDTLS_VIA_PADLOCK_HAVE_CODE)
Expand Down Expand Up @@ -401,7 +411,9 @@ MBEDTLS_MAYBE_UNUSED static void aes_gen_tables(void)
* generate the forward and reverse S-boxes
*/
FSb[0x00] = 0x63;
#if defined(MBEDTLS_AES_NEED_REVERSE_TABLES)
RSb[0x63] = 0x00;
#endif

for (i = 1; i < 256; i++) {
x = pow[255 - log[i]];
Expand All @@ -413,7 +425,9 @@ MBEDTLS_MAYBE_UNUSED static void aes_gen_tables(void)
x ^= y ^ 0x63;

FSb[i] = x;
#if defined(MBEDTLS_AES_NEED_REVERSE_TABLES)
RSb[x] = (unsigned char) i;
#endif
}

/*
Expand All @@ -435,10 +449,9 @@ MBEDTLS_MAYBE_UNUSED static void aes_gen_tables(void)
FT3[i] = ROTL8(FT2[i]);
#endif /* !MBEDTLS_AES_FEWER_TABLES */

#if defined(MBEDTLS_AES_NEED_REVERSE_TABLES)
x = RSb[i];

#if !defined(MBEDTLS_AES_DECRYPT_ALT) || \
(!defined(MBEDTLS_AES_SETKEY_DEC_ALT) && !defined(MBEDTLS_AES_USE_HARDWARE_ONLY))
RT0[i] = ((uint32_t) MUL(0x0E, x)) ^
((uint32_t) MUL(0x09, x) << 8) ^
((uint32_t) MUL(0x0D, x) << 16) ^
Expand All @@ -449,8 +462,7 @@ MBEDTLS_MAYBE_UNUSED static void aes_gen_tables(void)
RT2[i] = ROTL8(RT1[i]);
RT3[i] = ROTL8(RT2[i]);
#endif /* !MBEDTLS_AES_FEWER_TABLES */
#endif \
/* !defined(MBEDTLS_AES_DECRYPT_ALT) || (!defined(MBEDTLS_AES_SETKEY_DEC_ALT) && !defined(MBEDTLS_AES_USE_HARDWARE_ONLY)) */
#endif /* MBEDTLS_AES_NEED_REVERSE_TABLES */
}
}

Expand Down Expand Up @@ -682,7 +694,7 @@ int mbedtls_aes_setkey_enc(mbedtls_aes_context *ctx, const unsigned char *key,
/*
* AES key schedule (decryption)
*/
#if !defined(MBEDTLS_AES_SETKEY_DEC_ALT)
#if !defined(MBEDTLS_AES_SETKEY_DEC_ALT) && !defined(MBEDTLS_BLOCK_CIPHER_NO_DECRYPT)
int mbedtls_aes_setkey_dec(mbedtls_aes_context *ctx, const unsigned char *key,
unsigned int keybits)
{
Expand Down Expand Up @@ -751,7 +763,7 @@ int mbedtls_aes_setkey_dec(mbedtls_aes_context *ctx, const unsigned char *key,

return ret;
}
#endif /* !MBEDTLS_AES_SETKEY_DEC_ALT */
#endif /* !MBEDTLS_AES_SETKEY_DEC_ALT && !MBEDTLS_BLOCK_CIPHER_NO_DECRYPT */

#if defined(MBEDTLS_CIPHER_MODE_XTS)
static int mbedtls_aes_xts_decode_keys(const unsigned char *key,
Expand Down Expand Up @@ -940,7 +952,7 @@ int mbedtls_internal_aes_encrypt(mbedtls_aes_context *ctx,
/*
* AES-ECB block decryption
*/
#if !defined(MBEDTLS_AES_DECRYPT_ALT)
#if !defined(MBEDTLS_AES_DECRYPT_ALT) && !defined(MBEDTLS_BLOCK_CIPHER_NO_DECRYPT)
int mbedtls_internal_aes_decrypt(mbedtls_aes_context *ctx,
const unsigned char input[16],
unsigned char output[16])
Expand Down Expand Up @@ -997,7 +1009,7 @@ int mbedtls_internal_aes_decrypt(mbedtls_aes_context *ctx,

return 0;
}
#endif /* !MBEDTLS_AES_DECRYPT_ALT */
#endif /* !MBEDTLS_AES_DECRYPT_ALT && !MBEDTLS_BLOCK_CIPHER_NO_DECRYPT */

/* VIA Padlock and our intrinsics-based implementation of AESNI require
* the round keys to be aligned on a 16-byte boundary. We take care of this
Expand Down Expand Up @@ -1052,13 +1064,15 @@ int mbedtls_aes_crypt_ecb(mbedtls_aes_context *ctx,
#endif

#if !defined(MBEDTLS_AES_USE_HARDWARE_ONLY)
if (mode == MBEDTLS_AES_ENCRYPT) {
return mbedtls_internal_aes_encrypt(ctx, input, output);
} else {
#if !defined(MBEDTLS_BLOCK_CIPHER_NO_DECRYPT)
if (mode == MBEDTLS_AES_DECRYPT) {
return mbedtls_internal_aes_decrypt(ctx, input, output);
}
} else
#endif

{
return mbedtls_internal_aes_encrypt(ctx, input, output);
}
#endif /* !MBEDTLS_AES_USE_HARDWARE_ONLY */
}

#if defined(MBEDTLS_CIPHER_MODE_CBC)
Expand Down Expand Up @@ -1484,6 +1498,7 @@ int mbedtls_aes_crypt_ctr(mbedtls_aes_context *ctx,
*
* http://csrc.nist.gov/archive/aes/rijndael/rijndael-vals.zip
*/
#if !defined(MBEDTLS_BLOCK_CIPHER_NO_DECRYPT)
static const unsigned char aes_test_ecb_dec[][16] =
{
{ 0x44, 0x41, 0x6A, 0xC2, 0xD1, 0xF5, 0x3C, 0x58,
Expand All @@ -1495,6 +1510,7 @@ static const unsigned char aes_test_ecb_dec[][16] =
0x1F, 0x6F, 0x56, 0x58, 0x5D, 0x8A, 0x4A, 0xDE }
#endif
};
#endif

static const unsigned char aes_test_ecb_enc[][16] =
{
Expand Down Expand Up @@ -1876,7 +1892,7 @@ int mbedtls_aes_self_test(int verbose)
*/
{
static const int num_tests =
sizeof(aes_test_ecb_dec) / sizeof(*aes_test_ecb_dec);
sizeof(aes_test_ecb_enc) / sizeof(*aes_test_ecb_enc);

for (i = 0; i < num_tests << 1; i++) {
u = i >> 1;
Expand All @@ -1887,13 +1903,24 @@ int mbedtls_aes_self_test(int verbose)
mbedtls_printf(" AES-ECB-%3u (%s): ", keybits,
(mode == MBEDTLS_AES_DECRYPT) ? "dec" : "enc");
}
#if defined(MBEDTLS_BLOCK_CIPHER_NO_DECRYPT)
if (mode == MBEDTLS_AES_DECRYPT) {
if (verbose != 0) {
mbedtls_printf("skipped\n");
}
continue;
}
#endif

memset(buf, 0, 16);

#if !defined(MBEDTLS_BLOCK_CIPHER_NO_DECRYPT)
if (mode == MBEDTLS_AES_DECRYPT) {
ret = mbedtls_aes_setkey_dec(&ctx, key, keybits);
aes_tests = aes_test_ecb_dec[u];
} else {
} else
#endif
{
ret = mbedtls_aes_setkey_enc(&ctx, key, keybits);
aes_tests = aes_test_ecb_enc[u];
}
Expand Down
13 changes: 10 additions & 3 deletions library/aesce.c
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,7 @@ static uint8x16_t aesce_encrypt_block(uint8x16_t block,
/* Two rounds of AESCE decryption */
#define AESCE_DECRYPT_ROUND_X2 AESCE_DECRYPT_ROUND; AESCE_DECRYPT_ROUND

#if !defined(MBEDTLS_BLOCK_CIPHER_NO_DECRYPT)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here we can drop the guard and just use MBEDTLS_MAYBE_UNUSED as this is static, so the compiler will remove it (I tested this, no impact on code size).

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If a static function / variable is guarded by a single/simple macro, should we guard it directly or use MBEDTLS_MAYBE_UNUSED to let compiler remove it for us?

For example:

MBEDTLS_MAYBE_UNUSED static int aes_setkey_dec_wrap(void *ctx, const unsigned char *key,
                                                    unsigned int key_bitlen)
{
    return mbedtls_aes_setkey_dec((mbedtls_aes_context *) ctx, key, key_bitlen);
}

But I got

error log
./scripts/code_size_compare.py -o HEAD -c tfm-medium -a armv8-m --stdout --markdown
[INFO]: Measure code size between 2939bee-armv8-m-tfm-medium-cc and current-armv8-m-tfm-medium-cc by `size`.
[INFO]: Start to generate code size record for 2939bee.
[INFO]: Start to generate code size record for current.
[ERROR]: Command 'make -j lib CFLAGS='-Os -mcpu=cortex-m33 --target=arm-arm-none-eabi' CC=armclang' returned non-zero exit status 2.
Traceback (most recent call last):
  File "./scripts/code_size_compare.py", line 380, in _build_libraries
    subprocess.check_output(
  File "/usr/lib/python3.8/subprocess.py", line 415, in check_output
    return run(*popenargs, stdout=PIPE, timeout=timeout, check=True,
  File "/usr/lib/python3.8/subprocess.py", line 516, in run
    raise CalledProcessError(retcode, process.args,
subprocess.CalledProcessError: Command 'make -j lib CFLAGS='-Os -mcpu=cortex-m33 --target=arm-arm-none-eabi' CC=armclang' returned non-zero exit status 2.
[ERROR]: Process output:
   CC    aes.c
  CC    aesni.c
  CC    aesce.c
  CC    aria.c
  CC    asn1parse.c
  CC    asn1write.c
  CC    base64.c
  CC    bignum.c
  CC    bignum_core.c
  CC    bignum_mod.c
  CC    bignum_mod_raw.c
  CC    camellia.c
  CC    ccm.c
  CC    chacha20.c
  CC    chachapoly.c
  CC    cipher.c
  CC    cipher_wrap.c
  CC    cmac.c
  CC    constant_time.c
  CC    ctr_drbg.c
  CC    des.c
  CC    dhm.c
  CC    ecdh.c
  CC    ecdsa.c
  CC    ecjpake.c
  CC    ecp.c
  CC    ecp_curves.c
  CC    ecp_curves_new.c
  CC    entropy.c
  CC    entropy_poll.c
  Gen   error.c
cipher_wrap.c:244:12: error: call to undeclared function 'mbedtls_aes_setkey_dec'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
    return mbedtls_aes_setkey_dec((mbedtls_aes_context *) ctx, key, key_bitlen);
           ^
cipher_wrap.c:244:12: note: did you mean 'mbedtls_aes_setkey_enc'?
  CC    hkdf.c
../include/mbedtls/aes.h:167:5: note: 'mbedtls_aes_setkey_enc' declared here
int mbedtls_aes_setkey_enc(mbedtls_aes_context *ctx, const unsigned char *key,
    ^
  CC    gcm.c
1 error generated.
  CC    hmac_drbg.c
make[1]: *** [Makefile:313: cipher_wrap.o] Error 1
make[1]: *** Waiting for unfinished jobs....
make: *** [Makefile:18: lib] Error 2

I'm not sure why compiler doesn't remove aes_setkey_dec_wrap for us. But IMO, if the guard is not very complicated, maybe using guard is more reliable?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If a static function / variable is guarded by a single/simple macro, should we guard it directly or use MBEDTLS_MAYBE_UNUSED to let compiler remove it for us?

The compiler would remove the unused static functions and variables. But in your case, I think it is a compiler error before the removing happens. The preprocessor runs before the compiler, and it removes mbedtls_aes_setkey_dec since it is guarded by preprocessor macro. So you need to guard the function call.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No strong preference on this one. We use MBEDTLS_MAYBE_UNUSED in aes.c, because the guards got so complicated I wanted to remove as many as possible and let the compiler remove functions automatically as far as possible. But aesce.c doesn't have this problem so either approach is OK in my view.

static uint8x16_t aesce_decrypt_block(uint8x16_t block,
unsigned char *keys,
int rounds)
Expand Down Expand Up @@ -230,6 +231,7 @@ static uint8x16_t aesce_decrypt_block(uint8x16_t block,

return block;
}
#endif

/*
* AES-ECB block en(de)cryption
Expand All @@ -242,10 +244,13 @@ int mbedtls_aesce_crypt_ecb(mbedtls_aes_context *ctx,
uint8x16_t block = vld1q_u8(&input[0]);
unsigned char *keys = (unsigned char *) (ctx->buf + ctx->rk_offset);

if (mode == MBEDTLS_AES_ENCRYPT) {
block = aesce_encrypt_block(block, keys, ctx->nr);
} else {
#if !defined(MBEDTLS_BLOCK_CIPHER_NO_DECRYPT)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
#if !defined(MBEDTLS_BLOCK_CIPHER_NO_DECRYPT)
(void) mode;
#if !defined(MBEDTLS_BLOCK_CIPHER_NO_DECRYPT)

if (mode == MBEDTLS_AES_DECRYPT) {
block = aesce_decrypt_block(block, keys, ctx->nr);
} else
#endif
{
block = aesce_encrypt_block(block, keys, ctx->nr);
}
vst1q_u8(&output[0], block);

Expand All @@ -255,6 +260,7 @@ int mbedtls_aesce_crypt_ecb(mbedtls_aes_context *ctx,
/*
* Compute decryption round keys from encryption round keys
*/
#if !defined(MBEDTLS_BLOCK_CIPHER_NO_DECRYPT)
void mbedtls_aesce_inverse_key(unsigned char *invkey,
const unsigned char *fwdkey,
int nr)
Expand All @@ -269,6 +275,7 @@ void mbedtls_aesce_inverse_key(unsigned char *invkey,
vst1q_u8(invkey + i * 16, vld1q_u8(fwdkey + j * 16));

}
#endif

static inline uint32_t aes_rot_word(uint32_t word)
{
Expand Down
2 changes: 2 additions & 0 deletions library/aesce.h
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,7 @@ void mbedtls_aesce_gcm_mult(unsigned char c[16],
const unsigned char b[16]);


#if !defined(MBEDTLS_BLOCK_CIPHER_NO_DECRYPT)
/**
* \brief Internal round key inversion. This function computes
* decryption round keys from the encryption round keys.
Expand All @@ -110,6 +111,7 @@ void mbedtls_aesce_gcm_mult(unsigned char c[16],
void mbedtls_aesce_inverse_key(unsigned char *invkey,
const unsigned char *fwdkey,
int nr);
#endif /* !MBEDTLS_BLOCK_CIPHER_NO_DECRYPT */

/**
* \brief Internal key expansion for encryption
Expand Down
Loading