After having a deeper look at the spec, I think that the it would only mandate the key_attributes object to be correctly initialized before being used, i.e. what happens at line 21.
Initialisation in our case means that the alg field is initialized to 0 which, as the spec describes, corresponds to PSA_ALG_NONE, i.e. the most restrictive algorithm allowed. Hence a key attribute object that does not receive explicitly
a call to psa_set_key_algorithm is equivalent to an implicit call to psa_set_key_algorithm(&key_attributes, PSA_ALG_NONE);
I think this is the expected behaviour as the spec itself does not mandate a call to set_algorithm() for those cases that don’t need to allow any particular algorithm usage. Indeed, your failures seem to be unrelated to the presence of
this particular call, as for example the key_interface tests don’t call this function, but for the cipher tests are calling it explicitly, but in your case you mention that those tests are failing as well (i.e. test 1040). In brief without knowing the details
of what you’re actually testing, it’s difficult for me to give a plausible explanation. But if you want to point me to a change in gerrit, I can try to have a look and comment directly on the code.
Thanks,
Antonio
From: Swarowsky, Markus <Markus.Swarowsky@nordicsemi.no>
Sent: Thursday, October 13, 2022 14:45
To: Antonio De Angelis <Antonio.DeAngelis@arm.com>; tf-m@lists.trustedfirmware.org
Cc: nd <nd@arm.com>
Subject: Re: [TF-M] TF-M psa crypto test do not set the algorithms in psa_key_attributes_t
Hey Antonio,
Yes, I was referring to use of
psa_set_key_algorithm(&key_attributes, alg);
to set the policy and good to hear that it's good practice to set them.
The failing TF-M tests are
TFM_S_CRYPTO_TEST_1001/2/5/7/30/31/33/34/35/36/40/42/43/44/45/46/47/48 ,same for TFM_S_CRYPTO_TEST_10...
They all use psa_key_interface_test wher only usage and type is set if I got the code here right: https://git.trustedfirmware.org/TF-M/tf-m-tests.git/tree/test/secure_fw/suites/crypto/crypto_tests_common.c#n27
PSA-arch-tests
For test_c002/3/4/48/20/21/24-42/44/46-61_* fail,
As example for
test_c002_testing_crypto_key_management_apis the attributes are set up here https://github.com/ARM-software/psa-arch-tests/blob/main/api-tests/dev_apis/crypto/test_c002/test_c002.c#L59 if
I got it right.
And test_c005_testing_crypto_key_management_apis, which is passing the algorithm is set here: https://github.com/ARM-software/psa-arch-tests/blob/main/api-tests/dev_apis/crypto/test_c005/test_c005.c#L72
I hope this descripts the issue a bit better.
From: Antonio De Angelis <Antonio.DeAngelis@arm.com>
Sent: 13 October 2022 14:52
To: tf-m@lists.trustedfirmware.org <tf-m@lists.trustedfirmware.org>
Cc: Swarowsky, Markus <Markus.Swarowsky@nordicsemi.no>; Chris.Brand--- via TF-M <tf-m@lists.trustedfirmware.org>; nd <nd@arm.com>
Subject: RE: [TF-M] TF-M psa crypto test do not set the algorithms in psa_key_attributes_t
Hi Markus,
psa_set_key_algorithm() should be called when properly setting up the policy for permitted algorithm on a key. From what I could quickly see it seems that the positive-tests in the Crypto regression are setting them correctly.
An example snippet of what I am referring to:
/* Setup the key policy */
psa_set_key_usage_flags(&key_attributes, PSA_KEY_USAGE_VERIFY_HASH);
psa_set_key_algorithm(&key_attributes, alg);
psa_set_key_type(&key_attributes, key_type);
Are you referring to some particular test that is not setting the algorithm field in the key attributes? Could you point me to that? It might very well be that setting it has been overlooked when writing some _negative_ test that
wasn’t aiming at preparing the object correctly because it was just trying to cause a failure, hence it did not care about that algorithm policy field. But in general, I can confirm that it’s good practice to set the algorithm and if any of the tests are not
aligned to this pattern, they must be fixed upstream for consistency.
Thanks,
Antonio
From: Swarowsky, Markus via TF-M <tf-m@lists.trustedfirmware.org>
Sent: Thursday, October 13, 2022 13:14
To: Chris.Brand--- via TF-M <tf-m@lists.trustedfirmware.org>
Subject: [TF-M] TF-M psa crypto test do not set the algorithms in psa_key_attributes_t
Hello,
I was adding sanity checks to the psa_crypto_driver_wrappers to validate bits/type and policy->alg of the given psa_key_attributes_t attributes for the psa_generate_key, psa_import_key, and psa_copy_key calls.
To check it the given combination is within the psa specification.
In the PSA crypto spec it says:
The key permitted-algorithm policy is required for keys that will be used for a cryptographic operation, see Permitted
algorithms. [https://armmbed.github.io/mbed-crypto/html/api/keys/management.html?highlight=psa_generate_key#c.psa_generate_key]
Which is most likely the case for all key types except of PSA_KEY_TYPE_RAW_DATA
But after adding the sanity checks the TF-M crypto tests(test_c*_testing_* and TFM_S/NS_CRYPTO_TEST_10*) failed.
I took a look at these tests, it turned out that they don't set the algorithm flag for the key attributes.
My question now is, is the algorithm flag skipped on purpose or just missed during the test case creation as It should be set according to the psa crypto spec?
Thanks for the help and the feedback
Markus Swarowsky |
R & D Engineer
M +47 404 66 922 | Trondheim, Norway
nordicsemi.com | devzone.nordicsemi.com
Facebook | LinkedIn | Twitter | YouTube | Instagram
