From b1374099ccf0812f174e7b7d0389f9676efdbb82 Mon Sep 17 00:00:00 2001 From: Sharad Binjola Date: Thu, 18 Aug 2022 14:05:55 -0700 Subject: [PATCH] Address feedback around returning errors and adding comments --- src/darwin/Framework/CHIP/MTRNOCChainIssuer.h | 8 ++-- .../CHIP/MTROperationalCredentialsDelegate.h | 8 ++-- .../CHIP/MTROperationalCredentialsDelegate.mm | 44 ++++++++++++------- 3 files changed, 37 insertions(+), 23 deletions(-) diff --git a/src/darwin/Framework/CHIP/MTRNOCChainIssuer.h b/src/darwin/Framework/CHIP/MTRNOCChainIssuer.h index cf85d410b7f4f8..2ef76670532118 100644 --- a/src/darwin/Framework/CHIP/MTRNOCChainIssuer.h +++ b/src/darwin/Framework/CHIP/MTRNOCChainIssuer.h @@ -37,15 +37,15 @@ NS_ASSUME_NONNULL_BEGIN * rootCertificate, intermediateCertificate, operationalCertificate. * If ipk and adminSubject are passed, then they will be used in * the AddNOC command sent to the commissionee. If they are not passed, then the values - * provided in the ChipDeviceController initialization will be used. + * provided in the MTRDeviceController initialization will be used. * * All csr and attestation fields are provided to allow for custom attestestation checks. */ - (void)onNOCChainGenerationNeeded:(CSRInfo *)csrInfo attestationInfo:(AttestationInfo *)attestationInfo - onNOCChainGenerationComplete:(NSNumber * (^)(NSData * operationalCertificate, NSData * intermediateCertificate, - NSData * rootCertificate, NSData * ipk, - NSNumber * adminSubject))onNOCChainGenerationComplete; + onNOCChainGenerationComplete:(void (^)(NSData * operationalCertificate, NSData * intermediateCertificate, + NSData * rootCertificate, NSData * ipk, NSNumber * adminSubject, + NSError * __autoreleasing * error))onNOCChainGenerationComplete; @end diff --git a/src/darwin/Framework/CHIP/MTROperationalCredentialsDelegate.h b/src/darwin/Framework/CHIP/MTROperationalCredentialsDelegate.h index 74a621a2a12589..803e19eee30eed 100644 --- a/src/darwin/Framework/CHIP/MTROperationalCredentialsDelegate.h +++ b/src/darwin/Framework/CHIP/MTROperationalCredentialsDelegate.h @@ -126,11 +126,11 @@ class MTROperationalCredentialsDelegate : public chip::Controller::OperationalCr * 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); + void onNOCChainGenerationComplete(NSData * operationalCertificate, NSData * intermediateCertificate, NSData * rootCertificate, + NSData * _Nullable ipk, NSNumber * _Nullable adminSubject, NSError * __autoreleasing * error); + + void setNSError(CHIP_ERROR err, NSError * __autoreleasing * outError); CHIP_ERROR CallbackGenerateNOCChain(const chip::ByteSpan & csrElements, const chip::ByteSpan & csrNonce, const chip::ByteSpan & attestationSignature, const chip::ByteSpan & attestationChallenge, const chip::ByteSpan & DAC, diff --git a/src/darwin/Framework/CHIP/MTROperationalCredentialsDelegate.mm b/src/darwin/Framework/CHIP/MTROperationalCredentialsDelegate.mm index c451940e024aea..775828845203c1 100644 --- a/src/darwin/Framework/CHIP/MTROperationalCredentialsDelegate.mm +++ b/src/darwin/Framework/CHIP/MTROperationalCredentialsDelegate.mm @@ -184,6 +184,8 @@ }); VerifyOrReturnError(commissioningParameters.HasValue(), CHIP_ERROR_INCORRECT_STATE); + // Attestation Elements, nonce and signature will have a value in Commissioning Params as the CSR needs a signature or else we + // cannot trust it ReturnErrorOnFailure( chip::Credentials::DeconstructAttestationElements(commissioningParameters.Value().GetAttestationElements().Value(), certificationDeclarationSpan, attestationNonceSpan, timestampDeconstructed, firmwareInfoSpan, vendorReserved)); @@ -201,38 +203,50 @@ dispatch_sync(mNocChainIssuerQueue, ^{ [mNocChainIssuer onNOCChainGenerationNeeded:csrInfo attestationInfo:attestationInfo - onNOCChainGenerationComplete:^NSNumber *(NSData * operationalCertificate, NSData * intermediateCertificate, - NSData * rootCertificate, NSData * ipk, NSNumber * adminSubject) { - return onNOCChainGenerationComplete( - this, operationalCertificate, intermediateCertificate, rootCertificate, ipk, adminSubject); + onNOCChainGenerationComplete:^void(NSData * operationalCertificate, NSData * intermediateCertificate, + NSData * rootCertificate, NSData * ipk, NSNumber * adminSubject, NSError * __autoreleasing * error) { + onNOCChainGenerationComplete( + operationalCertificate, intermediateCertificate, rootCertificate, ipk, adminSubject, error); }]; }); return CHIP_NO_ERROR; } -NSNumber * MTROperationalCredentialsDelegate::onNOCChainGenerationComplete(MTROperationalCredentialsDelegate * thisDelegate, - NSData * operationalCertificate, NSData * intermediateCertificate, NSData * rootCertificate, NSData * _Nullable ipk, - NSNumber * _Nullable adminSubject) +void MTROperationalCredentialsDelegate::setNSError(CHIP_ERROR err, NSError * __autoreleasing * outError) { - 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()]); + if (outError) { + *outError = [MTRError errorForCHIPErrorCode:err]; + } +} + +void MTROperationalCredentialsDelegate::onNOCChainGenerationComplete(NSData * operationalCertificate, + NSData * intermediateCertificate, NSData * rootCertificate, NSData * _Nullable ipk, NSNumber * _Nullable adminSubject, + NSError * __autoreleasing * error) +{ + if (operationalCertificate == nil || intermediateCertificate == nil || rootCertificate == nil) { + setNSError(CHIP_ERROR_INVALID_ARGUMENT, error); + return; + } // use ipk and adminSubject from CommissioningParameters if not passed in __block chip::Optional commissioningParameters; dispatch_sync(mChipWorkQueue, ^{ commissioningParameters = mCppCommissioner->GetCommissioningParameters(); }); - VerifyOrReturnValue( - commissioningParameters.HasValue(), [NSNumber numberWithUnsignedInt:CHIP_ERROR_INCORRECT_STATE.AsInteger()]); + if (!commissioningParameters.HasValue()) { + setNSError(CHIP_ERROR_INCORRECT_STATE, error); + return; + } 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()]); + if ([ipk length] != sizeof(ipkValue)) { + setNSError(CHIP_ERROR_INCORRECT_STATE, error); + return; + } memcpy(&ipkValue[0], [ipk bytes], [ipk length]); ipkOptional.SetValue(ipkTempSpan); } else if (commissioningParameters.Value().GetIpk().HasValue()) { @@ -251,8 +265,8 @@ if (err != CHIP_NO_ERROR) { MTR_LOG_ERROR("Failed to SetNocChain for the device: %" CHIP_ERROR_FORMAT, err.Format()); + setNSError(CHIP_ERROR_INCORRECT_STATE, error); } - return [NSNumber numberWithUnsignedInt:err.AsInteger()]; } CHIP_ERROR MTROperationalCredentialsDelegate::LocalGenerateNOCChain(const chip::ByteSpan & csrElements,