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

Invalidate pending CASESession establishment when Fabric changes #19346

Merged
Show file tree
Hide file tree
Changes from 20 commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
a5015bf
Add invalidate pending CASESession establishment when Fabric changes
tehampson Jun 7, 2022
a5ebd31
Merge branch 'master' into invalidate-case-session-upon-fabric-update…
tehampson Jun 8, 2022
211a2a4
Address comment and clean up after merging master
tehampson Jun 8, 2022
a07d4c9
Fix runtime and compile time errors
tehampson Jun 8, 2022
0fbf786
Clean up a couple of TODOs
tehampson Jun 8, 2022
bed2c65
Breakout OnFabricNOCUpdated as a standalone callback
tehampson Jun 9, 2022
c3cbc93
restyle
tehampson Jun 9, 2022
65c6c5f
Merge branch 'master' into invalidate-case-session-upon-fabric-update…
tehampson Jun 9, 2022
9ff1382
restyle and figuring out unit tests
tehampson Jun 9, 2022
cd19588
Merge branch 'master' into invalidate-case-session-upon-fabric-update…
tehampson Jun 10, 2022
66474f2
Merge branch 'master' into invalidate-case-session-upon-fabric-update…
tehampson Jun 10, 2022
5a4d62a
Merge branch 'master' into invalidate-case-session-upon-fabric-update…
tehampson Jun 13, 2022
03a88f0
Add unit test and fix issue discovered by unit test
tehampson Jun 13, 2022
5c19b8d
Merge branch 'master' into invalidate-case-session-upon-fabric-update…
tehampson Jun 13, 2022
4ceff6b
Fix formating issue
tehampson Jun 13, 2022
cdb8b10
clean up TestCASESession
tehampson Jun 13, 2022
8e3e076
Merge branch 'master' into invalidate-case-session-upon-fabric-update…
tehampson Jun 14, 2022
91c5614
Change simulating updating fabric NOC test to run in host tests only
tehampson Jun 14, 2022
ad4d136
Address PR comments
tehampson Jun 15, 2022
5fe85ab
Address PR comments
tehampson Jun 15, 2022
7abbb1a
Merge branch 'master' into invalidate-case-session-upon-fabric-update…
tehampson Jun 16, 2022
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 @@ -460,6 +460,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.
tehampson marked this conversation as resolved.
Show resolved Hide resolved
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 @@ -754,6 +754,20 @@ class NotBeforeCollector : public Credentials::CertificateValidityPolicy
System::Clock::Seconds32 mLatestNotBefore;
};

CHIP_ERROR FabricTable::NotifyNOCUpdatedOnFabric(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 @@ -766,6 +780,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);

NotifyNOCUpdatedOnFabric(fabricIndex);

return CHIP_NO_ERROR;
}

Expand Down Expand Up @@ -892,8 +909,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
13 changes: 13 additions & 0 deletions src/credentials/FabricTable.h
Original file line number Diff line number Diff line change
Expand Up @@ -364,6 +364,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 @@ -404,6 +409,12 @@ class DLL_EXPORT FabricTable
*/
CHIP_ERROR UpdateFabric(FabricIndex fabricIndex, FabricInfo & fabricInfo);

// TODO this #if CONFIG_IM_BUILD_FOR_UNIT_TEST is temporary. There is a change incoming soon
// that will allow triggering NOC update directly.
#if CONFIG_IM_BUILD_FOR_UNIT_TEST
tehampson marked this conversation as resolved.
Show resolved Hide resolved
void SendUpdateFabricNotificationForTest(FabricIndex fabricIndex) { NotifyNOCUpdatedOnFabric(fabricIndex); }
#endif // CONFIG_IM_BUILD_FOR_UNIT_TEST

FabricInfo * FindFabric(const Crypto::P256PublicKey & rootPubKey, FabricId fabricId);
FabricInfo * FindFabricWithIndex(FabricIndex fabricIndex);
const FabricInfo * FindFabricWithIndex(FabricIndex fabricIndex) const;
Expand Down Expand Up @@ -594,6 +605,8 @@ class DLL_EXPORT FabricTable

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

CHIP_ERROR NotifyNOCUpdatedOnFabric(FabricIndex fabricIndex);

