Skip to content

Commit

Permalink
Revert "Add new API so it's possible to not leak a key (project-chip#…
Browse files Browse the repository at this point in the history 
…36299)" (project-chip#36339)

This reverts commit 08024d2 because it's
failing CI as committed.
  • Loading branch information
@bzbarsky-apple
bzbarsky-apple authored Nov 1, 2024
1 parent b0cc28a commit b54eaf8
Show file tree
Hide file tree
Showing 12 changed files with 41 additions and 170 deletions.
2 changes: 1 addition & 1 deletion examples/darwin-framework-tool/commands/common/CHIPToolKeypair.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
@interface CHIPToolKeypair : NSObject <MTRKeypair>
- (BOOL)initialize;
- (NSData *)signMessageECDSA_RAW:(NSData *)message;
- (SecKeyRef)copyPublicKey;
- (SecKeyRef)publicKey;
- (CHIP_ERROR)Serialize:(chip::Crypto::P256SerializedKeypair &)output;
- (CHIP_ERROR)Deserialize:(chip::Crypto::P256SerializedKeypair &)input;
- (CHIP_ERROR)createOrLoadKeys:(id)storage;
Expand Down
10 changes: 2 additions & 8 deletions examples/darwin-framework-tool/commands/common/CHIPToolKeypair.mm
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ - (NSData *)signMessageECDSA_RAW:(NSData *)message
return out_signature;
}

- (SecKeyRef)copyPublicKey
- (SecKeyRef)publicKey
{
if (_mPublicKey == nil) {
chip::Crypto::P256PublicKey publicKey = _mKeyPair.Pubkey();
Expand All @@ -79,13 +79,7 @@ - (SecKeyRef)copyPublicKey
};
_mPublicKey = SecKeyCreateWithData((__bridge CFDataRef) publicKeyNSData, (__bridge CFDictionaryRef) attributes, nullptr);
}

if (_mPublicKey) {
CFRetain(_mPublicKey);
return _mPublicKey;
}

return NULL;
return _mPublicKey;
}

