AppID: Restore `AppIdPolicy` implementations' ability to use the accepted credential, and allow credential checkers to pass data with accepted credentials

[bradjc]

Pull Request Overview

This PR enables two features for AppID:

1. Allows an AppID assignment policy to use the accepted credential when assigning IDs. This is important if the AppId is the public key used to verify a signature, for example. (This functionality used to exist and I removed it previously because of how process loading changed. Turns out we still need it.)

2. Helps developers use the accepted credential correctly when assigning an AppID. There are two issues/pitfalls that can arise when trying to use an accepted credential to assign an AppID.

   1. The checker, while verifying the credential, has some state about the verification process that is necessary for assigning the AppID. Some examples:
      • The checker maintains a list of valid public keys that can verify signatures. The checker knows which public key was used to verify the signature.
      • The checker knows if the app was signed by the kernel developer or a third party (or both).
      • The checker has multiple public keys per organization, and knows which one signed the app.
   2. TBF footers are not checked for integrity, and developers must not store any values in the footer that cannot be verified by the checker. Very little can be verified by the checker. If a public key is in the credential, the checker can validate that that public key matches the signature. But any other data cannot be verified and could be modified before the app is loaded on a board.

   To help with these issues, this PR also includes a usize that the checker can store with the accepted credential to provide information about the accepted credential that the AppID assignment policy can use.

   For issue 1: this would allow the process checker to keep track of some metadata about why it accepted the credential. For example it might store the index of the public key that verified the signature. Or it might store the upper 16 bits of the ShortId for the organization that signed the app. The value is opaque, and different policies/implementations can use it differently. The Compress trait (ie assigning ShortId) is not asynchronous. So from a software engineering viewpoint, whatever state the AppId assignment policy needs must be fetched in advance, and the policy can not re-run any verification steps. Therefore, the checker needs some way to communicate with the AppId assigner if the assigner is going to use any outcome of the verification process in its AppId assignment.

   For issue 2: this gives developers some way to attach state to an accepted credential without including that state in the credential itself. This isn't providing a correctness guarantee, but it is providing an alternative to what seems intuitively valid: just include it in the credential. Otherwise, developers that want to use different credentials to assign different AppIds can't (or have to build some sort of workaround). 

PART 1

First 4 commits.

As part of the refactor in #3849, I removed the ability to retrieve the accepted credential when comparing AppIds and setting ShortIds. This was a byproduct of not actually creating a full-fledged ProcessStandard object before performing those operations. We no longer needed to mark the process as having valid credentials.

Unfortunately, that change made it impossible to 1) assign ShortIds based on the accepted credential, 2) define an AppId based on an accepted credential, and 3) assign fixed ShortIds to credentialed processes and locally unique ShortIds to non-credentialed processes.

This PR restores that functionality.

PART 2

While thinking through if supporting get_credential() is even a good idea, I'm concerned it has a potential issue. The issue is: credentials are stored outside of the integrity region, so using them to make any decisions about an application must be done very carefully. While I think ultimately we do want the API to expose credentials to the appid assigner (and other modules), some care is required.

For example, say I want to assign ShortIds based on which company signed an app. The credential checker can validate the signature and decide whether to accept the signature or not. But then it is up to the appid assigner to actually create a ShortId. How does the AppId assigner know which company created the approved signature, and therefore which ShortId to assign? Right now I think there are two ways:

1. The credential itself includes some company identifier. When the assigner assigns the ShortId it checks the company listed in the credential and uses that to determine the correct ShortId.
2. The checker stores some state (somehow) that maps the signature to a company (for each application).

I don't think 2 is realistic. That would be possible, but would be a lot of overhead for a conceptually simple operation.

Option 1 does work, but has two drawbacks. First is the potential issue I mentioned about. The system must be very careful to not trust what is in the credential (ie in the TBF footer). For example, an attacker could use one company's signature for an app but change the metadata about which company signed it. The credential checker must check for this and reject the credential, but this would be easy to miss. The second issue is it requires some sort of registry at signing time that all signers agree to use to identify themselves.

