subsystem: settings: its: Add ITS backend #87778

seankyer · 2025-03-27T20:35:11Z

This PR adds an internal trusted storage (ITS) backend to settings subsystem, allowing users to store persistent data utilizing TF-M ITS transparently through the settings subsystem.

This allows for a non-secure app which may normally not have access to the Flash controller, for security reasons, to access persistent storage.

MaureenHelm · 2025-03-27T21:15:37Z

@de-nordic @tomi-font can you take a look?

tomi-font

The description doesn't seem to quite say what this achieves.
The way I understand this is, this is meant to redirect settings to TF-M's ITS.

What is the intent exactly behind that? Is it to somewhat transparently secure whatever is stored in settings?

seankyer · 2025-03-28T15:17:31Z

The description doesn't seem to quite say what this achieves. The way I understand this is, this is meant to redirect settings to TF-M's ITS.

What is the intent exactly behind that? Is it to somewhat transparently secure whatever is stored in settings?

Just updated the PR description. You are right. The main point is so that a Zephyr app running in non-secure domain can make use of the settings subsystem by using TF-M ITS to store potentially sensitive information (BLE Keys/Addresses/etc). By defining it as a settings backend, it integrates with Bluetooth subsystem seamlessly.

tomi-font · 2025-03-31T09:13:51Z

Just updated the PR description. You are right. The main point is so that a Zephyr app running in non-secure domain can make use of the settings subsystem by using TF-M ITS to store potentially sensitive information (BLE Keys/Addresses/etc). By defining it as a settings backend, it integrates with Bluetooth subsystem seamlessly.

Though why wouldn't code store potentially sensitive information through the PSA ITS/PS APIs? Now that those APIs are available also when not using TF-M (through the secure storage subsystem), it's not limited to TF-M-only use cases.
For example I know Bluetooth Mesh has started using persistent keys in the PSA Crypto API, which makes use of the ITS under the hood. (cc @alxelax)

One additional potential concern is if starting to store every setting in the ITS, it will add a significant delay to the calls, especially if using TF-M (partly because of its rather slow ITS implementation). And I'm wondering if that could cause problems because of added delays or also the fact that some implementations integrate with flash drivers to schedule NVM writes.

I mean I know it's not the new default and just adding one settings backend option, but trying to think about how usable that would be and if there is some other, better solution (e.g. making relevant code start using the PSA ITS/PS APIs).

seankyer · 2025-03-31T15:35:54Z

Just updated the PR description. You are right. The main point is so that a Zephyr app running in non-secure domain can make use of the settings subsystem by using TF-M ITS to store potentially sensitive information (BLE Keys/Addresses/etc). By defining it as a settings backend, it integrates with Bluetooth subsystem seamlessly.

Though why wouldn't code store potentially sensitive information through the PSA ITS/PS APIs? Now that those APIs are available also when not using TF-M (through the secure storage subsystem), it's not limited to TF-M-only use cases. For example I know Bluetooth Mesh has started using persistent keys in the PSA Crypto API, which makes use of the ITS under the hood. (cc @alxelax)

One additional potential concern is if starting to store every setting in the ITS, it will add a significant delay to the calls, especially if using TF-M (partly because of its rather slow ITS implementation). And I'm wondering if that could cause problems because of added delays or also the fact that some implementations integrate with flash drivers to schedule NVM writes.

I mean I know it's not the new default and just adding one settings backend option, but trying to think about how usable that would be and if there is some other, better solution (e.g. making relevant code start using the PSA ITS/PS APIs).

Yeah, this PR doesn't enforce that you must be running in non-secure context. A secure build could also make use of this backend. Although I use the example of non-secure and Bluetooth since that was the inspiration for creating this backend. Of course with this method, other OS-services that use settings subsystem would immediately work for a non-secure build.

Performance is a con of PSA ITS. I considered that the actual write to storage will take some time and could block some time sensitive operation (Bluetooth pairing for example). So, I added the SETTINGS_ITS_LAZY_PERSIST_DELAY_MS Kconfig. The setting will immediately be represented in memory, but the actual scheduled write to flash can be configured depending on the use case.

I supposed this is more about choice, as you said it's not a default option. Maybe I should add some documentation to the C file explaining the situation in which you'd use this backend? Or do you think it's required?

tomi-font · 2025-04-01T09:00:16Z

I see. I think you could elaborate in the help message of the SETTINGS_ITS Kconfig option rather. Citing use case(s), limitations... Documenting this, basically.
I'm not against this change. I'll just wait for other security folks to have a look to see if someone raises concerns before giving this a proper review.

