Software Update for the Internet of Things (SUIT) Manifest Extensions for Multiple Trust Domain
draft-ietf-suit-trust-domains-12

Thanks to Thomas Fossati, the IoT directorate reviewer for his telechat (and early) reviews:
https://datatracker.ietf.org/doc/review-ietf-suit-trust-domains-10-iotdir-telechat-fossati-2025-03-06/

Thank you for preparing this I-D. I do not see any transport-related concerns.

I do have some comments which ought to be helpful in preparing the next revision of this document:

1. Thanks to Thomas Fossati, for the IoT directorate review. Please consider the comments in: review-ietf-suit-trust-domains-10-iotdir-telechat-fossati-2025-03-06-00.

2. CDDL: It would be helpful to readers to define “CDDL” in the definitions of section 2. I think this should be supported by a normative reference.

3. Definitions: I think it would be helpful to also define TEE here as Trusted Execution Environment in Section 2. I see there are discrepancies in the different SUIT documents, is it possible to ensure that all documents have a common set of definitions - this could be achieved by 

4. RFC 1119: I’m quite a fan of RFC2119 requirements being clear when read as individual sentences - e.g., in the following I would suggest that you replace “This” by saying what is optionally to be implemented.
“This element is OPTIONAL for Recipients to implement.”

5. Table: It is helpful to add a caption to tables to describe the table contents (see Table 1).

6. Figure: I appreciated the figure at the end of section 4. CoSWID is not defined, please define in caption or definitions. This figure would benefit from a caption.

7. RFC 1119: I couldn’t work-out what was intended to be permitted by the following:
      “Commands defined outside of this document and
      [I-D.ietf-suit-manifest] MAY have additional restrictions.”
- Is this a “MAY” extend the command set - i.e. permitting extension in some way, if so please clarify this by other RFCs... or some other extension mechanism.
- If this simply saying there might be further restrictions, then I think it ought to be a little more explained what this impacts. 
I expect this is just a need for better wording.

8. Finally, I do have a concern that I would like to see addressed on the text around this:
“If a Recipient supports groups of interdependent Components (a Component Set), then it SHOULD verify “
- I appreciate the way that this I-D does already provide an explanation of the implication of not following all the other recommendations (SHOULDs). It would seem useful to also explain here what happens if an implementation does not verify this, or alternatively to motivate explicitly why it is good to verify.

# Gunter Van de Velde, RTG AD, comments for draft-ietf-suit-trust-domains-10

# The line numbers used are rendered from IETF idnits tool: https://author-tools.ietf.org/api/idnits?url=https://www.ietf.org/archive/id/draft-ietf-suit-trust-domains-10.txt

# for your convenience, please find some non-blocking COMMENTS

# I am not that familiar with IOT/SUIT technologies, and it was not an easy document for me to process. My review is rather generic and high level. If my comments refer to items obvious when familiar with the technology, then feel free to ignore.


# comments
# ========

101	   *  software Components from multiple software signing authorities.
103	   *  a mechanism to remove an unneeded Component

GV> idnit: Is there a reason why "Component" is written with a uppercase 'C'?

113	   Because of the more complex use cases that are typically targetted by
114	   devices implementing this specification, the applicable device class
115	   is typically Class 2+ and often isolation level Is8, for example Arm
116	   TrustZone for Cortex-M, as described in [I-D.ietf-iotops-7228bis]

GV> I looked up Arm in the 7228bis document and could not find it. Maybe an example that can be found in the document could be used instead? Class2+ is also not specified. I assume it reference to CLass2, 3, 4 etc... (higher as class 2), but i am not sure. Mostly this confusion is from not being overly familiar with IOT.
I am also not sure what Cortex-M or the trustzone refers towards?

118	   Dependency Manifests enable several additional use cases.  In

GV> What is a dependency manifest? This is the first time this term is used in the text.
I found that a manifest provides instructions and metadata necessary for a device to 
Verify the authenticity and integrity of an update, to determine whether the update is applicable, to retrieve the update payload (e.g., firmware image) and to install and activate the update in a secure manner.
I am tryng to understand what is a dependency manifest? Maybe it is specified later in the text and i did not come to it yet.

285	   *  All Dependency Manifests must be present before any Payload is
286	      fetched.

