From ac6953c767d243c91444c3343b392c0adf710a61 Mon Sep 17 00:00:00 2001 From: Justin Wood Date: Wed, 30 Oct 2024 09:11:15 -0700 Subject: [PATCH 01/16] Initial checkin --- .../commands/common/CHIPToolKeypair.h | 2 +- .../commands/common/CHIPToolKeypair.mm | 8 +++-- src/darwin/Framework/CHIP/MTRCertificates.mm | 18 ++++++++++- .../CHIP/MTRDeviceControllerFactory.mm | 18 ++++++++++- src/darwin/Framework/CHIP/MTRKeypair.h | 10 ++++-- .../Framework/CHIPTests/MTRCertificateTests.m | 32 +++++++++---------- .../CHIPTests/MTRCertificateValidityTests.m | 2 +- .../CHIPTests/MTRControllerAdvertisingTests.m | 2 +- .../Framework/CHIPTests/MTRControllerTests.m | 24 +++++++------- .../Framework/CHIPTests/MTRFabricInfoTests.m | 2 +- .../CHIPTests/MTRPerControllerStorageTests.m | 2 +- 11 files changed, 81 insertions(+), 39 deletions(-) diff --git a/examples/darwin-framework-tool/commands/common/CHIPToolKeypair.h b/examples/darwin-framework-tool/commands/common/CHIPToolKeypair.h index f0ee3f8db6092b..58be7f2ac4b041 100644 --- a/examples/darwin-framework-tool/commands/common/CHIPToolKeypair.h +++ b/examples/darwin-framework-tool/commands/common/CHIPToolKeypair.h @@ -22,7 +22,7 @@ @interface CHIPToolKeypair : NSObject - (BOOL)initialize; - (NSData *)signMessageECDSA_RAW:(NSData *)message; -- (SecKeyRef)publicKey; +- (SecKeyRef)copyPublicKey; - (CHIP_ERROR)Serialize:(chip::Crypto::P256SerializedKeypair &)output; - (CHIP_ERROR)Deserialize:(chip::Crypto::P256SerializedKeypair &)input; - (CHIP_ERROR)createOrLoadKeys:(id)storage; diff --git a/examples/darwin-framework-tool/commands/common/CHIPToolKeypair.mm b/examples/darwin-framework-tool/commands/common/CHIPToolKeypair.mm index ce0ef5819b6ac1..eabc6209521ab8 100644 --- a/examples/darwin-framework-tool/commands/common/CHIPToolKeypair.mm +++ b/examples/darwin-framework-tool/commands/common/CHIPToolKeypair.mm @@ -65,7 +65,7 @@ - (NSData *)signMessageECDSA_RAW:(NSData *)message return out_signature; } -- (SecKeyRef)publicKey +- (SecKeyRef)copyPublicKey { if (_mPublicKey == nil) { chip::Crypto::P256PublicKey publicKey = _mKeyPair.Pubkey(); @@ -79,7 +79,11 @@ - (SecKeyRef)publicKey }; _mPublicKey = SecKeyCreateWithData((__bridge CFDataRef) publicKeyNSData, (__bridge CFDictionaryRef) attributes, nullptr); } - return _mPublicKey; + + if (_mPublicKey) + return CFRetain(_mPublicKey); + + return NULL; } - (CHIP_ERROR)Deserialize:(chip::Crypto::P256SerializedKeypair &)input diff --git a/src/darwin/Framework/CHIP/MTRCertificates.mm b/src/darwin/Framework/CHIP/MTRCertificates.mm index b153cf4442b373..140dd5deb41aae 100644 --- a/src/darwin/Framework/CHIP/MTRCertificates.mm +++ b/src/darwin/Framework/CHIP/MTRCertificates.mm @@ -152,7 +152,23 @@ + (MTRCertificateDERBytes _Nullable)createOperationalCertificate:(id + (BOOL)keypair:(id)keypair matchesCertificate:(NSData *)certificate { P256PublicKey keypairPubKey; - CHIP_ERROR err = MTRP256KeypairBridge::MatterPubKeyFromSecKeyRef(keypair.publicKey, &keypairPubKey); + SecKeyRef publicKey = NULL; + + if ([keypair respondsToSelector:@selector(copyPublicKey)]) { + publicKey = [keypair copyPublicKey]; + } else { + publicKey = [keypair publicKey]; + if (publicKey) + CFRetain(publicKey); + } + + CHIP_ERROR err = MTRP256KeypairBridge::MatterPubKeyFromSecKeyRef(publicKey, &keypairPubKey); + + if (publicKey != NULL) { + CFRelease(publicKey); + publicKey = NULL; + } + if (err != CHIP_NO_ERROR) { MTR_LOG_ERROR("Can't extract public key from keypair: %s", ErrorStr(err)); return NO; diff --git a/src/darwin/Framework/CHIP/MTRDeviceControllerFactory.mm b/src/darwin/Framework/CHIP/MTRDeviceControllerFactory.mm index 9de51431bfe4f8..026beb279c4de3 100644 --- a/src/darwin/Framework/CHIP/MTRDeviceControllerFactory.mm +++ b/src/darwin/Framework/CHIP/MTRDeviceControllerFactory.mm @@ -819,7 +819,23 @@ - (BOOL)findMatchingFabric:(FabricTable &)fabricTable } else { // No root certificate means the nocSigner is using the root keys, because // consumers must provide a root certificate whenever an ICA is used. - CHIP_ERROR err = MTRP256KeypairBridge::MatterPubKeyFromSecKeyRef(params.nocSigner.publicKey, &pubKey); + SecKeyRef publicKey = NULL; + + if ([params.nocSigner respondsToSelector:@selector(copyPublicKey)]) { + publicKey = [params.nocSigner copyPublicKey]; + } else { + publicKey = [params.nocSigner publicKey]; + if (publicKey) + CFRetain(publicKey); + } + + CHIP_ERROR err = MTRP256KeypairBridge::MatterPubKeyFromSecKeyRef(publicKey, &pubKey); + + if (publicKey != NULL) { + CFRelease(publicKey); + publicKey = NULL; + } + if (err != CHIP_NO_ERROR) { MTR_LOG_ERROR("Can't extract public key from MTRKeypair: %s", ErrorStr(err)); return NO; diff --git a/src/darwin/Framework/CHIP/MTRKeypair.h b/src/darwin/Framework/CHIP/MTRKeypair.h index a4e4521b2864a6..f5043928f94458 100644 --- a/src/darwin/Framework/CHIP/MTRKeypair.h +++ b/src/darwin/Framework/CHIP/MTRKeypair.h @@ -33,9 +33,15 @@ NS_ASSUME_NONNULL_BEGIN @protocol MTRKeypair @required /** - * @brief Return public key for the keypair. + * @brief Return a copy of the public key for the keypair. */ -- (SecKeyRef)publicKey; +@property (nonatomic, readonly, copy) SecKeyRef copyPublicKey MTR_NEWLY_AVAILABLE; + +/** + * @brief Return public key for the keypair. DEPRECATED + */ + +@property (nonatomic, readonly, copy) SecKeyRef publicKey MTR_DEPRECATED("Please implement copyPublicKey", ios(16.4, 17.2), macos(13.3, 14.2), watchos(9.4, 10.2), tvos(16.4, 17.2)); @optional /** diff --git a/src/darwin/Framework/CHIPTests/MTRCertificateTests.m b/src/darwin/Framework/CHIPTests/MTRCertificateTests.m index fb6cfb9ecb7e67..c174f3321e5160 100644 --- a/src/darwin/Framework/CHIPTests/MTRCertificateTests.m +++ b/src/darwin/Framework/CHIPTests/MTRCertificateTests.m @@ -129,7 +129,7 @@ - (void)testGenerateIntermediateCert __auto_type * intermediateCert = [MTRCertificates createIntermediateCertificate:rootKeys rootCertificate:rootCert - intermediatePublicKey:intermediateKeys.publicKey + intermediatePublicKey:[intermediateKeys.copyPublicKey autorelease] issuerID:nil fabricID:nil error:nil]; @@ -161,7 +161,7 @@ - (void)testGenerateIntermediateCertWithValidityPeriod __auto_type * intermediateCert = [MTRCertificates createIntermediateCertificate:rootKeys rootCertificate:rootCert - intermediatePublicKey:intermediateKeys.publicKey + intermediatePublicKey:[intermediateKeys.copyPublicKey autorelease] issuerID:nil fabricID:nil validityPeriod:validityPeriod @@ -198,7 +198,7 @@ - (void)testGenerateIntermediateCertWithInfiniteValidity __auto_type * intermediateCert = [MTRCertificates createIntermediateCertificate:rootKeys rootCertificate:rootCert - intermediatePublicKey:intermediateKeys.publicKey + intermediatePublicKey:[intermediateKeys.copyPublicKey autorelease] issuerID:nil fabricID:nil validityPeriod:validityPeriod @@ -238,7 +238,7 @@ - (void)testGenerateOperationalCertNoIntermediate __auto_type * operationalCert = [MTRCertificates createOperationalCertificate:rootKeys signingCertificate:rootCert - operationalPublicKey:operationalKeys.publicKey + operationalPublicKey:[operationalKeys.copyPublicKey autorelease] fabricID:@1 nodeID:@1 caseAuthenticatedTags:cats @@ -277,7 +277,7 @@ - (void)testGenerateOperationalCertNoIntermediateWithValidityPeriod __auto_type * operationalCert = [MTRCertificates createOperationalCertificate:rootKeys signingCertificate:rootCert - operationalPublicKey:operationalKeys.publicKey + operationalPublicKey:[operationalKeys.copyPublicKey autorelease] fabricID:@1 nodeID:@1 caseAuthenticatedTags:cats @@ -321,7 +321,7 @@ - (void)testGenerateOperationalCertNoIntermediateWithInfiniteValidity __auto_type * operationalCert = [MTRCertificates createOperationalCertificate:rootKeys signingCertificate:rootCert - operationalPublicKey:operationalKeys.publicKey + operationalPublicKey:[operationalKeys.copyPublicKey autorelease] fabricID:@1 nodeID:@1 caseAuthenticatedTags:cats @@ -356,7 +356,7 @@ - (void)testGenerateOperationalCertWithIntermediate __auto_type * intermediateCert = [MTRCertificates createIntermediateCertificate:rootKeys rootCertificate:rootCert - intermediatePublicKey:intermediateKeys.publicKey + intermediatePublicKey:[intermediateKeys.copyPublicKey autorelease] issuerID:nil fabricID:nil error:nil]; @@ -367,7 +367,7 @@ - (void)testGenerateOperationalCertWithIntermediate __auto_type * operationalCert = [MTRCertificates createOperationalCertificate:intermediateKeys signingCertificate:intermediateCert - operationalPublicKey:operationalKeys.publicKey + operationalPublicKey:[operationalKeys.copyPublicKey autorelease] fabricID:@1 nodeID:@1 caseAuthenticatedTags:nil @@ -415,7 +415,7 @@ - (void)testGenerateOperationalCertErrorCases // Check basic case works __auto_type * operationalCert = [MTRCertificates createOperationalCertificate:rootKeys signingCertificate:rootCert - operationalPublicKey:operationalKeys.publicKey + operationalPublicKey:[operationalKeys.copyPublicKey autorelease] fabricID:@1 nodeID:@1 caseAuthenticatedTags:nil @@ -425,7 +425,7 @@ - (void)testGenerateOperationalCertErrorCases // CATs too long operationalCert = [MTRCertificates createOperationalCertificate:rootKeys signingCertificate:rootCert - operationalPublicKey:operationalKeys.publicKey + operationalPublicKey:[operationalKeys.copyPublicKey autorelease] fabricID:@1 nodeID:@1 caseAuthenticatedTags:longCats @@ -435,7 +435,7 @@ - (void)testGenerateOperationalCertErrorCases // Multiple CATs with the same identifier but different versions operationalCert = [MTRCertificates createOperationalCertificate:rootKeys signingCertificate:rootCert - operationalPublicKey:operationalKeys.publicKey + operationalPublicKey:[operationalKeys.copyPublicKey autorelease] fabricID:@1 nodeID:@1 caseAuthenticatedTags:catsWithSameIdentifier @@ -445,7 +445,7 @@ - (void)testGenerateOperationalCertErrorCases // CAT with invalid version operationalCert = [MTRCertificates createOperationalCertificate:rootKeys signingCertificate:rootCert - operationalPublicKey:operationalKeys.publicKey + operationalPublicKey:[operationalKeys.copyPublicKey autorelease] fabricID:@1 nodeID:@1 caseAuthenticatedTags:catsWithInvalidVersion @@ -455,7 +455,7 @@ - (void)testGenerateOperationalCertErrorCases // Signing key mismatch operationalCert = [MTRCertificates createOperationalCertificate:operationalKeys signingCertificate:rootCert - operationalPublicKey:operationalKeys.publicKey + operationalPublicKey:[operationalKeys.copyPublicKey autorelease] fabricID:@1 nodeID:@1 caseAuthenticatedTags:nil @@ -465,7 +465,7 @@ - (void)testGenerateOperationalCertErrorCases // Invalid fabric id operationalCert = [MTRCertificates createOperationalCertificate:rootKeys signingCertificate:rootCert - operationalPublicKey:operationalKeys.publicKey + operationalPublicKey:[operationalKeys.copyPublicKey autorelease] fabricID:@0 nodeID:@1 caseAuthenticatedTags:nil @@ -475,7 +475,7 @@ - (void)testGenerateOperationalCertErrorCases // Undefined node id operationalCert = [MTRCertificates createOperationalCertificate:rootKeys signingCertificate:rootCert - operationalPublicKey:operationalKeys.publicKey + operationalPublicKey:[operationalKeys.copyPublicKey autorelease] fabricID:@1 nodeID:@0 caseAuthenticatedTags:nil @@ -485,7 +485,7 @@ - (void)testGenerateOperationalCertErrorCases // Non-operational node id operationalCert = [MTRCertificates createOperationalCertificate:rootKeys signingCertificate:rootCert - operationalPublicKey:operationalKeys.publicKey + operationalPublicKey:[operationalKeys.copyPublicKey autorelease] fabricID:@1 nodeID:@(0xFFFFFFFFFFFFFFFFLLU) caseAuthenticatedTags:nil diff --git a/src/darwin/Framework/CHIPTests/MTRCertificateValidityTests.m b/src/darwin/Framework/CHIPTests/MTRCertificateValidityTests.m index ecc593e8432045..3c405d9f7f45e1 100644 --- a/src/darwin/Framework/CHIPTests/MTRCertificateValidityTests.m +++ b/src/darwin/Framework/CHIPTests/MTRCertificateValidityTests.m @@ -262,7 +262,7 @@ - (void)initStack:(MTRTestCertificateIssuer *)certificateIssuer __auto_type * controllerOperationalCert = [certificateIssuer issueOperationalCertificateForNode:@(kControllerId) - operationalPublicKey:controllerOperationalKeys.publicKey]; + operationalPublicKey:[controllerOperationalKeys.copyPublicKey autorelease]]; XCTAssertNotNil(controllerOperationalCert); __auto_type * params = [[MTRDeviceControllerStartupParams alloc] initWithIPK:certificateIssuer.rootKey.ipk diff --git a/src/darwin/Framework/CHIPTests/MTRControllerAdvertisingTests.m b/src/darwin/Framework/CHIPTests/MTRControllerAdvertisingTests.m index 4092a38b56f84e..dace586d422034 100644 --- a/src/darwin/Framework/CHIPTests/MTRControllerAdvertisingTests.m +++ b/src/darwin/Framework/CHIPTests/MTRControllerAdvertisingTests.m @@ -167,7 +167,7 @@ - (nullable MTRDeviceController *)startControllerWithRootKeys:(MTRTestKeys *)roo __auto_type * operational = [MTRCertificates createOperationalCertificate:rootKeys signingCertificate:root - operationalPublicKey:operationalKeys.publicKey + operationalPublicKey:[operationalKeys.copyPublicKey autorelease] fabricID:fabricID nodeID:nodeID caseAuthenticatedTags:nil diff --git a/src/darwin/Framework/CHIPTests/MTRControllerTests.m b/src/darwin/Framework/CHIPTests/MTRControllerTests.m index 436a0df230e3d9..03907e0a02e377 100644 --- a/src/darwin/Framework/CHIPTests/MTRControllerTests.m +++ b/src/darwin/Framework/CHIPTests/MTRControllerTests.m @@ -623,7 +623,7 @@ - (void)testControllerSignerKeyWithIntermediate __auto_type * intermediate = [MTRCertificates createIntermediateCertificate:rootKeys rootCertificate:root - intermediatePublicKey:intermediateKeys.publicKey + intermediatePublicKey:[intermediateKeys.copyPublicKey autorelease] issuerID:nil fabricID:nil error:nil]; @@ -863,7 +863,7 @@ - (void)testControllerRotateToICA __auto_type * intermediate = [MTRCertificates createIntermediateCertificate:rootKeys rootCertificate:root - intermediatePublicKey:intermediateKeys.publicKey + intermediatePublicKey:[intermediateKeys.copyPublicKey autorelease] issuerID:nil fabricID:nil error:nil]; @@ -925,7 +925,7 @@ - (void)testControllerRotateFromICA __auto_type * intermediate = [MTRCertificates createIntermediateCertificate:rootKeys rootCertificate:root - intermediatePublicKey:intermediateKeys.publicKey + intermediatePublicKey:[intermediateKeys.copyPublicKey autorelease] issuerID:nil fabricID:nil error:nil]; @@ -989,7 +989,7 @@ - (void)testControllerRotateICA __auto_type * intermediate1 = [MTRCertificates createIntermediateCertificate:rootKeys rootCertificate:root - intermediatePublicKey:intermediateKeys1.publicKey + intermediatePublicKey:[intermediateKeys1.copyPublicKey autorelease] issuerID:nil fabricID:nil error:nil]; @@ -1000,7 +1000,7 @@ - (void)testControllerRotateICA __auto_type * intermediate2 = [MTRCertificates createIntermediateCertificate:rootKeys rootCertificate:root - intermediatePublicKey:intermediateKeys2.publicKey + intermediatePublicKey:[intermediateKeys2.copyPublicKey autorelease] issuerID:nil fabricID:nil error:nil]; @@ -1064,7 +1064,7 @@ - (void)testControllerICAWithoutRoot __auto_type * intermediate = [MTRCertificates createIntermediateCertificate:rootKeys rootCertificate:root - intermediatePublicKey:intermediateKeys.publicKey + intermediatePublicKey:[intermediateKeys.copyPublicKey autorelease] issuerID:nil fabricID:nil error:nil]; @@ -1107,7 +1107,7 @@ - (void)testControllerProvideFullCertChain __auto_type * intermediate = [MTRCertificates createIntermediateCertificate:rootKeys rootCertificate:root - intermediatePublicKey:intermediateKeys.publicKey + intermediatePublicKey:[intermediateKeys.copyPublicKey autorelease] issuerID:nil fabricID:nil error:nil]; @@ -1118,7 +1118,7 @@ - (void)testControllerProvideFullCertChain __auto_type * operational = [MTRCertificates createOperationalCertificate:intermediateKeys signingCertificate:intermediate - operationalPublicKey:operationalKeys.publicKey + operationalPublicKey:[operationalKeys.copyPublicKey autorelease] fabricID:@123 nodeID:@456 caseAuthenticatedTags:nil @@ -1182,7 +1182,7 @@ - (void)testControllerProvideCertChainNoICA __auto_type * operational = [MTRCertificates createOperationalCertificate:rootKeys signingCertificate:root - operationalPublicKey:operationalKeys.publicKey + operationalPublicKey:[operationalKeys.copyPublicKey autorelease] fabricID:@123 nodeID:@456 caseAuthenticatedTags:nil @@ -1232,7 +1232,7 @@ - (void)testControllerCertChainFabricMismatchRoot __auto_type * operational = [MTRCertificates createOperationalCertificate:rootKeys signingCertificate:root - operationalPublicKey:operationalKeys.publicKey + operationalPublicKey:[operationalKeys.copyPublicKey autorelease] fabricID:@123 nodeID:@456 caseAuthenticatedTags:nil @@ -1276,7 +1276,7 @@ - (void)testControllerCertChainFabricMismatchIntermediate __auto_type * intermediate = [MTRCertificates createIntermediateCertificate:rootKeys rootCertificate:root - intermediatePublicKey:intermediateKeys.publicKey + intermediatePublicKey:[intermediateKeys.copyPublicKey autorelease] issuerID:nil fabricID:@111 error:nil]; @@ -1287,7 +1287,7 @@ - (void)testControllerCertChainFabricMismatchIntermediate __auto_type * operational = [MTRCertificates createOperationalCertificate:intermediateKeys signingCertificate:intermediate - operationalPublicKey:operationalKeys.publicKey + operationalPublicKey:[operationalKeys.copyPublicKey autorelease] fabricID:@123 nodeID:@456 caseAuthenticatedTags:nil diff --git a/src/darwin/Framework/CHIPTests/MTRFabricInfoTests.m b/src/darwin/Framework/CHIPTests/MTRFabricInfoTests.m index a98067098688b5..5cefe40b633853 100644 --- a/src/darwin/Framework/CHIPTests/MTRFabricInfoTests.m +++ b/src/darwin/Framework/CHIPTests/MTRFabricInfoTests.m @@ -160,7 +160,7 @@ - (void)testFabricInfoTwoFabrics __auto_type * intermediate = [MTRCertificates createIntermediateCertificate:rootKeys rootCertificate:root - intermediatePublicKey:intermediateKeys.publicKey + intermediatePublicKey:[intermediateKeys.copyPublicKey autorelease] issuerID:nil fabricID:nil error:nil]; diff --git a/src/darwin/Framework/CHIPTests/MTRPerControllerStorageTests.m b/src/darwin/Framework/CHIPTests/MTRPerControllerStorageTests.m index 071566381cc69c..9c0d94a0ce89c0 100644 --- a/src/darwin/Framework/CHIPTests/MTRPerControllerStorageTests.m +++ b/src/darwin/Framework/CHIPTests/MTRPerControllerStorageTests.m @@ -405,7 +405,7 @@ - (nullable MTRDeviceController *)startControllerWithRootKeys:(MTRTestKeys *)roo __auto_type * operational = [MTRCertificates createOperationalCertificate:rootKeys signingCertificate:root - operationalPublicKey:operationalKeys.publicKey + operationalPublicKey:[operationalKeys.copyPublicKey autorelease] fabricID:fabricID nodeID:nodeID caseAuthenticatedTags:caseAuthenticatedTags From 47899803de282740505d1231d1a8a7e901727825 Mon Sep 17 00:00:00 2001 From: Justin Wood Date: Wed, 30 Oct 2024 09:15:41 -0700 Subject: [PATCH 02/16] Fixing review feedback --- src/darwin/Framework/CHIP/MTRCertificates.mm | 3 ++- src/darwin/Framework/CHIP/MTRDeviceControllerFactory.mm | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/darwin/Framework/CHIP/MTRCertificates.mm b/src/darwin/Framework/CHIP/MTRCertificates.mm index 140dd5deb41aae..7dcec96cba695b 100644 --- a/src/darwin/Framework/CHIP/MTRCertificates.mm +++ b/src/darwin/Framework/CHIP/MTRCertificates.mm @@ -158,8 +158,9 @@ + (BOOL)keypair:(id)keypair matchesCertificate:(NSData *)certificate publicKey = [keypair copyPublicKey]; } else { publicKey = [keypair publicKey]; - if (publicKey) + if (publicKey) { CFRetain(publicKey); + } } CHIP_ERROR err = MTRP256KeypairBridge::MatterPubKeyFromSecKeyRef(publicKey, &keypairPubKey); diff --git a/src/darwin/Framework/CHIP/MTRDeviceControllerFactory.mm b/src/darwin/Framework/CHIP/MTRDeviceControllerFactory.mm index 026beb279c4de3..e287ef0446593f 100644 --- a/src/darwin/Framework/CHIP/MTRDeviceControllerFactory.mm +++ b/src/darwin/Framework/CHIP/MTRDeviceControllerFactory.mm @@ -825,8 +825,9 @@ - (BOOL)findMatchingFabric:(FabricTable &)fabricTable publicKey = [params.nocSigner copyPublicKey]; } else { publicKey = [params.nocSigner publicKey]; - if (publicKey) + if (publicKey) { CFRetain(publicKey); + } } CHIP_ERROR err = MTRP256KeypairBridge::MatterPubKeyFromSecKeyRef(publicKey, &pubKey); From e3735b233a9298676da8b2e5bc5a82384710650f Mon Sep 17 00:00:00 2001 From: Justin Wood Date: Wed, 30 Oct 2024 09:18:21 -0700 Subject: [PATCH 03/16] Adding braces --- .../darwin-framework-tool/commands/common/CHIPToolKeypair.mm | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/examples/darwin-framework-tool/commands/common/CHIPToolKeypair.mm b/examples/darwin-framework-tool/commands/common/CHIPToolKeypair.mm index eabc6209521ab8..d7932a8f5bb947 100644 --- a/examples/darwin-framework-tool/commands/common/CHIPToolKeypair.mm +++ b/examples/darwin-framework-tool/commands/common/CHIPToolKeypair.mm @@ -58,7 +58,7 @@ - (NSData *)signMessageECDSA_RAW:(NSData *)message { chip::Crypto::P256ECDSASignature signature; NSData * out_signature; - CHIP_ERROR signing_error = _mKeyPair.ECDSA_sign_msg((const uint8_t *) [message bytes], (size_t)[message length], signature); + CHIP_ERROR signing_error = _mKeyPair.ECDSA_sign_msg((const uint8_t *) [message bytes], (size_t) [message length], signature); if (signing_error != CHIP_NO_ERROR) return nil; out_signature = [NSData dataWithBytes:signature.Bytes() length:signature.Length()]; @@ -80,8 +80,9 @@ - (SecKeyRef)copyPublicKey _mPublicKey = SecKeyCreateWithData((__bridge CFDataRef) publicKeyNSData, (__bridge CFDictionaryRef) attributes, nullptr); } - if (_mPublicKey) + if (_mPublicKey) { return CFRetain(_mPublicKey); + } return NULL; } From 680ae77ca1ba047e52074f62b1ebfc56b25e7b1a Mon Sep 17 00:00:00 2001 From: "Restyled.io" Date: Wed, 30 Oct 2024 16:19:43 +0000 Subject: [PATCH 04/16] Restyled by clang-format --- .../darwin-framework-tool/commands/common/CHIPToolKeypair.mm | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/examples/darwin-framework-tool/commands/common/CHIPToolKeypair.mm b/examples/darwin-framework-tool/commands/common/CHIPToolKeypair.mm index d7932a8f5bb947..bcbbcf9a206ac2 100644 --- a/examples/darwin-framework-tool/commands/common/CHIPToolKeypair.mm +++ b/examples/darwin-framework-tool/commands/common/CHIPToolKeypair.mm @@ -58,7 +58,7 @@ - (NSData *)signMessageECDSA_RAW:(NSData *)message { chip::Crypto::P256ECDSASignature signature; NSData * out_signature; - CHIP_ERROR signing_error = _mKeyPair.ECDSA_sign_msg((const uint8_t *) [message bytes], (size_t) [message length], signature); + CHIP_ERROR signing_error = _mKeyPair.ECDSA_sign_msg((const uint8_t *) [message bytes], (size_t)[message length], signature); if (signing_error != CHIP_NO_ERROR) return nil; out_signature = [NSData dataWithBytes:signature.Bytes() length:signature.Length()]; From f75ad53e6dc5d9b343697350b09a48c52c451991 Mon Sep 17 00:00:00 2001 From: Justin Wood Date: Wed, 30 Oct 2024 10:22:29 -0700 Subject: [PATCH 05/16] Fixing build --- src/darwin/Framework/CHIP/MTRKeypair.h | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/src/darwin/Framework/CHIP/MTRKeypair.h b/src/darwin/Framework/CHIP/MTRKeypair.h index f5043928f94458..e51d89edbadb11 100644 --- a/src/darwin/Framework/CHIP/MTRKeypair.h +++ b/src/darwin/Framework/CHIP/MTRKeypair.h @@ -17,6 +17,7 @@ #import #import +#import NS_ASSUME_NONNULL_BEGIN @@ -31,19 +32,19 @@ NS_ASSUME_NONNULL_BEGIN * framework APIs. */ @protocol MTRKeypair -@required + +@optional /** - * @brief Return a copy of the public key for the keypair. + * @brief Returns a copy of the public key for the keypair. */ -@property (nonatomic, readonly, copy) SecKeyRef copyPublicKey MTR_NEWLY_AVAILABLE; +- (SecKeyRef)copyPublicKey MTR_NEWLY_AVAILABLE; /** - * @brief Return public key for the keypair. DEPRECATED + * @brief Returns public key for the keypair. DEPRECATED - please use copyPublicKey, otherwise this will leak */ -@property (nonatomic, readonly, copy) SecKeyRef publicKey MTR_DEPRECATED("Please implement copyPublicKey", ios(16.4, 17.2), macos(13.3, 14.2), watchos(9.4, 10.2), tvos(16.4, 17.2)); +- (SecKeyRef)publicKey MTR_DEPRECATED("Please implement copyPublicKey, this will leak otherwise", ios(16.4, 17.2), macos(13.3, 14.2), watchos(9.4, 10.2), tvos(16.4, 17.2)); -@optional /** * @brief A function to sign a message using ECDSA * From c3c885cdf29df7f7d960bd524c85f3b81d0603c7 Mon Sep 17 00:00:00 2001 From: "Restyled.io" Date: Wed, 30 Oct 2024 17:23:13 +0000 Subject: [PATCH 06/16] Restyled by clang-format --- src/darwin/Framework/CHIP/MTRKeypair.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/darwin/Framework/CHIP/MTRKeypair.h b/src/darwin/Framework/CHIP/MTRKeypair.h index e51d89edbadb11..d80b752a56e665 100644 --- a/src/darwin/Framework/CHIP/MTRKeypair.h +++ b/src/darwin/Framework/CHIP/MTRKeypair.h @@ -16,8 +16,8 @@ */ #import -#import #import +#import NS_ASSUME_NONNULL_BEGIN From f99b8098ad3d6ef2656b672d97036b7fa41eda95 Mon Sep 17 00:00:00 2001 From: Justin Wood Date: Wed, 30 Oct 2024 10:35:49 -0700 Subject: [PATCH 07/16] Fixing annotations --- src/darwin/Framework/CHIP/MTRKeypair.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/darwin/Framework/CHIP/MTRKeypair.h b/src/darwin/Framework/CHIP/MTRKeypair.h index d80b752a56e665..945014ec958fcd 100644 --- a/src/darwin/Framework/CHIP/MTRKeypair.h +++ b/src/darwin/Framework/CHIP/MTRKeypair.h @@ -43,7 +43,7 @@ NS_ASSUME_NONNULL_BEGIN * @brief Returns public key for the keypair. DEPRECATED - please use copyPublicKey, otherwise this will leak */ -- (SecKeyRef)publicKey MTR_DEPRECATED("Please implement copyPublicKey, this will leak otherwise", ios(16.4, 17.2), macos(13.3, 14.2), watchos(9.4, 10.2), tvos(16.4, 17.2)); +- (SecKeyRef)publicKey MTR_DEPRECATED("Please implement copyPublicKey, this will leak otherwise", ios(16.4, 18.3), macos(13.3, 15.3), watchos(9.4, 11.3), tvos(16.4, 18.3)); /** * @brief A function to sign a message using ECDSA From 3c5c598f09b5053cfeba166e672fd0a488dde732 Mon Sep 17 00:00:00 2001 From: Justin Wood Date: Wed, 30 Oct 2024 10:46:15 -0700 Subject: [PATCH 08/16] Update src/darwin/Framework/CHIP/MTRKeypair.h Co-authored-by: Boris Zbarsky --- src/darwin/Framework/CHIP/MTRKeypair.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/darwin/Framework/CHIP/MTRKeypair.h b/src/darwin/Framework/CHIP/MTRKeypair.h index 945014ec958fcd..e370808701d8e7 100644 --- a/src/darwin/Framework/CHIP/MTRKeypair.h +++ b/src/darwin/Framework/CHIP/MTRKeypair.h @@ -40,7 +40,7 @@ NS_ASSUME_NONNULL_BEGIN - (SecKeyRef)copyPublicKey MTR_NEWLY_AVAILABLE; /** - * @brief Returns public key for the keypair. DEPRECATED - please use copyPublicKey, otherwise this will leak + * @brief Returns public key for the keypair without adding a reference. DEPRECATED - please use copyPublicKey, otherwise this will leak. */ - (SecKeyRef)publicKey MTR_DEPRECATED("Please implement copyPublicKey, this will leak otherwise", ios(16.4, 18.3), macos(13.3, 15.3), watchos(9.4, 11.3), tvos(16.4, 18.3)); From 6d6b9528d9c8ee553468d1bdbfde094ca300ae84 Mon Sep 17 00:00:00 2001 From: Kiel Oleson Date: Wed, 30 Oct 2024 18:03:15 -0700 Subject: [PATCH 09/16] separate statements involving _mPublicKey to make compiler happy --- .../darwin-framework-tool/commands/common/CHIPToolKeypair.mm | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/examples/darwin-framework-tool/commands/common/CHIPToolKeypair.mm b/examples/darwin-framework-tool/commands/common/CHIPToolKeypair.mm index bcbbcf9a206ac2..a09975d68da41f 100644 --- a/examples/darwin-framework-tool/commands/common/CHIPToolKeypair.mm +++ b/examples/darwin-framework-tool/commands/common/CHIPToolKeypair.mm @@ -81,7 +81,8 @@ - (SecKeyRef)copyPublicKey } if (_mPublicKey) { - return CFRetain(_mPublicKey); + CFRetain(_mPublicKey); + return _mPublicKey; } return NULL; From bb09a76629c916f879a0e98a158710588a3a4b72 Mon Sep 17 00:00:00 2001 From: Kiel Oleson Date: Thu, 31 Oct 2024 11:14:19 -0700 Subject: [PATCH 10/16] use `CFAutorelease` on CoreFoundation typed public key copies --- .../Framework/CHIPTests/MTRCertificateTests.m | 67 +++++++++++++------ .../CHIPTests/MTRCertificateValidityTests.m | 5 +- .../CHIPTests/MTRControllerAdvertisingTests.m | 5 +- .../Framework/CHIPTests/MTRControllerTests.m | 59 ++++++++++++---- .../Framework/CHIPTests/MTRFabricInfoTests.m | 5 +- .../CHIPTests/MTRPerControllerStorageTests.m | 6 +- 6 files changed, 111 insertions(+), 36 deletions(-) diff --git a/src/darwin/Framework/CHIPTests/MTRCertificateTests.m b/src/darwin/Framework/CHIPTests/MTRCertificateTests.m index c174f3321e5160..02f1357a1d3817 100644 --- a/src/darwin/Framework/CHIPTests/MTRCertificateTests.m +++ b/src/darwin/Framework/CHIPTests/MTRCertificateTests.m @@ -126,13 +126,16 @@ - (void)testGenerateIntermediateCert __auto_type * intermediateKeys = [[MTRTestKeys alloc] init]; XCTAssertNotNil(intermediateKeys); + __auto_type * intermediatePublicKey = intermediateKeys.copyPublicKey; + XCTAssert(intermediatePublicKey != NULL); + CFAutorelease(intermediatePublicKey); __auto_type * intermediateCert = [MTRCertificates createIntermediateCertificate:rootKeys - rootCertificate:rootCert - intermediatePublicKey:[intermediateKeys.copyPublicKey autorelease] - issuerID:nil - fabricID:nil - error:nil]; + rootCertificate:rootCert + intermediatePublicKey:intermediatePublicKey + issuerID:nil + fabricID:nil + error:nil]; XCTAssertNotNil(intermediateCert); // Test round-trip through TLV format. @@ -155,13 +158,16 @@ - (void)testGenerateIntermediateCertWithValidityPeriod __auto_type * intermediateKeys = [[MTRTestKeys alloc] init]; XCTAssertNotNil(intermediateKeys); + __auto_type * intermediatePublicKey = intermediateKeys.copyPublicKey; + XCTAssert(intermediatePublicKey != NULL); + CFAutorelease(intermediatePublicKey); __auto_type * startDate = [MTRCertificateTests startDateWithTimeIntervalSinceNow:300]; __auto_type * validityPeriod = [[NSDateInterval alloc] initWithStartDate:startDate duration:400]; __auto_type * intermediateCert = [MTRCertificates createIntermediateCertificate:rootKeys rootCertificate:rootCert - intermediatePublicKey:[intermediateKeys.copyPublicKey autorelease] + intermediatePublicKey:intermediatePublicKey issuerID:nil fabricID:nil validityPeriod:validityPeriod @@ -192,13 +198,16 @@ - (void)testGenerateIntermediateCertWithInfiniteValidity __auto_type * intermediateKeys = [[MTRTestKeys alloc] init]; XCTAssertNotNil(intermediateKeys); + __auto_type * intermediatePublicKey = intermediateKeys.copyPublicKey; + XCTAssert(intermediatePublicKey != NULL); + CFAutorelease(intermediatePublicKey); __auto_type * startDate = [MTRCertificateTests startDateWithTimeIntervalSinceNow:300]; __auto_type * validityPeriod = [[NSDateInterval alloc] initWithStartDate:startDate endDate:[NSDate distantFuture]]; __auto_type * intermediateCert = [MTRCertificates createIntermediateCertificate:rootKeys rootCertificate:rootCert - intermediatePublicKey:[intermediateKeys.copyPublicKey autorelease] + intermediatePublicKey:intermediatePublicKey issuerID:nil fabricID:nil validityPeriod:validityPeriod @@ -229,6 +238,9 @@ - (void)testGenerateOperationalCertNoIntermediate __auto_type * operationalKeys = [[MTRTestKeys alloc] init]; XCTAssertNotNil(operationalKeys); + __auto_type * operationalPublicKey = [operationalKeys copyPublicKey]; + XCTAssert(operationalPublicKey != NULL); + CFAutorelease(operationalPublicKey); __auto_type * cats = [[NSMutableSet alloc] initWithCapacity:3]; // High bits are identifier, low bits are version. @@ -238,7 +250,7 @@ - (void)testGenerateOperationalCertNoIntermediate __auto_type * operationalCert = [MTRCertificates createOperationalCertificate:rootKeys signingCertificate:rootCert - operationalPublicKey:[operationalKeys.copyPublicKey autorelease] + operationalPublicKey:operationalPublicKey fabricID:@1 nodeID:@1 caseAuthenticatedTags:cats @@ -265,6 +277,9 @@ - (void)testGenerateOperationalCertNoIntermediateWithValidityPeriod __auto_type * operationalKeys = [[MTRTestKeys alloc] init]; XCTAssertNotNil(operationalKeys); + __auto_type * operationalPublicKey = [operationalKeys copyPublicKey]; + XCTAssert(operationalPublicKey != NULL); + CFAutorelease(operationalPublicKey); __auto_type * cats = [[NSMutableSet alloc] initWithCapacity:3]; // High bits are identifier, low bits are version. @@ -277,7 +292,7 @@ - (void)testGenerateOperationalCertNoIntermediateWithValidityPeriod __auto_type * operationalCert = [MTRCertificates createOperationalCertificate:rootKeys signingCertificate:rootCert - operationalPublicKey:[operationalKeys.copyPublicKey autorelease] + operationalPublicKey:operationalPublicKey fabricID:@1 nodeID:@1 caseAuthenticatedTags:cats @@ -309,6 +324,9 @@ - (void)testGenerateOperationalCertNoIntermediateWithInfiniteValidity __auto_type * operationalKeys = [[MTRTestKeys alloc] init]; XCTAssertNotNil(operationalKeys); + __auto_type * operationalPublicKey = [operationalKeys copyPublicKey]; + XCTAssert(operationalPublicKey != NULL); + CFAutorelease(operationalPublicKey); __auto_type * cats = [[NSMutableSet alloc] initWithCapacity:3]; // High bits are identifier, low bits are version. @@ -321,7 +339,7 @@ - (void)testGenerateOperationalCertNoIntermediateWithInfiniteValidity __auto_type * operationalCert = [MTRCertificates createOperationalCertificate:rootKeys signingCertificate:rootCert - operationalPublicKey:[operationalKeys.copyPublicKey autorelease] + operationalPublicKey:operationalPublicKey fabricID:@1 nodeID:@1 caseAuthenticatedTags:cats @@ -353,10 +371,13 @@ - (void)testGenerateOperationalCertWithIntermediate __auto_type * intermediateKeys = [[MTRTestKeys alloc] init]; XCTAssertNotNil(intermediateKeys); + __auto_type * intermediatePublicKey = [intermediateKeys copyPublicKey]; + XCTAssert(intermediatePublicKey != NULL); + CFAutorelease(intermediatePublicKey); __auto_type * intermediateCert = [MTRCertificates createIntermediateCertificate:rootKeys rootCertificate:rootCert - intermediatePublicKey:[intermediateKeys.copyPublicKey autorelease] + intermediatePublicKey:intermediatePublicKey issuerID:nil fabricID:nil error:nil]; @@ -364,10 +385,13 @@ - (void)testGenerateOperationalCertWithIntermediate __auto_type * operationalKeys = [[MTRTestKeys alloc] init]; XCTAssertNotNil(operationalKeys); + __auto_type * operationalPublicKey = [operationalKeys copyPublicKey]; + XCTAssert(operationalPublicKey != NULL); + CFAutorelease(operationalPublicKey); __auto_type * operationalCert = [MTRCertificates createOperationalCertificate:intermediateKeys signingCertificate:intermediateCert - operationalPublicKey:[operationalKeys.copyPublicKey autorelease] + operationalPublicKey:operationalPublicKey fabricID:@1 nodeID:@1 caseAuthenticatedTags:nil @@ -394,6 +418,9 @@ - (void)testGenerateOperationalCertErrorCases __auto_type * operationalKeys = [[MTRTestKeys alloc] init]; XCTAssertNotNil(operationalKeys); + __auto_type * operationalPublicKey = [operationalKeys copyPublicKey]; + XCTAssert(operationalPublicKey != NULL); + CFAutorelease(operationalPublicKey); __auto_type * longCats = [[NSMutableSet alloc] initWithCapacity:4]; [longCats addObject:@0x00010001]; @@ -415,7 +442,7 @@ - (void)testGenerateOperationalCertErrorCases // Check basic case works __auto_type * operationalCert = [MTRCertificates createOperationalCertificate:rootKeys signingCertificate:rootCert - operationalPublicKey:[operationalKeys.copyPublicKey autorelease] + operationalPublicKey:operationalPublicKey fabricID:@1 nodeID:@1 caseAuthenticatedTags:nil @@ -425,7 +452,7 @@ - (void)testGenerateOperationalCertErrorCases // CATs too long operationalCert = [MTRCertificates createOperationalCertificate:rootKeys signingCertificate:rootCert - operationalPublicKey:[operationalKeys.copyPublicKey autorelease] + operationalPublicKey:operationalPublicKey fabricID:@1 nodeID:@1 caseAuthenticatedTags:longCats @@ -435,7 +462,7 @@ - (void)testGenerateOperationalCertErrorCases // Multiple CATs with the same identifier but different versions operationalCert = [MTRCertificates createOperationalCertificate:rootKeys signingCertificate:rootCert - operationalPublicKey:[operationalKeys.copyPublicKey autorelease] + operationalPublicKey:operationalPublicKey fabricID:@1 nodeID:@1 caseAuthenticatedTags:catsWithSameIdentifier @@ -445,7 +472,7 @@ - (void)testGenerateOperationalCertErrorCases // CAT with invalid version operationalCert = [MTRCertificates createOperationalCertificate:rootKeys signingCertificate:rootCert - operationalPublicKey:[operationalKeys.copyPublicKey autorelease] + operationalPublicKey:operationalPublicKey fabricID:@1 nodeID:@1 caseAuthenticatedTags:catsWithInvalidVersion @@ -455,7 +482,7 @@ - (void)testGenerateOperationalCertErrorCases // Signing key mismatch operationalCert = [MTRCertificates createOperationalCertificate:operationalKeys signingCertificate:rootCert - operationalPublicKey:[operationalKeys.copyPublicKey autorelease] + operationalPublicKey:operationalPublicKey fabricID:@1 nodeID:@1 caseAuthenticatedTags:nil @@ -465,7 +492,7 @@ - (void)testGenerateOperationalCertErrorCases // Invalid fabric id operationalCert = [MTRCertificates createOperationalCertificate:rootKeys signingCertificate:rootCert - operationalPublicKey:[operationalKeys.copyPublicKey autorelease] + operationalPublicKey:operationalPublicKey fabricID:@0 nodeID:@1 caseAuthenticatedTags:nil @@ -475,7 +502,7 @@ - (void)testGenerateOperationalCertErrorCases // Undefined node id operationalCert = [MTRCertificates createOperationalCertificate:rootKeys signingCertificate:rootCert - operationalPublicKey:[operationalKeys.copyPublicKey autorelease] + operationalPublicKey:operationalPublicKey fabricID:@1 nodeID:@0 caseAuthenticatedTags:nil @@ -485,7 +512,7 @@ - (void)testGenerateOperationalCertErrorCases // Non-operational node id operationalCert = [MTRCertificates createOperationalCertificate:rootKeys signingCertificate:rootCert - operationalPublicKey:[operationalKeys.copyPublicKey autorelease] + operationalPublicKey:operationalPublicKey fabricID:@1 nodeID:@(0xFFFFFFFFFFFFFFFFLLU) caseAuthenticatedTags:nil diff --git a/src/darwin/Framework/CHIPTests/MTRCertificateValidityTests.m b/src/darwin/Framework/CHIPTests/MTRCertificateValidityTests.m index 3c405d9f7f45e1..719be071063fbe 100644 --- a/src/darwin/Framework/CHIPTests/MTRCertificateValidityTests.m +++ b/src/darwin/Framework/CHIPTests/MTRCertificateValidityTests.m @@ -259,10 +259,13 @@ - (void)initStack:(MTRTestCertificateIssuer *)certificateIssuer __auto_type * controllerOperationalKeys = [[MTRTestKeys alloc] init]; XCTAssertNotNil(controllerOperationalKeys); + __auto_type * controllerPublicKey = controllerOperationalKeys.copyPublicKey; + XCTAssert(controllerPublicKey != NULL); + CFAutorelease(controllerPublicKey); __auto_type * controllerOperationalCert = [certificateIssuer issueOperationalCertificateForNode:@(kControllerId) - operationalPublicKey:[controllerOperationalKeys.copyPublicKey autorelease]]; + operationalPublicKey:controllerPublicKey]; XCTAssertNotNil(controllerOperationalCert); __auto_type * params = [[MTRDeviceControllerStartupParams alloc] initWithIPK:certificateIssuer.rootKey.ipk diff --git a/src/darwin/Framework/CHIPTests/MTRControllerAdvertisingTests.m b/src/darwin/Framework/CHIPTests/MTRControllerAdvertisingTests.m index dace586d422034..bee6ee90df3524 100644 --- a/src/darwin/Framework/CHIPTests/MTRControllerAdvertisingTests.m +++ b/src/darwin/Framework/CHIPTests/MTRControllerAdvertisingTests.m @@ -164,10 +164,13 @@ - (nullable MTRDeviceController *)startControllerWithRootKeys:(MTRTestKeys *)roo __auto_type * root = [MTRCertificates createRootCertificate:rootKeys issuerID:@(1) fabricID:nil error:error]; XCTAssertNil(*error); XCTAssertNotNil(root); + __auto_type * publicKey = operationalKeys.copyPublicKey; + XCTAssert(publicKey != NULL); + CFAutorelease(publicKey); __auto_type * operational = [MTRCertificates createOperationalCertificate:rootKeys signingCertificate:root - operationalPublicKey:[operationalKeys.copyPublicKey autorelease] + operationalPublicKey:publicKey fabricID:fabricID nodeID:nodeID caseAuthenticatedTags:nil diff --git a/src/darwin/Framework/CHIPTests/MTRControllerTests.m b/src/darwin/Framework/CHIPTests/MTRControllerTests.m index 03907e0a02e377..c904e5c9588235 100644 --- a/src/darwin/Framework/CHIPTests/MTRControllerTests.m +++ b/src/darwin/Framework/CHIPTests/MTRControllerTests.m @@ -620,10 +620,13 @@ - (void)testControllerSignerKeyWithIntermediate __auto_type * intermediateKeys = [[MTRTestKeys alloc] init]; XCTAssertNotNil(intermediateKeys); + __auto_type * intermediatePublicKey = intermediateKeys.copyPublicKey; + XCTAssert(intermediatePublicKey != NULL); + CFAutorelease(intermediatePublicKey); __auto_type * intermediate = [MTRCertificates createIntermediateCertificate:rootKeys rootCertificate:root - intermediatePublicKey:[intermediateKeys.copyPublicKey autorelease] + intermediatePublicKey:intermediatePublicKey issuerID:nil fabricID:nil error:nil]; @@ -860,10 +863,12 @@ - (void)testControllerRotateToICA __auto_type * intermediateKeys = [[MTRTestKeys alloc] init]; XCTAssertNotNil(intermediateKeys); + __auto_type * intermediatePublicKey = intermediateKeys.copyPublicKey; + CFAutorelease(intermediatePublicKey); __auto_type * intermediate = [MTRCertificates createIntermediateCertificate:rootKeys rootCertificate:root - intermediatePublicKey:[intermediateKeys.copyPublicKey autorelease] + intermediatePublicKey:intermediatePublicKey issuerID:nil fabricID:nil error:nil]; @@ -922,10 +927,13 @@ - (void)testControllerRotateFromICA __auto_type * intermediateKeys = [[MTRTestKeys alloc] init]; XCTAssertNotNil(intermediateKeys); + __auto_type * intermediatePublicKey = [intermediateKeys copyPublicKey]; + XCTAssert(intermediatePublicKey != NULL); + CFAutorelease(intermediatePublicKey); __auto_type * intermediate = [MTRCertificates createIntermediateCertificate:rootKeys rootCertificate:root - intermediatePublicKey:[intermediateKeys.copyPublicKey autorelease] + intermediatePublicKey:intermediatePublicKey issuerID:nil fabricID:nil error:nil]; @@ -986,10 +994,13 @@ - (void)testControllerRotateICA __auto_type * intermediateKeys1 = [[MTRTestKeys alloc] init]; XCTAssertNotNil(intermediateKeys1); + __auto_type * intermediate1PublicKey = [intermediateKeys1 copyPublicKey]; + XCTAssert(intermediate1PublicKey != NULL); + CFAutorelease(intermediate1PublicKey); __auto_type * intermediate1 = [MTRCertificates createIntermediateCertificate:rootKeys rootCertificate:root - intermediatePublicKey:[intermediateKeys1.copyPublicKey autorelease] + intermediatePublicKey:intermediate1PublicKey issuerID:nil fabricID:nil error:nil]; @@ -997,10 +1008,13 @@ - (void)testControllerRotateICA __auto_type * intermediateKeys2 = [[MTRTestKeys alloc] init]; XCTAssertNotNil(intermediateKeys2); + __auto_type * intermediate2PublicKey = [intermediateKeys2 copyPublicKey]; + XCTAssert(intermediate2PublicKey != NULL); + CFAutorelease(intermediate2PublicKey); __auto_type * intermediate2 = [MTRCertificates createIntermediateCertificate:rootKeys rootCertificate:root - intermediatePublicKey:[intermediateKeys2.copyPublicKey autorelease] + intermediatePublicKey:intermediate2PublicKey issuerID:nil fabricID:nil error:nil]; @@ -1061,10 +1075,13 @@ - (void)testControllerICAWithoutRoot __auto_type * intermediateKeys = [[MTRTestKeys alloc] init]; XCTAssertNotNil(intermediateKeys); + __auto_type * intermediatePublicKey = [intermediateKeys copyPublicKey]; + XCTAssert(intermediatePublicKey != NULL); + CFAutorelease(intermediatePublicKey); __auto_type * intermediate = [MTRCertificates createIntermediateCertificate:rootKeys rootCertificate:root - intermediatePublicKey:[intermediateKeys.copyPublicKey autorelease] + intermediatePublicKey:intermediatePublicKey issuerID:nil fabricID:nil error:nil]; @@ -1104,10 +1121,13 @@ - (void)testControllerProvideFullCertChain __auto_type * intermediateKeys = [[MTRTestKeys alloc] init]; XCTAssertNotNil(intermediateKeys); + __auto_type * intermediatePublicKey = [intermediateKeys copyPublicKey]; + XCTAssert(intermediatePublicKey != NULL); + CFAutorelease(intermediatePublicKey); __auto_type * intermediate = [MTRCertificates createIntermediateCertificate:rootKeys rootCertificate:root - intermediatePublicKey:[intermediateKeys.copyPublicKey autorelease] + intermediatePublicKey:intermediatePublicKey issuerID:nil fabricID:nil error:nil]; @@ -1115,10 +1135,13 @@ - (void)testControllerProvideFullCertChain __auto_type * operationalKeys = [[MTRTestKeys alloc] init]; XCTAssertNotNil(operationalKeys); + __auto_type * operationalPublicKey = [operationalKeys copyPublicKey]; + XCTAssert(operationalPublicKey != NULL); + CFAutorelease(operationalPublicKey); __auto_type * operational = [MTRCertificates createOperationalCertificate:intermediateKeys signingCertificate:intermediate - operationalPublicKey:[operationalKeys.copyPublicKey autorelease] + operationalPublicKey:operationalPublicKey fabricID:@123 nodeID:@456 caseAuthenticatedTags:nil @@ -1179,10 +1202,13 @@ - (void)testControllerProvideCertChainNoICA __auto_type * operationalKeys = [[MTRTestKeys alloc] init]; XCTAssertNotNil(operationalKeys); + __auto_type * operationalPublicKey = [operationalKeys copyPublicKey]; + XCTAssert(operationalPublicKey != NULL); + CFAutorelease(operationalPublicKey); __auto_type * operational = [MTRCertificates createOperationalCertificate:rootKeys signingCertificate:root - operationalPublicKey:[operationalKeys.copyPublicKey autorelease] + operationalPublicKey:operationalPublicKey fabricID:@123 nodeID:@456 caseAuthenticatedTags:nil @@ -1229,10 +1255,13 @@ - (void)testControllerCertChainFabricMismatchRoot __auto_type * operationalKeys = [[MTRTestKeys alloc] init]; XCTAssertNotNil(operationalKeys); + __auto_type * operationalPublicKey = [operationalKeys copyPublicKey]; + XCTAssert(operationalPublicKey != NULL); + CFAutorelease(operationalPublicKey); __auto_type * operational = [MTRCertificates createOperationalCertificate:rootKeys signingCertificate:root - operationalPublicKey:[operationalKeys.copyPublicKey autorelease] + operationalPublicKey:operationalPublicKey fabricID:@123 nodeID:@456 caseAuthenticatedTags:nil @@ -1273,10 +1302,13 @@ - (void)testControllerCertChainFabricMismatchIntermediate __auto_type * intermediateKeys = [[MTRTestKeys alloc] init]; XCTAssertNotNil(intermediateKeys); + __auto_type * intermediatePublicKey = [intermediateKeys copyPublicKey]; + XCTAssert(intermediatePublicKey != NULL); + CFAutorelease(intermediatePublicKey); __auto_type * intermediate = [MTRCertificates createIntermediateCertificate:rootKeys rootCertificate:root - intermediatePublicKey:[intermediateKeys.copyPublicKey autorelease] + intermediatePublicKey:intermediatePublicKey issuerID:nil fabricID:@111 error:nil]; @@ -1284,10 +1316,13 @@ - (void)testControllerCertChainFabricMismatchIntermediate __auto_type * operationalKeys = [[MTRTestKeys alloc] init]; XCTAssertNotNil(operationalKeys); + __auto_type * operationalPublicKey = [operationalKeys copyPublicKey]; + XCTAssert(operationalPublicKey != NULL); + CFAutorelease(operationalPublicKey); __auto_type * operational = [MTRCertificates createOperationalCertificate:intermediateKeys signingCertificate:intermediate - operationalPublicKey:[operationalKeys.copyPublicKey autorelease] + operationalPublicKey:operationalPublicKey fabricID:@123 nodeID:@456 caseAuthenticatedTags:nil diff --git a/src/darwin/Framework/CHIPTests/MTRFabricInfoTests.m b/src/darwin/Framework/CHIPTests/MTRFabricInfoTests.m index 5cefe40b633853..acdc0ea45ff572 100644 --- a/src/darwin/Framework/CHIPTests/MTRFabricInfoTests.m +++ b/src/darwin/Framework/CHIPTests/MTRFabricInfoTests.m @@ -157,10 +157,13 @@ - (void)testFabricInfoTwoFabrics __auto_type * intermediateKeys = [[MTRTestKeys alloc] init]; XCTAssertNotNil(intermediateKeys); + __auto_type * intermediatePublicKey = intermediateKeys.copyPublicKey; + XCTAssert(intermediatePublicKey != NULL); + CFAutorelease(intermediatePublicKey); __auto_type * intermediate = [MTRCertificates createIntermediateCertificate:rootKeys rootCertificate:root - intermediatePublicKey:[intermediateKeys.copyPublicKey autorelease] + intermediatePublicKey:intermediatePublicKey issuerID:nil fabricID:nil error:nil]; diff --git a/src/darwin/Framework/CHIPTests/MTRPerControllerStorageTests.m b/src/darwin/Framework/CHIPTests/MTRPerControllerStorageTests.m index 9c0d94a0ce89c0..73b17734eb93b9 100644 --- a/src/darwin/Framework/CHIPTests/MTRPerControllerStorageTests.m +++ b/src/darwin/Framework/CHIPTests/MTRPerControllerStorageTests.m @@ -403,9 +403,13 @@ - (nullable MTRDeviceController *)startControllerWithRootKeys:(MTRTestKeys *)roo XCTAssertNil(*error); XCTAssertNotNil(root); + __auto_type * operationalPublicKey = [operationalKeys copyPublicKey]; + XCTAssert(operationalPublicKey != NULL); + CFAutorelease(operationalPublicKey); + __auto_type * operational = [MTRCertificates createOperationalCertificate:rootKeys signingCertificate:root - operationalPublicKey:[operationalKeys.copyPublicKey autorelease] + operationalPublicKey:operationalPublicKey fabricID:fabricID nodeID:nodeID caseAuthenticatedTags:caseAuthenticatedTags From a3ba7d23e979c5c5f26d77b1984c8074e1ec0828 Mon Sep 17 00:00:00 2001 From: Kiel Oleson Date: Thu, 31 Oct 2024 11:31:44 -0700 Subject: [PATCH 11/16] fix indent --- src/darwin/Framework/CHIPTests/MTRCertificateTests.m | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/darwin/Framework/CHIPTests/MTRCertificateTests.m b/src/darwin/Framework/CHIPTests/MTRCertificateTests.m index 02f1357a1d3817..1dfbfd5293af62 100644 --- a/src/darwin/Framework/CHIPTests/MTRCertificateTests.m +++ b/src/darwin/Framework/CHIPTests/MTRCertificateTests.m @@ -131,11 +131,11 @@ - (void)testGenerateIntermediateCert CFAutorelease(intermediatePublicKey); __auto_type * intermediateCert = [MTRCertificates createIntermediateCertificate:rootKeys - rootCertificate:rootCert - intermediatePublicKey:intermediatePublicKey - issuerID:nil - fabricID:nil - error:nil]; + rootCertificate:rootCert + intermediatePublicKey:intermediatePublicKey + issuerID:nil + fabricID:nil + error:nil]; XCTAssertNotNil(intermediateCert); // Test round-trip through TLV format. From 32e8525e44719be84bd7a754cc74be7b5f200a06 Mon Sep 17 00:00:00 2001 From: Kiel Oleson Date: Thu, 31 Oct 2024 13:52:51 -0700 Subject: [PATCH 12/16] implement `copyPublicKey` for `MTRTestKeys`; add TODO note about optional method calls --- src/darwin/Framework/CHIPTests/MTRCertificateTests.m | 3 +++ src/darwin/Framework/CHIPTests/TestHelpers/MTRTestKeys.m | 7 +++++++ 2 files changed, 10 insertions(+) diff --git a/src/darwin/Framework/CHIPTests/MTRCertificateTests.m b/src/darwin/Framework/CHIPTests/MTRCertificateTests.m index 1dfbfd5293af62..a9f49b0d6a9c83 100644 --- a/src/darwin/Framework/CHIPTests/MTRCertificateTests.m +++ b/src/darwin/Framework/CHIPTests/MTRCertificateTests.m @@ -126,6 +126,9 @@ - (void)testGenerateIntermediateCert __auto_type * intermediateKeys = [[MTRTestKeys alloc] init]; XCTAssertNotNil(intermediateKeys); + // TODO: there are several places in our tests where we call `copyPublicKey` on objects without first + // seeing if it's implemented by the receiver. (the `MTRKeypair` protocol says both `publicKey` and + // `copyPublicKey` are optional.) __auto_type * intermediatePublicKey = intermediateKeys.copyPublicKey; XCTAssert(intermediatePublicKey != NULL); CFAutorelease(intermediatePublicKey); diff --git a/src/darwin/Framework/CHIPTests/TestHelpers/MTRTestKeys.m b/src/darwin/Framework/CHIPTests/TestHelpers/MTRTestKeys.m index e6a74f25bdeccf..ac05011350c881 100644 --- a/src/darwin/Framework/CHIPTests/TestHelpers/MTRTestKeys.m +++ b/src/darwin/Framework/CHIPTests/TestHelpers/MTRTestKeys.m @@ -31,6 +31,13 @@ - (NSData *)publicKeyData return (__bridge_transfer NSData *) SecKeyCopyExternalRepresentation([self publicKey], nil); } +- (SecKeyRef)copyPublicKey { + // because this varies significantly from the actual implementations, + // it's probably not a great idea to rely on this for memory leak behavior information. + CFRetain(_publicKey); + return _publicKey; +} + - (instancetype)init { if (!(self = [super init])) { From 9c1d3592b3599f66fbcc1583eb08bdd5e4263edf Mon Sep 17 00:00:00 2001 From: Kiel Oleson Date: Thu, 31 Oct 2024 18:21:59 -0700 Subject: [PATCH 13/16] remove comment it's a test; this is the best we can do with an optional protocol method --- src/darwin/Framework/CHIPTests/MTRCertificateTests.m | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/darwin/Framework/CHIPTests/MTRCertificateTests.m b/src/darwin/Framework/CHIPTests/MTRCertificateTests.m index a9f49b0d6a9c83..d5be54e43816e9 100644 --- a/src/darwin/Framework/CHIPTests/MTRCertificateTests.m +++ b/src/darwin/Framework/CHIPTests/MTRCertificateTests.m @@ -126,10 +126,8 @@ - (void)testGenerateIntermediateCert __auto_type * intermediateKeys = [[MTRTestKeys alloc] init]; XCTAssertNotNil(intermediateKeys); - // TODO: there are several places in our tests where we call `copyPublicKey` on objects without first - // seeing if it's implemented by the receiver. (the `MTRKeypair` protocol says both `publicKey` and - // `copyPublicKey` are optional.) - __auto_type * intermediatePublicKey = intermediateKeys.copyPublicKey; + + __auto_type * intermediatePublicKey = [intermediateKeys copyPublicKey]; XCTAssert(intermediatePublicKey != NULL); CFAutorelease(intermediatePublicKey); From 7707afc337f09a29d4b553314f62df60b46af963 Mon Sep 17 00:00:00 2001 From: Kiel Oleson Date: Thu, 31 Oct 2024 18:22:33 -0700 Subject: [PATCH 14/16] consistent formatting for `copyPublicKey` calls --- src/darwin/Framework/CHIPTests/MTRControllerTests.m | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/darwin/Framework/CHIPTests/MTRControllerTests.m b/src/darwin/Framework/CHIPTests/MTRControllerTests.m index c904e5c9588235..afd755f8e5e757 100644 --- a/src/darwin/Framework/CHIPTests/MTRControllerTests.m +++ b/src/darwin/Framework/CHIPTests/MTRControllerTests.m @@ -620,7 +620,7 @@ - (void)testControllerSignerKeyWithIntermediate __auto_type * intermediateKeys = [[MTRTestKeys alloc] init]; XCTAssertNotNil(intermediateKeys); - __auto_type * intermediatePublicKey = intermediateKeys.copyPublicKey; + __auto_type * intermediatePublicKey = [intermediateKeys copyPublicKey]; XCTAssert(intermediatePublicKey != NULL); CFAutorelease(intermediatePublicKey); From c70e031aa2bd2dcbeddb7bc88e304370eff032e8 Mon Sep 17 00:00:00 2001 From: Kiel Oleson Date: Thu, 31 Oct 2024 18:24:12 -0700 Subject: [PATCH 15/16] reformat `copyPublicKey` in `MTRTestKeys` remove comment - not this method's job to worry about other implementation's potential side-effects --- src/darwin/Framework/CHIPTests/TestHelpers/MTRTestKeys.m | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/darwin/Framework/CHIPTests/TestHelpers/MTRTestKeys.m b/src/darwin/Framework/CHIPTests/TestHelpers/MTRTestKeys.m index ac05011350c881..090593a41babfb 100644 --- a/src/darwin/Framework/CHIPTests/TestHelpers/MTRTestKeys.m +++ b/src/darwin/Framework/CHIPTests/TestHelpers/MTRTestKeys.m @@ -31,9 +31,8 @@ - (NSData *)publicKeyData return (__bridge_transfer NSData *) SecKeyCopyExternalRepresentation([self publicKey], nil); } -- (SecKeyRef)copyPublicKey { - // because this varies significantly from the actual implementations, - // it's probably not a great idea to rely on this for memory leak behavior information. +- (SecKeyRef)copyPublicKey +{ CFRetain(_publicKey); return _publicKey; } From d5bfe8dc578f636a919aa0fcc0a2ca3b1b36c045 Mon Sep 17 00:00:00 2001 From: Justin Wood Date: Thu, 31 Oct 2024 18:25:38 -0700 Subject: [PATCH 16/16] Update src/darwin/Framework/CHIP/MTRKeypair.h Co-authored-by: Boris Zbarsky --- src/darwin/Framework/CHIP/MTRKeypair.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/darwin/Framework/CHIP/MTRKeypair.h b/src/darwin/Framework/CHIP/MTRKeypair.h index e370808701d8e7..d7381fe096f900 100644 --- a/src/darwin/Framework/CHIP/MTRKeypair.h +++ b/src/darwin/Framework/CHIP/MTRKeypair.h @@ -43,7 +43,7 @@ NS_ASSUME_NONNULL_BEGIN * @brief Returns public key for the keypair without adding a reference. DEPRECATED - please use copyPublicKey, otherwise this will leak. */ -- (SecKeyRef)publicKey MTR_DEPRECATED("Please implement copyPublicKey, this will leak otherwise", ios(16.4, 18.3), macos(13.3, 15.3), watchos(9.4, 11.3), tvos(16.4, 18.3)); +- (SecKeyRef)publicKey MTR_DEPRECATED("Please implement copyPublicKey, this will leak otherwise", ios(16.1, 18.3), macos(13.0, 15.3), watchos(9.1, 11.3), tvos(16.1, 18.3)); /** * @brief A function to sign a message using ECDSA