Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Restyle Invalidate pending CASESession establishment when Fabric changes #19347

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions src/app/clusters/bindings/BindingManager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,9 @@ class BindingFabricTableDelegate : public chip::FabricTable::Delegate

// Intentionally left blank
void OnFabricPersistedToStorage(chip::FabricTable & fabricTable, chip::FabricIndex fabricIndex) override {}

// Intentionally left blank
void OnFabricNOCUpdated(chip::FabricTable & fabricTable, chip::FabricIndex fabricIndex) override {}
};

BindingFabricTableDelegate gFabricTableDelegate;
Expand Down
3 changes: 3 additions & 0 deletions src/app/clusters/door-lock-server/door-lock-server.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,9 @@ class DoorLockClusterFabricDelegate : public chip::FabricTable::Delegate

// Intentionally left blank
void OnFabricPersistedToStorage(FabricTable & fabricTable, FabricIndex fabricIndex) override {}

// Intentionally left blank
void OnFabricNOCUpdated(chip::FabricTable & fabricTable, chip::FabricIndex fabricIndex) override {}
};
static DoorLockClusterFabricDelegate gFabricDelegate;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -447,6 +447,9 @@ class OpCredsFabricTableDelegate : public chip::FabricTable::Delegate
ChipLogValueX64(fabric->GetNodeId()), fabric->GetVendorId());
fabricListChanged();
}

// This is triggered by operation credential server so there is nothing additional that we need to do.
void OnFabricNOCUpdated(chip::FabricTable & fabricTable, chip::FabricIndex fabricIndex) override {}
};

OpCredsFabricTableDelegate gFabricDelegate;
Expand Down
6 changes: 6 additions & 0 deletions src/app/server/Server.h
Original file line number Diff line number Diff line change
Expand Up @@ -436,6 +436,12 @@ class Server
(void) fabricIndex;
}

void OnFabricNOCUpdated(chip::FabricTable & fabricTable, chip::FabricIndex fabricIndex) override
{
(void) fabricTable;
(void) fabricIndex;
}

private:
Server * mServer = nullptr;
};
Expand Down
6 changes: 6 additions & 0 deletions src/controller/CHIPDeviceControllerFactory.h
Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,12 @@ class DeviceControllerFactory
(void) fabricIndex;
}

void OnFabricNOCUpdated(chip::FabricTable & fabricTable, chip::FabricIndex fabricIndex) override
{
(void) fabricTable;
(void) fabricIndex;
}

private:
SessionManager * mSessionManager = nullptr;
Credentials::GroupDataProvider * mGroupDataProvider = nullptr;
Expand Down
22 changes: 21 additions & 1 deletion src/credentials/FabricTable.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -710,6 +710,20 @@ class NotBeforeCollector : public Credentials::CertificateValidityPolicy
System::Clock::Seconds32 mLatestNotBefore;
};

CHIP_ERROR FabricTable::SendFabricUpdateNOC(FabricIndex fabricIndex)
{
FabricTable::Delegate * delegate = mDelegateListRoot;
while (delegate)
{
// It is possible that delegate will remove itself from the list in OnFabricNOCUpdated,
// so we grab the next delegate in the list now.
FabricTable::Delegate * nextDelegate = delegate->next;
delegate->OnFabricNOCUpdated(*this, fabricIndex);
delegate = nextDelegate;
}
return CHIP_NO_ERROR;
}

CHIP_ERROR FabricTable::UpdateFabric(FabricIndex fabricIndex, FabricInfo & newFabricInfo)
{
FabricInfo * fabricInfo = FindFabricWithIndex(fabricIndex);
Expand All @@ -722,6 +736,9 @@ CHIP_ERROR FabricTable::UpdateFabric(FabricIndex fabricIndex, FabricInfo & newFa
// for CASE and current time is also unknown, the certificate
// validity policy will see this condition and can act appropriately.
mLastKnownGoodTime.UpdateLastKnownGoodChipEpochTime(notBeforeCollector.mLatestNotBefore);

SendFabricUpdateNOC(fabricIndex);

return CHIP_NO_ERROR;
}

Expand Down Expand Up @@ -848,8 +865,11 @@ CHIP_ERROR FabricTable::Delete(FabricIndex fabricIndex)
FabricTable::Delegate * delegate = mDelegateListRoot;
while (delegate)
{
// It is possible that delegate will remove itself from the list in OnFabricDeletedFromStorage,
// so we grab the next delegate in the list now.
FabricTable::Delegate * nextDelegate = delegate->next;
delegate->OnFabricDeletedFromStorage(*this, fabricIndex);
delegate = delegate->next;
delegate = nextDelegate;
}
}
return CHIP_NO_ERROR;
Expand Down
11 changes: 11 additions & 0 deletions src/credentials/FabricTable.h
Original file line number Diff line number Diff line change
Expand Up @@ -384,6 +384,11 @@ class DLL_EXPORT FabricTable
**/
virtual void OnFabricPersistedToStorage(FabricTable & fabricTable, FabricIndex fabricIndex) = 0;

