Hi Jens,

On Thu, 7 Mar 2019 at 11:21, Jens Wiklander <jens.wiklander@linaro.org> wrote:
Hi,

We have an ongoing action to integrate mbedtls in OP-TEE to be used in
OP-TEE core via the <crypto/crypto.h> interface.

Then there's also a library developed by NXP to take advantage of CAAM
for crypto acceleration.

The way we're dealing with different crypto algorithms today isn't
very modular. I think we can disregard the asymmetric algorithms for
now. We've already put the algorithms in different groups, hashes,
macs, ciphers (which includes modes like CBC, CTR etc), and authenc
(authenticated encryption). This was good enough for our initial
libtomcrypt integration, but I think it has been a bit painful for the
ongoing mbedtls integration. With CAAM it was solved by using an
entire new wrapper layer to work with libtomcrypt. Cedric has shown
benchmark that the new wrapping didn't cause any noticeable
degradation in performance.

To try a more fine grained modular approach I've converted the authenc
algorithms to use a struct crypto_authenc_ops internally to call the
needed functions [1].

Why use pointer to functions? We don't need to dynamically select an implementation at runtime, so we could use a simple "by name" function interface instead I suppose. Basically, make crypto_aes_ccm_init(), crypto_aes_ccm_update_aad(), etc. global functions. We already removed some crypto_ops structure in the past:
 
This makes it easier to replace one
implementation with another, it's just a matter of calling the correct
crypto_aes_gcm_alloc_ctx() function for AES-GCM and the rest of the
implementation will be selected automatically. The different functions
allocating an AEC-GCM context should probably have different names
instead of using the same as in [1], but that doesn't have to be
decided right now.

On the contrary, I think a single name is fine and probably easier to work with (for instance, by searching for the symbol name in vim you get all the possible implementations). Like C++ overloading if you want ;-)
 

With the approach in [1] applied on hashes, macs and ciphers too I
think it would be much easier to integrate any crypto library.
For paging we'll still have special interfaces to do SHA-256 or
AES-GCM, this is needed in order minimize the dependency graph for
unpaged code and also make sure that it's always callable from abort
mode (doesn't cause deadlock or whatever).

What do you think? Is this too much, too little?

A finer-grained interface certainly is a good idea. I agree it will likely help reduce the size and complexity of wrappers around other implementations.

Cheers,
-- 
Jerome
 

Cheers,
Jens

[1] https://github.com/OP-TEE/optee_os/pull/2862