From 38610553cde21cc6ed7fd08c600144538b129918 Mon Sep 17 00:00:00 2001 From: Boris Zbarsky Date: Wed, 11 Jan 2023 17:39:03 -0500 Subject: [PATCH] Allow using MTROperationalCertificateIssuer but letting the Matter framework perform device attestation checks. (#24371) * Allow using MTROperationalCertificateIssuer but letting the Matter framework perform device attestation checks. This gives MTROperationalCertificateIssuer implementations control over whether they want to take over the device attestation checks that require roots of trust or whether they want to allow Matter.framework to perform those checks itself, using the roots of trust it was provided. Fixes https://github.com/project-chip/connectedhomeip/issues/24310 * Address review comment. * Address more review comments. --- .../Framework/CHIP/MTRDeviceController.mm | 6 ++++ .../CHIP/MTRDeviceControllerStartupParams.h | 11 ------- .../CHIP/MTROperationalCertificateIssuer.h | 30 +++++++++++++++++++ 3 files changed, 36 insertions(+), 11 deletions(-) diff --git a/src/darwin/Framework/CHIP/MTRDeviceController.mm b/src/darwin/Framework/CHIP/MTRDeviceController.mm index 8474bbc517fd47..7dd6fba958193c 100644 --- a/src/darwin/Framework/CHIP/MTRDeviceController.mm +++ b/src/darwin/Framework/CHIP/MTRDeviceController.mm @@ -558,8 +558,12 @@ - (BOOL)setOperationalCertificateIssuer:(nullable id_operationalCredentialsDelegate->SetOperationalCertificateIssuer(operationalCertificateIssuer, queue); + usePartialDACVerifier = operationalCertificateIssuer.shouldSkipAttestationCertificateValidation; + } + if (usePartialDACVerifier) { self->_cppCommissioner->SetDeviceAttestationVerifier(self->_partialDACVerifier); } else { // TODO: Once we are not supporting setNocChainIssuer this @@ -921,6 +925,7 @@ - (void)onPairingDeleted:(NSError * _Nullable)error */ @interface MTROperationalCertificateChainIssuerShim : NSObject @property (nonatomic, readonly) id nocChainIssuer; +@property (nonatomic, readonly) BOOL shouldSkipAttestationCertificateValidation; - (instancetype)initWithIssuer:(id)nocChainIssuer; @end @@ -929,6 +934,7 @@ - (instancetype)initWithIssuer:(id)nocChainIssuer { if (self = [super init]) { _nocChainIssuer = nocChainIssuer; + _shouldSkipAttestationCertificateValidation = YES; } return self; } diff --git a/src/darwin/Framework/CHIP/MTRDeviceControllerStartupParams.h b/src/darwin/Framework/CHIP/MTRDeviceControllerStartupParams.h index 36090b9766dd44..a1c0241044fcaa 100644 --- a/src/darwin/Framework/CHIP/MTRDeviceControllerStartupParams.h +++ b/src/darwin/Framework/CHIP/MTRDeviceControllerStartupParams.h @@ -193,17 +193,6 @@ NS_ASSUME_NONNULL_BEGIN * when commmissioning devices. Allowed to be nil if this controller either * does not issue operational certificates at all or internally generates the * certificates to be issued. In the latter case, nocSigner must not be nil. - * - * When this property is non-nill, all device attestation checks that require - * some sort of trust anchors are delegated to the operationalCertificateIssuer. - * Specifically, the following device attestation checks are not performed and - * must be done by the operationalCertificateIssuer: - * - * (1) Make sure the PAA is valid and approved by CSA. - * (2) VID-scoped PAA check: if the PAA is VID scoped, then its VID must match the DAC VID. - * (3) cert chain check: verify PAI is signed by PAA, and DAC is signed by PAI. - * (4) PAA subject key id extraction: the PAA subject key must match the PAA key referenced in the PAI. - * (5) CD signature check: make sure a valid CSA CD key is used to sign the CD. */ @property (nonatomic, strong, nullable) id operationalCertificateIssuer MTR_NEWLY_AVAILABLE; diff --git a/src/darwin/Framework/CHIP/MTROperationalCertificateIssuer.h b/src/darwin/Framework/CHIP/MTROperationalCertificateIssuer.h index 84566e20dbee14..b02bf62feb7055 100644 --- a/src/darwin/Framework/CHIP/MTROperationalCertificateIssuer.h +++ b/src/darwin/Framework/CHIP/MTROperationalCertificateIssuer.h @@ -66,12 +66,42 @@ MTR_NEWLY_AVAILABLE * and resume when the completion is invoked with a non-nil * MTROperationalCertificateInfo. When the completion is invoked with an error, * commissioning will fail. + * + * This will be called on the dispatch queue passed as + * operationalCertificateIssuerQueue in the MTRDeviceControllerFactoryParams. */ - (void)issueOperationalCertificateForRequest:(MTROperationalCSRInfo *)csrInfo attestationInfo:(MTRAttestationInfo *)attestationInfo controller:(MTRDeviceController *)controller completion:(MTROperationalCertificateIssuedHandler)completion; +/** + * A way for MTROperationalCertificateIssuer to control whether it wants the + * Matter framework to perform device attestation checks that require trust + * anchors. If this returns NO, then productAttestationAuthorityCertificates + * should be passed in via MTRDeviceControllerFactoryParams, as well as any + * desired additional certificationDeclarationCertificates. + * + * If this returns YES, then all device attestation checks that require some + * sort of trust anchors are delegated to this MTROperationalCertificateIssuer, + * which can use the arguments passed to + * issueOperationalCertificateForRequest:attestationInfo:controller:completion: + * to perform the checks. + * + * Specifically, the following device attestation checks are not performed and + * must be done by the MTROperationalCertificateIssuer: + * + * (1) Make sure the PAA is valid and approved by CSA. + * (2) VID-scoped PAA check: if the PAA is VID scoped, then its VID must match the DAC VID. + * (3) cert chain check: verify PAI is signed by PAA, and DAC is signed by PAI. + * (4) PAA subject key id extraction: the PAA subject key must match the PAA key referenced in the PAI. + * (5) CD signature check: make sure a valid CSA CD key is used to sign the CD. + * + * This will be read on an arbitrary queue and must not block or call any + * Matter APIs. + */ +@property (nonatomic, readonly) BOOL shouldSkipAttestationCertificateValidation; + @end MTR_NEWLY_DEPRECATED("Please use MTROperationalCertificateIssuedHandler")