Re: Crypto driver -DCP

Re: Crypto driver -DCP

[Marek Vasut]

On Thursday, May 28, 2015 at 06:01:56 PM, Jay Monkman wrote:
> Marek,

Hi,

note that this discussion should really happen on the linux-crypto ML.
Moving the discussion to ML.

> I have a question about the proper way to implement a feature. The DCP
> has the ability to read a key for encryption/decryption from on chip
> fuses. The current driver doesn't support this and as far as I can tell,
> there's no clean way to do that within the Linux crypto API.
> 
> Do you have any suggestions on how to handle that?
> 
> My best idea so far is to modify the driver so that if setkey is passed
> a key with a length of 1 byte, the driver uses the on chip key. I'm not
> sure if that would make it into the official kernel.

What about defining CRYPTO_TFM_REQ_CPU_KEY and then notifying the MXS DCP
code to pick the key from CPU by doing something like this in your code:

crypto_ablkcipher_set_flags(tfm, CRYPTO_TFM_REQ_CPU_KEY);

I _think_ this might be the way to go. Do a git grep on CRYPTO_TFM_REQ to
get some usecases on the TFM flags.

Best regards,
Marek Vasut

Re: Crypto driver -DCP

[Marek Vasut]

On Friday, May 29, 2015 at 02:37:00 AM, Herbert Xu wrote:
> On Thu, May 28, 2015 at 11:01:56AM -0500, Jay Monkman wrote:
> > Marek,
> > 
> > I have a question about the proper way to implement a feature. The
> > DCP has the ability to read a key for encryption/decryption from on
> > chip fuses. The current driver doesn't support this and as far as I
> > can tell, there's no clean way to do that within the Linux crypto
> > API.
> > 
> > Do you have any suggestions on how to handle that?
> > 
> > My best idea so far is to modify the driver so that if setkey is
> > passed a key with a length of 1 byte, the driver uses the on chip
> > key. I'm not sure if that would make it into the official kernel.
> 
> What is this key and who is supposed to have access to it?

It's an AES128 key stored in the CPU's OTP registers, so noone ought
to be able to read it out.

Best regards,
Marek Vasut

Re: Crypto driver -DCP

[Herbert Xu]

On Fri, May 29, 2015 at 02:40:54AM +0200, Marek Vasut wrote:
> On Friday, May 29, 2015 at 02:37:00 AM, Herbert Xu wrote:
> > On Thu, May 28, 2015 at 11:01:56AM -0500, Jay Monkman wrote:
> > > Marek,
> > > 
> > > I have a question about the proper way to implement a feature. The
> > > DCP has the ability to read a key for encryption/decryption from on
> > > chip fuses. The current driver doesn't support this and as far as I
> > > can tell, there's no clean way to do that within the Linux crypto
> > > API.
> > > 
> > > Do you have any suggestions on how to handle that?
> > > 
> > > My best idea so far is to modify the driver so that if setkey is
> > > passed a key with a length of 1 byte, the driver uses the on chip
> > > key. I'm not sure if that would make it into the official kernel.
> > 
> > What is this key and who is supposed to have access to it?
> 
> It's an AES128 key stored in the CPU's OTP registers, so noone ought
> to be able to read it out.

No I mean who is supposed to have access to it? Everyone?

Cheers,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: Crypto driver -DCP

[Marek Vasut]

On Friday, May 29, 2015 at 02:45:16 AM, Herbert Xu wrote:
> On Fri, May 29, 2015 at 02:40:54AM +0200, Marek Vasut wrote:
> > On Friday, May 29, 2015 at 02:37:00 AM, Herbert Xu wrote:
> > > On Thu, May 28, 2015 at 11:01:56AM -0500, Jay Monkman wrote:
> > > > Marek,
> > > > 
> > > > I have a question about the proper way to implement a feature. The
> > > > DCP has the ability to read a key for encryption/decryption from on
> > > > chip fuses. The current driver doesn't support this and as far as I
> > > > can tell, there's no clean way to do that within the Linux crypto
> > > > API.
> > > > 
> > > > Do you have any suggestions on how to handle that?
> > > > 
> > > > My best idea so far is to modify the driver so that if setkey is
> > > > passed a key with a length of 1 byte, the driver uses the on chip
> > > > key. I'm not sure if that would make it into the official kernel.
> > > 
> > > What is this key and who is supposed to have access to it?
> > 
> > It's an AES128 key stored in the CPU's OTP registers, so noone ought
> > to be able to read it out.
> 
> No I mean who is supposed to have access to it? Everyone?

