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

Do not load duplicate certs in ChipCertificateSet #7115

Merged
merged 3 commits into from
May 27, 2021

Conversation

pan-apple
Copy link
Contributor

Problem

The certificate loading fails after a few tries when the same cert is loaded repeatedly.

Change overview

The ChipCertificateSet::LoadCert doesn't check if the cert (being loaded) is already present in the certificate set. It parses it and adds it to the set. The current limit of certs in a set is set to 4 (3 in tests). This PR adds a new method to ChipCertificateData to compare two certificates. The LoadCert() loads the certificate in a temporary ChipCertificateData instance, and compares it to the existing certs. If the cert is not found, it gets copied to one of the available slot in the set. If the set has no available slot, the function will return error.

Testing

Added a new unit test TestChipCert_LoadDuplicateCerts to test this behavior.
The rest of TestChipCert test suite ensures no existing functionality is broken.

@github-actions
Copy link

Size increase report for "gn_qpg6100-example-build" from 764df83

File Section File VM
chip-qpg6100-lighting-example.out .text 336 336
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
.debug_info,0,3102
.debug_loc,0,597
.text,336,336
.debug_str,0,68
.debug_line,0,62
.strtab,0,58
.debug_ranges,0,48
.symtab,0,32
.debug_frame,0,28
.debug_abbrev,0,23
.debug_aranges,0,8
.shstrtab,0,-2
[Unmapped],0,-336


@github-actions
Copy link

Size increase report for "nrfconnect-example-build" from 764df83

File Section File VM
chip-lock.elf text 324 324
chip-lock.elf device_handles -4 -4
Full report output
BLOAT REPORT

Files found only in the build output:
    report.csv

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

sections,vmsize,filesize
.debug_info,0,3160
.debug_loc,0,706
text,324,324
.debug_line,0,87
.debug_str,0,62
.strtab,0,58
.debug_ranges,0,56
.debug_abbrev,0,37
.symtab,0,32
.debug_frame,0,28
.debug_aranges,0,8
.shstrtab,0,-2
device_handles,-4,-4

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

sections,vmsize,filesize
.debug_info,0,504
.debug_str,0,58
.debug_loc,0,-66


@github-actions
Copy link

Size increase report for "esp32-example-build" from 764df83

File Section File VM
chip-all-clusters-app.elf .flash.text 328 328
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
.debug_info,0,3813
.flash.text,328,328
.debug_loc,0,275
.debug_line,0,264
.debug_str,0,67
.strtab,0,58
.debug_frame,0,24
.debug_ranges,0,24
.symtab,0,16
.debug_aranges,0,8
.xt.prop._ZTVN4chip11DeviceLayer37DeviceNetworkProvisioningDelegateImplE,0,1
.shstrtab,0,-2


src/credentials/CHIPCert.cpp Outdated Show resolved Hide resolved
src/credentials/CHIPCert.cpp Show resolved Hide resolved
src/credentials/CHIPCert.cpp Show resolved Hide resolved
@todo
Copy link

todo bot commented May 27, 2021

- Add an operator== on BitFlags class.

// TODO - Add an operator== on BitFlags class.
return mSubjectDN.IsEqual(other.mSubjectDN) && mIssuerDN.IsEqual(other.mIssuerDN) &&
mSubjectKeyId.IsEqual(other.mSubjectKeyId) && mAuthKeyId.IsEqual(other.mAuthKeyId) &&
(mNotBeforeTime == other.mNotBeforeTime) && (mNotAfterTime == other.mNotAfterTime) &&
(mPublicKeyLen == other.mPublicKeyLen) && (memcmp(mPublicKey, other.mPublicKey, mPublicKeyLen) == 0) &&
(mPubKeyCurveOID == other.mPubKeyCurveOID) && (mPubKeyAlgoOID == other.mPubKeyAlgoOID) &&
(mSigAlgoOID == other.mSigAlgoOID) && (mCertFlags.Raw() == other.mCertFlags.Raw()) &&
(mKeyUsageFlags.Raw() == other.mKeyUsageFlags.Raw()) && (mKeyPurposeFlags.Raw() == other.mKeyPurposeFlags.Raw()) &&
(mPathLenConstraint == other.mPathLenConstraint) && (mSignature.RLen == other.mSignature.RLen) &&
(memcmp(mSignature.R, other.mSignature.R, mSignature.RLen) == 0) && (mSignature.SLen == other.mSignature.SLen) &&
(memcmp(mSignature.S, other.mSignature.S, mSignature.SLen) == 0) &&


This comment was generated by todo based on a TODO comment in 9dddc28 in #7115. cc @pan-apple.

@bzbarsky-apple bzbarsky-apple merged commit c2cc13d into project-chip:master May 27, 2021
@pan-apple pan-apple deleted the duplicate-certs branch May 27, 2021 20:40
nikita-s-wrk pushed a commit to nikita-s-wrk/connectedhomeip that referenced this pull request Sep 23, 2021
* Do not load duplicate certs in ChipCertificateSet

* fix build issues

* 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.

5 participants