GV> I am confused what "must be present" exactly means. Does this mean that Dependency Manifest must be ready on the IOT for being consumed by an authority or  be read and available on the authority? (i fear i am lost on IOT procedures/terminology)

307	   In addition, when multiple Manifests are used for an Update, each
308	   Manifest's steps occur in a lockstep fashion; all Manifests have
309	   Dependency resolution performed before any Manifest performs a
310	   Payload fetch, etc.

GV> It it specified somewhere what lockstep fashion means in the context of SUIT? Can this reference be added?
I assume that "lockstep fashion" refers to the synchronized execution of update steps across multiple manifests. 

348	5.  Dependencies
349
350	   A Dependency is another SUIT_Envelope that describes additional
351	   Components.
352
353	   As described in Section 1, Dependencies enable several common use
354	   cases.

GV> This aligns with an observation i had earlier in this review. Maybe for clarification add a pointer to section 5 when introducing the term dependency manifest in the introduction section 1?

358	   This section augments the definitions in Required Checks
359	   (Section 6.2) of [I-D.ietf-suit-manifest].

GV> When reading the word "augment" then i understand that it refers to a language construct that allows you to insert new definitions into an existing model/procedure. Would it make sense to specify where the procedure documented in section 5.1 is inserted in (Section 6.2) of [I-D.ietf-suit-manifest]? I assume it is appended at the end? or is this inserted somewhere else?

424	   $$SUIT_Manifest_Extensions //=
425	       (suit-manifest-component-id => SUIT_Component_Identifier)

GV> Would it make sense to suggest that the syntax used here appears to be in CBOR Data Definition Language (CDDL)?

631	5.5.  Dependency Resolution
632
633	   The Dependency Resolution Command Sequence is a container for the
634	   Commands needed to acquire and process the Dependencies of the
635	   current Manifest.  All Dependency Manifests SHOULD be fetched before
636	   any Payload is fetched to ensure that all Manifests are available and
637	   authenticated before any of the (larger) Payloads are acquired.

GV> About the "SHOULD" mentioned here. What is the impact when the payload is fetched before all dependency manifests are fetched? If the procedure is broken in under that assumption, would it not be a "MUST" ?

Gunter Van de Velde
RTG Area Director

Brendan, 

The changes made in [1] address the comments raised in my previous ballot [2]. Thank you.

Cheers,
Med

[1] https://author-tools.ietf.org/iddiff?url1=draft-ietf-suit-trust-domains-10&url2=draft-ietf-suit-trust-domains-11&difftype=--hwdiff

[2] https://mailarchive.ietf.org/arch/msg/suit/6R9niNFWE1QfbEgg2QM1_E6fcCc/

Thank you to Tim Evens for the GENART review.

I support the DISCUSS positions of Mohamed Boucadair and Orie Steele.

** Section 2.  

-- This document already normatively cites draft-ietf-suit-manifest.  However, this document repeats many of the definitions here that are already present in draft-ietf-suit-manifest.  Why?

-- Why does this document provide alternative to draft-ietf-suit-manifest for identical terms.  For example:

(draft-ietf-suit-manifest) “   *  Payload: A piece of information to be delivered.  Typically       Firmware for the purposes of SUIT.”

(here) “    *  Payload: A piece of information to be delivered.  Typically Firmware/Software, configuration, or Resource data such as text or images.”

See also “Manifest, “Recipient”, and a number of others.

Are these definitional changes significant?

** Section 7
   This extension is backwards compatible when used with a Manifest
   Processor that supports the Update Procedure but = does not support
   the Staging Procedure and Installation Procedure:

What does the “=” mean here?

Thanks for addressing my comments in -11

Section 6:

        To avoid Dependency faults, a Manifest author MAY use explicit
        Dependencies where possible, or a Manifest processor MAY track
        references to loose Dependencies via reference counting in the
        same way as explicit Dependencies, as described in Section 5.6.5.

The construct "MAY ... where possible" sounds like that first MAY is really
a SHOULD? As in it is preferred over the second MAY "where possible"? And
in case the first MAY/SHOULD is not possible, it sounds like the second MAY
is really a MUST? Eg I don't think you can skip both MAYs and still be a
valid implementation?