From 5c16e5209e577c19ea878a0f24b6c98f7db11032 Mon Sep 17 00:00:00 2001 From: Justin Wood Date: Mon, 15 Aug 2022 13:33:53 -0700 Subject: [PATCH] Addressing second round of feedback --- .../Framework/CHIP/MTRDeviceController.h | 31 +--------- .../Framework/CHIP/MTRDeviceController.mm | 56 +---------------- src/darwin/Framework/CHIP/MTRNOCChainIssuer.h | 10 +++- .../CHIP/MTROperationalCredentialsDelegate.h | 25 +++++++- .../CHIP/MTROperationalCredentialsDelegate.mm | 60 ++++++++++++++++++- 5 files changed, 95 insertions(+), 87 deletions(-) diff --git a/src/darwin/Framework/CHIP/MTRDeviceController.h b/src/darwin/Framework/CHIP/MTRDeviceController.h index 23c0a6fa8b950b..faca45ddde863b 100644 --- a/src/darwin/Framework/CHIP/MTRDeviceController.h +++ b/src/darwin/Framework/CHIP/MTRDeviceController.h @@ -120,40 +120,15 @@ typedef void (^MTRDeviceConnectionCallback)(MTRBaseDevice * _Nullable device, NS */ - (void)setPairingDelegate:(id)delegate queue:(dispatch_queue_t)queue; -/** - * Set the Delegate for the Device Pairing as well as the Queue on which the Delegate callbacks will be triggered - * - * @param[in] delegate The delegate the pairing process should use - * - * @param[in] queue The queue on which the callbacks will be delivered - */ - /** * Sets this MTRDeviceController to use the given issuer for issuing operational certs. By default, the MTRDeviceController uses an - * internal, OperationalCredentialsDelegate (see MTROperationalCredentialsDelegate) + * internal issuer. * * @param[in] nocChainIssuer the NOC Chain issuer to use for issuer operational certs - */ -- (void)setNocChainIssuer:(id)nocChainIssuer; - -/** - * When a NOCChainIssuer is set for this controller, then onNOCChainGenerationNeeded will be called when the NOC CSR needs to be - * signed. This allows for custom credentials issuer implementations, for example, when a proprietary cloud API will perform the CSR - * signing. The commissioning workflow will stop upon the onNOCChainGenerationNeeded callback and resume once onNOCChainGeneration - * is called. - * - * Caller must pass a non-nil value for the rootCertificate, intermediateCertificate, operationalCertificate - * If ipk and adminSubject are non nil, then they will be used in the AddNOC command sent to the commissionee. If they are not - * populated, then the values provided in the ChipDeviceController initialization will be used. * - * @return error code (0 is no error) + * @param[in] queue The queue on which the callbacks will be delivered */ -- (NSNumber *)onNOCChainGenerationComplete:(NSData *)operationalCertificate - intermediateCertificate:(NSData *)intermediateCertificate - rootCertificate:(NSData *)rootCertificate - ipk:(NSData *)ipk - adminSubject:(NSNumber *)adminSubject - error:(NSError * __autoreleasing *)error; +- (void)setNocChainIssuer:(id)nocChainIssuer queue:(dispatch_queue_t)queue; /** * Shutdown the controller. Calls to shutdown after the first one are NO-OPs. diff --git a/src/darwin/Framework/CHIP/MTRDeviceController.mm b/src/darwin/Framework/CHIP/MTRDeviceController.mm index 60cd78a98dbac3..b170771f77ad0b 100644 --- a/src/darwin/Framework/CHIP/MTRDeviceController.mm +++ b/src/darwin/Framework/CHIP/MTRDeviceController.mm @@ -106,6 +106,7 @@ - (instancetype)initWithFactory:(MTRControllerFactory *)factory queue:(dispatch_ if ([self checkForInitError:(_operationalCredentialsDelegate != nullptr) logMsg:kErrorOperationalCredentialsInit]) { return nil; } + _operationalCredentialsDelegate->setChipWorkQueue(_chipWorkQueue); } return self; } @@ -656,68 +657,17 @@ - (void)setPairingDelegate:(id)delegate queue:(dispatc }); } -- (void)setNocChainIssuer:(id)nocChainIssuer +- (void)setNocChainIssuer:(id)nocChainIssuer queue:(dispatch_queue_t)queue { VerifyOrReturn([self checkIsRunning]); dispatch_sync(_chipWorkQueue, ^{ VerifyOrReturn([self checkIsRunning]); - self->_operationalCredentialsDelegate->SetNocChainIssuer(nocChainIssuer); + self->_operationalCredentialsDelegate->SetNocChainIssuer(nocChainIssuer, queue); }); } -- (NSNumber *)onNOCChainGenerationComplete:(NSData *)operationalCertificate - intermediateCertificate:(NSData *)intermediateCertificate - rootCertificate:(NSData *)rootCertificate - ipk:(NSData *)ipk - adminSubject:(NSNumber *)adminSubject - error:(NSError * __autoreleasing *)error -{ - VerifyOrReturnValue([self checkIsRunning:error], [NSNumber numberWithUnsignedInt:CHIP_ERROR_INCORRECT_STATE.AsInteger()]); - VerifyOrReturnValue(operationalCertificate != nil, [NSNumber numberWithUnsignedInt:CHIP_ERROR_INVALID_ARGUMENT.AsInteger()]); - VerifyOrReturnValue(intermediateCertificate != nil, [NSNumber numberWithUnsignedInt:CHIP_ERROR_INVALID_ARGUMENT.AsInteger()]); - VerifyOrReturnValue(rootCertificate != nil, [NSNumber numberWithUnsignedInt:CHIP_ERROR_INVALID_ARGUMENT.AsInteger()]); - - // use ipk and adminSubject from CommissioningParameters if not passed in - chip::Optional commissioningParameters - = _cppCommissioner->GetCommissioningParameters(); - VerifyOrReturnValue( - commissioningParameters.HasValue(), [NSNumber numberWithUnsignedInt:CHIP_ERROR_INCORRECT_STATE.AsInteger()]); - - chip::Optional ipkOptional; - uint8_t ipkValue[chip::CHIP_CRYPTO_SYMMETRIC_KEY_LENGTH_BYTES]; - chip::Crypto::AesCcm128KeySpan ipkTempSpan(ipkValue); - if (ipk != nil) { - VerifyOrReturnValue( - [ipk length] == sizeof(ipkValue), [NSNumber numberWithUnsignedInt:CHIP_ERROR_INCORRECT_STATE.AsInteger()]); - memcpy(&ipkValue[0], [ipk bytes], [ipk length]); - ipkOptional.SetValue(ipkTempSpan); - } else if (commissioningParameters.Value().GetIpk().HasValue()) { - ipkOptional.SetValue(commissioningParameters.Value().GetIpk().Value()); - } - - chip::Optional adminSubjectOptional; - if (adminSubject == nil) { - // if no value passed in, use value from CommissioningParameters - adminSubjectOptional.SetValue(commissioningParameters.Value().GetAdminSubject().ValueOr(chip::kUndefinedNodeId)); - } else { - adminSubjectOptional.SetValue(adminSubject.unsignedLongLongValue); - } - - __block CHIP_ERROR err = CHIP_NO_ERROR; - dispatch_sync(_chipWorkQueue, ^{ - VerifyOrReturn([self checkIsRunning:error]); - err = self->_operationalCredentialsDelegate->NOCChainGenerated(CHIP_NO_ERROR, AsByteSpan(operationalCertificate), - AsByteSpan(intermediateCertificate), AsByteSpan(rootCertificate), ipkOptional, adminSubjectOptional); - }); - - if (err != CHIP_NO_ERROR) { - MTR_LOG_ERROR("Failed to SetNocChain for the device: %" CHIP_ERROR_FORMAT, err.Format()); - } - return [NSNumber numberWithUnsignedInt:err.AsInteger()]; -} - - (BOOL)checkForInitError:(BOOL)condition logMsg:(NSString *)logMsg { if (condition) { diff --git a/src/darwin/Framework/CHIP/MTRNOCChainIssuer.h b/src/darwin/Framework/CHIP/MTRNOCChainIssuer.h index 8780b3f9173e72..cf85d410b7f4f8 100644 --- a/src/darwin/Framework/CHIP/MTRNOCChainIssuer.h +++ b/src/darwin/Framework/CHIP/MTRNOCChainIssuer.h @@ -26,12 +26,12 @@ NS_ASSUME_NONNULL_BEGIN @required /** - * @brief When a MTRNOCChainIssuer is set for this controller, then onNOCChainGenerationNeeded will be + * @brief When a MTRNOCChainIssuer is set for the MTRDeviceController, then onNOCChainGenerationNeeded will be * called when the NOC CSR needs to be signed. This allows for custom credentials issuer * implementations, for example, when a proprietary cloud API will perform the CSR signing. * The commissioning workflow will stop upon the onNOCChainGenerationNeeded callback and - * resume once onNOCChainGeneration is called. + * resume once onNOCChainGenerationComplete is called * The following fields MUST be passed to onNOCChainGenerationComplete with non-nil values: * rootCertificate, intermediateCertificate, operationalCertificate. @@ -41,7 +41,11 @@ NS_ASSUME_NONNULL_BEGIN * * All csr and attestation fields are provided to allow for custom attestestation checks. */ -- (void)onNOCChainGenerationNeeded:(CSRInfo *)csrInfo attestationInfo:(AttestationInfo *)attestationInfo; +- (void)onNOCChainGenerationNeeded:(CSRInfo *)csrInfo + attestationInfo:(AttestationInfo *)attestationInfo + onNOCChainGenerationComplete:(NSNumber * (^)(NSData * operationalCertificate, NSData * intermediateCertificate, + NSData * rootCertificate, NSData * ipk, + NSNumber * adminSubject))onNOCChainGenerationComplete; @end diff --git a/src/darwin/Framework/CHIP/MTROperationalCredentialsDelegate.h b/src/darwin/Framework/CHIP/MTROperationalCredentialsDelegate.h index f469261e2f458d..74a621a2a12589 100644 --- a/src/darwin/Framework/CHIP/MTROperationalCredentialsDelegate.h +++ b/src/darwin/Framework/CHIP/MTROperationalCredentialsDelegate.h @@ -64,7 +64,13 @@ class MTROperationalCredentialsDelegate : public chip::Controller::OperationalCr return mCppCommissioner == nullptr ? chip::NullOptional : mCppCommissioner->GetCommissioningParameters(); } - void SetNocChainIssuer(id nocChainIssuer) { mNocChainIssuer = nocChainIssuer; } + void setChipWorkQueue(dispatch_queue_t chipWorkQueue) { mChipWorkQueue = chipWorkQueue; } + + void SetNocChainIssuer(id nocChainIssuer, dispatch_queue_t nocChainIssuerQueue) + { + mNocChainIssuer = nocChainIssuer; + mNocChainIssuerQueue = nocChainIssuerQueue; + } CHIP_ERROR NOCChainGenerated(CHIP_ERROR status, const chip::ByteSpan & noc, const chip::ByteSpan & icac, const chip::ByteSpan & rcac, chip::Optional ipk, chip::Optional adminSubject); @@ -111,6 +117,21 @@ class MTROperationalCredentialsDelegate : public chip::Controller::OperationalCr chip::FabricId fabricId, const chip::CATValues & cats, const chip::Crypto::P256PublicKey & pubkey, chip::MutableByteSpan & noc); + /** + * When a NOCChainIssuer is set, then onNOCChainGenerationNeeded will be called when the NOC CSR needs to be + * signed. This allows for custom credentials issuer implementations, for example, when a proprietary cloud API will perform the + * CSR signing. The commissioning workflow will stop upon the onNOCChainGenerationNeeded callback and resume once + * onNOCChainGenerationComplete is called. + * + * Caller must pass a non-nil value for the rootCertificate, intermediateCertificate, operationalCertificate + * If ipk and adminSubject are non nil, then they will be used in the AddNOC command sent to the commissionee. If they are not + * populated, then the values provided in the MTRDeviceController initialization will be used. + * + * @return error code (0 is no error) + */ + NSNumber * onNOCChainGenerationComplete(MTROperationalCredentialsDelegate * thisDelegate, NSData * operationalCertificate, + NSData * intermediateCertificate, NSData * rootCertificate, NSData * _Nullable ipk, NSNumber * _Nullable adminSubject); + CHIP_ERROR CallbackGenerateNOCChain(const chip::ByteSpan & csrElements, const chip::ByteSpan & csrNonce, const chip::ByteSpan & attestationSignature, const chip::ByteSpan & attestationChallenge, const chip::ByteSpan & DAC, const chip::ByteSpan & PAI, chip::Callback::Callback * onCompletion); @@ -140,6 +161,8 @@ class MTROperationalCredentialsDelegate : public chip::Controller::OperationalCr chip::Controller::DeviceCommissioner * mCppCommissioner = nullptr; id _Nullable mNocChainIssuer; + dispatch_queue_t _Nullable mNocChainIssuerQueue; + dispatch_queue_t _Nullable mChipWorkQueue; chip::Callback::Callback * mOnNOCCompletionCallback = nullptr; }; diff --git a/src/darwin/Framework/CHIP/MTROperationalCredentialsDelegate.mm b/src/darwin/Framework/CHIP/MTROperationalCredentialsDelegate.mm index f799b4dafbdca0..b47fbf3fd46816 100644 --- a/src/darwin/Framework/CHIP/MTROperationalCredentialsDelegate.mm +++ b/src/darwin/Framework/CHIP/MTROperationalCredentialsDelegate.mm @@ -160,7 +160,8 @@ ReturnErrorOnFailure(reader.EnterContainer(containerType)); ReturnErrorOnFailure(reader.Next(kTLVType_ByteString, TLV::ContextTag(1))); - chip::ByteSpan csr(reader.GetReadPoint(), reader.GetLength()); + chip::ByteSpan csr; + reader.Get(csr); reader.ExitContainer(containerType); CSRInfo * csrInfo = [[CSRInfo alloc] initWithNonce:AsData(csrNonce) @@ -192,10 +193,65 @@ certificationDeclaration:AsData(certificationDeclarationSpan) firmwareInfo:AsData(firmwareInfoSpan)]; - [mNocChainIssuer onNOCChainGenerationNeeded:csrInfo attestationInfo:attestationInfo]; + dispatch_sync(mNocChainIssuerQueue, ^{ + [mNocChainIssuer onNOCChainGenerationNeeded:csrInfo + attestationInfo:attestationInfo + onNOCChainGenerationComplete:^NSNumber *(NSData * operationalCertificate, NSData * intermediateCertificate, + NSData * rootCertificate, NSData * ipk, NSNumber * adminSubject) { + __block NSNumber * rv = nil; + dispatch_sync(mChipWorkQueue, ^{ + rv = onNOCChainGenerationComplete( + this, operationalCertificate, intermediateCertificate, rootCertificate, ipk, adminSubject); + }); + return rv; + }]; + }); + return CHIP_NO_ERROR; } +NSNumber * MTROperationalCredentialsDelegate::onNOCChainGenerationComplete(MTROperationalCredentialsDelegate * thisDelegate, + NSData * operationalCertificate, NSData * intermediateCertificate, NSData * rootCertificate, NSData * _Nullable ipk, + NSNumber * _Nullable adminSubject) +{ + VerifyOrReturnValue(operationalCertificate != nil, [NSNumber numberWithUnsignedInt:CHIP_ERROR_INVALID_ARGUMENT.AsInteger()]); + VerifyOrReturnValue(intermediateCertificate != nil, [NSNumber numberWithUnsignedInt:CHIP_ERROR_INVALID_ARGUMENT.AsInteger()]); + VerifyOrReturnValue(rootCertificate != nil, [NSNumber numberWithUnsignedInt:CHIP_ERROR_INVALID_ARGUMENT.AsInteger()]); + + // use ipk and adminSubject from CommissioningParameters if not passed in + chip::Optional commissioningParameters + = mCppCommissioner->GetCommissioningParameters(); + VerifyOrReturnValue( + commissioningParameters.HasValue(), [NSNumber numberWithUnsignedInt:CHIP_ERROR_INCORRECT_STATE.AsInteger()]); + + chip::Optional ipkOptional; + uint8_t ipkValue[chip::CHIP_CRYPTO_SYMMETRIC_KEY_LENGTH_BYTES]; + chip::Crypto::AesCcm128KeySpan ipkTempSpan(ipkValue); + if (ipk != nil) { + VerifyOrReturnValue( + [ipk length] == sizeof(ipkValue), [NSNumber numberWithUnsignedInt:CHIP_ERROR_INCORRECT_STATE.AsInteger()]); + memcpy(&ipkValue[0], [ipk bytes], [ipk length]); + ipkOptional.SetValue(ipkTempSpan); + } else if (commissioningParameters.Value().GetIpk().HasValue()) { + ipkOptional.SetValue(commissioningParameters.Value().GetIpk().Value()); + } + + chip::Optional adminSubjectOptional; + if (adminSubject != nil) { + adminSubjectOptional.SetValue(adminSubject.unsignedLongLongValue); + } else { + adminSubjectOptional = commissioningParameters.Value().GetAdminSubject(); + } + + CHIP_ERROR err = NOCChainGenerated(CHIP_NO_ERROR, AsByteSpan(operationalCertificate), AsByteSpan(intermediateCertificate), + AsByteSpan(rootCertificate), ipkOptional, adminSubjectOptional); + + if (err != CHIP_NO_ERROR) { + MTR_LOG_ERROR("Failed to SetNocChain for the device: %" CHIP_ERROR_FORMAT, err.Format()); + } + return [NSNumber numberWithUnsignedInt:err.AsInteger()]; +} + CHIP_ERROR MTROperationalCredentialsDelegate::LocalGenerateNOCChain(const chip::ByteSpan & csrElements, const chip::ByteSpan & csrNonce, const chip::ByteSpan & attestationSignature, const chip::ByteSpan & attestationChallenge, const chip::ByteSpan & DAC, const chip::ByteSpan & PAI,