[Suit] Mohamed Boucadair's Discuss on draft-ietf-suit-trust-domains-10: (with DISCUSS and COMMENT)

[Mohamed Boucadair via Datatracker <[e-mail]>]

Mohamed Boucadair has entered the following ballot position for
draft-ietf-suit-trust-domains-10: Discuss

When responding, please keep the subject line intact and reply to all
email addresses included in the To and CC lines. (Feel free to cut this
introductory paragraph, however.)


Please refer to https://www.ietf.org/about/groups/iesg/statements/handling-ballot-positions/ 
for more information about how to handle DISCUSS and COMMENT positions.


The document, along with other ballot positions, can be found here:
https://datatracker.ietf.org/doc/draft-ietf-suit-trust-domains/



----------------------------------------------------------------------
DISCUSS:
----------------------------------------------------------------------

Hi Brendan and Ken,

Thank you of the effort put into this document.

This review takes into account RFC 9019, RFC 9124, and draft-ietf-suit-manifest.

I have a set of DISCUSS and COMMENT items. The list may seem long but that is a
sign that I enjoyed reading. Let's walk through and hope to converge before the
telechat.

Note that there are many nits in the document, I flagged some of them in my
marked version, which I have online. I can share the link offline with the
authors upon request.

# Meta Comments

## Lack of overview of blocking issues since the document was WGLC in 2023

I found that a WGLC was issued in 2023 against -02 but there is no mention in
the write-up what were the issues that hindered the publication of the document
since then. I also failed to find other records, including in the history tab
of the document Datatracker.

I checked  diff vs -02, but it would be good if we had a summary of the changes
summarized “somewhere” (the side-to-side diff is verbose as most of the changes
are minor ones): I tagged at least very few changes to the CDDL, removal of the
delegation part, and more examples.

Also checked the meeting presentation, but again no hint to understand the
delay/blocking points.

Maybe this is due to my poor search skills, I didn’t find a record of a second
WGLC or a conclusion of the first WGLC (-02).

## Need for helpers for those who will deploy

Approaching this from OPS, and how this can be put into effect, a short
overview of the various pieces being grafted would be helpful. I’m not asking
to have that in this document (but would happy if you do so), but I invite the
WG to consider having such an overview. If you already have, please share it
LOUDLY. Thank you.

Please note that I’m not looking for general matters such as 9019, but how the
various specs are glued together.

## Lack for friendly representation of CDDL structures

--This is not specific to this document, but this document exacerbate the need--

It is not straightforward to pragmatically append the CDDL augmentation to what
was defined in the base manifest spec and display graphically the augmentations.

It would be useful to have for CDDL a representation similar to RFC8340 for
abstract YANG data structure (RFC8791). Hope this will inspire some :-)

## Document Flow

CURRENT:
  5.4.1.  Multiple Manifest Processors

I feel like some of the text in this section should be provided early in the
document to help readers easily digest. Please think about it. Thanks.

# DISCUSS

## Link with the IM/Compatibility (RFC9124)

I see this spec as mainly targeting REQ.USE.MFST.COMPONENT from RFC9124. I
failed to see how the current spec satisfies the following:

   Satisfies:  USER_STORY.OVERRIDE (Section 4.4.3), USER_STORY.COMPONENT
      (Section 4.4.4)

## Applicability: RFC7228/RFC7228bs is normative

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

Should I-D.ietf-iotops-7228bis be normative as this defines the devices to
which the spec is applicable?

BTW, please fix some nits:

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

## Consistency with the base manifest

CURRENT:
   Steps 3 and 5 are added to the expected installation workflow of a
   Recipient:

   1.  Verify the signature of the Manifest.

   2.  Verify the applicability of the Manifest.

   3.  Resolve Dependencies.

   4.  Fetch Payload(s).

   5.  Verify Candidate.

   6.  Install Payload(s).

Putting aside steps 3/5, the list does not mirror all the items in 4.2 of the
manifest. I’d like to check this is done on purpose and understand why.

As I’m there, is there always one “candidate”? I suspect that I understand that
the sequential flow will lead to a candidate, but I’m not sure if there are
cases where we might have conditional parts of the code and that these
conditional parts are handled as single/distinct candidates.

## Amend base manifest spec

(1) CURRENT:
   The new metadata structure is shown below.

   +-------------------------+
   | Envelope                |
   +-------------------------+
   | Authentication Block    |
   | Manifest           --------------> +------------------------------+
   | Severable Elements      |          | Manifest                     |
   | Human-Readable Text     |          +------------------------------+
   | CoSWID                  |          | Structure Version            |
   | Integrated Dependencies |          | Sequence Number              |
   | Integrated Payloads     |          | Reference to Full Manifest   |
   +-------------------------+    +------ Common Structure             |
                                  | +---- Command Sequences            |
   +-------------------------+    | |   | Digests of Envelope Elements |
   | Common Structure        | <--+ |   +------------------------------+
   +-------------------------+      |
   | Dependency Indices      |      +-> +-----------------------+
   | Component IDs           |          | Command Sequence      |
   | Common Command Sequence ---------> +-----------------------+
   +-------------------------+          | List of (pairs of (  |
                                        |   * command code      |
                                        |   * argument /        |
                                        |      reporting policy |
                                        | ))                    |
                                        +-----------------------+

Do we need to tag this as updating the manifest as it amends it?

As I’m there:

  * please say this is an update of the figure in Section 4.2 if the manifest
  spec.

  * CoSWID: Where is this defined?

(2) The same DISCUSS is also triggered by the following:

CURRENT:

5.3.  Changes to Abstract Machine Description

   This section augments the Abstract Machine Description (Section 6.4)
   in [I-D.ietf-suit-manifest].

## CDDL is Normative

CURRENT:
   The following CDDL describes the Manifest Component ID:

RFC 8610  should be listed normative.

## CDDL Consistency

CURRENT:
   The Components specified by SUIT_Dependency will contain a Manifest
   Envelope that describes a Dependency of the current Manifest.  The
   Manifest is identified, but the Recipient should expect an Envelope
   when it acquires the Dependency.  This is because the Manifest is the
   one invariant element of the Envelope, where other elements may
   change by countersigning, adding authentication blocks, or severing
   elements.

   When executing suit-condition-image-match over a Component that is
   designated in SUIT_Dependency, the digest MUST be computed over just

I failed to find I don’t see SUIT_Dependency in the CDDL snippets.

Please check and let me know if I missed something or whether a fix is needed.
Thanks.

## Inappropriate use of the normative language

The normative language use in the document should be checked. I flagged at
least the following ones

(1)

CURRENT:
       Commands defined outside of this document and
      [I-D.ietf-suit-manifest] MAY have additional restrictions.

Consider: “Additional restrictions may be added by future commands”.

(2)

CURRENT:
      For example, every Dependency's Dependency Resolution
      step MUST be executed before any dependent's Payload fetch step.

s/MUST/must as this is an example

(3)

CURRENT:
   *  When a Manifest Processor supports multiple independent
      Components, they MAY have shared Dependencies.

Putting aside an apparent inconsistency between “independent” and “have
dependencies”, this sentence can be reworded to be affirmative (s/MAY/may)

(4)

CURRENT:
   Because the Author and the Distribution System have different roles
   and MAY be separate entities, it is highly RECOMMENDED to leverage
   permissions (see Section 9 of [I-D.ietf-suit-manifest]).  For
   example, The Device can protect itself from attacker who breaches the

The first MAY is weird (may, would be just fine) while “highly” in the second
part does not change the level of requirement :-) Consider delete that mention.

BTW, s/example, The Device/example, the Device

## Lack of elaboration of specific security considerations

I was expecting at least a reminder of cons specific to the multi trust domain
case. Adding specific pointers where this is discussed in 9124/9019 would be a
minimum.