My understanding (!) is that everyone should be able to use this key to
encrypt/decrypt their data using the AES128 engine in the DCP.

Best regards,
Marek Vasut

Re: Crypto driver -DCP

[Herbert Xu]

On Fri, May 29, 2015 at 03:00:35AM +0200, Marek Vasut wrote:
>
> My understanding (!) is that everyone should be able to use this key to
> encrypt/decrypt their data using the AES128 engine in the DCP.

If that's the case just make it the key you use if setkey has never
been called.

Cheers,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: Crypto driver -DCP

[Marek Vasut]

On Friday, May 29, 2015 at 03:23:59 AM, Herbert Xu wrote:
> On Fri, May 29, 2015 at 03:00:35AM +0200, Marek Vasut wrote:
> > My understanding (!) is that everyone should be able to use this key to
> > encrypt/decrypt their data using the AES128 engine in the DCP.
> 
> If that's the case just make it the key you use if setkey has never
> been called.

Shouldn't you always call setkey when using symetric crypto ?

Best regards,
Marek Vasut

Re: Crypto driver -DCP

[Herbert Xu]

On Fri, May 29, 2015 at 03:29:59AM +0200, Marek Vasut wrote:
> On Friday, May 29, 2015 at 03:23:59 AM, Herbert Xu wrote:
> > On Fri, May 29, 2015 at 03:00:35AM +0200, Marek Vasut wrote:
> > > My understanding (!) is that everyone should be able to use this key to
> > > encrypt/decrypt their data using the AES128 engine in the DCP.
> > 
> > If that's the case just make it the key you use if setkey has never
> > been called.
> 
> Shouldn't you always call setkey when using symetric crypto ?

Normally yes but we don't require it so in this case it should
be fine.

Cheers,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: Crypto driver -DCP

[Marek Vasut]

On Friday, May 29, 2015 at 03:32:11 AM, Herbert Xu wrote:
> On Fri, May 29, 2015 at 03:29:59AM +0200, Marek Vasut wrote:
> > On Friday, May 29, 2015 at 03:23:59 AM, Herbert Xu wrote:
> > > On Fri, May 29, 2015 at 03:00:35AM +0200, Marek Vasut wrote:
> > > > My understanding (!) is that everyone should be able to use this key
> > > > to encrypt/decrypt their data using the AES128 engine in the DCP.
> > > 
> > > If that's the case just make it the key you use if setkey has never
> > > been called.
> > 
> > Shouldn't you always call setkey when using symetric crypto ?
> 
> Normally yes but we don't require it so in this case it should
> be fine.

This does look somewhat hacky to me. Wouldn't it make more sense to
add a CRYPTO_TFM_REQ flag ?

Best regards,
Marek Vasut

Re: Crypto driver -DCP

[Herbert Xu]

On Fri, May 29, 2015 at 03:02:59PM +0200, Marek Vasut wrote:
>
> This does look somewhat hacky to me. Wouldn't it make more sense to
> add a CRYPTO_TFM_REQ flag ?

What are you going to do when you have more than one hard-coded
key? A flag is a non-starter.

Cheers,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: Crypto driver -DCP

[Marek Vasut]

On Friday, May 29, 2015 at 03:30:18 PM, Herbert Xu wrote:
> On Fri, May 29, 2015 at 03:02:59PM +0200, Marek Vasut wrote:
> > This does look somewhat hacky to me. Wouldn't it make more sense to
> > add a CRYPTO_TFM_REQ flag ?
> 
> What are you going to do when you have more than one hard-coded
> key? A flag is a non-starter.

Well, abusing a non-well-defined part of the API is also not a nice
way to do it. Maybe we should consider some other -- third -- option ?

Best regards,
Marek Vasut

Re: Crypto driver -DCP

[Herbert Xu]

On Mon, Jun 01, 2015 at 03:24:03PM +0200, Marek Vasut wrote:
> On Friday, May 29, 2015 at 03:30:18 PM, Herbert Xu wrote:
> > On Fri, May 29, 2015 at 03:02:59PM +0200, Marek Vasut wrote:
> > > This does look somewhat hacky to me. Wouldn't it make more sense to
> > > add a CRYPTO_TFM_REQ flag ?
> > 
> > What are you going to do when you have more than one hard-coded
> > key? A flag is a non-starter.
> 
> Well, abusing a non-well-defined part of the API is also not a nice
> way to do it. Maybe we should consider some other -- third -- option ?

I'm waiting to hear it :)
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: Crypto driver -DCP

[Jay Monkman]

