Callback chain in Security Manager

[marcuschangarm]

It would be useful if (at least) onLinkSecured could have multiple callbacks, since different modules could depend on knowing the security status of the connection.

[ciarmcom]

ARM Internal Ref: IOTSFW-1395

[andresag01]

@marcuschangarm you mentioned mentioned the onLinkSecured callback, but you also said "at least". What other callbacks in the security manager could benefit from a callchain? Currently, these are the callbacks in SecurityManager:

    SecuritySetupInitiatedCallback_t securitySetupInitiatedCallback;
    SecuritySetupCompletedCallback_t securitySetupCompletedCallback;
    LinkSecuredCallback_t            linkSecuredCallback;
    HandleSpecificEvent_t            securityContextStoredCallback;
    PasskeyDisplayCallback_t         passkeyDisplayCallback;

I would also like to point out that callchains have an associated cost in memory, including too many of these could significantly increase the memory requirements for the SecurityManager, specially in devices with tight memory constrains such as BBC micro:bit and MKit.

[marcuschangarm]

The problem I've been running into is how to make compartmentalized modules that run concurrently. I'm using onLinkSecured to indicate whether a connection is secured. I've have not found use for the other functions yet, hence the 'at least'.

[andresag01]

@rgrover @pan-: Having a second look at this issue, it seems a bit problematic to make the requested changes without modifying the existing API. This is because most of the callbacks in security manager require more than one parameter to be passed while the FunctionPointerWithContext can only handle a single parameter. A work around this is to put all the parameters inside a structure and pass this structure to the callback. Clearly, this requires changes to the API.

[rgrover]

It was an oversight on my part to not define structures for the callbacks in SecurityManager. I believe we should switch to using such structures and also allow chaining of callbacks. This would be a breaking change, but perhaps not too many people from the mbed-classic world would be using security. @pan- do you support this?

[pan-]

I would rather prefer to extend FunctionPointerWithContext and allow it to support multiples parameters instead of making a breaking change.

[marcuschangarm]

Why not remove FunctionPointerWithContext completely and switch to the new FunctionPointer class in core-util?

Maintaining two function pointer classes seems like a waste of resources.

[pan-]

Sure, the problem is that their is no function pointer implementation in mbed classic :(.

[marcuschangarm]

The new function pointers only depend on the standard library. Manually copy-pasting them would still be easier than maintaining a parallel function pointer class.

[andresag01]

If we change the FunctionPointerWithContext for the other function pointers then we are definitely making a very large breaking change.

[marcuschangarm]

A better question is, when are we going to feature freeze mbed classic (and only do bug support) and focus all our effort on mbed OS?

[pan-]

@andresag01

No we don't as long as the function pointer interface has the same "concept".

But it will be require to update the callchain because function pointer from mbed os is just a function pointer and not a function pointer and a node of a linked list.

@marcuschangarm

I agree with everything.