validate-update firmware checksum - #767
Conversation
There was a problem hiding this comment.
Thanks! Looks good, just some small remarks.
By the way, you should be able to update the nitrokey dependency in pyproject.toml so that it uses the unreleased changes, see https://github.com/Nitrokey/pynitrokey/blob/master/docs/developer-guide.md#patching-the-nitrokey-sdk (and then poetry lock).
| @click.command() | ||
| @click.argument("image", type=click.Path(exists=True, dir_okay=False)) | ||
| @click.option( | ||
| "--verify-from-source", |
There was a problem hiding this comment.
I would call it --verify-from-release or --verify-from-github because we still verify it against a pre-compiled binary, not against the source code.
| print(f" checksum: {metadata.inner_checksum.hex()}") | ||
|
|
||
| if not metadata.signed_by_nitrokey: | ||
| raise CliException("Verification failed for {variant.value}") |
There was a problem hiding this comment.
Please set a flag and raise the exception only after all variants have been checked.
There was a problem hiding this comment.
Changed to using an array to hold all errors
| bar = DownloadProgressBar(desc=update.tag) | ||
| checksum = update.checksum(callback=bar.update) | ||
| if not checksum == metadata.inner_checksum: | ||
| raise CliException("Does not match checksum") |
There was a problem hiding this comment.
Fixed as above
| release = firmware_repository.get_release_or_latest(metadata.version) | ||
| update = updates.get_firmware_update(ctx.model, release) | ||
| update.url = update.url.rsplit("/", 1)[0] | ||
| device = "nk3am" if variant == Variant.NRF52 else "nk3xn" |
There was a problem hiding this comment.
I think you also need to check the model here because this command could also be used with the NKPK.
There was a problem hiding this comment.
Fixed. Also the ihex in NKPK firmware did not have a section corresponding to bytes 845C to 845F. Hence this program was filling that with 0x00 while the original bin had 0xFF, hence failing the check. I modified it to use 0xFF for unspecified sections, now passing
2 participants
Run against the SDK in the PR to test