----------------------------------------------------------------------
COMMENT:
----------------------------------------------------------------------

# Title

OLD: SUIT Manifest Extensions for Multiple Trust Domains
NEW: Software Update for the Internet of Things (SUIT) Manifest Extensions for
Multiple Trust Domains

# Abstract

Consider swapping the two sentence for better flow

NEW:
   A device has
   more than one trust domain when it enables delegation of different
   rights to mutually distrusting entities for use for different
   purposes or Components in the context of firmware or software update.   This
   specification describes extensions to the Software Update for the Internet
   of Things (SUIT) Manifest format for use in deployments with multiple trust
   domains.

# Introduction

(1)

CURRENT:
   Devices that go beyond single-signer update require more complex
   rules for deploying software updates.  For example, devices may
   require:

   *  software Components from multiple software signing authorities.

   *  a mechanism to remove an unneeded Component

   *  single-object Dependencies

   *  a partly encrypted Manifest so that distribution does not reveal
      private information

   *  installation performed by a different execution mode than payload
      fetch

I have several comments on this part:

  + Was “single-signer update” introduced in some other documents? I failed to
  find this use in the suit manifest spec, rfc9019, etc. Same comment applies
  for “single-object Dependencies" term.

  + I was expecting to see explanation of “more complex” in the examples, which
  is not currently the case.

  + Consider s/software Components/Components given that “Component” is defined
  as “An updatable logical block of the Firmware, Software, configuration, or
  data of the Recipient”. No need to be redundant here.

  + Is the second bullet specific to the multi signer case? Isn’t this
  applicable even for the single-signer?

  + Where “payload fetch” mode is defined?

(2)

CURRENT:
  A device may contain a processor in its radio

I don’t parse this. Please reword.

(3) What is a device operator? Is it the vendor? Else? Please consider adding a
definition or supply a reference.

(4) Consider add references for TEEP-related entites:

CURRENT:
   *  A Trusted Application Manager (TAM) wants to distribute
      personalisation information to a Trusted Execution Environment in
      addition to a Trusted Application (TA),

(5) Add a reference for “core Manifest specification”. Also, update the teep
architecture to refer rfc9397, instead of the I-D.

CURRENT:
   These mechanisms are not part of the core Manifest specification, but
   they are needed for more advanced use cases, such as the architecture
   described in [I-D.ietf-teep-architecture].

(6) Fall a little bit short :-( A summary of the extensions would be helpful.

CURRENT:
   This specification extends the SUIT Manifest specification
   ([I-D.ietf-suit-manifest]).

# Section 2

I would avoid redundant terms but refer to draft-ietf-suit-manifest#section 2.
Only new terms should be listed here.

On the “see” use (not only in this section, but through the document)

CURRENT:
      authorization information, and severable elements (see Section 5.1
      of [I-D.ietf-suit-manifest]).

There are more than 60 occurrences of such in the document. Please simply
delete these as this makes the two too heavy. A reference is there to be
checked/viewed/seen/etc. :-)

# Section 3

(1) multi trust domain used without being adequately introduced early in the
document. Consider teasing this in the introduction:

CURRENT:
  The use of the features presented for use with multiple trust domains

(2) Other assumptions:

CURRENT:
   One additional assumption is added for the Update Procedure:

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

   One additional assumption is added to the Invocation Procedure:

Please call out where the “other” assumptions are defined? I suspect that you
meant 4.2 of the manifest. If so, please say so. If not, I need to understand
;-)

# Section 5

(1)

CURRENT:
   A Dependency is another SUIT_Envelope that describes additional
   Components.

   As described in Section 1, Dependencies enable several common use
   cases.

First, cite where the envelope is defined. Second, consider deleting the second
sentence; it does not bring much, IMO.

(2) ACLs

CURRENT:
  (no ACLs defining which authorities have access to different
   Components/Commands/Parameters),

How/where these ACLs are supplied? Can we have examples or a reference where
these examples are provided?

