From a09ba9d0e7356200cf7abf55eb61428d7087c85e Mon Sep 17 00:00:00 2001 From: Boris Zbarsky Date: Tue, 11 Apr 2023 12:14:53 -0400 Subject: [PATCH] Add a way to specify CASE Authenticated Tags in MTRDeviceControllerStartupParams (#26038) Fixes https://github.com/project-chip/connectedhomeip/issues/22597 --- src/darwin/Framework/CHIP/MTRCertificates.h | 6 +- .../Framework/CHIP/MTRDeviceController.mm | 32 ++- .../CHIP/MTRDeviceControllerStartupParams.h | 22 +- .../CHIP/MTRDeviceControllerStartupParams.mm | 42 +++- .../Framework/CHIPTests/MTRControllerTests.m | 189 ++++++++++++++++++ 5 files changed, 272 insertions(+), 19 deletions(-) diff --git a/src/darwin/Framework/CHIP/MTRCertificates.h b/src/darwin/Framework/CHIP/MTRCertificates.h index 499991009fa468..268ee3e223ac30 100644 --- a/src/darwin/Framework/CHIP/MTRCertificates.h +++ b/src/darwin/Framework/CHIP/MTRCertificates.h @@ -91,9 +91,9 @@ NS_ASSUME_NONNULL_BEGIN * fabricID must be a valid Matter fabric id. * * caseAuthenticatedTags may be nil to indicate no CASE Authenticated Tags - * should be used. If caseAuthenticatedTags is not nil, it must have length at - * most 3 and the values in the array are expected to be 32-bit unsigned Case - * Authenticated Tag values. + * should be used. If caseAuthenticatedTags is not nil, it must contain at most + * 3 numbers, which are expected to be 32-bit unsigned Case Authenticated Tag + * values. * * On failure returns nil and if "error" is not null sets *error to the relevant * error. diff --git a/src/darwin/Framework/CHIP/MTRDeviceController.mm b/src/darwin/Framework/CHIP/MTRDeviceController.mm index 2192e9bda46e81..9026251bae5fd3 100644 --- a/src/darwin/Framework/CHIP/MTRDeviceController.mm +++ b/src/darwin/Framework/CHIP/MTRDeviceController.mm @@ -302,10 +302,34 @@ - (BOOL)startup:(MTRDeviceControllerStartupParamsInternal *)startupParams } else { chip::MutableByteSpan noc(nocBuffer); + chip::CATValues cats = chip::kUndefinedCATs; + if (startupParams.caseAuthenticatedTags != nil) { + unsigned long long tagCount = startupParams.caseAuthenticatedTags.count; + if (tagCount > chip::kMaxSubjectCATAttributeCount) { + MTR_LOG_ERROR("%llu CASE Authenticated Tags cannot be represented in a certificate.", tagCount); + return; + } + + size_t tagIndex = 0; + for (NSNumber * boxedTag in startupParams.caseAuthenticatedTags) { + if (!chip::CanCastTo(boxedTag.unsignedLongLongValue)) { + MTR_LOG_ERROR("0x%llx is not a valid CASE Authenticated Tag value.", boxedTag.unsignedLongLongValue); + return; + } + + auto tag = static_cast(boxedTag.unsignedLongLongValue); + if (!chip::IsValidCASEAuthTag(tag)) { + MTR_LOG_ERROR("0x%" PRIx32 " is not a valid CASE Authenticated Tag value.", tag); + return; + } + + cats.values[tagIndex++] = tag; + } + } + if (commissionerParams.operationalKeypair != nullptr) { errorCode = _operationalCredentialsDelegate->GenerateNOC(startupParams.nodeID.unsignedLongLongValue, - startupParams.fabricID.unsignedLongLongValue, chip::kUndefinedCATs, - commissionerParams.operationalKeypair->Pubkey(), noc); + startupParams.fabricID.unsignedLongLongValue, cats, commissionerParams.operationalKeypair->Pubkey(), noc); if ([self checkForStartError:errorCode logMsg:kErrorGenerateNOC]) { return; @@ -325,8 +349,8 @@ - (BOOL)startup:(MTRDeviceControllerStartupParamsInternal *)startupParams return; } - errorCode = _operationalCredentialsDelegate->GenerateNOC(startupParams.nodeID.unsignedLongLongValue, - startupParams.fabricID.unsignedLongLongValue, chip::kUndefinedCATs, pubKey, noc); + errorCode = _operationalCredentialsDelegate->GenerateNOC( + startupParams.nodeID.unsignedLongLongValue, startupParams.fabricID.unsignedLongLongValue, cats, pubKey, noc); if ([self checkForStartError:errorCode logMsg:kErrorGenerateNOC]) { return; diff --git a/src/darwin/Framework/CHIP/MTRDeviceControllerStartupParams.h b/src/darwin/Framework/CHIP/MTRDeviceControllerStartupParams.h index 13053af8643c41..49970304ad517e 100644 --- a/src/darwin/Framework/CHIP/MTRDeviceControllerStartupParams.h +++ b/src/darwin/Framework/CHIP/MTRDeviceControllerStartupParams.h @@ -127,16 +127,28 @@ NS_ASSUME_NONNULL_BEGIN * * ** nodeID is allowed to be nil to indicate that the existing operational node * id should be used. The existing operational keys will also be used, - * unless operationalKeypair is provided. + * unless operationalKeypair is provided. The existing caseAuthenticatedTags + * will be used. * * ** If nodeID is not nil, a new operational certificate will be generated for * the provided node id (even if that matches the existing node id), using * either the operationalKeypair if that is provided or a new randomly - * generated operational key. - * + * generated operational key, and using the provided caseAuthenticatedTags. */ @property (nonatomic, copy, nullable) NSNumber * nodeID API_AVAILABLE(ios(16.4), macos(13.3), watchos(9.4), tvos(16.4)); +/** + * CASE authenticated tags to use for this controller's operational certificate. + * + * Only allowed to be not nil if nodeID is not nil. In particular, if + * operationalCertificate is not nil, must be nil. The provided operational + * certificate will be used as-is. + * + * If not nil, must contain at most 3 numbers, which are expected to be 32-bit + * unsigned Case Authenticated Tag values. + */ +@property (nonatomic, copy, nullable) NSSet * caseAuthenticatedTags MTR_NEWLY_AVAILABLE; + /** * Root certificate, in X.509 DER form, to use. * @@ -219,8 +231,8 @@ NS_ASSUME_NONNULL_BEGIN * * If not nil, and if operationalCertificate is nil, a new operational * certificate will be generated for the given operationalKeypair. The node id - * will for that certificated will be determined as described in the - * documentation for nodeID. + * for that certificate will be determined as described in the documentation for + * nodeID. */ @property (nonatomic, strong, nullable) id operationalKeypair; diff --git a/src/darwin/Framework/CHIP/MTRDeviceControllerStartupParams.mm b/src/darwin/Framework/CHIP/MTRDeviceControllerStartupParams.mm index 4023220fc34396..771995bf01bc66 100644 --- a/src/darwin/Framework/CHIP/MTRDeviceControllerStartupParams.mm +++ b/src/darwin/Framework/CHIP/MTRDeviceControllerStartupParams.mm @@ -98,6 +98,7 @@ - (instancetype)initWithParams:(MTRDeviceControllerStartupParams *)params _ipk = params.ipk; _vendorID = params.vendorID; _nodeID = params.nodeID; + _caseAuthenticatedTags = params.caseAuthenticatedTags; _rootCertificate = params.rootCertificate; _intermediateCertificate = params.intermediateCertificate; _operationalCertificate = params.operationalCertificate; @@ -190,6 +191,11 @@ - (instancetype)initWithParams:(MTRDeviceControllerStartupParams *)params return nil; } + if (self.caseAuthenticatedTags != nil && self.nodeID == nil) { + MTR_LOG_ERROR("caseAuthenticatedTags must be nil if nodeID is nil"); + return nil; + } + if (self.operationalCertificate != nil) { if (self.operationalKeypair == nil) { MTR_LOG_ERROR("Must have an operational keypair if an operational certificate is provided"); @@ -266,14 +272,16 @@ - (instancetype)initForExistingFabric:(FabricTable *)fabricTable if (self.operationalCertificate == nil && self.nodeID == nil) { self.nodeID = @(fabric->GetNodeId()); + // Make sure to preserve caseAuthenticatedTags from the existing certificate. + uint8_t nocBuf[Credentials::kMaxCHIPCertLength]; + MutableByteSpan noc(nocBuf); + CHIP_ERROR err = fabricTable->FetchNOCCert(fabric->GetFabricIndex(), noc); + if (err != CHIP_NO_ERROR) { + MTR_LOG_ERROR("Failed to get existing NOC: %s", ErrorStr(err)); + return nil; + } + if (self.operationalKeypair == nil) { - uint8_t nocBuf[Credentials::kMaxCHIPCertLength]; - MutableByteSpan noc(nocBuf); - CHIP_ERROR err = fabricTable->FetchNOCCert(fabric->GetFabricIndex(), noc); - if (err != CHIP_NO_ERROR) { - MTR_LOG_ERROR("Failed to get existing NOC: %s", ErrorStr(err)); - return nil; - } self.operationalCertificate = MatterCertToX509Data(noc); if (self.operationalCertificate == nil) { MTR_LOG_ERROR("Failed to convert TLV NOC to DER X.509: %s", ErrorStr(err)); @@ -285,6 +293,26 @@ - (instancetype)initForExistingFabric:(FabricTable *)fabricTable } } + CATValues cats; + err = Credentials::ExtractCATsFromOpCert(noc, cats); + if (err != CHIP_NO_ERROR) { + MTR_LOG_ERROR("Failed to extract existing CATs: %s", ErrorStr(err)); + return nil; + } + + auto tagCount = cats.GetNumTagsPresent(); + if (tagCount > 0) { + auto * catSet = [[NSMutableSet alloc] initWithCapacity:tagCount]; + for (auto & value : cats.values) { + if (value != kUndefinedCAT) { + [catSet addObject:@(value)]; + } + } + self.caseAuthenticatedTags = [NSSet setWithSet:catSet]; + } else { + self.caseAuthenticatedTags = nil; + } + usingExistingNOC = YES; } diff --git a/src/darwin/Framework/CHIPTests/MTRControllerTests.m b/src/darwin/Framework/CHIPTests/MTRControllerTests.m index d28beef64ccb42..63a5baf32b64d5 100644 --- a/src/darwin/Framework/CHIPTests/MTRControllerTests.m +++ b/src/darwin/Framework/CHIPTests/MTRControllerTests.m @@ -29,6 +29,22 @@ static uint16_t kTestVendorId = 0xFFF1u; +static void CheckStoredOpcertCats(id storage, uint8_t fabricIndex, NSSet * cats) +{ + __auto_type * certData = [storage storageDataForKey:[NSString stringWithFormat:@"f/%x/n", fabricIndex]]; + XCTAssertNotNil(certData); + + __auto_type * info = [[MTRCertificateInfo alloc] initWithTLVBytes:certData]; + XCTAssertNotNil(info); + + __auto_type * storedCATs = info.subject.caseAuthenticatedTags; + if (cats == nil) { + XCTAssertTrue(storedCATs.count == 0); + } else { + XCTAssertEqualObjects(storedCATs, cats); + } +} + @interface MTRControllerTests : XCTestCase @end @@ -1359,4 +1375,177 @@ - (void)testControllerExternallyProvidedOperationalKey XCTAssertFalse([factory isRunning]); } +- (void)testControllerCATs +{ + __auto_type * factory = [MTRDeviceControllerFactory sharedInstance]; + XCTAssertNotNil(factory); + + __auto_type * storage = [[MTRTestStorage alloc] init]; + __auto_type * factoryParams = [[MTRDeviceControllerFactoryParams alloc] initWithStorage:storage]; + XCTAssertTrue([factory startControllerFactory:factoryParams error:nil]); + XCTAssertTrue([factory isRunning]); + + __auto_type * rootKeys = [[MTRTestKeys alloc] init]; + XCTAssertNotNil(rootKeys); + + __auto_type * params = [[MTRDeviceControllerStartupParams alloc] initWithIPK:rootKeys.ipk fabricID:@(1) nocSigner:rootKeys]; + XCTAssertNotNil(params); + + params.vendorID = @(kTestVendorId); + + // + // Trying to bring up a controller with too-long CATs should fail. + // + __auto_type * tooLongCATs = [NSSet setWithObjects:@(0x10001), @(0x20001), @(0x30001), @(0x40001), nil]; + params.caseAuthenticatedTags = tooLongCATs; + MTRDeviceController * controller = [factory createControllerOnExistingFabric:params error:nil]; + XCTAssertNil(controller); + + // + // Trying to bring up a controller that has an invalid CAT value should + // fail. + // + __auto_type * invalidCATs = [NSSet setWithObjects:@(0x10001), @(0x20001), @(0x0), nil]; + params.caseAuthenticatedTags = invalidCATs; + controller = [factory createControllerOnExistingFabric:params error:nil]; + XCTAssertNil(controller); + + // + // Bring up a controller with valid CATs + // + __auto_type * validCATs = [NSSet setWithObjects:@(0x10001), @(0x20007), @(0x30005), nil]; + params.nodeID = @(17); + params.caseAuthenticatedTags = validCATs; + controller = [factory createControllerOnNewFabric:params error:nil]; + XCTAssertNotNil(controller); + XCTAssertTrue([controller isRunning]); + + // Check that the resulting certificate has the right CATs and node ID + CheckStoredOpcertCats(storage, 1, validCATs); + XCTAssertEqualObjects([controller controllerNodeID], @(17)); + + [controller shutdown]; + XCTAssertFalse([controller isRunning]); + + // + // Trying to bring up the same fabric without specifying any node id + // should not allow trying to specify the same CATs. + // + params.nodeID = nil; + params.caseAuthenticatedTags = validCATs; + controller = [factory createControllerOnExistingFabric:params error:nil]; + XCTAssertNil(controller); + + // + // Trying to bring up the same fabric without specifying any node id + // should not allow trying to specify different CATs. + // + __auto_type * newCATs = [NSSet setWithObjects:@(0x20005), @(0x70009), @(0x80004), nil]; + XCTAssertNotEqualObjects(validCATs, newCATs); + params.nodeID = nil; + params.caseAuthenticatedTags = newCATs; + controller = [factory createControllerOnExistingFabric:params error:nil]; + XCTAssertNil(controller); + + // + // Trying to bring up the same fabric without specifying any node id + // should end up using the existing CATs. + // + params.nodeID = nil; + params.caseAuthenticatedTags = nil; + controller = [factory createControllerOnExistingFabric:params error:nil]; + XCTAssertNotNil(controller); + XCTAssertTrue([controller isRunning]); + + // Check that the resulting certificate has the right CATs and node ID + CheckStoredOpcertCats(storage, 1, validCATs); + XCTAssertEqualObjects([controller controllerNodeID], @(17)); + + [controller shutdown]; + XCTAssertFalse([controller isRunning]); + + // + // Trying to bring up the same fabric without specifying any node id + // should end up using the existing CATs, even if a new + // operational key is specified. + // + __auto_type * operationalKeys = [[MTRTestKeys alloc] init]; + XCTAssertNotNil(operationalKeys); + params.nodeID = nil; + params.operationalKeypair = operationalKeys; + params.caseAuthenticatedTags = nil; + + controller = [factory createControllerOnExistingFabric:params error:nil]; + XCTAssertNotNil(controller); + XCTAssertTrue([controller isRunning]); + + // Check that the resulting certificate has the right CATs and node ID + CheckStoredOpcertCats(storage, 1, validCATs); + XCTAssertEqualObjects([controller controllerNodeID], @(17)); + + [controller shutdown]; + XCTAssertFalse([controller isRunning]); + + // + // Trying to bring up the same fabric while specifying the node ID, even if + // it's the same one, should pick up the new CATs. + // + params.nodeID = @(17); + params.operationalKeypair = operationalKeys; + params.caseAuthenticatedTags = newCATs; + + controller = [factory createControllerOnExistingFabric:params error:nil]; + XCTAssertNotNil(controller); + XCTAssertTrue([controller isRunning]); + + // Check that the resulting certificate has the right CATs and node ID + CheckStoredOpcertCats(storage, 1, newCATs); + XCTAssertEqualObjects([controller controllerNodeID], @(17)); + + [controller shutdown]; + XCTAssertFalse([controller isRunning]); + + // + // Trying to bring up the same fabric while specifying the node ID should + // let us remove CATs altogether. + // + params.nodeID = @(17); + params.operationalKeypair = operationalKeys; + params.caseAuthenticatedTags = nil; + + controller = [factory createControllerOnExistingFabric:params error:nil]; + XCTAssertNotNil(controller); + XCTAssertTrue([controller isRunning]); + + // Check that the resulting certificate has the right CATs and node ID + CheckStoredOpcertCats(storage, 1, nil); + XCTAssertEqualObjects([controller controllerNodeID], @(17)); + + [controller shutdown]; + XCTAssertFalse([controller isRunning]); + + // + // Trying to bring up the same fabric with too-long CATs should fail, if we + // are taking the provided CATs into account. + // + params.nodeID = @(17); + params.operationalKeypair = operationalKeys; + params.caseAuthenticatedTags = tooLongCATs; + controller = [factory createControllerOnExistingFabric:params error:nil]; + XCTAssertNil(controller); + + // + // Trying to bring up the same fabric with invalid CATs should fail, if we + // are taking the provided CATs into account. + // + params.nodeID = @(17); + params.operationalKeypair = operationalKeys; + params.caseAuthenticatedTags = invalidCATs; + controller = [factory createControllerOnExistingFabric:params error:nil]; + XCTAssertNil(controller); + + [factory stopControllerFactory]; + XCTAssertFalse([factory isRunning]); +} + @end