Skip to content

Commit

Permalink
[crypto] Introduce AES key abstraction (#23792)
Browse files Browse the repository at this point in the history
* [crypto] Introduce AES key abstraction

Add Aes128KeyHandle class that represents a platform-specific
AES key, either in the form of raw key material or key
reference. Also add SessionKeystore interface for managing
the lifetime of symmetric keys.

This is needed to enable more secure key management for
crypto backends that support invoking crypto operations
using key references, such as PSA crypto API.

Thanks to this class the session can only hold key references
while having the actual keys stored securely. Group key
management is still to be improved though.

* Update PSA implementation

* Code review p.2

* Code review p.3

1. Align AES key handle as uintptr_t instead of size_t.
2. Replace temporary implementation of PSA key derivation
   with a proper one that does not reveal derived keys.
3. Do not use DefaultSessionKeystore in controllers.

* Fix build
  • Loading branch information
Damian-Nordic authored and pull[bot] committed Oct 25, 2023
1 parent 457c8c1 commit 2762569
Show file tree
Hide file tree
Showing 62 changed files with 1,351 additions and 515 deletions.
2 changes: 2 additions & 0 deletions examples/chip-tool/commands/common/CHIPCommand.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -103,13 +103,15 @@ CHIP_ERROR CHIPCommand::MaybeSetUpStack()
factoryInitParams.operationalKeystore = &mOperationalKeystore;
factoryInitParams.opCertStore = &mOpCertStore;
factoryInitParams.enableServerInteractions = NeedsOperationalAdvertising();
factoryInitParams.sessionKeystore = &mSessionKeystore;

// Init group data provider that will be used for all group keys and IPKs for the
// chip-tool-configured fabrics. This is OK to do once since the fabric tables
// and the DeviceControllerFactory all "share" in the same underlying data.
// Different commissioner implementations may want to use alternate implementations
// of GroupDataProvider for injection through factoryInitParams.
sGroupDataProvider.SetStorageDelegate(&mDefaultStorage);
sGroupDataProvider.SetSessionKeystore(factoryInitParams.sessionKeystore);
ReturnLogErrorOnFailure(sGroupDataProvider.Init());
chip::Credentials::SetGroupDataProvider(&sGroupDataProvider);
factoryInitParams.groupDataProvider = &sGroupDataProvider;
Expand Down
2 changes: 2 additions & 0 deletions examples/chip-tool/commands/common/CHIPCommand.h
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
#include <credentials/GroupDataProviderImpl.h>
#include <credentials/PersistentStorageOpCertStore.h>
#include <crypto/PersistentStorageOperationalKeystore.h>
#include <crypto/RawKeySessionKeystore.h>

#pragma once

Expand Down Expand Up @@ -139,6 +140,7 @@ class CHIPCommand : public Command
#endif // CONFIG_USE_LOCAL_STORAGE
chip::PersistentStorageOperationalKeystore mOperationalKeystore;
chip::Credentials::PersistentStorageOpCertStore mOpCertStore;
chip::Crypto::RawKeySessionKeystore mSessionKeystore;

static chip::Credentials::GroupDataProviderImpl sGroupDataProvider;
CredentialIssuerCommands * mCredIssuerCmds;
Expand Down
1 change: 0 additions & 1 deletion examples/platform/linux/AppMain.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@
#include <lib/support/logging/CHIPLogging.h>

#include <credentials/DeviceAttestationCredsProvider.h>
#include <credentials/GroupDataProviderImpl.h>
#include <credentials/attestation_verifier/DefaultDeviceAttestationVerifier.h>
#include <credentials/attestation_verifier/DeviceAttestationVerifier.h>

Expand Down
4 changes: 4 additions & 0 deletions examples/platform/linux/CommissionerMain.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
#include <app/server/OnboardingCodesUtil.h>
#include <app/server/Server.h>
#include <crypto/CHIPCryptoPAL.h>
#include <crypto/RawKeySessionKeystore.h>
#include <lib/core/CHIPError.h>
#include <lib/core/NodeId.h>
#include <lib/support/logging/CHIPLogging.h>
Expand Down Expand Up @@ -122,6 +123,7 @@ MyServerStorageDelegate gServerStorage;
ExampleOperationalCredentialsIssuer gOpCredsIssuer;
NodeId gLocalId = kMaxOperationalNodeId;
Credentials::GroupDataProviderImpl gGroupDataProvider;
Crypto::RawKeySessionKeystore gSessionKeystore;

CHIP_ERROR InitCommissioner(uint16_t commissionerPort, uint16_t udcListenPort, FabricId fabricId)
{
Expand All @@ -132,8 +134,10 @@ CHIP_ERROR InitCommissioner(uint16_t commissionerPort, uint16_t udcListenPort, F
factoryParams.listenPort = commissionerPort;
factoryParams.fabricIndependentStorage = &gServerStorage;
factoryParams.fabricTable = &Server::GetInstance().GetFabricTable();
factoryParams.sessionKeystore = &gSessionKeystore;

gGroupDataProvider.SetStorageDelegate(&gServerStorage);
gGroupDataProvider.SetSessionKeystore(factoryParams.sessionKeystore);
ReturnErrorOnFailure(gGroupDataProvider.Init());
factoryParams.groupDataProvider = &gGroupDataProvider;

Expand Down
8 changes: 7 additions & 1 deletion examples/platform/nxp/se05x/linux/AppMain.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
#include <app/server/OnboardingCodesUtil.h>
#include <app/server/Server.h>
#include <crypto/CHIPCryptoPAL.h>
#include <crypto/DefaultSessionKeystore.h>
#include <lib/core/CHIPError.h>
#include <lib/core/NodeId.h>
#include <lib/support/logging/CHIPLogging.h>
Expand Down Expand Up @@ -292,13 +293,14 @@ int ChipLinuxAppInit(int argc, char * const argv[], OptionSet * customOptions)

struct CommonCaseDeviceServerInitParams_Se05x : public CommonCaseDeviceServerInitParams
{
CHIP_ERROR InitializeStaticResourcesBeforeServerInit() override
CHIP_ERROR InitializeStaticResourcesBeforeServerInit()
{
static chip::KvsPersistentStorageDelegate sKvsPersistenStorageDelegate;
static chip::PersistentStorageOperationalKeystoreHSM sPersistentStorageOperationalKeystore;
static chip::Credentials::PersistentStorageOpCertStore sPersistentStorageOpCertStore;
static chip::Credentials::GroupDataProviderImpl sGroupDataProvider;
static IgnoreCertificateValidityPolicy sDefaultCertValidityPolicy;
static chip::Crypto::DefaultSessionKeystore sSessionKeystore;

#if CHIP_CONFIG_ENABLE_SESSION_RESUMPTION
static chip::SimpleSessionResumptionStorage sSessionResumptionStorage;
Expand Down Expand Up @@ -333,8 +335,12 @@ struct CommonCaseDeviceServerInitParams_Se05x : public CommonCaseDeviceServerIni
this->opCertStore = &sPersistentStorageOpCertStore;
}

// Session Keystore injection
this->sessionKeystore = &sSessionKeystore;

// Group Data provider injection
sGroupDataProvider.SetStorageDelegate(this->persistentStorageDelegate);
sGroupDataProvider.SetSessionKeystore(this->sessionKeystore);
ReturnErrorOnFailure(sGroupDataProvider.Init());
this->groupDataProvider = &sGroupDataProvider;

Expand Down
18 changes: 17 additions & 1 deletion src/app/server/Server.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,7 @@ CHIP_ERROR Server::Init(const ServerInitParams & initParams)
VerifyOrExit(initParams.accessDelegate != nullptr, err = CHIP_ERROR_INVALID_ARGUMENT);
VerifyOrExit(initParams.aclStorage != nullptr, err = CHIP_ERROR_INVALID_ARGUMENT);
VerifyOrExit(initParams.groupDataProvider != nullptr, err = CHIP_ERROR_INVALID_ARGUMENT);
VerifyOrExit(initParams.sessionKeystore != nullptr, err = CHIP_ERROR_INVALID_ARGUMENT);
VerifyOrExit(initParams.operationalKeystore != nullptr, err = CHIP_ERROR_INVALID_ARGUMENT);
VerifyOrExit(initParams.opCertStore != nullptr, err = CHIP_ERROR_INVALID_ARGUMENT);

Expand Down Expand Up @@ -205,7 +206,8 @@ CHIP_ERROR Server::Init(const ServerInitParams & initParams)
#endif
SuccessOrExit(err);

err = mSessions.Init(&DeviceLayer::SystemLayer(), &mTransports, &mMessageCounterManager, mDeviceStorage, &GetFabricTable());
err = mSessions.Init(&DeviceLayer::SystemLayer(), &mTransports, &mMessageCounterManager, mDeviceStorage, &GetFabricTable(),
*initParams.sessionKeystore);
SuccessOrExit(err);

err = mFabricDelegate.Init(this);
Expand Down Expand Up @@ -538,4 +540,18 @@ void Server::ResumeSubscriptions()
}
#endif

KvsPersistentStorageDelegate CommonCaseDeviceServerInitParams::sKvsPersistenStorageDelegate;
PersistentStorageOperationalKeystore CommonCaseDeviceServerInitParams::sPersistentStorageOperationalKeystore;
Credentials::PersistentStorageOpCertStore CommonCaseDeviceServerInitParams::sPersistentStorageOpCertStore;
Credentials::GroupDataProviderImpl CommonCaseDeviceServerInitParams::sGroupDataProvider;
IgnoreCertificateValidityPolicy CommonCaseDeviceServerInitParams::sDefaultCertValidityPolicy;
#if CHIP_CONFIG_ENABLE_SESSION_RESUMPTION
SimpleSessionResumptionStorage CommonCaseDeviceServerInitParams::sSessionResumptionStorage;
#endif
#if CHIP_CONFIG_PERSIST_SUBSCRIPTIONS
app::SimpleSubscriptionResumptionStorage CommonCaseDeviceServerInitParams::sSubscriptionResumptionStorage;
#endif
app::DefaultAclStorage CommonCaseDeviceServerInitParams::sAclStorage;
Crypto::DefaultSessionKeystore CommonCaseDeviceServerInitParams::sSessionKeystore;

} // namespace chip
44 changes: 27 additions & 17 deletions src/app/server/Server.h
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
#include <credentials/GroupDataProviderImpl.h>
#include <credentials/OperationalCertificateStore.h>
#include <credentials/PersistentStorageOpCertStore.h>
#include <crypto/DefaultSessionKeystore.h>
#include <crypto/OperationalKeystore.h>
#include <crypto/PersistentStorageOperationalKeystore.h>
#include <inet/InetConfig.h>
Expand Down Expand Up @@ -85,8 +86,7 @@ using ServerTransportMgr = chip::TransportMgr<chip::Transport::UDP

struct ServerInitParams
{
ServerInitParams() = default;
virtual ~ServerInitParams() = default;
ServerInitParams() = default;

// Not copyable
ServerInitParams(const ServerInitParams &) = delete;
Expand Down Expand Up @@ -116,6 +116,8 @@ struct ServerInitParams
// Group data provider: MUST be injected. Used to maintain critical keys such as the Identity
// Protection Key (IPK) for CASE. Must be initialized before being provided.
Credentials::GroupDataProvider * groupDataProvider = nullptr;
// Session keystore: MUST be injected. Used to derive and manage lifecycle of symmetric keys.
Crypto::SessionKeystore * sessionKeystore = nullptr;
// Access control delegate: MUST be injected. Used to look up access control rules. Must be
// initialized before being provided.
Access::AccessControl::Delegate * accessDelegate = nullptr;
Expand Down Expand Up @@ -215,22 +217,8 @@ struct CommonCaseDeviceServerInitParams : public ServerInitParams
* @return CHIP_NO_ERROR on success or a CHIP_ERROR value from APIs called to initialize
* resources on failure.
*/
virtual CHIP_ERROR InitializeStaticResourcesBeforeServerInit()
CHIP_ERROR InitializeStaticResourcesBeforeServerInit()
{
static chip::KvsPersistentStorageDelegate sKvsPersistenStorageDelegate;
static chip::PersistentStorageOperationalKeystore sPersistentStorageOperationalKeystore;
static chip::Credentials::PersistentStorageOpCertStore sPersistentStorageOpCertStore;
static chip::Credentials::GroupDataProviderImpl sGroupDataProvider;
static IgnoreCertificateValidityPolicy sDefaultCertValidityPolicy;

#if CHIP_CONFIG_ENABLE_SESSION_RESUMPTION
static chip::SimpleSessionResumptionStorage sSessionResumptionStorage;
#endif
#if CHIP_CONFIG_PERSIST_SUBSCRIPTIONS
static chip::app::SimpleSubscriptionResumptionStorage sSubscriptionResumptionStorage;
#endif
static chip::app::DefaultAclStorage sAclStorage;

// KVS-based persistent storage delegate injection
if (persistentStorageDelegate == nullptr)
{
Expand Down Expand Up @@ -259,8 +247,12 @@ struct CommonCaseDeviceServerInitParams : public ServerInitParams
this->opCertStore = &sPersistentStorageOpCertStore;
}

// Session Keystore injection
this->sessionKeystore = &sSessionKeystore;

// Group Data provider injection
sGroupDataProvider.SetStorageDelegate(this->persistentStorageDelegate);
sGroupDataProvider.SetSessionKeystore(this->sessionKeystore);
ReturnErrorOnFailure(sGroupDataProvider.Init());
this->groupDataProvider = &sGroupDataProvider;

Expand Down Expand Up @@ -291,6 +283,21 @@ struct CommonCaseDeviceServerInitParams : public ServerInitParams

return CHIP_NO_ERROR;
}

private:
static KvsPersistentStorageDelegate sKvsPersistenStorageDelegate;
static PersistentStorageOperationalKeystore sPersistentStorageOperationalKeystore;
static Credentials::PersistentStorageOpCertStore sPersistentStorageOpCertStore;
static Credentials::GroupDataProviderImpl sGroupDataProvider;
static IgnoreCertificateValidityPolicy sDefaultCertValidityPolicy;
#if CHIP_CONFIG_ENABLE_SESSION_RESUMPTION
static SimpleSessionResumptionStorage sSessionResumptionStorage;
#endif
#if CHIP_CONFIG_PERSIST_SUBSCRIPTIONS
static app::SimpleSubscriptionResumptionStorage sSubscriptionResumptionStorage;
#endif
static app::DefaultAclStorage sAclStorage;
static Crypto::DefaultSessionKeystore sSessionKeystore;
};

/**
Expand Down Expand Up @@ -342,6 +349,8 @@ class Server

Credentials::GroupDataProvider * GetGroupDataProvider() { return mGroupsProvider; }

Crypto::SessionKeystore * GetSessionKeystore() const { return mSessionKeystore; }

#if CONFIG_NETWORK_LAYER_BLE
Ble::BleLayer * GetBleLayerObject() { return mBleLayer; }
#endif
Expand Down Expand Up @@ -556,6 +565,7 @@ class Server
app::SubscriptionResumptionStorage * mSubscriptionResumptionStorage;
Credentials::CertificateValidityPolicy * mCertificateValidityPolicy;
Credentials::GroupDataProvider * mGroupsProvider;
Crypto::SessionKeystore * mSessionKeystore;
app::DefaultAttributePersistenceProvider mAttributePersister;
GroupDataProviderListener mListener;
ServerFabricDelegate mFabricDelegate;
Expand Down
3 changes: 3 additions & 0 deletions src/app/tests/TestWriteInteraction.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
#include <app/InteractionModelEngine.h>
#include <app/tests/AppTestContext.h>
#include <credentials/GroupDataProviderImpl.h>
#include <crypto/DefaultSessionKeystore.h>
#include <lib/core/CHIPCore.h>
#include <lib/core/TLV.h>
#include <lib/core/TLVDebug.h>
Expand Down Expand Up @@ -48,6 +49,7 @@ constexpr uint16_t kMaxGroupsPerFabric = 5;
constexpr uint16_t kMaxGroupKeysPerFabric = 8;

chip::TestPersistentStorageDelegate gTestStorage;
chip::Crypto::DefaultSessionKeystore gSessionKeystore;
chip::Credentials::GroupDataProviderImpl gGroupsProvider(kMaxGroupsPerFabric, kMaxGroupKeysPerFabric);

} // namespace
Expand Down Expand Up @@ -1042,6 +1044,7 @@ int Test_Setup(void * inContext)
TestContext & ctx = *static_cast<TestContext *>(inContext);
gTestStorage.ClearStorage();
gGroupsProvider.SetStorageDelegate(&gTestStorage);
gGroupsProvider.SetSessionKeystore(&gSessionKeystore);
VerifyOrReturnError(CHIP_NO_ERROR == gGroupsProvider.Init(), FAILURE);
chip::Credentials::SetGroupDataProvider(&gGroupsProvider);

Expand Down
2 changes: 1 addition & 1 deletion src/app/tests/integration/chip_im_initiator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -716,7 +716,7 @@ int main(int argc, char * argv[])
SuccessOrExit(err);

err = gSessionManager.Init(&chip::DeviceLayer::SystemLayer(), &gTransportManager, &gMessageCounterManager, &gStorage,
&gFabricTable);
&gFabricTable, gSessionKeystore);
SuccessOrExit(err);

err = gExchangeManager.Init(&gSessionManager);
Expand Down
2 changes: 1 addition & 1 deletion src/app/tests/integration/chip_im_responder.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,7 @@ int main(int argc, char * argv[])
SuccessOrExit(err);

err = gSessionManager.Init(&chip::DeviceLayer::SystemLayer(), &gTransportManager, &gMessageCounterManager, &gStorage,
&gFabricTable);
&gFabricTable, gSessionKeystore);
SuccessOrExit(err);

err = gExchangeManager.Init(&gSessionManager);
Expand Down
1 change: 1 addition & 0 deletions src/app/tests/integration/common.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ chip::SessionHolder gSession;
chip::TestPersistentStorageDelegate gStorage;
chip::PersistentStorageOperationalKeystore gOperationalKeystore;
chip::Credentials::PersistentStorageOpCertStore gOpCertStore;
chip::Crypto::DefaultSessionKeystore gSessionKeystore;

void InitializeChip()
{
Expand Down
2 changes: 2 additions & 0 deletions src/app/tests/integration/common.h
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@

#include <app/util/basic-types.h>
#include <credentials/FabricTable.h>
#include <crypto/DefaultSessionKeystore.h>
#include <lib/support/TestPersistentStorageDelegate.h>
#include <messaging/ExchangeMgr.h>
#include <protocols/secure_channel/MessageCounterManager.h>
Expand All @@ -40,6 +41,7 @@ extern chip::SessionManager gSessionManager;
extern chip::secure_channel::MessageCounterManager gMessageCounterManager;
extern chip::SessionHolder gSession;
extern chip::TestPersistentStorageDelegate gStorage;
extern chip::Crypto::DefaultSessionKeystore gSessionKeystore;

constexpr chip::NodeId kTestNodeId = 0x1ULL;
constexpr chip::NodeId kTestNodeId1 = 0x2ULL;
Expand Down
5 changes: 4 additions & 1 deletion src/controller/CHIPDeviceControllerFactory.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@ CHIP_ERROR DeviceControllerFactory::InitSystemState()
params.fabricIndependentStorage = mFabricIndependentStorage;
params.enableServerInteractions = mEnableServerInteractions;
params.groupDataProvider = mSystemState->GetGroupDataProvider();
params.sessionKeystore = mSystemState->GetSessionKeystore();
params.fabricTable = mSystemState->Fabrics();
params.operationalKeystore = mOperationalKeystore;
params.opCertStore = mOpCertStore;
Expand Down Expand Up @@ -129,6 +130,7 @@ CHIP_ERROR DeviceControllerFactory::InitSystemState(FactoryInitParams params)
// OperationalCertificateStore needs to be provided to init the fabric table if fabric table is
// not provided wholesale.
ReturnErrorCodeIf((params.fabricTable == nullptr) && (params.opCertStore == nullptr), CHIP_ERROR_INVALID_ARGUMENT);
ReturnErrorCodeIf(params.sessionKeystore == nullptr, CHIP_ERROR_INVALID_ARGUMENT);

#if CONFIG_NETWORK_LAYER_BLE
#if CONFIG_DEVICE_LAYER
Expand Down Expand Up @@ -166,6 +168,7 @@ CHIP_ERROR DeviceControllerFactory::InitSystemState(FactoryInitParams params)
stateParams.exchangeMgr = chip::Platform::New<Messaging::ExchangeManager>();
stateParams.messageCounterManager = chip::Platform::New<secure_channel::MessageCounterManager>();
stateParams.groupDataProvider = params.groupDataProvider;
stateParams.sessionKeystore = params.sessionKeystore;

// if no fabricTable was provided, create one and track it in stateParams for cleanup
stateParams.fabricTable = params.fabricTable;
Expand Down Expand Up @@ -198,7 +201,7 @@ CHIP_ERROR DeviceControllerFactory::InitSystemState(FactoryInitParams params)

ReturnErrorOnFailure(stateParams.sessionMgr->Init(stateParams.systemLayer, stateParams.transportMgr,
stateParams.messageCounterManager, params.fabricIndependentStorage,
stateParams.fabricTable));
stateParams.fabricTable, *stateParams.sessionKeystore));
ReturnErrorOnFailure(stateParams.exchangeMgr->Init(stateParams.sessionMgr));
ReturnErrorOnFailure(stateParams.messageCounterManager->Init(stateParams.exchangeMgr));
ReturnErrorOnFailure(stateParams.unsolicitedStatusHandler->Init(stateParams.exchangeMgr));
Expand Down
7 changes: 4 additions & 3 deletions src/controller/CHIPDeviceControllerFactory.h
Original file line number Diff line number Diff line change
Expand Up @@ -94,15 +94,16 @@ struct SetupParams
CommissioningDelegate * defaultCommissioner = nullptr;
};

// TODO everything other than the fabric storage, group data provider, OperationalKeystore
// and OperationalCertificateStore here should be removed. We're blocked because of the
// need to support !CHIP_DEVICE_LAYER
// TODO everything other than the fabric storage, group data provider, OperationalKeystore,
// OperationalCertificateStore and SessionKeystore here should be removed. We're blocked
// because of the need to support !CHIP_DEVICE_LAYER
struct FactoryInitParams
{
System::Layer * systemLayer = nullptr;
PersistentStorageDelegate * fabricIndependentStorage = nullptr;
Credentials::CertificateValidityPolicy * certificateValidityPolicy = nullptr;
Credentials::GroupDataProvider * groupDataProvider = nullptr;
Crypto::SessionKeystore * sessionKeystore = nullptr;
Inet::EndPointManager<Inet::TCPEndPoint> * tcpEndPointManager = nullptr;
Inet::EndPointManager<Inet::UDPEndPoint> * udpEndPointManager = nullptr;
FabricTable * fabricTable = nullptr;
Expand Down
Loading

0 comments on commit 2762569

Please sign in to comment.