Hi Soby and Summer,

Thank you for pointing me to the complete chain of patches. I didn’t notice. With this, PSoC64 behaves quite well without any buffer changes. In fact, the results look better than previously published AN521 results – what are the latest AN521 results? It looks like 250 and 251 now passes but failed in previous AN521 results. Again, this is for PSoC64

As for Musca-B1, it looks like it is better (CC312 enabled). 241/242 is still failing and 250 is failing (passes for PSoC64). Disabling CC312 will result in 241/242 passing

Thank you,

Ray

From: Soby Mathew <Soby.Mathew@arm.com>
Sent: Thursday, October 15, 2020 8:01 AM
To: Summer Qin <Summer.Qin@arm.com>; Raymond Ngun <Raymond.Ngun@cypress.com>
Cc: nd <nd@arm.com>; tf-m@lists.trustedfirmware.org
Subject: RE: Behavior difference in Crypto IPC vs Library modes

Hi Raymond,

Some of the PSA Crypto tests require a larger buffer size and previously this was done within the build system. This size is required irrespective of IPC or Library mode. The new build system broke this buffer size configuration for API tests and the patch mentioned by Summer resolves that. Could you try with that and let us know ?

Regarding Musca-B1, we switched to using Cryptocell as default for that platform recently. There are some limitations for the CC-312 with respect to some crypto APIs and I suspect the failures are related to this. I will create a ticket to look further into this. Meanwhile could you try whether you have failures if you disable CC-312 for Musca-B1 :

diff --git a/platform/ext/target/musca_b1/config.cmake b/platform/ext/target/musca_b1/config.cmake

index b343af36..47a2bfad 100644

--- a/platform/ext/target/musca_b1/config.cmake

+++ b/platform/ext/target/musca_b1/config.cmake

@@ -6,5 +6,5 @@

#-------------------------------------------------------------------------------

set(PLATFORM_DUMMY_ATTEST_HAL FALSE CACHE BOOL "Use dummy boot hal implementation. Should not be used in production." FORCE)

-set(CRYPTO_HW_ACCELERATOR ON CACHE BOOL "Whether to enable the crypto hardware accelerator on supported platforms" FORCE)

+set(CRYPTO_HW_ACCELERATOR OFF CACHE BOOL "Whether to enable the crypto hardware accelerator on supported platforms" FORCE)

set(TFM_CRYPTO_TEST_ALG_CFB OFF CACHE BOOL "Test CFB cryptography mode" FORCE)

Best Regards

Soby Mathew

From: Summer Qin <Summer.Qin@arm.com>
Sent: 15 October 2020 07:58
To: Soby Mathew <Soby.Mathew@arm.com>; Raymond Ngun <Raymond.Ngun@cypress.com>
Cc: nd <nd@arm.com>; tf-m@lists.trustedfirmware.org
Subject: Re: Behavior difference in Crypto IPC vs Library modes

Hi Raymond,

Do you cherry-pick all the series patches (topic:

sm/new_build_crypto

) or just only pick the one Soby provided?

I testes on AN521, without all the series patches, 219, 241, 242, and 243 are failed. But when you cherry-pick all series patches, they can pass.

And I think patch https://review.trustedfirmware.org/c/TF-M/trusted-firmware-m/+/6251 already increase the size for CRYPTO_ENGINE_BUF_SIZE.

Thanks,

Summer

From: TF-M <tf-m-bounces@lists.trustedfirmware.org> on behalf of Raymond Ngun via TF-M <tf-m@lists.trustedfirmware.org>
Sent: Thursday, October 15, 2020 6:54 AM
To: Soby Mathew <Soby.Mathew@arm.com>
Cc: nd <nd@arm.com>; tf-m@lists.trustedfirmware.org <tf-m@lists.trustedfirmware.org>
Subject: Re: [TF-M] Behavior difference in Crypto IPC vs Library modes

Hi Soby,

Thank you for that fix! It does indeed fix this particular issue when using IPC.

On another note, I’ve been running Musca-B1 and the results differ from what you sent out in the past for AN521. Specifically, Musca-B1 fails 219, 241, 242, and 243. Is this something you can have a look at on the Musca-B1 side?

With that said, I’ve been running on PSoC64 and I can reproduce the AN521 results. I needed the patch you provided below but I was still running into memory issues and I had to bump the following (both of them).

#define TFM_CRYPTO_IOVEC_BUFFER_SIZE (8120)

#define TFM_CRYPTO_ENGINE_BUF_SIZE (0x5040) /* >8KB for EC signing in attest */

If I do not bump these, I would see 239 to 244 fail. Might you have any comments on the larger size requirements for these? Possibly when running in IPC mode?

Thank you,

Ray

From: Soby Mathew <Soby.Mathew@arm.com>
Sent: Wednesday, October 14, 2020 8:52 AM
To: Raymond Ngun <Raymond.Ngun@cypress.com>
Cc: nd <nd@arm.com>; tf-m@lists.trustedfirmware.org
Subject: RE: Behavior difference in Crypto IPC vs Library modes

Hi Raymond,

After further analysis, it seems to me that having separate checks for Library mode and IPC mode is the easiest way to go. The current design was done in such a way that both Library and IPC mode can reuse the same crypto service API involving IOVECs. Any change to how the API is invoked from the tfm_crypto_call_sfn() will have ramifications for Library mode.

I have done a patch with different checks for IPC and Library mode here: https://review.trustedfirmware.org/c/TF-M/trusted-firmware-m/+/6432 . The patch relaxes the checks for IPC mode to allow empty buffers and hardens the checks for Library mode. Hopefully this should resolve the issue.

Best Regards

Soby Mathew

From: Soby Mathew <Soby.Mathew@arm.com>
Sent: 12 October 2020 17:17
To: Soby Mathew <Soby.Mathew@arm.com>; Raymond Ngun <Raymond.Ngun@cypress.com>
Cc: nd <nd@arm.com>; tf-m@lists.trustedfirmware.org
Subject: RE: Behavior difference in Crypto IPC vs Library modes

Hi Raymond,

As you pointed out, the difference in this case basically boils down to how the 2 models handles empty buffers. In the library mode, the empty buffers are passed down to the target API whereas the IPC mode optimizes the empty buffer from the IOVEC by reducing the buffer length. This results in different error codes in the 2 modes.

The sanity check of IOVEC in incoming sizes is needed and I less inclined to remove it or enhance it. The error code certainly seems to be one way to resolve this problem. The other option is to make the IPC mode IOVEC less aggressive in optimizing away zero buffers from IOVEC (Need more investigation) thus attaining parity with library mode.

Best Regards

Soby Mathew

From: TF-M <tf-m-bounces@lists.trustedfirmware.org> On Behalf Of Soby Mathew via TF-M
Sent: 12 October 2020 11:50
To: Raymond Ngun <Raymond.Ngun@cypress.com>
Cc: nd <nd@arm.com>; tf-m@lists.trustedfirmware.org
Subject: Re: [TF-M] Behavior difference in Crypto IPC vs Library modes

Hi Raymond

Thanks for the detailed report. This issue was reported here https://developer.trustedfirmware.org/T822 previously but I didn’t get time to look into it further due to other priorities. Your analysis seems right and I will look further into this.

Best Regards

Soby Mathew

From: TF-M <tf-m-bounces@lists.trustedfirmware.org> On Behalf Of Raymond Ngun via TF-M
Sent: 10 October 2020 00:59
To: tf-m@lists.trustedfirmware.org
Subject: [TF-M] Behavior difference in Crypto IPC vs Library modes

Hi all,

I’m seeking some assistance in determining the correct fix for a difference in behavior between IPC and Library modes that cause the Crypto PSA Arch Tests to fail when using IPC. Specifically, I’ve been testing on a PSoC64 for IPC mode and Musca-B1 for Library mode. The problem I am encountering is related to this check in crypto (e.g. crypto_aead.c in secure_fw/partitions/crypto).

if ( !((in_len == 2) || (in_len == 3)) || (out_len > 1)) {

return PSA_ERROR_CONNECTION_REFUSED;

}

This is true for direct function call since in_len and out_len are sizes of in_vec[] and out_vec[]. However, in library mode, in_len and out_len is not based on the size of in_vec[] and out_vec[] but based on the contents. Specifically, out_len is determined via the following in tfm_crypto_call_sfn().

/* Check the number of out_vec filled */

while ((out_len > 0) && (msg->out_size[out_len - 1] == 0)) {

out_len--;

}

From the above, if out_size (which is passed in by the user) is 0, the resultant out_len will be 0. The out_len is passed into the crypto function and PSA_ERROR_CONNECTION_REFUSED is returned due to the check above. PSA, on the other hand, expects PSA_ERROR_NOT_SUPPORTED to be returned. Btw, in_len suffers from the same issue.

I’m not sure if the check above is valid for IPC mode. I’ve removed the check temporarily to avoid the problem. However, if the check still makes sense, possibly it should return PSA_ERROR_NOT_SUPPORTED instead of PSA_ERROR_CONNECTION_REFUSED.

Thank you. I look forward to comments.

Ray

This message and any attachments may contain confidential information from Cypress or its subsidiaries. If it has been received in error, please advise the sender and immediately delete this message.