FabricInfo mStates[CHIP_CONFIG_MAX_FABRICS];
PersistentStorageDelegate * mStorage = nullptr;
Crypto::OperationalKeystore * mOperationalKeystore = nullptr;
Expand Down
69 changes: 59 additions & 10 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(this);
}

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,34 @@ 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);
// Below VerifyOrReturnError is not SuccessOrExit since we only want to goto `exit:` after
// Init has been successfully called.
VerifyOrReturnError(fabricTable != nullptr, CHIP_ERROR_INVALID_ARGUMENT);
ReturnErrorOnFailure(Init(sessionManager, policy, delegate, previouslyEstablishedPeer));
tehampson marked this conversation as resolved.
Show resolved Hide resolved

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

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 +250,8 @@ CHIP_ERROR CASESession::EstablishSession(SessionManager & sessionManager, Fabric
// been initialized
SuccessOrExit(err);

SuccessOrExit(err = fabricTable->AddFabricDelegate(this));

mFabricsTable = fabricTable;
mFabricIndex = fabricInfo->GetFabricIndex();
mSessionResumptionStorage = sessionResumptionStorage;
Expand Down Expand Up @@ -251,12 +281,17 @@ void CASESession::OnResponseTimeout(ExchangeContext * ec)
VerifyOrReturn(mExchangeCtxt == ec, ChipLogError(SecureChannel, "CASESession::OnResponseTimeout exchange doesn't match"));
ChipLogError(SecureChannel, "CASESession timed out while waiting for a response from the peer. Current state was %u",
to_underlying(mState));
// Discard the exchange so that Clear() doesn't try closing it. The
// Discard the exchange so that Clear() doesn't try aborting 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 @@ -1692,6 +1727,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
tehampson marked this conversation as resolved.
Show resolved Hide resolved
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 theoretically pass along the 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 @@ -1788,12 +1839,10 @@ CHIP_ERROR CASESession::OnMessageReceived(ExchangeContext * ec, const PayloadHea
// Call delegate to indicate session establishment failure.
if (err != CHIP_NO_ERROR)
{
// Discard the exchange so that Clear() doesn't try closing it. The
// Discard the exchange so that Clear() doesn't try aborting 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
40 changes: 38 additions & 2 deletions src/protocols/secure_channel/CASESession.h
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ namespace chip {
// when implementing concurrent CASE session.
class DLL_EXPORT CASESession : public Messaging::UnsolicitedMessageHandler,
public Messaging::ExchangeDelegate,
public FabricTable::Delegate,
public PairingSession
{
public:
Expand All @@ -73,7 +74,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 +85,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 @@ -166,6 +167,28 @@ class DLL_EXPORT CASESession : public Messaging::UnsolicitedMessageHandler,
//// SessionDelegate ////
void OnSessionReleased() override;

//// FabricTable::Delegate Implementation ////
void OnFabricDeletedFromStorage(FabricTable & fabricTable, FabricIndex fabricIndex) override
{
(void) fabricTable;
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;
InvalidateIfPendingEstablishmentOnFabric(fabricIndex);
}

FabricIndex GetFabricIndex() const { return mFabricIndex; }

// TODO: remove Clear, we should create a new instance instead reset the old instance.
Expand All @@ -174,6 +197,7 @@ class DLL_EXPORT CASESession : public Messaging::UnsolicitedMessageHandler,
void Clear();

private:
friend class CASESessionForTest;
enum class State : uint8_t
{
kInitialized = 0,
Expand Down Expand Up @@ -237,13 +261,21 @@ 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();

CHIP_ERROR ValidateReceivedMessage(Messaging::ExchangeContext * ec, const PayloadHeader & payloadHeader,
const System::PacketBufferHandle & msg);

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

Crypto::Hash_SHA256_stream mCommissioningHash;
Crypto::P256PublicKey mRemotePubKey;
#ifdef ENABLE_HSM_CASE_EPHEMERAL_KEY
Expand Down Expand Up @@ -272,6 +304,10 @@ class DLL_EXPORT CASESession : public Messaging::UnsolicitedMessageHandler,
uint8_t mInitiatorRandom[kSigmaParamRandomNumberSize];

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