diff --git a/src/app/OperationalDeviceProxy.cpp b/src/app/OperationalDeviceProxy.cpp index 3a6ffe69bb6b96..51f3afea749696 100644 --- a/src/app/OperationalDeviceProxy.cpp +++ b/src/app/OperationalDeviceProxy.cpp @@ -191,7 +191,7 @@ CHIP_ERROR OperationalDeviceProxy::UpdateDeviceData(const Transport::PeerAddress return CHIP_NO_ERROR; } - mSecureSession.Get()->AsSecureSession()->SetPeerAddress(addr); + mSecureSession.Get().Value()->AsSecureSession()->SetPeerAddress(addr); } return err; @@ -305,7 +305,7 @@ CHIP_ERROR OperationalDeviceProxy::Disconnect() ReturnErrorCodeIf(mState != State::SecureConnected, CHIP_ERROR_INCORRECT_STATE); if (mSecureSession) { - mInitParams.sessionManager->ExpirePairing(mSecureSession.Get()); + mInitParams.sessionManager->ExpirePairing(mSecureSession.Get().Value()); } MoveToState(State::Initialized); diff --git a/src/app/OperationalDeviceProxy.h b/src/app/OperationalDeviceProxy.h index 5f3b7adaf75ca3..a1623c17733810 100644 --- a/src/app/OperationalDeviceProxy.h +++ b/src/app/OperationalDeviceProxy.h @@ -182,7 +182,7 @@ class DLL_EXPORT OperationalDeviceProxy : public DeviceProxy, Messaging::ExchangeManager * GetExchangeManager() const override { return mInitParams.exchangeMgr; } - chip::Optional GetSecureSession() const override { return mSecureSession.ToOptional(); } + chip::Optional GetSecureSession() const override { return mSecureSession.Get(); } Transport::PeerAddress GetPeerAddress() const { return mDeviceAddress; } diff --git a/src/app/ReadClient.cpp b/src/app/ReadClient.cpp index 7e1ea4efa8aaf2..e609d645774e86 100644 --- a/src/app/ReadClient.cpp +++ b/src/app/ReadClient.cpp @@ -269,7 +269,9 @@ CHIP_ERROR ReadClient::SendReadRequest(ReadPrepareParams & aReadPrepareParams) ReturnErrorOnFailure(request.EndOfReadRequestMessage().GetError()); ReturnErrorOnFailure(writer.Finalize(&msgBuf)); - mpExchangeCtx = mpExchangeMgr->NewContext(aReadPrepareParams.mSessionHolder.Get(), this); + VerifyOrReturnError(aReadPrepareParams.mSessionHolder, CHIP_ERROR_MISSING_SECURE_SESSION); + + mpExchangeCtx = mpExchangeMgr->NewContext(aReadPrepareParams.mSessionHolder.Get().Value(), this); VerifyOrReturnError(mpExchangeCtx != nullptr, err = CHIP_ERROR_NO_MEMORY); mpExchangeCtx->SetResponseTimeout(aReadPrepareParams.mTimeout); @@ -913,7 +915,9 @@ CHIP_ERROR ReadClient::SendSubscribeRequest(ReadPrepareParams & aReadPreparePara ReturnErrorOnFailure(err = request.EndOfSubscribeRequestMessage().GetError()); ReturnErrorOnFailure(writer.Finalize(&msgBuf)); - mpExchangeCtx = mpExchangeMgr->NewContext(aReadPrepareParams.mSessionHolder.Get(), this); + VerifyOrReturnError(aReadPrepareParams.mSessionHolder, CHIP_ERROR_MISSING_SECURE_SESSION); + + mpExchangeCtx = mpExchangeMgr->NewContext(aReadPrepareParams.mSessionHolder.Get().Value(), this); VerifyOrReturnError(mpExchangeCtx != nullptr, err = CHIP_ERROR_NO_MEMORY); mpExchangeCtx->SetResponseTimeout(kImMessageTimeout); diff --git a/src/app/ReadHandler.cpp b/src/app/ReadHandler.cpp index 0b33746e68fa41..c02d9d222f6e1a 100644 --- a/src/app/ReadHandler.cpp +++ b/src/app/ReadHandler.cpp @@ -221,7 +221,7 @@ CHIP_ERROR ReadHandler::SendStatusReport(Protocols::InteractionModel::Status aSt { VerifyOrReturnLogError(mpExchangeCtx == nullptr, CHIP_ERROR_INCORRECT_STATE); VerifyOrReturnLogError(mSessionHandle, CHIP_ERROR_INCORRECT_STATE); - mpExchangeCtx = InteractionModelEngine::GetInstance()->GetExchangeManager()->NewContext(mSessionHandle.Get(), this); + mpExchangeCtx = InteractionModelEngine::GetInstance()->GetExchangeManager()->NewContext(mSessionHandle.Get().Value(), this); } VerifyOrReturnLogError(mpExchangeCtx != nullptr, CHIP_ERROR_INCORRECT_STATE); @@ -240,7 +240,7 @@ CHIP_ERROR ReadHandler::SendReportData(System::PacketBufferHandle && aPayload, b { VerifyOrReturnLogError(mpExchangeCtx == nullptr, CHIP_ERROR_INCORRECT_STATE); VerifyOrReturnLogError(mSessionHandle, CHIP_ERROR_INCORRECT_STATE); - mpExchangeCtx = InteractionModelEngine::GetInstance()->GetExchangeManager()->NewContext(mSessionHandle.Get(), this); + mpExchangeCtx = InteractionModelEngine::GetInstance()->GetExchangeManager()->NewContext(mSessionHandle.Get().Value(), this); } VerifyOrReturnLogError(mpExchangeCtx != nullptr, CHIP_ERROR_INCORRECT_STATE); diff --git a/src/controller/CommissioneeDeviceProxy.cpp b/src/controller/CommissioneeDeviceProxy.cpp index 320e88eabe1bb2..d0d61ef8974ba7 100644 --- a/src/controller/CommissioneeDeviceProxy.cpp +++ b/src/controller/CommissioneeDeviceProxy.cpp @@ -47,7 +47,8 @@ CHIP_ERROR CommissioneeDeviceProxy::SendCommands(app::CommandSender * commandObj { VerifyOrReturnError(mSecureSession, CHIP_ERROR_INCORRECT_STATE); VerifyOrReturnError(commandObj != nullptr, CHIP_ERROR_INVALID_ARGUMENT); - return commandObj->SendCommandRequest(mSecureSession.Get(), timeout); + VerifyOrReturnError(mSecureSession.Get().HasValue(), CHIP_ERROR_MISSING_SECURE_SESSION); + return commandObj->SendCommandRequest(mSecureSession.Get().Value(), timeout); } void CommissioneeDeviceProxy::OnSessionReleased() @@ -60,7 +61,7 @@ CHIP_ERROR CommissioneeDeviceProxy::CloseSession() ReturnErrorCodeIf(mState != ConnectionState::SecureConnected, CHIP_ERROR_INCORRECT_STATE); if (mSecureSession) { - mSessionManager->ExpirePairing(mSecureSession.Get()); + mSessionManager->ExpirePairing(mSecureSession.Get().Value()); } mState = ConnectionState::NotConnected; mPairing.Clear(); @@ -87,7 +88,7 @@ CHIP_ERROR CommissioneeDeviceProxy::UpdateDeviceData(const Transport::PeerAddres return CHIP_NO_ERROR; } - Transport::SecureSession * secureSession = mSecureSession.Get()->AsSecureSession(); + Transport::SecureSession * secureSession = mSecureSession.Get().Value()->AsSecureSession(); secureSession->SetPeerAddress(addr); return CHIP_NO_ERROR; diff --git a/src/controller/CommissioneeDeviceProxy.h b/src/controller/CommissioneeDeviceProxy.h index 34cbfe7aeeb6f3..972497a060e3f2 100644 --- a/src/controller/CommissioneeDeviceProxy.h +++ b/src/controller/CommissioneeDeviceProxy.h @@ -137,7 +137,7 @@ class CommissioneeDeviceProxy : public DeviceProxy, public SessionDelegate CHIP_ERROR SetPeerId(ByteSpan rcac, ByteSpan noc) override; const Transport::PeerAddress & GetPeerAddress() const { return mDeviceAddress; } - chip::Optional GetSecureSession() const override { return mSecureSession.ToOptional(); } + chip::Optional GetSecureSession() const override { return mSecureSession.Get(); } Messaging::ExchangeManager * GetExchangeManager() const override { return mExchangeMgr; } diff --git a/src/controller/python/chip/clusters/attribute.cpp b/src/controller/python/chip/clusters/attribute.cpp index c260cb94ce4cc5..16cee29f58740d 100644 --- a/src/controller/python/chip/clusters/attribute.cpp +++ b/src/controller/python/chip/clusters/attribute.cpp @@ -85,7 +85,7 @@ OnReportBeginCallback gOnReportEndCallback = nullptr; void PythonResubscribePolicy(uint32_t aNumCumulativeRetries, uint32_t & aNextSubscriptionIntervalMsec, bool & aShouldResubscribe) { - aShouldResubscribe = false; + aShouldResubscribe = true; } class ReadClientCallback : public ReadClient::Callback diff --git a/src/lib/core/CHIPError.cpp b/src/lib/core/CHIPError.cpp index 785ee0b7af07e9..2f85e0576de0b9 100644 --- a/src/lib/core/CHIPError.cpp +++ b/src/lib/core/CHIPError.cpp @@ -713,6 +713,9 @@ bool FormatCHIPError(char * buf, uint16_t bufSize, CHIP_ERROR err) case CHIP_ERROR_IM_MALFORMED_STATUS_RESPONSE_MESSAGE.AsInteger(): desc = "Malformed Interaction Model Status Response Message"; break; + case CHIP_ERROR_MISSING_SECURE_SESSION.AsInteger(): + desc = "Missing secure session"; + break; } #endif // !CHIP_CONFIG_SHORT_ERROR_STR diff --git a/src/lib/core/CHIPError.h b/src/lib/core/CHIPError.h index 732d1f60d4241d..bfb9c366c618c4 100644 --- a/src/lib/core/CHIPError.h +++ b/src/lib/core/CHIPError.h @@ -2389,8 +2389,13 @@ using CHIP_ERROR = ::chip::ChipError; #define CHIP_ERROR_IM_MALFORMED_STATUS_RESPONSE_MESSAGE CHIP_CORE_ERROR(0xe1) /** - * @} + * @def CHIP_ERROR_MISSING_SECURE_SESSION + * + * @brief + * + * A secure session is needed to do work, but is missing/is not present. */ +#define CHIP_ERROR_MISSING_SECURE_SESSION CHIP_CORE_ERROR(0xe2) // clang-format on diff --git a/src/messaging/ExchangeContext.cpp b/src/messaging/ExchangeContext.cpp index 28592c19457d06..7388158eb74a43 100644 --- a/src/messaging/ExchangeContext.cpp +++ b/src/messaging/ExchangeContext.cpp @@ -183,8 +183,15 @@ CHIP_ERROR ExchangeContext::SendMessage(Protocols::Id protocolId, uint8_t msgTyp return CHIP_ERROR_INTERNAL; } + // + // Our session should still be valid, since it had been evicted, all exchanges on that session would have been torn down + // as a side-effect. So for us to get here to send a message and still not have a session, something terribly wrong must have + // happened. + // + VerifyOrDie(mSession); + // Create a new scope for `err`, to avoid shadowing warning previous `err`. - CHIP_ERROR err = mDispatch.SendMessage(GetExchangeMgr()->GetSessionManager(), mSession.Get(), mExchangeId, IsInitiator(), + CHIP_ERROR err = mDispatch.SendMessage(GetExchangeMgr()->GetSessionManager(), mSession.Get().Value(), mExchangeId, IsInitiator(), GetReliableMessageContext(), reliableTransmissionRequested, protocolId, msgType, std::move(msgBuf)); if (err != CHIP_NO_ERROR && IsResponseExpected()) diff --git a/src/messaging/ExchangeContext.h b/src/messaging/ExchangeContext.h index c0211c857384e4..9a516fa8d813a7 100644 --- a/src/messaging/ExchangeContext.h +++ b/src/messaging/ExchangeContext.h @@ -156,7 +156,12 @@ class DLL_EXPORT ExchangeContext : public ReliableMessageContext, ExchangeMessageDispatch & GetMessageDispatch() { return mDispatch; } - SessionHandle GetSessionHandle() const { return mSession.Get(); } + SessionHandle GetSessionHandle() const { + VerifyOrDie(mSession); + auto sessionHandle = mSession.Get(); + return std::move(sessionHandle.Value()); + } + bool HasSessionHandle() const { return mSession; } uint16_t GetExchangeId() const { return mExchangeId; } diff --git a/src/protocols/echo/EchoClient.cpp b/src/protocols/echo/EchoClient.cpp index 6805dabb67a995..ee546d4db449d5 100644 --- a/src/protocols/echo/EchoClient.cpp +++ b/src/protocols/echo/EchoClient.cpp @@ -70,8 +70,10 @@ CHIP_ERROR EchoClient::SendEchoRequest(System::PacketBufferHandle && payload, Me mExchangeCtx = nullptr; } + VerifyOrReturnError(mSecureSession, CHIP_ERROR_INVALID_MESSAGE_TYPE); + // Create a new exchange context. - mExchangeCtx = mExchangeMgr->NewContext(mSecureSession.Get(), this); + mExchangeCtx = mExchangeMgr->NewContext(mSecureSession.Get().Value(), this); if (mExchangeCtx == nullptr) { return CHIP_ERROR_NO_MEMORY; diff --git a/src/protocols/secure_channel/PairingSession.cpp b/src/protocols/secure_channel/PairingSession.cpp index 01c428020464b3..0ab53dc06e6a07 100644 --- a/src/protocols/secure_channel/PairingSession.cpp +++ b/src/protocols/secure_channel/PairingSession.cpp @@ -67,7 +67,8 @@ void PairingSession::Finish() CHIP_ERROR err = ActivateSecureSession(address); if (err == CHIP_NO_ERROR) { - mDelegate->OnSessionEstablished(mSecureSessionHolder.Get()); + VerifyOrDie(mSecureSessionHolder); + mDelegate->OnSessionEstablished(mSecureSessionHolder.Get().Value()); } else { @@ -155,14 +156,14 @@ void PairingSession::Clear() if (mSecureSessionHolder) { - SessionHandle session = mSecureSessionHolder.Get(); + auto session = mSecureSessionHolder.Get(); // Call Release before ExpirePairing because we don't want to receive OnSessionReleased() event here mSecureSessionHolder.Release(); - if (!session->AsSecureSession()->IsActiveSession() && mSessionManager != nullptr) + if (!session.Value()->AsSecureSession()->IsActiveSession() && mSessionManager != nullptr) { // Make sure to clean up our pending session, since we're the only // ones who have access to it do do so. - mSessionManager->ExpirePairing(session); + mSessionManager->ExpirePairing(session.Value()); } } diff --git a/src/transport/SessionHolder.h b/src/transport/SessionHolder.h index 28321d9e71c2e9..60ea973b05988a 100644 --- a/src/transport/SessionHolder.h +++ b/src/transport/SessionHolder.h @@ -51,10 +51,9 @@ class SessionHolder : public SessionDelegate, public IntrusiveListNodeBase void Release(); operator bool() const { return mSession.HasValue(); } - SessionHandle Get() const { return SessionHandle{ mSession.Value().Get() }; } - Optional ToOptional() const + Optional Get() const { - return mSession.HasValue() ? chip::MakeOptional(Get()) : chip::Optional::Missing(); + return mSession.HasValue() ? chip::MakeOptional(mSession.Value().Get()) : chip::Optional::Missing(); } Transport::Session * operator->() const { return &mSession.Value().Get(); }