Merge pull request #15092 from bergzand/pr/suit/per_component_flags

suit: Introduce per-component flags

sys/include/suit.h

@@ -64,6 +64,17 @@ extern "C" {
  */
 #define SUIT_VERSION                        (1)
 
+/**
+ * @name SUIT manifest status flags
+ *
+ * These flags apply to the full manifest.
+ * @{
+ */
+/**
+ * @brief Bit flags used to determine if SUIT manifest contains components
+ */
+#define SUIT_STATE_HAVE_COMPONENTS          (1 << 0)
+
 /**
  * @brief COSE signature OK
  */
@@ -73,6 +84,7 @@ extern "C" {
  * @brief COSE payload matches SUIT manifest digest
  */
 #define SUIT_STATE_FULLY_AUTHENTICATED      (1 << 2)
+/** @} */
 
 /**
  * @brief SUIT error codes
@@ -165,12 +177,25 @@ typedef struct {
     uint16_t offset;    /**< offset to the start of the content */
 } suit_param_ref_t;
 
+/**
+ * @name SUIT component flags.
+ *
+ * These state flags apply to individual components inside a manifest.
+ * @{
+ */
+#define SUIT_COMPONENT_STATE_FETCHED       (1 << 0) /**< Component is fetched */
+#define SUIT_COMPONENT_STATE_FETCH_FAILED  (1 << 1) /**< Component fetched but failed */
+#define SUIT_COMPONENT_STATE_VERIFIED      (1 << 2) /**< Component is verified */
+#define SUIT_COMPONENT_STATE_FINALIZED     (1 << 3) /**< Component successfully installed */
+/** @} */
+
 /**
  * @brief SUIT component struct as decoded from the manifest
  *
  * The parameters are references to CBOR-encoded information in the manifest.
  */
 typedef struct {
+    uint16_t state;                             /**< Component status flags */
     suit_param_ref_t identifier;                /**< Component identifier */
     suit_param_ref_t param_vendor_id;           /**< Vendor ID */
     suit_param_ref_t param_class_id;            /**< Class ID */
@@ -205,14 +230,6 @@ typedef struct {
     size_t urlbuf_len;              /**< Length of the manifest url */
 } suit_manifest_t;
 
-/**
- * @brief Bit flags used to determine if SUIT manifest contains components
- */
-#define SUIT_MANIFEST_HAVE_COMPONENTS   (0x1)
-/**
- * @brief Bit flags used to determine if SUIT manifest contains an image
- */
-#define SUIT_MANIFEST_HAVE_IMAGE        (0x2)
 
 /**
  * @brief Component index representing all components
@@ -253,6 +270,32 @@ int suit_parse(suit_manifest_t *manifest, const uint8_t *buf, size_t len);
  */
 int suit_policy_check(suit_manifest_t *manifest);
 
+/**
+ * @brief Set a component flag
+ *
+ * @param   component   Component to set flag for
+ * @param   flag        Flag to set
+ */
+static inline void suit_component_set_flag(suit_component_t *component,
+                                           uint16_t flag)
+{
+    component->state |= flag;
+}
+
+/**
+ * @brief Check a component flag
+ *
+ * @param   component   Component to check a flag for
+ * @param   flag        Flag to check
+ *
+ * @returns             True if the flag is set
+ */
+static inline bool suit_component_check_flag(suit_component_t *component,
+                                             uint16_t flag)
+{
+    return (component->state & flag);
+}
+
 /**
  * @brief Helper function for writing bytes on flash a specified offset
  *

sys/suit/handlers_command_seq.c

@@ -279,6 +279,12 @@ static int _dtv_fetch(suit_manifest_t *manifest, int key,
 
     suit_component_t *comp = _get_component(manifest);
 
+    /* Deny the fetch if the component was already fetched before */
+    if (suit_component_check_flag(comp, SUIT_COMPONENT_STATE_FETCHED)) {
+        LOG_ERROR("Component already fetched before\n");
+        return SUIT_ERR_INVALID_MANIFEST;
+    }
+
     nanocbor_value_t param_uri;
     suit_param_ref_to_cbor(manifest, &comp->param_uri,
                            &param_uri);
@@ -316,13 +322,16 @@ static int _dtv_fetch(suit_manifest_t *manifest, int key,
         return res;
     }
 
+    suit_component_set_flag(comp, SUIT_COMPONENT_STATE_FETCHED);
+
     if (res) {
-        LOG_INFO("image download failed\n)");
+        suit_component_set_flag(comp, SUIT_COMPONENT_STATE_FETCH_FAILED);
+        /* TODO: The leftover data from a failed fetch should be purged. It
+         * could contain potential malicous data from an attacker */
+        LOG_INFO("image download failed with code %i\n", res);
         return res;
     }
 
-    manifest->state |= SUIT_MANIFEST_HAVE_IMAGE;
-
     LOG_DEBUG("Update OK\n");
     return SUIT_OK;
 }
@@ -363,6 +372,14 @@ static int _dtv_verify_image_match(suit_manifest_t *manifest, int key,
         return SUIT_ERR_INVALID_MANIFEST;
     }
 
+    /* Only check the component if it is fetched, but not failed */
+    if (!suit_component_check_flag(comp, SUIT_COMPONENT_STATE_FETCHED) ||
+            suit_component_check_flag(comp, SUIT_COMPONENT_STATE_FETCH_FAILED)) {
+        LOG_ERROR("Fetch failed, or nothing fetched, nothing to check: %u\n",
+                  comp->state);
+        return SUIT_ERR_INVALID_MANIFEST;
+    }
+
     LOG_INFO("Verifying image digest\n");
     nanocbor_value_t _v;
     if (suit_param_ref_to_cbor(manifest, &comp->param_digest, &_v) == 0) {

sys/suit/handlers_common.c

@@ -76,7 +76,7 @@ static int _component_handler(suit_manifest_t *manifest, int key,
     }
     manifest->components_len = n;
     if (n) {
-        manifest->state |= SUIT_MANIFEST_HAVE_COMPONENTS;
+        manifest->state |= SUIT_STATE_HAVE_COMPONENTS;
     }
     return 0;
 }

sys/suit/transport/coap.c

@@ -363,11 +363,6 @@ static void _suit_handle_url(const char *url)
             return;
         }
 
-        LOG_INFO("suit_parse() success\n");
-        if (!(manifest.state & SUIT_MANIFEST_HAVE_IMAGE)) {
-            LOG_INFO("manifest parsed, but no image fetched\n");
-            return;
-        }
 #endif
         if (res == 0) {
             const riotboot_hdr_t *hdr = riotboot_slot_get_hdr(