/**
* Gets called when operational credentials are changed.
**/
virtual void OnFabricNOCUpdated(FabricTable & fabricTable, FabricIndex fabricIndex) = 0;

// Intrusive list pointer for FabricTable to manage the entries.
Delegate * next = nullptr;
};
Expand Down Expand Up @@ -424,6 +429,10 @@ class DLL_EXPORT FabricTable
*/
CHIP_ERROR UpdateFabric(FabricIndex fabricIndex, FabricInfo & fabricInfo);

#if CONFIG_IM_BUILD_FOR_UNIT_TEST
void SendUpdateFabricNotificationForTest(FabricIndex fabricIndex) { SendFabricUpdateNOC(fabricIndex); }
#endif // CONFIG_IM_BUILD_FOR_UNIT_TEST

FabricInfo * FindFabric(Credentials::P256PublicKeySpan rootPubKey, FabricId fabricId);
FabricInfo * FindFabricWithIndex(FabricIndex fabricIndex);
const FabricInfo * FindFabricWithIndex(FabricIndex fabricIndex) const;
Expand Down Expand Up @@ -609,6 +618,8 @@ class DLL_EXPORT FabricTable

CHIP_ERROR AddNewFabricInner(FabricInfo & fabric, FabricIndex * assignedIndex);

CHIP_ERROR SendFabricUpdateNOC(FabricIndex fabricIndex);

FabricInfo mStates[CHIP_CONFIG_MAX_FABRICS];
PersistentStorageDelegate * mStorage = nullptr;
Crypto::OperationalKeystore * mOperationalKeystore = nullptr;
Expand Down
63 changes: 55 additions & 8 deletions src/protocols/secure_channel/CASESession.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -144,12 +144,30 @@ void CASESession::Clear()
mState = State::kInitialized;
Crypto::ClearSecretData(mIPK);

if (mFabricsTable)
{
mFabricsTable->RemoveFabricDelegate(&mFabricDelegate);
}

mLocalNodeId = kUndefinedNodeId;
mPeerNodeId = kUndefinedNodeId;
mFabricsTable = nullptr;
mFabricIndex = kUndefinedFabricIndex;
}

void CASESession::InvalidateIfPendingEstablishmentOnFabric(FabricIndex fabricIndex)
{
if (mFabricIndex != fabricIndex)
{
return;
}
if (!IsSessionEstablishmentInProgress())
{
return;
}
AbortPendingEstablish(CHIP_ERROR_CANCELLED);
}

