Add test for driver-only RSA (crypto only)

[lpy4105]

Description

Fix: #8553

Two test components added:

• test_psa_crypto_config_accel_rsa_crypto
• test_psa_crypto_config_reference_rsa_crypto

PR checklist

• changelog not required, test components
• backport not required, no driver support on 2.28
• tests not required

[lpy4105]

A question for reviewers.

[mpg]

Looks pretty good to me, thanks! Just minor things, plus one question about something I need to understand better before I can approve.

[mpg]

Question: it's not obvious to me why you picked those exact guards, can you explain?

[lpy4105]

These macros come from:

mbedtls/library/psa_crypto.c

Lines 1375 to 1405 in f3ccfdd

	switch (slot->attr.type) {
	#if (defined(MBEDTLS_PSA_BUILTIN_KEY_TYPE_RSA_KEY_PAIR_IMPORT) && \
	defined(MBEDTLS_PSA_BUILTIN_KEY_TYPE_RSA_KEY_PAIR_EXPORT)) || \
	defined(MBEDTLS_PSA_BUILTIN_KEY_TYPE_RSA_PUBLIC_KEY)
	case PSA_KEY_TYPE_RSA_KEY_PAIR:
	case PSA_KEY_TYPE_RSA_PUBLIC_KEY:
	/* TODO: reporting the public exponent for opaque keys
	* is not yet implemented.
	* https://github.com/ARMmbed/mbed-crypto/issues/216
	*/
	if (!psa_key_lifetime_is_external(slot->attr.lifetime)) {
	mbedtls_rsa_context *rsa = NULL;
	status = mbedtls_psa_rsa_load_representation(
	slot->attr.type,
	slot->key.data,
	slot->key.bytes,
	&rsa);
	if (status != PSA_SUCCESS) {
	break;
	}
	status = psa_get_rsa_public_exponent(rsa,
	attributes);
	mbedtls_rsa_free(rsa);
	mbedtls_free(rsa);
	}
	break;
	#endif /* (defined(MBEDTLS_PSA_BUILTIN_KEY_TYPE_RSA_KEY_PAIR_IMPORT) && \
	* defined(MBEDTLS_PSA_BUILTIN_KEY_TYPE_RSA_KEY_PAIR_EXPORT)) ||
	* defined(MBEDTLS_PSA_BUILTIN_KEY_TYPE_RSA_PUBLIC_KEY) */

If these conditions are not fitted, psa_get_key_attributes can't get RSA key attributes but returns PSA_SUCCESS.

[mpg]

Thanks for the explanation! I'll note the comment there is outdated (references an issue that was closed) and I'm tempted to slightly expand the scope of that PR to include updating that comment, but I'm not sure what the proper replacement would be yet.

@gilles-peskine-arm It looks like the prediction in the last comment in the issue listed above was not realised, as we seem to now find ourselves in a situation where we have an (experimental, in crypto_extra.h) API for custom domains parameters, with psa_set/get_key_domain_parameters() allowing to set/get custom public exponents for RSA keys, but no corresponding support in the driver API that I could find.

More precisely, I'm not sure how psa_get_key_attributes() is supposed to populate the attributes structure with the public exponent in case RSA keys are supported only by a driver? I think it's OK if we have a gap for now (which could be closed either by expanding driver support or by removing the experimental domain_parameters extension), but then we need to make sure we have an open issue tracking it.

Also, if we accept this gap for now, I'm a bit uncomfortable with the fact that psa_get_key_attributes() and then psa_get_key_domain_parameters() will both return success even when the actual value of e can't be accessed. I'd say this would deserve at least a warning in the documentation of psa_get_key_domain_parameters() that with RSA keys handled by a driver, the exponent will always be reported as the default one regardless of its actual value. Or should we make psa_get_key_domain_parameters() return an error in this case?

[gilles-peskine-arm]

We're in the uncomfortable position where domain parameters are deprecated, so we haven't included them in newer parts of the library, such as the driver interface. But we haven't removed them partly because removing takes time and partly because there is one thing that we care about that requires them, namely generating an RSA key with a non-default exponent.

My preferred resolution would be to:

1. Finish designing psa_generate_key_ext and implement it and its driver counterpart.
2. In a subsequent minor release, remove domain parameters.
3. Now you don't have this problem anymore.

But obviously that would take time that we don't have.

In the meantime, I think ideally psa_get_key_domain_parameters should return an error if it can't obtain the public exponent. psa_get_key_attributes must always succeed (except if access to the key object fails in some way, e.g. an invalid key identifier or an operational error).

[mpg]

Ah, good to know it's already tracked.

Then for this PR I'd like to suggest the following approach. In the test function: keep calling psa_get_key_attributes() etc. in all builds. However, for psa_get_key_domain_parameters(), instead of using PSA_ASSERT() (meaning it must always succeed) store the return value then:

#if WE_HAVE_ENOUGH_BUILTIN_RSA_FOR_THIS_TO_WORK
// assert status was SUCCESS
// check the value is as expected
#else
// assert status for NOT_SUPPORTED
// ignore unused parameters
#endif

And make the necessary changes for this test to pass.

Add comments / update existing comments to explain that this is a temporary situation with a reference to #6494.

@lpy4105 Does that work for you? That's arguably a slight scope extension, but it seems natural to fix the issue that was uncovered by the improved testing.

[mpg]

LGTM, thanks!

I'm withholding approval for now because I still want to understand better the limitation you uncovered about custom domain parameters (ie, non-default public exponents). It probably won't be resolved here, but I'd like to make sure we have task(s) created to track it before closing that discussion.

[mpg]

LGTM if the CI agrees

[mpg]

LGTM, thanks!

[valeriosetti]

LGTM

[gilles-peskine-arm]

Setting domain_parameters_size to SIZE_MAX is wrong: it's a promise that you can read SIZE_MAX bytes from domain_parameters. This might not be critical since this is a private field, and its only accessor function psa_get_domain_parameters takes care of that case. But at the very least, it needs to be documented in the definition of struct psa_key_attributes_s in psa/crypto_struct.h.

#8644