Given these two options and their drawbacks, the second half of this PR allows a credential checker to return an opaque usize with any accepted credential. This usize can then convey information between the credential checker and the app id assigner on an application-by-application basis. This would simplify allowing the kernel to maintain some sort of key library where the accepted signature is matched to a pub/pri key pair and identified by some value. Or even the usize could be used to communication the intended ShortId directly.

Testing Strategy

Only compile tested, but the code is pretty straightforward.

TODO or Help Wanted

1. I'm not in love with this implementation for part 1. It turns out to be very easy to implement this by adding an OptionalCell to ProcessBinary which holds the credential if one was accepted. In practice, everywhere the credential is needed (ie creating a shortid, creating a ProcessStandard, etc.) we are already passing a ProcessBinary. But, does a ProcessBinary logically have an optional approved credential? Maybe? ProcessBinary was really intended to represent static state stored in flash.

   Separating the two is certainly possible, but it adds a bunch of boilerplate code (ie creating a new type that holds both a ProcessBinary and an Option<Credential>, and passing that around instead) to get to virtually the same exact place. Is that worth it? I'm not convinced. I don't think the current implementation is wrong, and it saves a bit of extra overhead in terms of code maintenance.

   Status: this implementation might be ok.

2. Change the optionalcell in process binary to contain a struct.

Documentation Updated

• [x] Updated the relevant files in /docs, or no updates are required.

Formatting

• [x] Ran make prepush.

[phil-levis]

"The system must be very careful to not trust what is in the credential (ie in the TBF footer). For example, an attacker could use one company's signature for an app but change the metadata about which company signed it. "

A proper credential has integrity, for the reasons you note. The signature binds to a signing identity; you should not be able to change which company signed it. Can you walk through a more specific example?

[bradjc]

I can try. In this scenario there are two trusted signers, CompanyA and CompanyB. The kernel runs apps from both of them, but assigns a different range of shortids to each company.

If we (just for ease of illustration) assume a credential is a JSON blob that looks like:

{
"signer": "CompanyA",
"signature": "ae890b34..."
}

Let's say that is a valid credential (ie, company A did in fact sign and that signature is correct).

I can then create a new credential that looks like:

{
"signer": "CompanyB",
"signature": "ae890b34..."
}

and replace the valid credential with this one in the TBF footer.

Now, a credential verifier in the kernel, if it only checks the signature, will see that the signature is valid. The credential checker will also see that CompanyB is a valid signer. However, it also must explicitly check that the signature was generated by CompanyB. It wasn't so that check will fail. It's probably reasonable for the checker to reject this credential (even though the signature is valid and from a valid signer).

A proper system with a proper implementation shouldn't have any issue. But, this is somewhat subtle, and leaves room for mistakes.

Also, providing the credential to the AppIdAssigner somewhat invites this kind of implementation because it is really easy to think something along the lines of "oh, I can just put the shortid in the credential". Or, "I can look at the credential for the signing company". That second one is possible to do correctly, but the credential checker has to be implemented in a specific way.

This is also based on the realities of the Tock use case. Meaning, the AppIdAssigner is almost certainly not going to duplicate the effort of being able to verify anything. So it will have to trust the checker policy implementation.

[alistair23]

How does the AppId assigner know which company created the approved signature, and therefore which ShortId to assign? Right now I think there are two ways:

It isn't just which company. A single company could have multiple certificates. I feel like we really need the entire certificate chain

Now, a credential verifier in the kernel, if it only checks the signature, will see that the signature is valid. The credential checker will also see that CompanyB is a valid signer. However, it also must explicitly check that the signature was generated by CompanyB. It wasn't so that check will fail. It's probably reasonable for the checker to reject this credential (even though the signature is valid and from a valid signer).

The signature isn't valid though, it isn't signed by the cert chain in the footer. I agree Crypto is really hard and easy to make mistakes, but these checks will have to be done otherwise the signatures are useless.

Also, providing the credential to the AppIdAssigner somewhat invites this kind of implementation because it is really easy to think something along the lines of "oh, I can just put the shortid in the credential". Or, "I can look at the credential for the signing company". That second one is possible to do correctly, but the credential checker has to be implemented in a specific way.

I assume AppIdAssigner is the Compress trait. At this point hasn't AppCredentialsPolicy checked the credentials and we know they are valid (unless they are changed in flash while running I guess)? Isn't that the idea of the AppCredentialsPolicy?

[bradjc]

A single company could have multiple certificates.

I'm using "company" as short hand. You can replace that with "signing entity" if you want.

The signature isn't valid though, it isn't signed by the cert chain in the footer.

I would encourage you to read through the AppID TRD. The mechanism is flexible by design. We can't unilaterally say that a particular signature is not valid. Also, there is no defined or required cert chain.

I assume AppIdAssigner is the Compress trait. At this point hasn't AppCredentialsPolicy checked the credentials and we know they are valid (unless they are changed in flash while running I guess)? Isn't that the idea of the AppCredentialsPolicy?

This is exactly what this PR is about. The checker runs, and the AppIdPolicy (sorry, slightly wrong name I was using) only gets 1 bit of information (accept or not) from the checker. I don't think that is enough.

[alistair23]

I would encourage you to read through the AppID TRD. The mechanism is flexible by design. We can't unilaterally say that a particular signature is not valid. Also, there is no defined or required cert chain.

If an app is signed by a different TbfFooterV2Credentials then the one supplied it isn't validly signed. At least in any setup where you are checking signatures (obviously AppCredentialsPolicy can just return true, but it's still not a valid signature it's just not checked).

This is exactly what this PR is about. The checker runs, and the AppIdPolicy (sorry, slightly wrong name I was using) only gets 1 bit of information (accept or not) from the checker. I don't think that is enough.

I agree with part 1. That's an easy one

I'm not convinced on the second part. The justification in the PR doesn't seem clear

[bradjc]

Allowing the checker to explicitly communicate with the appid assigner helps with implementing complex appid assignment policies.

Today, Compress only gets the process binary and (in part 1 of this PR) the accepted credential. The credential might only look like:

{
  "signing_pub_key": "8109afe45...",
  "signature": "090b4ea..."
}

The appid assignment policy might want to do quite a bit. For example, assign certain shortids based on which of the company's keys was used for the signature, use a shortid TBF header or not (or for only some apps), use a fixed list for some apps. Modifying the TBF header may not be possible (particularly if the app is already signed). The checker must ensure the signature is valid, and in the process of that operation, may want to save some state about why the credential it accepted was accepted, as that might be useful for the appid assigner. This is a mechanism to support that. This would help avoid re-doing work and/or help simplify the in-kernel implementation of the appid assigner.

[alistair23]

I understand it's useful to share some of the state, but weren't they separated in the first place for a reason?

Also, why a usize? It seems very C-like to pass a arbitrary value around like that. Espicially one that changes bit widths if Tock is ported to a 64-bit platform.

Also, won't this break if someone swaps the AppCredentialsPolicy implementation but not the Compress implementation? It seems to break all of the type guarantees as well as the meaning of the value changes based on the implementation

[bradjc]

Also, why a usize? It seems very C-like to pass a arbitrary value around like that. Espicially one that changes bit widths if Tock is ported to a 64-bit platform.

I'm not sure what else to use.

Also, won't this break if someone swaps the AppCredentialsPolicy implementation but not the Compress implementation? It seems to break all of the type guarantees as well as the meaning of the value changes based on the implementation

Yes. But potentially so would any security guarantees your kernel has (had).

[bradjc]

I added the struct.

Also, how big of a deal is using the nonzerousize? I switched back to a usize because not being able to use 0 seems like a fairly bit usability hit (for example to identify public key 0).

[lschuermann]

Also, how big of a deal is using the nonzerousize? I switched back to a usize because not being able to use 0 seems like a fairly bit usability hit (for example to identify public key 0).

Without actually checking it, it might not matter in this enum, as Rust may be able to niche-fill the Accept(None) and Accept(Some(_)) variants into the enum discriminant itself. However, were you to copy / move the Option<usize> outside of the enum, it would for sure occupy two words of memory instead of one.