seankyer · 2025-04-01T15:16:46Z

I see. I think you could elaborate in the help message of the SETTINGS_ITS Kconfig option rather. Citing use case(s), limitations... Documenting this, basically. I'm not against this change. I'll just wait for other security folks to have a look to see if someone raises concerns before giving this a proper review.

Thanks. I will update that Kconfig help message.

MaureenHelm · 2025-04-11T13:52:53Z

I'm not against this change. I'll just wait for other security folks to have a look to see if someone raises concerns before giving this a proper review.

@ceolin @d3zd3z @dleach02 can you take a look?

subsys/settings/src/settings_its.c

subsys/settings/Kconfig

alxelax · 2025-04-15T06:55:01Z

I guess this Settings backend will behave differently to the others since it has compile time configured available entities for writing.
Some Zephyr subsystems do not consider this limitation in their implementations. For example, Bluetooth Mesh stores replay protection cache in Settings. The cache size depends on network topology and not known in advance.

I'm sure this should be mentioned clearly in Zephyr's documentation. Until this PR all Settings backends were changeable without impact on workability of Settings' API users.
FYI @Laczen

IMHO, to hide keys it is better to use PSA API for key management with Persistent lifetime property (it is how PSA API has been designed to hide keys in persistent memory) rather to create custom solutions.

Laczen · 2025-04-15T08:05:54Z

I guess this Settings backend will behave differently to the others since it has compile time configured available entities for writing.

