Only include psa_pake_setup() and friends if some PAKE algorithms are required

[tom-cosgrove-arm]

This removes several functions if no PAKE algorithms are specified, saving up to ~1.5KB by removing the functions psa_pake_abort(), psa_pake_complete_inputs(), psa_pake_get_implicit_key(), psa_pake_input(), psa_pake_output(), psa_pake_set_password_key(), psa_pake_set_peer(), psa_pake_set_role(), psa_pake_set_user() and psa_pake_setup() completely if they are not needed.

Code size improvements for Armv8-M/Thumb2:

orig:1d046fa0d	orig:6d62faca8	Delta	Filename
37556, 4	36014, 4	-1542+0	psa_crypto.o

PR checklist

Please tick as appropriate and edit the reasons (e.g.: "backport: not needed because this is a new feature")

• changelog provided
• backport not required - PAKE is development-only
• tests not required

[tom-cosgrove-arm]

Note that if we change things to just return NOT_SUPPORTED, code size goes up quite a bit (for Armv8-M/Thumb2)

orig:6d62faca8	orig:ea62b510a	Delta	Filename
36014, 4	36234, 4	+220+0	psa_crypto.o

with the change

diff --git a/library/psa_crypto.c b/library/psa_crypto.c
index 2bd4df107..172cecf7d 100644
--- a/library/psa_crypto.c
+++ b/library/psa_crypto.c
@@ -7384,11 +7384,15 @@ psa_status_t psa_crypto_driver_pake_get_cipher_suite(
     return PSA_SUCCESS;
 }
 
