From 15824268c3678aaa51097c4ca95099895d10ac4e Mon Sep 17 00:00:00 2001 From: Yufeng Wang Date: Mon, 6 Feb 2023 15:53:39 -0800 Subject: [PATCH] [Andriod] Remove extra sub-interface layer from DeviceAttestationDelegate interface (#24771) * Remove DeviceAttestationCompletionCallback and DeviceAttestationFailureCallback sub-interfaces from DeviceAttestationDelegate * Address review comments * Move DeviceController init from onCreateView() to onCreate() --- .../DeviceProvisioningFragment.kt | 98 +++++++++++++------ .../java/CHIPDeviceController-JNI.cpp | 10 +- .../java/DeviceAttestationDelegateBridge.cpp | 31 ++---- .../ChipDeviceController.java | 39 ++------ .../DeviceAttestationDelegate.java | 52 ++++------ 5 files changed, 107 insertions(+), 123 deletions(-) diff --git a/examples/android/CHIPTool/app/src/main/java/com/google/chip/chiptool/provisioning/DeviceProvisioningFragment.kt b/examples/android/CHIPTool/app/src/main/java/com/google/chip/chiptool/provisioning/DeviceProvisioningFragment.kt index 62135d93eca831..9c540ee4be7fa1 100644 --- a/examples/android/CHIPTool/app/src/main/java/com/google/chip/chiptool/provisioning/DeviceProvisioningFragment.kt +++ b/examples/android/CHIPTool/app/src/main/java/com/google/chip/chiptool/provisioning/DeviceProvisioningFragment.kt @@ -30,8 +30,8 @@ import androidx.appcompat.app.AlertDialog import androidx.fragment.app.Fragment import androidx.lifecycle.lifecycleScope import chip.devicecontroller.AttestationInfo -import chip.devicecontroller.DeviceAttestationDelegate.DeviceAttestationCompletionCallback -import chip.devicecontroller.DeviceAttestationDelegate.DeviceAttestationFailureCallback +import chip.devicecontroller.ChipDeviceController +import chip.devicecontroller.DeviceAttestationDelegate import chip.devicecontroller.NetworkCredentials import com.google.chip.chiptool.NetworkCredentialsParcelable import com.google.chip.chiptool.ChipClient @@ -57,8 +57,15 @@ class DeviceProvisioningFragment : Fragment() { private val networkCredentialsParcelable: NetworkCredentialsParcelable? get() = arguments?.getParcelable(ARG_NETWORK_CREDENTIALS) + private lateinit var deviceController: ChipDeviceController + private lateinit var scope: CoroutineScope + override fun onCreate(savedInstanceState: Bundle?) { + super.onCreate(savedInstanceState) + deviceController = ChipClient.getDeviceController(requireContext()) + } + override fun onCreateView( inflater: LayoutInflater, container: ViewGroup?, @@ -66,6 +73,7 @@ class DeviceProvisioningFragment : Fragment() { ): View { scope = viewLifecycleOwner.lifecycleScope deviceInfo = checkNotNull(requireArguments().getParcelable(ARG_DEVICE_INFO)) + return inflater.inflate(R.layout.single_fragment_container, container, false).apply { if (savedInstanceState == null) { if (deviceInfo.ipAddress != null) { @@ -82,13 +90,65 @@ class DeviceProvisioningFragment : Fragment() { gatt = null } + override fun onDestroy() { + super.onDestroy() + deviceController.close() + deviceController.setDeviceAttestationDelegate(0, EmptyAttestationDelegate()) + } + + private class EmptyAttestationDelegate : DeviceAttestationDelegate { + override fun onDeviceAttestationCompleted( + devicePtr: Long, + attestationInfo: AttestationInfo, + errorCode: Int) {} + } + + private fun setAttestationDelegate() { + deviceController.setDeviceAttestationDelegate(DEVICE_ATTESTATION_FAILED_TIMEOUT + ) { devicePtr, attestationInfo, errorCode -> + Log.i(TAG, "Device attestation errorCode: $errorCode, " + + "Look at 'src/credentials/attestation_verifier/DeviceAttestationVerifier.h' " + + "AttestationVerificationResult enum to understand the errors") + + val activity = requireActivity() + + if (errorCode == STATUS_PAIRING_SUCCESS) { + activity.runOnUiThread(Runnable { + deviceController.continueCommissioning(devicePtr, true) + }) + + return@setDeviceAttestationDelegate + } + + activity.runOnUiThread(Runnable { + val dialog = AlertDialog.Builder(activity) + .setPositiveButton("Continue", + DialogInterface.OnClickListener { dialog, id -> + deviceController.continueCommissioning(devicePtr, true) + }) + .setNegativeButton("No", + DialogInterface.OnClickListener { dialog, id -> + deviceController.continueCommissioning(devicePtr, false) + }) + .setTitle("Device Attestation") + .setMessage("Device Attestation failed for device under commissioning. Do you wish to continue pairing?") + .create() + + dialog.show() + }) + } + } + private fun pairDeviceWithAddress() { // IANA CHIP port val port = 5540 val id = DeviceIdUtil.getNextAvailableId(requireContext()) - val deviceController = ChipClient.getDeviceController(requireContext()) + DeviceIdUtil.setNextAvailableId(requireContext(), id + 1) deviceController.setCompletionListener(ConnectionCallback()) + + setAttestationDelegate() + deviceController.pairDeviceWithAddress( id, deviceInfo.ipAddress, @@ -103,9 +163,7 @@ class DeviceProvisioningFragment : Fragment() { if (gatt != null) { return } - scope.launch { - val deviceController = ChipClient.getDeviceController(requireContext()) val bluetoothManager = BluetoothManager() showMessage( @@ -135,36 +193,14 @@ class DeviceProvisioningFragment : Fragment() { if (wifi != null) { network = NetworkCredentials.forWiFi(NetworkCredentials.WiFiCredentials(wifi.ssid, wifi.password)) } + val thread = networkParcelable.threadCredentials if (thread != null) { network = NetworkCredentials.forThread(NetworkCredentials.ThreadCredentials(thread.operationalDataset)) } - deviceController.setDeviceAttestationFailureCallback(DEVICE_ATTESTATION_FAILED_TIMEOUT - ) { devicePtr, errorCode -> - Log.i(TAG, "Device attestation errorCode: $errorCode, " + - "Look at 'src/credentials/attestation_verifier/DeviceAttestationVerifier.h' " + - "AttestationVerificationResult enum to understand the errors") - requireActivity().runOnUiThread(Runnable { - val alertDialog: AlertDialog? = activity?.let { - val builder = AlertDialog.Builder(it) - builder.apply { - setPositiveButton("Continue", - DialogInterface.OnClickListener { dialog, id -> - deviceController.continueCommissioning(devicePtr, true) - }) - setNegativeButton("No", - DialogInterface.OnClickListener { dialog, id -> - deviceController.continueCommissioning(devicePtr, false) - }) - } - builder.setTitle("Device Attestation") - builder.setMessage("Device Attestation failed for device under commissioning. Do you wish to continue pairing?") - // Create the AlertDialog - builder.create() - } - alertDialog?.show() - }) - } + + setAttestationDelegate() + deviceController.pairDevice(gatt, connId, deviceId, deviceInfo.setupPinCode, network) DeviceIdUtil.setNextAvailableId(requireContext(), deviceId + 1) } diff --git a/src/controller/java/CHIPDeviceController-JNI.cpp b/src/controller/java/CHIPDeviceController-JNI.cpp index 44f305d2bb13d5..78601f521f0182 100644 --- a/src/controller/java/CHIPDeviceController-JNI.cpp +++ b/src/controller/java/CHIPDeviceController-JNI.cpp @@ -1555,14 +1555,14 @@ CHIP_ERROR CreateDeviceAttestationDelegateBridge(JNIEnv * env, jlong handle, job CHIP_ERROR err = CHIP_NO_ERROR; chip::Optional timeoutSecs = chip::MakeOptional(static_cast(failSafeExpiryTimeoutSecs)); bool shouldWaitAfterDeviceAttestation = false; - jclass completionCallbackCls = nullptr; + jclass deviceAttestationDelegateCls = nullptr; jobject deviceAttestationDelegateRef = env->NewGlobalRef(deviceAttestationDelegate); + VerifyOrExit(deviceAttestationDelegateRef != nullptr, err = CHIP_JNI_ERROR_NULL_OBJECT); - JniReferences::GetInstance().GetClassRef( - env, "chip/devicecontroller/DeviceAttestationDelegate$DeviceAttestationCompletionCallback", completionCallbackCls); - VerifyOrExit(completionCallbackCls != nullptr, err = CHIP_JNI_ERROR_TYPE_NOT_FOUND); + JniReferences::GetInstance().GetClassRef(env, "chip/devicecontroller/DeviceAttestationDelegate", deviceAttestationDelegateCls); + VerifyOrExit(deviceAttestationDelegateCls != nullptr, err = CHIP_JNI_ERROR_TYPE_NOT_FOUND); - if (env->IsInstanceOf(deviceAttestationDelegate, completionCallbackCls)) + if (env->IsInstanceOf(deviceAttestationDelegate, deviceAttestationDelegateCls)) { shouldWaitAfterDeviceAttestation = true; } diff --git a/src/controller/java/DeviceAttestationDelegateBridge.cpp b/src/controller/java/DeviceAttestationDelegateBridge.cpp index be46a783b197ad..50020b93a603b1 100644 --- a/src/controller/java/DeviceAttestationDelegateBridge.cpp +++ b/src/controller/java/DeviceAttestationDelegateBridge.cpp @@ -71,19 +71,15 @@ void DeviceAttestationDelegateBridge::OnDeviceAttestationCompleted( mResult = attestationResult; if (mDeviceAttestationDelegate != nullptr) { - JNIEnv * env = JniReferences::GetInstance().GetEnvForCurrentThread(); - jclass completionCallbackCls = nullptr; - JniReferences::GetInstance().GetClassRef( - env, "chip/devicecontroller/DeviceAttestationDelegate$DeviceAttestationCompletionCallback", completionCallbackCls); - VerifyOrReturn(completionCallbackCls != nullptr, - ChipLogError(Controller, "Could not find device attestation completion callback class.")); - jclass failureCallbackCls = nullptr; - JniReferences::GetInstance().GetClassRef( - env, "chip/devicecontroller/DeviceAttestationDelegate$DeviceAttestationFailureCallback", failureCallbackCls); - VerifyOrReturn(failureCallbackCls != nullptr, - ChipLogError(Controller, "Could not find device attestation failure callback class.")); + JNIEnv * env = JniReferences::GetInstance().GetEnvForCurrentThread(); + jclass deviceAttestationDelegateCls = nullptr; - if (env->IsInstanceOf(mDeviceAttestationDelegate, completionCallbackCls)) + JniReferences::GetInstance().GetClassRef(env, "chip/devicecontroller/DeviceAttestationDelegate", + deviceAttestationDelegateCls); + VerifyOrReturn(deviceAttestationDelegateCls != nullptr, + ChipLogError(Controller, "Could not find device attestation delegate class.")); + + if (env->IsInstanceOf(mDeviceAttestationDelegate, deviceAttestationDelegateCls)) { jmethodID onDeviceAttestationCompletedMethod; JniReferences::GetInstance().FindMethod(env, mDeviceAttestationDelegate, "onDeviceAttestationCompleted", @@ -98,17 +94,6 @@ void DeviceAttestationDelegateBridge::OnDeviceAttestationCompleted( env->CallVoidMethod(mDeviceAttestationDelegate, onDeviceAttestationCompletedMethod, reinterpret_cast(device), javaAttestationInfo, static_cast(attestationResult)); } - else if ((attestationResult != chip::Credentials::AttestationVerificationResult::kSuccess) && - env->IsInstanceOf(mDeviceAttestationDelegate, failureCallbackCls)) - { - jmethodID onDeviceAttestationFailedMethod; - JniReferences::GetInstance().FindMethod(env, mDeviceAttestationDelegate, "onDeviceAttestationFailed", "(JI)V", - &onDeviceAttestationFailedMethod); - VerifyOrReturn(onDeviceAttestationFailedMethod != nullptr, - ChipLogError(Controller, "Could not find deviceAttestation failed method")); - env->CallVoidMethod(mDeviceAttestationDelegate, onDeviceAttestationFailedMethod, reinterpret_cast(device), - static_cast(attestationResult)); - } } } diff --git a/src/controller/java/src/chip/devicecontroller/ChipDeviceController.java b/src/controller/java/src/chip/devicecontroller/ChipDeviceController.java index bdcd60233c0b63..8775e687f3f0f5 100644 --- a/src/controller/java/src/chip/devicecontroller/ChipDeviceController.java +++ b/src/controller/java/src/chip/devicecontroller/ChipDeviceController.java @@ -85,42 +85,23 @@ public void setNOCChainIssuer(NOCChainIssuer issuer) { } /** - * If DeviceAttestationCompletionCallback is setted, then it will always be called when device - * attestation completes. + * If DeviceAttestationDelegate is setted, then it will always be called when device attestation + * completes. In case the device attestation fails, the client can decide to continue or stop the + * commissioning. * - *

When {@link - * DeviceAttestationDelegate.DeviceAttestationCompletionCallback#onDeviceAttestationCompleted(long, - * long, AttestationInfo, int)} is received, {@link #continueCommissioning(long, boolean)} must be + *

When {@link DeviceAttestationDelegate#onDeviceAttestationCompleted(long, long, + * AttestationInfo, int)} is received, {@link #continueCommissioning(long, boolean)} must be * called. * * @param failSafeExpiryTimeoutSecs the value to set for the fail-safe timer before * onDeviceAttestationCompleted is invoked. The unit is seconds. - * @param completionCallback the callback will be invoked when deviceattestation completed with - * device info for additional verification. + * @param deviceAttestationDelegate the delegate for device attestation completed with device info + * for additional verification. */ - public void setDeviceAttestationCompletionCallback( - int failSafeExpiryTimeoutSecs, - DeviceAttestationDelegate.DeviceAttestationCompletionCallback completionCallback) { + public void setDeviceAttestationDelegate( + int failSafeExpiryTimeoutSecs, DeviceAttestationDelegate deviceAttestationDelegate) { setDeviceAttestationDelegate( - deviceControllerPtr, failSafeExpiryTimeoutSecs, completionCallback); - } - - /** - * If DeviceAttestationFailureCallback is setted, then it will be called when device attestation - * fails, and the client can decide to continue or stop the commissioning. - * - *

When {@link - * DeviceAttestationDelegate.DeviceAttestationFailureCallback#onDeviceAttestationFailed(long, - * long, int)} is received, {@link #continueCommissioning(long, boolean)} must be called. - * - * @param failSafeExpiryTimeoutSecs the value to set for the fail-safe timer before - * onDeviceAttestationFailed is invoked. The unit is seconds. - * @param failureCallback the callback will be invoked when device attestation failed. - */ - public void setDeviceAttestationFailureCallback( - int failSafeExpiryTimeoutSecs, - DeviceAttestationDelegate.DeviceAttestationFailureCallback failureCallback) { - setDeviceAttestationDelegate(deviceControllerPtr, failSafeExpiryTimeoutSecs, failureCallback); + deviceControllerPtr, failSafeExpiryTimeoutSecs, deviceAttestationDelegate); } public void pairDevice( diff --git a/src/controller/java/src/chip/devicecontroller/DeviceAttestationDelegate.java b/src/controller/java/src/chip/devicecontroller/DeviceAttestationDelegate.java index f6de1f11333a32..cffb227f5d7824 100644 --- a/src/controller/java/src/chip/devicecontroller/DeviceAttestationDelegate.java +++ b/src/controller/java/src/chip/devicecontroller/DeviceAttestationDelegate.java @@ -1,17 +1,13 @@ package chip.devicecontroller; /** - * Only one of the following delegate callbacks should be implemented. + * Delegate for device attestation verifiers. * - *

If one of the following callbacks is implemented, {@link - * ChipDeviceController#continueCommissioning(long, boolean)} is expected to be called if - * commissioning should continue. + *

If DeviceAttestationDelegate is implemented, then onDeviceAttestationCompleted will always be + * called when device attestation completes. * - *

If DeviceAttestationCompletionCallback is implemented, then it will always be called when - * device attestation completes. - * - *

If DeviceAttestationFailureCallback is implemented, then it will be called when device - * attestation fails, and the client can decide to continue or stop the commissioning. + *

If device attestation fails, {@link ChipDeviceController#continueCommissioning(long, boolean)} + * is expected to be called to continue or stop the commissioning. * *

For example: * @@ -24,30 +20,16 @@ * */ public interface DeviceAttestationDelegate { - - public interface DeviceAttestationCompletionCallback extends DeviceAttestationDelegate { - /** - * The callback will be invoked when device attestation completed with device info for - * additional verification. - * - *

This allows the callback to stop commissioning after examining the device info (DAC, PAI, - * CD). - * - * @param devicePtr Handle of device being commissioned - * @param attestationInfo Attestation information for the device - * @param errorCode Error code on attestation failure. 0 if success. - */ - void onDeviceAttestationCompleted( - long devicePtr, AttestationInfo attestationInfo, int errorCode); - } - - public interface DeviceAttestationFailureCallback extends DeviceAttestationDelegate { - /** - * The callback will be invoked when device attestation failed - * - * @param devicePtr Handle of device being commissioned - * @param errorCode Error code for the failure. - */ - void onDeviceAttestationFailed(long devicePtr, int errorCode); - } + /** + * The callback will be invoked when device attestation completed with device info for additional + * verification. + * + *

This allows the callback to stop commissioning after examining the device info (DAC, PAI, + * CD). + * + * @param devicePtr Handle of device being commissioned + * @param attestationInfo Attestation information for the device, null is errorCode is 0. + * @param errorCode Error code on attestation failure. 0 if succeed. + */ + void onDeviceAttestationCompleted(long devicePtr, AttestationInfo attestationInfo, int errorCode); }