rename parameters in decrypt filter; add testcase for failing writing to RAM

nordic-mik7 · nordic-mik7 · commit 98310249ae30 · 2025-01-07T12:42:27.000+01:00
diff --git a/subsys/suit/stream/stream_filters/include/suit_decrypt_filter.h b/subsys/suit/stream/stream_filters/include/suit_decrypt_filter.h
@@ -18,17 +18,17 @@ extern "C" {
 /**
  * @brief Get decrypt filter object
  *
- * @param[out] dec_sink  Pointer to destination sink_stream to pass decrypted data
+ * @param[out] in_sink   Pointer to input sink_stream to pass encrypted data
  * @param[in]  enc_info  Pointer to the structure with encryption info.
  * @param[in]  class_id  Pointer to the manifest class ID of the destination component
- * @param[in]  enc_sink  Pointer to source sink_stream to be filled with encrypted data
+ * @param[in]  out_sink  Pointer to output sink_stream to be filled with decrypted data
  *
  * @return SUIT_PLAT_SUCCESS if success otherwise error code
  */
-suit_plat_err_t suit_decrypt_filter_get(struct stream_sink *dec_sink,
+suit_plat_err_t suit_decrypt_filter_get(struct stream_sink *in_sink,
 					struct suit_encryption_info *enc_info,
 					const suit_manifest_class_id_t *class_id,
-					struct stream_sink *enc_sink);
+					struct stream_sink *out_sink);
 
 #ifdef __cplusplus
 }
