suit: Adapt manifest handling to use common storage backend

examples/suit_update/Makefile

@@ -46,7 +46,7 @@ QUIET ?= 1
 #
 
 USEMODULE += nanocoap_sock sock_util
-USEMODULE += suit suit_transport_coap
+USEMODULE += suit suit_transport_coap suit_storage_flashwrite
 
 # Display a progress bar during firmware download
 USEMODULE += progress_bar

sys/include/suit.h

@@ -36,7 +36,6 @@
 #include "cose/sign.h"
 #include "nanocbor/nanocbor.h"
 #include "uuid.h"
-#include "riotboot/flashwrite.h"
 
 #ifdef __cplusplus
 extern "C" {
@@ -205,8 +204,7 @@ typedef struct {
  *
  * Breaks a dependency chain
  */
-typedef struct suit_storage suit_storage_t;
+typedef struct suit_storage suit_storage_ref_t;
-
 
 /**
  * @brief SUIT component struct as decoded from the manifest
@@ -214,7 +212,7 @@ typedef struct suit_storage suit_storage_t;
  * The parameters are references to CBOR-encoded information in the manifest.
  */
 typedef struct {
-    suit_storage_t *storage_backend;            /**< Storage backend used */
+    suit_storage_ref_t *storage_backend;        /**< Storage backend used */
     uint16_t state;                             /**< Component status flags */
     suit_param_ref_t identifier;                /**< Component identifier */
     suit_param_ref_t param_vendor_id;           /**< Vendor ID */
@@ -243,14 +241,12 @@ typedef struct {
     suit_component_t components[CONFIG_SUIT_COMPONENT_MAX];
     unsigned components_len;        /**< Current number of components */
     uint8_t component_current;      /**< Current component index */
-    riotboot_flashwrite_t *writer;  /**< Pointer to the riotboot flash writer */
     /** Manifest validation buffer */
     uint8_t validation_buf[SUIT_COSE_BUF_SIZE];
     char *urlbuf;                   /**< Buffer containing the manifest url */
     size_t urlbuf_len;              /**< Length of the manifest url */
 } suit_manifest_t;
 
-
 /**
  * @brief Component index representing all components
  *
@@ -321,7 +317,7 @@ static inline bool suit_component_check_flag(suit_component_t *component,
  *
  * Each component part is prefixed with @p separator
  *
- * @return          SUIT_OK if succesful
+ * @return          SUIT_OK if successful
  * @return          negative error code on error
  */
 int suit_component_name_to_string(const suit_manifest_t *manifest,
@@ -340,8 +336,8 @@ int suit_component_name_to_string(const suit_manifest_t *manifest,
  * @return              0 on success
  * @return              <0 on error
  */
-int suit_flashwrite_helper(void *arg, size_t offset, uint8_t *buf, size_t len,
+int suit_storage_helper(void *arg, size_t offset, uint8_t *buf, size_t len,
-                           int more);
+                        int more);
 
 #ifdef __cplusplus
 }

sys/suit/handlers_command_seq.c

@@ -26,20 +26,34 @@
 #include <nanocbor/nanocbor.h>
 #include <assert.h>
 
+#include "hashes/sha256.h"
+
 #include "kernel_defines.h"
 #include "suit/conditions.h"
 #include "suit/handlers.h"
 #include "suit/policy.h"
+#include "suit/storage.h"
 #include "suit.h"
-#include "riotboot/hdr.h"
-#include "riotboot/slot.h"
 
 #ifdef MODULE_SUIT_TRANSPORT_COAP
 #include "suit/transport/coap.h"
 #endif
+#include "suit/transport/mock.h"
 
 #include "log.h"
 
+static int _get_component_size(suit_manifest_t *manifest,
+                               suit_component_t *comp,
+                               uint32_t *img_size)
+{
+    nanocbor_value_t param_size;
+    if ((suit_param_ref_to_cbor(manifest, &comp->param_size, &param_size) == 0) ||
+            (nanocbor_get_uint32(&param_size, img_size) < 0)) {
+        return SUIT_ERR_INVALID_MANIFEST;
+    }
+    return SUIT_OK;
+}
+
 static suit_component_t *_get_component(suit_manifest_t *manifest)
 {
     /* Out-of-bounds check has been done in the _dtv_set_comp_idx, True/False
@@ -126,12 +140,16 @@ static int _cond_comp_offset(suit_manifest_t *manifest,
     suit_param_ref_to_cbor(manifest, &comp->param_component_offset,
                            &param_offset);
     nanocbor_get_uint32(&param_offset, &offset);
-    uint32_t other_offset = (uint32_t)riotboot_slot_offset(
-        riotboot_slot_other());
 
-    LOG_INFO("Comparing manifest offset %"PRIx32" with other slot offset %"PRIx32"\n",
+    if (!suit_storage_has_offset(comp->storage_backend)) {
-             offset, other_offset);
+        return SUIT_ERR_COND;
-    return other_offset == offset ? SUIT_OK : SUIT_ERR_COND;
+    }
+
+    LOG_INFO("Comparing manifest offset %"PRIx32" with other slot offset\n",
+             offset);
+
+    return suit_storage_match_offset(comp->storage_backend, offset) ?
+        SUIT_OK : SUIT_ERR_COND;
 }
 
 static int _dtv_set_comp_idx(suit_manifest_t *manifest,
@@ -156,8 +174,21 @@ static int _dtv_set_comp_idx(suit_manifest_t *manifest,
         return SUIT_ERR_INVALID_MANIFEST;
     }
 
+    suit_component_t *component = &manifest->components[new_index];
+    char name[CONFIG_SUIT_COMPONENT_MAX_NAME_LEN];
+
+    suit_storage_t *storage = component->storage_backend;
+    char separator = suit_storage_get_separator(storage);
+
+    /* Done this before in the component stage, shouldn't be different now */
+    suit_component_name_to_string(manifest, component,
+                                  separator, name, sizeof(name));
+
+    suit_storage_set_active_location(storage, name);
+
     /* Update the manifest context */
     manifest->component_current = new_index;
+
     LOG_INFO("Setting component index to %d\n",
               (int)manifest->component_current);
     return 0;
@@ -262,6 +293,25 @@ static int _dtv_set_param(suit_manifest_t *manifest, int key,
     return SUIT_OK;
 }
 
+static int _start_storage(suit_manifest_t *manifest, suit_component_t *comp)
+{
+    uint32_t img_size = 0;
+    char name[CONFIG_SUIT_COMPONENT_MAX_NAME_LEN];
+    char separator = suit_storage_get_separator(comp->storage_backend);
+
+    if (_get_component_size(manifest, comp, &img_size) < 0) {
+        return SUIT_ERR_INVALID_MANIFEST;
+    }
+
+    /* Done this before in the component stage, shouldn't be different now */
+    suit_component_name_to_string(manifest, comp,
+                                  separator, name, sizeof(name));
+
+    suit_storage_set_active_location(comp->storage_backend, name);
+
+    return suit_storage_start(comp->storage_backend, manifest, img_size);
+}
+
 static int _dtv_fetch(suit_manifest_t *manifest, int key,
                       nanocbor_value_t *_it)
 {
@@ -299,8 +349,10 @@ static int _dtv_fetch(suit_manifest_t *manifest, int key,
     LOG_DEBUG("_dtv_fetch() fetching \"%s\" (url_len=%u)\n", manifest->urlbuf,
               (unsigned)url_len);
 
-    int target_slot = riotboot_slot_other();
+    if (_start_storage(manifest, comp) < 0) {
-    riotboot_flashwrite_init(manifest->writer, target_slot);
+        LOG_ERROR("Unable to start storage backend\n");
+        return SUIT_ERR_STORAGE;
+    }
 
     res = -1;
 
@@ -308,7 +360,7 @@ static int _dtv_fetch(suit_manifest_t *manifest, int key,
 #ifdef MODULE_SUIT_TRANSPORT_COAP
     else if (strncmp(manifest->urlbuf, "coap://", 7) == 0) {
         res = suit_coap_get_blockwise_url(manifest->urlbuf, COAP_BLOCKSIZE_64,
-                                          suit_flashwrite_helper,
+                                          suit_storage_helper,
                                           manifest);
     }
 #endif
@@ -355,6 +407,46 @@ static int _get_digest(nanocbor_value_t *bstr, const uint8_t **digest, size_t *d
     return nanocbor_get_bstr(&arr_it, digest, digest_len);
 }
 
+static int _validate_payload(suit_component_t *component, const uint8_t *digest, size_t payload_size)
+{
+    uint8_t payload_digest[SHA256_DIGEST_LENGTH];
+    suit_storage_t *storage = component->storage_backend;
+
+    if (suit_storage_has_readptr(storage)) {
+        /* Direct read possible */
+        const uint8_t *payload = NULL;
+        size_t payload_len = 0;
+
+        suit_storage_read_ptr(storage, &payload, &payload_len);
+        if (payload_size != payload_len) {
+            return SUIT_ERR_STORAGE_EXCEEDED;
+        }
+        sha256(payload, payload_len, payload_digest);
+    }
+    else {
+        /* Piecewise feeding */
+        sha256_context_t ctx;
+        sha256_init(&ctx);
+        size_t pos = 0;
+        while (pos < payload_size) {
+            uint8_t buf[64];
+
+            size_t read_len = (payload_size - pos) > sizeof(buf) ?
+                              sizeof(buf) : payload_size - pos;
+
+            suit_storage_read(storage, buf, pos, read_len);
+            sha256_update(&ctx, buf, read_len);
+
+            pos += read_len;
+        }
+        sha256_final(&ctx, payload_digest);
+    }
+
+    return (memcmp(digest, payload_digest, SHA256_DIGEST_LENGTH) == 0) ?
+        SUIT_OK : SUIT_ERR_DIGEST_MISMATCH;
+}
+
+
 static int _dtv_verify_image_match(suit_manifest_t *manifest, int key,
                                    nanocbor_value_t *_it)
 {
@@ -362,13 +454,10 @@ static int _dtv_verify_image_match(suit_manifest_t *manifest, int key,
     LOG_DEBUG("dtv_image_match\n");
     const uint8_t *digest;
     size_t digest_len;
-    int target_slot = riotboot_slot_other();
     suit_component_t *comp = _get_component(manifest);
 
     uint32_t img_size;
-    nanocbor_value_t param_size;
+    if (_get_component_size(manifest, comp, &img_size) < 0) {
-    if ((suit_param_ref_to_cbor(manifest, &comp->param_size, &param_size) == 0) ||
-            (nanocbor_get_uint32(&param_size, &img_size) < 0)) {
         return SUIT_ERR_INVALID_MANIFEST;
     }
 
@@ -393,21 +482,20 @@ static int _dtv_verify_image_match(suit_manifest_t *manifest, int key,
         return SUIT_ERR_INVALID_MANIFEST;
     }
 
-    res = riotboot_flashwrite_verify_sha256(digest,
+    /* TODO: replace with generic verification (not only sha256) */
-                                            img_size,
+    LOG_INFO("Starting digest verification against image\n");
-                                            target_slot);
+    res = _validate_payload(comp, digest, img_size);
-    if (res != 0) {
+    if (res == SUIT_OK) {
-        return SUIT_ERR_COND;
+        LOG_INFO("Install correct payload\n");
+        suit_storage_install(comp->storage_backend, manifest);
     }
-
+    else {
-    /**
+        LOG_INFO("Erasing bad payload\n");
-     * SUIT transport mock doesn't implement the the 'finish' function. Can be
+        if (comp->storage_backend->driver->erase) {
-     * removed as soon as there is a proper storage abstraction layer for SUIT
+            suit_storage_erase(comp->storage_backend);
-     */
+        }
-    if (!IS_ACTIVE(MODULE_SUIT_TRANSPORT_MOCK)) {
-        riotboot_flashwrite_finish(manifest->writer);
     }
-    return SUIT_OK;
+    return res;
 }
 
 /* begin{code-style-ignore} */

sys/suit/handlers_common.c

@@ -13,8 +13,8 @@
  * @file
  * @brief       SUIT handler implementations for the manifest Common sections
  *
- * This file contains functions to handle the common info sections of a manifest.
+ * This file contains functions to handle the common info sections of a
- * This includes components, dependencies and command sequences.
+ * manifest. This includes components, dependencies and command sequences.
  *
  * @author      Koen Zandberg <koen@bergzand.net>
  *
@@ -26,6 +26,7 @@
 
 #include "kernel_defines.h"
 #include "suit/handlers.h"
+#include "suit/storage.h"
 #include "suit.h"
 
 #include "log.h"
@@ -61,7 +62,9 @@ static int _component_handler(suit_manifest_t *manifest, int key,
                       nanocbor_get_type(it));
             return SUIT_ERR_INVALID_MANIFEST;
         }
-        suit_param_cbor_to_ref(manifest, &component->identifier, &comp);
+        /* The array is stored so that the number of bytestrings in the array
+         * can be retrieved later */
+        suit_param_cbor_to_ref(manifest, &component->identifier, &arr);
         /* Ensure that all parts of the identifier are a bstr */
         while (!nanocbor_at_end(&comp)) {
             const uint8_t *identifier;
@@ -72,6 +75,16 @@ static int _component_handler(suit_manifest_t *manifest, int key,
             }
         }
         nanocbor_leave_container(&arr, &comp);
+
+        /* find storage handler for component */
+        suit_storage_t *storage = suit_storage_find_by_component(manifest,
+                                                                 component);
+        if (!storage) {
+            LOG_ERROR("No storage handler found for component\n");
+            return SUIT_ERR_UNSUPPORTED;
+        }
+        component->storage_backend = storage;
+
         n++;
     }
     manifest->components_len = n;
@@ -97,8 +110,7 @@ int _common_sequence_handler(suit_manifest_t *manifest, int key,
     (void)key;
     LOG_DEBUG("Starting conditional sequence handler\n");
     return suit_handle_manifest_structure_bstr(manifest, it,
-                                               suit_command_sequence_handlers,
+            suit_command_sequence_handlers, suit_command_sequence_handlers_len);
-                                               suit_command_sequence_handlers_len);
 }
 
 /* begin{code-style-ignore} */

sys/suit/handlers_global.c

@@ -29,6 +29,7 @@
 #include "suit/handlers.h"
 #include "suit/policy.h"
 #include "suit.h"
+#include "suit/storage.h"
 
 extern int _common_sequence_handler(suit_manifest_t *manifest, int key,
                                     nanocbor_value_t *it);
@@ -55,27 +56,25 @@ static int _seq_no_handler(suit_manifest_t *manifest, int key,
 {
     (void)key;
 
-    int32_t seq_nr;
+    uint32_t seq_nr;
 
-    if (nanocbor_get_int32(it, &seq_nr) < 0) {
+    if (nanocbor_get_uint32(it, &seq_nr) < 0) {
         LOG_INFO("Unable to get sequence number\n");
         return SUIT_ERR_INVALID_MANIFEST;
     }
-    const riotboot_hdr_t *hdr = riotboot_slot_get_hdr(riotboot_slot_current());
+
-    if (seq_nr <= (int32_t)hdr->version) {
+    uint32_t stored_seq_no = 0;
-        LOG_INFO("%" PRId32 " <= %" PRId32 "\n", seq_nr, hdr->version);
+    if (suit_storage_get_highest_seq_no(&stored_seq_no) < 0) {
-        LOG_INFO("seq_nr <= running image\n)");
+        return SUIT_ERR_STORAGE;
+    }
+    LOG_INFO("Manifest seq_no: %"PRIu32", highest available: %"PRIu32"\n",
+             seq_nr, stored_seq_no);
+
+    if (seq_nr <= stored_seq_no) {
+        LOG_ERROR("seq_nr <= running image\n)");
         return SUIT_ERR_SEQUENCE_NUMBER;
     }
 
-    hdr = riotboot_slot_get_hdr(riotboot_slot_other());
-    if (riotboot_hdr_validate(hdr) == 0) {
-        if (seq_nr <= (int32_t)hdr->version) {
-            LOG_INFO("%" PRIu32 " <= %" PRIu32 "\n", seq_nr, hdr->version);
-            LOG_INFO("seq_nr <= other image\n)");
-            return SUIT_ERR_SEQUENCE_NUMBER;
-        }
-    }
     LOG_INFO("suit: validated sequence number\n)");
     manifest->validated |= SUIT_VALIDATED_SEQ_NR;
     return SUIT_OK;

sys/suit/transport/coap.c

@@ -37,12 +37,12 @@
 
 #ifdef MODULE_RIOTBOOT_SLOT
 #include "riotboot/slot.h"
-#include "riotboot/flashwrite.h"
 #endif
 
 #ifdef MODULE_SUIT
 #include "suit.h"
 #include "suit/handlers.h"
+#include "suit/storage.h"
 #endif
 
 #if defined(MODULE_PROGRESS_BAR)
@@ -53,8 +53,8 @@
 #include "debug.h"
 
 #ifndef SUIT_COAP_STACKSIZE
-/* allocate stack needed to keep a page buffer and do manifest validation */
+/* allocate stack needed to do manifest validation */
-#define SUIT_COAP_STACKSIZE (3 * THREAD_STACKSIZE_LARGE + FLASHPAGE_SIZE)
+#define SUIT_COAP_STACKSIZE (3 * THREAD_STACKSIZE_LARGE)
 #endif
 
 #ifndef SUIT_COAP_PRIO
@@ -349,11 +349,9 @@ static void _suit_handle_url(const char *url)
         LOG_INFO("suit_coap: got manifest with size %u\n", (unsigned)size);
 
 #ifdef MODULE_SUIT
-        riotboot_flashwrite_t writer;
         suit_manifest_t manifest;
         memset(&manifest, 0, sizeof(manifest));
 
-        manifest.writer = &writer;
         manifest.urlbuf = _url;
         manifest.urlbuf_len = SUIT_URL_MAX;
 
@@ -384,17 +382,16 @@ static void _suit_handle_url(const char *url)
     }
 }
 
-int suit_flashwrite_helper(void *arg, size_t offset, uint8_t *buf, size_t len,
+int suit_storage_helper(void *arg, size_t offset, uint8_t *buf, size_t len,
-                           int more)
+                        int more)
 {
     suit_manifest_t *manifest = (suit_manifest_t *)arg;
-    riotboot_flashwrite_t *writer = manifest->writer;
 
     uint32_t image_size;
     nanocbor_value_t param_size;
     size_t total = offset + len;
-    suit_param_ref_t *ref_size =
+    suit_component_t *comp = &manifest->components[manifest->component_current];
-        &manifest->components[manifest->component_current].param_size;
+    suit_param_ref_t *ref_size = &comp->param_size;
 
     /* Grab the total image size from the manifest */
     if ((suit_param_ref_to_cbor(manifest, ref_size, &param_size) == 0) ||
@@ -403,28 +400,10 @@ int suit_flashwrite_helper(void *arg, size_t offset, uint8_t *buf, size_t len,
         return -1;
     }
 
-
-    if (offset == 0) {
-        if (len < RIOTBOOT_FLASHWRITE_SKIPLEN) {
-            LOG_WARNING("_suit_flashwrite(): offset==0, len<4. aborting\n");
-            return -1;
-        }
-        offset = RIOTBOOT_FLASHWRITE_SKIPLEN;
-        buf += RIOTBOOT_FLASHWRITE_SKIPLEN;
-        len -= RIOTBOOT_FLASHWRITE_SKIPLEN;
-    }
-
-    if (writer->offset != offset) {
-        LOG_WARNING(
-            "_suit_flashwrite(): writer->offset=%u, offset==%u, aborting\n",
-            (unsigned)writer->offset, (unsigned)offset);
-        return -1;
-    }
-
     if (image_size < offset + len) {
         /* Extra newline at the start to compensate for the progress bar */
         LOG_ERROR(
-            "\n_suit_flashwrite(): Image beyond size, offset + len=%u, "
+            "\n_suit_coap(): Image beyond size, offset + len=%u, "
             "image_size=%u\n", (unsigned)(total), (unsigned)image_size);
         return -1;
     }
@@ -437,7 +416,13 @@ int suit_flashwrite_helper(void *arg, size_t offset, uint8_t *buf, size_t len,
 
     _print_download_progress(manifest, offset, len, image_size);
 
-    return riotboot_flashwrite_putbytes(writer, buf, len, more);
+    int res = suit_storage_write(comp->storage_backend, manifest, buf, offset, len);
+    if (!more) {
+        LOG_INFO("Finalizing payload store\n");
+        /* Finalize the write if no more data available */
+        res = suit_storage_finish(comp->storage_backend, manifest);
+    }
+    return res;
 }
 
 static void *_suit_coap_thread(void *arg)