Re: [PATCH 2/2] crypto: ccree: enable support for hardware keys

On 04/03/2018 12:19 PM, Herbert Xu wrote:

On Sat, Mar 31, 2018 at 08:30:46PM +0300, Gilad Ben-Yossef wrote:

quoted

However, as it uses the exact same mechanism of the regular xts-aes-ccree
but takes the key from another source, I've marked it with a test of
alg_test_null() on the premise that if the xts-aes-ccree works, so must this.

Well it appears to be stubbed out as cc_is_hw_key always returns
false.

quoted

quoted

Are there other crypto drivers doing this?

I thought the exact same thing until I ran into a presentation about the s390
secure keys implementation. I basically imitated their use (or abuse?)
of the Crypto API
assuming it is the way to go.

Take a look at arch/s390/crypto/paes_s390.c

It's always nice to discover code that was never reviewed.

The general approach seems sane though.

quoted

Anyway, if this is not the way to go I'd be more than happy to do whatever
work is needed to create the right interface.

PS. I'd be out of the office and away from email access to the coming week, so
kindly excuse any delay in response.

I think it's fine.  However, I don't really like the idea of everyone
coming up with their own "paes", i.e., your patch's use of "haes".
It should be OK to just use paes for everyone, no?

As to your patch specifically, there is one issue where you're
directly dereferencing the key as a struct.  This is a no-no because
the key may have come from user-space.  You must treat it as a
binary blob.  The s390 code seems to do this correctly.

Cheers,

I would be happy to have a generic kernel interface for some kind
of key token as a binary blob. I am also open to use the kernel key
ring for future extensions. But please understand we needed a way
to support our hardware keys and I think the chosen design isn't
so bad.

regards Harald Freudenberger
using the kernel key ring in future extensions.