Thanks for the additional details. I think I now understand what's your goal here. So, the spec does not mandate that only legal combinations of key_alg and key_type should be allowed when the key is imported. Of course, an illegal combination of these two fields will fail in the moment when a crypto operation tries to use that key, but implementations can choose either to fail on psa_import_key() or return success, in the cases where key_type and alg don't match.
Having said that, I think that loading a key_type with PSA_ALG_NONE should be considered still a valid combination, even if your implementation wants to check key_type/key_alg compatibility as part of psa_import_key(). This is because there might be legitimate use cases where the user wants to use the Crypto service just as secure key storage, i.e. they don't plan to perform any algorithm on the key hence it makes sense for them to set PSA_ALG_NONE.
So in brief, I think the approach of letting PSA_ALG_NONE for any key type is recommended.
Please do let me know if any doubts.
Thanks, Antonio
From: Swarowsky, Markus Markus.Swarowsky@nordicsemi.no Sent: Friday, October 14, 2022 11:37 To: tf-m@lists.trustedfirmware.org; Antonio De Angelis Antonio.DeAngelis@arm.com Subject: Re: [TF-M] Re: TF-M psa crypto test do not set the algorithms in psa_key_attributes_t
Hey,
The checks are actually done in some nordic code, so can't point to public Commit so far. What is done is basically checking if the given key type matches the permitted algorithm e.g key type :PSA_KEY_TYPE_CHACHA20 then the algorithm should only be PSA_ALG_STREAM_CIPHER or PSA_ALG_CHACHA20_POLY1305, based on: https://armmbed.github.io/mbed-crypto/html/api/keys/types.html#PSA_KEY_TYPE_...
Which fails if the algorithm field is set to PSA_ALG_NONE, which I understood not the way it should be used, but that might be wrong. Then I could allow PSA_ALG_NONE for any key type.
________________________________ From: Antonio De Angelis via TF-M <tf-m@lists.trustedfirmware.orgmailto:tf-m@lists.trustedfirmware.org> Sent: 13 October 2022 16:11 To: tf-m@lists.trustedfirmware.orgmailto:tf-m@lists.trustedfirmware.org <tf-m@lists.trustedfirmware.orgmailto:tf-m@lists.trustedfirmware.org> Cc: nd <nd@arm.commailto:nd@arm.com> Subject: [TF-M] Re: TF-M psa crypto test do not set the algorithms in psa_key_attributes_t
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.nomailto:Markus.Swarowsky@nordicsemi.no> Sent: Thursday, October 13, 2022 14:45 To: Antonio De Angelis <Antonio.DeAngelis@arm.commailto:Antonio.DeAngelis@arm.com>; tf-m@lists.trustedfirmware.orgmailto:tf-m@lists.trustedfirmware.org Cc: nd <nd@arm.commailto: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/suit...https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit.trustedfirmware.org%2FTF-M%2Ftf-m-tests.git%2Ftree%2Ftest%2Fsecure_fw%2Fsuites%2Fcrypto%2Fcrypto_tests_common.c%23n27&data=05%7C01%7Cmarkus.swarowsky%40nordicsemi.no%7C9acd20eca9664bbc466e08daad24db30%7C28e5afa2bf6f419a8cf6b31c6e9e5e8d%7C0%7C0%7C638012671054541566%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=2xvp3tmoQRTZ6UYrjMzzvtym1NMNzJcu%2FR68cXX4%2FSw%3D&reserved=0
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/...https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2FARM-software%2Fpsa-arch-tests%2Fblob%2Fmain%2Fapi-tests%2Fdev_apis%2Fcrypto%2Ftest_c002%2Ftest_c002.c%23L59&data=05%7C01%7Cmarkus.swarowsky%40nordicsemi.no%7C9acd20eca9664bbc466e08daad24db30%7C28e5afa2bf6f419a8cf6b31c6e9e5e8d%7C0%7C0%7C638012671054697806%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=3f2mw6WhZ4wdFg9MUkeQ3PsQlYoNK5UWR3BR0lpjIMs%3D&reserved=0 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/...https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2FARM-software%2Fpsa-arch-tests%2Fblob%2Fmain%2Fapi-tests%2Fdev_apis%2Fcrypto%2Ftest_c005%2Ftest_c005.c%23L72&data=05%7C01%7Cmarkus.swarowsky%40nordicsemi.no%7C9acd20eca9664bbc466e08daad24db30%7C28e5afa2bf6f419a8cf6b31c6e9e5e8d%7C0%7C0%7C638012671054697806%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=a%2BENyru3YLsQg62roSwKMZTb6xwCbN0mdcxstvi4cBU%3D&reserved=0
I hope this descripts the issue a bit better.
________________________________
From: Antonio De Angelis <Antonio.DeAngelis@arm.commailto:Antonio.DeAngelis@arm.com> Sent: 13 October 2022 14:52 To: tf-m@lists.trustedfirmware.orgmailto:tf-m@lists.trustedfirmware.org <tf-m@lists.trustedfirmware.orgmailto:tf-m@lists.trustedfirmware.org> Cc: Swarowsky, Markus <Markus.Swarowsky@nordicsemi.nomailto:Markus.Swarowsky@nordicsemi.no>; Chris.Brand--- via TF-M <tf-m@lists.trustedfirmware.orgmailto:tf-m@lists.trustedfirmware.org>; nd <nd@arm.commailto: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.orgmailto:tf-m@lists.trustedfirmware.org> Sent: Thursday, October 13, 2022 13:14 To: Chris.Brand--- via TF-M <tf-m@lists.trustedfirmware.orgmailto: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 algorithmshttps://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Farmmbed.github.io%2Fmbed-crypto%2Fhtml%2Fapi%2Fkeys%2Fpolicy.html%23permitted-algorithms&data=05%7C01%7Cmarkus.swarowsky%40nordicsemi.no%7C9acd20eca9664bbc466e08daad24db30%7C28e5afa2bf6f419a8cf6b31c6e9e5e8d%7C0%7C0%7C638012671054697806%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=iUfh%2F8jSZW8BHp2M9xKuX5mnN6DLYWpHwrnWjz8tMQ0%3D&reserved=0. [https://armmbed.github.io/mbed-crypto/html/api/keys/management.html?highligh...https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Farmmbed.github.io%2Fmbed-crypto%2Fhtml%2Fapi%2Fkeys%2Fmanagement.html%3Fhighlight%3Dpsa_generate_key%23c.psa_generate_key&data=05%7C01%7Cmarkus.swarowsky%40nordicsemi.no%7C9acd20eca9664bbc466e08daad24db30%7C28e5afa2bf6f419a8cf6b31c6e9e5e8d%7C0%7C0%7C638012671054697806%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=VHY0ITtgurt3G45X2NeMajpzfSf6WumMmBaz2j8uXb8%3D&reserved=0] 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.comhttps://eur03.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww.nordicsemi.com%2F&data=05%7C01%7Cmarkus.swarowsky%40nordicsemi.no%7C9acd20eca9664bbc466e08daad24db30%7C28e5afa2bf6f419a8cf6b31c6e9e5e8d%7C0%7C0%7C638012671054697806%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=6SESteMoC%2B1ntPvUisG7XyBZWsLLL1%2F4kZv%2B9rdHbeo%3D&reserved=0 | devzone.nordicsemi.comhttps://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fdevzone.nordicsemi.com%2F&data=05%7C01%7Cmarkus.swarowsky%40nordicsemi.no%7C9acd20eca9664bbc466e08daad24db30%7C28e5afa2bf6f419a8cf6b31c6e9e5e8d%7C0%7C0%7C638012671054697806%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=sJldpAAqsLjYE%2FM3QMQcqL4n2OIgHR4hZPsO1hXzcMc%3D&reserved=0
Facebookhttps://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.facebook.com%2Fnordicsemiconductor%2F&data=05%7C01%7Cmarkus.swarowsky%40nordicsemi.no%7C9acd20eca9664bbc466e08daad24db30%7C28e5afa2bf6f419a8cf6b31c6e9e5e8d%7C0%7C0%7C638012671054697806%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=FJ6BkCVD5PCBKDfpBGg4vwM%2FLAEs8Bow2DxCP9%2BjGLs%3D&reserved=0 | LinkedInhttps://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.linkedin.com%2Fcompany%2Fnordic-semiconductor-asa%2F&data=05%7C01%7Cmarkus.swarowsky%40nordicsemi.no%7C9acd20eca9664bbc466e08daad24db30%7C28e5afa2bf6f419a8cf6b31c6e9e5e8d%7C0%7C0%7C638012671054697806%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=ygfDc5KnFZRBWRCw4XWjnHo1g3UVJxEApPSx2WV8XBc%3D&reserved=0 | Twitterhttps://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Ftwitter.com%2FNordicTweets&data=05%7C01%7Cmarkus.swarowsky%40nordicsemi.no%7C9acd20eca9664bbc466e08daad24db30%7C28e5afa2bf6f419a8cf6b31c6e9e5e8d%7C0%7C0%7C638012671054697806%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=w0ardfaXr3bppThh9N5SyjKC5BsmkAs1xN%2B3hbzv4Jk%3D&reserved=0 | YouTubehttps://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.youtube.com%2Fuser%2FNordicSemi&data=05%7C01%7Cmarkus.swarowsky%40nordicsemi.no%7C9acd20eca9664bbc466e08daad24db30%7C28e5afa2bf6f419a8cf6b31c6e9e5e8d%7C0%7C0%7C638012671054697806%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=PO%2Fq7i%2BBHEBJNXCvp0MC0JPMrUo0Xm3oEy23Q5Re8Ho%3D&reserved=0 | Instagramhttps://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.instagram.com%2Fnordicsemi%2F&data=05%7C01%7Cmarkus.swarowsky%40nordicsemi.no%7C9acd20eca9664bbc466e08daad24db30%7C28e5afa2bf6f419a8cf6b31c6e9e5e8d%7C0%7C0%7C638012671054854048%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=f2IvPGRC8s2w5guB%2FgavVoLfVe8afxO1OqcEaaspWdo%3D&reserved=0