-#if defined(PSA_WANT_ALG_SOME_PAKE)
 psa_status_t psa_pake_setup(
     psa_pake_operation_t *operation,
     const psa_pake_cipher_suite_t *cipher_suite)
 {
+#if !defined(PSA_WANT_ALG_SOME_PAKE)
+    (void) operation;
+    (void) cipher_suite;
+    return PSA_ERROR_NOT_SUPPORTED;
+#else
     psa_status_t status = PSA_ERROR_CORRUPTION_DETECTED;
 
     if (operation->stage != PSA_PAKE_OPERATION_STAGE_SETUP) {
@@ -7431,12 +7435,18 @@ psa_status_t psa_pake_setup(
 exit:
     psa_pake_abort(operation);
     return status;
+#endif /* PSA_WANT_ALG_SOME_PAKE */
 }
 
 psa_status_t psa_pake_set_password_key(
     psa_pake_operation_t *operation,
     mbedtls_svc_key_id_t password)
 {
+#if !defined(PSA_WANT_ALG_SOME_PAKE)
+    (void) operation;
+    (void) password;
+    return PSA_ERROR_NOT_SUPPORTED;
+#else
     psa_status_t status = PSA_ERROR_CORRUPTION_DETECTED;
     psa_status_t unlock_status = PSA_ERROR_CORRUPTION_DETECTED;
     psa_key_slot_t *slot = NULL;
@@ -7480,6 +7490,7 @@ exit:
     }
     unlock_status = psa_unlock_key_slot(slot);
     return (status == PSA_SUCCESS) ? unlock_status : status;
+#endif /* PSA_WANT_ALG_SOME_PAKE */
 }
 
 psa_status_t psa_pake_set_user(
@@ -7487,6 +7498,12 @@ psa_status_t psa_pake_set_user(
     const uint8_t *user_id,
     size_t user_id_len)
 {
+#if !defined(PSA_WANT_ALG_SOME_PAKE)
+    (void) operation;
+    (void) user_id;
+    (void) user_id_len;
+    return PSA_ERROR_NOT_SUPPORTED;
+#else
     psa_status_t status = PSA_ERROR_CORRUPTION_DETECTED;
 
     if (operation->stage != PSA_PAKE_OPERATION_STAGE_COLLECT_INPUTS) {
@@ -7517,6 +7534,7 @@ psa_status_t psa_pake_set_user(
 exit:
     psa_pake_abort(operation);
     return status;
+#endif /* PSA_WANT_ALG_SOME_PAKE */
 }
 
 psa_status_t psa_pake_set_peer(
@@ -7524,6 +7542,12 @@ psa_status_t psa_pake_set_peer(
     const uint8_t *peer_id,
     size_t peer_id_len)
 {
+#if !defined(PSA_WANT_ALG_SOME_PAKE)
+    (void) operation;
+    (void) peer_id;
+    (void) peer_id_len;
+    return PSA_ERROR_NOT_SUPPORTED;
+#else
     psa_status_t status = PSA_ERROR_CORRUPTION_DETECTED;
 
     if (operation->stage != PSA_PAKE_OPERATION_STAGE_COLLECT_INPUTS) {
@@ -7554,12 +7578,18 @@ psa_status_t psa_pake_set_peer(
 exit:
     psa_pake_abort(operation);
     return status;
+#endif /* PSA_WANT_ALG_SOME_PAKE */
 }
 
 psa_status_t psa_pake_set_role(
     psa_pake_operation_t *operation,
     psa_pake_role_t role)
 {
+#if !defined(PSA_WANT_ALG_SOME_PAKE)
+    (void) operation;
+    (void) role;
+    return PSA_ERROR_NOT_SUPPORTED;
+#else
     psa_status_t status = PSA_ERROR_CORRUPTION_DETECTED;
 
     if (operation->stage != PSA_PAKE_OPERATION_STAGE_COLLECT_INPUTS) {
@@ -7584,6 +7614,7 @@ psa_status_t psa_pake_set_role(
 exit:
     psa_pake_abort(operation);
     return status;
+#endif /* PSA_WANT_ALG_SOME_PAKE */
 }
 
 /* Auxiliary function to convert core computation stage(step, sequence, state) to single driver step. */
@@ -7642,6 +7673,7 @@ static psa_crypto_driver_pake_step_t convert_jpake_computation_stage_to_driver_s
 }
 #endif /* PSA_WANT_ALG_JPAKE */
 
+#if defined(PSA_WANT_ALG_SOME_PAKE)
 static psa_status_t psa_pake_complete_inputs(
     psa_pake_operation_t *operation)
 {
@@ -7691,6 +7723,7 @@ static psa_status_t psa_pake_complete_inputs(
     }
     return status;
 }
+#endif /* PSA_WANT_ALG_SOME_PAKE */
 
 #if defined(PSA_WANT_ALG_JPAKE)
 static psa_status_t psa_jpake_output_prologue(
@@ -7793,6 +7826,14 @@ psa_status_t psa_pake_output(
     size_t output_size,
     size_t *output_length)
 {
+#if !defined(PSA_WANT_ALG_SOME_PAKE)
+    (void) operation;
+    (void) step;
+    (void) output;
+    (void) output_size;
+    (void) output_length;
+    return PSA_ERROR_NOT_SUPPORTED;
+#else
     psa_status_t status = PSA_ERROR_CORRUPTION_DETECTED;
     psa_crypto_driver_pake_step_t driver_step = PSA_JPAKE_STEP_INVALID;
     *output_length = 0;
@@ -7856,6 +7897,7 @@ psa_status_t psa_pake_output(
 exit:
     psa_pake_abort(operation);
     return status;
+#endif /* PSA_WANT_ALG_SOME_PAKE */
 }
 
 #if defined(PSA_WANT_ALG_JPAKE)
@@ -7958,6 +8000,13 @@ psa_status_t psa_pake_input(
     const uint8_t *input,
     size_t input_length)
 {
+#if !defined(PSA_WANT_ALG_SOME_PAKE)
+    (void) operation;
+    (void) step;
+    (void) input;
+    (void) input_length;
+    return PSA_ERROR_NOT_SUPPORTED;
+#else
     psa_status_t status = PSA_ERROR_CORRUPTION_DETECTED;
     psa_crypto_driver_pake_step_t driver_step = PSA_JPAKE_STEP_INVALID;
     const size_t max_input_length = (size_t) PSA_PAKE_INPUT_SIZE(operation->alg,
@@ -8023,12 +8072,18 @@ psa_status_t psa_pake_input(
 exit:
     psa_pake_abort(operation);
     return status;
+#endif /* PSA_WANT_ALG_SOME_PAKE */
 }
 
 psa_status_t psa_pake_get_implicit_key(
     psa_pake_operation_t *operation,
     psa_key_derivation_operation_t *output)
 {
+#if !defined(PSA_WANT_ALG_SOME_PAKE)
+    (void) operation;
+    (void) output;
+    return PSA_ERROR_NOT_SUPPORTED;
+#else
     psa_status_t status = PSA_ERROR_CORRUPTION_DETECTED;
     psa_status_t abort_status = PSA_ERROR_CORRUPTION_DETECTED;
     uint8_t shared_key[MBEDTLS_PSA_JPAKE_BUFFER_SIZE];
@@ -8073,11 +8128,16 @@ psa_status_t psa_pake_get_implicit_key(
 exit:
     abort_status = psa_pake_abort(operation);
     return status == PSA_SUCCESS ? abort_status : status;
+#endif /* PSA_WANT_ALG_SOME_PAKE */
 }
 
 psa_status_t psa_pake_abort(
     psa_pake_operation_t *operation)
 {
+#if !defined(PSA_WANT_ALG_SOME_PAKE)
+    (void) operation;
+    return PSA_ERROR_NOT_SUPPORTED;
+#else
     psa_status_t status = PSA_SUCCESS;
 
     if (operation->stage == PSA_PAKE_OPERATION_STAGE_COMPUTATION) {
@@ -8100,7 +8160,7 @@ psa_status_t psa_pake_abort(
     memset(operation, 0, sizeof(psa_pake_operation_t));
 
     return status;
-}
 #endif /* PSA_WANT_ALG_SOME_PAKE */
+}
 
 #endif /* MBEDTLS_PSA_CRYPTO_C */

[gilles-peskine-arm]

Note that if we change things to just return NOT_SUPPORTED, code size goes up quite a bit

Do we care though? If these functions aren't called by anything, surely any linker from this century should remove them.

Note that we might make a difference between the PAKE functions (which are not called by TF-M's crypto service) and other PSA library entry points (which are). We should probably measure the code size in a complete program rather than a library, and that complete program would need to include a call (with arguments that the compiler can't use for partial evaluation) to all the entry points we want to have in the build.

[daverodgman]

There are a bunch more functions immediately above this: psa_crypto_driver_pake_get_cipher_suite(), psa_crypto_driver_pake_get_peer(), etc. Excluding these as well seems to make a difference - is there a reason these are not covered?

@gilles-peskine-arm this is a fair point - I discussed this with @tom-cosgrove-arm recently. Since we are using TF-M as our size optimisation use-case, we'd really have to start building TF-M to get a more complete answer, which would get complicated. I'm not sure if Tom had some ideas about improving the way we measure. But for now, I think this is OK.

[daverodgman]

LGTM, but seems to miss some functions that could additionally be excluded

[daverodgman]

Suggested change

	if no PAKE algorithms are requested
	if no PAKE algorithms are requested.

[gilles-peskine-arm]

we'd really have to start building TF-M to get a more complete answer, which would get complicated

Building TF-M is the ultimately precise measurement, but we don't need to go that far to get a good approximation. We only need the crypto service (plus any function that TF-M calls directly for its own sake, I'm not sure if there are any), and even there we don't need to plug it into an actual IPC mechanism. Since the set of cryptographic services offered by TF-M does not change often, we can take a copy of the files, or even of the list of functions now, and build a basic main out of that.

[davidhorstmann-arm]

LGTM, thanks!