Skip to content

Commit

Permalink
Expose the Basic Information VID/PID as part of attestation results. (#…
Browse files Browse the repository at this point in the history
…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.
  • Loading branch information
bzbarsky-apple authored and pull[bot] committed Oct 17, 2023
1 parent d325d9e commit fe2ad23
Show file tree
Hide file tree
Showing 7 changed files with 40 additions and 6 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,9 @@ static inline Platform::ScopedMemoryBufferWithSize<uint8_t> 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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -295,7 +295,6 @@ class DeviceAttestationVerifier
{
public:
AttestationDeviceInfo(const AttestationInfo & attestationInfo);
AttestationDeviceInfo(const ByteSpan & attestationElementsBuffer, const ByteSpan paiDerBuffer, const ByteSpan dacDerBuffer);

~AttestationDeviceInfo() = default;

Expand All @@ -318,10 +317,16 @@ class DeviceAttestationVerifier
}
}

uint16_t BasicInformationVendorId() const { return mBasicInformationVendorId; }

uint16_t BasicInformationProductId() const { return mBasicInformationProductId; }

private:
Platform::ScopedMemoryBufferWithSize<uint8_t> mPaiDerBuffer;
Platform::ScopedMemoryBufferWithSize<uint8_t> mDacDerBuffer;
Platform::ScopedMemoryBufferWithSize<uint8_t> mCdBuffer;
uint16_t mBasicInformationVendorId;
uint16_t mBasicInformationProductId;
};

typedef void (*OnAttestationInformationVerification)(void * context, const AttestationInfo & info,
Expand Down
18 changes: 16 additions & 2 deletions src/darwin/Framework/CHIP/MTRDeviceAttestationDelegate.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
4 changes: 4 additions & 0 deletions src/darwin/Framework/CHIP/MTRDeviceAttestationDelegate.mm
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
5 changes: 5 additions & 0 deletions src/darwin/Framework/CHIPTests/MTRPairingTests.m
Original file line number Diff line number Diff line change
Expand Up @@ -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];
}

Expand Down

0 comments on commit fe2ad23

Please sign in to comment.