Subject: [Suit] Benjamin Kaduk's No Objection on draft-ietf-suit-information-model-08: (with COMMENT)

From: Benjamin Kaduk via Datatracker <[email-masked]>
Date: Wed, 02 Dec 2020 22:45:37 -0800

Benjamin Kaduk has entered the following ballot position for
draft-ietf-suit-information-model-08: No Objection

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/iesg/statement/discuss-criteria.html
for more information about IESG DISCUSS and COMMENT positions.

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

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

(I did not attempt to cross-check that the forward- and backward-
references among threats and mitigating requirements and manifest
(envelope) elements are consistent.  That said, I did by accident find
one instance of apparent inconsistency, noted inline, so a dedicated
pass to check these seems in order.)

I'll also briefly mention here that I left a longer note about the
delegation chain appearing to be a bit underspecified; that's probably the
most important of my comments (the document in general is in
good shape).

Section 1

   The information model describes all the information elements required
   to secure firmware updates of IoT devices from the threats described
   in Section 4.1 and enables the user stories captured in Section 4.4.

The Introduction has to stand on its own, without the context of the
Abstract.  So, having the document start off like this is not great
writing style; there's no previous discussion to justify the definite
article in "the information model", for one, and we start off by just
forward-referencing that other sections will do some things.  Including
the entire abstract contents wholesale before this would be a decent
start, though additional editing is expected to be in order (the
abstract and introduction play different roles and generally the one is
not a strict subset of the other).

Section 2

   Secure time and secure clock refer to a set of requirements on time
   sources.  For local time sources, this primarily means that the clock
   must be monotonically increasing, including across power cycles,
   firmware updates, etc. [...]

But it doesn't have to be anywhere close to an actual reference time
source, just monotonic?

Section 3.3

   identically named entities from different geographic regions from
   colliding in their customer's infrastructure.  Recommended practice
   is to use [RFC4122] version 5 UUIDs with the vendor's domain name and
   the DNS name space ID.  Other options include type 1 and type 4
   UUIDs.

We should probably pick one of 'version' and 'type' when referring to
the UUID constructions.

Section 3.4

Does "more granular" mean more fine-grained or more coarse-grained?

Section 3.6

   When a payload applies to multiple versions of a firmware, the
   required image version list specifies which versions must be present
   for the update to be applied.  This allows the update author to

It's really hard for me to parse "which versions must be present" as
anything other than "there can be multiple versions present at the same
time and all of the listed versions must be present", even though I
suspect that the intent is that "the single version that is currently
installed must be in the list of required versions".

Section 3.10.2

   A device supports a full filesystem.  The Author chooses to use the

nit: I suggest "full-featured filesystem", since "full filesystem" can
be used to mean "a filesystem with no free space".

Section 3.12

Where is "the resource" defined?  I don't see it in this doc or in the
architecture doc.  The antepenultimate paragraph suggests that it is
intended to be synonymous with the 'payload'...

Section 3.15

   This element is REQUIRED in non-dependency manifests and represents
   the foundation of all security properties of the manifest.  Manifests
   which are included as dependencies by another manifest SHOULD include
   a signature so that the recipient can distinguish between different
   actors with different permissions.

nit: maybe add a few more words to clarify that the actors being
distinguished are the signers (authors?) of the various manifest
contents?

Section 3.16

I assume since this is IoT that the "additional installation
instructions" will need to be machine-readable, but "install on sunday
at 0200" and other examples could lead the reader to expect something
human readable, especially when "run a script" is included as a separate
option.  It might be worth a bit of clarification.

Section 3.18

   A list of other manifests that are required by the current manifest.
   Manifests are identified an unambiguous way, such as a digest.

"cryptographic digest" might be needed to ensure the desired property.

Section 3.21

I'm not entirely sure what unqualified "source" and "destination" are
intended to refer to in the context of loading a firmware image.

Section 3.24

I don't think the text here is enough to give a picture of what the
delegation chain is supposed to do.  Later discussion suggests that it
is a way to change or augment the set of entities authorized to sign
(top-level) manifests, but this text seems to just talk about
"authorization functionality" and not what is being authorized.  (It
also doesn't talk about what trust anchor the crypto chains to and
whether it needs to be the same thing that the manifest signatures chain
to, but that is perhaps excusable in an information model.)

