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

Updated ECDSA Signature Representation in the CHIP Certificate. #7384

Conversation

emargolis
Copy link
Contributor

Problem

Currently SDK implements ECDSA signature in the CHIP Certificate as a TLV structure of R and S parameters:

ecdsa-signature => STRUCTURE [tag-order]
{
r [1] : OCTET STRING,
s [2] : OCTET STRING,
}

Recently, the spec was updated to represent ECDSA signature as Octet String:

ec-signature => OCTET STRING [ length (CHIP_CRYPTO_GROUP_SIZE_BYTES * 2) ]

Change overview

  • Added function to convert ASN.1 DER encoded Integers
  • Added functions to convert ASN.1 DER encoded ECDSA signature
  • Updated CHIP certificate structure to use new signature encoding
  • Updated unit tests

Testing

Unit tests

Ticket #7382

@boring-cyborg boring-cyborg bot added the crypto label Jun 4, 2021
@woody-apple
Copy link
Contributor

@todo
Copy link

todo bot commented Jun 4, 2021

Consider renaming these values to be closer to definisions in the spec:

// TODO: Consider renaming these values to be closer to definisions in the spec:
// CHIP_CRYPTO_GROUP_SIZE_BYTES
// CHIP_CRYPTO_PUBLIC_KEY_SIZE_BYTES
const size_t kP256_FE_Length = 32;
const size_t kP256_ECDSA_Signature_Length_Raw = (2 * kP256_FE_Length);
const size_t kP256_Point_Length = (2 * kP256_FE_Length + 1);
const size_t kSHA256_Hash_Length = 32;
const size_t kMax_ECDH_Secret_Length = kP256_FE_Length;
const size_t kMax_ECDSA_Signature_Length = 72;


This comment was generated by todo based on a TODO comment in 34bdbf0 in #7384. cc @emargolis.

@woody-apple
Copy link
Contributor

/rebase

@woody-apple woody-apple force-pushed the feature/emargolis/chip-cert-ec-signature branch from 34bdbf0 to 87e6a49 Compare June 5, 2021 00:16
@woody-apple
Copy link
Contributor

@andy31415 ?

@woody-apple
Copy link
Contributor

@saurabhst ?

src/credentials/CHIPCertFromX509.cpp Show resolved Hide resolved
src/credentials/CHIPCertToX509.cpp Show resolved Hide resolved
@emargolis emargolis force-pushed the feature/emargolis/chip-cert-ec-signature branch from 87e6a49 to 8e21a98 Compare June 8, 2021 00:31
@todo
Copy link

todo bot commented Jun 8, 2021

Review and consider replacing some data pointer/len pairs with ByteSpan and FixedByteSpan types.

// TODO: Review and consider replacing some data pointer/len pairs with ByteSpan and FixedByteSpan types.
ByteSpan mCertificate; /**< Original raw buffer data. */
ChipDN mSubjectDN; /**< Certificate Subject DN. */
ChipDN mIssuerDN; /**< Certificate Issuer DN. */


This comment was generated by todo based on a TODO comment in 8e21a98 in #7384. cc @emargolis.

@github-actions
Copy link

github-actions bot commented Jun 8, 2021

Size increase report for "nrfconnect-example-build" from 2423a26

File Section File VM
chip-lock.elf device_handles 4 4
chip-lock.elf text -20 -20
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_str,0,37
.debug_abbrev,0,20
.debug_line,0,4
.debug_loc,0,-1
.debug_info,0,-308

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

sections,vmsize,filesize
.debug_loc,0,1403
.debug_str,0,433
.debug_ranges,0,216
.strtab,0,199
.debug_frame,0,124
.debug_abbrev,0,109
.symtab,0,96
.debug_line,0,43
.debug_aranges,0,32
device_handles,4,4
.shstrtab,0,1
text,-20,-20
.debug_info,0,-104


@github-actions
Copy link

github-actions bot commented Jun 8, 2021

Size increase report for "esp32-example-build" from 2423a26

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

Files found only in the build output:
    report.csv

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

sections,vmsize,filesize
.debug_loc,0,465
.debug_str,0,433
.debug_line,0,311
.debug_ranges,0,232
.strtab,0,199
.debug_frame,0,96
.symtab,0,48
.debug_aranges,0,32
.shstrtab,0,1
.flash.text,-8,-8
.debug_info,0,-585
.debug_abbrev,0,-800

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

sections,vmsize,filesize


@github-actions
Copy link

github-actions bot commented Jun 8, 2021

Size increase report for "gn_qpg6100-example-build" from 2423a26

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

Files found only in the build output:
    report.csv

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

sections,vmsize,filesize
.debug_loc,0,1407
.debug_str,0,433
.debug_ranges,0,208
.strtab,0,199
.symtab,0,144
.debug_frame,0,120
.debug_line,0,57
.debug_aranges,0,32
[Unmapped],0,16
.shstrtab,0,1
.text,-16,-16
.debug_info,0,-26
.debug_abbrev,0,-503

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'


@woody-apple woody-apple merged commit 85eb604 into project-chip:master Jun 8, 2021
nikita-s-wrk pushed a commit to nikita-s-wrk/connectedhomeip that referenced this pull request Sep 23, 2021
@emargolis emargolis deleted the feature/emargolis/chip-cert-ec-signature branch December 14, 2021 10:07
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.

5 participants