[riot-notifications] [RIOT-OS/RIOT] SUIT: provide manifest validation module (#11118)

Koen Zandberg notifications at github.com
Thu Mar 7 13:07:41 CET 2019


Thanks for the quick feedback.

> As I understand it, the API to this module can reasonably be expected to remain stable, right? In particular, do we expect the vendor, device, and class IDs to be fields that remain in the manifest? I think so. So this would provide a stable API to higher layers which would abstract away changes in the different SUIT manifest versions? Also additions of new manifest formats obv.

Exactly, Looking at both the [binary manifest draft](https://tools.ietf.org/html/draft-pagel-suit-manifest-00) and the [CBOR based manifest]draft(https://tools.ietf.org/html/draft-moran-suit-manifest-03) I think we can reasonably expect that the ID's remain in UUID format.

One minor remark I have for the current API as proposed here is that I want to have a `suit_manifest_t`  typedeffed to `suit_cbor_manifest_t`. This way we can migrate to a different struct content in the future (one that allows for multiple types of manifests for example)

> re testing. Personally I think it would be better to unit test this module and provide stubs for the lower (eg CBOR) module. However another option could be to integration test this with the lower module (keeping in the CBOR parser-specific tests alongside it). What do you think? 

Maybe a somewhat similar structure as done with the crypto unittests is preferred here. Separate unittests for each manifest format and a high level one to test integration.

> Whatever the case, some tests are needed IMO

I've added a unittests testing the [conditions validation logic](https://github.com/RIOT-OS/RIOT/pull/11118/files#diff-fc4311b2da5c5f7150f696b240e0d82aR169) as this is IMHO the most error prone part of the code. Let me know if you think I should test other things too.
As described above, I can split this out to a generic `tests-suit.c` and move the tests from #10315 to `tests-suit_cbor.c`, all contained in `tests/unittests/tests-suit` or something along those lines.

On a separate note, I think it would also be good to extend this with a `suit_get_url()`.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/RIOT-OS/RIOT/pull/11118#issuecomment-470501566
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.riot-os.org/pipermail/notifications/attachments/20190307/f5625708/attachment.html>


More information about the notifications mailing list