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

CSR support in crypto HSM #7897

Conversation

sujaygkulkarni-nxp
Copy link
Contributor

@sujaygkulkarni-nxp sujaygkulkarni-nxp commented Jun 25, 2021

Problem

No CSR support in HSM layer
No HSM support for CASE protocol

Change overview

  • Added CSR support in HSM layer
  • Using P256 key from HSM for CASE operational key
  • Using P256 key from HSM for CASE ephermel key
  • PBKDF using HSM is disabled by default

Testing

  • Tested manually on k32 and Linux
  • Tested CSR functionality using CHIPCryptoPALTest

memcpy((void *) &data_to_hash[buffer_index], pubkey, pubKeyLen);

// Copy subject (in the current implementation only organisation name info is added) and organisation OID
buffer_index = buffer_index - (1 + 1 + sizeof(SUBJECT_STR) - 1);
Copy link
Member

@jmartinez-silabs jmartinez-silabs Jul 2, 2021

Choose a reason for hiding this comment

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

I don't really like how the buffer_index is decrease/calculated through out this function. It seems error prone.
The use of constants could help make this clearer
buffer_index -= kTlvHeader

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor Author

@sujaygkulkarni-nxp sujaygkulkarni-nxp Jul 5, 2021

Choose a reason for hiding this comment

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

Sure. Updated the code by using kTlvHeader,
Also before accessing the buffer using buffer index, check is added as,
"VerifyOrExit(buffer_index > 0, error = CHIP_ERROR_INTERNAL);"

@github-actions
Copy link

github-actions bot commented Jul 6, 2021

Size increase report for "esp32-example-build" from edd134c

File Section File VM
chip-shell.elf .flash.text -4 -4
chip-temperature-measurement-app.elf .flash.text 60 60
Full report output
BLOAT REPORT

Files found only in the build output:
    report.csv

Comparing ./master_artifact/chip-lock-app.elf and ./pull_artifact/chip-lock-app.elf:

sections,vmsize,filesize
.debug_abbrev,0,700
.debug_info,0,273
.debug_line,0,4
.debug_loc,0,3

Comparing ./master_artifact/chip-all-clusters-app.elf and ./pull_artifact/chip-all-clusters-app.elf:

sections,vmsize,filesize
.debug_abbrev,0,1773
.debug_info,0,352
.debug_line,0,4
.debug_loc,0,-5

Comparing ./master_artifact/chip-shell.elf and ./pull_artifact/chip-shell.elf:

sections,vmsize,filesize
.debug_abbrev,0,292
.debug_info,0,56
.debug_line,0,4
[Unmapped],0,4
.debug_loc,0,-4
.flash.text,-4,-4

Comparing ./master_artifact/chip-temperature-measurement-app.elf and ./pull_artifact/chip-temperature-measurement-app.elf:

sections,vmsize,filesize
.debug_abbrev,0,1193
.debug_info,0,242
.flash.text,60,60
.debug_loc,0,5
.debug_line,0,4
[Unmapped],0,-60

Comparing ./master_artifact/chip-persistent-storage.elf and ./pull_artifact/chip-persistent-storage.elf:

sections,vmsize,filesize

Comparing ./master_artifact/chip-pigweed-app.elf and ./pull_artifact/chip-pigweed-app.elf:

sections,vmsize,filesize


@Jagadish-NXP Jagadish-NXP force-pushed the feature/CSR-support-in-crypto-hsm branch from 06e89fd to cedbf09 Compare July 7, 2021 03:39
@andy31415 andy31415 merged commit 1f44fdb into project-chip:master Jul 7, 2021
@sujaygkulkarni-nxp sujaygkulkarni-nxp deleted the feature/CSR-support-in-crypto-hsm branch July 8, 2021 16:52
@bzbarsky-apple
Copy link
Contributor

@sujaygkulkarni-nxp is ENABLE_HSM_CASE_EPHERMAL_KEY supposed to be ENABLE_HSM_CASE_EPHEMERAL_KEY?

andy31415 pushed a commit that referenced this pull request Jul 16, 2021
* Added CSR support in HSM

* updated keyid

* added support for linux in simw build file

* build fix

* Added logs

* build fix

* Using HSM for ops key

* Using HSM for OPS key

* Using HSM for ops key

* added logs

* logs

* restyled

* updated readme

* updated readme

* restyled

* using kTlvHeader for buffer index update

* updating buffer index using kTlvHeader

* restyled

* Trigger Build

* Trigger Build

* Trigger Build

Co-authored-by: Jagadish B E <[email protected]>
Co-authored-by: Jagadish-NXP <[email protected]>
hank820 pushed a commit to hank820/connectedhomeip that referenced this pull request Sep 15, 2021
* Added CSR support in HSM

* updated keyid

* added support for linux in simw build file

* build fix

* Added logs

* build fix

* Using HSM for ops key

* Using HSM for OPS key

* Using HSM for ops key

* added logs

* logs

* restyled

* updated readme

* updated readme

* restyled

* using kTlvHeader for buffer index update

* updating buffer index using kTlvHeader

* restyled

* Trigger Build

* Trigger Build

* Trigger Build

Co-authored-by: Jagadish B E <[email protected]>
Co-authored-by: Jagadish-NXP <[email protected]>
nikita-s-wrk pushed a commit to nikita-s-wrk/connectedhomeip that referenced this pull request Sep 23, 2021
* Added CSR support in HSM

* updated keyid

* added support for linux in simw build file

* build fix

* Added logs

* build fix

* Using HSM for ops key

* Using HSM for OPS key

* Using HSM for ops key

* added logs

* logs

* restyled

* updated readme

* updated readme

* restyled

* using kTlvHeader for buffer index update

* updating buffer index using kTlvHeader

* restyled

* Trigger Build

* Trigger Build

* Trigger Build

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

Successfully merging this pull request may close these issues.

9 participants