Need a solution to get correct generated SecureSessionHandle for each new connection

[yufengwangca]

Problem

I found #3728 broke the end-to-end messaging testing which is temporarily disabled in CI.

With this change, the app does not specify the target NodeID, but relay on the following callback to update the SecureSessionHandle.

void OnNewConnection(chip::SecureSessionHandle session, chip::SecureSessionMgr * mgr) override {
mSecureSession = session;
}
We can only specify one delegate of SecureSessionMgr at a time, but there are multiple connections managed by the SecureSessionMgr, how we make sure the SecureSessionMgr can pass the generated SecureSessionHandle to the corresponding target of its connection?

For example, there are multiple objects are set as the delegate of SecureSessionMgr, including DeviceController, ExchangeManager and application itself. The later SetDelegate() will overwrite the previous one.

Echo Application:
gSessionManager.SetDelegate(&gTestSecureSessionMgrDelegate); // Set the echo app as the delegate of SecureSessionMgr.
err = gExchangeManager.Init(&gSessionManager); // Set the ExchangeManager as the delegate of SecureSessionMgr.

The echo application won't received the generated SecureSessionHandle and it will sendMessage to garbage target.

[kghost]

For server side

The application will receive an ExchangeContext from ExchangeDelegate::OnMessageReceived, then a session is accessible via ExchangeContext::GetSecureSession

For client side

The protocols will take either a Channel or an ExchangeContext as the construct argument instead of a sessoin, both of them have a session embedded.

[yufengwangca]

The client does not know the keyid of SecureSessionHandle at the first place, how to get it before calling sendMessage? will this get addressed in your channel PR?

[kghost]

Yes, the session is derived from OnNewConnection, the client doesn't need to care about the key is.

But before that, I think you temporary fix is fine.

[yufengwangca]

That is exactly issue I hit, there are multiple instances get set as the delegate of SecureSessionMgr, only the last one get OnNewConnection called

[woody-apple]

@pan-apple can you weigh in here?

[yufengwangca]

@pan-apple @andreilitvin @bzbarsky-apple @erjiaqing

Hi Mingjie,

It seems there is no easy solution for this issue, I just did some background study and try to understand why we have this problem at the first place and how Weave handle this case.

It seems the motivation to introduce SessionHanlde is we believe it is not enough to use PeerNodeId as key to find PeerConnectionState. I am thinking of the scenarios we might have the confusion.

1. Can't distinguish PASE and CASE connections to the same peer node
   PASE and CASE are never active at the same time, that is not an issue.

2. Can't distinguish Unicast/Multicast connections to the same peer node.
   GroupID a NodeID are within different address domain, we should be able to identify this by just looking at the Peer node ID. In worst case, we can use a different wrapper SendGroupMessage API in the secure session manager to find the PeerConnectionState for multicast message.

Since weave does not need to use keyID to identify the connection, I would like to understand what requirements make CHIP different than Weave in this case. If we could use the solution from Weave, then all issues are going away.

[kghost]

@yufengwangca

Currently we are using PeerNodeId and PeerKeyId to identify a session, but it is only a temporary solution. In the future, we will associate metadata to each session.

#4019 will be the solution to this problem. A secure session is only obtainable via a Channel object. Associated metadata will be stored inside the Channel and accessible to applications during its lifespan. It also requires an API to persistent the session key and associated metadata, we can provide an API to return a serializable POD object contains all information to be persistent, and let application to handle the actual storage.