On 05/28/2015 08:32 PM, Herbert Xu wrote:
> On Fri, May 29, 2015 at 03:29:59AM +0200, Marek Vasut wrote:
>> On Friday, May 29, 2015 at 03:23:59 AM, Herbert Xu wrote:
>>> On Fri, May 29, 2015 at 03:00:35AM +0200, Marek Vasut wrote:
>>>> My understanding (!) is that everyone should be able to use this key to
>>>> encrypt/decrypt their data using the AES128 engine in the DCP.
>>> If that's the case just make it the key you use if setkey has never
>>> been called.
>> Shouldn't you always call setkey when using symetric crypto ?
> Normally yes but we don't require it so in this case it should
> be fine.
>
> Cheers,

Okay, that seems obvious after you suggested it.

I have another question. The DCP (and other crypto accelerators on other 
SOCs) supports key slots - basically write only RAM that's used to store 
keys so they can be used for encrypt/decrypt operations. DCP supports 4 
key slots, other devices have different numbers. Do you have any 
suggestion for how to add support for something like that to the driver?

Thanks.

Re: Crypto driver -DCP

[Herbert Xu]

On Tue, Jun 02, 2015 at 01:57:28PM -0500, Jay Monkman wrote:
>
> I have another question. The DCP (and other crypto accelerators on
> other SOCs) supports key slots - basically write only RAM that's
> used to store keys so they can be used for encrypt/decrypt
> operations. DCP supports 4 key slots, other devices have different
> numbers. Do you have any suggestion for how to add support for
> something like that to the driver?

So these would allow faster switching of keys I presume?

If so that's a resource the driver should manage.  The simplest
would be some kind of LRU scheme where you expire the least recently
used key.

Cheers,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: Crypto driver -DCP

[Marek Vasut]

On Monday, June 01, 2015 at 04:50:15 PM, Herbert Xu wrote:
> On Mon, Jun 01, 2015 at 03:24:03PM +0200, Marek Vasut wrote:
> > On Friday, May 29, 2015 at 03:30:18 PM, Herbert Xu wrote:
> > > On Fri, May 29, 2015 at 03:02:59PM +0200, Marek Vasut wrote:
> > > > This does look somewhat hacky to me. Wouldn't it make more sense to
> > > > add a CRYPTO_TFM_REQ flag ?
> > > 
> > > What are you going to do when you have more than one hard-coded
> > > key? A flag is a non-starter.
> > 
> > Well, abusing a non-well-defined part of the API is also not a nice
> > way to do it. Maybe we should consider some other -- third -- option ?
> 
> I'm waiting to hear it :)

I cannot tell, I wasn't digging in the API for a while now, so I forgot
the details. Maybe someone else can chime in ?

Best regards,
Marek Vasut

Re: Crypto driver -DCP

[Jay Monkman]

On 06/02/2015 09:11 PM, Herbert Xu wrote:
> On Tue, Jun 02, 2015 at 01:57:28PM -0500, Jay Monkman wrote:
>>
>> I have another question. The DCP (and other crypto accelerators on
>> other SOCs) supports key slots - basically write only RAM that's
>> used to store keys so they can be used for encrypt/decrypt
>> operations. DCP supports 4 key slots, other devices have different
>> numbers. Do you have any suggestion for how to add support for
>> something like that to the driver?
>
> So these would allow faster switching of keys I presume?

That would be one use, but a more likely use would be to prevent access 
to the keys. A system could write keys to the key slots in the 
bootloader or in a TrustZone secure world. Then those keys could be used 
for crypto operations in Linux without ever exposing them. Key slots can 
be written to, but cannot be read from.

Even with keys stored in key slots, other keys may be used. For example, 
someone could do:
     operation w/ key in slot 1
     operation w/ key provided in descriptor
     operation w/ key in slot 1

I don't think an LRU scheme would allow something like that.

Re: Crypto driver -DCP

[Herbert Xu]

On Wed, Jun 03, 2015 at 03:02:13PM -0500, Jay Monkman wrote:
>
> That would be one use, but a more likely use would be to prevent
> access to the keys. A system could write keys to the key slots in
> the bootloader or in a TrustZone secure world. Then those keys could
> be used for crypto operations in Linux without ever exposing them.
> Key slots can be written to, but cannot be read from.
> 
> Even with keys stored in key slots, other keys may be used. For
> example, someone could do:
>     operation w/ key in slot 1
>     operation w/ key provided in descriptor
>     operation w/ key in slot 1
> 
> I don't think an LRU scheme would allow something like that.

In that case I would suggest using setkey with a length other
than that of a valid AES key.  For example, you could use a one-
byte value to select the key slot.

Cheers,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: Crypto driver -DCP

[Marek Vasut]

