Fixes related to PASE-only commissioning on Android (project-chip#22140)

* Draft: fixes related to PASE-only commissioning on Android

* fix build

* avoid null pointer in cleanup

src/controller/AutoCommissioner.cpp

@@ -139,6 +139,12 @@ CHIP_ERROR AutoCommissioner::SetCommissioningParameters(const CommissioningParam
     }
     mParams.SetCSRNonce(ByteSpan(mCSRNonce, sizeof(mCSRNonce)));
 
+    if (params.GetSkipCommissioningComplete().HasValue())
+    {
+        ChipLogProgress(Controller, "Setting PASE-only commissioning from parameters");
+        mParams.SetSkipCommissioningComplete(params.GetSkipCommissioningComplete().Value());
+    }
+
     return CHIP_NO_ERROR;
 }
 
@@ -252,6 +258,10 @@ CommissioningStage AutoCommissioner::GetNextCommissioningStageInternal(Commissio
         }
         else
         {
+            if (mParams.GetSkipCommissioningComplete().ValueOr(false))
+            {
+                return CommissioningStage::kCleanup;
+            }
             return CommissioningStage::kFindOperational;
         }
     case CommissioningStage::kWiFiNetworkSetup:
@@ -280,11 +290,19 @@ CommissioningStage AutoCommissioner::GetNextCommissioningStageInternal(Commissio
         {
             return CommissioningStage::kThreadNetworkEnable;
         }
+        else if (mParams.GetSkipCommissioningComplete().ValueOr(false))
+        {
+            return CommissioningStage::kCleanup;
+        }
         else
         {
             return CommissioningStage::kFindOperational;
         }
     case CommissioningStage::kThreadNetworkEnable:
+        if (mParams.GetSkipCommissioningComplete().ValueOr(false))
+        {
+            return CommissioningStage::kCleanup;
+        }
         return CommissioningStage::kFindOperational;
     case CommissioningStage::kFindOperational:
         return CommissioningStage::kSendComplete;

src/controller/CHIPDeviceController.cpp

@@ -1560,6 +1560,14 @@ void DeviceCommissioner::CommissioningStageComplete(CHIP_ERROR err, Commissionin
 {
     // Once this stage is complete, reset mDeviceBeingCommissioned - this will be reset when the delegate calls the next step.
     MATTER_TRACE_EVENT_SCOPE("CommissioningStageComplete", "DeviceCommissioner");
+    if (mDeviceBeingCommissioned == nullptr)
+    {
+        // We are getting a stray callback (e.g. due to un-cancellable
+        // operations) when we are not in fact commissioning anything.  Just
+        // ignore it.
+        return;
+    }
+
     NodeId nodeId            = mDeviceBeingCommissioned->GetDeviceId();
     DeviceProxy * proxy      = mDeviceBeingCommissioned;
     mDeviceBeingCommissioned = nullptr;

src/controller/CommissioningDelegate.cpp

@@ -113,6 +113,10 @@ const char * StageToString(CommissioningStage stage)
         return "Cleanup";
         break;
 
+    case kNeedsNetworkCreds:
+        return "NeedsNetworkCreds";
+        break;
+
     default:
         return "???";
         break;

src/controller/CommissioningDelegate.h

@@ -378,6 +378,15 @@ class CommissioningParameters
         return *this;
     }
 
+    // Only perform the PASE steps of commissioning.
+    // Commissioning will be completed by another admin on the network.
+    Optional<bool> GetSkipCommissioningComplete() const { return mSkipCommissioningComplete; }
+    CommissioningParameters & SetSkipCommissioningComplete(bool skipCommissioningComplete)
+    {
+        mSkipCommissioningComplete = MakeOptional(skipCommissioningComplete);
+        return *this;
+    }
+
 private:
     // Items that can be set by the commissioner
     Optional<uint16_t> mFailsafeTimerSeconds;
@@ -407,6 +416,7 @@ class CommissioningParameters
         nullptr; // Delegate to handle device attestation failures during commissioning
     Optional<bool> mAttemptWiFiNetworkScan;
     Optional<bool> mAttemptThreadNetworkScan; // This automatically gets set to false when a ThreadOperationalDataset is set
+    Optional<bool> mSkipCommissioningComplete;
 };
 
 struct RequestedCertificate

src/controller/java/AndroidDeviceControllerWrapper.cpp

@@ -75,7 +75,7 @@ AndroidDeviceControllerWrapper * AndroidDeviceControllerWrapper::AllocateNew(
     chip::Inet::EndPointManager<Inet::UDPEndPoint> * udpEndPointManager, AndroidOperationalCredentialsIssuerPtr opCredsIssuerPtr,
     jobject keypairDelegate, jbyteArray rootCertificate, jbyteArray intermediateCertificate, jbyteArray nodeOperationalCertificate,
     jbyteArray ipkEpochKey, uint16_t listenPort, uint16_t controllerVendorId, uint16_t failsafeTimerSeconds,
-    bool attemptNetworkScanWiFi, bool attemptNetworkScanThread, CHIP_ERROR * errInfoOnFailure)
+    bool attemptNetworkScanWiFi, bool attemptNetworkScanThread, bool skipCommissioningComplete, CHIP_ERROR * errInfoOnFailure)
 {
     if (errInfoOnFailure == nullptr)
     {
@@ -156,6 +156,7 @@ AndroidDeviceControllerWrapper * AndroidDeviceControllerWrapper::AllocateNew(
     params.SetFailsafeTimerSeconds(failsafeTimerSeconds);
     params.SetAttemptWiFiNetworkScan(attemptNetworkScanWiFi);
     params.SetAttemptThreadNetworkScan(attemptNetworkScanThread);
+    params.SetSkipCommissioningComplete(skipCommissioningComplete);
     wrapper->UpdateCommissioningParameters(params);
 
     CHIP_ERROR err = wrapper->mGroupDataProvider.Init();

src/controller/java/AndroidDeviceControllerWrapper.h

@@ -138,6 +138,7 @@ class AndroidDeviceControllerWrapper : public chip::Controller::DevicePairingDel
      * @param[in] failsafeTimerSeconds the failsafe timer in seconds
      * @param[in] attemptNetworkScanWiFi whether to attempt a network scan when configuring the network for a WiFi device
      * @param[in] attemptNetworkScanThread whether to attempt a network scan when configuring the network for a Thread device
+     * @param[in] skipCommissioningComplete whether to skip the CASE commissioningComplete command during commissioning
      * @param[out] errInfoOnFailure a pointer to a CHIP_ERROR that will be populated if this method returns nullptr
      */
     static AndroidDeviceControllerWrapper *
@@ -147,7 +148,7 @@ class AndroidDeviceControllerWrapper : public chip::Controller::DevicePairingDel
                 AndroidOperationalCredentialsIssuerPtr opCredsIssuer, jobject keypairDelegate, jbyteArray rootCertificate,
                 jbyteArray intermediateCertificate, jbyteArray nodeOperationalCertificate, jbyteArray ipkEpochKey,
                 uint16_t listenPort, uint16_t controllerVendorId, uint16_t failsafeTimerSeconds, bool attemptNetworkScanWiFi,
-                bool attemptNetworkScanThread, CHIP_ERROR * errInfoOnFailure);
+                bool attemptNetworkScanThread, bool skipCommissioningComplete, CHIP_ERROR * errInfoOnFailure);
 
     chip::Controller::AndroidOperationalCredentialsIssuer * GetAndroidOperationalCredentialsIssuer()
     {

src/controller/java/CHIPDeviceController-JNI.cpp

@@ -281,6 +281,11 @@ JNI_METHOD(jlong, newDeviceController)(JNIEnv * env, jobject self, jobject contr
                                                         &getAttemptNetworkScanThread);
     SuccessOrExit(err);
 
+    jmethodID getSkipCommissioningComplete;
+    err = chip::JniReferences::GetInstance().FindMethod(env, controllerParams, "getSkipCommissioningComplete", "()Z",
+                                                        &getSkipCommissioningComplete);
+    SuccessOrExit(err);
+
     jmethodID getKeypairDelegate;
     err = chip::JniReferences::GetInstance().FindMethod(env, controllerParams, "getKeypairDelegate",
                                                         "()Lchip/devicecontroller/KeypairDelegate;", &getKeypairDelegate);
@@ -319,15 +324,16 @@ JNI_METHOD(jlong, newDeviceController)(JNIEnv * env, jobject self, jobject contr
         uint16_t failsafeTimerSeconds      = env->CallIntMethod(controllerParams, getFailsafeTimerSeconds);
         bool attemptNetworkScanWiFi        = env->CallBooleanMethod(controllerParams, getAttemptNetworkScanWiFi);
         bool attemptNetworkScanThread      = env->CallBooleanMethod(controllerParams, getAttemptNetworkScanThread);
+        bool skipCommissioningComplete     = env->CallBooleanMethod(controllerParams, getSkipCommissioningComplete);
         uint64_t adminSubject              = env->CallLongMethod(controllerParams, getAdminSubject);
 
         std::unique_ptr<chip::Controller::AndroidOperationalCredentialsIssuer> opCredsIssuer(
             new chip::Controller::AndroidOperationalCredentialsIssuer());
         wrapper = AndroidDeviceControllerWrapper::AllocateNew(
-            sJVM, self, kLocalDeviceId, chip::kUndefinedCATs, &DeviceLayer::SystemLayer(), DeviceLayer::TCPEndPointManager(),
-            DeviceLayer::UDPEndPointManager(), std::move(opCredsIssuer), keypairDelegate, rootCertificate, intermediateCertificate,
-            operationalCertificate, ipk, listenPort, controllerVendorId, failsafeTimerSeconds, attemptNetworkScanWiFi,
-            attemptNetworkScanThread, &err);
+            sJVM, self, kLocalDeviceId, fabricId, chip::kUndefinedCATs, &DeviceLayer::SystemLayer(),
+            DeviceLayer::TCPEndPointManager(), DeviceLayer::UDPEndPointManager(), std::move(opCredsIssuer), keypairDelegate,
+            rootCertificate, intermediateCertificate, operationalCertificate, ipk, listenPort, controllerVendorId,
+            failsafeTimerSeconds, attemptNetworkScanWiFi, attemptNetworkScanThread, skipCommissioningComplete, &err);
         SuccessOrExit(err);
 
         if (adminSubject != kUndefinedNodeId)
@@ -379,7 +385,7 @@ JNI_METHOD(void, commissionDevice)
 
     ChipLogProgress(Controller, "commissionDevice() called");
 
-    CommissioningParameters commissioningParams = CommissioningParameters();
+    CommissioningParameters commissioningParams = wrapper->GetCommissioningParameters();
     if (networkCredentials != nullptr)
     {
         err = wrapper->ApplyNetworkCredentials(commissioningParams, networkCredentials);
@@ -417,7 +423,7 @@ JNI_METHOD(void, pairDevice)
 #endif
                                                 .SetPeerAddress(Transport::PeerAddress::BLE());
 
-    CommissioningParameters commissioningParams = CommissioningParameters();
+    CommissioningParameters commissioningParams = wrapper->GetCommissioningParameters();
     wrapper->ApplyNetworkCredentials(commissioningParams, networkCredentials);
 
     if (csrNonce != nullptr)
@@ -451,7 +457,8 @@ JNI_METHOD(void, pairDeviceWithAddress)
             .SetDiscriminator(discriminator)
             .SetSetupPINCode(pinCode)
             .SetPeerAddress(Transport::PeerAddress::UDP(const_cast<char *>(addrJniString.c_str()), port));
-    CommissioningParameters commissioningParams = CommissioningParameters();
+
+    CommissioningParameters commissioningParams = wrapper->GetCommissioningParameters();
     if (csrNonce != nullptr)
     {
         JniByteArray jniCsrNonce(env, csrNonce);

src/controller/java/src/chip/devicecontroller/ControllerParams.java

@@ -10,6 +10,7 @@ public final class ControllerParams {
   private final int failsafeTimerSeconds;
   private final boolean attemptNetworkScanWiFi;
   private final boolean attemptNetworkScanThread;
+  private final boolean skipCommissioningComplete;
   @Nullable private final KeypairDelegate keypairDelegate;
   @Nullable private final byte[] rootCertificate;
   @Nullable private final byte[] intermediateCertificate;
@@ -26,6 +27,7 @@ private ControllerParams(Builder builder) {
     this.failsafeTimerSeconds = builder.failsafeTimerSeconds;
     this.attemptNetworkScanWiFi = builder.attemptNetworkScanWiFi;
     this.attemptNetworkScanThread = builder.attemptNetworkScanThread;
+    this.skipCommissioningComplete = builder.skipCommissioningComplete;
     this.keypairDelegate = builder.keypairDelegate;
     this.rootCertificate = builder.rootCertificate;
     this.intermediateCertificate = builder.intermediateCertificate;
@@ -55,6 +57,10 @@ public boolean getAttemptNetworkScanThread() {
     return attemptNetworkScanThread;
   }
 
+  public boolean getSkipCommissioningComplete() {
+    return skipCommissioningComplete;
+  }
+
   public KeypairDelegate getKeypairDelegate() {
     return keypairDelegate;
   }
@@ -104,6 +110,7 @@ public static class Builder {
     private int failsafeTimerSeconds = 30;
     private boolean attemptNetworkScanWiFi = false;
     private boolean attemptNetworkScanThread = false;
+    private boolean skipCommissioningComplete = false;
     @Nullable private KeypairDelegate keypairDelegate = null;
     @Nullable private byte[] rootCertificate = null;
     @Nullable private byte[] intermediateCertificate = null;
@@ -181,6 +188,24 @@ public Builder setAttemptNetworkScanThread(boolean attemptNetworkScanThread) {
       return this;
     }
 
+    /**
+     * Disable the CASE phase of commissioning when the CommissioningComplete command is sent by
+     * this ChipDeviceCommissioner.
+     *
+     * <p>Specifically, this sets SkipCommissioningComplete in the CommissioningParameters passed to
+     * the CommissioningDelegate.
+     *
+     * <p>A controller will set this to true when the CASE phase of commissioning is done by a
+     * separate process, for example, by a Hub on the network.
+     *
+     * @param skipCommissioningComplete
+     * @return
+     */
+    public Builder setSkipCommissioningComplete(boolean skipCommissioningComplete) {
+      this.skipCommissioningComplete = skipCommissioningComplete;
+      return this;
+    }
+
     public Builder setKeypairDelegate(KeypairDelegate keypairDelegate) {
       this.keypairDelegate = keypairDelegate;
       return this;