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

HSM support for ECC Public key operations #6858

Merged
merged 2 commits into from
May 20, 2021
Merged

HSM support for ECC Public key operations #6858

merged 2 commits into from
May 20, 2021

Conversation

sujaygkulkarni-nxp
Copy link
Contributor

Problem
HSM support needs to be added for ECC Public key crypto operations

Summary of Changes
HSM support for ECC Public key crypto operations (Msg Verify and Hash Verify)

@boring-cyborg boring-cyborg bot added the crypto label May 16, 2021
@restyled-io restyled-io bot mentioned this pull request May 16, 2021
Closed
@woody-apple
Copy link
Contributor

@balducci-apple @pan-apple ?

jmartinez-silabs
jmartinez-silabs approved these changes May 17, 2021
View reviewed changes
Copy link
Member

@jmartinez-silabs jmartinez-silabs left a comment

Choose a reason for hiding this comment

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

Code lgtm.
Just wondering why all this code is needed under src/crypto/hsm/nxp.
Is this an augment to mbedtls? Is it part in some way of nxp sdk?

@sujaygkulkarni-nxp
Copy link
Contributor Author

Code lgtm.
Just wondering why all this code is needed under src/crypto/hsm/nxp.
Is this an augment to mbedtls? Is it part in some way of nxp sdk?

  1. src/crypto/hsm folder can hold implementation for different hsm modules. The implementation in this pull request is specific to NXP hsm module.
  2. hsm implementation can be seen as alternative to mbedtls / openssl crypto implementation. Required crypto operations can be offloaded to hsm modules.

@mspang mspang requested a review from emargolis May 19, 2021 05:30
@andy31415
Copy link
Contributor

Could you add unit tests to validate the added code?

@sujaygkulkarni-nxp
Copy link
Contributor Author

Could you add unit tests to validate the added code?

CHIPCryptoPALTest already had the ECC sign/verify tests (modified to support ECC on HSM) which will validate the current implementation.

@andy31415 andy31415 merged commit dfb2285 into project-chip:master May 20, 2021
@sujaygkulkarni-nxp sujaygkulkarni-nxp deleted the feature/ecc-pub-hsm branch June 25, 2021 06:10
nikita-s-wrk pushed a commit to nikita-s-wrk/connectedhomeip that referenced this pull request Sep 23, 2021
@sujaygkulkarni-nxp @Jagadish-NXP
HSM support for ECC Public key operations (project-chip#6858)
b8c7b93 
* Added HSM support for ECC Public key crypto operations

* restyled

Co-authored-by: Jagadish B E <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jmartinez-silabs jmartinez-silabs jmartinez-silabs approved these changes

@jelderton jelderton jelderton approved these changes

@woody-apple woody-apple woody-apple approved these changes

@andy31415 andy31415 andy31415 approved these changes

@chrisdecenzo chrisdecenzo Awaiting requested review from chrisdecenzo

@LuDuda LuDuda Awaiting requested review from LuDuda

@mspang mspang Awaiting requested review from mspang

@pan-apple pan-apple Awaiting requested review from pan-apple

@saurabhst saurabhst Awaiting requested review from saurabhst

@emargolis emargolis Awaiting requested review from emargolis

Assignees
No one assigned
Labels
crypto review - approved
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@sujaygkulkarni-nxp @woody-apple @andy31415 @jelderton @jmartinez-silabs @Jagadish-NXP