sys/riotboot: add initial image digest verification

[kaspar030]

Contribution description

This PR adds image verification using sha256 to riotboot.

Testing procedure

As is, this code is never compiled. It has been split out from a large SUIT pr which will be opened soon. I suggest reviewing the code here and use the SUIT PR as testbed.

Issues/PRs references

#11818

[kaspar030]

(just realized that the .c is missing a license header and doxygen, thus marked WIP)

[kaspar030]

• added missing licence. Not WIP anymore.

[fjmolinas]

I tested the use case in #11818. As for all I can see the returned value is incorrect and the digest validation is failing.

I'm getting this:

sha256_digest:82015820a72ebb4ef82107d3de4b9586f0d18cd692aeb0c7cef27547be0a238f
digest:       a72ebb4ef82107d3de4b9586f0d18cd692aeb0c7cef27547be0a238fcaf99704

[fjmolinas]

Is this RIOTBOOT_MAGIC? If it is I would replace "RIOT" by RIOTBOOT_MAGIC, and 4 by sizeof(RIOTBOOT_MAGIC) to avoid magic numbers.

[fjmolinas]

hmm this can't be done since RIOTBOOT_MAGIC is a numerical value

[kaspar030]

yup. Maybe ("RIOT", 4) is clear enough for now?

[fjmolinas]

@kaspar030 I can't think of a workaround for this. Can you just add a comment like:

"Add RIOTBOOT_MAGIC since it isn't written into flash until riotboot_flashwrite_finnish()"

[kaspar030]

done, also added a comment explaining the "+4, -4".

[fjmolinas]

Cool, please squash

[fjmolinas]

Same as in upper comment.

[fjmolinas]

This return is wrong, should be:

memcmp(sha256_digest, digest, SHA256_DIGEST_LENGTH)

as it is now it returns 0 for invalid digests and in #11818 it is used as if 0 meant the verify happened OK.

[fjmolinas]

@kaspar030 testing on #11818 after implementing the requested changes I was still getting an error but this was not do to riotboot_flashwrite_verify_sha256, I was able to verify that the verified shah matched the generated shah in gen_manifest.py. The error was in the input digest which was including the difest id of 4 bytes. If the input digest is offset-ed by 4 bytes we get the same result, here is the diff:

diff --git a/sys/riotboot/flashwrite_verify_sha256.c b/sys/riotboot/flashwrite_verify_sha256.c
index f740ba676..ead09e695 100644
--- a/sys/riotboot/flashwrite_verify_sha256.c
+++ b/sys/riotboot/flashwrite_verify_sha256.c
@@ -47,5 +47,5 @@ int riotboot_flashwrite_verify_sha256(const uint8_t *sha256_digest, size_t img_l
     sha256_update(&sha256, img_start + 4, img_len - 4);
     sha256_final(&sha256, digest);
 
-    return memcmp(sha256_digest, digest, SHA256_DIGEST_LENGTH) == 0;
+    return memcmp(sha256_digest, digest, SHA256_DIGEST_LENGTH);
 }
diff --git a/sys/suit/v4/handlers.c b/sys/suit/v4/handlers.c
index 9029780bb..9f90f02e8 100644
--- a/sys/suit/v4/handlers.c
+++ b/sys/suit/v4/handlers.c
@@ -284,7 +284,7 @@ static int _dtv_fetch(suit_v4_manifest_t *manifest, int key, CborValue *_it)
         return res;
     }
 
-    res = riotboot_flashwrite_verify_sha256(digest, manifest->components[0].size, target_slot);
+    res = riotboot_flashwrite_verify_sha256(digest + 4, manifest->components[0].size, target_slot);
     if (res) {
         LOG_INFO("image verification failed\n");
         return res;

Current output:

input digest     : 8158208ceb8c0435a2413395352b19c5aee2fd157cc837541fecc12a824ccc49
verifiied digest: 8ceb8c0435a2413395352b19c5aee2fd157cc837541fecc12a824ccc49d06c52

Affter applying the diff:

input digest     : ba66634284b5fc220f433714f3bfb49ccde53b7200521c75f74d98099ce773e3
verifiied digest: ba66634284b5fc220f433714f3bfb49ccde53b7200521c75f74d98099ce773e3

If you address the return and the magic numbers we can move forward with this one and fix the issue in #11818.

[kaspar030]

return memcmp(sha256_digest, digest, SHA256_DIGEST_LENGTH);

Ok. I changed this to return memcmp(sha256_digest, digest, SHA256_DIGEST_LENGTH) != 0; to be in line with the documentation.

[kaspar030]

The error was in the input digest which was including the difest id of 4 bytes. If the input digest is offset-ed by 4 bytes we get the same result, here is the diff:

I've added the digest offset to #11818.

[fjmolinas]

I've tested using #11818 that the function outputs the correct shah and fails when it doesn't match. ACK, and lets wait for murdock to be green.

[kaspar030]

ACK, and lets wait for murdock to be green.

Note, Murdock does not currently build this code. It is only built as part of #11818.