All settings backends have this limitations although it might not be directly visible. Settings on nvs only support 16383 settings to be stored (but normally the storage would be exhausted before that). Settings on zms only allows 16 entries with the same name hash value (a rogue device can easily use this to block further storage. Settings on file/fcb is limited by the available storage.

There is no real difference between the proposal and other settings backends.

Some Zephyr subsystems do not consider this limitation in their implementations. For example, Bluetooth Mesh stores >replay protection cache in Settings. The cache size depends on network topology and not known in advance.

The replay protection cache is IMHO a design flaw in btmesh because it supposes unlimited storage possibility. On small devices this unlimited storage possibility cannot be guaranteed.

I'm sure this should be mentioned clearly in Zephyr's documentation. Until this PR all Settings backends were changeable without impact on workability of Settings' API users. FYI @Laczen

The settings backend solutions in no way guarantee that everything that needs to be written can be written to it. When it is impossible to store something the settings subsystem returns that it cannot write the data. All subsystems should be able to handle this in a correct way.

alxelax · 2025-04-15T08:35:09Z

Some Zephyr subsystems do not consider this limitation in their implementations. For example, Bluetooth Mesh stores >replay protection cache in Settings. The cache size depends on network topology and not known in advance.

The replay protection cache is IMHO a design flaw in btmesh because it supposes unlimited storage possibility. On small devices this unlimited storage possibility cannot be guaranteed.

@Laczen, kind of arguable to me, if you chose wrong platform (low capacity) for complex network solution it is not problem of design. Naive to expect that 128kb flash device can manage hundreds node network as provisioner. Uncontrolled garbage collection and not ability to do it in background thread is a more serious settings design problem. It is not possible to solve by platform selection.
However, this is off-topic and not related to this PR. We can discuss it in separate topic if you wish.

de-nordic

Looks OK, the code is nicely commented.

@kartben Can you take a look at Kconfig helps, you are better at describing these things.

subsys/settings/Kconfig

subsys/settings/src/settings_its.c

Laczen · 2025-04-15T11:45:09Z

Some Zephyr subsystems do not consider this limitation in their implementations. For example, Bluetooth Mesh stores >replay protection cache in Settings. The cache size depends on network topology and not known in advance.

The replay protection cache is IMHO a design flaw in btmesh because it supposes unlimited storage possibility. On small devices this unlimited storage possibility cannot be guaranteed.

@Laczen, kind of arguable to me, if you chose wrong platform (low capacity) for complex network solution it is not problem of design. Naive to expect that 128kb flash device can manage hundreds node network as provisioner. Uncontrolled garbage collection and not ability to do it in background thread is a more serious settings design problem. It is not possible to solve by platform selection. However, this is off-topic and not related to this PR. We can discuss it in separate topic if you wish.

@alxelax I only commented because you mentioned me. Maybe it is off-topic, but I don't think so. You stated that the PR should not be added because of a builtin limitation that is not considered by the btmesh subsystem for the replay protection cache. But in this case the btmesh subsystem is making some expectation that is not provided by the settings subsystem at all. The problem is not the size of the network (this is easily solved), the problem is that if a device somehow can block storage (e.g. by exhausting the storage) the replay protection will fail. It is up to the btmesh subsystem to solve this problem.

Other items like timely response for critical settings and or uncontrolled garbage collections are also valid issues where in the current state subsystems have to take into account that the settings subsystem has its limitations. I am no longer working on any of these issues, sorry.

Copilot

Copilot reviewed 4 out of 9 changed files in this pull request and generated no comments.

Files not reviewed (5)

subsys/settings/Kconfig: Language not supported
subsys/settings/src/CMakeLists.txt: Language not supported
tests/subsys/settings/its/CMakeLists.txt: Language not supported
tests/subsys/settings/its/prj.conf: Language not supported
tests/subsys/settings/its/src/CMakeLists.txt: Language not supported

Comments suppressed due to low confidence (2)

subsys/settings/src/settings_its.c:55

The format specifiers for 'write_size' and 'remaining' use '%d' even though they are of type size_t. Consider using '%zu' for size_t to ensure accurate logging.

LOG_ERR("Error storing %d bytes of metadata! Bytes Remaining: %d, status: %d", write_size, remaining, status);

subsys/settings/src/settings_its.c:67

The use of '%d' for 'sizeof(entries)' (a size_t) and '%lld' for uid may lead to incorrect formatting. Consider using '%zu' for size_t and verifying the correct format for uid.

LOG_DBG("ITS entries stored successfully - bytes_saved: %d num_entries: %d max_uid: %lld", sizeof(entries), entries_count, uid);

MaureenHelm · 2025-06-25T13:25:56Z

@tomi-font @valeriosetti @ceolin can you take another look? would like to try to get this into 4.2

valeriosetti · 2025-06-25T22:06:55Z

subsys/settings/src/settings_its.c

+
+static int increment_uid(psa_storage_uid_t *uid)
+{
+	if (*uid < ZEPHYR_PSA_TFM_ITS_KEY_ID_RANGE_BEGIN ||


This can potentially be a dumb question, but where are ZEPHYR_PSA_TFM_ITS_KEY_ID_RANGE_BEGIN and ZEPHYR_PSA_TFM_ITS_KEY_ID_RANGE_SIZE defined? I can see other similar definitions in zephyr/include/zephyr/psa/key_ids.h, but not these ones.

Agreed. You missed adding the definitions to zephyr/psa/key_ids.h. And CI is green while it definitely should not. It would make sense to have at least one twister test scenario that covers this settings backend and that is built as part of the PR's CI to catch such things. Now we don't even know if this compiles at all.

sorry, my update to that file locally was excluded from the commit.

I needed to rebase, Twister in this PR should now be building the ITS test I added.
INFO - 292/942 max32657evkit/max32657/ns settings.its NOT RUN (build <zephyr>)

subsys/settings/src/settings_its.c

valeriosetti · 2025-06-25T22:31:25Z

subsys/settings/src/settings_its.c

+
+void worker_persist_entries_struct_fn(struct k_work *work)
+{
+	k_mutex_lock(&worker_mutex, K_FOREVER);


I don't think this usage of the mutex is enough. It does not prevent entries[] to be modified (calling settings_its_save()) while store_entries() is executed and that can cause corruption problems.
You can keep mutex lock/unlock in this function, but you also need to use the same mutex in settings_its_save().

tomi-font · 2025-06-26T09:59:36Z

Won't have the time to really review this before next Monday so I'm dismissing my review in case this gets approved and mergeable before then.

#87778 (comment)

valeriosetti · 2025-06-26T12:43:36Z

subsys/settings/Kconfig

+	  Note: Be aware of other subsystems using ITS implicitly (BLE Mesh, for example) which
+	  may conflict with this settings backend. ITS is intended to store sensitive data on-chip,
+	  such as cryptographic keys, credentials and configuration data and is not intended for
+	  storing large amounts of data.


IIUC as soon as you fix the ZEPHYR_PSA_TFM_ITS_KEY_ID_RANGE_[BEGIN|RANGE] definition this potential conflict with IDs associated to other subsystems should no more be a problem. Am I wrong? In case not, you can remove this warning.

Correct, I will reword this to be about the user' application specifically. I don't think we have a mechanism to throw a warning if the user's application itself uses a reserved UID range

seankyer · 2025-06-26T19:04:40Z

Hey @tomi-font with my latest push, the bot assigned you as the PR assignee.
Since you can't review until Monday would you mind assigning to someone else so we can hopefully get this in? Thanks

tomi-font · 2025-07-02T13:04:31Z

subsys/settings/Kconfig

+	  to use to store sensitive information.
+	  ITS backend can be used for storing sensitive information such as keys, passwords,
+	  or other secrets.
+	  The backend may be used for code running from secure or non-secure partition.


Code running on the secure side won't be using Zephyr and so cannot in any way use that? Unless I'm missing something.

Removed this line

tomi-font · 2025-07-02T13:05:30Z

subsys/settings/Kconfig

+	  Note: If using ITS as part of a user application, be aware of reserved UIDs defined
+	  in zephyr/include/zephyr/psa/key_ids.h to avoid UID collisions.


This is not something that should be told to the user. The user is only using the Settings API which has nothing to do with this. Using the proper key IDs is the responsibility of the implementation.

I meant to warn the user if they intended to use this Backend and also make use of ITS directly in their application (for some other arbitrary reason). But I will remove it, maybe it's irrelevant to add to this Kconfig.

tomi-font · 2025-07-02T13:07:46Z

include/zephyr/psa/key_ids.h

@@ -44,6 +44,10 @@ typedef uint32_t psa_key_id_t;
 #define ZEPHYR_PSA_WIFI_CREDENTIALS_KEY_ID_RANGE_BEGIN (psa_key_id_t)0x20010000
 #define ZEPHYR_PSA_WIFI_CREDENTIALS_KEY_ID_RANGE_SIZE  0x100 /* 256 */

+/** PSA key ID range to be used by TF-M ITS */
+#define ZEPHYR_PSA_TFM_ITS_KEY_ID_RANGE_BEGIN (psa_key_id_t)0x20010100
+#define ZEPHYR_PSA_TFM_ITS_KEY_ID_RANGE_SIZE  0x10000 /* 64 Ki */


No, this range is not for "TF-M ITS", it's for the TF-M ITS Settings backend. So I would rather call the macros ZEPHYR_PSA_SETTINGS_TFM_ITS_*.

tomi-font · 2025-07-02T13:09:07Z

subsys/settings/Kconfig

+config SETTINGS_TFM_ITS
+	bool "Internal Trusted Storage (ITS) settings backend"
+	depends on TFM_PARTITION_INTERNAL_TRUSTED_STORAGE
+	help


A note about the RAM consumption is missing: #87778 (comment)

I added it to the SETTINGS_TFM_ITS_NUM_ENTRIES Kconfig help section https://github.com/zephyrproject-rtos/zephyr/pull/87778/files/7a25bd6f81682de166513b2c9670fc5ab689a90f#diff-db8ba35bdefd7627bfa4c2a0d7d13c9d1f20787ddf7181333981f6c01a32cc34R247 should it be moved? Is the wording ok?

Add settings_its backend in order for a non-secure Bluetooth application to be able to save Bluetooth settings to persistent storage. Signed-off-by: Sean Kyer <Sean.Kyer@analog.com>

Add ITS tests for ITS settings backend. Part of enabling settings subsystem for non-secure builds. Signed-off-by: Sean Kyer <Sean.Kyer@analog.com>

Added the ability to remove settings when using the ITS backend settings subsystem. Signed-off-by: Sean Kyer <Sean.Kyer@analog.com>

sonarqubecloud · 2025-07-02T17:09:53Z

Quality Gate passed

Issues
2 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
22.2% Duplication on New Code

See analysis details on SonarQube Cloud

seankyer · 2025-07-02T17:48:23Z

There are max32657evkit/max32657/ns related Twister failures in CI, which shouldnt be linked to this PR. @ozersa

@tomi-font @valeriosetti in the meantime could you please revisit? Thanks

ozersa · 2025-07-02T19:14:03Z

There are max32657evkit/max32657/ns related Twister failures in CI, which shouldnt be linked to this PR. @ozersa

@tomi-font @valeriosetti in the meantime could you please revisit? Thanks

Possible fix here:

tomi-font · 2025-07-03T10:43:26Z

@tomi-font @valeriosetti in the meantime could you please revisit? Thanks

FYI @valeriosetti is away until July 14th.

github-actions bot added the area: Settings Settings subsystem label Mar 27, 2025

tomi-font previously requested changes Mar 28, 2025

View reviewed changes

tomi-font requested a review from ceolin March 28, 2025 07:49

seankyer requested a review from tomi-font March 28, 2025 15:48

tomi-font requested review from d3zd3z, dleach02, ithinuel, Vge0rge, frkv, valeriosetti and wearyzen April 1, 2025 06:25

seankyer force-pushed the feat/settings-its-backend branch 2 times, most recently from 138eda6 to f1b58e6 Compare April 1, 2025 15:54

valeriosetti requested changes Apr 14, 2025

View reviewed changes

subsys/settings/src/settings_its.c Show resolved Hide resolved

subsys/settings/Kconfig Outdated Show resolved Hide resolved

subsys/settings/Kconfig Outdated Show resolved Hide resolved

subsys/settings/Kconfig Outdated Show resolved Hide resolved

seankyer force-pushed the feat/settings-its-backend branch from f1b58e6 to 6a627a0 Compare April 14, 2025 15:16

seankyer requested a review from valeriosetti April 14, 2025 19:11

de-nordic reviewed Apr 15, 2025

View reviewed changes

kartben requested a review from Copilot April 15, 2025 12:03

Copilot AI reviewed Apr 15, 2025

View reviewed changes

de-nordic previously approved these changes Jun 25, 2025

View reviewed changes

valeriosetti requested changes Jun 25, 2025

View reviewed changes

valeriosetti reviewed Jun 26, 2025

View reviewed changes

seankyer dismissed de-nordic’s stale review via 94708eb June 26, 2025 17:20

seankyer force-pushed the feat/settings-its-backend branch from ebcbc00 to 94708eb Compare June 26, 2025 17:20

github-actions bot added platform: ADI Analog Devices, Inc. area: Secure Storage Secure Storage labels Jun 26, 2025

github-actions bot requested review from MaureenHelm and ozersa June 26, 2025 17:21

github-actions bot assigned tomi-font Jun 26, 2025

seankyer requested a review from valeriosetti June 26, 2025 17:22

seankyer mentioned this pull request Jun 26, 2025

Triage Permission Request #92267

Closed

seankyer force-pushed the feat/settings-its-backend branch from 94708eb to 7a25bd6 Compare June 27, 2025 17:11

tomi-font removed their assignment Jun 30, 2025

tomi-font requested changes Jul 2, 2025

View reviewed changes

seankyer force-pushed the feat/settings-its-backend branch from 7a25bd6 to b076f88 Compare July 2, 2025 15:26

github-actions bot assigned tomi-font Jul 2, 2025

seankyer added 2 commits July 2, 2025 08:53

subsystem: settings: Add ITS backend

2d69535

Add settings_its backend in order for a non-secure Bluetooth application to be able to save Bluetooth settings to persistent storage. Signed-off-by: Sean Kyer <Sean.Kyer@analog.com>

tests: subsystem: settings: Add ITS backend tests

c3ee626

Add ITS tests for ITS settings backend. Part of enabling settings subsystem for non-secure builds. Signed-off-by: Sean Kyer <Sean.Kyer@analog.com>

seankyer force-pushed the feat/settings-its-backend branch from b076f88 to 0176656 Compare July 2, 2025 15:53

subsystem: settings: Add setting deletion to ITS backend

fa7b4b4

Added the ability to remove settings when using the ITS backend settings subsystem. Signed-off-by: Sean Kyer <Sean.Kyer@analog.com>

seankyer force-pushed the feat/settings-its-backend branch from 0176656 to fa7b4b4 Compare July 2, 2025 16:44

		Note: If using ITS as part of a user application, be aware of reserved UIDs defined
		in zephyr/include/zephyr/psa/key_ids.h to avoid UID collisions.

subsystem: settings: its: Add ITS backend #87778

Are you sure you want to change the base?

subsystem: settings: its: Add ITS backend #87778

Conversation

seankyer commented Mar 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

MaureenHelm commented Mar 27, 2025

Uh oh!

tomi-font left a comment

Choose a reason for hiding this comment

Uh oh!

seankyer commented Mar 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tomi-font commented Mar 31, 2025

Uh oh!

seankyer commented Mar 31, 2025

Uh oh!

tomi-font commented Apr 1, 2025

Uh oh!

seankyer commented Apr 1, 2025

Uh oh!

MaureenHelm commented Apr 11, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

alxelax commented Apr 15, 2025

Uh oh!

Laczen commented Apr 15, 2025

Uh oh!

alxelax commented Apr 15, 2025

Uh oh!

de-nordic left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Laczen commented Apr 15, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

MaureenHelm commented Jun 25, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

seankyer Jun 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tomi-font commented Jun 26, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

seankyer commented Jun 26, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

seankyer commented Mar 27, 2025 •

edited

Loading

seankyer commented Mar 28, 2025 •

edited

Loading

seankyer Jun 26, 2025 •

edited

Loading