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.

Sign up for GitHub

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

Jump to bottom

boot: Add MCUBOOT_HW_KEY support for image encryption #1722

Merged
merged 2 commits into from
Jun 12, 2024
Merged

boot: Add MCUBOOT_HW_KEY support for image encryption #1722

merged 2 commits into from
Jun 12, 2024

Conversation

DineshDK03
Copy link
Contributor

Currently encryption supports only private key embed in mcuboot itself. To support MCUBOOT_HW_KEY for image encryption boot_retrieve_private_key() hook is added.

This hook helps retrieving private key from trusted sources like OTP, TPM.

d3zd3z
d3zd3z requested changes Jun 8, 2023
View reviewed changes
Copy link
Member

@d3zd3z d3zd3z left a comment

Choose a reason for hiding this comment

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

I like the idea of this change. I'd like to see if we can do this without introducing more ifdefs throughout the code. Perhaps we make the main code so that it always calls this function, and then, depending on the feature (ifdefs around whole functions are better than ones sprinkled through the code), we could just return a copy of the compiled in one. This could be conditionalized out if a given developer wishes to provide their own implementation.

boot/bootutil/src/encrypted.c Outdated Show resolved Hide resolved
@DineshDK03 DineshDK03 mentioned this pull request Jun 9, 2023
Closed
@de-nordic de-nordic added the crypto Encryption support label Jun 14, 2023
@DineshDK03 DineshDK03 requested a review from d3zd3z June 16, 2023 04:39
@DineshDK03
Copy link
Contributor Author

@d3zd3z please revisit.

@DineshDK03
Copy link
Contributor Author

@d3zd3z Ping

@DineshDK03 DineshDK03 requested a review from mingulov September 21, 2023 13:02
@DineshDK03 DineshDK03 requested a review from mingulov October 3, 2023 04:23
@jimmyzumthurm
Copy link

Any update on this ? We have similar solution currently in our software by patching MCUboot, but would highly appreciate if that would be officially available

@DineshDK03
Copy link
Contributor Author

Any update on this ? We have similar solution currently in our software by patching MCUboot, but would highly appreciate if that would be officially available

@jimmyzumthurm. I have already addressed the comments from @mingulov and @d3zd3z and waiting for their acceptance to merge this PR.

@mingulov
Copy link
Contributor

Any update on this ? We have similar solution currently in our software by patching MCUboot, but would highly appreciate if that would be officially available

@jimmyzumthurm. I have already addressed the comments from @mingulov and @d3zd3z and waiting for their acceptance to merge this PR.

I am not an official approver / maintainer for MCUboot, so unfortunately my acceptance would not help anyhow.

@DineshDK03
Copy link
Contributor Author

@d3zd3z @de-nordic @nordicjm please have a look.

@DineshDK03
Copy link
Contributor Author

@d3zd3z ping

@nordicjm
Copy link
Collaborator

@sigvartmh

@DineshDK03
Copy link
Contributor Author

@d3zd3z @sigvartmh @nordicjm @de-nordic please have a look, if all is good, kindly merge this PR.

d3zd3z
d3zd3z requested changes Dec 12, 2023
View reviewed changes
Copy link
Member

@d3zd3z d3zd3z left a comment

Choose a reason for hiding this comment

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

Other than the whitespace issue, this looks good now.

boot/bootutil/src/encrypted.c Outdated Show resolved Hide resolved
@DineshDK03
Copy link
Contributor Author

@d3zd3z Thanks for the review and fixed all the whitespace issues now.

@d3zd3z d3zd3z self-requested a review December 14, 2023 15:23
@DineshDK03
Copy link
Contributor Author

@d3zd3z pushed changes to fix the CI. please approve to run the workflow jobs.

@DineshDK03 DineshDK03 requested a review from d3zd3z December 26, 2023 05:26
d3zd3z
d3zd3z requested changes Feb 8, 2024
View reviewed changes
boot/mbed/app_enc_keys.c Outdated Show resolved Hide resolved
@DineshDK03 DineshDK03 requested a review from d3zd3z March 3, 2024 13:02
@DineshDK03
Copy link
Contributor Author

@d3zd3z please have a look, all the comments addressed and waiting to be merged.

@davidvincze davidvincze requested review from davidvincze and removed request for mingulov May 23, 2024 11:39
davidvincze
davidvincze reviewed May 23, 2024
View reviewed changes
boot/bootutil/include/bootutil/enc_key.h Outdated Show resolved Hide resolved
boot/cypress/MCUBootApp/keys.c Outdated Show resolved Hide resolved
boot/mbed/app_enc_keys.c Outdated Show resolved Hide resolved
boot/zephyr/keys.c Outdated Show resolved Hide resolved
ci/mynewt_keys/enc_kw/src/keys.c Outdated Show resolved Hide resolved
ci/mynewt_keys/enc_rsa/src/keys.c Outdated Show resolved Hide resolved
sim/mcuboot-sys/csupport/keys.c Outdated Show resolved Hide resolved
@davidvincze
Copy link
Collaborator

davidvincze commented May 23, 2024 •
edited
Loading

@DineshDK03 please add a top commit that adds a release note snippet for this feature.
(https://github.com/mcu-tools/mcuboot/blob/main/docs/SubmittingPatches.md#release-notes)
Thank you!

@DineshDK03 DineshDK03 requested a review from davidvincze May 28, 2024 15:16
davidvincze
davidvincze reviewed May 29, 2024
View reviewed changes
sim/mcuboot-sys/csupport/keys.c Outdated Show resolved Hide resolved
boot/bootutil/include/bootutil/enc_key.h Outdated Show resolved Hide resolved
DineshDK03 added 2 commits May 29, 2024 19:47
@DineshDK03
boot: Add MCUBOOT_HW_KEY support for image encryption
7d8664c 
Currently encryption supports only private key embed
in mcuboot itself. To support MCUBOOT_HW_KEY for image
encryption boot_retrieve_private_key() hook is added.

This hook helps retrieving private key from trusted
sources like OTP, TPM.

Signed-off-by: Dinesh Kumar K <[email protected]>
@DineshDK03
docs: Add release note snippet on MCUBOOT_HW_KEY for image encryption
5697293 
Signed-off-by: Dinesh Kumar K <[email protected]>
@DineshDK03 DineshDK03 requested a review from davidvincze May 29, 2024 14:19
@davidvincze davidvincze requested a review from mingulov May 30, 2024 09:34
@davidvincze
Copy link
Collaborator

@d3zd3z any other comments on this PR or is it good to go?

@d3zd3z d3zd3z merged commit 03171ff into mcu-tools:main Jun 12, 2024
58 checks passed
@davidvincze davidvincze mentioned this pull request Jul 26, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@davidvincze davidvincze davidvincze approved these changes

@d3zd3z d3zd3z d3zd3z approved these changes

@mingulov mingulov Awaiting requested review from mingulov

Assignees

@d3zd3z d3zd3z

Labels
crypto Encryption support
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@DineshDK03 @jimmyzumthurm @mingulov @nordicjm @davidvincze @d3zd3z @de-nordic