Skip to content

Commit

Permalink
Remove controller API for device pairing without security (#10935)
Browse files Browse the repository at this point in the history
* Remove controller API for device pairing using test keys

* remove from Android/JNI
  • Loading branch information
pan-apple authored and pull[bot] committed Nov 30, 2021
1 parent 41184b8 commit 1124904
Show file tree
Hide file tree
Showing 10 changed files with 4 additions and 163 deletions.
8 changes: 4 additions & 4 deletions docs/QUICK_START.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,10 @@ platforms.

## Wi-Fi Nodes

| <div style="width:200px">Controller / Admin</div> | <div style="width:200px">Node</div> | Description |
| ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
| [**chip-tool**](https://github.com/project-chip/connectedhomeip/blob/master/examples/chip-tool/README.md) (Linux / Mac)<br> Includes docs for all the cluster commands supported <br> | **all-clusters-app** <li> [M5Stack](https://github.com/project-chip/connectedhomeip/blob/master/examples/all-clusters-app/esp32/README.md) (ESP)<li>[Linux](https://github.com/project-chip/connectedhomeip/tree/master/examples/all-clusters-app/linux) simulation | Use the command line tool on a laptop to pair with and control an embedded Wi-Fi platform. This demo supports the “all-clusters-app”, so it provides the basic onoff light test and more. Two incremental modes of operation are supported for testing (configurable via Kconfig tool when building M5): <ol><li> Unsecured (Wi-Fi only) <br> Bluetooth LE Rendezvous Mode Bypass = 1 <li> PASE secured <br> (BLE pairing, Wi-Fi operational) </ol> |
| [**chip-device-ctrl.py**](https://github.com/project-chip/connectedhomeip/blob/master/src/controller/python/README.md) | **all-clusters-app**<li> [M5Stack](https://github.com/project-chip/connectedhomeip/blob/master/examples/all-clusters-app/esp32/README.md) (ESP)<li> [Linux](https://github.com/project-chip/connectedhomeip/tree/master/examples/all-clusters-app/linux) simulation | Same as above, but uses the pychip tool as Controller Node, and focuses on Bluetooth LE flow rather than Bypass. |
| <div style="width:200px">Controller / Admin</div> | <div style="width:200px">Node</div> | Description |
| ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
| [**chip-tool**](https://github.com/project-chip/connectedhomeip/blob/master/examples/chip-tool/README.md) (Linux / Mac)<br> Includes docs for all the cluster commands supported <br> | **all-clusters-app** <li> [M5Stack](https://github.com/project-chip/connectedhomeip/blob/master/examples/all-clusters-app/esp32/README.md) (ESP)<li>[Linux](https://github.com/project-chip/connectedhomeip/tree/master/examples/all-clusters-app/linux) simulation | Use the command line tool on a laptop to pair with and control an embedded Wi-Fi platform. This demo supports the “all-clusters-app”, so it provides the basic onoff light test and more. |
| [**chip-device-ctrl.py**](https://github.com/project-chip/connectedhomeip/blob/master/src/controller/python/README.md) | **all-clusters-app**<li> [M5Stack](https://github.com/project-chip/connectedhomeip/blob/master/examples/all-clusters-app/esp32/README.md) (ESP)<li> [Linux](https://github.com/project-chip/connectedhomeip/tree/master/examples/all-clusters-app/linux) simulation | Same as above, but uses the pychip tool as Controller Node. |

## Thread Nodes

Expand Down
7 changes: 0 additions & 7 deletions examples/chip-tool/commands/pairing/Commands.h
Original file line number Diff line number Diff line change
Expand Up @@ -26,12 +26,6 @@ class Unpair : public PairingCommand
Unpair() : PairingCommand("unpair", PairingMode::None, PairingNetworkType::None) {}
};

class PairBypass : public PairingCommand
{
public:
PairBypass() : PairingCommand("bypass", PairingMode::Bypass, PairingNetworkType::None) {}
};

class PairQRCode : public PairingCommand
{
public:
Expand Down Expand Up @@ -159,7 +153,6 @@ void registerCommandsPairing(Commands & commands)

commands_list clusterCommands = {
make_unique<Unpair>(),
make_unique<PairBypass>(),
make_unique<PairQRCode>(),
make_unique<PairManualCode>(),
make_unique<PairBleWiFi>(),
Expand Down
9 changes: 0 additions & 9 deletions examples/chip-tool/commands/pairing/PairingCommand.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -57,9 +57,6 @@ CHIP_ERROR PairingCommand::RunInternal(NodeId remoteId)
case PairingMode::None:
err = Unpair(remoteId);
break;
case PairingMode::Bypass:
err = PairWithoutSecurity(remoteId, PeerAddress::UDP(mRemoteAddr.address, mRemotePort));
break;
case PairingMode::QRCode:
err = PairWithQRCode(remoteId);
break;
Expand Down Expand Up @@ -151,12 +148,6 @@ CHIP_ERROR PairingCommand::PairWithMdns(NodeId remoteId)
return mController.DiscoverCommissionableNodes(filter);
}

CHIP_ERROR PairingCommand::PairWithoutSecurity(NodeId remoteId, PeerAddress address)
{
ChipSerializedDevice serializedTestDevice;
return mController.PairTestDeviceWithoutSecurity(remoteId, address, serializedTestDevice);
}

CHIP_ERROR PairingCommand::Unpair(NodeId remoteId)
{
CHIP_ERROR err = mController.UnpairDevice(remoteId);
Expand Down
6 changes: 0 additions & 6 deletions examples/chip-tool/commands/pairing/PairingCommand.h
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@
enum class PairingMode
{
None,
Bypass,
QRCode,
ManualCode,
Ble,
Expand Down Expand Up @@ -82,10 +81,6 @@ class PairingCommand : public CHIPCommand,
{
case PairingMode::None:
break;
case PairingMode::Bypass:
AddArgument("device-remote-ip", &mRemoteAddr);
AddArgument("device-remote-port", 0, UINT16_MAX, &mRemotePort);
break;
case PairingMode::QRCode:
case PairingMode::ManualCode:
AddArgument("payload", &mOnboardingPayload);
Expand Down Expand Up @@ -176,7 +171,6 @@ class PairingCommand : public CHIPCommand,
CHIP_ERROR PairWithQRCode(NodeId remoteId);
CHIP_ERROR PairWithManualCode(NodeId remoteId);
CHIP_ERROR PairWithCode(NodeId remoteId, chip::SetupPayload payload);
CHIP_ERROR PairWithoutSecurity(NodeId remoteId, PeerAddress address);
CHIP_ERROR Unpair(NodeId remoteId);
CHIP_ERROR OpenCommissioningWindow();

Expand Down
72 changes: 0 additions & 72 deletions src/controller/CHIPDeviceController.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -819,78 +819,6 @@ CHIP_ERROR DeviceCommissioner::PairDevice(NodeId remoteDeviceId, RendezvousParam
return err;
}

CHIP_ERROR DeviceCommissioner::PairTestDeviceWithoutSecurity(NodeId remoteDeviceId, const Transport::PeerAddress & peerAddress,
SerializedDevice & serialized)
{
CHIP_ERROR err = CHIP_NO_ERROR;
Device * device = nullptr;

SecurePairingUsingTestSecret * testSecurePairingSecret = nullptr;

// Check that the caller has provided an IP address (instead of a BLE peer address)
VerifyOrExit(peerAddress.GetTransportType() == Transport::Type::kUdp, err = CHIP_ERROR_INVALID_ARGUMENT);
VerifyOrExit(IsOperationalNodeId(remoteDeviceId), err = CHIP_ERROR_INVALID_ARGUMENT);

VerifyOrExit(mState == State::Initialized, err = CHIP_ERROR_INCORRECT_STATE);
VerifyOrExit(mDeviceBeingPaired == kNumMaxActiveDevices, err = CHIP_ERROR_INCORRECT_STATE);

testSecurePairingSecret = chip::Platform::New<SecurePairingUsingTestSecret>();
VerifyOrExit(testSecurePairingSecret != nullptr, err = CHIP_ERROR_NO_MEMORY);

mDeviceBeingPaired = GetInactiveDeviceIndex();
VerifyOrExit(mDeviceBeingPaired < kNumMaxActiveDevices, err = CHIP_ERROR_NO_MEMORY);
device = &mActiveDevices[mDeviceBeingPaired];

testSecurePairingSecret->ToSerializable(device->GetPairing());

device->Init(GetControllerDeviceInitParams(), remoteDeviceId, peerAddress, mFabricIndex);

device->Serialize(serialized);

err = mSystemState->SessionMgr()->NewPairing(Optional<Transport::PeerAddress>::Value(peerAddress), device->GetDeviceId(),
testSecurePairingSecret, CryptoContext::SessionRole::kInitiator, mFabricIndex);
if (err != CHIP_NO_ERROR)
{
ChipLogError(Controller, "Failed in setting up secure channel: err %s", ErrorStr(err));
OnSessionEstablishmentError(err);
}
SuccessOrExit(err);

mPairedDevices.Insert(device->GetDeviceId());
mPairedDevicesUpdated = true;

// Note - This assumes storage is synchronous, the device must be in storage before we can cleanup
// the rendezvous session and mark pairing success
PersistDevice(device);
// Also persist the device list at this time
// This makes sure that a newly added device is immediately available
PersistDeviceList();

if (mPairingDelegate != nullptr)
{
mPairingDelegate->OnStatusUpdate(DevicePairingDelegate::SecurePairingSuccess);
}

RendezvousCleanup(CHIP_NO_ERROR);

exit:
if (testSecurePairingSecret != nullptr)
{
chip::Platform::Delete(testSecurePairingSecret);
}

if (err != CHIP_NO_ERROR)
{
if (device != nullptr)
{
ReleaseDevice(device);
mDeviceBeingPaired = kNumMaxActiveDevices;
}
}

return err;
}

CHIP_ERROR DeviceCommissioner::StopPairing(NodeId remoteDeviceId)
{
VerifyOrReturnError(mState == State::Initialized, CHIP_ERROR_INCORRECT_STATE);
Expand Down
3 changes: 0 additions & 3 deletions src/controller/CHIPDeviceController.h
Original file line number Diff line number Diff line change
Expand Up @@ -412,9 +412,6 @@ class DLL_EXPORT DeviceCommissioner : public DeviceController,
*/
CHIP_ERROR PairDevice(NodeId remoteDeviceId, RendezvousParameters & params);

[[deprecated("Available until Rendezvous is implemented")]] CHIP_ERROR
PairTestDeviceWithoutSecurity(NodeId remoteDeviceId, const Transport::PeerAddress & peerAddress, SerializedDevice & serialized);

/**
* @brief
* This function stops a pairing process that's in progress. It does not delete the pairing of a previously
Expand Down
24 changes: 0 additions & 24 deletions src/controller/java/CHIPDeviceController-JNI.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -286,30 +286,6 @@ JNI_METHOD(void, getConnectedDevicePointer)(JNIEnv * env, jobject self, jlong ha
wrapper->Controller()->GetConnectedDevice(nodeId, &connectedDeviceCallback->mOnSuccess, &connectedDeviceCallback->mOnFailure);
}

JNI_METHOD(void, pairTestDeviceWithoutSecurity)(JNIEnv * env, jobject self, jlong handle, jstring deviceAddr)
{
chip::DeviceLayer::StackLock lock;
CHIP_ERROR err = CHIP_NO_ERROR;
AndroidDeviceControllerWrapper * wrapper = AndroidDeviceControllerWrapper::FromJNIHandle(handle);
chip::Inet::IPAddress deviceIPAddr;

ChipLogProgress(Controller, "pairTestDeviceWithoutSecurity() called with IP Address");

const char * deviceAddrStr = env->GetStringUTFChars(deviceAddr, 0);
deviceIPAddr.FromString(deviceAddrStr, deviceIPAddr);
env->ReleaseStringUTFChars(deviceAddr, deviceAddrStr);

Controller::SerializedDevice mSerializedTestDevice;
err = wrapper->Controller()->PairTestDeviceWithoutSecurity(kRemoteDeviceId, chip::Transport::PeerAddress::UDP(deviceIPAddr),
mSerializedTestDevice);

if (err != CHIP_NO_ERROR)
{
ChipLogError(Controller, "Failed to connect to device.");
ThrowError(env, err);
}
}

JNI_METHOD(void, disconnectDevice)(JNIEnv * env, jobject self, jlong handle, jlong deviceId)
{
chip::DeviceLayer::StackLock lock;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -100,10 +100,6 @@ public void unpairDevice(long deviceId) {
unpairDevice(deviceControllerPtr, deviceId);
}

public void pairTestDeviceWithoutSecurity(String ipAddress) {
pairTestDeviceWithoutSecurity(deviceControllerPtr, ipAddress);
}

/**
* Through GetConnectedDeviceCallback, returns a pointer to a connected device or an error.
*
Expand Down Expand Up @@ -253,8 +249,6 @@ private native void pairDeviceWithAddress(
private native void getConnectedDevicePointer(
long deviceControllerPtr, long deviceId, long callbackHandle);

private native void pairTestDeviceWithoutSecurity(long deviceControllerPtr, String ipAddress);

private native boolean disconnectDevice(long deviceControllerPtr, long deviceId);

private native void deleteDeviceController(long deviceControllerPtr);
Expand Down
5 changes: 0 additions & 5 deletions src/darwin/Framework/CHIP/CHIPDeviceController.h
Original file line number Diff line number Diff line change
Expand Up @@ -49,11 +49,6 @@ typedef void (^CHIPDeviceConnectionCallback)(CHIPDevice * _Nullable device, NSEr
setupPINCode:(uint32_t)setupPINCode
error:(NSError * __autoreleasing *)error;

- (BOOL)pairDeviceWithoutSecurity:(uint64_t)deviceID
address:(NSString *)address
port:(uint16_t)port
error:(NSError * __autoreleasing *)error;

- (BOOL)pairDevice:(uint64_t)deviceID
onboardingPayload:(NSString *)onboardingPayload
onboardingPayloadType:(CHIPOnboardingPayloadType)onboardingPayloadType
Expand Down
27 changes: 0 additions & 27 deletions src/darwin/Framework/CHIP/CHIPDeviceController.mm
Original file line number Diff line number Diff line change
Expand Up @@ -320,33 +320,6 @@ - (BOOL)pairDevice:(uint64_t)deviceID
return success;
}

- (BOOL)pairDeviceWithoutSecurity:(uint64_t)deviceID
address:(NSString *)address
port:(uint16_t)port
error:(NSError * __autoreleasing *)error
{
__block CHIP_ERROR errorCode = CHIP_ERROR_INCORRECT_STATE;
__block BOOL success = NO;
if (![self isRunning]) {
success = ![self checkForError:errorCode logMsg:kErrorNotRunning error:error];
return success;
}
dispatch_sync(_chipWorkQueue, ^{
chip::Controller::SerializedDevice serializedTestDevice;
chip::Inet::IPAddress addr;
chip::Inet::IPAddress::FromString([address UTF8String], addr);

if ([self isRunning]) {
_operationalCredentialsDelegate->SetDeviceID(deviceID);
errorCode = _cppCommissioner->PairTestDeviceWithoutSecurity(
deviceID, chip::Transport::PeerAddress::UDP(addr, port), serializedTestDevice);
}
success = ![self checkForError:errorCode logMsg:kErrorPairDevice error:error];
});

return success;
}

- (BOOL)pairDevice:(uint64_t)deviceID
onboardingPayload:(NSString *)onboardingPayload
onboardingPayloadType:(CHIPOnboardingPayloadType)onboardingPayloadType
Expand Down

0 comments on commit 1124904

Please sign in to comment.