From 73c2ff1c9a69fe4eae4e8e68c7a4e2ea043887dd Mon Sep 17 00:00:00 2001 From: Boris Zbarsky Date: Thu, 15 Jun 2023 22:26:52 -0400 Subject: [PATCH] Expose the Basic Information VID/PID as part of attestation results. (#27282) * Expose the Basic Information VID/PID as part of attestation results. This lets consumers see what actual VID/PID was checked against the certification declaration during attestation. While the VID is available from the certification declaration (assuming attestation passed), without this change there is no way to recover the PID short of reading the Basic Information cluster and hoping that it's not malicious and returns the same value as the one that was previously validated against the certification declaration. Removes declaration for an un-implemented constructor of AttestationDeviceInfo. * Address review comments. --- .../DeviceAttestationVerifier.cpp | 4 +++- .../DeviceAttestationVerifier.h | 7 ++++++- .../CHIP/MTRDeviceAttestationDelegate.h | 18 ++++++++++++++++-- .../CHIP/MTRDeviceAttestationDelegate.mm | 4 ++++ .../CHIP/MTRDeviceAttestationDelegateBridge.mm | 4 +++- .../MTRDeviceAttestationDelegate_Internal.h | 4 +++- .../Framework/CHIPTests/MTRPairingTests.m | 5 +++++ 7 files changed, 40 insertions(+), 6 deletions(-) diff --git a/src/credentials/attestation_verifier/DeviceAttestationVerifier.cpp b/src/credentials/attestation_verifier/DeviceAttestationVerifier.cpp index ccaaa0eed88553..cc5d9d3030de80 100644 --- a/src/credentials/attestation_verifier/DeviceAttestationVerifier.cpp +++ b/src/credentials/attestation_verifier/DeviceAttestationVerifier.cpp @@ -124,7 +124,9 @@ static inline Platform::ScopedMemoryBufferWithSize CopyByteSpanHelper(c } DeviceAttestationVerifier::AttestationDeviceInfo::AttestationDeviceInfo(const AttestationInfo & attestationInfo) : - mPaiDerBuffer(CopyByteSpanHelper(attestationInfo.paiDerBuffer)), mDacDerBuffer(CopyByteSpanHelper(attestationInfo.dacDerBuffer)) + mPaiDerBuffer(CopyByteSpanHelper(attestationInfo.paiDerBuffer)), + mDacDerBuffer(CopyByteSpanHelper(attestationInfo.dacDerBuffer)), mBasicInformationVendorId(attestationInfo.vendorId), + mBasicInformationProductId(attestationInfo.productId) { ByteSpan certificationDeclarationSpan; ByteSpan attestationNonceSpan; diff --git a/src/credentials/attestation_verifier/DeviceAttestationVerifier.h b/src/credentials/attestation_verifier/DeviceAttestationVerifier.h index b79dd392aeb3bc..0f32a91b206cdd 100644 --- a/src/credentials/attestation_verifier/DeviceAttestationVerifier.h +++ b/src/credentials/attestation_verifier/DeviceAttestationVerifier.h @@ -295,7 +295,6 @@ class DeviceAttestationVerifier { public: AttestationDeviceInfo(const AttestationInfo & attestationInfo); - AttestationDeviceInfo(const ByteSpan & attestationElementsBuffer, const ByteSpan paiDerBuffer, const ByteSpan dacDerBuffer); ~AttestationDeviceInfo() = default; @@ -318,10 +317,16 @@ class DeviceAttestationVerifier } } + uint16_t BasicInformationVendorId() const { return mBasicInformationVendorId; } + + uint16_t BasicInformationProductId() const { return mBasicInformationProductId; } + private: Platform::ScopedMemoryBufferWithSize mPaiDerBuffer; Platform::ScopedMemoryBufferWithSize mDacDerBuffer; Platform::ScopedMemoryBufferWithSize mCdBuffer; + uint16_t mBasicInformationVendorId; + uint16_t mBasicInformationProductId; }; typedef void (*OnAttestationInformationVerification)(void * context, const AttestationInfo & info, diff --git a/src/darwin/Framework/CHIP/MTRDeviceAttestationDelegate.h b/src/darwin/Framework/CHIP/MTRDeviceAttestationDelegate.h index a76f0c20145a17..889ba88096e69e 100644 --- a/src/darwin/Framework/CHIP/MTRDeviceAttestationDelegate.h +++ b/src/darwin/Framework/CHIP/MTRDeviceAttestationDelegate.h @@ -28,15 +28,29 @@ NS_ASSUME_NONNULL_BEGIN + (instancetype)new NS_UNAVAILABLE; /** - * The vendor ID for the device from the Device Attestation Certificate. May be nil only if attestation was unsucessful. + * The vendor ID from the Device Attestation Certificate. May be nil only if attestation was unsuccessful. */ @property (nonatomic, readonly, nullable) NSNumber * vendorID API_AVAILABLE(ios(16.4), macos(13.3), watchos(9.4), tvos(16.4)); /** - * The product ID for the device from the Device Attestation Certificate. May be nil only if attestation was unsucessful. + * The product ID from the Device Attestation Certificate. May be nil only if attestation was unsuccessful. */ @property (nonatomic, readonly, nullable) NSNumber * productID API_AVAILABLE(ios(16.4), macos(13.3), watchos(9.4), tvos(16.4)); +/** + * The vendor ID value from the device's Basic Information cluster that was used + * for device attestation. If attestation succeeds, this must match the vendor + * ID from the certification declaration. + */ +@property (nonatomic, readonly) NSNumber * basicInformationVendorID MTR_NEWLY_AVAILABLE; + +/** + * The product ID value from the device's Basic Information cluster that was + * used for device attestation. If attestation succeeds, this must match one of + * the product IDs from the certification declaration. + */ +@property (nonatomic, readonly) NSNumber * basicInformationProductID MTR_NEWLY_AVAILABLE; + @property (nonatomic, readonly) MTRCertificateDERBytes dacCertificate; @property (nonatomic, readonly) MTRCertificateDERBytes dacPAICertificate; @property (nonatomic, readonly, nullable) NSData * certificateDeclaration; diff --git a/src/darwin/Framework/CHIP/MTRDeviceAttestationDelegate.mm b/src/darwin/Framework/CHIP/MTRDeviceAttestationDelegate.mm index a31292f7f75102..273f03ef31f623 100644 --- a/src/darwin/Framework/CHIP/MTRDeviceAttestationDelegate.mm +++ b/src/darwin/Framework/CHIP/MTRDeviceAttestationDelegate.mm @@ -28,11 +28,15 @@ @implementation MTRDeviceAttestationDeviceInfo - (instancetype)initWithDACCertificate:(MTRCertificateDERBytes)dacCertificate dacPAICertificate:(MTRCertificateDERBytes)dacPAICertificate certificateDeclaration:(NSData *)certificateDeclaration + basicInformationVendorID:(NSNumber *)basicInformationVendorID + basicInformationProductID:(NSNumber *)basicInformationProductID { if (self = [super init]) { _dacCertificate = [dacCertificate copy]; _dacPAICertificate = [dacPAICertificate copy]; _certificateDeclaration = [certificateDeclaration copy]; + _basicInformationVendorID = [basicInformationVendorID copy]; + _basicInformationProductID = [basicInformationProductID copy]; struct AttestationCertVidPid dacVidPid; if (ExtractVIDPIDFromX509Cert(AsByteSpan(_dacCertificate), dacVidPid) == CHIP_NO_ERROR) { diff --git a/src/darwin/Framework/CHIP/MTRDeviceAttestationDelegateBridge.mm b/src/darwin/Framework/CHIP/MTRDeviceAttestationDelegateBridge.mm index 65528045710008..b9179185f959f1 100644 --- a/src/darwin/Framework/CHIP/MTRDeviceAttestationDelegateBridge.mm +++ b/src/darwin/Framework/CHIP/MTRDeviceAttestationDelegateBridge.mm @@ -43,7 +43,9 @@ MTRDeviceAttestationDeviceInfo * deviceInfo = [[MTRDeviceAttestationDeviceInfo alloc] initWithDACCertificate:dacData dacPAICertificate:paiData - certificateDeclaration:cdData]; + certificateDeclaration:cdData + basicInformationVendorID:@(info.BasicInformationVendorId()) + basicInformationProductID:@(info.BasicInformationProductId())]; NSError * error = (attestationResult == chip::Credentials::AttestationVerificationResult::kSuccess) ? nil : [MTRError errorForCHIPErrorCode:CHIP_ERROR_INTEGRITY_CHECK_FAILED]; diff --git a/src/darwin/Framework/CHIP/MTRDeviceAttestationDelegate_Internal.h b/src/darwin/Framework/CHIP/MTRDeviceAttestationDelegate_Internal.h index 9b978a903be76c..a19db3f6e6c591 100644 --- a/src/darwin/Framework/CHIP/MTRDeviceAttestationDelegate_Internal.h +++ b/src/darwin/Framework/CHIP/MTRDeviceAttestationDelegate_Internal.h @@ -23,7 +23,9 @@ NS_ASSUME_NONNULL_BEGIN - (instancetype)initWithDACCertificate:(MTRCertificateDERBytes)dacCertificate dacPAICertificate:(MTRCertificateDERBytes)dacPAICertificate - certificateDeclaration:(NSData *)certificateDeclaration; + certificateDeclaration:(NSData *)certificateDeclaration + basicInformationVendorID:(NSNumber *)basicInformationVendorID + basicInformationProductID:(NSNumber *)basicInformationProductID; @end diff --git a/src/darwin/Framework/CHIPTests/MTRPairingTests.m b/src/darwin/Framework/CHIPTests/MTRPairingTests.m index b9d012776a0d2a..5043744286486a 100644 --- a/src/darwin/Framework/CHIPTests/MTRPairingTests.m +++ b/src/darwin/Framework/CHIPTests/MTRPairingTests.m @@ -64,6 +64,11 @@ - (void)deviceAttestationCompletedForController:(MTRDeviceController *)controlle error:(NSError * _Nullable)error { [self.expectation fulfill]; + // Hard-coded to what our example server app uses for now. + // TODO: Build an example that uses the "origin" bits that allow a DAC and + // CD to have different vendor IDs, and verify things here. + XCTAssertEqualObjects(attestationDeviceInfo.basicInformationVendorID, @(0xFFF1)); + XCTAssertEqualObjects(attestationDeviceInfo.basicInformationProductID, @(0x8001)); [controller continueCommissioningDevice:opaqueDeviceHandle ignoreAttestationFailure:NO error:nil]; }