From d15be4cbcc70c833789365ba2d0cb862cfc54e4f Mon Sep 17 00:00:00 2001 From: Boris Zbarsky Date: Mon, 23 Jan 2023 23:16:09 -0500 Subject: [PATCH] Make sure MTRDeviceControllerStartupParams initWithParams actually copies all the fields. (#24605) Also fixes the error propagation in DeviceCommissioner::OnDeviceNOCChainGeneration to not override the provided error status. --- src/controller/CHIPDeviceController.cpp | 4 +- .../CHIP/MTRDeviceControllerStartupParams.mm | 2 + .../Framework/CHIPTests/MTRDeviceTests.m | 2 +- .../MTROperationalCertificateIssuerTests.m | 171 ++++++++++++++++++ .../Matter.xcodeproj/project.pbxproj | 4 + 5 files changed, 180 insertions(+), 3 deletions(-) create mode 100644 src/darwin/Framework/CHIPTests/MTROperationalCertificateIssuerTests.m diff --git a/src/controller/CHIPDeviceController.cpp b/src/controller/CHIPDeviceController.cpp index 2d78993a69ad48..a5bd64a78b98fa 100644 --- a/src/controller/CHIPDeviceController.cpp +++ b/src/controller/CHIPDeviceController.cpp @@ -1248,14 +1248,14 @@ void DeviceCommissioner::OnDeviceNOCChainGeneration(void * context, CHIP_ERROR s // The placeholder IPK is not satisfactory, but is there to fill the NocChain struct on error. It will still fail. const uint8_t placeHolderIpk[] = { 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00 }; - if (!ipk.HasValue()) + if (status == CHIP_NO_ERROR && !ipk.HasValue()) { ChipLogError(Controller, "Did not have an IPK from the OperationalCredentialsIssuer! Cannot commission."); status = CHIP_ERROR_INVALID_ARGUMENT; } ChipLogProgress(Controller, "Received callback from the CA for NOC Chain generation. Status %s", ErrorStr(status)); - if (commissioner->mState != State::Initialized) + if (status == CHIP_NO_ERROR && commissioner->mState != State::Initialized) { status = CHIP_ERROR_INCORRECT_STATE; } diff --git a/src/darwin/Framework/CHIP/MTRDeviceControllerStartupParams.mm b/src/darwin/Framework/CHIP/MTRDeviceControllerStartupParams.mm index 557864bc28e60b..ba247576e916fa 100644 --- a/src/darwin/Framework/CHIP/MTRDeviceControllerStartupParams.mm +++ b/src/darwin/Framework/CHIP/MTRDeviceControllerStartupParams.mm @@ -102,6 +102,8 @@ - (instancetype)initWithParams:(MTRDeviceControllerStartupParams *)params _intermediateCertificate = params.intermediateCertificate; _operationalCertificate = params.operationalCertificate; _operationalKeypair = params.operationalKeypair; + _operationalCertificateIssuer = params.operationalCertificateIssuer; + _operationalCertificateIssuerQueue = params.operationalCertificateIssuerQueue; return self; } diff --git a/src/darwin/Framework/CHIPTests/MTRDeviceTests.m b/src/darwin/Framework/CHIPTests/MTRDeviceTests.m index f7ab5557af14db..a07236c2ebdd86 100644 --- a/src/darwin/Framework/CHIPTests/MTRDeviceTests.m +++ b/src/darwin/Framework/CHIPTests/MTRDeviceTests.m @@ -104,7 +104,7 @@ - (void)controller:(MTRDeviceController *)controller commissioningSessionEstabli error:&commissionError]; XCTAssertNil(commissionError); - // Keep waiting for controller:MTRXPCListenerSampleTests.mcommissioningComplete + // Keep waiting for controller:commissioningComplete: } - (void)controller:(MTRDeviceController *)controller commissioningComplete:(NSError *)error diff --git a/src/darwin/Framework/CHIPTests/MTROperationalCertificateIssuerTests.m b/src/darwin/Framework/CHIPTests/MTROperationalCertificateIssuerTests.m new file mode 100644 index 00000000000000..2b6cdba1af7df9 --- /dev/null +++ b/src/darwin/Framework/CHIPTests/MTROperationalCertificateIssuerTests.m @@ -0,0 +1,171 @@ +/* + * Copyright (c) 2023 Project CHIP Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +// module headers +#import + +#import "MTRErrorTestUtils.h" +#import "MTRTestKeys.h" +#import "MTRTestStorage.h" + +// system dependencies +#import + +static const uint16_t kPairingTimeoutInSeconds = 10; +static const uint64_t kDeviceId = 0x12344321; +static NSString * kOnboardingPayload = @"MT:-24J0AFN00KA0648G00"; +static const uint16_t kLocalPort = 5541; +static const uint16_t kTestVendorId = 0xFFF1u; + +// Singleton controller we use. +static MTRDeviceController * sController = nil; + +// Keys we can use to restart the controller. +static MTRTestKeys * sTestKeys = nil; + +@interface MTROperationalCertificateIssureTestDeviceControllerDelegate : NSObject +@property (nonatomic, strong) XCTestExpectation * expectation; +@end + +@implementation MTROperationalCertificateIssureTestDeviceControllerDelegate +- (id)initWithExpectation:(XCTestExpectation *)expectation +{ + self = [super init]; + if (self) { + _expectation = expectation; + } + return self; +} + +- (void)controller:(MTRDeviceController *)controller commissioningSessionEstablishmentDone:(NSError *)error +{ + XCTAssertEqual(error.code, 0); + + NSError * commissionError = nil; + [sController commissionNodeWithID:@(kDeviceId) + commissioningParams:[[MTRCommissioningParameters alloc] init] + error:&commissionError]; + XCTAssertNil(commissionError); + + // Keep waiting for controller:commissioningComplete: +} + +- (void)controller:(MTRDeviceController *)controller commissioningComplete:(NSError *)error +{ + XCTAssertNotNil(error); + XCTAssertEqual(error.domain, MTRErrorDomain); + XCTAssertEqual(error.code, MTRErrorCodeIntegrityCheckFailed); + [_expectation fulfill]; + _expectation = nil; +} + +@end + +@interface OperationalCertificateIssuer : NSObject +@property (nonatomic, readonly) BOOL shouldSkipAttestationCertificateValidation; +@end + +@implementation OperationalCertificateIssuer + +- (instancetype)init +{ + if (self = [super init]) { + _shouldSkipAttestationCertificateValidation = NO; + } + return self; +} + +- (void)issueOperationalCertificateForRequest:(MTROperationalCSRInfo *)csrInfo + attestationInfo:(MTRAttestationInfo *)attestationInfo + controller:(MTRDeviceController *)controller + completion:(MTROperationalCertificateIssuedHandler)completion +{ + XCTAssertNotNil(csrInfo); + XCTAssertNotNil(attestationInfo); + XCTAssertEqual(controller, sController); + + completion(nil, [NSError errorWithDomain:MTRErrorDomain code:MTRErrorCodeIntegrityCheckFailed userInfo:nil]); +} + +@end + +@interface MTROperationalCertificateIssuerTests : XCTestCase +@end + +@implementation MTROperationalCertificateIssuerTests + +- (void)setUp +{ + [super setUp]; + [self setContinueAfterFailure:NO]; +} + +- (void)testFailedCertificateIssuance +{ + XCTestExpectation * expectation = [self expectationWithDescription:@"Pairing Complete"]; + + __auto_type * factory = [MTRDeviceControllerFactory sharedInstance]; + XCTAssertNotNil(factory); + + __auto_type * storage = [[MTRTestStorage alloc] init]; + __auto_type * factoryParams = [[MTRDeviceControllerFactoryParams alloc] initWithStorage:storage]; + factoryParams.port = @(kLocalPort); + + BOOL ok = [factory startControllerFactory:factoryParams error:nil]; + XCTAssertTrue(ok); + + __auto_type * testKeys = [[MTRTestKeys alloc] init]; + XCTAssertNotNil(testKeys); + + sTestKeys = testKeys; + + __auto_type * certificateIssuer = [[OperationalCertificateIssuer alloc] init]; + + // Needs to match what startControllerOnExistingFabric calls elsewhere in + // this file do. + __auto_type * params = [[MTRDeviceControllerStartupParams alloc] initWithIPK:testKeys.ipk fabricID:@(1) nocSigner:testKeys]; + params.vendorID = @(kTestVendorId); + params.operationalCertificateIssuer = certificateIssuer; + params.operationalCertificateIssuerQueue = dispatch_get_main_queue(); + + MTRDeviceController * controller = [factory createControllerOnNewFabric:params error:nil]; + XCTAssertNotNil(controller); + + sController = controller; + + __auto_type * deviceControllerDelegate = + [[MTROperationalCertificateIssureTestDeviceControllerDelegate alloc] initWithExpectation:expectation]; + dispatch_queue_t callbackQueue = dispatch_queue_create("com.chip.device_controller_delegate", DISPATCH_QUEUE_SERIAL); + + [controller setDeviceControllerDelegate:deviceControllerDelegate queue:callbackQueue]; + + NSError * error; + __auto_type * payload = [MTRSetupPayload setupPayloadWithOnboardingPayload:kOnboardingPayload error:&error]; + XCTAssertNotNil(payload); + XCTAssertNil(error); + + [controller setupCommissioningSessionWithPayload:payload newNodeID:@(kDeviceId) error:&error]; + XCTAssertNil(error); + + [self waitForExpectationsWithTimeout:kPairingTimeoutInSeconds handler:nil]; + + [controller shutdown]; + XCTAssertFalse([controller isRunning]); + + [factory stopControllerFactory]; +} + +@end diff --git a/src/darwin/Framework/Matter.xcodeproj/project.pbxproj b/src/darwin/Framework/Matter.xcodeproj/project.pbxproj index 6369a943c8ec7e..2637440c2273a4 100644 --- a/src/darwin/Framework/Matter.xcodeproj/project.pbxproj +++ b/src/darwin/Framework/Matter.xcodeproj/project.pbxproj @@ -50,6 +50,7 @@ 3DECCB742934C21B00585AEC /* MTRDefines.h in Headers */ = {isa = PBXBuildFile; fileRef = 3DECCB732934C21B00585AEC /* MTRDefines.h */; settings = {ATTRIBUTES = (Public, ); }; }; 3DFCB32C29678C9500332B35 /* MTRConversion.h in Headers */ = {isa = PBXBuildFile; fileRef = 3DFCB32B29678C9500332B35 /* MTRConversion.h */; }; 51029DF6293AA6100087AFB0 /* MTROperationalCertificateIssuer.mm in Sources */ = {isa = PBXBuildFile; fileRef = 51029DF5293AA6100087AFB0 /* MTROperationalCertificateIssuer.mm */; }; + 510CECA8297F72970064E0B3 /* MTROperationalCertificateIssuerTests.m in Sources */ = {isa = PBXBuildFile; fileRef = 510CECA6297F72470064E0B3 /* MTROperationalCertificateIssuerTests.m */; }; 511913FB28C100EF009235E9 /* MTRBaseSubscriptionCallback.mm in Sources */ = {isa = PBXBuildFile; fileRef = 511913F928C100EF009235E9 /* MTRBaseSubscriptionCallback.mm */; }; 511913FC28C100EF009235E9 /* MTRBaseSubscriptionCallback.h in Headers */ = {isa = PBXBuildFile; fileRef = 511913FA28C100EF009235E9 /* MTRBaseSubscriptionCallback.h */; }; 5129BCFD26A9EE3300122DDF /* MTRError.h in Headers */ = {isa = PBXBuildFile; fileRef = 5129BCFC26A9EE3300122DDF /* MTRError.h */; settings = {ATTRIBUTES = (Public, ); }; }; @@ -239,6 +240,7 @@ 3DECCB732934C21B00585AEC /* MTRDefines.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = MTRDefines.h; sourceTree = ""; }; 3DFCB32B29678C9500332B35 /* MTRConversion.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = MTRConversion.h; sourceTree = ""; }; 51029DF5293AA6100087AFB0 /* MTROperationalCertificateIssuer.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = MTROperationalCertificateIssuer.mm; sourceTree = ""; }; + 510CECA6297F72470064E0B3 /* MTROperationalCertificateIssuerTests.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = MTROperationalCertificateIssuerTests.m; sourceTree = ""; }; 511913F928C100EF009235E9 /* MTRBaseSubscriptionCallback.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = MTRBaseSubscriptionCallback.mm; sourceTree = ""; }; 511913FA28C100EF009235E9 /* MTRBaseSubscriptionCallback.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = MTRBaseSubscriptionCallback.h; sourceTree = ""; }; 5129BCFC26A9EE3300122DDF /* MTRError.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = MTRError.h; sourceTree = ""; }; @@ -607,6 +609,7 @@ 517BF3F2282B62CB00A8B7DB /* MTRCertificateTests.m */, 7596A8502878709F004DAE0E /* MTRAsyncCallbackQueueTests.m */, 51669AEF2913204400F4AA36 /* MTRBackwardsCompatTests.m */, + 510CECA6297F72470064E0B3 /* MTROperationalCertificateIssuerTests.m */, B202529D2459E34F00F97062 /* Info.plist */, ); path = CHIPTests; @@ -891,6 +894,7 @@ 1E5801C328941C050033A199 /* MTRTestOTAProvider.m in Sources */, 5A6FEC9D27B5E48900F25F42 /* MTRXPCProtocolTests.m in Sources */, 5AE6D4E427A99041001F2493 /* MTRDeviceTests.m in Sources */, + 510CECA8297F72970064E0B3 /* MTROperationalCertificateIssuerTests.m in Sources */, 5A7947DE27BEC3F500434CF2 /* MTRXPCListenerSampleTests.m in Sources */, B2F53AF2245B0DCF0010745E /* MTRSetupPayloadParserTests.m in Sources */, 517BF3F3282B62CB00A8B7DB /* MTRCertificateTests.m in Sources */,