Re: [Suit] manifest18 review

[Brendan Moran <[e-mail]>]

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=
[e-mail]> 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
> [e-mail]
> https://www.ietf.org/mailman/listinfo/suit
>