Section 4

We should probably mention the URI security considerations (RFC 3986)
somewhere, as well as that remote resources need to receive an equal
amount of cryptographic protection as the manifest itself, when
dereferencing URIs.

We might also discuss the risks when a device believes that it possesses
a source of secure time but the timesource is actually flawed.

It might also be worth reiterating the topic that came up during one of
the other review threads: firmware update is *by definition* remote code
execution, so if you trust an entity to provide your firmware, you are
trusting them to do the right thing.  Many classes of attack involving
malicious or modified payloads then become irrelevant, so we are left
with just needing to verify that it did come from a trusted party and is
not going backwards, topics that are covered quite well already
(including TOCTOU).

Section 4.2

A bit of intro for the format of each subsection might be helpful; e.g.,
the optional presence of the "Threat Escalation" portion took me some
time to understand.

Section 4.2.9

   If an attacker can install their firmware on a device, by
   manipulating either payload or metadata, then they have complete

This feels like an "e.g." kind of thing, with payload and metadata
modification not necessarily being an exhaustive list of ways to get an
attacker's firmware on a device.

Section 4.2.11.1

   This is a denial of service because it can render devices inoperable.
   This is an elevation of privilege because it allows the attacker to
   make installation decisions that should be made by the Operator.

In this example it seems like the decision was supposed to have been
made by the Network Operator specifically, but perhaps I misunderstand.

Section 4.3.4

   The authenticity of an update MUST be demonstrable.  Typically, this
   means that updates must be digitally authenticated.  Because the
   manifest contains information about how to install the update, the
   manifest's authenticity MUST also be demonstrable.  To reduce the
   overhead required for validation, the manifest contains the digest of
   the firmware image, rather than a second digital signature.  The
   authenticity of the manifest can be verified with a digital signature
   or Message Authentication Code.  The authenticity of the firmware
   image is tied to the manifest by the use of a digest of the firmware
   image.

I suggest "cryptographic digest" here as well (twice).

Section 4.3.5

   Implemented by: Payload Format (Section 3.8), Storage Location
   (Section 3.10)

I know I said I didn't try to cross-check these (which is true!), but I
did notice that "Storage Location" was a bit funny for implementing an
authenticated payload type, and indeed §3.10 does not mention
REQ.SEC.AUTH.IMG_TYPE.

Section 4.3.16

   Status reports from the device to any remote system SHOULD be
   performed over an authenticated, confidential channel in order to
   prevent modification or spoofing of the reports.

Why is this only SHOULD (not MUST)?

Sections 4.3.16-4.3.20

None of these have "Implemented by" lines.  Should they?

Section 4.4.1

   o  Use a table of hashes to ensure that each block of the payload is
      validate before writing.

nit: either "valid" or "validated".

Section 4.4.7

   As a firmware author, I want to prevent confidential information from
   being disclosed during firmware updates.  It is assumed that channel
   security or at-rest encryption is adequate to protect the manifest
   itself against information disclosure.

This is the "image confidentiality" section, so I'm not sure why we're
talking about protecting the manifest itself.

Section 4.5.3

   Implemented by Manifest Element: Dependencies, StorageIdentifier,
   ComponentIdentifier

(nit?) should this just be "Implemented by:"?

Section 4.5.3.1

   An IoT device with multiple microcontrollers in the same physical
   device (HeSA) will likely require multiple payloads with different
   component identifiers.

Please expand HeSA.

Section 4.5.9

   systems.  This requires the manifest to specify the digest of each
   statically linked dependency.  In addition, the manifest format MUST

I don't understand how this follows.  Aren't statically linked
dependencies incorporated into the image itself, in which case knowing
their individual digest values serve no purpose?
(Also, "cryptographic digest" again.)

Section 4.5.12

   The structure of the manifest MUST be simple to parse, without need
   for a general-purpose parser.

I'm not sure what constitutes a "general-purpose parser" for this
purpose.

Section 7.1

RFCs 5652 and 8152 are in the context of a "such as" qualifier (even
though it's for an overall MAY requirement), and as such probably don't
strictly need to be classified as normative.