Skip to content

Commit

Permalink
Async send sigma3 (#25695)
Browse files Browse the repository at this point in the history
* Break CASESession::SendSigma3 into fg/bg parts

This unblocks the main event loop while performance intensive
(e.g. crypto) parts of the process occur.

* Fix host tests

* Remove temp log statements

* Restyle

* Refactor CASESession::SendSigma3

Add more structure to manage multiple steps of work.

* Remove temporary logging

* Restyle

* Minor cleanup

* Minor cleanup

* Restyle

* Use Platform::SharedPtr

Also add alias template for Platform::WeakPtr.

* Add mutex to FabricTable

This supports locking during SignWithOpKeypair, and other operations
that may alter state in the fabric table, while CASESession is
performing work in the background during session establishment.

CASESession registers as a fabric table listener, and when a fabric
is removed (e.g. timeout) it attempts to cancel any outstanding work,
and also clears out the fabric index in the work helper data.

Therefore, if outstanding work has made it into SignWithOpKeypair,
it should be OK until complete.

It still relies on other tasks not altering FabricInfo, or the
configured OperationalKeystore, but that would have had to have been
true before anyways.

The mutex was not made recursive. It's omitted from a few functions,
which should be OK for now, and there should be cleanup on a subsequent
commit (and probably fix up const-ness of member functions, and
factoring of API vs. impl functions). This commit is to flush out
build/test errors on all CI platforms, and to discuss/review/comment on
the general approach.

* Remove mutex, only async sometimes

It's tricky to async background the signing operation, because of the
two ways operational signing occurs.

Legacy way: opkeypair manually added for fabric info
Recommended way: opkeystore handles everything

Removed std::mutex because it wasn't supported by all platforms.

Instead, made background signing occur only if using the operational
keystore (recommended way), since implementors can perform any needed
mutual exclusion in the operational keystore. If using manually added
operational keypairs (legacy way), keep signing in the foreground,
since it's not feasible to mutex the entire fabric table and typically
the operations is simpler anyways.

* Clean up error handling

* Restyle

* Only store data.fabricTable if fg case

Store only one of data.fabricTable or data.keystore.

* Declare wither signing in background is supported

OperationalKeystore declares whether it supports this capability.
If so, then CASE session establishment may take advantage of it.
If not, then CASE session establishment must use foreground.

* Make some variables const

* Clean up a few comments
  • Loading branch information
mlepage-google authored and pull[bot] committed Jul 6, 2023
1 parent f741939 commit 2707111
Show file tree
Hide file tree
Showing 6 changed files with 385 additions and 118 deletions.
8 changes: 8 additions & 0 deletions src/credentials/FabricTable.h
Original file line number Diff line number Diff line change
Expand Up @@ -717,6 +717,14 @@ class DLL_EXPORT FabricTable
*/
bool HasOperationalKeyForFabric(FabricIndex fabricIndex) const;

/**
* @brief Returns the operational keystore. This is used for
* CASE and the only way the keystore should be used.
*
* @return The operational keystore, nullptr otherwise.
*/
const Crypto::OperationalKeystore * GetOperationalKeystore() { return mOperationalKeystore; }

/**
* @brief Add a pending trusted root certificate for the next fabric created with `AddNewPendingFabric*` methods.
*
Expand Down
14 changes: 13 additions & 1 deletion src/crypto/OperationalKeystore.h
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,19 @@ class OperationalKeystore
virtual void RevertPendingKeypair() = 0;

// ==== Primary operation required: signature
/**
* @brief Whether `SignWithOpKeypair` may be performed in the background.
*
* If true, `CASESession` may attempt to perform `SignWithOpKeypair` in the
* background. In this case, `OperationalKeystore` should protect itself,
* e.g. with a mutex, as the signing could occur at any time during session
* establishment.
*
* @retval true if `SignWithOpKeypair` may be performed in the background
* @retval false if `SignWithOpKeypair` may NOT be performed in the background
*/
virtual bool SupportsSignWithOpKeypairInBackground() const { return false; }

/**
* @brief Sign a message with a fabric's currently-active operational keypair.
*
Expand All @@ -164,7 +177,6 @@ class OperationalKeystore
* @retval CHIP_ERROR_INVALID_FABRIC_INDEX if no active key is found for the given `fabricIndex` or if
* `fabricIndex` is invalid.
* @retval other CHIP_ERROR value on internal crypto engine errors
*
*/
virtual CHIP_ERROR SignWithOpKeypair(FabricIndex fabricIndex, const ByteSpan & message,
Crypto::P256ECDSASignature & outSignature) const = 0;
Expand Down
3 changes: 3 additions & 0 deletions src/lib/support/CHIPMem.h
Original file line number Diff line number Diff line change
Expand Up @@ -193,6 +193,9 @@ inline SharedPtr<T> MakeShared(Args &&... args)
return SharedPtr<T>(New<T>(std::forward<Args>(args)...), Deleter<T>());
}

template <typename T>
using WeakPtr = std::weak_ptr<T>;

// See MemoryDebugCheckPointer().
extern bool MemoryInternalCheckPointer(const void * p, size_t min_size);

Expand Down
Loading

0 comments on commit 2707111

Please sign in to comment.