TRD: Tock Storage Permissions #4021

bradjc · 2024-06-06T17:08:22Z

Pull Request Overview

This pull request creates a TRD about permissions for persistent storage in Tock. Importantly, this document does not consider the type of storage (e.g., key-value, files, arbitrary buffers, etc.) or the storage mechanism (e.g., flash, FRAM, EEPROM, block storage, cloud, etc.). This document is only concerned with creating a global policy for storage access control that any persistent storage mechanism can use. This document also does not prescribe exactly how permissions are assigned to applications, but does offer some potential approaches.

Part of #3876

Motivation

I want to create this for three reasons:

To codify that ShortIds are what Tock uses to associate stored state with specific applications.
To clarify storage permissions generally and how they relate to the TBF header of the same name.
To provide a global policy for permissions that is consistent no matter the specifics of the physical data storage mechanism present on a board.

Implementation

At the time of opening this PR the kernel is not in compliance with this TRD. However, I believe the changes required are fairly small, as storage permissions are not widely used and the current API is similar to what is already in the kernel.

Changes from the kernel today

The initial draft of this TRD contains ~~two~~ one notable changes from how the kernel works today:

1. Applications now by default do not need explicit permission to read their own stored state. Even if an application can no longer write new data, it can still read its existing data. A storage permission implementation can support denying read/modify access to an application's own data, but there must be an explicit field or flag to denote this.
2. All stored data are marked with the application's ShortId instead of the write ID in the TBF header.

Rendered

Testing Strategy

Not needed for the initial TRD.

TODO or Help Wanted

RFC

Documentation Updated

Updated the relevant files in /docs, or no updates are required.

Formatting

Ran make prepush.

doc/reference/trd-storage-permissions.md

alistair23 · 2024-06-07T00:22:17Z

doc/reference/trd-storage-permissions.md

+2. The label stored with the persistent data when the data are written is the
+   application's short AppID.


This should use the writeID. That is why it exists and how everything has been designed.

A writeID gives us more flexibility in terms of say A/B updating applications or other more complex setups

I disagree. We need to unify write id and app id, and having two adds confusion.

