Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SUIT: Upgrade to draft-ietf-suit-manifest-09 #14436

Merged
merged 3 commits into from
Sep 24, 2020

Conversation

bergzand
Copy link
Member

@bergzand bergzand commented Jul 3, 2020

Contribution description

This PR upgrades the current manifest generator and parser to be ietf-v7ietf-v9 compliant. Everything under dist/tools/suit/suit-manifest-generator is imported from ARMmbed/suit-manifest-generator and will be migrated to a package when the spec is stabilized.

Testing procedure

  • tests/suit_manifest must pass
  • upgrades must be possible with the flow in examples/suit_update.

Issues/PRs references

None

@bergzand bergzand added Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation Area: tools Area: Supplementary tools Area: OTA Area: Over-the-air updates labels Jul 3, 2020
@benpicco benpicco added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Jul 3, 2020
@cgundogan cgundogan added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR and removed CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Jul 6, 2020
@bergzand bergzand changed the title SUIT: Upgrade to draft-ietf-suit-manifest-07 SUIT: Upgrade to draft-ietf-suit-manifest-09 Jul 15, 2020
@bergzand
Copy link
Member Author

Updated to be draft-ietf-suit-manifest-09 compatible.

@benpicco
Copy link
Contributor

Looks like we'll need pyhsslms on Murdock now

@bergzand
Copy link
Member Author

Looks like we'll need pyhsslms on Murdock now

I've stripped the hss-lms key functionality from the tool for now.

@benpicco
Copy link
Contributor

Well that's also a solution 😄
Should we open a PR in riotdocker so this can eventually be added back?

@bergzand
Copy link
Member Author

Well that's also a solution smile

It's not like there is any use in having support for it in auxiliary tooling without support for HSS-LMS in RIOT itself.

Should we open a PR in riotdocker so this can eventually be added back?

That's probably the best long-term solution.

Copy link
Contributor

@benpicco benpicco left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Travis has some complaints about missing documentation to some public functions.

@@ -144,6 +144,8 @@ The `suit-tool` supports three sub-commands:
* `create` generates a new manifest.
* `sign` signs a manifest.
* `parse` parses an existing manifest into cbor-debug or a json representation.
* `keygen` Create a signing key. Not for production use.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is that a private key? And why not for production use?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ahh, crap, this was not supposed to be reviewed, everything under dist/tools/suit/suit-manifest-generator is imported from ARMmbed/suit-manifest-generator and will be migrated to a package when the spec is stabilized.
I'd rather not start fixing nitpicks in "external" code and only fix the bare minimum required for integration.

sys/suit/handlers_command_seq.c Outdated Show resolved Hide resolved
sys/suit/handlers_command_seq.c Show resolved Hide resolved
@benpicco
Copy link
Contributor

Murdock will need cbor2 still.

@benpicco benpicco added State: waiting for CI update State: The PR requires an Update to CI to be performed first CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR and removed CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Jul 27, 2020
@benpicco
Copy link
Contributor

@kaspar030 the workers will need an update

@benpicco benpicco removed the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Aug 10, 2020
@bergzand bergzand added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR and removed State: waiting for CI update State: The PR requires an Update to CI to be performed first CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Sep 17, 2020
@emmanuelsearch
Copy link
Member

emmanuelsearch commented Sep 23, 2020

@benpicco @fjmolinas @kaspar030 good to go? Squashing time?

sys/include/suit.h Outdated Show resolved Hide resolved
Copy link
Contributor

@fjmolinas fjmolinas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see that now you can eventually have more than one component, can test/suit_manifest be updated to test for that?

sys/include/suit.h Show resolved Hide resolved
sys/include/suit/handlers.h Outdated Show resolved Hide resolved
sys/suit/handlers_common.c Show resolved Hide resolved
@fjmolinas
Copy link
Contributor

fjmolinas commented Sep 24, 2020

Now the update starts but digest validation is failing, any ideas?

riotboot_flashwrite: initializing update to target slot 1
Fetching firmware |█████████████████████████| 100%
Verifying image digest
suit_parse() failed. res=-4
Timeout in expect script at "return child.expect([r"Fetching firmware \|[█ ]+\|\s+\d+\%"," (examples/suit_update/tests/01-run.py:70)

Also I don't know if I ended up making this comment or if it got lost but can the tests/suit_manifest be adapted to test for multiple components?

@bergzand
Copy link
Member Author

Also I don't know if I ended up making this comment or if it got lost but can the tests/suit_manifest be adapted to test for multiple components?

Had this around somewhere from a previous hackathon. See 7da04ac

@bergzand
Copy link
Member Author

bergzand commented Sep 24, 2020

Now the update starts but digest validation is failing, any ideas?

Yup, see ARMmbed/suit-manifest-generator#42, included the fix in our clone of that repo.

@fjmolinas
Copy link
Contributor

Its working for me now! Please squash @bergzand!

ifconfig
Iface  4  HWaddr: 4A:D4:58:2E:EB:63
          L2-PDU:1500 MTU:1500  HL:64  RTR
          Source address length: 6
          Link type: wired
          inet6 addr: fe80::48d4:58ff:fe2e:eb63  scope: link  VAL
pinging node...
PING fe80::48d4:58ff:fe2e:eb63%riot0(fe80::48d4:58ff:fe2e:eb63%riot0) 56 data bytes

--- fe80::48d4:58ff:fe2e:eb63%riot0 ping statistics ---
1 packets transmitted, 1 received, 0% packet loss, time 0ms
rtt min/avg/max/mdev = 41.168/41.168/41.168/0.000 ms
pinging node succeeded.
TEST PASSED

Copy link
Contributor

@fjmolinas fjmolinas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK.

@bergzand
Copy link
Member Author

Squashed!

@benpicco
Copy link
Contributor

Murdock is not happy

@benpicco benpicco added State: waiting for CI update State: The PR requires an Update to CI to be performed first and removed State: waiting for CI update State: The PR requires an Update to CI to be performed first labels Sep 24, 2020
@fjmolinas
Copy link
Contributor

Go!

@fjmolinas fjmolinas merged commit ad9e35c into RIOT-OS:master Sep 24, 2020
@bergzand bergzand deleted the pr/suit/ietf_v7 branch September 24, 2020 20:19
@bergzand
Copy link
Member Author

Thanks for testing and reviewing!

@emmanuelsearch
Copy link
Member

@bergzand yay!

@benpicco
Copy link
Contributor

benpicco commented Sep 26, 2020

With this I now get Segmentation fault (core dumped) when trying to build tests/suit_manifest on Ubuntu 20.04 (Python 3.8.2)

The crasher is python3 /home/benpicco/dev/RIOT/dist/tools/suit/suit-manifest-generator/bin/suit-tool

Never mind, updating Python fixed it (literally just running sudo apt full-upgrade)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: OTA Area: Over-the-air updates Area: tools Area: Supplementary tools CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants