Skip to content

Commit

Permalink
Persist and reload message counters on the controller (#7106)
Browse files Browse the repository at this point in the history 
* Persist and reload message counters on the controller

* Refactor CHIPDevice storage into an API on CHIPDevice

* Rename PersistentStorageUtils -> PersistentStorageMacros

* Fix the cluster tests

* Fix ordering of initializers

* Update comment to describe why counters are being stored

* Moved comments around

* Fix typo

* Address review comments

* remove unnecessary semicolon

Co-authored-by: Boris Zbarsky <[email protected]>
  • Loading branch information
@sagar-apple @bzbarsky-apple @pull
2 people authored and pull[bot] committed Jul 27, 2021
1 parent 559d288 commit d31b67b
Show file tree
Hide file tree
Showing 10 changed files with 190 additions and 77 deletions.
81 changes: 75 additions & 6 deletions src/controller/CHIPDevice.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,9 +46,12 @@
#include <support/CHIPMem.h>
#include <support/CodeUtils.h>
#include <support/ErrorStr.h>
#include <support/PersistentStorageMacros.h>
#include <support/SafeInt.h>
#include <support/logging/CHIPLogging.h>
#include <system/TLVPacketBufferBackingStore.h>
#include <transport/MessageCounter.h>
#include <transport/PeerMessageCounter.h>

using namespace chip::Inet;
using namespace chip::System;
Expand Down Expand Up @@ -150,8 +153,10 @@ CHIP_ERROR Device::SendCommands(app::CommandSender * commandObj)

CHIP_ERROR Device::Serialize(SerializedDevice & output)
{
CHIP_ERROR error = CHIP_NO_ERROR;
uint16_t serializedLen = 0;
CHIP_ERROR error = CHIP_NO_ERROR;
uint16_t serializedLen = 0;
uint32_t localMessageCounter = 0;
uint32_t peerMessageCounter = 0;
SerializableDevice serializable;

static_assert(BASE64_ENCODED_LEN(sizeof(serializable)) <= sizeof(output.inner),
Expand All @@ -164,6 +169,14 @@ CHIP_ERROR Device::Serialize(SerializedDevice & output)
serializable.mDevicePort = Encoding::LittleEndian::HostSwap16(mDeviceAddress.GetPort());
serializable.mAdminId = Encoding::LittleEndian::HostSwap16(mAdminId);

Transport::PeerConnectionState * connectionState = mSessionManager->GetPeerConnectionState(mSecureSession);
VerifyOrExit(connectionState != nullptr, error = CHIP_ERROR_INCORRECT_STATE);
localMessageCounter = connectionState->GetSessionMessageCounter().GetLocalMessageCounter().Value();
peerMessageCounter = connectionState->GetSessionMessageCounter().GetPeerMessageCounter().GetCounter();

serializable.mLocalMessageCounter = Encoding::LittleEndian::HostSwap32(localMessageCounter);
serializable.mPeerMessageCounter = Encoding::LittleEndian::HostSwap32(peerMessageCounter);

serializable.mCASESessionKeyId = Encoding::LittleEndian::HostSwap16(mCASESessionKeyId);
serializable.mDeviceProvisioningComplete = (mDeviceProvisioningComplete) ? 1 : 0;

Expand Down Expand Up @@ -215,10 +228,12 @@ CHIP_ERROR Device::Deserialize(const SerializedDevice & input)
IPAddress::FromString(Uint8::to_const_char(serializable.mDeviceAddr), sizeof(serializable.mDeviceAddr) - 1, ipAddress),
error = CHIP_ERROR_INVALID_ADDRESS);

mPairing = serializable.mOpsCreds;
mDeviceId = Encoding::LittleEndian::HostSwap64(serializable.mDeviceId);
port = Encoding::LittleEndian::HostSwap16(serializable.mDevicePort);
mAdminId = Encoding::LittleEndian::HostSwap16(serializable.mAdminId);
mPairing = serializable.mOpsCreds;
mDeviceId = Encoding::LittleEndian::HostSwap64(serializable.mDeviceId);
port = Encoding::LittleEndian::HostSwap16(serializable.mDevicePort);
mAdminId = Encoding::LittleEndian::HostSwap16(serializable.mAdminId);
mLocalMessageCounter = Encoding::LittleEndian::HostSwap32(serializable.mLocalMessageCounter);
mPeerMessageCounter = Encoding::LittleEndian::HostSwap32(serializable.mPeerMessageCounter);

mCASESessionKeyId = Encoding::LittleEndian::HostSwap16(serializable.mCASESessionKeyId);
mDeviceProvisioningComplete = (serializable.mDeviceProvisioningComplete != 0);
Expand Down Expand Up @@ -258,10 +273,43 @@ CHIP_ERROR Device::Deserialize(const SerializedDevice & input)
return error;
}

CHIP_ERROR Device::Persist()
{
CHIP_ERROR error = CHIP_NO_ERROR;
if (mStorageDelegate != nullptr)
{
SerializedDevice serialized;
SuccessOrExit(error = Serialize(serialized));

// TODO: no need to base-64 the serialized values AGAIN
PERSISTENT_KEY_OP(GetDeviceId(), kPairedDeviceKeyPrefix, key,
error = mStorageDelegate->SyncSetKeyValue(key, serialized.inner, sizeof(serialized.inner)));
if (error != CHIP_NO_ERROR)
{
ChipLogError(Controller, "Failed to persist device %d", error);
}
}
exit:
return error;
}

void Device::OnNewConnection(SecureSessionHandle session)
{
mState = ConnectionState::SecureConnected;
mSecureSession = session;

// Reset the message counters here because this is the first time we get a handle to the secure session.
// Since CHIPDevices can be serialized/deserialized in the middle of what is conceptually a single PASE session
// we need to restore the session counters along with the session information.
Transport::PeerConnectionState * connectionState = mSessionManager->GetPeerConnectionState(mSecureSession);
VerifyOrReturn(connectionState != nullptr);
MessageCounter & localCounter = connectionState->GetSessionMessageCounter().GetLocalMessageCounter();
if (localCounter.SetCounter(mLocalMessageCounter))
{
ChipLogError(Controller, "Unable to restore local counter to %d", mLocalMessageCounter);
}
Transport::PeerMessageCounter & peerCounter = connectionState->GetSessionMessageCounter().GetPeerMessageCounter();
peerCounter.SetCounter(mPeerMessageCounter);
}

void Device::OnConnectionExpired(SecureSessionHandle session)
Expand Down Expand Up @@ -470,5 +518,26 @@ void Device::AddReportHandler(EndpointId endpoint, ClusterId cluster, AttributeI
mCallbacksMgr.AddReportCallback(mDeviceId, endpoint, cluster, attribute, onReportCallback);
}

Device::~Device()
{
if (mExchangeMgr)
{
// Ensure that any exchange contexts we have open get closed now,
// because we don't want them to call back in to us after this
// point.
mExchangeMgr->CloseAllContextsForDelegate(this);
}

if (mStorageDelegate != nullptr && mSessionManager != nullptr)
{
// If a session can be found, persist the device so that we track the newest message counter values
Transport::PeerConnectionState * connectionState = mSessionManager->GetPeerConnectionState(mSecureSession);
if (connectionState != nullptr)
{
Persist();
}
}
}

} // namespace Controller
} // namespace chip
50 changes: 28 additions & 22 deletions src/controller/CHIPDevice.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -73,11 +73,11 @@ using DeviceTransportMgr = TransportMgr<Transport::UDP /* IPv6 */

