Fix test function derive_key_exercise()

[mpg]

mbedtls_test_psa_setup_key_derivation_wrap() returns 1 for success, 0 for error, so the test here was wrong. Other uses of this function in the same file have the correct check (either ! or == 0). This was causing tests using derive_key_exercises() to be marked as PASS even in builds where other derive key tests fail which made me notice.

Gatekeeper checklist

• changelog not required - test only
• backport [Backport 2.28] Fix test function derive_key_exercise() #6880
• tests provided - the PR is fixing one

[gilles-peskine-arm]

I don't think we should just exit and mark the test as passed; if we're not doing the actual testing this should be marked as SKIP.

Where is this happening? It's the job of mbedtls_test_psa_setup_key_derivation_wrap to mark the test case as failed.

Returning 1 for success and 0 for failure is a violation of our documented coding guidelines. We're also supposed to test with == 0 or != 0. Having consistent conventions is supposed to help avoid errors like this.

I'm afraid I introduced this convention in test code: auxiliary functions return booleans (int because we don't assume that bool exists) with 1 meaning “I did my job successfully” and 0 meaning “something went wrong and I marked the test case as failed”. I did it precisely so that the expected usage is if (!auxiliary_function()) goto exit; or TEST_ASSERT(auxiliary_function()). I think this reduces the risk of using the wrong direction (but of course it doesn't eliminate it, as we saw here).

I do agree that it's problematic that this is the opposite convention from library functions in terms of what 0 vs nonzero means. But I'm not convinced that changing to 0 for error and 1 for success would be less error-prone. And using library error codes wouldn't really make sense either (would they even be MBEDTLS_ERR_xxx or PSA_ERROR_xxx?) since there's no library error to report.

[gilles-peskine-arm]

Looks correct to me now, with no follow-up needed (unless it causes an existing test to break).

[mpg]

I don't think we should just exit and mark the test as passed; if we're not doing the actual testing this should be marked as SKIP.

Where is this happening? It's the job of mbedtls_test_psa_setup_key_derivation_wrap to mark the test case as failed.

Ah right, missed that, thanks.

[tom-cosgrove-arm]

LGTM