secure_storage: add a ZMS-based implementation of the ITS store module

[tomi-font]

See the commit messages.

A follow-up PR will introduce the (static) allocation of psa_storage_uid_t values for end users of the PSA Secure Storage API.

[Laczen]

@tomi-font,
a. do not use storage_partition, this is already used for other purposes and cannot be used by two systems at the same time,
b. zms, nvs, or settings are not really a good fit for secure storage as they use more flash pages (at least 2) and ensure persistence, something that might not be required for secure storage,
c. the construction of the zms uid is fragile and might break as a result of firmware updates,

[Laczen]

@tomi-font I would advise on the following alternative approach:

1. A config variable CONFIG_ITS_NUM_ASSETS is defined,
2. A config variable CONFIG_ITS_FIRST_ASSET_ID is defined,

The id's CONFIG_ITS_FIRST_ASSET_ID until CONFIG_ITS_FIRST_ASSET_ID + CONFIG_ITS_NUM_ASSETS - 1 are used to store metadata (psa_its_id + uid.... + flags + nonce + tag),
The id's CONFIG_ITS_FIRST_ASSET_ID + CONFIG_ITS_NUM_ASSETS until CONFIG_ITS_FIRST_ASSET_ID + 2 * CONFIG_ITS_NUM_ASSETS -1 are used to store the data,

In the saving process the data is first written, then the metadata. In the reading process the metadata id's are searched for the correct psa_its_id, ... , a "reread" can be performed if needed (depending on the flags), followed by the read/validation of the data. At start any data that does not have a corresponding (or invalid) metadata can be removed.

This approach is also applicable to nvs, allows simultaneous use of nvs/... for secure storage, settings and other data storage (e.g when CONFIG_SETTINGS_NVS is defined the CONFIG_ITS_FIRST_ASSET_ID could be calculated as 0x8000 - 2 * CONFIG_ITS_NUM_ASSETS).

[tomi-font]

a. do not use storage_partition, this is already used for other purposes and cannot be used by two systems at the same time

Good point. I was thinking about how to handle potential conflicts, but I think that I will just remove the possibility to use storage_partition.

b. zms, nvs, or settings are not really a good fit for secure storage as they use more flash pages (at least 2) and ensure persistence, something that might not be required for secure storage

This makes no sense.

c. the construction of the zms uid is fragile and might break as a result of firmware updates

No it's not. And related to that, I will address the topic of UID assignment (for secure storage) in a follow-up PR.

I would advise on the following alternative approach:
[...]

I don't want to reinvent the storage wheel, plus I don't see the advantage in what you're proposing. Might as well just use the settings-based implementation.
For what you're proposing, there would need to be a mechanism to allocate ID ranges to different users of a same partition, which we don't currently have.

[Laczen]

b. zms, nvs, or settings are not really a good fit for secure storage as they use more flash pages (at least 2) and ensure persistence, something that might not be required for secure storage

This makes no sense.

Ok if you think they are a good fit.

c. the construction of the zms uid is fragile and might break as a result of firmware updates

No it's not. And related to that, I will address the topic of UID assignment (for secure storage) in a follow-up PR.

No need to address this in a follow-up PR, do it now before it gets used.

I would advise on the following alternative approach:
[...]

I don't want to reinvent the storage wheel, plus I don't see the advantage in what you're proposing. Might as well just use the settings-based implementation. For what you're proposing, there would need to be a mechanism to allocate ID ranges to different users of a same partition, which we don't currently have.

There are already users that are using the NVS backend for both settings and data storage, the allocation of ID ranges could be formalized.

If on the other hand it is needed that the secure storage has its own backend (as implied by the PR), a slightly modified copy of the flash backend used in tf-m would be more appropriate.

If the settings-based implementation fits the need, why propose a different alternative?

[tomi-font]

No need to address this in a follow-up PR, do it now before it gets used.

What I am talking about is not specific to what is added in this PR. It's about the PSA Secure Storage API overall. And the rationale for not including it as part of this PR is to keep the scope of a PR focused on one topic.

There are already users that are using the NVS backend for both settings and data storage, the allocation of ID ranges could be formalized.

I agree that it could and even should be formalized.

If on the other hand it is needed that the secure storage has its own backend (as implied by the PR)

No, it's not needed. This PR is just to allow direct use of a partition for more control and efficiency of the storage. For the majority of cases it will actually still default to using settings because storage_partition will be defined but not secure_storage_partition.

[Laczen]

No, it's not needed. This PR is just to allow direct use of a partition for more control and efficiency of the storage. For the majority of cases it will actually still default to using settings because storage_partition will be defined but not secure_storage_partition.

If it is worth using a partition for more control and efficiency of storage it would be good to target as much users as possible. NVS is still much broader used (it is more efficient for most users) and has a large install base. It doesn't seem a good choice to leave these users without a possible more efficient storage solution.

Splitting up the storage into metadata (uid, caller uid, flags, (optional) nonce, (optional) tag) and the data itself would give the following advantages:

1. No limitation is build into the uid,
2. No limitation is build into the caller uid,
3. The stack usage would be reduced as it is no longer required to create the combination of metadata and data before writing to storage,
4. It can be used by both nvs and zms,

[rghaddab]

Thank you for adding ZMS backend to secure_storage.
Some few comments but all minors.
Also maybe add a separate commit where you clang-format old code to get rid of github warnings

[tomi-font]

Also maybe add a separate commit where you clang-format old code to get rid of github warnings

I'm not a fan of the suggestions. Plus they're just notices. (And have sparked some debate on Discord.) They can be hidden with the a key. 🙂

[rghaddab]

Only some minor comments that you are free to take into account or not

[rghaddab]

only for a certain type of flash (that requires erase)

[de-nordic]

only for a certain type of flash (that requires erase)

I would rather say "for devices that require erase". There are devices that have erase and have paging (only for that purpose) but you can just write them without explicitly erasing first and without loosing written data in process.

[tomi-font]

Rebased to fix a conflict in overlay-default_transform.conf (now overlay-transform_default.conf).

[tomi-font]

@Laczen Following the contribution process, I am dismissing your review as you have just nacked this PR on the premise that the implementation is not compliant with the specification, which is something that has already been discussed and approved in the security working group. Plus this subsystem is still new and experimental; improvements are to come which will make it more usable.
Adding the DNM label for one business day.

#81235 (comment)

[Laczen]

@tomi-font, I will be short on this:

1. The proposal will not allow zephyr to function as a key store for other devices, which is the reason a uint64 uid was created in the first place,
2. The proposal forces the use of zms which is suboptimal for most devices,
3. The proposal will force an extra 2 sectors for a solution using psa crypto and secure storage, effectively increasing the flash usage of psa crypto with 8kB in most applications (without taking into account the flash requirement for the zms system),
4. The only reason I can see for the proposed solution is to have faster access to secure storage information, but you might be surprised by the fact that this fast access can be quite slow when zms (or nvs which has the same properties is in a proces of garbage collection. Any subsystem that needs access to a key fast (because it needs to respond in a timely fashion) cannot count on the key access being fast.
5. A storage solution can sometimes be a pain, it lives independently of images and cannot be easily modified. Even when the system is called experimental this cannot be applied to something that is independent of an image.

The responsability of the addition of this solution is entirely yours. I have given my feedback on the proposal as well as a means to remove its shortcomings.

As a last remark I would advise you to modify the psa crypto test to also validate using a vendor key as this is the most basic requirement.