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

Update operational credentials code to match the specifications #7500

Merged
merged 9 commits into from
Jun 11, 2021

Conversation

pan-apple
Copy link
Contributor

Problem

Need updates to operational credentials provisioning code to match the latest specifications.

Change overview

  1. Remove trusted-root cluster
  2. Move commands from trusted-root cluster to operational-credentials-cluster
  3. Regenerate the ZAP code
  4. Update provisioning flow to first provision root certificate, followed by operational certificates
  5. Add support for provisioning ICA certificates, if available. Use new API to generate certificate array and send the array to the device.

Testing

This change impacts the device commissioning flow.
Tested commissioning using Python controller app, chip-tool, and iOS CHIPTool app.

@woody-apple
Copy link
Contributor

@pan-apple
Copy link
Contributor Author

rebased

@pan-apple
Copy link
Contributor Author

rebased

@github-actions
Copy link

Size increase report for "nrfconnect-example-build" from d29749b

File Section File VM
chip-lock.elf device_handles 8 8
chip-lock.elf rodata -32 -32
chip-lock.elf text -408 -408
Full report output
BLOAT REPORT

Files found only in the build output:
    report.csv

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

sections,vmsize,filesize
.debug_info,0,17
.debug_line,0,-1

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

sections,vmsize,filesize
device_handles,8,8
.shstrtab,0,-1
rodata,-32,-32
.debug_aranges,0,-40
.symtab,0,-96
.debug_frame,0,-132
.strtab,0,-291
.debug_ranges,0,-328
text,-408,-408
.debug_str,0,-415
.debug_loc,0,-1501
.debug_abbrev,0,-5233
.debug_line,0,-7712
.debug_info,0,-172703


@github-actions
Copy link

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

File Section File VM
chip-all-clusters-app.elf .flash.rodata -80 -80
chip-all-clusters-app.elf .flash.text -436 -436
Full report output
BLOAT REPORT

Files found only in the build output:
    report.csv

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

sections,vmsize,filesize

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

sections,vmsize,filesize
[Unmapped],0,80
.debug_aranges,0,-40
.xt.prop._ZN32OpCredsAdminPairingTableDelegate25OnAdminPersistedToStorageEPN4chip9Transport16AdminPairingInfoE,0,-40
.xt.prop._ZN4chip8Platform4Impl22ScopedMemoryBufferBaseINS1_24PlatformMemoryManagementEE5AllocEj,0,-76
.flash.rodata,-80,-80
.debug_frame,0,-88
.symtab,0,-96
.shstrtab,0,-193
.debug_ranges,0,-288
.strtab,0,-291
.debug_str,0,-410
.flash.text,-436,-436
.debug_loc,0,-1649
.debug_abbrev,0,-4754
.debug_line,0,-8066
.debug_info,0,-83533


@github-actions
Copy link

Size increase report for "gn_qpg6100-example-build" from d29749b

File Section File VM
chip-qpg6100-lighting-example.out .text -432 -432
Full report output
BLOAT REPORT

Files found only in the build output:
    report.csv

Comparing ./master_artifact/chip-qpg6100-lighting-example.out.map and ./pull_artifact/chip-qpg6100-lighting-example.out.map:

BLOAT EXECUTION FAILED WITH CODE 1:
bloaty: unknown file type for file './pull_artifact/chip-qpg6100-lighting-example.out.map'

Comparing ./master_artifact/chip-qpg6100-lighting-example.out and ./pull_artifact/chip-qpg6100-lighting-example.out:

sections,vmsize,filesize
[Unmapped],0,432
.shstrtab,0,-1
.debug_aranges,0,-40
.symtab,0,-112
.debug_frame,0,-132
.debug_ranges,0,-280
.strtab,0,-291
.text,-432,-432
.debug_str,0,-443
.debug_loc,0,-1424
.debug_abbrev,0,-4391
.debug_line,0,-6968
.debug_info,0,-64202


Copy link
Contributor

@tcarmelveilleux tcarmelveilleux left a comment

Choose a reason for hiding this comment

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

There are many steps that are starting to line-up to spec, so this is great to see!

However, note that the operational certificate provisioning is still not "to spec", due to the NodeID being locally generated, and the full CSR not being passed along with signature and DA keys such that a CA could actually trust it.

@pan-apple
Copy link
Contributor Author

There are many steps that are starting to line-up to spec, so this is great to see!

However, note that the operational certificate provisioning is still not "to spec", due to the NodeID being locally generated, and the full CSR not being passed along with signature and DA keys such that a CA could actually trust it.

The node ID is assigned by the commissioner app right now. I don't think it's being locally generated.
DA keys and signature verification depend on device attestation implementation. I understand that attestation happens before operational credentials provisioning as per spec. But until that feature is implemented there's no way to check the signature of OpCSR.

@bzbarsky-apple bzbarsky-apple merged commit e7ff132 into project-chip:master Jun 11, 2021
@pan-apple pan-apple deleted the op-creds-spec-updates branch June 11, 2021 15:54
nikita-s-wrk pushed a commit to nikita-s-wrk/connectedhomeip that referenced this pull request Sep 23, 2021
…ect-chip#7500)

* Move trusted root certs to operational credentials cluster

* update zap files

* Regenerate zap code and move trusted root code to op cred cluster

* Send root cert before sending operational certificates

* Add support for ICA certificates

* fix telink build

* fix build after rebase

* add comment to describe why 2 certs are being stored

* address review comments
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.

7 participants