Skip to content

Add support for an app_id#27

Closed
alistair23 wants to merge 3 commits intotock:masterfrom
alistair23:alistair/app_id
Closed

Add support for an app_id#27
alistair23 wants to merge 3 commits intotock:masterfrom
alistair23:alistair/app_id

Conversation

@alistair23
Copy link
Copy Markdown
Contributor

Add support for an app_id. This can be speicified via the command line
with the --app-id argument. If one isn't specified one will be generated
based on a hash of the name.

App IDs will eventually be used by the kernel to track apps across
updates, layout changes and name changes. They should be unique and not
change during the lifetime of an app.

Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
Add support for an app_id. This can be speicified via the command line
with the --app-id argument. If one isn't specified one will be generated
based on a hash of the name.

App IDs will eventually be used by the kernel to track apps across
updates, layout changes and name changes. They should be unique and not
change during the lifetime of an app.

Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
Comment thread src/header.rs
Comment on lines +217 to +228
hasher.update(self.package_name.clone());
}

// Generate an AppID from the package name
if app_id.is_none() {
let hash = hasher.finalize();
self.hdr_main.app_id = hash[0] as u32
| (hash[1] as u32) << 8
| (hash[2] as u32) << 16
| (hash[3] as u32) << 24;
} else {
self.hdr_main.app_id = app_id.unwrap();
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the package name does not exist this will use the default value inside the hasher, which could lead to two unnamed packages having the same AppId. The same problem would exist for two packages with the same name being flashed. This seems like it could be a pretty big problem if the kernel assumed that passed appids do not collide.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm... Good point.

The goal was so that casual users would generally end up with app-id's that are constant over updates and layout changes.

I could swap this to just be a random number, but then apps will never have the same ID unless the user manually specifies it. I'm not sure if that matters much though.

I'll look into this more

Copy link
Copy Markdown
Member

@lschuermann lschuermann Oct 23, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The goal was so that casual users would generally end up with app-id's that are constant over updates and layout changes.

I think that this is a large part of why we want to have application ids in the first place. I'd refrain from using something that cannot be user controlled. I don't think colliding application ids are an issue, as long as they do not replace the AppId found in the process struct (they serve different functions after all). See my comment for further explanation of my reasoning.

@alistair23 alistair23 mentioned this pull request Oct 23, 2020
2 tasks
@lschuermann
Copy link
Copy Markdown
Member

lschuermann commented Oct 23, 2020

tl;dr: I like this approach, we should refrain from using sections of the app in the hash which might change to allow for application id persistency.

I think that this discussion on the mailing list and this email relevant. In my opinion we should differentiate between

  • application ids
  • verifiable ids

where an application id is a small (native data-width) size id used throughout the kernel during runtime for efficiency reasons and a verifiable id is an identifier based on cryptographic processes.

I like that this introduces the notion of an application id or more specifically, a default format for those (after all, the application ids do not have to be based on the TBF header and can be set arbitrarily using a nonstandard method to load the apps, thereby can be based on some kind of verifiable id if required) .

We should in my opinion make sure that the application ids are dependent on a constant, user specified value (such as the package name, as you proposed). Using a hash of other parts of the application which are not user defined, but depend on the application's elf sections would mix the concepts of verifiable ids and application ids, and make any persistent resource allocation during updates etc. hard if not impossible.

As a conclusion I think that application ids do not necessarily have to be unique over processes on a kernel, as for that we have the (now quite confusingly named) AppId already (we should definitely also keep this, as it increases on an app restart and therefore tracks the process instance):
https://github.com/tock/tock/blob/07bd824508b420c0daa1133e2a3bfa2550718306/kernel/src/process.rs#L776-L780

Rather, the application id would be akin to the concept of namespaces under Linux, which in Tock controls resource allocation (for instance file ownership, in the context of a filesystem). Using this definition an empty package name might be a non-issue, since those processes would be running in the "default namespace".

If a user wants two identically named apps (or apps without a name) to not have the same application id, we could offer an option that takes an arbitrary 32-bit value and uses this instead.

Copy link
Copy Markdown
Contributor

@bradjc bradjc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Block because at minimum we can't change the TBF main field, we have to add a TLV.

I would like to see us move quickly on a unique, persistent ID for applications. However, I do not think that elf2tab should insert this because that has the implication that it will be a global identifier. Since elf2tab cannot enforce that the identifier will be globally unique, it can lead to confusion if a collision happens. I would rather the tool that loads applications be responsible for setting/checking the identifier.

@alistair23
Copy link
Copy Markdown
Contributor Author

I'm closing this as it seems no one is really on board with an AppiID in elf2tab.

@alistair23 alistair23 closed this Oct 26, 2020
bors Bot added a commit to tock/tock that referenced this pull request May 11, 2021
2172: TBF Header Permissions r=hudson-ayers a=alistair23

### Pull Request Overview

This PR is a second attempt at my [original `app_id` PR](tock/elf2tab#27).

This is similar to an old PR: #1300

This PR adds a permission section to the TBF header. This permission section will contain a list of syscalls that the app is allowed and going to perform.

This only allows apps to interact with approved capsules. There are cases where we only want a single app to use a capsule. Take for example a Root of Trust (RoT) that uses I2C to control other devices. In this case we only want 1 approved app to have that control, while other (possibly malicious) apps are blocked from accessing the hardware.

We also then have a 64-bit bit mask of the commands that are allowed. Currently some capsules have large command numbers (like 100, 200 ect) and they will need to be updated in order to work with a bit mask.

The options can also be auto generated for apps. I can currently auto-generate them based on the compiled code for libtock-rs.

### Security

From a thread model perspective this isn't ideal as we don't expect to always trust the TBF, see: https://github.com/tock/tock/blob/master/doc/threat_model/TBF_Headers.md

For secure applications though we can have the loader enforce/double check the permissions.

### Testing Strategy

~~I have tested this by adding support to Elf2tab and by auto-generating the list in libtock-rs.~~

~~See tock/elf2tab#28 for Elf2tab details.~~

### TODO or Help Wanted

A implementation in Tock after this is approved.

### Documentation Updated

- [X] Updated the relevant files in `/docs`, or no updates are required.

### Formatting

- [X] Ran `make prepush`.


Co-authored-by: Alistair Francis <alistair.francis@wdc.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants