Skip to content

Commit

Permalink
[Andriod] Remove extra sub-interface layer from DeviceAttestationDele…
Browse files Browse the repository at this point in the history 
…gate interface (#24771)

* Remove DeviceAttestationCompletionCallback and DeviceAttestationFailureCallback sub-interfaces from DeviceAttestationDelegate

* Address review comments

* Move DeviceController init from onCreateView() to onCreate()
  • Loading branch information
@yufengwangca @pull
yufengwangca authored and pull[bot] committed Jan 10, 2024
1 parent 18e4109 commit 1614668
Show file tree
Hide file tree
Showing 5 changed files with 107 additions and 123 deletions.
98 changes: 67 additions & 31 deletions ...ool/app/src/main/java/com/google/chip/chiptool/provisioning/DeviceProvisioningFragment.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -57,15 +57,23 @@ 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?,
savedInstanceState: Bundle?
): 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) {
Expand All @@ -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,
Expand All @@ -103,9 +163,7 @@ class DeviceProvisioningFragment : Fragment() {
if (gatt != null) {
return
}

scope.launch {
val deviceController = ChipClient.getDeviceController(requireContext())
val bluetoothManager = BluetoothManager()

showMessage(
Expand Down Expand Up @@ -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)
}
Expand Down
10 changes: 5 additions & 5 deletions src/controller/java/CHIPDeviceController-JNI.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1555,14 +1555,14 @@ CHIP_ERROR CreateDeviceAttestationDelegateBridge(JNIEnv * env, jlong handle, job
CHIP_ERROR err = CHIP_NO_ERROR;
chip::Optional<uint16_t> timeoutSecs = chip::MakeOptional(static_cast<uint16_t>(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;
}
Expand Down
31 changes: 8 additions & 23 deletions src/controller/java/DeviceAttestationDelegateBridge.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -98,17 +94,6 @@ void DeviceAttestationDelegateBridge::OnDeviceAttestationCompleted(
env->CallVoidMethod(mDeviceAttestationDelegate, onDeviceAttestationCompletedMethod, reinterpret_cast<jlong>(device),
javaAttestationInfo, static_cast<jint>(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<jlong>(device),
static_cast<jint>(attestationResult));
}
}
}

Expand Down
39 changes: 10 additions & 29 deletions src/controller/java/src/chip/devicecontroller/ChipDeviceController.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*
* <p>When {@link
* DeviceAttestationDelegate.DeviceAttestationCompletionCallback#onDeviceAttestationCompleted(long,
* long, AttestationInfo, int)} is received, {@link #continueCommissioning(long, boolean)} must be
* <p>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.
*
* <p>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(
Expand Down
52 changes: 17 additions & 35 deletions src/controller/java/src/chip/devicecontroller/DeviceAttestationDelegate.java
View file
Original file line number Diff line number Diff line change
@@ -1,17 +1,13 @@
package chip.devicecontroller;

/**
* Only one of the following delegate callbacks should be implemented.
* Delegate for device attestation verifiers.
*
* <p>If one of the following callbacks is implemented, {@link
* ChipDeviceController#continueCommissioning(long, boolean)} is expected to be called if
* commissioning should continue.
* <p>If DeviceAttestationDelegate is implemented, then onDeviceAttestationCompleted will always be
* called when device attestation completes.
*
* <p>If DeviceAttestationCompletionCallback is implemented, then it will always be called when
* device attestation completes.
*
* <p>If DeviceAttestationFailureCallback is implemented, then it will be called when device
* attestation fails, and the client can decide to continue or stop the commissioning.
* <p>If device attestation fails, {@link ChipDeviceController#continueCommissioning(long, boolean)}
* is expected to be called to continue or stop the commissioning.
*
* <p>For example:
*
Expand All @@ -24,30 +20,16 @@
* </pre>
*/
public interface DeviceAttestationDelegate {

public interface DeviceAttestationCompletionCallback extends DeviceAttestationDelegate {
/**
* The callback will be invoked when device attestation completed with device info for
* additional verification.
*
* <p>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.
*
* <p>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);
}

0 comments on commit 1614668

Please sign in to comment.