Subject: Re: [Suit] CBOR pull parsing vs. packed binary format extraction

From: David Brown <[email-masked]>
Date: Wed, 12 Dec 2018 16:34:32 -0700

On Tue, Nov 13, 2018 at 12:24:17PM +0000, Brendan Moran wrote:

>I have worked up an example pull parser for Class 1 devices. It is
>published in the suit-manifest-generator repo:
>
>https://github.com/ARMmbed/suit-manifest-generator/tree/master/parser-example
>
>This repo demonstrates constructing a highly specialised parser for
>the draft-moran-suit-manifest-03 draft (with one small modification
>that will be included in the 04 draft) as detailed in README.md.

This parser doesn't appear to check the size of the buffer it is
decoding, and especially when verifying the signature just uses the
length in the CBOR data without regards to whether it has that much
data available.

At a minimum, it needs to be very strict about sizes when verifying
the signature, and probably should be throughout.

David

---

From: David Brown <[email-masked]>
Date: Wed, 12 Dec 2018 16:38:37 -0700

On Tue, Nov 13, 2018 at 12:24:17PM +0000, Brendan Moran wrote:

>The parser consumes 560 bytes of flash, uses only a single pointer of
>state between function calls, returns interpreted integers and
>references to byte strings to save memory when called, and uses no
>dynamic memory. It is guaranteed not to issue unaligned accesses,
>since all accesses are explicitly byte-wise.

As a comparison with MCUboot, which currently uses a manifest encoded
with a simple TLV format.  This manifest is a little simpler and more
specific than SUIT's, does not describe any dependencies, and
basically represents the signature.

The code to traverse the tlv itself is 388 bytes.  The whole code to
verify signatures (using mbed TLS for the RSA operations) is 1220
bytes of Cortex-M code.

>The parser can be used in a low-resource mode, consuming 333 bytes of
>flash, but giving up structural validation of the CBOR, and assuming
>that any fixed components (such as containers) of the manifest are
>correct

I'm not sure if this low-resource version is useful, since a malicious
manifest could cause it to exceed the input buffer.  In fact, as
mentioned previously, I think the existing (560 bytes) of code will
grow somewhat when the bounds for at least the signature are checked.

David

---

From: Derek Atkins <[email-masked]>
Date: Thu, 13 Dec 2018 09:32:15 -0500

> grow somewhat when the bounds for at least the signature are checked.

The parser knows the total size of the manifest (or at least it SHOULD
know that).  A broken manifest can re-split itself however it wants, but
an overrun (beyond the size of the manifest) should be easily blocked.
For example, if the manifest is 128 bytes total, but the data claims a
signature size of 256, the parser should be able to throw that out
easily.

I don't see a way to block a manifest saying a signature is X+Y bytes
(going across the manifest data separation boundaries).  For example, if
the total manifest is 1000 bytes, and the signature data claims to be
256 (but really it's only 128), I don't see a way ANY parser could
properly handle this case.  Technically (IMHO) this is not an overrun,
but hopefully the signature verification process will fail.

> David

-derek
-- 
       Derek Atkins                 617-623-3745
       [email-masked]             www.ihtfp.com
       Computer and Internet Security Consultant