- (CHIP_ERROR)Deserialize:(chip::Crypto::P256SerializedKeypair &)input
Expand Down
19 changes: 1 addition & 18 deletions src/darwin/Framework/CHIP/MTRCertificates.mm
View file
Original file line number Diff line number Diff line change
Expand Up @@ -152,24 +152,7 @@ + (MTRCertificateDERBytes _Nullable)createOperationalCertificate:(id<MTRKeypair>
+ (BOOL)keypair:(id<MTRKeypair>)keypair matchesCertificate:(NSData *)certificate
{
P256PublicKey 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;
}

CHIP_ERROR err = MTRP256KeypairBridge::MatterPubKeyFromSecKeyRef(keypair.publicKey, &keypairPubKey);
if (err != CHIP_NO_ERROR) {
MTR_LOG_ERROR("Can't extract public key from keypair: %s", ErrorStr(err));
return NO;
Expand Down
19 changes: 1 addition & 18 deletions src/darwin/Framework/CHIP/MTRDeviceControllerFactory.mm
View file
Original file line number Diff line number Diff line change
Expand Up @@ -819,24 +819,7 @@ - (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.
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;
}

CHIP_ERROR err = MTRP256KeypairBridge::MatterPubKeyFromSecKeyRef(params.nocSigner.publicKey, &pubKey);
if (err != CHIP_NO_ERROR) {
MTR_LOG_ERROR("Can't extract public key from MTRKeypair: %s", ErrorStr(err));
return NO;
Expand Down
15 changes: 4 additions & 11 deletions src/darwin/Framework/CHIP/MTRKeypair.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
*/

#import <Foundation/Foundation.h>
#import <Matter/Matter.h>
#import <Security/Security.h>

NS_ASSUME_NONNULL_BEGIN
Expand All @@ -32,19 +31,13 @@ NS_ASSUME_NONNULL_BEGIN
* framework APIs.
*/
@protocol MTRKeypair <NSObject>

@optional
@required
/**
* @brief Returns a copy of the public key for the keypair.
* @brief Return public key for the keypair.
*/
- (SecKeyRef)copyPublicKey MTR_NEWLY_AVAILABLE;

/**
* @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.1, 18.3), macos(13.0, 15.3), watchos(9.1, 11.3), tvos(16.1, 18.3));
- (SecKeyRef)publicKey;

@optional
/**
* @brief A function to sign a message using ECDSA
*
Expand Down
60 changes: 16 additions & 44 deletions src/darwin/Framework/CHIPTests/MTRCertificateTests.m
View file
Original file line number Diff line number Diff line change
Expand Up @@ -127,13 +127,9 @@ - (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:intermediatePublicKey
intermediatePublicKey:intermediateKeys.publicKey
issuerID:nil
fabricID:nil
error:nil];
Expand All @@ -159,16 +155,13 @@ - (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:intermediatePublicKey
intermediatePublicKey:intermediateKeys.publicKey
issuerID:nil
fabricID:nil
validityPeriod:validityPeriod
Expand Down Expand Up @@ -199,16 +192,13 @@ - (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:intermediatePublicKey
intermediatePublicKey:intermediateKeys.publicKey
issuerID:nil
fabricID:nil
validityPeriod:validityPeriod
Expand Down Expand Up @@ -239,9 +229,6 @@ - (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.
Expand All @@ -251,7 +238,7 @@ - (void)testGenerateOperationalCertNoIntermediate

__auto_type * operationalCert = [MTRCertificates createOperationalCertificate:rootKeys
signingCertificate:rootCert
operationalPublicKey:operationalPublicKey
operationalPublicKey:operationalKeys.publicKey
fabricID:@1
nodeID:@1
caseAuthenticatedTags:cats
Expand All @@ -278,9 +265,6 @@ - (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.
Expand All @@ -293,7 +277,7 @@ - (void)testGenerateOperationalCertNoIntermediateWithValidityPeriod

__auto_type * operationalCert = [MTRCertificates createOperationalCertificate:rootKeys
signingCertificate:rootCert
operationalPublicKey:operationalPublicKey
operationalPublicKey:operationalKeys.publicKey
fabricID:@1
nodeID:@1
caseAuthenticatedTags:cats
Expand Down Expand Up @@ -325,9 +309,6 @@ - (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.
Expand All @@ -340,7 +321,7 @@ - (void)testGenerateOperationalCertNoIntermediateWithInfiniteValidity

__auto_type * operationalCert = [MTRCertificates createOperationalCertificate:rootKeys
signingCertificate:rootCert
operationalPublicKey:operationalPublicKey
operationalPublicKey:operationalKeys.publicKey
fabricID:@1
nodeID:@1
caseAuthenticatedTags:cats
Expand Down Expand Up @@ -372,27 +353,21 @@ - (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:intermediatePublicKey
intermediatePublicKey:intermediateKeys.publicKey
issuerID:nil
fabricID:nil
error:nil];
XCTAssertNotNil(intermediateCert);

__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:operationalPublicKey
operationalPublicKey:operationalKeys.publicKey
fabricID:@1
nodeID:@1
caseAuthenticatedTags:nil
Expand All @@ -419,9 +394,6 @@ - (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];
Expand All @@ -443,7 +415,7 @@ - (void)testGenerateOperationalCertErrorCases
// Check basic case works
__auto_type * operationalCert = [MTRCertificates createOperationalCertificate:rootKeys
signingCertificate:rootCert
operationalPublicKey:operationalPublicKey
operationalPublicKey:operationalKeys.publicKey
fabricID:@1
nodeID:@1
caseAuthenticatedTags:nil
Expand All @@ -453,7 +425,7 @@ - (void)testGenerateOperationalCertErrorCases
// CATs too long
operationalCert = [MTRCertificates createOperationalCertificate:rootKeys
signingCertificate:rootCert
operationalPublicKey:operationalPublicKey
operationalPublicKey:operationalKeys.publicKey
fabricID:@1
nodeID:@1
caseAuthenticatedTags:longCats
Expand All @@ -463,7 +435,7 @@ - (void)testGenerateOperationalCertErrorCases
// Multiple CATs with the same identifier but different versions
operationalCert = [MTRCertificates createOperationalCertificate:rootKeys
signingCertificate:rootCert
operationalPublicKey:operationalPublicKey
operationalPublicKey:operationalKeys.publicKey
fabricID:@1
nodeID:@1
caseAuthenticatedTags:catsWithSameIdentifier
Expand All @@ -473,7 +445,7 @@ - (void)testGenerateOperationalCertErrorCases
// CAT with invalid version
operationalCert = [MTRCertificates createOperationalCertificate:rootKeys
signingCertificate:rootCert
operationalPublicKey:operationalPublicKey
operationalPublicKey:operationalKeys.publicKey
fabricID:@1
nodeID:@1
caseAuthenticatedTags:catsWithInvalidVersion
Expand All @@ -483,7 +455,7 @@ - (void)testGenerateOperationalCertErrorCases
// Signing key mismatch
operationalCert = [MTRCertificates createOperationalCertificate:operationalKeys
signingCertificate:rootCert
operationalPublicKey:operationalPublicKey
operationalPublicKey:operationalKeys.publicKey
fabricID:@1
nodeID:@1
caseAuthenticatedTags:nil
Expand All @@ -493,7 +465,7 @@ - (void)testGenerateOperationalCertErrorCases
// Invalid fabric id
operationalCert = [MTRCertificates createOperationalCertificate:rootKeys
signingCertificate:rootCert
operationalPublicKey:operationalPublicKey
operationalPublicKey:operationalKeys.publicKey
fabricID:@0
nodeID:@1
caseAuthenticatedTags:nil
Expand All @@ -503,7 +475,7 @@ - (void)testGenerateOperationalCertErrorCases
// Undefined node id
operationalCert = [MTRCertificates createOperationalCertificate:rootKeys
signingCertificate:rootCert
operationalPublicKey:operationalPublicKey
operationalPublicKey:operationalKeys.publicKey
fabricID:@1
nodeID:@0
caseAuthenticatedTags:nil
Expand All @@ -513,7 +485,7 @@ - (void)testGenerateOperationalCertErrorCases
// Non-operational node id
operationalCert = [MTRCertificates createOperationalCertificate:rootKeys
signingCertificate:rootCert
operationalPublicKey:operationalPublicKey
operationalPublicKey:operationalKeys.publicKey
fabricID:@1
nodeID:@(0xFFFFFFFFFFFFFFFFLLU)
caseAuthenticatedTags:nil
Expand Down
5 changes: 1 addition & 4 deletions src/darwin/Framework/CHIPTests/MTRCertificateValidityTests.m
View file
Original file line number Diff line number Diff line change
Expand Up @@ -259,13 +259,10 @@ - (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:controllerPublicKey];
operationalPublicKey:controllerOperationalKeys.publicKey];
XCTAssertNotNil(controllerOperationalCert);

__auto_type * params = [[MTRDeviceControllerStartupParams alloc] initWithIPK:certificateIssuer.rootKey.ipk
Expand Down
Loading

0 comments on commit b54eaf8

Please sign in to comment.