On Thursday, June 04, 2015 at 05:24:00 AM, Herbert Xu wrote:
> On Wed, Jun 03, 2015 at 03:02:13PM -0500, Jay Monkman wrote:
> > That would be one use, but a more likely use would be to prevent
> > access to the keys. A system could write keys to the key slots in
> > the bootloader or in a TrustZone secure world. Then those keys could
> > be used for crypto operations in Linux without ever exposing them.
> > Key slots can be written to, but cannot be read from.
> > 
> > Even with keys stored in key slots, other keys may be used. For
> > 
> > example, someone could do:
> >     operation w/ key in slot 1
> >     operation w/ key provided in descriptor
> >     operation w/ key in slot 1
> > 
> > I don't think an LRU scheme would allow something like that.
> 
> In that case I would suggest using setkey with a length other
> than that of a valid AES key.  For example, you could use a one-
> byte value to select the key slot.

Is this really a valid way to go about crypto -- introduce all kinds
of obscure nuances into the API which are driver specific at best ?

Best regards,
Marek Vasut

Re: Crypto driver -DCP

[Herbert Xu]

On Thu, Jun 04, 2015 at 05:34:39PM +0200, Marek Vasut wrote:
> 
> Is this really a valid way to go about crypto -- introduce all kinds
> of obscure nuances into the API which are driver specific at best ?

So what do you suggest?
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: Crypto driver -DCP

[Marek Vasut]

On Friday, June 05, 2015 at 05:54:33 AM, Herbert Xu wrote:
> On Thu, Jun 04, 2015 at 05:34:39PM +0200, Marek Vasut wrote:
> > Is this really a valid way to go about crypto -- introduce all kinds
> > of obscure nuances into the API which are driver specific at best ?
> 
> So what do you suggest?

Well what about extending the API ?

Since Jay seems to be looking at the keyslots as well, it might make sense
to consider the "DCP key" as a special case of a "fixed" keyslot.

In general, it would probably make sense to add a flag to .setkey() to
store the key in a keyslot. The keyslot allocation would be up to the
driver. In case all keyslots would be full, the setkey() with the flag
set would simply fail. This would imply you would need to have a counterpart
function to .setkey() to free keyslots -- something like .unsetkey() .

The .setkey() could also be extended with one more argument, integer one,
to let users select which fixed keyslot to load the key from. Wouldn't
that resolve the issue with multiple fixed keyslots too ?

Best regards,
Marek Vasut

Re: Crypto driver -DCP

[Herbert Xu]

On Fri, Jun 05, 2015 at 04:38:03PM +0200, Marek Vasut wrote:
> 
> In general, it would probably make sense to add a flag to .setkey() to
> store the key in a keyslot. The keyslot allocation would be up to the
> driver. In case all keyslots would be full, the setkey() with the flag
> set would simply fail. This would imply you would need to have a counterpart
> function to .setkey() to free keyslots -- something like .unsetkey() .

Changing setkey is going to cause too much churn.  In any case
I don't think these key slots should be written to by the kernel
since the intention appears to be for entites outside the kernel
to place secret keys in there that can then be used but not read
by the kernel.

So as far as the kernel is concerned these are constant keys.

Therefore there should be no need for unsetkey.

So perhaps just add a new call setkey_slot that is optional and
only needs to be implemented by drivers such as ccp.

Cheers,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: Crypto driver -DCP

[Marek Vasut]

On Monday, June 08, 2015 at 06:52:00 AM, Herbert Xu wrote:
> On Fri, Jun 05, 2015 at 04:38:03PM +0200, Marek Vasut wrote:
> > In general, it would probably make sense to add a flag to .setkey() to
> > store the key in a keyslot. The keyslot allocation would be up to the
> > driver. In case all keyslots would be full, the setkey() with the flag
> > set would simply fail. This would imply you would need to have a
> > counterpart function to .setkey() to free keyslots -- something like
> > .unsetkey() .
> 
> Changing setkey is going to cause too much churn.  In any case
> I don't think these key slots should be written to by the kernel
> since the intention appears to be for entites outside the kernel
> to place secret keys in there that can then be used but not read
> by the kernel.

You mean like bootloader or you mean userspace code ? Maybe Jay
can explain how these slots are intended to be used. I'd like to
know the expected usecase.

> So as far as the kernel is concerned these are constant keys.
> 
> Therefore there should be no need for unsetkey.
> 
> So perhaps just add a new call setkey_slot that is optional and
> only needs to be implemented by drivers such as ccp.

Might work as well ... but let's maybe wait for the usecase.

Best regards,
Marek Vasut