[nrf fromlist] imgtool: Add pure signature support #342
Conversation
There was a problem hiding this comment.
This is not going to work, the compression one will be merged which will then cause a merge conflict and the only way to avoid that is to rebase this original PR once the compression PR is merged upstream, and david did not seem ok with that when I asked him to review it the other day "where are the mcuboot changes?", but irrespective this will have a merge conflict and will not be able to get merged
Conflicts are unavoidable then; the missing change is in draft now: mcu-tools/mcuboot#2080 |
de-nordic
force-pushed
the
img-pure
branch
3 times, most recently
from
5322612
to
5857311
Compare
scripts/imgtool/image.py
Outdated
| @@ -20,7 +20,15 @@ | |||
| Image signing and management. | |||
There was a problem hiding this comment.
commit 35c9291fcafafe8608722e0ec3c801178884f0ef does not exist
There was a problem hiding this comment.
Rebased on NCS and it seems that the additional commits, revert and bring new, are no longer needed, so all that is left is the commit with pure support.
There was a problem hiding this comment.
This cannot be applied until compression support is merged into upstream and your upstream PR is rebased and this cherry-picked again:
git cherry-pick e872745275fe6920494caf297b1ce996c59ea21f
Auto-merging scripts/imgtool/image.py
CONFLICT (content): Merge conflict in scripts/imgtool/image.py
Auto-merging scripts/imgtool/main.py
CONFLICT (content): Merge conflict in scripts/imgtool/main.py
error: could not apply e8727452... imgtool: Add pure signature support
hint: After resolving the conflicts, mark them with
hint: "git add/rm <pathspec>", then run
hint: "git cherry-pick --continue".
hint: You can instead skip this commit with "git cherry-pick --skip".
hint: To abort and get back to the state before "git cherry-pick",
hint: run "git cherry-pick --abort".
hint: Disable this message with "git config advice.mergeConflict false"
I will ping David to take a look today at the tech call later
|
@nordicjm Please take a look |
|
This needs someone that knows python to take a look, I have no idea what I'm looking at |
scripts/imgtool/image.py
Outdated
| @@ -203,11 +211,17 @@ def key_and_user_sha_to_alg_and_tlv(key, user_sha): | |||
| # If key is not None, then we have to filter hash to only allowed | |||
| allowed = None | |||
| try: | |||
| allowed = ALLOWED_KEY_SHA[type(key)] | |||
| if is_pure: | |||
There was a problem hiding this comment.
Nit: It is fine, but it is better to keep only single call between try..except. Code can look like so:
allowed_key_ssh = ALLOWED_PURE_KEY_SHA if is_pure else ALLOWED_KEY_SHA
try:
allowed = allowed_key_ssh[type(key)]
except KeyError:
...
scripts/imgtool/image.py
Outdated
| message = digest; | ||
| tlv.add(hash_tlv, digest) | ||
| self.image_hash = digest | ||
| if not is_pure: |
There was a problem hiding this comment.
Nit: I would replace if not is_pure: with:
if is_pure:
...
else:
...There was a problem hiding this comment.
This part is no longer there.
Thanks. I am generally working on re-factoring some of the code here, I mean unrelated to pure thing, but that will take me a moment to sort out, and I do not want to do it in this PR. |
|
@nordicjm We had received proper Python review there and approval. |
nvlsianpu
force-pushed
the
img-pure
branch
2 times, most recently
from
d4491da
to
5c29840
Compare
Adds PureEdDSA signature support. The change includes implementation of SIG_PURE TLV that, when present, indicates the signature that is present is Pure type. Upstream PR: mcu-tools/mcuboot#2063 Signed-off-by: Dominik Ermel <dominik.ermel@nordicsemi.no> Signed-off-by: Andrzej Puzdrowski <andrzej.puzdrowski@nordicsemi.no>
| tlv.add(hash_tlv, digest) | ||
| self.image_hash = digest |
There was a problem hiding this comment.
image_hash of this object is used externally.
Kept creating the field for external usage. Signed-off-by: Andrzej Puzdrowski <andrzej.puzdrowski@nordicsemi.no>
nvlsianpu
dismissed
nordicjm’s stale review
reviewed by others, The Reviewer claim that won't review Python.
Adds PureEdDSA signature support.
The change includes implementation of SIG_PURE TLV that, when present, indicates the signature that is present is Pure type.
Upstream PR: mcu-tools/mcuboot#2063
Rebased on top of: #344