Re: [Suit] manifest18 review

Thanks Brendan. This mail went into my spam folder, otherwise I would reply sooner.

  1.  An alternative is something like a "content" parameter which is used by both the "write" directive, and a "check content" directive. If the content parameter is written during the shared sequence, it can e.g. be written and checked during installation, and then checked again during the "validate" sequence.
  2.  The tag does indeed do this, but the tag is strictly speaking optional. This is one of my pet peeves after having designed a few different custom serialization formats: Always include a version number first. IMO adding a version number with a '0' key at the very beginning is a relatively cheap way to save future headache, and have simpler resolution of versions. That's my two cents, I'll leave it up to you.
  3.  "invoke" is completely fine with me.
  4.  Did you have concerns about the shared command sequence not having the same name as the common block? IMO this isn't actually all bad, since common block vs common sequence is also a sort of collision between slightly different concepts. That said, I also wouldn't mind if you wanted to rename the common block to "shared block".
  5.
  6.  Thanks, IMO this should have a short note in the text.
  7.  No, it's fine, but should probably be clarified in the text.

Again, thank you,
Øyvind

From: Brendan Moran <brendan.moran.ietf@gmail.com>
Sent: Thursday, August 11, 2022 23:24
To: Rønningstad, Øyvind <Oyvind.Ronningstad@nordicsemi.no>
Cc: suit@ietf.org
Subject: Re: [Suit] manifest18 review

Hi Øyvind,

Thank you for your review. I have started working on making some of the changes you've suggested. I do have a few questions:

1. The write directive. I'm not fundamentally opposed to this directive, but I do have some questions about the intended behaviour. Primarily, I'm concerned about the verification of data written in this way.

- When a component is written with the write directive, how is the write checked to ensure that it was completed correctly?
- When using a SUIT manifest for secure boot, how is the component verified an expected value?

Obviously when a component is smaller than a digest, a direct comparison is a good option, but this is not a feature of the SUIT manifest. So then, how should we proceed? Do we need a new command for verifying small components against raw data that's present in the manifest?

What happens if the component is written in a severable section? Then the data is lost when that section is severed. Do we need a rule that raw data must not be placed in a severable section because it then becomes unverifiable?

2. The problem with an envelope version is that it can't be trusted because it is not part of the signed container. I'm not sure how much of a problem this is. We could add a version but I'm not sure what it would tell you that the tag doesn't. The only time that the tag would need to be changed is if we change the delegation mechanism, the authorisation object or the manifest *before* the version number (which is element 1). In other words, as long as we don't change the delegation or authentication block (which is just a digest followed by a list of COSE objects anyway) we should be fine? Is there a scenario I haven't considered?

3. You're right that the two meanings of run have a namespace collision. We have a few words to choose from: run, execute, invoke. We use "invoke" as a catch-all for secure boot and secure program launch already. What would you think of replacing run(current) with invoke(current)?

4. I'm fine, conceptually, with renaming "common sequence" to "shared sequence," but this does come with the side-effect that the common block will now contain a shared command sequence instead of a common command sequence. I don't think that this is an issue and I do see the problem with lexical scanning of "common sequence" versus "command sequence."

5. false is not a valid value for IndexArg in the CDDL. Internally, a parser may use false for the Component Index when a Dependency Index is set, but this has no effect on the CDDL. You're right, this should be uint/[+uint]/true

6. I think that the restriction in 6.7 it only applies when Strict Order = false.

7. Try Each was intended as "try each until success or options exhausted" Do you think this needs a different name?

Best Regards,
Brendan

On Mon, Aug 8, 2022 at 1:23 PM Rønningstad, Øyvind <Oyvind.Ronningstad=40nordicsemi.no@dmarc.ietf.org<mailto:40nordicsemi.no@dmarc.ietf.org>> wrote:
Hi everyone, here is another list of questions, suggestions, and clarifications for the manifest.

  *   (I've mentioned this before, but can't recall that we have properly discussed it) I feel the spec is missing a "write" directive for writing very small values (e.g. 32-bit configuration values). The only way to do this now would be to add the 4 bytes as a payload (possibly integrated), and have a digest check in the manifest followed by a copy. This is obviously a lot of wasted bytes and cycles. Can we add a suit-directive-write directive with a bstr argument that will be written directly to the current component?
  *   I think the envelope should have a suit-envelope-version analogous to suit-manifest-version, since it is the top-level type, and therefore needs to be correctly decoded before the suit-manifest-version can be checked.
  *   In Table 1 "run" is described as "run(current)" and "run sequence" is described as "exec(arg)". I suggest renaming "run sequence" to "execute sequence" in the whole document to avoid confusing the two concepts (which are quite different). Another source of confusion is that "the run sequence" can apply to the "run" command sequence in the manifest.
  *   I suggest renaming "common sequence" to "shared sequence" since it is less similar-sounding to "command sequence", and arguably slightly more descriptive.
  *   Is "false" a valid value for IndexArg (6.5 seems to imply no)? If not, the CDDL should be changed from "IndexArg /= bool" to "IndexArg /= true".
  *   6.7 (The section on parallell processing) states "To isolate each sequence from each other sequence, each sequence MUST begin with a Set Component Index directive with the following exception: when the index is either True or an array of indices, the Set Component Index is implied. Any further Set Component Index directives MUST cause an Abort". Does this restriction on Set Component Index inside Run Sequence always apply, or only when Strict Order is False in the surrounding context?
  *   In directive-try-each, if one sequence succeeds, should the try-each end, or should all sequences in the try-each be executed regardless?

BR, Øyvind
_______________________________________________
Suit mailing list
Suit@ietf.org<mailto:Suit@ietf.org>
https://www.ietf.org/mailman/listinfo/suit<https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.ietf.org%2Fmailman%2Flistinfo%2Fsuit&data=05%7C01%7COyvind.Ronningstad%40nordicsemi.no%7C18de4c33b7dc4604728508da7bdfdcc3%7C28e5afa2bf6f419a8cf6b31c6e9e5e8d%7C0%7C0%7C637958498806505111%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C2000%7C%7C%7C&sdata=zoWjfmStxEtNTIqz5eVfP6skW5iaIfu8FgxPnxlRg0c%3D&reserved=0>