Re: [Suit] Some proposals for updates to the SUIT manifest specification

[Brendan Moran <[e-mail]>]

Hi Koen,

Thanks for taking time to assess the impact of these proposals. I haven’t dealt with the text section yet either!

More comments inline...

Best Regards,
Brendan

On 30 Jun 2020, at 13:20, Koen Zandberg <[e-mail]<mailto:[e-mail]>> wrote:

On 24 Jun 2020, at 12:28, Brendan Moran <[e-mail]<mailto:[e-mail]>> wrote:


Here is our proposal to correct the mismatch between the manifest and the text section:

SUIT_Text_Map = {
* SUIT_Component_Identifier => { SUIT_Text_Component_Keys },
SUIT_Text_Keys,
}
This will allow any set of components to be described.

If I understand correctly, this creates a map with mixed bstr/integer
keys. I agree with what you already mentioned, it's not intended for
constrained nodes, this should be fine.

Yes, the idea here was that it’s problematic to rely on the order being the same between the components listed in common and components listed in Text. Because of this, we can list them by component identifier. We could also use an array and have a key for the component ID, but this breaks the semantic of (int => text) pairs. Henk suggested that we could just use the component ID itself to act as the key for the map. This means that we don’t have to add additional hierarchy for, e.g. a list of components.

The other option would be a map of components with component index as key. This would produce less duplication of component IDs (though I don’t think we care about that) and fit the semantics used elsewhere.

I’m ambivalent about which of these to use, but I think we need one of the two. For clarity, I think we should choose one of the two following structures:

Option 1:
SUIT_Text_Map = {
* SUIT_Component_Identifier => { SUIT_Text_Component_Keys },
SUIT_Text_Keys,
}

Option 2:
SUIT_Text_Map = {
  ? suit-text-components => { * (uint => { SUIT_Text_Component_Keys } ) },
  SUIT_Text_Keys,
}

The tree traversals for extracting a list of components are much simpler in Option 2, but it introduces a non-text value for a numeric key, which I don’t like. I’m split. Maybe there’s a way to make one of these two cleaner.

To explain what I don’t like about Option 1, imagine that you extract the text section into a hash map. If you want to render a list of affected components, you have to find them all in the map first. That means you need to get a list of all keys and test the type of each one to see if it’s numeric (text) or an array of bstr (Component ID). This means that even finding out how many components there are is O(n). With the Option 2, finding all the components is O(1).

To be clear, I’m not saying that the processing complexity is a big concern to me, I’m just saying it’s a good illustration of what I dislike about Option 1. I’m not saying that me mildly disliking this is a deal-breaker. I’m just wondering if we can come up with something better.

Input welcome!

Remaining question from my
side, is `suit-text-version-dependencies` the human readable form of
`suit-parameter-version`? Noticing that the description of
`suit-text-version-dependencies` mentions that it is a list, does it
require a similar treatment as the component text?

Yes, I believe that’s correct. It should also be a list. Actually, I think that might be easier to solve. I would move the suit-text-version-dependencies key out of the manifest text, into component text. Now, components version dependencies can just be present in the component list, and the suit-text-version-dependencies does not need to be a list.


The second point is Dependency Components. This information element was introduced to help distinguish between components that are defined in the current manifest and those that are defined by its dependencies. This is a loose definition, however, and I don’t believe that it fits the current definition of the manifest. I believe we should remove Dependency Components. There are two consequences of this action:
...
No real opinion on this yet. What you say makes sense. Is there an
implementation around supporting multiple components? The parser in RIOT
supports only a single component. If there is no guarantee that the
dependency component information is correct then in my opinion it can be
removed from the spec.

I haven’t seen a multi-component implementation yet. It sounds like we’re all on the same page. I’ll remove the dependency components section.

Finally, I have a question regarding the fields wrapped in byte strings. Are the byte strings necessary? I’d like to hear from those who have implemented the specification. Are the byte strings helpful? Or are they pain points?

...
My opinion on each of the bstrs is:

1) We must keep the bstr wrappers around all envelope elements.
Yes please, this is one of the few places where they really help.
2) I think we should keep the bstr around structured parameters (Arrays/Maps). This is very useful for late parsing of parameters because they do not always live within the extent of the current manifest.
I also strongly agree here. Exactly what you say, allows for
significantly easier iterating over parameters with late parsing.

Sounds like we’re aligned here.

3) I’m not entirely sure we should keep the bstr wrapper around the common block, but I think it’s useful. It always lives within the extent of the current manifest, so overruns will be detected on the first pass by the parser, and the extent of the common block can be recorded prior to the execution of any command sequences.
4) I would not argue against removing the bstr wrappers around command sequences, provided that we register a tag for either the command sequence or the SUIT Digest.

I think the other option is that we could use separate keys for digest and for command sequence. What I don’t like about that option is that it’s not clear what a device should do if both are present and one of the goals of this specification is to eliminate that kind of uncertainty. That being the case, I think I would want a SUIT_Digest tag or a SUIT_Command_Sequence tag. Probably a Command Sequence tag, since that would make the manifest smaller when including digests.

5) I think we could safely remove the bstr wrappers around the elements inside of the common block, including the command sequence. This incurs a negligible execution time overhead when accessing some common elements, but produces manifests that are a few bytes smaller.
I think these last three can also be removed, but I'm not completely
confident yet on this. I think we've discussed this before, any CBOR
structure that is optional to parse by the parser must allow for
skipping it, and skipping a complex structured parameter is relative
costly in CBOR parsers. I think (for now) that anything that must be
parsed doesn't need to be bstring wrapped. This aligns with keeping the
bstr wrappers on the envelope elements, the authentication is handled by
a separate COSE library and can be skipped by the SUIT parser. With late
parameter parsing it is also essential that all parameter values are
simple CBOR structures, which is the case when maps/lists are bstr wrapped.
We would very much appreciate input on each of these three topics. Unless there are strong objections, we will modify the text section as described and remove the Dependency Components list. The status of the bstr wrappers is less clear, so we will wait for feedback.

I'll get back to you on this as soon as I have numbers for the last 3
bstr options.

I look forward to your feedback!


Koen
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.