Skip to content

Commit

Permalink
Remove unused key id persistence. (#15447)
Browse files Browse the repository at this point in the history
We are writing the key ids, but never reading them.  And there is no
real reason to persist these; they should just be ephemeral as long as
the sessions are not being persisted.
  • Loading branch information
bzbarsky-apple authored Feb 23, 2022
1 parent 9fa5641 commit ca38f0b
Show file tree
Hide file tree
Showing 3 changed files with 0 additions and 52 deletions.
47 changes: 0 additions & 47 deletions src/controller/CHIPDeviceController.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -344,15 +344,6 @@ CHIP_ERROR DeviceController::SetPairedDeviceList(ByteSpan serialized)
return err;
}

void DeviceController::PersistNextKeyId()
{
if (mStorageDelegate != nullptr && mState == State::Initialized)
{
uint16_t nextKeyID = mIDAllocator.Peek();
mStorageDelegate->SyncSetKeyValue(kNextAvailableKeyID, &nextKeyID, sizeof(nextKeyID));
}
}

CHIP_ERROR DeviceController::GetPeerAddressAndPort(PeerId peerId, Inet::IPAddress & addr, uint16_t & port)
{
VerifyOrReturnError(mState == State::Initialized, CHIP_ERROR_INCORRECT_STATE);
Expand Down Expand Up @@ -607,22 +598,6 @@ CHIP_ERROR DeviceCommissioner::Init(CommissionerInitParams params)

params.systemState->SessionMgr()->RegisterRecoveryDelegate(*this);

#if 0 //
// We cannot reinstantiate session ID allocator state from each fabric-scoped commissioner
// individually because the session ID allocator space is and must be shared for all users
// of the Session Manager. Disable persistence for now. #12821 tracks a proper fix this issue.
//

uint16_t nextKeyID = 0;
uint16_t size = sizeof(nextKeyID);
CHIP_ERROR error = mStorageDelegate->SyncGetKeyValue(kNextAvailableKeyID, &nextKeyID, size);
if ((error != CHIP_NO_ERROR) || (size != sizeof(nextKeyID)))
{
nextKeyID = 0;
}
ReturnErrorOnFailure(mIDAllocator.ReserveUpTo(nextKeyID));
#endif

mPairingDelegate = params.pairingDelegate;
if (params.defaultCommissioner != nullptr)
{
Expand Down Expand Up @@ -856,22 +831,9 @@ CHIP_ERROR DeviceCommissioner::EstablishPASEConnection(NodeId remoteDeviceId, Re
Optional<ReliableMessageProtocolConfig>::Value(GetLocalMRPConfig()), exchangeCtxt, this);
SuccessOrExit(err);

// Immediately persist the updated mNextKeyID value
// TODO maybe remove FreeRendezvousSession() since mNextKeyID is always persisted immediately
//
// Disabling session ID persistence (see previous comment in Init() about persisting key ids)
//
// PersistNextKeyId();

exit:
if (err != CHIP_NO_ERROR)
{
// Delete the current rendezvous session only if a device is not currently being paired.
if (mDeviceBeingCommissioned == nullptr)
{
FreeRendezvousSession();
}

if (device != nullptr)
{
ReleaseCommissioneeDevice(device);
Expand Down Expand Up @@ -942,8 +904,6 @@ CHIP_ERROR DeviceCommissioner::StopPairing(NodeId remoteDeviceId)
CommissioneeDeviceProxy * device = FindCommissioneeDevice(remoteDeviceId);
VerifyOrReturnError(device != nullptr, CHIP_ERROR_INVALID_DEVICE_DESCRIPTOR);

FreeRendezvousSession();

ReleaseCommissioneeDevice(device);
return CHIP_NO_ERROR;
}
Expand All @@ -954,15 +914,8 @@ CHIP_ERROR DeviceCommissioner::UnpairDevice(NodeId remoteDeviceId)
return CHIP_NO_ERROR;
}

void DeviceCommissioner::FreeRendezvousSession()
{
PersistNextKeyId();
}

void DeviceCommissioner::RendezvousCleanup(CHIP_ERROR status)
{
FreeRendezvousSession();

if (mDeviceBeingCommissioned != nullptr)
{
// Release the commissionee device. For BLE, this is stored,
Expand Down
4 changes: 0 additions & 4 deletions src/controller/CHIPDeviceController.h
Original file line number Diff line number Diff line change
Expand Up @@ -365,8 +365,6 @@ class DLL_EXPORT DeviceController : public SessionRecoveryDelegate
CHIP_ERROR SetPairedDeviceList(ByteSpan pairedDeviceSerializedSet);
ControllerDeviceInitParams GetControllerDeviceInitParams();

void PersistNextKeyId();

OperationalCredentialsDelegate * mOperationalCredentialsDelegate;

SessionIDAllocator mIDAllocator;
Expand Down Expand Up @@ -692,8 +690,6 @@ class DLL_EXPORT DeviceCommissioner : public DeviceController,

void SetupCluster(ClusterBase & base, DeviceProxy * proxy, EndpointId endpoint, Optional<System::Clock::Timeout> timeout);

void FreeRendezvousSession();

CHIP_ERROR LoadKeyId(PersistentStorageDelegate * delegate, uint16_t & out);

void OnSessionEstablishmentTimeout();
Expand Down
1 change: 0 additions & 1 deletion src/lib/support/PersistentStorageMacros.h
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ namespace chip {

constexpr const char kPairedDeviceListKeyPrefix[] = "ListPairedDevices";
constexpr const char kPairedDeviceKeyPrefix[] = "PairedDevice";
constexpr const char kNextAvailableKeyID[] = "StartKeyID";

// This macro generates a key for storage using a node ID and a key prefix, and performs the given action
// on that key.
Expand Down

0 comments on commit ca38f0b

Please sign in to comment.