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 7 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 @@ -458,6 +458,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 @@ -371,6 +371,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 @@ -199,6 +199,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
15 changes: 14 additions & 1 deletion src/credentials/FabricTable.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -738,6 +738,16 @@ 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);

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;
}

Expand Down Expand Up @@ -848,8 +858,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
5 changes: 5 additions & 0 deletions src/credentials/FabricTable.h
Original file line number Diff line number Diff line change
Expand Up @@ -363,6 +363,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
46 changes: 35 additions & 11 deletions src/protocols/secure_channel/CASESession.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -144,12 +144,26 @@ void CASESession::Clear()
mState = State::kInitialized;
Crypto::ClearSecretData(mIPK);

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

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

void CASESession::InvalidateIfPendingEstablishment()
{
if (!IsSessionEstablishmentInProgress())
{
return;
}
AbortPendingEstablish(CHIP_ERROR_CANCELLED);
}

CHIP_ERROR CASESession::Init(SessionManager & sessionManager, Credentials::CertificateValidityPolicy * policy,
SessionEstablishmentDelegate * delegate)
{
Expand All @@ -172,23 +186,31 @@ CHIP_ERROR CASESession::Init(SessionManager & sessionManager, Credentials::Certi
}

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

CHIP_ERROR err = CHIP_NO_ERROR;
SuccessOrExit(err = fabricTable->AddFabricDelegate(&mFabricDelegate));
tehampson marked this conversation as resolved.
Show resolved Hide resolved

mRole = CryptoContext::SessionRole::kResponder;
mFabricsTable = fabrics;
mFabricsTable = fabricTable;
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, FabricIndex fabricIndex,
Expand Down Expand Up @@ -219,6 +241,8 @@ CHIP_ERROR CASESession::EstablishSession(SessionManager & sessionManager, Fabric
// been initialized
SuccessOrExit(err);

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

mFabricsTable = fabricTable;
mFabricIndex = fabricIndex;
mSessionResumptionStorage = sessionResumptionStorage;
Expand Down Expand Up @@ -248,12 +272,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));
AbortPendingEstablish(CHIP_ERROR_TIMEOUT);
}

void CASESession::AbortPendingEstablish(CHIP_ERROR err)
{
// 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(CHIP_ERROR_TIMEOUT);
mDelegate->OnSessionEstablishmentError(err);
}

CHIP_ERROR CASESession::DeriveSecureSession(CryptoContext & session) const
Expand Down Expand Up @@ -1758,12 +1787,7 @@ 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
// exchange will handle that.
DiscardExchange();
Clear();
// Do this last in case the delegate frees us.
mDelegate->OnSessionEstablishmentError(err);
AbortPendingEstablish(err);
}
return err;
}
Expand Down
44 changes: 42 additions & 2 deletions src/protocols/secure_channel/CASESession.h
Original file line number Diff line number Diff line change
Expand Up @@ -72,14 +72,14 @@ 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
*
* @return CHIP_ERROR The result of initialization
*/
CHIP_ERROR ListenForSessionEstablishment(
SessionManager & sessionManager, FabricTable * fabrics, SessionResumptionStorage * sessionResumptionStorage,
SessionManager & sessionManager, FabricTable * fabricTable, SessionResumptionStorage * sessionResumptionStorage,
Credentials::CertificateValidityPolicy * policy, SessionEstablishmentDelegate * delegate,
Optional<ReliableMessageProtocolConfig> mrpConfig = Optional<ReliableMessageProtocolConfig>::Missing());

Expand Down Expand Up @@ -168,7 +168,44 @@ class DLL_EXPORT CASESession : public Messaging::UnsolicitedMessageHandler,
**/
void Clear();

void InvalidateIfPendingEstablishment();

private:
class CASESessionFabricDelegate final : public chip::FabricTable::Delegate
tehampson marked this conversation as resolved.
Show resolved Hide resolved
{
public:
CASESessionFabricDelegate(CASESession * caseSession) : mCASESession(caseSession) {}

void OnFabricDeletedFromStorage(FabricTable & fabricTable, FabricIndex fabricIndex) override
{
(void) fabricTable;
(void) fabricIndex;
mCASESession->InvalidateIfPendingEstablishment();
tehampson marked this conversation as resolved.
Show resolved Hide resolved
}

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;
(void) fabricIndex;
mCASESession->InvalidateIfPendingEstablishment();
}

private:
CASESession * mCASESession;
};

enum class State : uint8_t
{
kInitialized = 0,
Expand Down Expand Up @@ -223,6 +260,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 @@ -257,6 +296,7 @@ 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;
};

Expand Down
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 @@ -169,6 +169,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