From 0ed910c32abc1eefae4040392399d36c5df99a63 Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Fri, 8 Dec 2023 13:18:21 -0500 Subject: [PATCH 1/3] Mark several clusters that are zigbee only as deprecated. (#30871) * mark several clusters as zigbee-only * update comment * zap regen --------- Co-authored-by: Andrei Litvin --- .../all-clusters-common/all-clusters-app.matter | 8 ++++---- .../tv-casting-common/tv-casting-app.matter | 2 +- .../zap-templates/zcl/data-model/chip/pwm-cluster.xml | 3 ++- .../zcl/data-model/draft/barrier-control-cluster.xml | 3 ++- .../draft/electrical-measurement-cluster.xml | 6 +++++- .../data-model/draft/input-output-value-clusters.xml | 3 ++- .../draft/onoff-switch-configuration-cluster.xml | 3 ++- src/controller/data_model/controller-clusters.matter | 10 +++++----- 8 files changed, 23 insertions(+), 15 deletions(-) diff --git a/examples/all-clusters-app/all-clusters-common/all-clusters-app.matter b/examples/all-clusters-app/all-clusters-common/all-clusters-app.matter index c8c0cc26d379bb..a9e2f8cfb4ee98 100644 --- a/examples/all-clusters-app/all-clusters-common/all-clusters-app.matter +++ b/examples/all-clusters-app/all-clusters-common/all-clusters-app.matter @@ -466,7 +466,7 @@ cluster OnOff = 6 { } /** Attributes and commands for configuring On/Off switching devices. */ -cluster OnOffSwitchConfiguration = 7 { +deprecated cluster OnOffSwitchConfiguration = 7 { revision 1; // NOTE: Default/not specifically set readonly attribute enum8 switchType = 0; @@ -605,7 +605,7 @@ cluster LevelControl = 8 { } /** An interface for reading the value of a binary measurement and accessing various characteristics of that measurement. */ -cluster BinaryInputBasic = 15 { +deprecated cluster BinaryInputBasic = 15 { revision 1; // NOTE: Default/not specifically set attribute optional char_string<16> activeText = 4; @@ -3792,7 +3792,7 @@ cluster WindowCovering = 258 { } /** This cluster provides control of a barrier (garage door). */ -cluster BarrierControl = 259 { +deprecated cluster BarrierControl = 259 { revision 1; // NOTE: Default/not specifically set bitmap BarrierControlCapabilities : bitmap8 { @@ -5415,7 +5415,7 @@ cluster LowPower = 1288 { } /** Attributes related to the electrical properties of a device. This cluster is used by power outlets and other devices that need to provide instantaneous data as opposed to metrology data which should be retrieved from the metering cluster.. */ -cluster ElectricalMeasurement = 2820 { +deprecated cluster ElectricalMeasurement = 2820 { revision 3; readonly attribute optional bitmap32 measurementType = 0; diff --git a/examples/tv-casting-app/tv-casting-common/tv-casting-app.matter b/examples/tv-casting-app/tv-casting-common/tv-casting-app.matter index 9225e5bc56bf32..36474e7f23eda3 100644 --- a/examples/tv-casting-app/tv-casting-common/tv-casting-app.matter +++ b/examples/tv-casting-app/tv-casting-common/tv-casting-app.matter @@ -326,7 +326,7 @@ cluster LevelControl = 8 { } /** An interface for reading the value of a binary measurement and accessing various characteristics of that measurement. */ -cluster BinaryInputBasic = 15 { +deprecated cluster BinaryInputBasic = 15 { revision 1; // NOTE: Default/not specifically set attribute optional char_string<16> activeText = 4; diff --git a/src/app/zap-templates/zcl/data-model/chip/pwm-cluster.xml b/src/app/zap-templates/zcl/data-model/chip/pwm-cluster.xml index ba31ad6d185b85..dcd0acd3c4d4ff 100644 --- a/src/app/zap-templates/zcl/data-model/chip/pwm-cluster.xml +++ b/src/app/zap-templates/zcl/data-model/chip/pwm-cluster.xml @@ -17,7 +17,8 @@ limitations under the License. - + + General Pulse Width Modulation 0x001c diff --git a/src/app/zap-templates/zcl/data-model/draft/barrier-control-cluster.xml b/src/app/zap-templates/zcl/data-model/draft/barrier-control-cluster.xml index ce3f7494c76d2d..282cb205020935 100644 --- a/src/app/zap-templates/zcl/data-model/draft/barrier-control-cluster.xml +++ b/src/app/zap-templates/zcl/data-model/draft/barrier-control-cluster.xml @@ -30,7 +30,8 @@ limitations under the License. - + + Barrier Control Closures 0x0103 diff --git a/src/app/zap-templates/zcl/data-model/draft/electrical-measurement-cluster.xml b/src/app/zap-templates/zcl/data-model/draft/electrical-measurement-cluster.xml index 3fca4504c4fcbe..e9246a1e3ff84d 100644 --- a/src/app/zap-templates/zcl/data-model/draft/electrical-measurement-cluster.xml +++ b/src/app/zap-templates/zcl/data-model/draft/electrical-measurement-cluster.xml @@ -17,7 +17,11 @@ limitations under the License. - + + Electrical Measurement Home Automation Attributes related to the electrical properties of a device. This cluster is used by power outlets and other devices that need to provide instantaneous data as opposed to metrology data which should be retrieved from the metering cluster.. diff --git a/src/app/zap-templates/zcl/data-model/draft/input-output-value-clusters.xml b/src/app/zap-templates/zcl/data-model/draft/input-output-value-clusters.xml index dc52fd7c90532a..132de384def242 100644 --- a/src/app/zap-templates/zcl/data-model/draft/input-output-value-clusters.xml +++ b/src/app/zap-templates/zcl/data-model/draft/input-output-value-clusters.xml @@ -17,7 +17,8 @@ limitations under the License. - + + Binary Input (Basic) General An interface for reading the value of a binary measurement and accessing various characteristics of that measurement. diff --git a/src/app/zap-templates/zcl/data-model/draft/onoff-switch-configuration-cluster.xml b/src/app/zap-templates/zcl/data-model/draft/onoff-switch-configuration-cluster.xml index 1662b2b442cf01..eb74be7c600b06 100644 --- a/src/app/zap-templates/zcl/data-model/draft/onoff-switch-configuration-cluster.xml +++ b/src/app/zap-templates/zcl/data-model/draft/onoff-switch-configuration-cluster.xml @@ -17,7 +17,8 @@ limitations under the License. - + + On/off Switch Configuration General Attributes and commands for configuring On/Off switching devices. diff --git a/src/controller/data_model/controller-clusters.matter b/src/controller/data_model/controller-clusters.matter index 73cb03e45c0693..6dfaf7efbd68c7 100644 --- a/src/controller/data_model/controller-clusters.matter +++ b/src/controller/data_model/controller-clusters.matter @@ -394,7 +394,7 @@ cluster OnOff = 6 { } /** Attributes and commands for configuring On/Off switching devices. */ -cluster OnOffSwitchConfiguration = 7 { +deprecated cluster OnOffSwitchConfiguration = 7 { revision 1; // NOTE: Default/not specifically set readonly attribute enum8 switchType = 0; @@ -533,7 +533,7 @@ cluster LevelControl = 8 { } /** An interface for reading the value of a binary measurement and accessing various characteristics of that measurement. */ -cluster BinaryInputBasic = 15 { +deprecated cluster BinaryInputBasic = 15 { revision 1; // NOTE: Default/not specifically set attribute optional char_string<16> activeText = 4; @@ -554,7 +554,7 @@ cluster BinaryInputBasic = 15 { } /** Cluster to control pulse width modulation */ -provisional cluster PulseWidthModulation = 28 { +deprecated cluster PulseWidthModulation = 28 { revision 1; // NOTE: Default/not specifically set readonly attribute command_id generatedCommandList[] = 65528; @@ -5439,7 +5439,7 @@ cluster WindowCovering = 258 { } /** This cluster provides control of a barrier (garage door). */ -cluster BarrierControl = 259 { +deprecated cluster BarrierControl = 259 { revision 1; // NOTE: Default/not specifically set bitmap BarrierControlCapabilities : bitmap8 { @@ -8067,7 +8067,7 @@ cluster ContentAppObserver = 1296 { } /** Attributes related to the electrical properties of a device. This cluster is used by power outlets and other devices that need to provide instantaneous data as opposed to metrology data which should be retrieved from the metering cluster.. */ -cluster ElectricalMeasurement = 2820 { +deprecated cluster ElectricalMeasurement = 2820 { revision 3; readonly attribute optional bitmap32 measurementType = 0; From 78fb77f8c8e8375deb0a0349cd8fd879e895926e Mon Sep 17 00:00:00 2001 From: Arkadiusz Bokowy Date: Fri, 8 Dec 2023 20:22:21 +0100 Subject: [PATCH 2/3] Split Init into SetUp/SetUpTestSuite in LoopbackMessagingContext (#30892) * Split Init into SetUp/SetUpTestSuite in LoopbackMessagingContext * Update messaging tests * Update protocols tests * Restyled by clang-format * Cleanup --------- Co-authored-by: Restyled.io --- src/app/tests/AppTestContext.cpp | 15 +-- src/app/tests/AppTestContext.h | 34 +---- src/app/tests/TestAclAttribute.cpp | 40 ++---- src/messaging/tests/MessagingContext.h | 120 +++++++++++++----- .../tests/TestAbortExchangesForFabric.cpp | 8 +- src/messaging/tests/TestExchangeHolder.cpp | 6 +- src/messaging/tests/TestExchangeMgr.cpp | 6 +- src/messaging/tests/TestMessagingLayer.cpp | 6 +- .../tests/TestReliableMessageProtocol.cpp | 31 +++-- .../secure_channel/tests/TestCASESession.cpp | 117 ++++++++--------- .../secure_channel/tests/TestPASESession.cpp | 62 +++------ 11 files changed, 213 insertions(+), 232 deletions(-) diff --git a/src/app/tests/AppTestContext.cpp b/src/app/tests/AppTestContext.cpp index 995bfac46cdfea..6f22097d7d11e8 100644 --- a/src/app/tests/AppTestContext.cpp +++ b/src/app/tests/AppTestContext.cpp @@ -41,10 +41,8 @@ namespace Test { CHIP_ERROR AppContext::SetUpTestSuite() { CHIP_ERROR err = CHIP_NO_ERROR; - VerifyOrExit((err = chip::Platform::MemoryInit()) == CHIP_NO_ERROR, - ChipLogError(AppServer, "Init CHIP memory failed: %" CHIP_ERROR_FORMAT, err.Format())); - VerifyOrExit((err = LoopbackTransportManager::Init()) == CHIP_NO_ERROR, - ChipLogError(AppServer, "Init LoopbackTransportManager failed: %" CHIP_ERROR_FORMAT, err.Format())); + VerifyOrExit((err = LoopbackMessagingContext::SetUpTestSuite()) == CHIP_NO_ERROR, + ChipLogError(AppServer, "SetUpTestSuite lo messaging context failed: %" CHIP_ERROR_FORMAT, err.Format())); VerifyOrExit((err = chip::DeviceLayer::PlatformMgr().InitChipStack()) == CHIP_NO_ERROR, ChipLogError(AppServer, "Init CHIP stack failed: %" CHIP_ERROR_FORMAT, err.Format())); exit: @@ -54,15 +52,14 @@ CHIP_ERROR AppContext::SetUpTestSuite() void AppContext::TearDownTestSuite() { chip::DeviceLayer::PlatformMgr().Shutdown(); - LoopbackTransportManager::Shutdown(); - chip::Platform::MemoryShutdown(); + LoopbackMessagingContext::TearDownTestSuite(); } CHIP_ERROR AppContext::SetUp() { CHIP_ERROR err = CHIP_NO_ERROR; - VerifyOrExit((err = MessagingContext::Init(&GetTransportMgr(), &GetIOContext())) == CHIP_NO_ERROR, - ChipLogError(AppServer, "Init MessagingContext failed: %" CHIP_ERROR_FORMAT, err.Format())); + VerifyOrExit((err = LoopbackMessagingContext::SetUp()) == CHIP_NO_ERROR, + ChipLogError(AppServer, "SetUp lo messaging context failed: %" CHIP_ERROR_FORMAT, err.Format())); VerifyOrExit((err = app::InteractionModelEngine::GetInstance()->Init( &GetExchangeManager(), &GetFabricTable(), app::reporting::GetDefaultReportScheduler())) == CHIP_NO_ERROR, ChipLogError(AppServer, "Init InteractionModelEngine failed: %" CHIP_ERROR_FORMAT, err.Format())); @@ -79,7 +76,7 @@ void AppContext::TearDown() Access::GetAccessControl().Finish(); Access::ResetAccessControlToDefault(); chip::app::InteractionModelEngine::GetInstance()->Shutdown(); - MessagingContext::Shutdown(); + LoopbackMessagingContext::TearDown(); } } // namespace Test diff --git a/src/app/tests/AppTestContext.h b/src/app/tests/AppTestContext.h index e4a1691f32382b..40605f237db7ab 100644 --- a/src/app/tests/AppTestContext.h +++ b/src/app/tests/AppTestContext.h @@ -28,39 +28,13 @@ class AppContext : public LoopbackMessagingContext { public: // Performs shared setup for all tests in the test suite - virtual CHIP_ERROR SetUpTestSuite(); + CHIP_ERROR SetUpTestSuite() override; // Performs shared teardown for all tests in the test suite - virtual void TearDownTestSuite(); + void TearDownTestSuite() override; // Performs setup for each individual test in the test suite - virtual CHIP_ERROR SetUp(); + CHIP_ERROR SetUp() override; // Performs teardown for each individual test in the test suite - virtual void TearDown(); - - // Helpers that can be used directly by the nlTestSuite - - static int nlTestSetUpTestSuite(void * context) - { - auto err = static_cast(context)->SetUpTestSuite(); - return err == CHIP_NO_ERROR ? SUCCESS : FAILURE; - } - - static int nlTestTearDownTestSuite(void * context) - { - static_cast(context)->TearDownTestSuite(); - return SUCCESS; - } - - static int nlTestSetUp(void * context) - { - auto err = static_cast(context)->SetUp(); - return err == CHIP_NO_ERROR ? SUCCESS : FAILURE; - } - - static int nlTestTearDown(void * context) - { - static_cast(context)->TearDown(); - return SUCCESS; - } + void TearDown() override; }; } // namespace Test diff --git a/src/app/tests/TestAclAttribute.cpp b/src/app/tests/TestAclAttribute.cpp index 9632062a0838fe..16766caf02bb8e 100644 --- a/src/app/tests/TestAclAttribute.cpp +++ b/src/app/tests/TestAclAttribute.cpp @@ -82,24 +82,14 @@ class TestDeviceTypeResolver : public AccessControl::DeviceTypeResolver class TestAccessContext : public chip::Test::AppContext { public: - static int Initialize(void * context) + // Performs setup for each individual test in the test suite + CHIP_ERROR SetUp() override { - if (AppContext::Initialize(context) != SUCCESS) - return FAILURE; + ReturnErrorOnFailure(chip::Test::AppContext::SetUp()); Access::GetAccessControl().Finish(); Access::GetAccessControl().Init(GetTestAccessControlDelegate(), gDeviceTypeResolver); - return SUCCESS; - } - - static int Finalize(void * context) - { - if (AppContext::Finalize(context) != SUCCESS) - return FAILURE; - return SUCCESS; + return CHIP_NO_ERROR; } - -private: - chip::MonotonicallyIncreasingCounter mEventCounter; }; class MockInteractionModelApp : public chip::app::ReadClient::Callback @@ -264,27 +254,19 @@ void TestAclAttribute::TestACLDeniedAttribute(nlTestSuite * apSuite, void * apCo namespace { -/** - * Test Suite. It lists all the test functions. - */ - -// clang-format off -const nlTest sTests[] = -{ +const nlTest sTests[] = { NL_TEST_DEF("TestACLDeniedAttribute", chip::app::TestAclAttribute::TestACLDeniedAttribute), - NL_TEST_SENTINEL() + NL_TEST_SENTINEL(), }; -// clang-format on -// clang-format off -nlTestSuite sSuite = -{ +nlTestSuite sSuite = { "TestAclAttribute", &sTests[0], - TestAccessContext::Initialize, - TestAccessContext::Finalize + TestAccessContext::nlTestSetUpTestSuite, + TestAccessContext::nlTestTearDownTestSuite, + TestAccessContext::nlTestSetUp, + TestAccessContext::nlTestTearDown, }; -// clang-format on } // namespace diff --git a/src/messaging/tests/MessagingContext.h b/src/messaging/tests/MessagingContext.h index ed6ead5d2da3ae..a6a6d34fa1c36c 100644 --- a/src/messaging/tests/MessagingContext.h +++ b/src/messaging/tests/MessagingContext.h @@ -208,35 +208,61 @@ class LoopbackMessagingContext : public LoopbackTransportManager, public Messagi public: virtual ~LoopbackMessagingContext() {} - /// Initialize the underlying layers. - virtual CHIP_ERROR Init() + // Performs shared setup for all tests in the test suite + virtual CHIP_ERROR SetUpTestSuite() { - ReturnErrorOnFailure(chip::Platform::MemoryInit()); - ReturnErrorOnFailure(LoopbackTransportManager::Init()); - ReturnErrorOnFailure(MessagingContext::Init(&GetTransportMgr(), &GetIOContext())); - return CHIP_NO_ERROR; + CHIP_ERROR err = CHIP_NO_ERROR; + VerifyOrExit((err = chip::Platform::MemoryInit()) == CHIP_NO_ERROR, + ChipLogError(AppServer, "Init CHIP memory failed: %" CHIP_ERROR_FORMAT, err.Format())); + VerifyOrExit((err = LoopbackTransportManager::Init()) == CHIP_NO_ERROR, + ChipLogError(AppServer, "Init LoopbackTransportManager failed: %" CHIP_ERROR_FORMAT, err.Format())); + exit: + return err; } - // Shutdown all layers, finalize operations - virtual void Shutdown() + // Performs shared teardown for all tests in the test suite + virtual void TearDownTestSuite() { - MessagingContext::Shutdown(); LoopbackTransportManager::Shutdown(); chip::Platform::MemoryShutdown(); } - // Init/Shutdown Helpers that can be used directly as the nlTestSuite - // initialize/finalize function. - static int Initialize(void * context) + // Performs setup for each individual test in the test suite + virtual CHIP_ERROR SetUp() + { + CHIP_ERROR err = CHIP_NO_ERROR; + VerifyOrExit((err = MessagingContext::Init(&GetTransportMgr(), &GetIOContext())) == CHIP_NO_ERROR, + ChipLogError(AppServer, "Init MessagingContext failed: %" CHIP_ERROR_FORMAT, err.Format())); + exit: + return err; + } + + // Performs teardown for each individual test in the test suite + virtual void TearDown() { MessagingContext::Shutdown(); } + + // Helpers that can be used directly by the nlTestSuite + + static int nlTestSetUpTestSuite(void * context) + { + auto err = static_cast(context)->SetUpTestSuite(); + return err == CHIP_NO_ERROR ? SUCCESS : FAILURE; + } + + static int nlTestTearDownTestSuite(void * context) + { + static_cast(context)->TearDownTestSuite(); + return SUCCESS; + } + + static int nlTestSetUp(void * context) { - auto * ctx = static_cast(context); - return ctx->Init() == CHIP_NO_ERROR ? SUCCESS : FAILURE; + auto err = static_cast(context)->SetUp(); + return err == CHIP_NO_ERROR ? SUCCESS : FAILURE; } - static int Finalize(void * context) + static int nlTestTearDown(void * context) { - auto * ctx = static_cast(context); - ctx->Shutdown(); + static_cast(context)->TearDown(); return SUCCESS; } @@ -249,35 +275,61 @@ class UDPMessagingContext : public UDPTransportManager, public MessagingContext public: virtual ~UDPMessagingContext() {} - /// Initialize the underlying layers. - virtual CHIP_ERROR Init() + // Performs shared setup for all tests in the test suite + virtual CHIP_ERROR SetUpTestSuite() { - ReturnErrorOnFailure(chip::Platform::MemoryInit()); - ReturnErrorOnFailure(UDPTransportManager::Init()); - ReturnErrorOnFailure(MessagingContext::Init(&GetTransportMgr(), &GetIOContext())); - return CHIP_NO_ERROR; + CHIP_ERROR err = CHIP_NO_ERROR; + VerifyOrExit((err = chip::Platform::MemoryInit()) == CHIP_NO_ERROR, + ChipLogError(AppServer, "Init CHIP memory failed: %" CHIP_ERROR_FORMAT, err.Format())); + VerifyOrExit((err = UDPTransportManager::Init()) == CHIP_NO_ERROR, + ChipLogError(AppServer, "Init UDPTransportManager failed: %" CHIP_ERROR_FORMAT, err.Format())); + exit: + return err; } - // Shutdown all layers, finalize operations - virtual void Shutdown() + // Performs shared teardown for all tests in the test suite + virtual void TearDownTestSuite() { - MessagingContext::Shutdown(); UDPTransportManager::Shutdown(); chip::Platform::MemoryShutdown(); } - // Init/Shutdown Helpers that can be used directly as the nlTestSuite - // initialize/finalize function. - static int Initialize(void * context) + // Performs setup for each individual test in the test suite + virtual CHIP_ERROR SetUp() + { + CHIP_ERROR err = CHIP_NO_ERROR; + VerifyOrExit((err = MessagingContext::Init(&GetTransportMgr(), &GetIOContext())) == CHIP_NO_ERROR, + ChipLogError(AppServer, "Init MessagingContext failed: %" CHIP_ERROR_FORMAT, err.Format())); + exit: + return err; + } + + // Performs teardown for each individual test in the test suite + virtual void TearDown() { MessagingContext::Shutdown(); } + + // Helpers that can be used directly by the nlTestSuite + + static int nlTestSetUpTestSuite(void * context) + { + auto err = static_cast(context)->SetUpTestSuite(); + return err == CHIP_NO_ERROR ? SUCCESS : FAILURE; + } + + static int nlTestTearDownTestSuite(void * context) + { + static_cast(context)->TearDownTestSuite(); + return SUCCESS; + } + + static int nlTestSetUp(void * context) { - auto * ctx = static_cast(context); - return ctx->Init() == CHIP_NO_ERROR ? SUCCESS : FAILURE; + auto err = static_cast(context)->SetUp(); + return err == CHIP_NO_ERROR ? SUCCESS : FAILURE; } - static int Finalize(void * context) + static int nlTestTearDown(void * context) { - auto * ctx = static_cast(context); - ctx->Shutdown(); + static_cast(context)->TearDown(); return SUCCESS; } diff --git a/src/messaging/tests/TestAbortExchangesForFabric.cpp b/src/messaging/tests/TestAbortExchangesForFabric.cpp index 5354ee4cbde781..4c2d682441ab82 100644 --- a/src/messaging/tests/TestAbortExchangesForFabric.cpp +++ b/src/messaging/tests/TestAbortExchangesForFabric.cpp @@ -258,12 +258,16 @@ const nlTest sTests[] = { NL_TEST_SENTINEL(), }; +// clang-format off nlTestSuite sSuite = { "Test-AbortExchangesForFabric", &sTests[0], - TestContext::Initialize, - TestContext::Finalize, + TestContext::nlTestSetUpTestSuite, + TestContext::nlTestTearDownTestSuite, + TestContext::nlTestSetUp, + TestContext::nlTestTearDown, }; +// clang-format on } // namespace diff --git a/src/messaging/tests/TestExchangeHolder.cpp b/src/messaging/tests/TestExchangeHolder.cpp index 611e2039b886b5..9325350d262e1b 100644 --- a/src/messaging/tests/TestExchangeHolder.cpp +++ b/src/messaging/tests/TestExchangeHolder.cpp @@ -821,8 +821,10 @@ nlTestSuite sSuite = { "Test-TestExchangeHolder", &sTests[0], - TestContext::Initialize, - TestContext::Finalize + TestContext::nlTestSetUpTestSuite, + TestContext::nlTestTearDownTestSuite, + TestContext::nlTestSetUp, + TestContext::nlTestTearDown, }; // clang-format on diff --git a/src/messaging/tests/TestExchangeMgr.cpp b/src/messaging/tests/TestExchangeMgr.cpp index b3fbba2f1b98bc..69e1fbaa8f9afa 100644 --- a/src/messaging/tests/TestExchangeMgr.cpp +++ b/src/messaging/tests/TestExchangeMgr.cpp @@ -285,8 +285,10 @@ nlTestSuite sSuite = { "Test-CHIP-ExchangeManager", &sTests[0], - TestContext::Initialize, - TestContext::Finalize + TestContext::nlTestSetUpTestSuite, + TestContext::nlTestTearDownTestSuite, + TestContext::nlTestSetUp, + TestContext::nlTestTearDown, }; // clang-format on diff --git a/src/messaging/tests/TestMessagingLayer.cpp b/src/messaging/tests/TestMessagingLayer.cpp index 7e0285d6f27170..2c05c067989e42 100644 --- a/src/messaging/tests/TestMessagingLayer.cpp +++ b/src/messaging/tests/TestMessagingLayer.cpp @@ -162,8 +162,10 @@ nlTestSuite sSuite = { "Test-CHIP-MessagingLayer", &sTests[0], - TestContext::Initialize, - TestContext::Finalize + TestContext::nlTestSetUpTestSuite, + TestContext::nlTestTearDownTestSuite, + TestContext::nlTestSetUp, + TestContext::nlTestTearDown, }; // clang-format on diff --git a/src/messaging/tests/TestReliableMessageProtocol.cpp b/src/messaging/tests/TestReliableMessageProtocol.cpp index 8c8443fd81d59e..db5dc144b0c2ae 100644 --- a/src/messaging/tests/TestReliableMessageProtocol.cpp +++ b/src/messaging/tests/TestReliableMessageProtocol.cpp @@ -58,8 +58,6 @@ using namespace chip::Messaging; using namespace chip::Protocols; using namespace chip::System::Clock::Literals; -using TestContext = Test::LoopbackMessagingContext; - const char PAYLOAD[] = "Hello!"; // The CHIP_CONFIG_MRP_RETRY_INTERVAL_SENDER_BOOST can be set to non-zero value @@ -70,6 +68,19 @@ const char PAYLOAD[] = "Hello!"; // CHIP_CONFIG_MRP_RETRY_INTERVAL_SENDER_BOOST for more details. constexpr auto retryBoosterTimeout = CHIP_CONFIG_RMP_DEFAULT_MAX_RETRANS * CHIP_CONFIG_MRP_RETRY_INTERVAL_SENDER_BOOST; +class TestContext : public chip::Test::LoopbackMessagingContext +{ +public: + // Performs setup for each individual test in the test suite + CHIP_ERROR SetUp() override + { + ReturnErrorOnFailure(chip::Test::LoopbackMessagingContext::SetUp()); + GetSessionAliceToBob()->AsSecureSession()->SetRemoteSessionParameters(GetLocalMRPConfig().ValueOr(GetDefaultMRPConfig())); + GetSessionBobToAlice()->AsSecureSession()->SetRemoteSessionParameters(GetLocalMRPConfig().ValueOr(GetDefaultMRPConfig())); + return CHIP_NO_ERROR; + } +}; + class MockAppDelegate : public UnsolicitedMessageHandler, public ExchangeDelegate { public: @@ -326,7 +337,6 @@ class TestReliableMessageProtocol static void CheckGetBackoff(nlTestSuite * inSuite, void * inContext); static void CheckApplicationResponseDelayed(nlTestSuite * inSuite, void * inContext); static void CheckApplicationResponseNeverComes(nlTestSuite * inSuite, void * inContext); - static int InitializeTestCase(void * inContext); }; void TestReliableMessageProtocol::CheckAddClearRetrans(nlTestSuite * inSuite, void * inContext) @@ -2134,14 +2144,6 @@ void TestReliableMessageProtocol::CheckApplicationResponseNeverComes(nlTestSuite NL_TEST_ASSERT(inSuite, err == CHIP_NO_ERROR); } -int TestReliableMessageProtocol::InitializeTestCase(void * inContext) -{ - TestContext & ctx = *static_cast(inContext); - ctx.GetSessionAliceToBob()->AsSecureSession()->SetRemoteSessionParameters(GetLocalMRPConfig().ValueOr(GetDefaultMRPConfig())); - ctx.GetSessionBobToAlice()->AsSecureSession()->SetRemoteSessionParameters(GetLocalMRPConfig().ValueOr(GetDefaultMRPConfig())); - return SUCCESS; -} - /** * TODO: A test that we should have but can't write with the existing * infrastructure we have: @@ -2203,9 +2205,10 @@ const nlTest sTests[] = { nlTestSuite sSuite = { "Test-CHIP-ReliableMessageProtocol", &sTests[0], - TestContext::Initialize, - TestContext::Finalize, - TestReliableMessageProtocol::InitializeTestCase, + TestContext::nlTestSetUpTestSuite, + TestContext::nlTestTearDownTestSuite, + TestContext::nlTestSetUp, + TestContext::nlTestTearDown, }; // clang-format on diff --git a/src/protocols/secure_channel/tests/TestCASESession.cpp b/src/protocols/secure_channel/tests/TestCASESession.cpp index b4e090d3eb6b93..e72da8af1cbad7 100644 --- a/src/protocols/secure_channel/tests/TestCASESession.cpp +++ b/src/protocols/secure_channel/tests/TestCASESession.cpp @@ -53,11 +53,18 @@ using namespace chip::Transport; using namespace chip::Messaging; using namespace chip::Protocols; -using TestContext = Test::LoopbackMessagingContext; - namespace chip { namespace { +class TestContext : public Test::LoopbackMessagingContext +{ +public: + // Performs shared setup for all tests in the test suite + CHIP_ERROR SetUpTestSuite() override; + // Performs shared teardown for all tests in the test suite + void TearDownTestSuite() override; +}; + void ServiceEvents(TestContext & ctx) { // Takes a few rounds of this because handling IO messages may schedule work, @@ -155,6 +162,11 @@ class TestOperationalKeystore : public chip::Crypto::OperationalKeystore mSingleFabricIndex = fabricIndex; mKeypair = std::move(keypair); } + void Shutdown() + { + mSingleFabricIndex = kUndefinedFabricIndex; + mKeypair = nullptr; + } bool HasPendingOpKeypair() const override { return false; } bool HasOpKeypairForFabric(FabricIndex fabricIndex) const override { return mSingleFabricIndex != kUndefinedFabricIndex; } @@ -215,6 +227,8 @@ Crypto::DefaultSessionKeystore gDeviceSessionKeystore; Credentials::PersistentStorageOpCertStore gCommissionerOpCertStore; Credentials::PersistentStorageOpCertStore gDeviceOpCertStore; +CASEServer gPairingServer; + NodeId Node01_01 = 0xDEDEDEDE00010001; NodeId Node01_02 = 0xDEDEDEDE00010002; @@ -312,6 +326,37 @@ CHIP_ERROR InitCredentialSets() return CHIP_NO_ERROR; } +CHIP_ERROR TestContext::SetUpTestSuite() +{ + ConfigInitializeNodes(false); + CHIP_ERROR err = CHIP_NO_ERROR; + VerifyOrExit((err = LoopbackMessagingContext::SetUpTestSuite()) == CHIP_NO_ERROR, + ChipLogError(AppServer, "SetUpTestSuite lo messaging context failed: %" CHIP_ERROR_FORMAT, err.Format())); + VerifyOrExit((err = chip::DeviceLayer::PlatformMgr().InitChipStack()) == CHIP_NO_ERROR, + ChipLogError(AppServer, "Init CHIP stack failed: %" CHIP_ERROR_FORMAT, err.Format())); + VerifyOrExit((err = InitFabricTable(gCommissionerFabrics, &gCommissionerStorageDelegate, /* opKeyStore = */ nullptr, + &gCommissionerOpCertStore)) == CHIP_NO_ERROR, + ChipLogError(AppServer, "InitFabricTable failed: %" CHIP_ERROR_FORMAT, err.Format())); + VerifyOrExit((err = InitCredentialSets()) == CHIP_NO_ERROR, + ChipLogError(AppServer, "InitCredentialSets failed: %" CHIP_ERROR_FORMAT, err.Format())); + chip::DeviceLayer::SetSystemLayerForTesting(&GetSystemLayer()); +exit: + return err; +} + +void TestContext::TearDownTestSuite() +{ + chip::DeviceLayer::SetSystemLayerForTesting(nullptr); + gDeviceOperationalKeystore.Shutdown(); + gPairingServer.Shutdown(); + gCommissionerStorageDelegate.ClearStorage(); + gDeviceStorageDelegate.ClearStorage(); + gCommissionerFabrics.DeleteAllFabrics(); + gDeviceFabrics.DeleteAllFabrics(); + chip::DeviceLayer::PlatformMgr().Shutdown(); + LoopbackMessagingContext::TearDownTestSuite(); +} + } // anonymous namespace // Specifically for SimulateUpdateNOCInvalidatePendingEstablishment, we need it to be static so that the class below can @@ -492,8 +537,6 @@ void TestCASESession::SecurePairingHandshakeTest(nlTestSuite * inSuite, void * i SecurePairingHandshakeTestCommon(inSuite, inContext, sessionManager, pairingCommissioner, delegateCommissioner); } -CASEServer gPairingServer; - void TestCASESession::SecurePairingHandshakeServerTest(nlTestSuite * inSuite, void * inContext) { // TODO: Add cases for mismatching IPK config between initiator/responder @@ -1193,76 +1236,18 @@ static const nlTest sTests[] = }; // clang-format on -int CASE_TestSecurePairing_Setup(void * inContext); -int CASE_TestSecurePairing_Teardown(void * inContext); - // clang-format off static nlTestSuite sSuite = { "Test-CHIP-SecurePairing-CASE", &sTests[0], - CASE_TestSecurePairing_Setup, - CASE_TestSecurePairing_Teardown, + TestContext::nlTestSetUpTestSuite, + TestContext::nlTestTearDownTestSuite, + TestContext::nlTestSetUp, + TestContext::nlTestTearDown, }; // clang-format on -namespace { -/* - * Set up the test suite. - */ -CHIP_ERROR CASETestSecurePairingSetup(void * inContext) -{ - TestContext & ctx = *reinterpret_cast(inContext); - - ctx.ConfigInitializeNodes(false); - ReturnErrorOnFailure(ctx.Init()); - - ReturnErrorOnFailure(InitFabricTable(gCommissionerFabrics, &gCommissionerStorageDelegate, /* opKeyStore = */ nullptr, - &gCommissionerOpCertStore)); - - return InitCredentialSets(); -} -} // anonymous namespace - -/** - * Set up the test suite. - */ -int CASE_TestSecurePairing_Setup(void * inContext) -{ - chip::Platform::MemoryInit(); - - chip::DeviceLayer::PlatformMgr().InitChipStack(); - - CHIP_ERROR err = CASETestSecurePairingSetup(inContext); - if (err != CHIP_NO_ERROR) - { - ChipLogError(Support, "Failed to init tests %" CHIP_ERROR_FORMAT, err.Format()); - return FAILURE; - } - - TestContext & ctx = *reinterpret_cast(inContext); - chip::DeviceLayer::SetSystemLayerForTesting(&ctx.GetSystemLayer()); - - return SUCCESS; -} - -/** - * Tear down the test suite. - */ -int CASE_TestSecurePairing_Teardown(void * inContext) -{ - chip::DeviceLayer::SetSystemLayerForTesting(nullptr); - - gPairingServer.Shutdown(); - gCommissionerStorageDelegate.ClearStorage(); - gDeviceStorageDelegate.ClearStorage(); - gCommissionerFabrics.DeleteAllFabrics(); - gDeviceFabrics.DeleteAllFabrics(); - static_cast(inContext)->Shutdown(); - chip::DeviceLayer::PlatformMgr().Shutdown(); - return SUCCESS; -} - /** * Main */ diff --git a/src/protocols/secure_channel/tests/TestPASESession.cpp b/src/protocols/secure_channel/tests/TestPASESession.cpp index c7c350fc3c80df..01e987d425446b 100644 --- a/src/protocols/secure_channel/tests/TestPASESession.cpp +++ b/src/protocols/secure_channel/tests/TestPASESession.cpp @@ -83,6 +83,17 @@ constexpr Spake2pVerifierSerialized sTestSpake2p01_SerializedVerifier = { 0xB7, 0xC0, 0x7F, 0xCC, 0x06, 0x27, 0xA1, 0xB8, 0x57, 0x3A, 0x14, 0x9F, 0xCD, 0x1F, 0xA4, 0x66, 0xCF }; +class TestContext : public chip::Test::LoopbackMessagingContext +{ +public: + // Performs shared setup for all tests in the test suite + CHIP_ERROR SetUpTestSuite() override + { + ConfigInitializeNodes(false); + return chip::Test::LoopbackMessagingContext::SetUpTestSuite(); + } +}; + class PASETestLoopbackTransportDelegate : public Test::LoopbackTransportDelegate { public: @@ -90,8 +101,6 @@ class PASETestLoopbackTransportDelegate : public Test::LoopbackTransportDelegate bool mMessageDropped = false; }; -using TestContext = chip::Test::LoopbackMessagingContext; - class TestSecurePairingDelegate : public SessionEstablishmentDelegate { public: @@ -509,62 +518,31 @@ void PASEVerifierSerializeTest(nlTestSuite * inSuite, void * inContext) // Test Suite -/** - * Test Suite that lists all the test functions. - */ -// clang-format off -static const nlTest sTests[] = -{ - NL_TEST_DEF("WaitInit", SecurePairingWaitTest), - NL_TEST_DEF("Start", SecurePairingStartTest), - NL_TEST_DEF("Handshake", SecurePairingHandshakeTest), +static const nlTest sTests[] = { + NL_TEST_DEF("WaitInit", SecurePairingWaitTest), + NL_TEST_DEF("Start", SecurePairingStartTest), + NL_TEST_DEF("Handshake", SecurePairingHandshakeTest), NL_TEST_DEF("Handshake with Commissioner MRP Parameters", SecurePairingHandshakeWithCommissionerMRPTest), NL_TEST_DEF("Handshake with Device MRP Parameters", SecurePairingHandshakeWithDeviceMRPTest), NL_TEST_DEF("Handshake with Both MRP Parameters", SecurePairingHandshakeWithAllMRPTest), NL_TEST_DEF("Handshake with packet loss", SecurePairingHandshakeWithPacketLossTest), NL_TEST_DEF("Failed Handshake", SecurePairingFailedHandshake), NL_TEST_DEF("PASE Verifier Serialize", PASEVerifierSerializeTest), - - NL_TEST_SENTINEL() + NL_TEST_SENTINEL(), }; -int TestSecurePairing_Setup(void * inContext); -int TestSecurePairing_Teardown(void * inContext); - // clang-format off static nlTestSuite sSuite = { "Test-CHIP-SecurePairing-PASE", &sTests[0], - TestSecurePairing_Setup, - TestSecurePairing_Teardown, + TestContext::nlTestSetUpTestSuite, + TestContext::nlTestTearDownTestSuite, + TestContext::nlTestSetUp, + TestContext::nlTestTearDown, }; // clang-format on -// clang-format on -// -/** - * Set up the test suite. - */ -int TestSecurePairing_Setup(void * inContext) -{ - auto & ctx = *static_cast(inContext); - - // Initialize System memory and resources - ctx.ConfigInitializeNodes(false); - VerifyOrReturnError(TestContext::Initialize(inContext) == SUCCESS, FAILURE); - - return SUCCESS; -} - -/** - * Tear down the test suite. - */ -int TestSecurePairing_Teardown(void * inContext) -{ - return TestContext::Finalize(inContext); -} - } // anonymous namespace /** From ba8ab24e575c4872ebddfd794a1a486f8f3f5d18 Mon Sep 17 00:00:00 2001 From: mkardous-silabs <84793247+mkardous-silabs@users.noreply.github.com> Date: Fri, 8 Dec 2023 16:27:55 -0500 Subject: [PATCH 3/3] [ICD] Refactor ICD Monitoring Table to support HMAC handle and AES key handle (#30881) * Rename key to keyHandle * Refactor ICD Monitoring Table to not assume key type Add Hmac Key Handle to ICD Monitoring Table * remove utility functions * replace helpers with memcpy * Add test validation for Hmac key handle * Apply suggestions from code review Co-authored-by: Boris Zbarsky * Clarify comments --------- Co-authored-by: Boris Zbarsky --- src/app/icd/ICDCheckInSender.cpp | 6 +- src/app/icd/ICDCheckInSender.h | 2 +- src/app/icd/ICDMonitoringTable.cpp | 84 ++++++++++++++++++------ src/app/icd/ICDMonitoringTable.h | 5 +- src/app/tests/TestICDMonitoringTable.cpp | 52 ++++++++++++++- src/crypto/SessionKeystore.h | 28 +++++--- 6 files changed, 143 insertions(+), 34 deletions(-) diff --git a/src/app/icd/ICDCheckInSender.cpp b/src/app/icd/ICDCheckInSender.cpp index 173a1df50ac037..ea827c2c7ac6c1 100644 --- a/src/app/icd/ICDCheckInSender.cpp +++ b/src/app/icd/ICDCheckInSender.cpp @@ -61,7 +61,7 @@ CHIP_ERROR ICDCheckInSender::SendCheckInMsg(const Transport::PeerAddress & addr) VerifyOrReturnError(!buffer.IsNull(), CHIP_ERROR_NO_MEMORY); MutableByteSpan output{ buffer->Start(), buffer->MaxDataLength() }; - ReturnErrorOnFailure(CheckinMessage::GenerateCheckinMessagePayload(mKey, mICDCounter, ByteSpan(), output)); + ReturnErrorOnFailure(CheckinMessage::GenerateCheckinMessagePayload(mAesKeyHandle, mICDCounter, ByteSpan(), output)); buffer->SetDataLength(static_cast(output.size())); VerifyOrReturnError(mExchangeManager->GetSessionManager() != nullptr, CHIP_ERROR_INTERNAL); @@ -89,8 +89,8 @@ CHIP_ERROR ICDCheckInSender::RequestResolve(ICDMonitoringEntry & entry, FabricTa AddressResolve::NodeLookupRequest request(peerId); - memcpy(mKey.AsMutable(), entry.key.As(), - sizeof(Crypto::Symmetric128BitsKeyByteArray)); + memcpy(mAesKeyHandle.AsMutable(), + entry.aesKeyHandle.As(), sizeof(Crypto::Symmetric128BitsKeyByteArray)); CHIP_ERROR err = AddressResolve::Resolver::Instance().LookupNode(request, mAddressLookupHandle); diff --git a/src/app/icd/ICDCheckInSender.h b/src/app/icd/ICDCheckInSender.h index 0055d66804d0fb..b6e2dfa2212f92 100644 --- a/src/app/icd/ICDCheckInSender.h +++ b/src/app/icd/ICDCheckInSender.h @@ -50,7 +50,7 @@ class ICDCheckInSender : public AddressResolve::NodeListener Messaging::ExchangeManager * mExchangeManager = nullptr; - Crypto::Aes128KeyHandle mKey = Crypto::Aes128KeyHandle(); + Crypto::Aes128KeyHandle mAesKeyHandle = Crypto::Aes128KeyHandle(); uint32_t mICDCounter = 0; }; diff --git a/src/app/icd/ICDMonitoringTable.cpp b/src/app/icd/ICDMonitoringTable.cpp index 83a06c4f35ad80..57e6fa265821f1 100644 --- a/src/app/icd/ICDMonitoringTable.cpp +++ b/src/app/icd/ICDMonitoringTable.cpp @@ -25,7 +25,8 @@ enum class Fields : uint8_t { kCheckInNodeID = 1, kMonitoredSubject = 2, - kKey = 3, + kAesKeyHandle = 3, + kHmacKeyHandle = 4, }; CHIP_ERROR ICDMonitoringEntry::UpdateKey(StorageKeyName & skey) @@ -42,8 +43,12 @@ CHIP_ERROR ICDMonitoringEntry::Serialize(TLV::TLVWriter & writer) const ReturnErrorOnFailure(writer.Put(TLV::ContextTag(Fields::kCheckInNodeID), checkInNodeID)); ReturnErrorOnFailure(writer.Put(TLV::ContextTag(Fields::kMonitoredSubject), monitoredSubject)); - ByteSpan buf(key.As()); - ReturnErrorOnFailure(writer.Put(TLV::ContextTag(Fields::kKey), buf)); + ByteSpan aesKeybuf(aesKeyHandle.As()); + ReturnErrorOnFailure(writer.Put(TLV::ContextTag(Fields::kAesKeyHandle), aesKeybuf)); + + ByteSpan hmacKeybuf(hmacKeyHandle.As()); + ReturnErrorOnFailure(writer.Put(TLV::ContextTag(Fields::kHmacKeyHandle), hmacKeybuf)); + ReturnErrorOnFailure(writer.EndContainer(outer)); return CHIP_NO_ERROR; } @@ -69,18 +74,38 @@ CHIP_ERROR ICDMonitoringEntry::Deserialize(TLV::TLVReader & reader) case to_underlying(Fields::kMonitoredSubject): ReturnErrorOnFailure(reader.Get(monitoredSubject)); break; - case to_underlying(Fields::kKey): { - ByteSpan buf(key.AsMutable()); + case to_underlying(Fields::kAesKeyHandle): { + ByteSpan buf; ReturnErrorOnFailure(reader.Get(buf)); // Since we are storing either the raw key or a key ID, we must // simply copy the data as is in the keyHandle. - // Calling SetKey here would create another key in storage and will cause - // key leakage in some implementation. - memcpy(key.AsMutable(), buf.data(), + // Calling SetKey here would create another keyHandle in storage and will cause + // key leaks in some implementations. + memcpy(aesKeyHandle.AsMutable(), buf.data(), sizeof(Crypto::Symmetric128BitsKeyByteArray)); keyHandleValid = true; } break; + case to_underlying(Fields::kHmacKeyHandle): { + ByteSpan buf; + CHIP_ERROR error = reader.Get(buf); + + if (error != CHIP_NO_ERROR) + { + // If retreiving the hmac key handle failed, we need to set an invalid key handle + // even if the AesKeyHandle is valid. + keyHandleValid = false; + return error; + } + + // Since we are storing either the raw key or a key ID, we must + // simply copy the data as is in the keyHandle. + // Calling SetKey here would create another keyHandle in storage and will cause + // key leaks in some implementations. + memcpy(hmacKeyHandle.AsMutable(), buf.data(), + sizeof(Crypto::Symmetric128BitsKeyByteArray)); + } + break; default: break; } @@ -108,16 +133,29 @@ CHIP_ERROR ICDMonitoringEntry::SetKey(ByteSpan keyData) Crypto::Symmetric128BitsKeyByteArray keyMaterial; memcpy(keyMaterial, keyData.data(), sizeof(Crypto::Symmetric128BitsKeyByteArray)); - ReturnErrorOnFailure(symmetricKeystore->CreateKey(keyMaterial, key)); - keyHandleValid = true; + // TODO - Add function to set PSA key lifetime + ReturnErrorOnFailure(symmetricKeystore->CreateKey(keyMaterial, aesKeyHandle)); + CHIP_ERROR error = symmetricKeystore->CreateKey(keyMaterial, hmacKeyHandle); - return CHIP_NO_ERROR; + if (error == CHIP_NO_ERROR) + { + // If the creation of the HmacKeyHandle succeeds, both key handles are valid + keyHandleValid = true; + } + else + { + // Creation of the HmacKeyHandle failed, we need to delete the AesKeyHandle to avoid a key leak + symmetricKeystore->DestroyKey(this->aesKeyHandle); + } + + return error; } CHIP_ERROR ICDMonitoringEntry::DeleteKey() { VerifyOrReturnError(symmetricKeystore != nullptr, CHIP_ERROR_INTERNAL); - symmetricKeystore->DestroyKey(this->key); + symmetricKeystore->DestroyKey(this->aesKeyHandle); + symmetricKeystore->DestroyKey(this->hmacKeyHandle); keyHandleValid = false; return CHIP_NO_ERROR; } @@ -141,7 +179,7 @@ bool ICDMonitoringEntry::IsKeyEquivalent(ByteSpan keyData) uint64_t data = Crypto::GetRandU64(), validation, encrypted; validation = data; - err = Crypto::AES_CCM_encrypt(reinterpret_cast(&data), sizeof(data), nullptr, 0, tempEntry.key, aead, + err = Crypto::AES_CCM_encrypt(reinterpret_cast(&data), sizeof(data), nullptr, 0, tempEntry.aesKeyHandle, aead, Crypto::CHIP_CRYPTO_AEAD_NONCE_LENGTH_BYTES, reinterpret_cast(&encrypted), mic, Crypto::CHIP_CRYPTO_AEAD_MIC_LENGTH_BYTES); @@ -149,7 +187,7 @@ bool ICDMonitoringEntry::IsKeyEquivalent(ByteSpan keyData) if (err == CHIP_NO_ERROR) { err = Crypto::AES_CCM_decrypt(reinterpret_cast(&encrypted), sizeof(encrypted), nullptr, 0, mic, - Crypto::CHIP_CRYPTO_AEAD_MIC_LENGTH_BYTES, key, aead, + Crypto::CHIP_CRYPTO_AEAD_MIC_LENGTH_BYTES, aesKeyHandle, aead, Crypto::CHIP_CRYPTO_AEAD_NONCE_LENGTH_BYTES, reinterpret_cast(&data)); } tempEntry.DeleteKey(); @@ -175,7 +213,11 @@ ICDMonitoringEntry & ICDMonitoringEntry::operator=(const ICDMonitoringEntry & ic index = icdMonitoringEntry.index; keyHandleValid = icdMonitoringEntry.keyHandleValid; symmetricKeystore = icdMonitoringEntry.symmetricKeystore; - memcpy(key.AsMutable(), icdMonitoringEntry.key.As(), + memcpy(aesKeyHandle.AsMutable(), + icdMonitoringEntry.aesKeyHandle.As(), + sizeof(Crypto::Symmetric128BitsKeyByteArray)); + memcpy(hmacKeyHandle.AsMutable(), + icdMonitoringEntry.hmacKeyHandle.As(), sizeof(Crypto::Symmetric128BitsKeyByteArray)); return *this; @@ -211,12 +253,16 @@ CHIP_ERROR ICDMonitoringTable::Set(uint16_t index, const ICDMonitoringEntry & en VerifyOrReturnError(kUndefinedNodeId != entry.checkInNodeID, CHIP_ERROR_INVALID_ARGUMENT); VerifyOrReturnError(kUndefinedNodeId != entry.monitoredSubject, CHIP_ERROR_INVALID_ARGUMENT); VerifyOrReturnError(entry.keyHandleValid, CHIP_ERROR_INVALID_ARGUMENT); + ICDMonitoringEntry e(this->mFabric, index); e.checkInNodeID = entry.checkInNodeID; e.monitoredSubject = entry.monitoredSubject; e.index = index; - memcpy(e.key.AsMutable(), entry.key.As(), - sizeof(Crypto::Symmetric128BitsKeyByteArray)); + + memcpy(e.aesKeyHandle.AsMutable(), + entry.aesKeyHandle.As(), sizeof(Crypto::Symmetric128BitsKeyByteArray)); + memcpy(e.hmacKeyHandle.AsMutable(), + entry.hmacKeyHandle.As(), sizeof(Crypto::Symmetric128BitsKeyByteArray)); return e.Save(this->mStorage); } @@ -225,8 +271,8 @@ CHIP_ERROR ICDMonitoringTable::Remove(uint16_t index) { ICDMonitoringEntry entry(mSymmetricKeystore, this->mFabric); - // Retrieve entry and delete the key first as to not - // cause any key leakage. + // Retrieve entry and delete the keyHandle first as to not + // cause any key leaks. this->Get(index, entry); ReturnErrorOnFailure(entry.DeleteKey()); diff --git a/src/app/icd/ICDMonitoringTable.h b/src/app/icd/ICDMonitoringTable.h index c97703b226aa75..e996d0c757cc23 100644 --- a/src/app/icd/ICDMonitoringTable.h +++ b/src/app/icd/ICDMonitoringTable.h @@ -33,7 +33,7 @@ using SymmetricKeystore = SessionKeystore; namespace chip { -inline constexpr size_t kICDMonitoringBufferSize = 40; +inline constexpr size_t kICDMonitoringBufferSize = 60; struct ICDMonitoringEntry : public PersistentData { @@ -104,7 +104,8 @@ struct ICDMonitoringEntry : public PersistentData chip::FabricIndex fabricIndex = kUndefinedFabricIndex; chip::NodeId checkInNodeID = kUndefinedNodeId; uint64_t monitoredSubject = static_cast(0); - Crypto::Aes128KeyHandle key = Crypto::Aes128KeyHandle(); + Crypto::Aes128KeyHandle aesKeyHandle = Crypto::Aes128KeyHandle(); + Crypto::Hmac128KeyHandle hmacKeyHandle = Crypto::Hmac128KeyHandle(); bool keyHandleValid = false; uint16_t index = 0; Crypto::SymmetricKeystore * symmetricKeystore = nullptr; diff --git a/src/app/tests/TestICDMonitoringTable.cpp b/src/app/tests/TestICDMonitoringTable.cpp index 170ece20e1778d..1a6460e31640e3 100644 --- a/src/app/tests/TestICDMonitoringTable.cpp +++ b/src/app/tests/TestICDMonitoringTable.cpp @@ -155,11 +155,16 @@ void TestSaveAndLoadRegistrationValue(nlTestSuite * aSuite, void * aContext) // Retrieve first entry err = loading.Get(0, entry); + NL_TEST_ASSERT(aSuite, CHIP_NO_ERROR == err); NL_TEST_ASSERT(aSuite, kTestFabricIndex1 == entry.fabricIndex); NL_TEST_ASSERT(aSuite, kClientNodeId11 == entry.checkInNodeID); NL_TEST_ASSERT(aSuite, kClientNodeId12 == entry.monitoredSubject); NL_TEST_ASSERT(aSuite, entry.IsKeyEquivalent(ByteSpan(kKeyBuffer1a))); + NL_TEST_ASSERT(aSuite, + memcmp(entry1.hmacKeyHandle.As(), + entry.hmacKeyHandle.As(), + sizeof(Crypto::Symmetric128BitsKeyByteArray)) == 0); // Retrieve second entry err = loading.Get(1, entry); @@ -168,6 +173,10 @@ void TestSaveAndLoadRegistrationValue(nlTestSuite * aSuite, void * aContext) NL_TEST_ASSERT(aSuite, kClientNodeId12 == entry.checkInNodeID); NL_TEST_ASSERT(aSuite, kClientNodeId11 == entry.monitoredSubject); NL_TEST_ASSERT(aSuite, entry.IsKeyEquivalent(ByteSpan(kKeyBuffer2a))); + NL_TEST_ASSERT(aSuite, + memcmp(entry2.hmacKeyHandle.As(), + entry.hmacKeyHandle.As(), + sizeof(Crypto::Symmetric128BitsKeyByteArray)) == 0); // No more entries err = loading.Get(2, entry); @@ -176,6 +185,7 @@ void TestSaveAndLoadRegistrationValue(nlTestSuite * aSuite, void * aContext) // Remove first entry saving.Remove(0); + ICDMonitoringEntry entry4(&keystore); entry4.checkInNodeID = kClientNodeId13; entry4.monitoredSubject = kClientNodeId11; @@ -190,6 +200,10 @@ void TestSaveAndLoadRegistrationValue(nlTestSuite * aSuite, void * aContext) NL_TEST_ASSERT(aSuite, kClientNodeId12 == entry.checkInNodeID); NL_TEST_ASSERT(aSuite, kClientNodeId11 == entry.monitoredSubject); NL_TEST_ASSERT(aSuite, entry.IsKeyEquivalent(ByteSpan(kKeyBuffer2a))); + NL_TEST_ASSERT(aSuite, + memcmp(entry2.hmacKeyHandle.As(), + entry.hmacKeyHandle.As(), + sizeof(Crypto::Symmetric128BitsKeyByteArray)) == 0); // Retrieve second entry err = loading.Get(1, entry); @@ -198,6 +212,10 @@ void TestSaveAndLoadRegistrationValue(nlTestSuite * aSuite, void * aContext) NL_TEST_ASSERT(aSuite, kClientNodeId13 == entry.checkInNodeID); NL_TEST_ASSERT(aSuite, kClientNodeId11 == entry.monitoredSubject); NL_TEST_ASSERT(aSuite, entry.IsKeyEquivalent(ByteSpan(kKeyBuffer1b))); + NL_TEST_ASSERT(aSuite, + memcmp(entry4.hmacKeyHandle.As(), + entry.hmacKeyHandle.As(), + sizeof(Crypto::Symmetric128BitsKeyByteArray)) == 0); } void TestSaveAllInvalidRegistrationValues(nlTestSuite * aSuite, void * aContext) @@ -296,14 +314,22 @@ void TestSaveLoadRegistrationValueForMultipleFabrics(nlTestSuite * aSuite, void NL_TEST_ASSERT(aSuite, kClientNodeId11 == entry.checkInNodeID); NL_TEST_ASSERT(aSuite, kClientNodeId12 == entry.monitoredSubject); NL_TEST_ASSERT(aSuite, entry.IsKeyEquivalent(ByteSpan(kKeyBuffer1a))); + NL_TEST_ASSERT(aSuite, + memcmp(entry1.hmacKeyHandle.As(), + entry.hmacKeyHandle.As(), + sizeof(Crypto::Symmetric128BitsKeyByteArray)) == 0); - // Retrieve fabric2, second entry + // Retrieve fabric1, second entry err = table1.Get(1, entry); NL_TEST_ASSERT(aSuite, CHIP_NO_ERROR == err); NL_TEST_ASSERT(aSuite, kTestFabricIndex1 == entry.fabricIndex); NL_TEST_ASSERT(aSuite, kClientNodeId12 == entry.checkInNodeID); NL_TEST_ASSERT(aSuite, kClientNodeId11 == entry.monitoredSubject); NL_TEST_ASSERT(aSuite, entry.IsKeyEquivalent(ByteSpan(kKeyBuffer1b))); + NL_TEST_ASSERT(aSuite, + memcmp(entry2.hmacKeyHandle.As(), + entry.hmacKeyHandle.As(), + sizeof(Crypto::Symmetric128BitsKeyByteArray)) == 0); // Retrieve fabric2, first entry err = table2.Get(0, entry); @@ -312,6 +338,10 @@ void TestSaveLoadRegistrationValueForMultipleFabrics(nlTestSuite * aSuite, void NL_TEST_ASSERT(aSuite, kClientNodeId21 == entry.checkInNodeID); NL_TEST_ASSERT(aSuite, kClientNodeId22 == entry.monitoredSubject); NL_TEST_ASSERT(aSuite, entry.IsKeyEquivalent(ByteSpan(kKeyBuffer2a))); + NL_TEST_ASSERT(aSuite, + memcmp(entry3.hmacKeyHandle.As(), + entry.hmacKeyHandle.As(), + sizeof(Crypto::Symmetric128BitsKeyByteArray)) == 0); } void TestDeleteValidEntryFromStorage(nlTestSuite * aSuite, void * context) @@ -358,6 +388,10 @@ void TestDeleteValidEntryFromStorage(nlTestSuite * aSuite, void * context) NL_TEST_ASSERT(aSuite, kClientNodeId11 == entry.checkInNodeID); NL_TEST_ASSERT(aSuite, kClientNodeId12 == entry.monitoredSubject); NL_TEST_ASSERT(aSuite, entry.IsKeyEquivalent(ByteSpan(kKeyBuffer1a))); + NL_TEST_ASSERT(aSuite, + memcmp(entry1.hmacKeyHandle.As(), + entry.hmacKeyHandle.As(), + sizeof(Crypto::Symmetric128BitsKeyByteArray)) == 0); // Retrieve second entry (not modified) err = table1.Get(1, entry); @@ -366,6 +400,10 @@ void TestDeleteValidEntryFromStorage(nlTestSuite * aSuite, void * context) NL_TEST_ASSERT(aSuite, kClientNodeId12 == entry.checkInNodeID); NL_TEST_ASSERT(aSuite, kClientNodeId11 == entry.monitoredSubject); NL_TEST_ASSERT(aSuite, entry.IsKeyEquivalent(ByteSpan(kKeyBuffer2a))); + NL_TEST_ASSERT(aSuite, + memcmp(entry2.hmacKeyHandle.As(), + entry.hmacKeyHandle.As(), + sizeof(Crypto::Symmetric128BitsKeyByteArray)) == 0); // Remove (existing) err = table1.Remove(0); @@ -381,6 +419,10 @@ void TestDeleteValidEntryFromStorage(nlTestSuite * aSuite, void * context) NL_TEST_ASSERT(aSuite, kClientNodeId12 == entry.checkInNodeID); NL_TEST_ASSERT(aSuite, kClientNodeId11 == entry.monitoredSubject); NL_TEST_ASSERT(aSuite, entry.IsKeyEquivalent(ByteSpan(kKeyBuffer2a))); + NL_TEST_ASSERT(aSuite, + memcmp(entry2.hmacKeyHandle.As(), + entry.hmacKeyHandle.As(), + sizeof(Crypto::Symmetric128BitsKeyByteArray)) == 0); // Retrieve fabric2, first entry err = table2.Get(0, entry); @@ -389,6 +431,10 @@ void TestDeleteValidEntryFromStorage(nlTestSuite * aSuite, void * context) NL_TEST_ASSERT(aSuite, kClientNodeId21 == entry.checkInNodeID); NL_TEST_ASSERT(aSuite, kClientNodeId22 == entry.monitoredSubject); NL_TEST_ASSERT(aSuite, entry.IsKeyEquivalent(ByteSpan(kKeyBuffer1b))); + NL_TEST_ASSERT(aSuite, + memcmp(entry3.hmacKeyHandle.As(), + entry.hmacKeyHandle.As(), + sizeof(Crypto::Symmetric128BitsKeyByteArray)) == 0); // Remove all (fabric 1) err = table1.RemoveAll(); @@ -404,6 +450,10 @@ void TestDeleteValidEntryFromStorage(nlTestSuite * aSuite, void * context) NL_TEST_ASSERT(aSuite, kClientNodeId21 == entry.checkInNodeID); NL_TEST_ASSERT(aSuite, kClientNodeId22 == entry.monitoredSubject); NL_TEST_ASSERT(aSuite, entry.IsKeyEquivalent(ByteSpan(kKeyBuffer1b))); + NL_TEST_ASSERT(aSuite, + memcmp(entry3.hmacKeyHandle.As(), + entry.hmacKeyHandle.As(), + sizeof(Crypto::Symmetric128BitsKeyByteArray)) == 0); // Remove all (fabric 2) err = table2.RemoveAll(); diff --git a/src/crypto/SessionKeystore.h b/src/crypto/SessionKeystore.h index dbf0ea3996bfe2..b5d1a26e30e6a9 100644 --- a/src/crypto/SessionKeystore.h +++ b/src/crypto/SessionKeystore.h @@ -29,12 +29,20 @@ namespace Crypto { * The session keystore interface provides an abstraction that allows the application to store * session keys in a secure environment. It uses the concept of key handles that isolate the * application from the actual key material. + * + * @note Refactor has begun to refactor this API into two disctinct APIs : SymmetrycKeyStore & SessionKeyDerivation + * Work has not been completed so the SessionKeystore has APIs that shouldn't go together for the time being + * The SessionKeystore APIs are split into two sections, one for each futur API. */ class SessionKeystore { public: virtual ~SessionKeystore() {} + /**************************** + * SymmetricKeyStore APIs + *****************************/ + /** * @brief Import raw key material and return a key handle for a key that be used to do AES 128 encryption. * @@ -59,6 +67,18 @@ class SessionKeystore */ virtual CHIP_ERROR CreateKey(const Symmetric128BitsKeyByteArray & keyMaterial, Hmac128KeyHandle & key) = 0; + /** + * @brief Destroy key. + * + * The method can take an uninitialized handle in which case it is a no-op. + * As a result of calling this method, the handle is put in the uninitialized state. + */ + virtual void DestroyKey(Symmetric128BitsKeyHandle & key) = 0; + + /**************************** + * SessionKeyDerivation APIs + *****************************/ + /** * @brief Derive key from a shared secret. * @@ -83,14 +103,6 @@ class SessionKeystore virtual CHIP_ERROR DeriveSessionKeys(const ByteSpan & secret, const ByteSpan & salt, const ByteSpan & info, Aes128KeyHandle & i2rKey, Aes128KeyHandle & r2iKey, AttestationChallenge & attestationChallenge) = 0; - - /** - * @brief Destroy key. - * - * The method can take an uninitialized handle in which case it is a no-op. - * As a result of calling this method, the handle is put in the uninitialized state. - */ - virtual void DestroyKey(Symmetric128BitsKeyHandle & key) = 0; }; /**