The original design was added as an experimental feature, and appid was designed with exactly this case in mind (it just didn't exist yet so writeid was needed).

Making sure I understand correctly, this replaces what writeID currently allows for by saying that now:

We might have apps A, B, C

We could give A {read/append}, B {read/append/modify}, and C {read}

We could give all three apps access to the same storage

A record could be created by A or B, and could only be modified by B

Any of A, B, or C could determine who last wrote a record because on a read operation the shortId which last wrote the record will always be returned [and e.g., if A created a record, but B modified it, when A reads it again it will see that the record was written by B]

Any of A, B, or C could determine who last wrote a record because on a read operation the shortId which last wrote the record will always be returned [and e.g., if A created a record, but B modified it, when A reads it again it will see that the record was written by B]

I don't know if this is true.

Perhaps we should be explicit about what modify does with the storage id. I think I was thinking it does not change the stored id.

I guess the question maybe is the purpose of the ID.

Is the purpose to identify what app "owns" a storage region (implicitly saying that multiple apps can't share/co-own a region)?

Or, as in Alistair's use case, is it so that apps which do share some space can identify who last wrote the record?

Or, are both useful, and there should be two IDs (per record? high overhead? ...per region?)?

If using shortIDs don't you end up with data that isn't "owned" by any application?

Yes, but isn't that reasonable? If I replace Firefox with Chrome, it makes sense to me that whatever state Firefox stored is no longer used and isn't given to Chrome (but would be if I re-installed Firefox in the future). If I update Firefox 126 with Firefox 127, then it maintains access to any of its prior stored state (and would with ShortId).

Firefox and Chrome are two different applications, not an update. Plus that's a desktop system, not an embedded system.

Maybe a more concrete example will help. Say you have a FIDOv1 application. It works great and everyone is using it. It uses writeID and readID A.

You want to update to FIDOv2 support. To avoid breaking anything, FIDOv2 is a new application. So you flash FIDOv2 application with writeID and readID A. Now two applications are running.

For legacy FIDOv1 operations the FIDOv1 application handles it. For FIDOv2 ops the new FIDOv2 application handles it. But the data is stored in the same place/region/store (basically they share the data, the FIDOv1 application just never parses the new data as it doesn't understand it).

Eventually the FIDOv1 application is removed and the data is all migrated to the FIDOv2 app.

The key part is that the FIDOv1 application is never updated. Let's say due to compliance, code or testing reasons.

That above works fine with with this TRD.

The tricky part is what happens if you incrementally want to support FIDOv1 in the new FIDOv2 application. Or if you only have partial support. So you maybe enrol new FIDOv1 requests with the new application, but handle the challenges in the legacy one. You basically want to fallback to the legacy FIDOv1 application in some situations. Data can be written by the new application with the original writeID (A) to maintain compatibility.

Using a fixed ShortID, which has to be unique, backs you into a corner here.

I agree it isn't super common, and most use cases don't care. But for a small select few it matters. Especially if you are unable or unwilling to update existing applications. Which in embedded, certified systems can be hard

If I'm understanding correctly the use case is:

A new application (FIDOv2) needs to be able to write new data that a legacy application (FIDOv1) can access. It may not be possible or easy to update the storage permissions for the legacy application (FIDOv1).

Sharing a WriteID makes this trivial because the two separate applications effectively share all state.

I'm concerned about complicating the entire storage design in Tock because of this use case. But:

In Section 8 I outline a few examples of what different storage systems might exist. It seems to me there are generally going to be three classes:

State which is stored and retrieved based on some record-specific name. For example, using a filename, a key name (in a key-value pair), or a record name.

State which is stored and retrieved based on its use case or based on which application "owns" it (ie wrote it originally, or uses it exclusively). For example, configuration state, arbitrary per-application state (like appstate.rs), openthread's dataset, or LoRa connection parameters/keys (also essentially per-application state).

State which is in a shared log. For example, logging. (I'm not sure if there are more examples of this.)

I can imagine implementing the underlying storage for the FIDOv1/2 example with either options 1 or 2.

With a storage system of type 2, each app (FIDOv1 and FIDOv2) would have their own storage region they can use. They each access it the stored state in its entirety, and probably have some data structure in the stored state. In the upgrade scenario, to allow FIDOv2 to save new requests that the FIDOv1 app can use, FIDOv2 is installed with modify permissions for ShortId(FIDOv1). Since the FIDOv1 state already exists, FIDOv2 just needs to modify it.

Using a storage system of type 1 seems like where the issue of not sharing writeIDs becomes a challenge. Any new fido request handled by the new app must be saved as a new object with a unique name (hence it would have FIDOv2's ShortId as its storage identifier). But, even if FIDOv2 could write with FIDOv1's writeID, it would need to store with a name such that FIDOv1 would find it and be able to use it.

I'm trying to judge how likely it is that this type of backwards compatibility use case would use named storage (type 1) rather than per-app storage (type 2). It seems to me that it would be hard to implement this with type 1.

Type 2, on the other hand, seems like a natural fit, and would be easy to support with the permissions model. For example, imagine the storage provides each app with its own table in a database. FIDOv1 stores requests as rows in its table. FIDOv2, with modify access for ShortId(FIDOv1) can append rows to that table. FIDOv1 then will get the new data with its normal queries.

My question is, then, how important is supporting this use case on a type 1 storage, where the new app would be required to write completely new objects to the storage system? Is it reasonable to say: to enable this use case either use a type 2 storage or design the kernel to be able to modify storage permissions for legacy apps (ie giving FIDOv1 read access to ShortId(FIDOv2))?

Using a storage system of type 1 seems like where the issue of not sharing writeIDs becomes a challenge. Any new fido request handled by the new app must be saved as a new object with a unique name (hence it would have FIDOv2's ShortId as its storage identifier).

Exactly. And FIDO data would probably be stored in a key-value store (type 1 in your example).

The other point to remember is that when the FIDOv1 app was written, it wasn't written knowing this update would happen. So you can't plan ahead

But, even if FIDOv2 could write with FIDOv1's writeID, it would need to store with a name such that FIDOv1 would find it and be able to use it.

Yeah, but that would be pretty easy. The keys would just be a hash of the token used in the CTAP data. Both v1 an v2 could easily use the same scheme for the "legacy" FIDOv1 setup.

I'm trying to judge how likely it is that this type of backwards compatibility use case would use named storage (type 1) rather than per-app storage (type 2). It seems to me that it would be hard to implement this with type 1.

Which is what I'm worried about. I would expect FIDO data to be using named storage (Tock has a key-value store for this use case).

And this is just one example I could easily think of, I suspect there might be a few more complex cases we lock out using shortIDs for the write ID. That might be a trade off worth making, but we should at least list the cases we don't support and justify why we don't support them.

My question is, then, how important is supporting this use case on a type 1 storage, where the new app would be required to write completely new objects to the storage system? Is it reasonable to say: to enable this use case either use a type 2 storage or design the kernel to be able to modify storage permissions for legacy apps (ie giving FIDOv1 read access to ShortId(FIDOv2))?

I personally don't think so. But it's not the worst thing in the world to say that. But I do think that should be written down and documented as something we don't support, with justification on why we chose not to support it

Type 2, on the other hand, seems like a natural fit, and would be easy to support with the permissions model. For example, imagine the storage provides each app with its own table in a database. FIDOv1 stores requests as rows in its table. FIDOv2, with modify access for ShortId(FIDOv1) can append rows to that table. FIDOv1 then will get the new data with its normal queries.

Wouldn't the new data be written with the new FIDOv2 write/short ID? Adding something to a database feels like a write (new data) then an append (modify existing data).

I guess you could debate either way, maybe something else we should clarify?

doc/reference/trd-storage-permissions.md

Remove text about read/modify by default.

doc/reference/trd-storage-permissions.md

ppannuto · 2024-06-07T19:14:09Z

doc/reference/trd-storage-permissions.md

+2. The label stored with the persistent data when the data are written is the
+   application's short AppID.


Making sure I understand correctly, this replaces what writeID currently allows for by saying that now:

We might have apps A, B, C

We could give A {read/append}, B {read/append/modify}, and C {read}

We could give all three apps access to the same storage

A record could be created by A or B, and could only be modified by B

Any of A, B, or C could determine who last wrote a record because on a read operation the shortId which last wrote the record will always be returned [and e.g., if A created a record, but B modified it, when A reads it again it will see that the record was written by B]

doc/reference/trd-storage-permissions.md

ppannuto · 2024-06-07T19:18:18Z

doc/reference/trd-storage-permissions.md

+4. How permissions are mapped to applications must be customizable for different
+   Tock kernels.


What does this mean?

I would think we would, at least to start, want a more limited/specific set of ways to give apps storage permissions?

e.g., my instinct was that putting storage permissions in TBF headers would be sufficient?

This feels a bit like under-specifying a hard problem here—maybe it's clearer below?

This means that the design for storage permissions cannot be such that there is only one way to specify per-app permissions.

Using TBF header's doesn't work well because that allows applications to set their own permissions. Signing can help, but a kernel may want to support signed apps from untrusted developers.

I listed a couple ideas in the TRD. What is the benefit of trying to be prescriptive?

Maybe just delete this bullet here?

Using TBF header's doesn't work well because that allows applications to set their own permissions. Signing can help, but a kernel may want to support signed apps from untrusted developers.

The solution to this was have the board main() verify that applications have permitted headers on a case by case basis.

Maybe just delete this bullet here?

I think this is an important requirement. It basically motivates why we have a StoragePermissions trait rather than a struct. It makes explicit that the design we want is flexible (eg like the scheduler) rather than fixed (more like the grants implementation).

With that table design, if a new app (CCC) is deployed that wants to read from storage ID 2, how does the kernel know that is acceptable? Who "owns" storage ID 2? AAA1? AAA2?

I think company A "owns" storage IDs 1 and 2. So an app CCC from company C would not be able to read storage ID 2. Assuming the header tried to add read id 2 for CCC, the kernel storage permissions policy would deny that request.

Now, if the goal is to allow CCC to read A's "owned" storage ID 2, then I'm not sure how to do that in this model. If we go back a step or too, the kernel could maintain a different table:

AppID Trust Storage Permissions Header Acceptable WriteIDs

AAA* Yes 1,2

BBB* Yes 3,4

CCC* Yes 5,6

DDD* No 7,8

* No

WriteIDs are allocated per company in advance. Read/modify permissions are trusted if signed by a trusted company.

I'm lost as to what we are actually discussing here. This TRD does not prevent (nor mandate) a kernel from supporting a Storage Permission TBF header. The only restriction is the StorageID comes from AppID and not from any other source. If a kernel wants to maintain any of these hypothetical tables it can. The only difference between this hypothetical discussion and what is implementable per this TRD is that the kernel storage permissions policy may have to allocate additional read and modify permissions.

I'm responding to this statement from Alistair:

Using TBF header's doesn't work well because that allows applications to set their own permissions. Signing can help, but a kernel may want to support signed apps from untrusted developers.

The solution to this was have the board main() verify that applications have permitted headers on a case by case basis.

I simply don't see how that is possible in a way that doesn't do one of the following:

Make storage IDs effectively aliases for app IDs (in which case storage IDs seem redundant)

Substantially duplicate the app ID infrastructure (e.g. have two parallel cryptographic signing systems).

What signature chain?

You can sign the TBF object (header and binary) with a certificate. That certificate can be signed by other certificates creating a chain, which can be stored in the TBF footer.

The kernel can then include details on trusted rootCAs (a serial number to compare against would be enough) and compare against that to verify if an application is signed by a trusted party.

If you're referring to the signature chain for cryptographically-verified app IDs, then that would require the kernel to store a table mapping app IDs to the storage IDs those app IDs have permission to use. If that's the case, then why have these TBF headers at all?

I don't think the kernel would use appIDs for this. The main part is checking the rootCA that is used to sign the binary and TBF header.

The TBF headers can still reduce scope. So if company Foo creates the kernel, only apps signed by company Foo rootCA can access read/write IDs 1, 2 and 3.

Company Foo can then create multiple apps, each one maybe only using a single read/write ID for isolation. Third party apps can then using read/write ID 4.

I don't think the kernel would use appIDs for this. The main part is checking the rootCA that is used to sign the binary and TBF header.

If a kernel isn't using AppId (the system described in the appid trd), but is verifying signatures, at some point it is quite different from upstream tock. This storage TRD, at the end of the day, is just some text living on a server somewhere. It might be reasonable at that point to just ignore this TRD. This TRD is based on AppId being the method for cryptographically verifying applications.

I said I don't think the kernel will use app IDs to map storage IDs to permissions. App IDs can be used elsewhere

doc/reference/trd-storage-permissions.md

I don't think this is final by any means, but this describes how storage permission policies are set for each process.

To better help explain the storage permissions, we document how capsules would use the storage permissions. This might also serve as a guide for developers of storage abstractions.

Help explain how the storage permissions might be used in a couple use cases.

bradjc · 2024-06-10T16:38:04Z

I added more text to follow up on comments and to add more explanation. I want to say up front that this doesn't mean any of the design is set in stone. But it is likely helpful to keep some use cases in mind.

I tried to describe how a capsule author would use permissions, how permissions are assigned to processes, and some example storage abstractions and how they might use the permissions.

bradjc · 2024-06-10T22:18:39Z

doc/reference/trd-storage-permissions.md

+trait StoragePermissions {
+    /// Check if these storage permissions grant read access to the stored state
+    /// marked with identifier `stored_id`.
+    fn check_read_permission(&self, stored_id: u32) -> bool;
+
+    /// Check if these storage permissions grant modify access to the stored
+    /// state marked with identifier `stored_id`.
+    fn check_modify_permission(&self, stored_id: u32) -> bool;
+
+    /// Retrieve the identifier to use when storing state, if the application
+    /// has permission to write. Returns `None` if the application cannot write.
+    fn get_write_id(&self) -> Option<u32>;
+}


Unfortunately I can't get trait StoragePermissions to work. The issue is we want to be able to pass something (say a &'static dyn StoragePermissions) to capsules to use as the permissions for some storage operation. But doing that requires a static object somewhere for each process that implements the correct policy for that process. That becomes a pretty invasive change.

Instead I have gone back to StoragePermissions being a fixed object (actually an enum). It has many constructors to allow boards flexibility in how permissions are assigned, but the actually methods for how permissions are stored are somewhat fixed.

It is now implemented as an enum.

The policy is generally the same but the specific implementation is different.

bradjc · 2024-06-11T18:24:51Z

I have a branch with these changes that compiles and I updated the document to match the implementation.

doc/reference/trd-storage-permissions.md

kupiakos · 2024-06-14T16:56:29Z

doc/reference/trd-storage-permissions.md

+
+1. **Write**: The application can write data.
+1. **Read**: The application can read data.
+1. **Modify**: The application can modify existing data.


What about checking the presence of the data in the first place? Is that considered a read?

ppannuto · 2024-06-14T18:24:44Z

This proposal was discussed extensively on the core call this week.

Summarizing the major points of consensus:

The intent and design of AppId is to act as the central source of truth for application identity, and policies around identity.
Allowing flexibility in AppId::ShortId policy in turn means that the properties of uniqueness, consistency, and other concerns for ShortIds cannot be stated globally a priori for ‘all of Tock’ — this flexibility is intentional and valuable, but it is a bit challenging to understand this limitation in the current documentation of ShortId.
Ultimately, most if not all forms of a dedicated writeId would want most if not all of the properties that ShortIds can provide.
To that end, as an OS for resource constrained systems, duplicating identify and verification logic—especially any trying to provide strong cryptographic/security properties—seems undesirable. Indeed, it was less clear when there would be a system which would desire different properties for writeId and ShortId.
It was clear that certain use cases of writeId have expectations above and beyond those few guaranteed by the abstract definition of ShortId, however, boards/apps/users requiring these properties should enforce them as part of AppId::ShortId policy.
We would like thus like to start with the simpler model as-proposed in the current TRD. We believe this should cover use cases, however, should time and experience prove that a separate writeId or some other equivalent is necessary, a new / revised TRD will be welcome.

The proposal was subsequently voted upon by all present on the call, and caveated on small language tweaks (already merged) approved unanimously (with two abstentions).

alistair23 · 2024-06-16T22:38:42Z

Should we document the design considerations in the TRD at least?

I feel it's worth covering that certain use cases are unsupported and explaining why

ppannuto · 2024-06-18T20:29:55Z

When this came up before, I believe the consensus was that TRDs are the venue for how things should be done, not the home of the complete decision tree to get there. That's how I ended up with https://github.com/tock/tock/blob/master/doc/rfcs/2023-08-18--CommandZeroSemantics.md, since it was requested to pull that info out of the update to the TRD.

Could probably capture something similar here.

alexandruradovici · 2024-06-25T02:35:40Z

doc/reference/trd-storage-permissions.md

+
+/// Retrieve the identifier to use when storing state, if the application
+/// has permission to write. Returns `None` if the application cannot write.
+pub fn get_write_id(&self) -> Option<u32>;


I'm not sure if I correctly understand the meaning if store_id. Is this the actial short_id that stored the data?

doc: trd: storage permissions

fcb5c80

github-actions bot added the documentation label Jun 6, 2024

This was referenced Jun 6, 2024

RFC: Flash Storage with Isolation #3905

Closed

AppID Roadmap #3876

Closed

alistair23 suggested changes Jun 7, 2024

View reviewed changes

github-actions bot assigned lschuermann Jun 7, 2024

trd: storage: clarify permission types

cc30d2e

Remove text about read/modify by default.

ppannuto reviewed Jun 7, 2024

View reviewed changes

bradjc added 4 commits June 10, 2024 12:31

trd: storage: add link to short id

2a1eb32

trd: storage: add section on policy

0692a8a

I don't think this is final by any means, but this describes how storage permission policies are set for each process.

trd: storage: explain how capsules should use SP

dda945c

To better help explain the storage permissions, we document how capsules would use the storage permissions. This might also serve as a guide for developers of storage abstractions.

trd: storage: add storage examples and SP mappings

8ab88f6

Help explain how the storage permissions might be used in a couple use cases.

bradjc commented Jun 10, 2024

View reviewed changes

bradjc added 2 commits June 11, 2024 14:23

doc: trd: storage: update to StoragePermissions

18bce19

It is now implemented as an enum.

doc: trd: storage: update policy trait

c6900e2

The policy is generally the same but the specific implementation is different.

ppannuto reviewed Jun 14, 2024

View reviewed changes

doc/reference/trd-storage-permissions.md Show resolved Hide resolved

kupiakos reviewed Jun 14, 2024

View reviewed changes

Add explicit refernce to AppIds as policy

96f0403

ppannuto approved these changes Jun 14, 2024

View reviewed changes

ppannuto added this pull request to the merge queue Jun 14, 2024

Merged via the queue into master with commit 32ad86b Jun 14, 2024
18 checks passed

ppannuto deleted the trd-storage branch June 14, 2024 18:36

alexandruradovici reviewed Jun 25, 2024

View reviewed changes

		2. The label stored with the persistent data when the data are written is the
		application's short AppID.

		4. How permissions are mapped to applications must be customizable for different
		Tock kernels.

TRD: Tock Storage Permissions #4021

TRD: Tock Storage Permissions #4021

Uh oh!

Conversation

Uh oh!

Pull Request Overview

Motivation

Implementation

Changes from the kernel today

Testing Strategy

TODO or Help Wanted

Documentation Updated

Formatting

Uh oh!

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!

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!

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!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

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!

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!

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!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!