From e9510a243f84e3b96a5985974953a092343d1d32 Mon Sep 17 00:00:00 2001 From: Matt Swartwout Date: Thu, 4 Apr 2024 09:54:09 -0700 Subject: [PATCH 01/10] Fix initialization-order-fiasco --- src/app/server/Server.cpp | 2 -- src/app/server/Server.h | 7 ++++--- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/src/app/server/Server.cpp b/src/app/server/Server.cpp index d2983636450691..48f92e425c6599 100644 --- a/src/app/server/Server.cpp +++ b/src/app/server/Server.cpp @@ -87,8 +87,6 @@ class DeviceTypeResolver : public chip::Access::AccessControl::DeviceTypeResolve namespace chip { -Server Server::sServer; - #if CHIP_CONFIG_ENABLE_SERVER_IM_EVENT #define CHIP_NUM_EVENT_LOGGING_BUFFERS 3 static uint8_t sInfoEventBuffer[CHIP_DEVICE_CONFIG_EVENT_LOGGING_INFO_BUFFER_SIZE]; diff --git a/src/app/server/Server.h b/src/app/server/Server.h index 302471b8bebb89..a1d8550d095e5b 100644 --- a/src/app/server/Server.h +++ b/src/app/server/Server.h @@ -386,13 +386,14 @@ class Server return System::SystemClock().GetMonotonicMicroseconds64() - mInitTimestamp; } - static Server & GetInstance() { return sServer; } + static Server & GetInstance() { + static Server sServer = new Server(); + return *sServer; + } private: Server() {} - static Server sServer; - void InitFailSafe(); void OnPlatformEvent(const DeviceLayer::ChipDeviceEvent & event); void CheckServerReadyEvent(); From d73e6e1ca1b4798924a9131e661426346e2d5020 Mon Sep 17 00:00:00 2001 From: "Restyled.io" Date: Thu, 4 Apr 2024 17:09:37 +0000 Subject: [PATCH 02/10] Restyled by clang-format --- src/app/server/Server.h | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/app/server/Server.h b/src/app/server/Server.h index a1d8550d095e5b..504901fe950b2a 100644 --- a/src/app/server/Server.h +++ b/src/app/server/Server.h @@ -386,9 +386,10 @@ class Server return System::SystemClock().GetMonotonicMicroseconds64() - mInitTimestamp; } - static Server & GetInstance() { - static Server sServer = new Server(); - return *sServer; + static Server & GetInstance() + { + static Server sServer = new Server(); + return *sServer; } private: From d5282f6b0634150f32ee990a5e3b4c0eb6f1470e Mon Sep 17 00:00:00 2001 From: Matt Swartwout Date: Thu, 4 Apr 2024 10:24:05 -0700 Subject: [PATCH 03/10] Fix pointer declaration --- src/app/server/Server.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/app/server/Server.h b/src/app/server/Server.h index 504901fe950b2a..e758099ec2f7d6 100644 --- a/src/app/server/Server.h +++ b/src/app/server/Server.h @@ -388,7 +388,7 @@ class Server static Server & GetInstance() { - static Server sServer = new Server(); + static Server* sServer = new Server(); return *sServer; } From 7b9c3a7f14f6e91830df0bf25944219036d18bc1 Mon Sep 17 00:00:00 2001 From: "Restyled.io" Date: Thu, 4 Apr 2024 17:24:37 +0000 Subject: [PATCH 04/10] Restyled by clang-format --- src/app/server/Server.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/app/server/Server.h b/src/app/server/Server.h index e758099ec2f7d6..861344e686ba61 100644 --- a/src/app/server/Server.h +++ b/src/app/server/Server.h @@ -388,7 +388,7 @@ class Server static Server & GetInstance() { - static Server* sServer = new Server(); + static Server * sServer = new Server(); return *sServer; } From 9011e3e48011474353327062cf1cd2d05052644e Mon Sep 17 00:00:00 2001 From: Matt Swartwout Date: Wed, 17 Apr 2024 23:27:47 +0000 Subject: [PATCH 05/10] Do not set mLocalMRPConfig in PairingSession constructor --- src/protocols/secure_channel/CASESession.cpp | 9 ++++++--- src/protocols/secure_channel/PASESession.cpp | 6 ++++-- src/protocols/secure_channel/PairingSession.h | 3 ++- 3 files changed, 12 insertions(+), 6 deletions(-) diff --git a/src/protocols/secure_channel/CASESession.cpp b/src/protocols/secure_channel/CASESession.cpp index 4436d7fbb77881..5de6778c49fde2 100644 --- a/src/protocols/secure_channel/CASESession.cpp +++ b/src/protocols/secure_channel/CASESession.cpp @@ -708,7 +708,8 @@ CHIP_ERROR CASESession::SendSigma1() ReturnErrorOnFailure( tlvWriter.PutBytes(TLV::ContextTag(4), mEphemeralKey->Pubkey(), static_cast(mEphemeralKey->Pubkey().Length()))); - ReturnErrorOnFailure(EncodeSessionParameters(TLV::ContextTag(5), mLocalMRPConfig, tlvWriter)); + VerifyOrReturnError(mLocalMRPConfig.HasValue(), CHIP_ERROR_INCORRECT_STATE); + ReturnErrorOnFailure(EncodeSessionParameters(TLV::ContextTag(5), mLocalMRPConfig.Value(), tlvWriter)); // Try to find persistent session, and resume it. bool resuming = false; @@ -955,7 +956,8 @@ CHIP_ERROR CASESession::SendSigma2Resume() ReturnErrorOnFailure(tlvWriter.Put(TLV::ContextTag(3), GetLocalSessionId().Value())); - ReturnErrorOnFailure(EncodeSessionParameters(TLV::ContextTag(4), mLocalMRPConfig, tlvWriter)); + VerifyOrReturnError(mLocalMRPConfig.HasValue(), CHIP_ERROR_INCORRECT_STATE); + ReturnErrorOnFailure(EncodeSessionParameters(TLV::ContextTag(4), mLocalMRPConfig.Value(), tlvWriter)); ReturnErrorOnFailure(tlvWriter.EndContainer(outerContainerType)); ReturnErrorOnFailure(tlvWriter.Finalize(&msg_R2_resume)); @@ -1091,7 +1093,8 @@ CHIP_ERROR CASESession::SendSigma2() ReturnErrorOnFailure(tlvWriterMsg2.PutBytes(TLV::ContextTag(4), msg_R2_Encrypted.Get(), static_cast(msg_r2_signed_enc_len + CHIP_CRYPTO_AEAD_MIC_LENGTH_BYTES))); - ReturnErrorOnFailure(EncodeSessionParameters(TLV::ContextTag(5), mLocalMRPConfig, tlvWriterMsg2)); + VerifyOrReturnError(mLocalMRPConfig.HasValue(), CHIP_ERROR_INCORRECT_STATE); + ReturnErrorOnFailure(EncodeSessionParameters(TLV::ContextTag(5), mLocalMRPConfig.Value(), tlvWriterMsg2)); ReturnErrorOnFailure(tlvWriterMsg2.EndContainer(outerContainerType)); ReturnErrorOnFailure(tlvWriterMsg2.Finalize(&msg_R2)); diff --git a/src/protocols/secure_channel/PASESession.cpp b/src/protocols/secure_channel/PASESession.cpp index ca2524d5e3f302..6f9ebf9747eb1b 100644 --- a/src/protocols/secure_channel/PASESession.cpp +++ b/src/protocols/secure_channel/PASESession.cpp @@ -300,7 +300,8 @@ CHIP_ERROR PASESession::SendPBKDFParamRequest() ReturnErrorOnFailure(tlvWriter.Put(TLV::ContextTag(3), kDefaultCommissioningPasscodeId)); ReturnErrorOnFailure(tlvWriter.PutBoolean(TLV::ContextTag(4), mHavePBKDFParameters)); - ReturnErrorOnFailure(EncodeSessionParameters(TLV::ContextTag(5), mLocalMRPConfig, tlvWriter)); + VerifyOrReturnError(mLocalMRPConfig.HasValue(), CHIP_ERROR_INCORRECT_STATE); + ReturnErrorOnFailure(EncodeSessionParameters(TLV::ContextTag(5), mLocalMRPConfig.Value(), tlvWriter)); ReturnErrorOnFailure(tlvWriter.EndContainer(outerContainerType)); ReturnErrorOnFailure(tlvWriter.Finalize(&req)); @@ -420,7 +421,8 @@ CHIP_ERROR PASESession::SendPBKDFParamResponse(ByteSpan initiatorRandom, bool in ReturnErrorOnFailure(tlvWriter.EndContainer(pbkdfParamContainer)); } - ReturnErrorOnFailure(EncodeSessionParameters(TLV::ContextTag(5), mLocalMRPConfig, tlvWriter)); + VerifyOrReturnError(mLocalMRPConfig.HasValue(), CHIP_ERROR_INCORRECT_STATE); + ReturnErrorOnFailure(EncodeSessionParameters(TLV::ContextTag(5), mLocalMRPConfig.Value(), tlvWriter)); ReturnErrorOnFailure(tlvWriter.EndContainer(outerContainerType)); ReturnErrorOnFailure(tlvWriter.Finalize(&resp)); diff --git a/src/protocols/secure_channel/PairingSession.h b/src/protocols/secure_channel/PairingSession.h index ea69f65bfaccb7..f28578bdac556a 100644 --- a/src/protocols/secure_channel/PairingSession.h +++ b/src/protocols/secure_channel/PairingSession.h @@ -244,7 +244,8 @@ class DLL_EXPORT PairingSession : public SessionDelegate // mLocalMRPConfig is our config which is sent to the other end and used by the peer session. // mRemoteSessionParams is received from other end and set to our session. - ReliableMessageProtocolConfig mLocalMRPConfig = GetLocalMRPConfig().ValueOr(GetDefaultMRPConfig()); + // It is set the first time that session establishment is initiated. + Optional mLocalMRPConfig; SessionParameters mRemoteSessionParams; private: From e87c03d6eeff6d1c3797b3b99fb6ccb8eb0e6b82 Mon Sep 17 00:00:00 2001 From: Matt Swartwout Date: Wed, 17 Apr 2024 23:31:47 +0000 Subject: [PATCH 06/10] Revert "Restyled by clang-format" This reverts commit 7b9c3a7f14f6e91830df0bf25944219036d18bc1. --- src/app/server/Server.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/app/server/Server.h b/src/app/server/Server.h index 575d2240bc2086..dc6f9a20e42a70 100644 --- a/src/app/server/Server.h +++ b/src/app/server/Server.h @@ -400,7 +400,7 @@ class Server static Server & GetInstance() { - static Server * sServer = new Server(); + static Server* sServer = new Server(); return *sServer; } From 35eb4dd630137ffa22627f88fd2d191871da7b9d Mon Sep 17 00:00:00 2001 From: Matt Swartwout Date: Wed, 17 Apr 2024 23:32:01 +0000 Subject: [PATCH 07/10] Revert "Fix pointer declaration" This reverts commit d5282f6b0634150f32ee990a5e3b4c0eb6f1470e. --- src/app/server/Server.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/app/server/Server.h b/src/app/server/Server.h index dc6f9a20e42a70..c1b42390781733 100644 --- a/src/app/server/Server.h +++ b/src/app/server/Server.h @@ -400,7 +400,7 @@ class Server static Server & GetInstance() { - static Server* sServer = new Server(); + static Server sServer = new Server(); return *sServer; } From 2a4fa19e2f3f3baf93f60905d78ac612db369e18 Mon Sep 17 00:00:00 2001 From: Matt Swartwout Date: Wed, 17 Apr 2024 23:32:14 +0000 Subject: [PATCH 08/10] Revert "Restyled by clang-format" This reverts commit d73e6e1ca1b4798924a9131e661426346e2d5020. --- src/app/server/Server.h | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/app/server/Server.h b/src/app/server/Server.h index c1b42390781733..52ddafa7f42b0e 100644 --- a/src/app/server/Server.h +++ b/src/app/server/Server.h @@ -398,10 +398,9 @@ class Server return System::SystemClock().GetMonotonicMicroseconds64() - mInitTimestamp; } - static Server & GetInstance() - { - static Server sServer = new Server(); - return *sServer; + static Server & GetInstance() { + static Server sServer = new Server(); + return *sServer; } private: From 99066adb41d05afbb4b89b7a4a44f2df347fad0a Mon Sep 17 00:00:00 2001 From: Matt Swartwout Date: Wed, 17 Apr 2024 23:32:25 +0000 Subject: [PATCH 09/10] Revert "Fix initialization-order-fiasco" This reverts commit e9510a243f84e3b96a5985974953a092343d1d32. --- src/app/server/Server.cpp | 2 ++ src/app/server/Server.h | 7 +++---- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/src/app/server/Server.cpp b/src/app/server/Server.cpp index a21ac3c4b32cdb..57abf8087f59ca 100644 --- a/src/app/server/Server.cpp +++ b/src/app/server/Server.cpp @@ -87,6 +87,8 @@ class DeviceTypeResolver : public chip::Access::AccessControl::DeviceTypeResolve namespace chip { +Server Server::sServer; + #if CHIP_CONFIG_ENABLE_SERVER_IM_EVENT #define CHIP_NUM_EVENT_LOGGING_BUFFERS 3 static uint8_t sInfoEventBuffer[CHIP_DEVICE_CONFIG_EVENT_LOGGING_INFO_BUFFER_SIZE]; diff --git a/src/app/server/Server.h b/src/app/server/Server.h index 52ddafa7f42b0e..8f6fcd5abecc66 100644 --- a/src/app/server/Server.h +++ b/src/app/server/Server.h @@ -398,14 +398,13 @@ class Server return System::SystemClock().GetMonotonicMicroseconds64() - mInitTimestamp; } - static Server & GetInstance() { - static Server sServer = new Server(); - return *sServer; - } + static Server & GetInstance() { return sServer; } private: Server() {} + static Server sServer; + void InitFailSafe(); void OnPlatformEvent(const DeviceLayer::ChipDeviceEvent & event); void CheckServerReadyEvent(); From 42026d5a8f04ad387e0b47abb8deb9a2ebe7fd23 Mon Sep 17 00:00:00 2001 From: Matt Swartwout Date: Wed, 17 Apr 2024 23:35:52 +0000 Subject: [PATCH 10/10] Add missing MakeOptional --- src/protocols/secure_channel/CASESession.cpp | 4 ++-- src/protocols/secure_channel/PASESession.cpp | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/protocols/secure_channel/CASESession.cpp b/src/protocols/secure_channel/CASESession.cpp index 5de6778c49fde2..9391ab621ebb9f 100644 --- a/src/protocols/secure_channel/CASESession.cpp +++ b/src/protocols/secure_channel/CASESession.cpp @@ -477,7 +477,7 @@ CASESession::PrepareForSessionEstablishment(SessionManager & sessionManager, Fab mFabricsTable = fabricTable; mRole = CryptoContext::SessionRole::kResponder; mSessionResumptionStorage = sessionResumptionStorage; - mLocalMRPConfig = mrpLocalConfig.ValueOr(GetDefaultMRPConfig()); + mLocalMRPConfig = MakeOptional(mrpLocalConfig.ValueOr(GetDefaultMRPConfig())); ChipLogDetail(SecureChannel, "Allocated SecureSession (%p) - waiting for Sigma1 msg", mSecureSessionHolder.Get().Value()->AsSecureSession()); @@ -525,7 +525,7 @@ CHIP_ERROR CASESession::EstablishSession(SessionManager & sessionManager, Fabric mFabricsTable = fabricTable; mFabricIndex = fabricInfo->GetFabricIndex(); mSessionResumptionStorage = sessionResumptionStorage; - mLocalMRPConfig = mrpLocalConfig.ValueOr(GetDefaultMRPConfig()); + mLocalMRPConfig = MakeOptional(mrpLocalConfig.ValueOr(GetDefaultMRPConfig())); mExchangeCtxt.Value()->UseSuggestedResponseTimeout(kExpectedSigma1ProcessingTime); mPeerNodeId = peerScopedNodeId.GetNodeId(); diff --git a/src/protocols/secure_channel/PASESession.cpp b/src/protocols/secure_channel/PASESession.cpp index 6f9ebf9747eb1b..7a7d8dd36a32e6 100644 --- a/src/protocols/secure_channel/PASESession.cpp +++ b/src/protocols/secure_channel/PASESession.cpp @@ -195,7 +195,7 @@ CHIP_ERROR PASESession::WaitForPairing(SessionManager & sessionManager, const Sp mIterationCount = pbkdf2IterCount; mNextExpectedMsg.SetValue(MsgType::PBKDFParamRequest); mPairingComplete = false; - mLocalMRPConfig = mrpLocalConfig.ValueOr(GetDefaultMRPConfig()); + mLocalMRPConfig = MakeOptional(mrpLocalConfig.ValueOr(GetDefaultMRPConfig())); ChipLogDetail(SecureChannel, "Waiting for PBKDF param request"); @@ -225,7 +225,7 @@ CHIP_ERROR PASESession::Pair(SessionManager & sessionManager, uint32_t peerSetUp mExchangeCtxt.Value()->UseSuggestedResponseTimeout(kExpectedLowProcessingTime); - mLocalMRPConfig = mrpLocalConfig.ValueOr(GetDefaultMRPConfig()); + mLocalMRPConfig = MakeOptional(mrpLocalConfig.ValueOr(GetDefaultMRPConfig())); err = SendPBKDFParamRequest(); SuccessOrExit(err);