CHIP_ERROR CASESession::Init(SessionManager & sessionManager, Credentials::CertificateValidityPolicy * policy,
SessionEstablishmentDelegate * delegate, const ScopedNodeId & sessionEvictionHint)
{
Expand All @@ -172,24 +190,32 @@ CHIP_ERROR CASESession::Init(SessionManager & sessionManager, Credentials::Certi
}

CHIP_ERROR
CASESession::PrepareForSessionEstablishment(SessionManager & sessionManager, FabricTable * fabrics,
CASESession::PrepareForSessionEstablishment(SessionManager & sessionManager, FabricTable * fabricTable,
SessionResumptionStorage * sessionResumptionStorage,
Credentials::CertificateValidityPolicy * policy,
SessionEstablishmentDelegate * delegate, ScopedNodeId previouslyEstablishedPeer,
Optional<ReliableMessageProtocolConfig> mrpConfig)
{
VerifyOrReturnError(fabrics != nullptr, CHIP_ERROR_INVALID_ARGUMENT);
VerifyOrReturnError(fabricTable != nullptr, CHIP_ERROR_INVALID_ARGUMENT);
ReturnErrorOnFailure(Init(sessionManager, policy, delegate, previouslyEstablishedPeer));

mFabricsTable = fabrics;
CHIP_ERROR err = CHIP_NO_ERROR;
SuccessOrExit(err = fabricTable->AddFabricDelegate(&mFabricDelegate));

mFabricsTable = fabricTable;
mRole = CryptoContext::SessionRole::kResponder;
mSessionResumptionStorage = sessionResumptionStorage;
mLocalMRPConfig = mrpConfig;

ChipLogDetail(SecureChannel, "Allocated SecureSession (%p) - waiting for Sigma1 msg",
mSecureSessionHolder.Get().Value()->AsSecureSession());

return CHIP_NO_ERROR;
exit:
if (err != CHIP_NO_ERROR)
{
Clear();
}
return err;
}

CHIP_ERROR CASESession::EstablishSession(SessionManager & sessionManager, FabricTable * fabricTable, ScopedNodeId peerScopedNodeId,
Expand Down Expand Up @@ -222,6 +248,8 @@ CHIP_ERROR CASESession::EstablishSession(SessionManager & sessionManager, Fabric
// been initialized
SuccessOrExit(err);

SuccessOrExit(err = fabricTable->AddFabricDelegate(&mFabricDelegate));

mFabricsTable = fabricTable;
mFabricIndex = fabricInfo->GetFabricIndex();
mSessionResumptionStorage = sessionResumptionStorage;
Expand Down Expand Up @@ -254,9 +282,14 @@ void CASESession::OnResponseTimeout(ExchangeContext * ec)
// Discard the exchange so that Clear() doesn't try closing it. The
// exchange will handle that.
DiscardExchange();
AbortPendingEstablish(CHIP_ERROR_TIMEOUT);
}

void CASESession::AbortPendingEstablish(CHIP_ERROR err)
{
Clear();
// Do this last in case the delegate frees us.
mDelegate->OnSessionEstablishmentError(CHIP_ERROR_TIMEOUT);
mDelegate->OnSessionEstablishmentError(err);
}

CHIP_ERROR CASESession::DeriveSecureSession(CryptoContext & session) const
Expand Down Expand Up @@ -1662,6 +1695,22 @@ CHIP_ERROR CASESession::OnMessageReceived(ExchangeContext * ec, const PayloadHea
Protocols::SecureChannel::MsgType msgType = static_cast<Protocols::SecureChannel::MsgType>(payloadHeader.GetMessageType());
SuccessOrExit(err);

#if CONFIG_IM_BUILD_FOR_UNIT_TEST
if (mStopHandshakeAtState.HasValue() && mState == mStopHandshakeAtState.Value())
{
mStopHandshakeAtState = Optional<State>::Missing();
// For testing purposes we are trying to stop a successful CASESession from happening by dropping part of the
// handshake in the middle. We are trying to keep both sides of the CASESession establishment in an active
// pending state. In order to keep this side open we have to tell the exchange context that we will send an
// async message.
//
// Should you need to resume the CASESession you could theortically pass along msg to a callback that gets
// registered when setting mStopHandshakeAtState.
mExchangeCtxt->WillSendMessage();
return CHIP_NO_ERROR;
}
#endif // CONFIG_IM_BUILD_FOR_UNIT_TEST

#if CHIP_CONFIG_SLOW_CRYPTO
if (msgType == Protocols::SecureChannel::MsgType::CASE_Sigma1 || msgType == Protocols::SecureChannel::MsgType::CASE_Sigma2 ||
msgType == Protocols::SecureChannel::MsgType::CASE_Sigma2Resume ||
Expand Down Expand Up @@ -1761,9 +1810,7 @@ CHIP_ERROR CASESession::OnMessageReceived(ExchangeContext * ec, const PayloadHea
// Discard the exchange so that Clear() doesn't try closing it. The
// exchange will handle that.
DiscardExchange();
Clear();
// Do this last in case the delegate frees us.
mDelegate->OnSessionEstablishmentError(err);
AbortPendingEstablish(err);
}
return err;
}
Expand Down
69 changes: 58 additions & 11 deletions src/protocols/secure_channel/CASESession.h
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,19 @@ class DLL_EXPORT CASESession : public Messaging::UnsolicitedMessageHandler,
public PairingSession
{
public:
// Public so it is accessible to unit test
enum class State : uint8_t
{
kInitialized = 0,
kSentSigma1 = 1,
kSentSigma2 = 2,
kSentSigma3 = 3,
kSentSigma1Resume = 4,
kSentSigma2Resume = 5,
kFinished = 6,
kFinishedViaResume = 7,
};

~CASESession() override;

Transport::SecureSession::Type GetSecureSessionType() const override { return Transport::SecureSession::Type::kCASE; }
Expand All @@ -73,7 +86,7 @@ class DLL_EXPORT CASESession : public Messaging::UnsolicitedMessageHandler,
* Initialize using configured fabrics and wait for session establishment requests.
*
* @param sessionManager session manager from which to allocate a secure session object
* @param fabrics Table of fabrics that are currently configured on the device
* @param fabricTable Table of fabrics that are currently configured on the device
* @param policy Optional application-provided certificate validity policy
* @param delegate Callback object
* @param previouslyEstablishedPeer If a session had previously been established successfully to a peer, this should
Expand All @@ -84,7 +97,7 @@ class DLL_EXPORT CASESession : public Messaging::UnsolicitedMessageHandler,
* @return CHIP_ERROR The result of initialization
*/
CHIP_ERROR PrepareForSessionEstablishment(
SessionManager & sessionManager, FabricTable * fabrics, SessionResumptionStorage * sessionResumptionStorage,
SessionManager & sessionManager, FabricTable * fabricTable, SessionResumptionStorage * sessionResumptionStorage,
Credentials::CertificateValidityPolicy * policy, SessionEstablishmentDelegate * delegate,
ScopedNodeId previouslyEstablishedPeer,
Optional<ReliableMessageProtocolConfig> mrpConfig = Optional<ReliableMessageProtocolConfig>::Missing());
Expand Down Expand Up @@ -173,17 +186,44 @@ class DLL_EXPORT CASESession : public Messaging::UnsolicitedMessageHandler,
**/
void Clear();

void InvalidateIfPendingEstablishmentOnFabric(FabricIndex fabricIndex);

#if CONFIG_IM_BUILD_FOR_UNIT_TEST
void SetStopSigmaHandshakeAt(Optional<State> state) { mStopHandshakeAtState = state; }
#endif // CONFIG_IM_BUILD_FOR_UNIT_TEST

private:
enum class State : uint8_t
class CASESessionFabricDelegate final : public chip::FabricTable::Delegate
{
kInitialized = 0,
kSentSigma1 = 1,
kSentSigma2 = 2,
kSentSigma3 = 3,
kSentSigma1Resume = 4,
kSentSigma2Resume = 5,
kFinished = 6,
kFinishedViaResume = 7,
public:
CASESessionFabricDelegate(CASESession * caseSession) : mCASESession(caseSession) {}

void OnFabricDeletedFromStorage(FabricTable & fabricTable, FabricIndex fabricIndex) override
{
(void) fabricTable;
mCASESession->InvalidateIfPendingEstablishmentOnFabric(fabricIndex);
}

void OnFabricRetrievedFromStorage(FabricTable & fabricTable, FabricIndex fabricIndex) override
{
(void) fabricTable;
(void) fabricIndex;
}

void OnFabricPersistedToStorage(FabricTable & fabricTable, FabricIndex fabricIndex) override
{
(void) fabricTable;
(void) fabricIndex;
}

void OnFabricNOCUpdated(chip::FabricTable & fabricTable, chip::FabricIndex fabricIndex) override
{
(void) fabricTable;
mCASESession->InvalidateIfPendingEstablishmentOnFabric(fabricIndex);
}

private:
CASESession * mCASESession;
};

/*
Expand Down Expand Up @@ -237,6 +277,8 @@ class DLL_EXPORT CASESession : public Messaging::UnsolicitedMessageHandler,
void OnSuccessStatusReport() override;
CHIP_ERROR OnFailureStatusReport(Protocols::SecureChannel::GeneralStatusCode generalCode, uint16_t protocolCode) override;

void AbortPendingEstablish(CHIP_ERROR err);

CHIP_ERROR GetHardcodedTime();

CHIP_ERROR SetEffectiveTime();
Expand Down Expand Up @@ -271,7 +313,12 @@ class DLL_EXPORT CASESession : public Messaging::UnsolicitedMessageHandler,
// Sigma1 initiator random, maintained to be reused post-Sigma1, such as when generating Sigma2 S2RK key
uint8_t mInitiatorRandom[kSigmaParamRandomNumberSize];

CASESessionFabricDelegate mFabricDelegate{ this };
State mState;

#if CONFIG_IM_BUILD_FOR_UNIT_TEST
Optional<State> mStopHandshakeAtState = Optional<State>::Missing();
#endif // CONFIG_IM_BUILD_FOR_UNIT_TEST
};

} // namespace chip
11 changes: 11 additions & 0 deletions src/protocols/secure_channel/PairingSession.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,17 @@ CHIP_ERROR PairingSession::DecodeMRPParametersIfPresent(TLV::Tag expectedTag, TL
return tlvReader.ExitContainer(containerType);
}

bool PairingSession::IsSessionEstablishmentInProgress()
{
if (!mSecureSessionHolder)
{
return false;
}

Transport::SecureSession * secureSession = mSecureSessionHolder->AsSecureSession();
return secureSession->IsPairing();
}

void PairingSession::Clear()
{
// Clear acts like the destructor if PairingSession, if it is call during
Expand Down
2 changes: 2 additions & 0 deletions src/protocols/secure_channel/PairingSession.h
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,8 @@ class DLL_EXPORT PairingSession : public SessionDelegate
*/
CHIP_ERROR DecodeMRPParametersIfPresent(TLV::Tag expectedTag, TLV::ContiguousBufferTLVReader & tlvReader);

bool IsSessionEstablishmentInProgress();

// TODO: remove Clear, we should create a new instance instead reset the old instance.
void Clear();

Expand Down
Loading