[BUG] DefaultSessionResumptionStorage::Save does not reuse index locations to save data for the same node

[msandstedt]

Reproduction steps

DefaultSessionResumptionStorage::Save does not reuse index locations to save data for the same node. This has a few consequences:

1. Each subsequent session resumption write for a given node will unnecessarily evict entries for other nodes.
2. Backing storage in the state table won't ever have more than one entry for the given node. This leaves it out of sync with the index.
3. Most seriously, entries in the link table are leaked at each occurrence.

The leak is specifically here:

connectedhomeip/src/protocols/secure_channel/DefaultSessionResumptionStorage.cpp

Line 62 in 7301ecf

ReturnErrorOnFailure(SaveState(node, resumptionId, sharedSecret, peerCATs));

Note that the fact Delete always returns CHIP_NO_ERROR keeps eviction working. But the leak remains and is serious.

Bug prevalence

Always

GitHub hash of the SDK that was being used

fa8db56

Platform

core

Platform Version(s)

No response

Anything else?

No response

[msandstedt]

Oops, I think we may also be leaking storage here:

connectedhomeip/src/protocols/secure_channel/DefaultSessionResumptionStorage.cpp

Line 62 in 7301ecf

ReturnErrorOnFailure(SaveState(node, resumptionId, sharedSecret, peerCATs));

The link path is /g/s/<resumptionId>. If we delete the state entry first, the link entry is orphaned. That's pretty serious.

[bzbarsky-apple]

We definitely leak g/s/* entries, in our testing....

[msandstedt]

Yup. We do. This will slowly kill nodes.