Skip to content

Commit

Permalink
Stop calling into the device attestation delegate on the Matter work …
Browse files Browse the repository at this point in the history
…queue. (#23867)

Also add some documentation to MTRStorage and MTRKeypair about expected callee
behavior.

Partially addresses #23277
  • Loading branch information
bzbarsky-apple authored Dec 5, 2022
1 parent 26c57c9 commit 350b2ca
Show file tree
Hide file tree
Showing 6 changed files with 20 additions and 8 deletions.
3 changes: 2 additions & 1 deletion src/darwin/Framework/CHIP/MTRCommissioningParameters.h
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,8 @@ NS_ASSUME_NONNULL_BEGIN
* An optional delegate that can be notified upon completion of device
* attestation. See documentation for MTRDeviceAttestationDelegate for
* details.
*
* The delegate methods will be invoked on an arbitrary thread.
*/
@property (nonatomic, strong, nullable) id<MTRDeviceAttestationDelegate> deviceAttestationDelegate;
/**
Expand All @@ -68,7 +70,6 @@ NS_ASSUME_NONNULL_BEGIN
*
* If nil, the fail-safe will not be extended before calling into the
* deviceAttestationDelegate.
*/
@property (nonatomic, copy, nullable) NSNumber * failSafeExpiryTimeout MTR_NEWLY_AVAILABLE;

Expand Down
4 changes: 1 addition & 3 deletions src/darwin/Framework/CHIP/MTRDeviceAttestationDelegate.h
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,7 @@ NS_ASSUME_NONNULL_BEGIN
@end

/**
* The protocol definition for the MTRDeviceAttestationDelegate
*
* All delegate methods will be called on the callers queue.
* The protocol definition for the MTRDeviceAttestationDelegate.
*/
@protocol MTRDeviceAttestationDelegate <NSObject>
@optional
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,12 +28,12 @@ NS_ASSUME_NONNULL_BEGIN
class MTRDeviceAttestationDelegateBridge : public chip::Credentials::DeviceAttestationDelegate {
public:
MTRDeviceAttestationDelegateBridge(MTRDeviceController * deviceController,
id<MTRDeviceAttestationDelegate> deviceAttestationDelegate, dispatch_queue_t queue,
chip::Optional<uint16_t> expiryTimeoutSecs, bool shouldWaitAfterDeviceAttestation = false)
id<MTRDeviceAttestationDelegate> deviceAttestationDelegate, chip::Optional<uint16_t> expiryTimeoutSecs,
bool shouldWaitAfterDeviceAttestation = false)
: mResult(chip::Credentials::AttestationVerificationResult::kSuccess)
, mDeviceController(deviceController)
, mDeviceAttestationDelegate(deviceAttestationDelegate)
, mQueue(queue)
, mQueue(dispatch_queue_create("com.csa.matter.framework.device_attestation.workqueue", DISPATCH_QUEUE_SERIAL))
, mExpiryTimeoutSecs(expiryTimeoutSecs)
, mShouldWaitAfterDeviceAttestation(shouldWaitAfterDeviceAttestation)
{
Expand Down
2 changes: 1 addition & 1 deletion src/darwin/Framework/CHIP/MTRDeviceController.mm
Original file line number Diff line number Diff line change
Expand Up @@ -439,7 +439,7 @@ - (BOOL)commissionNodeWithID:(NSNumber *)nodeID
shouldWaitAfterDeviceAttestation = YES;
}
_deviceAttestationDelegateBridge = new MTRDeviceAttestationDelegateBridge(
self, commissioningParams.deviceAttestationDelegate, _chipWorkQueue, timeoutSecs, shouldWaitAfterDeviceAttestation);
self, commissioningParams.deviceAttestationDelegate, timeoutSecs, shouldWaitAfterDeviceAttestation);
params.SetDeviceAttestationDelegate(_deviceAttestationDelegateBridge);
}

Expand Down
10 changes: 10 additions & 0 deletions src/darwin/Framework/CHIP/MTRKeypair.h
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,16 @@

NS_ASSUME_NONNULL_BEGIN

/**
* This protocol is used by the Matter framework to sign messages with a private
* key and verify signatures with a public key.
*
* The Matter framework may call keypair methods from arbitrary threads and
* concurrently.
*
* Implementations of the keypair methods must not call into any Matter
* framework APIs.
*/
@protocol MTRKeypair <NSObject>
@required
/**
Expand Down
3 changes: 3 additions & 0 deletions src/darwin/Framework/CHIP/MTRStorage.h
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,9 @@ NS_ASSUME_NONNULL_BEGIN
*
* The Matter framework may call storage methods from arbitrary threads, but
* will not call storage methods concurrently.
*
* Implementations of the storage methods must not call into any Matter
* framework APIs.
*/
MTR_NEWLY_AVAILABLE
@protocol MTRStorage <NSObject>
Expand Down

0 comments on commit 350b2ca

Please sign in to comment.