struct ControllerDeviceInitParams
{
DeviceTransportMgr * transportMgr = nullptr;
SecureSessionMgr * sessionMgr = nullptr;
Messaging::ExchangeManager * exchangeMgr = nullptr;
Inet::InetLayer * inetLayer = nullptr;

DeviceTransportMgr * transportMgr = nullptr;
SecureSessionMgr * sessionMgr = nullptr;
Messaging::ExchangeManager * exchangeMgr = nullptr;
Inet::InetLayer * inetLayer = nullptr;
PersistentStorageDelegate * storageDelegate = nullptr;
Credentials::OperationalCredentialSet * credentials = nullptr;
#if CONFIG_NETWORK_LAYER_BLE
Ble::BleLayer * bleLayer = nullptr;
Expand All @@ -87,16 +87,7 @@ struct ControllerDeviceInitParams
class DLL_EXPORT Device : public Messaging::ExchangeDelegate, public SessionEstablishmentDelegate
{
public:
~Device()
{
if (mExchangeMgr)
{
// Ensure that any exchange contexts we have open get closed now,
// because we don't want them to call back in to us after this
// point.
mExchangeMgr->CloseAllContextsForDelegate(this);
}
}
~Device();

enum class PairingWindowOption
{
Expand Down Expand Up @@ -173,13 +164,14 @@ class DLL_EXPORT Device : public Messaging::ExchangeDelegate, public SessionEsta
*/
void Init(ControllerDeviceInitParams params, uint16_t listenPort, Transport::AdminId admin)
{
mTransportMgr = params.transportMgr;
mSessionManager = params.sessionMgr;
mExchangeMgr = params.exchangeMgr;
mInetLayer = params.inetLayer;
mListenPort = listenPort;
mAdminId = admin;
mCredentials = params.credentials;
mTransportMgr = params.transportMgr;
mSessionManager = params.sessionMgr;
mExchangeMgr = params.exchangeMgr;
mInetLayer = params.inetLayer;
mListenPort = listenPort;
mAdminId = admin;
mStorageDelegate = params.storageDelegate;
mCredentials = params.credentials;
#if CONFIG_NETWORK_LAYER_BLE
mBleLayer = params.bleLayer;
#endif
Expand Down Expand Up @@ -226,6 +218,13 @@ class DLL_EXPORT Device : public Messaging::ExchangeDelegate, public SessionEsta
**/
CHIP_ERROR Deserialize(const SerializedDevice & input);

/**
* @brief Serialize and store the Device in persistent storage
*
* @return Returns a CHIP_ERROR if either serialization or storage fails
*/
CHIP_ERROR Persist();

/**
* @brief
* Called when a new pairing is being established
Expand Down Expand Up @@ -406,6 +405,9 @@ class DLL_EXPORT Device : public Messaging::ExchangeDelegate, public SessionEsta

uint8_t mSequenceNumber = 0;

uint32_t mLocalMessageCounter = 0;
uint32_t mPeerMessageCounter = 0;

app::CHIPDeviceCallbacksMgr & mCallbacksMgr = app::CHIPDeviceCallbacksMgr::GetInstance();

/**
Expand Down Expand Up @@ -440,6 +442,8 @@ class DLL_EXPORT Device : public Messaging::ExchangeDelegate, public SessionEsta
uint16_t mCASESessionKeyId = 0;

Credentials::OperationalCredentialSet * mCredentials = nullptr;

PersistentStorageDelegate * mStorageDelegate = nullptr;
};

/**
Expand Down Expand Up @@ -494,6 +498,8 @@ typedef struct SerializableDevice
uint8_t mDeviceTransport;
uint8_t mDeviceProvisioningComplete;
uint8_t mInterfaceName[kMaxInterfaceName];
uint32_t mLocalMessageCounter; /* This field is serialized in LittleEndian byte order */
uint32_t mPeerMessageCounter; /* This field is serialized in LittleEndian byte order */
} SerializableDevice;

typedef struct SerializedDevice
Expand Down
50 changes: 15 additions & 35 deletions src/controller/CHIPDeviceController.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@
#include <support/CHIPMem.h>
#include <support/CodeUtils.h>
#include <support/ErrorStr.h>
#include <support/PersistentStorageMacros.h>
#include <support/SafeInt.h>
#include <support/ScopedBuffer.h>
#include <support/TimeUtils.h>
Expand Down Expand Up @@ -93,10 +94,6 @@ namespace Controller {

using namespace chip::Encoding;

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

#if CHIP_DEVICE_CONFIG_ENABLE_MDNS
constexpr uint16_t kMdnsPort = 5353;
#endif
Expand All @@ -107,20 +104,6 @@ constexpr uint32_t kMaxCHIPOpCertLength = 1024;
constexpr uint32_t kMaxCHIPCSRLength = 1024;
constexpr uint32_t kOpCSRNonceLength = 32;

// This macro generates a key using node ID an key prefix, and performs the given action
// on that key.
#define PERSISTENT_KEY_OP(node, keyPrefix, key, action) \
do \
{ \
constexpr size_t len = std::extent<decltype(keyPrefix)>::value; \
nlSTATIC_ASSERT_PRINT(len > 0, "keyPrefix length must be known at compile time"); \
/* 2 * sizeof(NodeId) to accomodate 2 character for each byte in Node Id */ \
char key[len + 2 * sizeof(NodeId) + 1]; \
nlSTATIC_ASSERT_PRINT(sizeof(node) <= sizeof(uint64_t), "Node ID size is greater than expected"); \
snprintf(key, sizeof(key), "%s%" PRIx64, keyPrefix, node); \
action; \
} while (0)

DeviceController::DeviceController()
{
mState = State::NotInitialized;
Expand Down Expand Up @@ -486,19 +469,13 @@ CHIP_ERROR DeviceController::UpdateDevice(Device * device, uint64_t fabricId)

void DeviceController::PersistDevice(Device * device)
{
// mStorageDelegate would not be null for a typical pairing scenario, as Pair()
// requires a valid storage delegate. However, test pairing usecase, that's used
// mainly by test applications, do not require a storage delegate. This is to
// reduce overheads on these tests.
// Let's make sure the delegate object is available before calling into it.
if (mStorageDelegate != nullptr && mState == State::Initialized)
if (mState == State::Initialized)
{
SerializedDevice serialized;
device->Serialize(serialized);

// TODO: no need to base-64 the serialized values AGAIN
PERSISTENT_KEY_OP(device->GetDeviceId(), kPairedDeviceKeyPrefix, key,
mStorageDelegate->SyncSetKeyValue(key, serialized.inner, sizeof(serialized.inner)));
device->Persist();
}
else
{
ChipLogError(Controller, "Failed to persist device. Controller not initialized.");
}
}

Expand Down Expand Up @@ -778,11 +755,14 @@ void DeviceController::OnCommissionableNodeFound(const chip::Mdns::Commissionabl

ControllerDeviceInitParams DeviceController::GetControllerDeviceInitParams()
{
return ControllerDeviceInitParams{ .transportMgr = mTransportMgr,
.sessionMgr = mSessionMgr,
.exchangeMgr = mExchangeMgr,
.inetLayer = mInetLayer,
.credentials = &mCredentials };
return ControllerDeviceInitParams{
.transportMgr = mTransportMgr,
.sessionMgr = mSessionMgr,
.exchangeMgr = mExchangeMgr,
.inetLayer = mInetLayer,
.storageDelegate = mStorageDelegate,
.credentials = &mCredentials,
};
}

DeviceCommissioner::DeviceCommissioner() :
Expand Down
4 changes: 2 additions & 2 deletions src/darwin/CHIPTool/CHIPTool/View Controllers/QRCode/QRCodeViewController.m
View file
Original file line number Diff line number Diff line change
Expand Up @@ -392,10 +392,9 @@ - (void)onPairingComplete:(NSError *)error
if (error.code != CHIPSuccess) {
NSLog(@"Got pairing error back %@", error);
} else {
dispatch_after(dispatch_time(DISPATCH_TIME_NOW, DISPATCH_TIME_NOW), dispatch_get_main_queue(), ^{
dispatch_async(dispatch_get_main_queue(), ^{
[self->_deviceList refreshDeviceList];
[self retrieveAndSendWifiCredentials];
[self setVendorIDOnAccessory];
});
}
}
Expand Down Expand Up @@ -623,6 +622,7 @@ - (void)onAddressUpdated:(NSError *)error
NSLog(@"Error retrieving device informations over Mdns: %@", error);
return;
}
[self setVendorIDOnAccessory];
}

- (void)updateUIFields:(CHIPSetupPayload *)payload decimalString:(nullable NSString *)decimalString
Expand Down
1 change: 1 addition & 0 deletions src/lib/support/BUILD.gn
View file
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ static_library("support") {
"LifetimePersistedCounter.h",
"PersistedCounter.cpp",
"PersistedCounter.h",
"PersistentStorageMacros.h",
"Pool.cpp",
"Pool.h",
"PrivateHeap.cpp",
Expand Down
48 changes: 48 additions & 0 deletions src/lib/support/PersistentStorageMacros.h
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
/*
*
* Copyright (c) 2021 Project CHIP Authors
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

/**
* @file
* This file defines and implements some utlities for generating persistent storage keys
*
*/

#pragma once

#include <support/CodeUtils.h>

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.
#define PERSISTENT_KEY_OP(node, keyPrefix, key, action) \
do \
{ \
constexpr size_t len = std::extent<decltype(keyPrefix)>::value; \
nlSTATIC_ASSERT_PRINT(len > 0, "keyPrefix length must be known at compile time"); \
/* 2 * sizeof(NodeId) to accomodate 2 character for each byte in Node Id */ \
char key[len + 2 * sizeof(NodeId) + 1]; \
nlSTATIC_ASSERT_PRINT(sizeof(node) <= sizeof(uint64_t), "Node ID size is greater than expected"); \
snprintf(key, sizeof(key), "%s%" PRIx64, keyPrefix, node); \
action; \
} while (0)

} // namespace chip
Loading

0 comments on commit d31b67b

Please sign in to comment.