Revert "suit: Rework SDFW and SDFW Recovery sinks"

This reverts commit 88380ce.

Signed-off-by: Håkon Amundsen <[email protected]>

Author: hakonfam

subsys/suit/plat_err/include/suit_plat_err.h

@@ -97,7 +97,6 @@ typedef int suit_plat_err_t;
 #define SUIT_PLAT_ERR_UNSUPPORTED      -2010 /**< Attempt to perform an unsupported operation */
 #define SUIT_PLAT_ERR_IPC	       -2011 /**< IPC error */
 #define SUIT_PLAT_ERR_NO_RESOURCES     -2012 /**< Not enough resources */
-#define SUIT_PLAT_ERR_SDRFW_FAILURE    -2013 /**< Failure during SDFW Recovery update */
 
 /**
  * If the error code is a common platform error code return it.

subsys/suit/platform/sdfw/src/suit_plat_copy.c

@@ -251,11 +251,9 @@ int suit_plat_copy(suit_component_t dst_handle, suit_component_t src_handle,
 	}
 
 	if (ret == SUIT_SUCCESS) {
-		plat_ret =
-			suit_generic_address_streamer_stream(payload_ptr, payload_size, &dst_sink);
-		if (plat_ret != SUIT_PLAT_SUCCESS) {
-			LOG_ERR("memptr_streamer failed - error %i", plat_ret);
-			ret = suit_plat_err_to_processor_err_convert(plat_ret);
+		ret = suit_generic_address_streamer_stream(payload_ptr, payload_size, &dst_sink);
+		if (ret != SUIT_PLAT_SUCCESS) {
+			LOG_ERR("memptr_streamer failed - error %i", ret);
 		}
 	}
 #endif /* CONFIG_SUIT_STREAM_SOURCE_MEMPTR */

subsys/suit/platform/src/suit_plat_error_convert.c

@@ -24,9 +24,6 @@ int suit_plat_err_to_processor_err_convert(suit_plat_err_t plat_err)
 	case SUIT_PLAT_ERR_CBOR_DECODING:
 		proc_err = SUIT_ERR_DECODING;
 		break;
-	case SUIT_PLAT_ERR_SDRFW_FAILURE:
-		proc_err = SUIT_FAIL_CONDITION;
-		break;
 	/* To be extended */
 	default:
 		/* Return SUIT_ERR_CRASH */

subsys/suit/stream/stream_sinks/src/suit_sdfw_recovery_sink.c

@@ -14,10 +14,18 @@
 #include <sdfw/sdfw_update.h>
 #include <suit_plat_mem_util.h>
 
+#include <suit_digest_sink.h>
+
 #define SUIT_MAX_SDFW_RECOVERY_COMPONENTS 1
 
+#define SDFW_RECOVERY_SINK_ERR_AGAIN                                                               \
+	1 /* Reboot is needed before proceeding. Call the API again.                               \
+	   */
+
 LOG_MODULE_REGISTER(suit_sdfw_recovery_sink, CONFIG_SUIT_LOG_LEVEL);
 
+typedef int sdf_sink_err_t;
+
 struct sdfw_recovery_sink_context {
 	bool in_use;
 	bool write_called;
@@ -36,19 +44,56 @@ static struct sdfw_recovery_sink_context *get_new_context(void)
 	return NULL;
 }
 
-static suit_plat_err_t schedule_update(const uint8_t *buf, size_t size)
+static digest_sink_err_t verify_digest(uint8_t *buf, size_t buf_size, psa_algorithm_t algorithm,
+				       uint8_t *expected_digest)
 {
-	int err = 0;
+	struct stream_sink digest_sink;
+	suit_plat_err_t err = suit_digest_sink_get(&digest_sink, algorithm, expected_digest);
 
-	if (!suit_plat_mem_clear_sicr_update_registers()) {
-		LOG_ERR("Failed to clear update registers");
-		/* By design SDFW Recovery update error should not result in failing whole
-		 * installation.
-		 * Because of that, set specific error code instead of SUIT_PLAT_ERR_CRASH.
-		 */
-		return SUIT_PLAT_ERR_SDRFW_FAILURE;
+	if (err != SUIT_PLAT_SUCCESS) {
+		LOG_ERR("Failed to get digest sink: %d", err);
+		return err;
+	}
+
+	err = digest_sink.write(digest_sink.ctx, buf, buf_size);
+	if (err != SUIT_PLAT_SUCCESS) {
+		LOG_ERR("Failed to write to stream: %d", err);
+		(void)digest_sink.release(digest_sink.ctx);
+		return err;
 	}
 
+	digest_sink_err_t ret = suit_digest_sink_digest_match(digest_sink.ctx);
+
+	err = digest_sink.release(digest_sink.ctx);
+	if (err != SUIT_PLAT_SUCCESS) {
+		LOG_WRN("Failed to release stream: %d", err);
+	}
+
+	return ret;
+}
+
+static suit_plat_err_t clear_urot_update_status(void)
+{
+	mram_erase((uintptr_t)&NRF_SICR->UROT.UPDATE,
+		   sizeof(NRF_SICR->UROT.UPDATE) / CONFIG_SDFW_MRAM_WORD_SIZE);
+
+	/* Clearing the registers is crucial for correct handling by SecROM. */
+	/* Incorrect mram_erase behavior was observed on FPGA. */
+	/* Since mram_erase returns void, there is a need for extra check and returning error code
+	 * to handle such case.
+	 */
+	if (NRF_SICR->UROT.UPDATE.STATUS == SICR_UROT_UPDATE_STATUS_CODE_None &&
+	    NRF_SICR->UROT.UPDATE.OPERATION == SICR_UROT_UPDATE_OPERATION_OPCODE_Nop) {
+		return SUIT_PLAT_SUCCESS;
+	} else {
+		return SUIT_PLAT_ERR_IO;
+	}
+}
+
+static suit_plat_err_t schedule_sdfw_recovery_update(const uint8_t *buf, size_t size)
+{
+	int err = 0;
+
 	const struct sdfw_update_blob update_blob = {
 		.manifest_addr =
 			(uintptr_t)(buf + CONFIG_SUIT_SDFW_RECOVERY_UPDATE_SIGNED_MANIFEST_OFFSET),
@@ -77,115 +122,126 @@ static suit_plat_err_t schedule_update(const uint8_t *buf, size_t size)
 
 	if (err) {
 		LOG_ERR("Failed to schedule SDFW Recovery update: %d", err);
-		err = SUIT_PLAT_ERR_CRASH;
-	} else {
-		err = SUIT_PLAT_SUCCESS;
+		return SUIT_PLAT_ERR_CRASH;
 	}
 
-	return err;
+	LOG_INF("SDFW Recovery update scheduled");
+
+	return SUIT_PLAT_SUCCESS;
 }
 
-static void reboot_to_continue(void)
+static sdf_sink_err_t check_update_candidate(const uint8_t *buf, size_t size)
 {
-	if (IS_ENABLED(CONFIG_SUIT_UPDATE_REBOOT_ENABLED)) {
-		LOG_INF("Reboot the system to continue update");
+	uint8_t *candidate_binary_start =
+		(uint8_t *)(buf + CONFIG_SUIT_SDFW_RECOVERY_UPDATE_FIRMWARE_OFFSET);
+	uint8_t *candidate_digest_in_manifest =
+		(uint8_t *)(buf + CONFIG_SUIT_SDFW_RECOVERY_UPDATE_DIGEST_OFFSET);
+	uint8_t *current_sdfw_recovery_digest =
+		(uint8_t *)(NRF_SICR->UROT.RECOVERY.SM.TBS.FW.DIGEST);
 
-		LOG_PANIC();
+	/* First check if calculated digest of candidate matches the digest from Signed Manifest */
+	digest_sink_err_t err =
+		verify_digest(candidate_binary_start,
+			      size - (size_t)CONFIG_SUIT_SDFW_RECOVERY_UPDATE_FIRMWARE_OFFSET,
+			      PSA_ALG_SHA_512, candidate_digest_in_manifest);
+
+	if (err != SUIT_PLAT_SUCCESS) {
+		if (err == DIGEST_SINK_ERR_DIGEST_MISMATCH) {
+			LOG_ERR("Candidate inconsistent");
+		} else {
+			LOG_ERR("Failed to calculate digest: %d", err);
+		}
 
-		sys_reboot(SYS_REBOOT_COLD);
-	} else {
-		LOG_DBG("Reboot disabled - perform manually");
+		return SUIT_PLAT_ERR_CRASH;
 	}
-}
 
-static suit_plat_err_t schedule_update_and_reboot(const uint8_t *buf, size_t size)
-{
-	suit_plat_err_t err = schedule_update(buf, size);
+	LOG_DBG("Candidate consistent");
 
+	/* Then compare candidate's digest with current SDFW Recovery digest */
+	err = verify_digest(candidate_binary_start,
+			    size - (size_t)CONFIG_SUIT_SDFW_RECOVERY_UPDATE_FIRMWARE_OFFSET,
+			    PSA_ALG_SHA_512, current_sdfw_recovery_digest);
 	if (err == SUIT_PLAT_SUCCESS) {
-		reboot_to_continue();
-		if (IS_ENABLED(CONFIG_SUIT_UPDATE_REBOOT_ENABLED)) {
-			/* If this code is reached, it means that reboot did not work. */
-			/* In such case report an error. */
-			LOG_ERR("Expected reboot did not happen");
-			err = SUIT_PLAT_ERR_UNREACHABLE_PATH;
+		LOG_INF("Same candidate - skip update");
+		return SUIT_PLAT_SUCCESS;
+	} else if (err == DIGEST_SINK_ERR_DIGEST_MISMATCH) {
+		LOG_INF("Different candidate");
+		err = schedule_sdfw_recovery_update(buf, size);
+		if (err == SUIT_PLAT_SUCCESS) {
+			LOG_DBG("Update scheduled");
+			err = SDFW_RECOVERY_SINK_ERR_AGAIN;
 		}
+		return err;
 	}
 
-	return err;
+	LOG_ERR("Failed to calculate digest: %d", err);
+	return SUIT_PLAT_ERR_CRASH;
 }
 
-static suit_plat_err_t update_already_ongoing(const uint8_t *buf, size_t size)
+static void reboot_to_continue(void)
 {
-	suit_plat_err_t err = SUIT_PLAT_SUCCESS;
+	if (IS_ENABLED(CONFIG_SUIT_UPDATE_REBOOT_ENABLED)) {
+		LOG_INF("Reboot the system to continue SDFW Recovery update");
 
-	enum sdfw_update_status update_status = sdfw_update_initial_status_get();
+		LOG_PANIC();
 
-	/* Candidate is different than current FW but SDFW Recovery update is already ongoing. */
-	switch (update_status) {
-	case SDFW_UPDATE_STATUS_NONE: {
-		/* No pending operation even though operation indicates SDFW Recovery update.
-		 * Yet candidate differs from current FW, so schedule the update.
-		 */
-		err = schedule_update_and_reboot(buf, size);
-		break;
-	}
-	default: {
-		/* SecROM indicates error during update */
-		LOG_ERR("Update failure: %08x", update_status);
-		/* By design SDFW Recovery update error should not result in failing whole
-		 * installation.
-		 * Because of that, set specific error code instead of SUIT_PLAT_ERR_CRASH.
-		 */
-		err = SUIT_PLAT_ERR_SDRFW_FAILURE;
-		break;
-	}
+		sys_reboot(SYS_REBOOT_COLD);
+	} else {
+		LOG_DBG("Reboot disabled - perform manually");
 	}
-
-	return err;
 }
 
-static suit_plat_err_t update_needed_action(const uint8_t *buf, size_t size)
+static suit_plat_err_t check_urot_none(const uint8_t *buf, size_t size)
 {
-	suit_plat_err_t err = SUIT_PLAT_SUCCESS;
+	/* Detect update candidate. */
+	/* It is enough to check Public Key Size field which occupies first 4B of Signed Manifest.
+	 */
+	if (*((uint32_t *)buf) == EMPTY_STORAGE_VALUE) {
+		LOG_INF("Update candidate not found");
+		return SUIT_PLAT_ERR_NOT_FOUND;
+	}
 
-	enum sdfw_update_operation initial_operation = sdfw_update_initial_operation_get();
+	LOG_INF("Update candidate found");
 
-	switch (initial_operation) {
-	case SDFW_UPDATE_OPERATION_NOP:
-	case SDFW_UPDATE_OPERATION_UROT_ACTIVATE: {
-		/* No previously running update or after the other slot update. */
-		/* Schedule an update of this slot. */
-		err = schedule_update_and_reboot(buf, size);
-		break;
-	}
-	case SDFW_UPDATE_OPERATION_RECOVERY_ACTIVATE: {
-		/* SDFW Recovery update already ongoing */
-		err = update_already_ongoing(buf, size);
-		break;
-	}
-	default: {
-		LOG_ERR("Unhandled operation: %08x", initial_operation);
-		err = SUIT_PLAT_ERR_CRASH;
-		break;
-	}
+	suit_plat_err_t err = check_update_candidate(buf, size);
+
+	if (err == SDFW_RECOVERY_SINK_ERR_AGAIN) {
+		/* Update scheduled, continue after reboot */
+		reboot_to_continue();
+		if (IS_ENABLED(CONFIG_SUIT_UPDATE_REBOOT_ENABLED)) {
+			/* If this code is reached, it means that reboot did not work. */
+			/* In such case report an error and convert the error code. */
+			LOG_ERR("Expected reboot did not happen");
+			err = SUIT_PLAT_ERR_UNREACHABLE_PATH;
+		}
 	}
 
 	return err;
 }
 
-static bool is_update_needed(const uint8_t *buf, size_t size)
+static suit_plat_err_t check_recovery_activated(const uint8_t *buf, size_t size)
 {
-	const uint8_t *candidate_digest_in_manifest =
-		(uint8_t *)(buf + CONFIG_SUIT_SDFW_RECOVERY_UPDATE_DIGEST_OFFSET);
-	const uint8_t *current_sdfw_recovery_digest =
-		(uint8_t *)(NRF_SICR->UROT.RECOVERY.SM.TBS.FW.DIGEST);
+	uint8_t *candidate_binary_start =
+		(uint8_t *)(buf + CONFIG_SUIT_SDFW_RECOVERY_UPDATE_FIRMWARE_OFFSET);
+	uint8_t *current_sdfw_digest = (uint8_t *)(NRF_SICR->UROT.RECOVERY.SM.TBS.FW.DIGEST);
+
+	/* Compare candidate's digest with current SDFW Recovery digest */
+	digest_sink_err_t err =
+		verify_digest(candidate_binary_start,
+			      size - (size_t)CONFIG_SUIT_SDFW_RECOVERY_UPDATE_FIRMWARE_OFFSET,
+			      PSA_ALG_SHA_512, current_sdfw_digest);
+	if (err != SUIT_PLAT_SUCCESS) {
+		if (err == DIGEST_SINK_ERR_DIGEST_MISMATCH) {
+			LOG_ERR("Digest mismatch - update failure");
+			return SUIT_PLAT_ERR_AUTHENTICATION;
+		}
 
-	bool digests_match = memcmp(candidate_digest_in_manifest, current_sdfw_recovery_digest,
-				    PSA_HASH_LENGTH(PSA_ALG_SHA_512)) == 0;
+		LOG_ERR("Failed to calculate digest: %d", err);
+		return SUIT_PLAT_ERR_CRASH;
+	}
 
-	/* Update is needed when candidate's digest doesn't match current FW digest */
-	return !digests_match;
+	LOG_DBG("Digest match - update success");
+	return SUIT_PLAT_SUCCESS;
 }
 
 /* NOTE: Size means size of the SDFW binary to be updated,
@@ -211,12 +267,57 @@ static suit_plat_err_t write(void *ctx, const uint8_t *buf, size_t size)
 	context->write_called = true;
 
 	suit_plat_err_t err = SUIT_PLAT_SUCCESS;
+	bool clear_registers = true;
 
-	if (is_update_needed(buf, size)) {
-		LOG_INF("Update needed");
-		err = update_needed_action(buf, size);
-	} else {
-		LOG_INF("Update not needed");
+	switch (NRF_SICR->UROT.UPDATE.STATUS) {
+	case SICR_UROT_UPDATE_STATUS_CODE_None: {
+		err = check_urot_none(buf, size);
+		/* Potential start of update process - SecROM needs the registers to be set */
+		clear_registers = false;
+		break;
+	}
+
+	case SICR_UROT_UPDATE_STATUS_CODE_RecoveryActivated: {
+		err = check_recovery_activated(buf, size);
+		clear_registers = true;
+		break;
+	}
+
+		/* TODO: Add handling of status RecoveryUnconfirmed and RecoveryConfirmed.
+		 *       For now the defines for these states are missing in mdk header files.
+		 *       NCSDK-26939
+		 */
+
+	case SICR_UROT_UPDATE_STATUS_CODE_UROTActivated:
+	case SICR_UROT_UPDATE_STATUS_CODE_VerifyOK:
+	case SICR_UROT_UPDATE_STATUS_CODE_AROTRecovery: {
+		LOG_ERR("Unsupported Recovery update status: 0x%08X", NRF_SICR->UROT.UPDATE.STATUS);
+		err = SUIT_PLAT_ERR_INCORRECT_STATE;
+		clear_registers = true;
+		break;
+	}
+
+	default: {
+		LOG_ERR("SDFW Recovery update failure: 0x%08X", NRF_SICR->UROT.UPDATE.STATUS);
+		err = NRF_SICR->UROT.UPDATE.STATUS;
+		clear_registers = true;
+		break;
+	}
+	}
+
+	if (clear_registers) {
+		suit_plat_err_t clear_err = clear_urot_update_status();
+
+		if (clear_err) {
+			LOG_ERR("Failed to clear UROT update status");
+			/* If the only error was during register clearing - report it. */
+			/* Otherwise report the original cause of failure. */
+			if (err == SUIT_PLAT_SUCCESS) {
+				err = clear_err;
+			}
+		} else {
+			LOG_DBG("UROT update status cleared");
+		}
 	}
 
 	return err;