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.

Sign up for GitHub

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

psa: Define mbedtls_ecc_group_to_psa() inline #3301

Merged
merged 2 commits into from
May 19, 2020
Merged

psa: Define mbedtls_ecc_group_to_psa() inline #3301

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1bde9cd
psa: Clear bits in mbedtls_ecc_group_to_psa()
Patater May 5, 2020
2f0eb51
psa: Define mbedtls_ecc_group_to_psa() inline
dgreen-arm Apr 24, 2020
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
4 changes: 4 additions & 0 deletions ChangeLog.d/inline-mbedtls_gcc_group_to_psa.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
Bugfix
* Fix potential linker errors on dual world platforms by inlining
mbedtls_gcc_group_to_psa(). This allows the pk.c module to link separately
from psa_crypto.c. Fixes #3300.
51 changes: 49 additions & 2 deletions include/psa/crypto_extra.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -578,8 +578,55 @@ psa_status_t psa_get_key_domain_parameters(
* (`PSA_ECC_CURVE_xxx`).
* \return \c 0 on failure (\p grpid is not recognized).
*/
psa_ecc_curve_t mbedtls_ecc_group_to_psa( mbedtls_ecp_group_id grpid,
size_t *bits );
static inline psa_ecc_curve_t mbedtls_ecc_group_to_psa( mbedtls_ecp_group_id grpid,
Copy link
Contributor

Choose a reason for hiding this comment

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

This is not a small function thus is it ok to inline it regarding code size? No other way to do that?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure it is so large, but I'm not one to decide. The compiler may decide it is too large or complicated and avoid inlining, when code size optimizing flags are employed.

Alternatives would include making a new C file to implement this function, included in the S target build and again in the NS target build.

Copy link
Contributor

Choose a reason for hiding this comment

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

I decided to do some measurements:

scripts/config.pl baremetal
make CC=arm-none-eabi-gcc CFLAGS='-Os -mthumb -march=armv6-m' lib
arm-none-eabi-size -t library/libmbedcrypto.a | tail -n1

Before this PR: 236581
After this PR: 236601
Difference: +20 bytes

IMO this difference is perfectly acceptable, especially considering that this function is meant to disappear at some point, as it's an artifact of the transition from the legacy crypto APIs to PSA (should probably become useless at some point in the 3.x line and then be removed in 4.0).

Copy link
Contributor

Choose a reason for hiding this comment

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

Given the small size impact and that it is a temporary situation, fine by me then.

size_t *bits )
{
switch( grpid )
{
case MBEDTLS_ECP_DP_SECP192R1:
*bits = 192;
return( PSA_ECC_CURVE_SECP_R1 );
case MBEDTLS_ECP_DP_SECP224R1:
*bits = 224;
return( PSA_ECC_CURVE_SECP_R1 );
case MBEDTLS_ECP_DP_SECP256R1:
*bits = 256;
return( PSA_ECC_CURVE_SECP_R1 );
case MBEDTLS_ECP_DP_SECP384R1:
*bits = 384;
return( PSA_ECC_CURVE_SECP_R1 );
case MBEDTLS_ECP_DP_SECP521R1:
*bits = 521;
return( PSA_ECC_CURVE_SECP_R1 );
case MBEDTLS_ECP_DP_BP256R1:
*bits = 256;
return( PSA_ECC_CURVE_BRAINPOOL_P_R1 );
case MBEDTLS_ECP_DP_BP384R1:
*bits = 384;
return( PSA_ECC_CURVE_BRAINPOOL_P_R1 );
case MBEDTLS_ECP_DP_BP512R1:
*bits = 512;
return( PSA_ECC_CURVE_BRAINPOOL_P_R1 );
case MBEDTLS_ECP_DP_CURVE25519:
*bits = 255;
return( PSA_ECC_CURVE_MONTGOMERY );
case MBEDTLS_ECP_DP_SECP192K1:
*bits = 192;
return( PSA_ECC_CURVE_SECP_K1 );
case MBEDTLS_ECP_DP_SECP224K1:
*bits = 224;
return( PSA_ECC_CURVE_SECP_K1 );
case MBEDTLS_ECP_DP_SECP256K1:
*bits = 256;
return( PSA_ECC_CURVE_SECP_K1 );
case MBEDTLS_ECP_DP_CURVE448:
*bits = 448;
return( PSA_ECC_CURVE_MONTGOMERY );
default:
*bits = 0;
return( 0 );
}
}

/** Convert an ECC curve identifier from the PSA encoding to Mbed TLS.
*
Expand Down
49 changes: 0 additions & 49 deletions library/psa_crypto.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -375,55 +375,6 @@ static inline int psa_key_slot_is_external( const psa_key_slot_t *slot )
#endif /* MBEDTLS_PSA_CRYPTO_SE_C */

#if defined(MBEDTLS_ECP_C)
psa_ecc_curve_t mbedtls_ecc_group_to_psa( mbedtls_ecp_group_id grpid,
size_t *bits )
{
switch( grpid )
{
case MBEDTLS_ECP_DP_SECP192R1:
*bits = 192;
return( PSA_ECC_CURVE_SECP_R1 );
case MBEDTLS_ECP_DP_SECP224R1:
*bits = 224;
return( PSA_ECC_CURVE_SECP_R1 );
case MBEDTLS_ECP_DP_SECP256R1:
*bits = 256;
return( PSA_ECC_CURVE_SECP_R1 );
case MBEDTLS_ECP_DP_SECP384R1:
*bits = 384;
return( PSA_ECC_CURVE_SECP_R1 );
case MBEDTLS_ECP_DP_SECP521R1:
*bits = 521;
return( PSA_ECC_CURVE_SECP_R1 );
case MBEDTLS_ECP_DP_BP256R1:
*bits = 256;
return( PSA_ECC_CURVE_BRAINPOOL_P_R1 );
case MBEDTLS_ECP_DP_BP384R1:
*bits = 384;
return( PSA_ECC_CURVE_BRAINPOOL_P_R1 );
case MBEDTLS_ECP_DP_BP512R1:
*bits = 512;
return( PSA_ECC_CURVE_BRAINPOOL_P_R1 );
case MBEDTLS_ECP_DP_CURVE25519:
*bits = 255;
return( PSA_ECC_CURVE_MONTGOMERY );
case MBEDTLS_ECP_DP_SECP192K1:
*bits = 192;
return( PSA_ECC_CURVE_SECP_K1 );
case MBEDTLS_ECP_DP_SECP224K1:
*bits = 224;
return( PSA_ECC_CURVE_SECP_K1 );
case MBEDTLS_ECP_DP_SECP256K1:
*bits = 256;
return( PSA_ECC_CURVE_SECP_K1 );
case MBEDTLS_ECP_DP_CURVE448:
*bits = 448;
return( PSA_ECC_CURVE_MONTGOMERY );
default:
return( 0 );
}
}

mbedtls_ecp_group_id mbedtls_ecc_group_of_psa( psa_ecc_curve_t curve,
size_t byte_length )
{
Expand Down