diff --git a/subsys/suit/stream/stream_filters/src/suit_decrypt_filter.c b/subsys/suit/stream/stream_filters/src/suit_decrypt_filter.c
@@ -24,7 +24,7 @@ LOG_MODULE_REGISTER(suit_decrypt_filter, CONFIG_SUIT_LOG_LEVEL);
 struct decrypt_ctx {
 	mbedtls_svc_key_id_t cek_key_id;
 	psa_aead_operation_t operation;
-	struct stream_sink enc_sink;
+	struct stream_sink out_sink;
 	size_t tag_size;
 	size_t stored_tag_bytes;
 	uint8_t tag[PSA_AEAD_TAG_MAX_SIZE];
@@ -67,8 +67,8 @@ static suit_plat_err_t erase(void *ctx)
 		decrypt_ctx->stored_tag_bytes = 0;
 		memset(decrypt_ctx->tag, 0, sizeof(decrypt_ctx->tag));
 
-		if (decrypt_ctx->enc_sink.erase != NULL) {
-			res = decrypt_ctx->enc_sink.erase(decrypt_ctx->enc_sink.ctx);
+		if (decrypt_ctx->out_sink.erase != NULL) {
+			res = decrypt_ctx->out_sink.erase(decrypt_ctx->out_sink.ctx);
 		}
 	} else {
 		res = SUIT_PLAT_ERR_INVAL;
@@ -131,7 +131,7 @@ static suit_plat_err_t write(void *ctx, const uint8_t *buf, size_t size)
 			goto cleanup;
 		}
 
-		err = decrypt_ctx->enc_sink.write(decrypt_ctx->enc_sink.ctx, decrypted_buf,
+		err = decrypt_ctx->out_sink.write(decrypt_ctx->out_sink.ctx, decrypted_buf,
 						  decrypted_len);
 
 		if (err != SUIT_PLAT_SUCCESS) {
@@ -195,13 +195,15 @@ static suit_plat_err_t flush(void *ctx)
 		} else {
 			LOG_INF("Firmware decryption successful");
 
-			/* Using enc_sink without a write API is blocked by the filter constructor.
+			/* Using out_sink without a write API is blocked by the filter constructor.
 			 */
 			if (decrypted_len > 0) {
-				res = decrypt_ctx->enc_sink.write(decrypt_ctx->enc_sink.ctx,
+				res = decrypt_ctx->out_sink.write(decrypt_ctx->out_sink.ctx,
 								  decrypted_buf, decrypted_len);
 				if (res != SUIT_PLAT_SUCCESS) {
 					LOG_ERR("Failed to write decrypted data: %d", res);
+					/* Revert all the changes so that no decrypted data remains */
+					erase(decrypt_ctx);
 				}
 			}
 		}
@@ -236,16 +238,16 @@ static suit_plat_err_t release(void *ctx)
 
 	suit_plat_err_t res = flush(ctx);
 
-	if (decrypt_ctx->enc_sink.release != NULL) {
+	if (decrypt_ctx->out_sink.release != NULL) {
 		suit_plat_err_t release_ret =
-			decrypt_ctx->enc_sink.release(decrypt_ctx->enc_sink.ctx);
+			decrypt_ctx->out_sink.release(decrypt_ctx->out_sink.ctx);
 
 		if (res == SUIT_SUCCESS) {
 			res = release_ret;
 		}
 	}
 
-	zeroize(&decrypt_ctx->enc_sink, sizeof(struct stream_sink));
+	zeroize(&decrypt_ctx->out_sink, sizeof(struct stream_sink));
 
 	decrypt_ctx->in_use = false;
 
@@ -261,8 +263,8 @@ static suit_plat_err_t used_storage(void *ctx, size_t *size)
 		return SUIT_PLAT_ERR_INVAL;
 	}
 
-	if (decrypt_ctx->enc_sink.used_storage != NULL) {
-		return decrypt_ctx->enc_sink.used_storage(decrypt_ctx->enc_sink.ctx, size);
+	if (decrypt_ctx->out_sink.used_storage != NULL) {
+		return decrypt_ctx->out_sink.used_storage(decrypt_ctx->out_sink.ctx, size);
 	}
 
 	return SUIT_PLAT_ERR_UNSUPPORTED;
@@ -344,10 +346,10 @@ static suit_plat_err_t get_psa_alg_info(enum suit_cose_alg cose_alg_id, psa_algo
 	return SUIT_PLAT_SUCCESS;
 }
 
-suit_plat_err_t suit_decrypt_filter_get(struct stream_sink *dec_sink,
+suit_plat_err_t suit_decrypt_filter_get(struct stream_sink *in_sink,
 					struct suit_encryption_info *enc_info,
 					const suit_manifest_class_id_t *class_id,
-					struct stream_sink *enc_sink)
+					struct stream_sink *out_sink)
 {
 	suit_plat_err_t ret = SUIT_PLAT_SUCCESS;
 
@@ -356,8 +358,8 @@ suit_plat_err_t suit_decrypt_filter_get(struct stream_sink *dec_sink,
 		return SUIT_PLAT_ERR_BUSY;
 	}
 
-	if ((enc_info == NULL) || (enc_sink == NULL) || (dec_sink == NULL) ||
-	    (enc_sink->write == NULL) || class_id == NULL) {
+	if ((enc_info == NULL) || (out_sink == NULL) || (in_sink == NULL) ||
+	    (out_sink->write == NULL) || class_id == NULL) {
 		return SUIT_PLAT_ERR_INVAL;
 	}
 
@@ -411,22 +413,22 @@ suit_plat_err_t suit_decrypt_filter_get(struct stream_sink *dec_sink,
 	}
 
 	ctx.stored_tag_bytes = 0;
-	memcpy(&ctx.enc_sink, enc_sink, sizeof(struct stream_sink));
+	memcpy(&ctx.out_sink, out_sink, sizeof(struct stream_sink));
 
-	dec_sink->ctx = &ctx;
+	in_sink->ctx = &ctx;
 
-	dec_sink->write = write;
-	dec_sink->erase = erase;
-	dec_sink->release = release;
-	dec_sink->flush = flush;
-	if (enc_sink->used_storage != NULL) {
-		dec_sink->used_storage = used_storage;
+	in_sink->write = write;
+	in_sink->erase = erase;
+	in_sink->release = release;
+	in_sink->flush = flush;
+	if (out_sink->used_storage != NULL) {
+		in_sink->used_storage = used_storage;
 	} else {
-		dec_sink->used_storage = NULL;
+		in_sink->used_storage = NULL;
 	}
 
 	/* Seeking is not possible on encrypted payload. */
-	dec_sink->seek = NULL;
+	in_sink->seek = NULL;
 
 	return SUIT_PLAT_SUCCESS;
 }
diff --git a/tests/subsys/suit/unit/suit_decrypt_filter/src/main.c b/tests/subsys/suit/unit/suit_decrypt_filter/src/main.c
@@ -71,6 +71,7 @@ struct suit_decrypt_filter_tests_fixture {
 	size_t decrypted_output_length;
 	psa_status_t aead_update_status;
 	psa_status_t aead_verify_status;
+	suit_plat_err_t ram_sink_write_status;
 
 	// ram_sink interfaces call counters. Not really 'fixtures', but something 
 	// we verify as a side effect of test run. 
@@ -158,7 +159,7 @@ static suit_plat_err_t write_ram(void *ctx, const uint8_t *buf, size_t size)
 
 	tests_fixture.ram_sink_write_call_cnt++;
 
-	return SUIT_PLAT_SUCCESS;
+	return tests_fixture.ram_sink_write_status;
 }
 
 static suit_plat_err_t used_storage(void *ctx, size_t *size)
@@ -462,14 +463,6 @@ ZTEST_F(suit_decrypt_filter_tests, test_flush_verify_fail)
 			"Incorrect number of ram_sink write function calls");
 	zassert_equal(psa_aead_abort_fake.call_count, 1,
 			"Invalid number of calls to psa_aead_abort");
-	// zassert_equal(fixture->dec_sink.ctx->cek_key_id, 0,
-	// 		"Invalid cek_key_id of decrypt stream context");
-	// zassert_equal(fixture->dec_sink.ctx->operation, 0,
-	// 		"Invalid operation of decrypt stream context");
-	// zassert_equal(fixture->dec_sink.ctx->tag_size, 0,
-	// 		"Invalid tag_size of decrypt stream context");
-	// zassert_equal(fixture->dec_sink.ctx->tag_bytes, 0,
-	// 		"Invalid tag_bytes of decrypt stream context");
 }
 
 ZTEST_F(suit_decrypt_filter_tests, test_flush_happy_path)
@@ -506,12 +499,43 @@ ZTEST_F(suit_decrypt_filter_tests, test_flush_happy_path)
 			"Incorrect number of ram_sink write function calls");
 	zassert_equal(psa_aead_abort_fake.call_count, 0,
 			"Invalid number of calls to psa_aead_abort");
-	// zassert_equal(fixture->dec_sink.ctx->cek_key_id, 0,
-	// 		"Invalid cek_key_id of decrypt stream context");
-	// zassert_equal(fixture->dec_sink.ctx->operation, 0,
-	// 		"Invalid operation of decrypt stream context");
-	// zassert_equal(fixture->dec_sink.ctx->tag_size, 0,
-	// 		"Invalid tag_size of decrypt stream context");
-	// zassert_equal(fixture->dec_sink.ctx->tag_bytes, 0,
-	// 		"Invalid tag_bytes of decrypt stream context");
+}
+
+ZTEST_F(suit_decrypt_filter_tests, test_flush_ram_write_fail)
+{
+	struct suit_encryption_info enc_info = ENC_INFO_DEFAULT_INIT;
+
+	suit_plat_decode_key_id_fake.custom_fake = custom_suit_plat_decode_key_id;
+	suit_plat_err_t err = suit_decrypt_filter_get(&fixture->dec_sink, &enc_info, 
+											&sample_class_id, &fixture->ram_sink);
+
+	zassert_equal(err, SUIT_PLAT_SUCCESS,
+		    "Incorrect error code when getting decrypt filter");
+
+	psa_aead_update_fake.custom_fake = custom_psa_aead_update;
+	fixture->aead_update_status = PSA_SUCCESS;
+	fixture->decrypted_output_length = 100; // Anything greater than 0.
+
+	err = fixture->dec_sink.write(fixture->dec_sink.ctx, ciphertext_direct, 
+								sizeof(ciphertext_direct));
+	
+	zassert_equal(err, SUIT_PLAT_SUCCESS,
+			"Incorrect error code when calling filter write interface");
+
+	psa_aead_verify_fake.custom_fake = custom_psa_aead_verify;
+	fixture->aead_verify_status = PSA_SUCCESS;
+	fixture->decrypted_output_length = 100; // Anything greater than 0.
+	tests_fixture.ram_sink_write_status = SUIT_PLAT_ERR_NOMEM;
+
+	err = fixture->dec_sink.flush(fixture->dec_sink.ctx);
+
+	zassert_equal(err, SUIT_PLAT_ERR_NOMEM,
+			"Incorrect error code when calling filter flush interface");
+	zassert_equal(fixture->ram_sink_write_call_cnt, 2, // Called 2 times because we
+													   // also call write (< SINGLE_CHUNK_SIZE).
+			"Incorrect number of ram_sink write function calls");
+	zassert_equal(psa_aead_abort_fake.call_count, 0,
+			"Invalid number of calls to psa_aead_abort");
+	zassert_equal(fixture->ram_sink_erase_call_cnt, 1,
+			"Incorrect number of ram_sink erase function calls");
 }
