From ac1ebc508d97dcde9e5606ca68ad72961589f11e Mon Sep 17 00:00:00 2001
From: Boris Zbarsky <bzbarsky@apple.com>
Date: Thu, 29 Feb 2024 21:59:14 -0500
Subject: [PATCH] Fix incorrect snapshotting of MRP config by CASE client.
 (#32380)

* Fix incorrect snapshotting of MRP config by CASE client.

Right now, we snapshot the MRP config as part of the CASEClientInitParams during stack startup.

After that, we will use that snapshotted config whenever we are the CASE initiator.

This config will not match the parameters we use as CASE responder or advertise
over DNS-SD if the local MRP configuration ever changes.  Which for an ICD it can.

The fix is to stop the incorrect snapshotting and get the information we need
from the right source of truth when we need it.

* Address review comments.

* Address issue with CASE session not assuming the right things when NullOptional is passed in.
---
 src/app/CASEClient.cpp                         | 5 ++++-
 src/app/CASEClient.h                           | 6 +++++-
 src/app/server/Server.cpp                      | 4 +++-
 src/controller/CHIPDeviceControllerFactory.cpp | 4 +++-
 4 files changed, 15 insertions(+), 4 deletions(-)

diff --git a/src/app/CASEClient.cpp b/src/app/CASEClient.cpp
index 7ce5a539e6105a..5eef5088fd77c1 100644
--- a/src/app/CASEClient.cpp
+++ b/src/app/CASEClient.cpp
@@ -16,6 +16,7 @@
  */
 
 #include <app/CASEClient.h>
+#include <messaging/ReliableMessageProtocolConfig.h>
 
 namespace chip {
 
@@ -49,10 +50,12 @@ CHIP_ERROR CASEClient::EstablishSession(const CASEClientInitParams & params, con
     Messaging::ExchangeContext * exchange = params.exchangeMgr->NewContext(session.Value(), &mCASESession);
     VerifyOrReturnError(exchange != nullptr, CHIP_ERROR_INTERNAL);
 
+    const Optional<ReliableMessageProtocolConfig> & mrpLocalConfig =
+        params.mrpLocalConfig.HasValue() ? params.mrpLocalConfig : GetLocalMRPConfig();
     mCASESession.SetGroupDataProvider(params.groupDataProvider);
     ReturnErrorOnFailure(mCASESession.EstablishSession(*params.sessionManager, params.fabricTable, peer, exchange,
                                                        params.sessionResumptionStorage, params.certificateValidityPolicy, delegate,
-                                                       params.mrpLocalConfig));
+                                                       mrpLocalConfig));
 
     return CHIP_NO_ERROR;
 }
diff --git a/src/app/CASEClient.h b/src/app/CASEClient.h
index b640ae066f0f09..7ef968d544165f 100644
--- a/src/app/CASEClient.h
+++ b/src/app/CASEClient.h
@@ -34,7 +34,11 @@ struct CASEClientInitParams
     Messaging::ExchangeManager * exchangeMgr                           = nullptr;
     FabricTable * fabricTable                                          = nullptr;
     Credentials::GroupDataProvider * groupDataProvider                 = nullptr;
-    Optional<ReliableMessageProtocolConfig> mrpLocalConfig             = Optional<ReliableMessageProtocolConfig>::Missing();
+
+    // mrpLocalConfig should not generally be set to anything other than
+    // NullOptional.  Doing that can lead to different parts of the system
+    // claiming different MRP parameters for the same node.
+    Optional<ReliableMessageProtocolConfig> mrpLocalConfig = NullOptional;
 
     CHIP_ERROR Validate() const
     {
diff --git a/src/app/server/Server.cpp b/src/app/server/Server.cpp
index f2a562020091a3..e046080c1089a5 100644
--- a/src/app/server/Server.cpp
+++ b/src/app/server/Server.cpp
@@ -313,7 +313,9 @@ CHIP_ERROR Server::Init(const ServerInitParams & initParams)
             .exchangeMgr       = &mExchangeMgr,
             .fabricTable       = &mFabrics,
             .groupDataProvider = mGroupsProvider,
-            .mrpLocalConfig    = GetLocalMRPConfig(),
+            // Don't provide an MRP local config, so each CASE initiation will use
+            // the then-current value.
+            .mrpLocalConfig = NullOptional,
         },
         .clientPool            = &mCASEClientPool,
         .sessionSetupPool      = &mSessionSetupPool,
diff --git a/src/controller/CHIPDeviceControllerFactory.cpp b/src/controller/CHIPDeviceControllerFactory.cpp
index ebc728ca206875..25bcd12b190a09 100644
--- a/src/controller/CHIPDeviceControllerFactory.cpp
+++ b/src/controller/CHIPDeviceControllerFactory.cpp
@@ -270,7 +270,9 @@ CHIP_ERROR DeviceControllerFactory::InitSystemState(FactoryInitParams params)
         .exchangeMgr               = stateParams.exchangeMgr,
         .fabricTable               = stateParams.fabricTable,
         .groupDataProvider         = stateParams.groupDataProvider,
-        .mrpLocalConfig            = GetLocalMRPConfig(),
+        // Don't provide an MRP local config, so each CASE initiation will use
+        // the then-current value.
+        .mrpLocalConfig = NullOptional,
     };
 
     CASESessionManagerConfig sessionManagerConfig = {