(3) Lack of interpreter behavior

CURRENT:
   If the Manifest contains more than one Component and/or Dependency,
   each Command sequence MUST begin with a Set Component Index Command.

Can we include a reminder about what to do when a mandatory condition is not
met, I guess the process will abort.

A generic statement would be helpful as this applies to other similar
statements in the document.

(4) Where Update is defined?

CURRENT:
   1.  The dependent MUST populate all Command sequences for the current
       Procedure (Update or Invoke).

Can we please add a pointer where this command is defined?

(5)

CURRENT:
   In complex systems, it may not always be clear where the Root
   Manifest should be stored;

Maybe

NEW:
   In complex systems, it may not always be clear where the Root
   Manifest is stored;

(6) Broken reference

CURRENT:
   This section augments the Abstract Machine Description (Section 6.4)
   in [I-D.ietf-suit-manifest].

Please check as there is no 6.4 in this doc. I guess you meant 6.4 of manifest.
Please fix.

(7)

CURRENT:
   *  Five new Commands are introduced:

      -  Set Parameters

      -  Process Dependency

      -  Is Dependency

      -  Dependency Integrity

      -  Unlink

Consider adding a statemen to basically invite readers to look at Section 5.6.

(8) what is reference counting/reference count?

CURRENT:
   *  When a Manifest Processor supports shared Dependencies, it MUST
      support reference counting of those Dependencies.

That’is? I don’t get the meaning here. I see “reference count” used in some
other parts but I have the same issue to parse the meaning, though.

(9) Lack of elaboration

CURRENT:
   3.  Loads the specified Component as a Dependency Manifest Envelope.

What does that mean concretely? Failed to find an elaboration of this in the
doc.

(10) Lack of reference

CURRENT:
   Similar to suit-directive-override-parameters, suit-directive-set-

Cite 8.4.10.3 of manifest (idem for other similar constructs)

(11) Some more reasoning is needed:

CURRENT:
   parameters allows the Manifest to configure behavior of future
   Directives by changing Parameters that are read by those Directives.
   Set Parameters is for use when Dependencies are used because it
   allows a Manifest to modify the behavior of its Dependencies.

This explains why this is useful for dependency, but does not explain it can’t
be used for other contexts.

(12) Factorize and ease identify failure conditions

CURRENT:
   If the current Component index does not have an entry in the suit-
   dependencies map, then this Command MUST Abort.

   If the current Component index has not been the target of a suit-
   condition-dependency-integrity, then this Command MUST Abort.

   If the current Component is True, then this Directive applies to all
   Dependencies.  If the current section is "Common metadata," then the
   Command sequence MUST Abort.

May factorize the abort part and list below the conditions as bullet list.

Also, please consider having a section with a summary of abort conditions
defined in this specification.

(13) TTL and Configurable parameters

CURRENT:
   The Manifest Processor MAY cache the results of these operations for
   later use from the context of the current Manifest.

Is there any TTL associated with this? If so, is that controllable/tunable?

Similar to the abort comment, consider having a dedicated section on
configurable parameters supported by the specification.

I see that you mention at least two conditions to flush out the cache. That’s
great.

# Section 8

Please check this sentence:

CURRENT:
  The Distribution System can Set the Parameter URI in the Fetch/

# Section 9

Any reason the labels weren’t assigned to be consistent with the flow use?

CURRENT:
             | 7     | Dependency Integrity | Section 5.6.4 |
             +-------+----------------------+---------------+
             | 8     | Is Dependency        | Section 5.6.3 |
             +-------+----------------------+---------------+
             | 11    | Process Dependency   | Section 5.6.2 |
             +-------+----------------------+---------------+
             | 19    | Set Parameters       | Section 5.6.1 |
             +-------+----------------------+---------------+
             | 33    | Unlink               | Section 5.6.5 |
             +-------+----------------------+---------------+

Hope this helps.

Cheers,
Med