CHIP credential serialization #6400

jpk233 · 2021-04-30T15:19:53Z

Problem

We're lacking support to adequately parse elements from CHIP and non-CHIP certificates.

Change overview

Added method to retrieve CHIP certificate value from certificate.
Enhanced OperationalCredentialSet class with Serialize/Deserialize methods. User can now serialize/deserialize a specific Trusted Root ID

Testing

Unit tests were added for GetCertChipId and for OperationalCredentialSet serialization
This was manually tested using Android controller (chip-tool) and x86/arm64 linux device (lighting-app)

todo · 2021-04-30T15:19:55Z

change constants

connectedhomeip/src/credentials/CHIPOperationalCredentials.cpp

Lines 314 to 324 in 19e2599

    
           // TODO change constants 
        
           SuccessOrExit(err = certificateSet.Init(3, 1024)); 
        
           err = certificateSet.LoadCert(serializable.mRootCertificate, serializable.mRootCertificateLen, 
        
                                         BitFlags<CertDecodeFlags>(CertDecodeFlags::kIsTrustAnchor)); 
        
           SuccessOrExit(err); 
        
           trustedRootId.mId  = certificateSet.GetLastCert()->mAuthKeyId.mId; 
        
           trustedRootId.mLen = certificateSet.GetLastCert()->mAuthKeyId.mLen; 
        
           err = certificateSet.LoadCert(serializable.mCACertificate, serializable.mCACertificateLen,

This comment was generated by todo based on a `TODO` comment in `19e2599` in #6400. cc @jpk233.

todo · 2021-04-30T15:19:58Z

Serialize the CASESession to the given serializable data structure for secure pairing

connectedhomeip/src/credentials/CHIPOperationalCredentials.h

Lines 238 to 248 in 19e2599

    
                * @brief TODO Serialize the CASESession to the given serializable data structure for secure pairing 
        
                **/ 
        
               CHIP_ERROR ToSerializable(const CertificateKeyId & trustedRootId, OperationalCredentialSerializable & output); 
        
               /** @brief TODO Reconstruct secure pairing class from the serializable data structure. 
        
                * 
        
                * @return TODO Returns a CHIP_ERROR on error, CHIP_NO_ERROR otherwise 
        
                **/ 
        
               CHIP_ERROR FromSerializable(const OperationalCredentialSerializable & output); 
        
               P256Keypair & GetDevOpCredKeypair(const CertificateKeyId & trustedRootId) { return *GetNodeKeypairAt(trustedRootId); }

This comment was generated by todo based on a `TODO` comment in `19e2599` in #6400. cc @jpk233.

todo · 2021-04-30T15:20:01Z

Reconstruct secure pairing class from the serializable data structure.

connectedhomeip/src/credentials/CHIPOperationalCredentials.h

Lines 242 to 250 in 19e2599

    
               /** @brief TODO Reconstruct secure pairing class from the serializable data structure. 
        
                * 
        
                * @return TODO Returns a CHIP_ERROR on error, CHIP_NO_ERROR otherwise 
        
                **/ 
        
               CHIP_ERROR FromSerializable(const OperationalCredentialSerializable & output); 
        
               P256Keypair & GetDevOpCredKeypair(const CertificateKeyId & trustedRootId) { return *GetNodeKeypairAt(trustedRootId); } 
        
               CHIP_ERROR SetDevOpCredKeypair(const CertificateKeyId & trustedRootId, P256Keypair * newKeypair);

This comment was generated by todo based on a `TODO` comment in `19e2599` in #6400. cc @jpk233.

todo · 2021-04-30T15:20:04Z

Returns a CHIP_ERROR on error, CHIP_NO_ERROR otherwise

connectedhomeip/src/credentials/CHIPOperationalCredentials.h

Lines 244 to 250 in 19e2599

    
                * @return TODO Returns a CHIP_ERROR on error, CHIP_NO_ERROR otherwise 
        
                **/ 
        
               CHIP_ERROR FromSerializable(const OperationalCredentialSerializable & output); 
        
               P256Keypair & GetDevOpCredKeypair(const CertificateKeyId & trustedRootId) { return *GetNodeKeypairAt(trustedRootId); } 
        
               CHIP_ERROR SetDevOpCredKeypair(const CertificateKeyId & trustedRootId, P256Keypair * newKeypair);

This comment was generated by todo based on a `TODO` comment in `19e2599` in #6400. cc @jpk233.

woody-apple · 2021-04-30T18:56:07Z

@pan-apple ?

bitkis · 2021-04-30T16:29:48Z

src/credentials/CHIPCert.cpp

@@ -177,6 +178,9 @@ CHIP_ERROR ChipCertificateSet::LoadCert(TLVReader & reader, BitFlags<CertDecodeF

    cert = new (&mCerts[mCertCount]) ChipCertificateData();

+    cert->mCertificateBegin = chipCert;
+    cert->mCertificateLen   = static_cast<uint16_t>(chipCertLen);


Better idea would be getting sizes of chipCertLen and mCertificateLen in sync to avoid the cast

Modified as per your comments, thanks

src/credentials/CHIPCert.cpp

src/credentials/CHIPOperationalCredentials.cpp

pan-apple · 2021-05-03T19:34:27Z

@jpk233 , maybe we can just use the DER formatted cert, and store it as is. If the KVS doesn't support storage of binary data, we can convert it to base64 string and store that instead.

pan-apple · 2021-05-03T19:41:37Z

For extracting Pubkey, how about converting X509 cert to CHIPCert, and get the value from there? CHIPCert is TLV formatted, and you can use kTag_EllipticCurvePublicKey to get the pubkey.

src/credentials/CHIPOperationalCredentials.h

src/credentials/CHIPCert.cpp

emargolis · 2021-05-04T17:42:43Z

src/credentials/CHIPCert.cpp

@@ -765,6 +772,32 @@ CHIP_ERROR ChipDN::GetCertType(uint8_t & certType) const
    return err;
 }

+CHIP_ERROR ChipDN::GetCertChipVal(uint64_t & chipVal) const


Suggested change

CHIP_ERROR ChipDN::GetCertChipVal(uint64_t & chipVal) const

CHIP_ERROR ChipDN::GetCertChipId(uint64_t & chipId) const

src/credentials/CHIPCertFromX509.cpp

src/credentials/CHIPOperationalCredentials.h

emargolis · 2021-05-04T18:40:02Z

src/credentials/CHIPCertFromX509.cpp

@@ -693,5 +857,19 @@ DLL_EXPORT CHIP_ERROR ConvertX509CertToChipCert(const uint8_t * x509Cert, uint32
    return err;
 }

+DLL_EXPORT CHIP_ERROR ExtractPubkeyFromX509Cert(const uint8_t * x509Cert, uint32_t x509CertLen, Crypto::P256PublicKey & pubkey)


Wanted to understand the usecase here? Specifically, why there is a need to extract public key from X509 certificate but there is no need to extract public key from the CHIP certificate format?

This has been removed as it's more appropriate for a separate PR. But for the usecase: during Device Attestation, we need to extract a public key from a X509 Certificate in order to validate the nonce.

jpk233 · 2021-05-20T15:48:05Z

@jpk233 , maybe we can just use the DER formatted cert, and store it as is. If the KVS doesn't support storage of binary data, we can convert it to base64 string and store that instead.

It's being serialized as base64 currently

For extracting Pubkey, how about converting X509 cert to CHIPCert, and get the value from there? CHIPCert is TLV formatted, and you can use kTag_EllipticCurvePublicKey to get the pubkey.

This change is meant to be applicable to both CHIP and non-CHIP certs, which is why it's not converted to CHIPCert format.

…st new method for retrieving Certificate CHIP ID

jpk233 · 2021-05-24T18:24:21Z

Oh, and please document the lifetime assumptions involved....

Updated CHIPOperationalCredentials.h to document assumptions for the new methods.

src/credentials/CHIPCert.h

src/credentials/CHIPOperationalCredentials.h

…r to avoid stack limitations

src/credentials/tests/TestChipOperationalCredentials.cpp

src/credentials/CHIPOperationalCredentials.cpp

src/credentials/CHIPOperationalCredentials.h

github-actions · 2021-06-02T19:35:40Z

Size increase report for "nrfconnect-example-build" from 5d54fac

File	Section	File	VM
chip-lock.elf	bss	0	512
chip-lock.elf	text	392	392
chip-lock.elf	device_handles	-8	-8
chip-shell.elf	text	100	100
chip-shell.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,2916238
.debug_str,0,86365
.debug_line,0,20689
.debug_abbrev,0,20529
.debug_loc,0,2219
bss,512,0
text,392,392
.debug_ranges,0,216
.debug_frame,0,200
.strtab,0,72
.symtab,0,64
.debug_aranges,0,32
device_handles,-8,-8

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

sections,vmsize,filesize
.debug_info,0,393718
.debug_str,0,92482
.debug_line,0,3526
.debug_abbrev,0,3436
.debug_loc,0,234
text,100,100
.debug_frame,0,36
.debug_aranges,0,8
device_handles,-4,-4

github-actions · 2021-06-02T19:35:48Z

Size increase report for "esp32-example-build" from 5d54fac

File	Section	File	VM
chip-all-clusters-app.elf	.dram0.bss	0	512
chip-all-clusters-app.elf	.flash.text	384	384

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_info,0,37365
.debug_line,0,3412
.debug_loc,0,2341
.debug_str,0,1136
.dram0.bss,512,0
.debug_ranges,0,416
.flash.text,384,384
.debug_frame,0,96
.strtab,0,72
.debug_aranges,0,32
.symtab,0,16
.debug_abbrev,0,-738

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

sections,vmsize,filesize

github-actions · 2021-06-02T19:36:14Z

Size increase report for "gn_qpg6100-example-build" from 5d54fac

File	Section	File	VM
chip-qpg6100-lighting-example.out	.text	416	416
chip-qpg6100-lighting-example.out	.bss	0	128
chip-qpg6100-lighting-example.out	.heap	0	-128

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,29514
.debug_loc,0,2338
.debug_abbrev,0,2129
.debug_line,0,1619
.debug_str,0,1164
.text,416,416
.debug_ranges,0,256
.debug_frame,0,224
.bss,128,0
.strtab,0,72
.symtab,0,64
.debug_aranges,0,32
.heap,-128,0
[Unmapped],0,-416

woody-apple · 2021-06-02T19:47:31Z

/rebase

* CHIP certificate value changes * Addressed review comments, removed TODOs and ExtractPubKey method * Added 2 new tests: - Test OperationalCredentialSet Serialization - Test new method for retrieving Certificate CHIP ID * Fix CI builds * Update with bzbarsky-apple's suggestions, document lifetime assumption * Additional documentation, moved serializable objects to .data in order to avoid stack limitations * Cleanup old code remnants, size check for optional CA certificate * Sync with master, CHIPCert updates Co-authored-by: Boris Itkis <boris.itkis@gmail.com>

CHIP certificate value changes

19e2599

pullapprove bot requested review from chrisdecenzo, jelderton, jmartinez-silabs, LuDuda, mspang, saurabhst and woody-apple April 30, 2021 15:23

jpk233 changed the title ~~CHIP certificate value changes~~ CHIP credential serialization Apr 30, 2021

bitkis suggested changes Apr 30, 2021

View reviewed changes

pan-apple requested a review from emargolis May 3, 2021 19:41

emargolis reviewed May 4, 2021

View reviewed changes

Addressed review comments, removed TODOs and ExtractPubKey method

f07bc43

woody-apple reviewed May 18, 2021

View reviewed changes

This comment has been minimized.

Sign in to view

woody-apple self-requested a review May 18, 2021 01:38

pullapprove bot added the review - pending label May 18, 2021

bitkis approved these changes May 20, 2021

View reviewed changes

jpk233 requested review from bzbarsky-apple and emargolis May 20, 2021 15:41

jelderton approved these changes May 20, 2021

View reviewed changes

emargolis approved these changes May 20, 2021

View reviewed changes

bitkis and others added 4 commits May 21, 2021 17:59

Added 2 new tests: - Test OperationalCredentialSet Serialization - Te…

66bed37

…st new method for retrieving Certificate CHIP ID

Fix CI builds

3d6b1b5

Update with bzbarsky-apple's suggestions, document lifetime assumption

36202ba

Merge branch 'master' into credentials

fef031d

bzbarsky-apple reviewed May 24, 2021

View reviewed changes

src/credentials/CHIPCert.h Outdated Show resolved Hide resolved

src/credentials/CHIPOperationalCredentials.h Outdated Show resolved Hide resolved

src/credentials/CHIPOperationalCredentials.h Outdated Show resolved Hide resolved

Additional documentation, moved serializable objects to .data in orde…

3f0b71e

…r to avoid stack limitations

emargolis reviewed May 25, 2021

View reviewed changes

Cleanup old code remnants, size check for optional CA certificate

c351ba7

jpk233 requested a review from woody-apple May 25, 2021 19:32

jmartinez-silabs approved these changes May 26, 2021

View reviewed changes

pullapprove bot added review - changed requested and removed review - pending labels May 26, 2021

jpk233 added 5 commits May 26, 2021 10:36

Merge branch 'master' into credentials

b51cbaf

Merge branch 'master' into credentials

f242a96

Merge branch 'master' into credentials

4f2f703

Sync with master, CHIPCert updates

d7dccb0

Merge branch 'master' into credentials

8e8c04f

woody-apple approved these changes Jun 2, 2021

View reviewed changes

woody-apple mentioned this pull request Jun 3, 2021

Problem with Restyler Job #983850 restyled-io/restyler#143

Closed

woody-apple added the restyled-ignore label Jun 3, 2021

woody-apple approved these changes Jun 3, 2021

View reviewed changes

woody-apple merged commit da45bb3 into project-chip:master Jun 3, 2021

jpk233 deleted the credentials branch June 3, 2021 16:40

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

CHIP credential serialization #6400

CHIP credential serialization #6400

jpk233 commented Apr 30, 2021 •

edited

Loading

todo bot commented Apr 30, 2021

todo bot commented Apr 30, 2021

todo bot commented Apr 30, 2021

todo bot commented Apr 30, 2021

woody-apple commented Apr 30, 2021

bitkis Apr 30, 2021

jpk233 May 10, 2021

pan-apple commented May 3, 2021

pan-apple commented May 3, 2021

emargolis May 4, 2021

emargolis May 4, 2021

jpk233 May 10, 2021

This comment has been minimized.

jpk233 commented May 20, 2021

jpk233 commented May 24, 2021

github-actions bot commented Jun 2, 2021

github-actions bot commented Jun 2, 2021

github-actions bot commented Jun 2, 2021

woody-apple commented Jun 2, 2021

	CHIP_ERROR ChipDN::GetCertChipVal(uint64_t & chipVal) const
	CHIP_ERROR ChipDN::GetCertChipId(uint64_t & chipId) const

CHIP credential serialization #6400

CHIP credential serialization #6400

Conversation

jpk233 commented Apr 30, 2021 • edited Loading

Problem

Change overview

Testing

todo bot commented Apr 30, 2021

change constants

This comment was generated by todo based on a TODO comment in 19e2599 in #6400. cc @jpk233.

todo bot commented Apr 30, 2021

Serialize the CASESession to the given serializable data structure for secure pairing

This comment was generated by todo based on a TODO comment in 19e2599 in #6400. cc @jpk233.

todo bot commented Apr 30, 2021

Reconstruct secure pairing class from the serializable data structure.

This comment was generated by todo based on a TODO comment in 19e2599 in #6400. cc @jpk233.

todo bot commented Apr 30, 2021

Returns a CHIP_ERROR on error, CHIP_NO_ERROR otherwise

This comment was generated by todo based on a TODO comment in 19e2599 in #6400. cc @jpk233.

woody-apple commented Apr 30, 2021

bitkis Apr 30, 2021

Choose a reason for hiding this comment

jpk233 May 10, 2021

Choose a reason for hiding this comment

pan-apple commented May 3, 2021

pan-apple commented May 3, 2021

emargolis May 4, 2021

Choose a reason for hiding this comment

emargolis May 4, 2021

Choose a reason for hiding this comment

jpk233 May 10, 2021

Choose a reason for hiding this comment

This comment has been minimized.

jpk233 commented May 20, 2021

jpk233 commented May 24, 2021

github-actions bot commented Jun 2, 2021

github-actions bot commented Jun 2, 2021

github-actions bot commented Jun 2, 2021

woody-apple commented Jun 2, 2021

jpk233 commented Apr 30, 2021 •

edited

Loading

This comment was generated by todo based on a `TODO` comment in `19e2599` in #6400. cc @jpk233.

This comment was generated by todo based on a `TODO` comment in `19e2599` in #6400. cc @jpk233.

This comment was generated by todo based on a `TODO` comment in `19e2599` in #6400. cc @jpk233.

This comment was generated by todo based on a `TODO` comment in `19e2599` in #6400. cc @jpk233.