Re: [PATCH] crypto: implement DH primitives under akcipher API

[Marcel Holtmann]

Hi Tadeusz,

>>> In this way we can define a generic user side of the key exchange interface,
 and on the the driver side of the akcipher, the implementations would 
 overload
 the existing akcipher encrypt(), decrypt(), set_pub_key(), set_priv_key() 
 methods
 and do what should be done for a given algorithm. We just need to agree on 
 the
 format of the input parameters to these operations.
>> I think trying to heavily map this to encrypt and decrypt is a bad idea. 
>> These callbacks are not really designed to return what you need them to 
>> return. Key exchange is different.
>> 
>> So why are we trying to push this into akcipher in the first place? I mean 
>> lets just create a keyexch (or similar name) and design it explicitly for 
>> key exchange handling. We are also not trying to map hashes into skcipher. I 
>> think the clear distinction of semantics calls for a different API.
>> 
> 
> I just think that before introducing new API we should try to reuse what's 
> already
> there. Looking at RSA and DH, they basically perform x = a^b mod p, so we 
> should
> have what's needed for both and we just could overload the methods already 
> defined.
> I'm fine either way. Herbert, what's your preference?

I do not think that it is a valid argument because the operations are the same 
to use akcipher. While operations are similar, the inputs are totally 
different. And it is really not performing encrypt or decrypt operations here. 
You use different inputs and get different outputs. You are not encrypting or 
decrypting anything. Especially key generation via decrypt is something I can 
not wrap my mind around.

>>> Agree, we need to support all the cases, but dealing with different type of 
>>> keys
 needs to happen above this API. This API should solely be an interface to 
 math
 primitives of certain operations, which can be implemented in SW or can be 
 HW
 accelerated. Modules like public_key or afalg_akcipher need to be smart 
 enough
 and know what key types they deal with and do the right thing depending on 
 that
 knowledge.
>> I am not sure this can be that easily generalized. Just pushing the problem 
>> of key types and formats off to userspace just forces complexity and extra 
>> knowledge there. We need to be really carefully here. And I really dislike 
>> introducing custom ASN.1 structures that are not backed by an actual RFC 
>> document.
> 
> I'm not saying that we should push this to user space. I'm just saying this 
> should
> be handled above the crypto API.
> 
>>> Agree, and to make this work with afalg_akcipher, I have proposed new
 ALG_SET_KEY_ID and ALG_SET_KEY_ID operations. See here: 
 https://lkml.org/lkml/2015/12/26/65
 The plan is that in this case the afalg_akcipher will not use akcipher api 
 at all.
 In this case the algif_akcipher will use the request_key() interface to 
 retrieve
 the key type info and will use the operations that the new TPM interface 
 will define.
>> The ALG_SET_KEY_ID is great for skcipher where you key is a fixed size blob. 
>> There is works really well. And you will need if we let keyctl derive the 
>> session key from a set of asymmetric keys.
>> 
>> I get that we want to enable OpenSSL and others to gain access to kernel 
>> crypto operations. We want the same actually for ELL as well. However what 
>> is dangerous here is that AF_ALG can never support hardware based keys. So 
>> that means support in OpenSSL is broken by design. See earlier emails from 
>> David Woodhouse. If you have your certificates / keys in hardware then you 
>> need to be able to make them work with OpenSSL. We can not ignore this. It 
>> is a design we need to consider from the beginning.
> 
> If we will have the TPM interface that allows to invoke functions for HW keys 
> and
> the *_KEY_ID, then I don't see why AF_ALG would not work. As I said, in case 
> when
> the keys will be in HW the algif_akcipher will not use akcipher api, but use 
> the
> TPM defined functions in the same way as the keyctl would use it.
> So I think both AF_ALG and keyctl should work just fine.

When David and I talked to Herbert at the kernel summit in Seoul, he said that 
the crypto subsystem (and with that AF_ALG) always needs to support software 
fallback crypto. In case of hardware based keys, there is no software fallback 
possible. If you have a hardware key, then it always needs to use the hardware 
crypto that comes with the key.

And that is precisely my point in going forward with keyctl for all of these. 
We need _one_ interface that can handle software and hardware keys / key pairs.

Regards

Marcel

--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] crypto: implement DH primitives under akcipher API

[Tadeusz Struk]

Hi Marcel,
On 03/07/2016 02:29 PM, Marcel Holtmann wrote:
>> In this way we can define a generic user side of the key exchange interface,
>> > and on the the driver side of the akcipher, the implementations would 
>> > overload
>> > the existing akcipher encrypt(), decrypt(), set_pub_key(), set_priv_key() 
>> > methods
>> > and do what should be done for a given algorithm. We just need to agree on 
>> > the
>> > format of the input parameters to these operations.
> I think trying to heavily map this to encrypt and decrypt is a bad idea. 
> These callbacks are not really designed to return what you need them to 
> return. Key exchange is different.
> 
> So why are we trying to push this into akcipher in the first place? I mean 
> lets just create a keyexch (or similar name) and design it explicitly for key 
> exchange handling. We are also not trying to map hashes into skcipher. I 
> think the clear distinction of semantics calls for a different API.
> 

I just think that before introducing new API we should try to reuse what's 
already
there. Looking at RSA and DH, they basically perform x = a^b mod p, so we should
have what's needed for both and we just could overload the methods already 
defined.
I'm fine either way. Herbert, what's your preference?

>> Agree, we need to support all the cases, but dealing with different type of 
>> keys
>> > needs to happen above this API. This API should solely be an interface to 
>> > math
>> > primitives of certain operations, which can be implemented in SW or can be 
>> > HW
>> > accelerated. Modules like public_key or afalg_akcipher need to be smart 
>> > enough
>> > and know what key types they deal with and do the right thing depending on 
>> > that
>> > knowledge.
> I am not sure this can be that easily generalized. Just pushing the problem 
> of key types and formats off to userspace just forces complexity and extra 
> knowledge there. We need to be really carefully here. And I really dislike 
> introducing custom ASN.1 structures that are not backed by an actual RFC 
> document.

I'm not saying that we should push this to user space. I'm just saying this 
should
be handled above the crypto API.

>> Agree, and to make this work with afalg_akcipher, I have proposed new
>> > ALG_SET_KEY_ID and ALG_SET_KEY_ID operations. See here: 
>> > https://lkml.org/lkml/2015/12/26/65
>> > The plan is that in this case the afalg_akcipher will not use akcipher api 
>> > at all.
>> > In this case the algif_akcipher will use the request_key() interface to 
>> > retrieve
>> > the key type info and will use the operations that the new TPM interface 
>> > will define.
> The ALG_SET_KEY_ID is great for skcipher where you key is a fixed size blob. 
> There is works really well. And you will need if we let keyctl derive the 
> session key from a set of asymmetric keys.
> 
> I get that we want to enable OpenSSL and others to gain access to kernel 
> crypto operations. We want the same actually for ELL as well. However what is 
> dangerous here is that AF_ALG can never support hardware based keys. So that 
> means support in OpenSSL is broken by design. See earlier emails from David 
> Woodhouse. If you have your certificates / keys in hardware then you need to 
> be able to make them work with OpenSSL. We can not ignore this. It is a 
> design we need to consider from the beginning.

If we will have the TPM interface that allows to invoke functions for HW keys 
and
the *_KEY_ID, then I don't see why AF_ALG would not work. As I said, in case 
when
the keys will be in HW the algif_akcipher will not use akcipher api, but use the
TPM defined functions in the same way as the keyctl would use it.
So I think both AF_ALG and keyctl should work just fine.
Thanks,
-- 
TS
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] crypto: implement DH primitives under akcipher API

[Marcel Holtmann]

Hi Tadeusz,

>> And I have the feeling that akcipher is not the best approach for adding a 
>> key exchange method. I think we need a new method for doing exactly that. At 
>> the base of it, the key exchange is fundamentally different.
> 
> It is unfortunate that, unlike the symmetric ciphers, not all the
> asymmetric ciphers have the same simple encrypt/decrypt interface.
> As you said key exchange algorithms are different. To solve that we should
> define new methods in akcipher.h for key exchange as follows:
> 
> diff --git a/include/crypto/akcipher.h b/include/crypto/akcipher.h
> index c37cc59..d50d834 100644
> --- a/include/crypto/akcipher.h
> +++ b/include/crypto/akcipher.h
> @@ -383,4 +383,41 @@ static inline int crypto_akcipher_set_priv_key(struct 
> crypto_akcipher *tfm,
> 
>   return alg->set_priv_key(tfm, key, keylen);
> }
> +
> +/**
> + * crypto_akcipher_gen_public() - Invoke appropriate key exchange function
> + *
> + * Function invokes the specific key exchange function, which calculates
> + * the public component.
> + *
> + * @req: asymmetric key request
> + *
> + * Return: zero on success; error code in case of error
> + */
> +static inline int crypto_akcipher_gen_public(struct akcipher_request *req)
> +{
> + struct crypto_akcipher *tfm = crypto_akcipher_reqtfm(req);
> + struct akcipher_alg *alg = crypto_akcipher_alg(tfm);
> +
> + return alg->encrypt(req);
> +}
> +
> +/**
> + * crypto_akcipher_gen_shared() - Invoke appropriate key exchange function
> + *
> + * Function invokes the specific key exchange function, which calculates
> + * the shared secret component.
> + *
> + * @req: asymmetric key request
> + *
> + * Return: zero on success; error code in case of error
> + */
> +static inline int crypto_akcipher_gen_shared(struct akcipher_request *req)
> +{
> + struct crypto_akcipher *tfm = crypto_akcipher_reqtfm(req);
> + struct akcipher_alg *alg = crypto_akcipher_alg(tfm);
> +
> + return alg->decrypt(req);
> +}
> +
> #endif
> 
> In this way we can define a generic user side of the key exchange interface,
> and on the the driver side of the akcipher, the implementations would overload
> the existing akcipher encrypt(), decrypt(), set_pub_key(), set_priv_key() 
> methods
> and do what should be done for a given algorithm. We just need to agree on the
> format of the input parameters to these operations.

I think trying to heavily map this to encrypt and decrypt is a bad idea. These 
callbacks are not really designed to return what you need them to return. Key 
exchange is different.

So why are we trying to push this into akcipher in the first place? I mean lets 
just create a keyexch (or similar name) and design it explicitly for key 
exchange handling. We are also not trying to map hashes into skcipher. I think 
the clear distinction of semantics calls for a different API.

>> From an API point of view, I am also not convinced that it is a good idea to 
>> generate the private keys used on the fly. I think this all needs to be a 
>> lot more deterministic and flexible. In addition there are cases where you 
>> want to point to specific private / public key pair that you locally have. 
>> There are even protocols like Bluetooth that have defined fixed debug key 
>> pairs. If we can not support that, then this approach is not generic enough.
> 
> Agree, we need to support all the cases, but dealing with different type of 
> keys
> needs to happen above this API. This API should solely be an interface to math
> primitives of certain operations, which can be implemented in SW or can be HW
> accelerated. Modules like public_key or afalg_akcipher need to be smart enough
> and know what key types they deal with and do the right thing depending on 
> that
> knowledge.

I am not sure this can be that easily generalized. Just pushing the problem of 
key types and formats off to userspace just forces complexity and extra 
knowledge there. We need to be really carefully here. And I really dislike 
introducing custom ASN.1 structures that are not backed by an actual RFC 
document.

>> So my thinking actually is that we need a new key exchange abstraction in 
>> the crypto stack. However I am not sure that an userspace facing API should 
>> be done via AF_ALG. I think that does not fit. I think that doing it via 
>> keyctl is a lot more logical place.
> 
> The advantage of exposing akcipher via AF_ALG is that we can support generic
> applications like OpenSSL or web server. I agree that for some use cases 
> keyctl
> will make more sense, but this shouldn't prevent us from allowing the 
> mentioned
> applications using hardware crypto accelerators that implement e.g. RSA, and 
> can
> work with SW keys. The two interfaces should coexist, and they should work 
> together.
> Keyctl should internally use akcipher for SW keys instead of introducing its 
> own
> implementation, and algif_akcipher should use keyring to deal with HW keys.
> I'm working with David Howells to 

Re: [PATCH] crypto: implement DH primitives under akcipher API

[Marcel Holtmann]

Hi Stephan,

>>> +static int dh_check_params_length(unsigned int p_len)
>>> +{
>>> +   switch (p_len) {
>>> +   case 768:
>>> +   case 1024:
>>> +   case 1536:
>>> +   case 2048:
>>> +   case 3072:
>>> +   case 4096:
>>> +   return 0;
>>> +   }
>>> +   return -EINVAL;
>>> +}
>> 
>> As far back as 1999, the FreeS/WAN project refused to
>> implement the 768-bit IPsec group 1 (even though it was
>> the only one required by the RFCs) because it was not thought
>> secure enough. I think the most-used group was 1536-bit
>> group 5; it wasn't in the original RFCs but nearly everyone
>> implemented it.
>> 
 And besides, I would like to disallow all < 2048 right from the start.
>> 
>> I'm not up-to-date on the performance of attacks. You may be right,
>> or perhaps the minimum should be even higher. Certainly there is
>> no reason to support 768 or 1024-bit groups.
>> 
>> On the other hand, we should consider keeping the 1536-bit
>> group since it is very widely used, likely including by people
>> we'll want to interoperate with.
> 
> Considering the recent attacks which kind of broke DH <= 768, all smaller 
> than 
> 1024 is not to be used any more. Even 1024 bit is not too far off from 768 so 
> I would consider not using 1024 to keep a safety margin.
> 
> It is widely suggested to use 2048 and higher. But for compatibility, I agree 
> with 1536.
> 
> However, that leaves us with the open question for the upper bound. I have no 
> idea which sizes hardware may implement. But considering that OpenSSH uses DH 
> up to and including 8192 these days (see /etc/ssh/moduli), I would suggest to 
> allow 8192 too.
> 
> In addition, we should all be prepared to increase the limit with a 
> reasonable 
> effort. Thus, if we have implementations covering hardware support, they 
> should cope with reasonable efforts.

and why is this not an userspace policy decision on what minimum it allows? Why 
are we discussing this in the context of the kernel in the first place? The 
kernel should just expose the support for it.

Regards

Marcel

--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] crypto: implement DH primitives under akcipher API

[Stephan Mueller]

Am Mittwoch, 2. März 2016, 08:03:44 schrieb Sandy Harris:

Hi Sandy,

> Salvatore Benedetto  wrote:
> >> > > > +static int dh_check_params_length(unsigned int p_len)
> >> > > > +{
> >> > > > +   switch (p_len) {
> >> > > > +   case 768:
> >> > > > +   case 1024:
> >> > > > +   case 1536:
> >> > > > +   case 2048:
> >> > > > +   case 3072:
> >> > > > +   case 4096:
> >> > > > +   return 0;
> >> > > > +   }
> >> > > > +   return -EINVAL;
> >> > > > +}
> 
> As far back as 1999, the FreeS/WAN project refused to
> implement the 768-bit IPsec group 1 (even though it was
> the only one required by the RFCs) because it was not thought
> secure enough. I think the most-used group was 1536-bit
> group 5; it wasn't in the original RFCs but nearly everyone
> implemented it.
> 
> >> And besides, I would like to disallow all < 2048 right from the start.
> 
> I'm not up-to-date on the performance of attacks. You may be right,
> or perhaps the minimum should be even higher. Certainly there is
> no reason to support 768 or 1024-bit groups.
> 
> On the other hand, we should consider keeping the 1536-bit
> group since it is very widely used, likely including by people
> we'll want to interoperate with.

Considering the recent attacks which kind of broke DH <= 768, all smaller than 
1024 is not to be used any more. Even 1024 bit is not too far off from 768 so 
I would consider not using 1024 to keep a safety margin.

It is widely suggested to use 2048 and higher. But for compatibility, I agree 
with 1536.

However, that leaves us with the open question for the upper bound. I have no 
idea which sizes hardware may implement. But considering that OpenSSH uses DH 
up to and including 8192 these days (see /etc/ssh/moduli), I would suggest to 
allow 8192 too.

In addition, we should all be prepared to increase the limit with a reasonable 
effort. Thus, if we have implementations covering hardware support, they 
should cope with reasonable efforts.

Ciao
Stephan
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] crypto: implement DH primitives under akcipher API

[Marcel Holtmann]

Hi Salvatore,

>>> Implement Diffie-Hellman primitives required by the scheme under the
>>> akcipher API. Here is how it works.
>>> 1) Call set_pub_key() by passing DH parameters (p,g) in PKCS3 format
>>> 2) Call set_priv_key() to set your own private key (xa) in raw format
>> 
>> this combination seems odd since it is normally the remote public key and 
>> the local private key. Generally the public key and private key are both 
>> remote ones.
> 
> I'm not sure I understand what you mean here. Usually the public key is
> remote and the private key is local. How can the private key be remote?

I accidentally mistyped. I meant of course local.

>> 
>> For using PKCS3 format is this standardized somewhere? I don't think it is a 
>> good idea to invent new ones here.
> 
> PKCS3 is the format used by openssl for genating DH params, that's why I
> used it.

Is that OpenSSL specific or backed up by a RFC?

>> 
>> In addition, how would this work for ECDH?
> 
> Don't know. There is not even ECC support right now.

If you call something dh-generic, then we need to think about how it would work 
for all the other ciphers that it might be used with. Making this RSA specific 
is not a good idea.

> 
>> 
>>> 3) Call decrypt() without passing any data as input to get back the
>>>  public part which will be computed as g^xa mod p
>>> 4) Call encrypt() by passing the counter part public key (yb) in raw format
>>>  as input to get back the shared secret calculated as zz = yb^xa mod p
>>> 
>>> A test is included in the patch. Test vector has been generated with
>>> openssl
>>> 
>>> Signed-off-by: Salvatore Benedetto 
>>> ---
>>> crypto/Kconfig|   8 ++
>>> crypto/Makefile   |   7 ++
>>> crypto/dh.c   | 264 
>>> ++
>>> crypto/pkcs3.asn1 |   5 ++
>>> crypto/tcrypt.c   |   4 +
>>> crypto/testmgr.c  | 140 +++--
>>> crypto/testmgr.h  | 208 +-
>>> 7 files changed, 627 insertions(+), 9 deletions(-)
>>> create mode 100644 crypto/dh.c
>>> create mode 100644 crypto/pkcs3.asn1
>>> 
>>> diff --git a/crypto/Kconfig b/crypto/Kconfig
>>> index f6bfdda..fd5b78d 100644
>>> --- a/crypto/Kconfig
>>> +++ b/crypto/Kconfig
>>> @@ -101,6 +101,14 @@ config CRYPTO_RSA
>>> help
>>>   Generic implementation of the RSA public key algorithm.
>>> 
>>> +config CRYPTO_DH
>>> +   tristate "Diffie-Hellman algorithm"
>>> +   select CRYPTO_AKCIPHER
>>> +   select MPILIB
>>> +   select ASN1
>> 
>> I really wonder that depending on ASN1 is a good idea here. As mentioned 
>> above ECDH would make sense to actually have supported from the beginning. 
>> The Bluetooth subsystem could be then converted to utilize in kernel ECC key 
>> generation and ECDH shared secret computation. It would be good to show this 
>> is truly generic DH.
>> 
> 
> This is an RFC. I understand it is not the best approach, but
> the idea behind was to try to reuse the akcipher for DH.

And I have the feeling that akcipher is not the best approach for adding a key 
exchange method. I think we need a new method for doing exactly that. At the 
base of it, the key exchange is fundamentally different.

>From an API point of view, I am also not convinced that it is a good idea to 
>generate the private keys used on the fly. I think this all needs to be a lot 
>more deterministic and flexible. In addition there are cases where you want to 
>point to specific private / public key pair that you locally have. There are 
>even protocols like Bluetooth that have defined fixed debug key pairs. If we 
>can not support that, then this approach is not generic enough.

So my thinking actually is that we need a new key exchange abstraction in the 
crypto stack. However I am not sure that an userspace facing API should be done 
via AF_ALG. I think that does not fit. I think that doing it via keyctl is a 
lot more logical place.

It also means that we need a separate keyctl to actually generate the local 
private / public key pairs first. I think that makes sense no matter what. You 
can generate the keys, the private key stays in kernel memory forever and you 
can read out the public key. Some protocols will throw away the keys after 
single use, but others might actually reuse them. Or as mentioned above has 
fixed keys for debugging purposes. Using keyctl should then also make it easy 
to handle RSA vs ECC for the key generation since we need to be able to store 
both types at some point anyway. Also in cases where keys are RSA keys in ASN.1 
format in the first place or are learned from certificates are already present 
and uniquely presented in the kernel. No need to invent yet another format for 
keys.

Especially in the case where you create a session key based out of certificates 
and existing public / private key pairs, it makes sense that keyctl can turn 
them directly into a new key. In most cases these are symmetric keys that can 

Re: [PATCH] crypto: implement DH primitives under akcipher API

[Sandy Harris]

Salvatore Benedetto  wrote:

>> > > > +static int dh_check_params_length(unsigned int p_len)
>> > > > +{
>> > > > +   switch (p_len) {
>> > > > +   case 768:
>> > > > +   case 1024:
>> > > > +   case 1536:
>> > > > +   case 2048:
>> > > > +   case 3072:
>> > > > +   case 4096:
>> > > > +   return 0;
>> > > > +   }
>> > > > +   return -EINVAL;
>> > > > +}
As far back as 1999, the FreeS/WAN project refused to
implement the 768-bit IPsec group 1 (even though it was
the only one required by the RFCs) because it was not thought
secure enough. I think the most-used group was 1536-bit
group 5; it wasn't in the original RFCs but nearly everyone
implemented it.

>> And besides, I would like to disallow all < 2048 right from the start.

I'm not up-to-date on the performance of attacks. You may be right,
or perhaps the minimum should be even higher. Certainly there is
no reason to support 768 or 1024-bit groups.

On the other hand, we should consider keeping the 1536-bit
group since it is very widely used, likely including by people
we'll want to interoperate with.

> Hmm.. What range would you suggest?

There are at least two RFCs which define additional groups.
Why not just add some or all of those?
https://tools.ietf.org/html/rfc3526
https://tools.ietf.org/html/rfc5114
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] crypto: implement DH primitives under akcipher API

[Salvatore Benedetto]

On Tue, Mar 01, 2016 at 12:25:33PM -0800, Marcel Holtmann wrote:

Hi Marcel,

> Hi Salvatore,
> 
> > Implement Diffie-Hellman primitives required by the scheme under the
> > akcipher API. Here is how it works.
> > 1) Call set_pub_key() by passing DH parameters (p,g) in PKCS3 format
> > 2) Call set_priv_key() to set your own private key (xa) in raw format
> 
> this combination seems odd since it is normally the remote public key and the 
> local private key. Generally the public key and private key are both remote 
> ones.

I'm not sure I understand what you mean here. Usually the public key is
remote and the private key is local. How can the private key be remote?

> 
> For using PKCS3 format is this standardized somewhere? I don't think it is a 
> good idea to invent new ones here.

PKCS3 is the format used by openssl for genating DH params, that's why I
used it.

> 
> In addition, how would this work for ECDH?

Don't know. There is not even ECC support right now.

> 
> > 3) Call decrypt() without passing any data as input to get back the
> >   public part which will be computed as g^xa mod p
> > 4) Call encrypt() by passing the counter part public key (yb) in raw format
> >   as input to get back the shared secret calculated as zz = yb^xa mod p
> > 
> > A test is included in the patch. Test vector has been generated with
> > openssl
> > 
> > Signed-off-by: Salvatore Benedetto 
> > ---
> > crypto/Kconfig|   8 ++
> > crypto/Makefile   |   7 ++
> > crypto/dh.c   | 264 
> > ++
> > crypto/pkcs3.asn1 |   5 ++
> > crypto/tcrypt.c   |   4 +
> > crypto/testmgr.c  | 140 +++--
> > crypto/testmgr.h  | 208 +-
> > 7 files changed, 627 insertions(+), 9 deletions(-)
> > create mode 100644 crypto/dh.c
> > create mode 100644 crypto/pkcs3.asn1
> > 
> > diff --git a/crypto/Kconfig b/crypto/Kconfig
> > index f6bfdda..fd5b78d 100644
> > --- a/crypto/Kconfig
> > +++ b/crypto/Kconfig
> > @@ -101,6 +101,14 @@ config CRYPTO_RSA
> > help
> >   Generic implementation of the RSA public key algorithm.
> > 
> > +config CRYPTO_DH
> > +   tristate "Diffie-Hellman algorithm"
> > +   select CRYPTO_AKCIPHER
> > +   select MPILIB
> > +   select ASN1
> 
> I really wonder that depending on ASN1 is a good idea here. As mentioned 
> above ECDH would make sense to actually have supported from the beginning. 
> The Bluetooth subsystem could be then converted to utilize in kernel ECC key 
> generation and ECDH shared secret computation. It would be good to show this 
> is truly generic DH.
> 

This is an RFC. I understand it is not the best approach, but
the idea behind was to try to reuse the akcipher for DH.

Thanks for your comments.

Regards,
Salvatore

> Regards
> 
> Marcel
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] crypto: implement DH primitives under akcipher API

[Marcel Holtmann]

Hi Salvatore,

> Implement Diffie-Hellman primitives required by the scheme under the
> akcipher API. Here is how it works.
> 1) Call set_pub_key() by passing DH parameters (p,g) in PKCS3 format
> 2) Call set_priv_key() to set your own private key (xa) in raw format

this combination seems odd since it is normally the remote public key and the 
local private key. Generally the public key and private key are both remote 
ones.

For using PKCS3 format is this standardized somewhere? I don't think it is a 
good idea to invent new ones here.

In addition, how would this work for ECDH?

> 3) Call decrypt() without passing any data as input to get back the
>   public part which will be computed as g^xa mod p
> 4) Call encrypt() by passing the counter part public key (yb) in raw format
>   as input to get back the shared secret calculated as zz = yb^xa mod p
> 
> A test is included in the patch. Test vector has been generated with
> openssl
> 
> Signed-off-by: Salvatore Benedetto 
> ---
> crypto/Kconfig|   8 ++
> crypto/Makefile   |   7 ++
> crypto/dh.c   | 264 ++
> crypto/pkcs3.asn1 |   5 ++
> crypto/tcrypt.c   |   4 +
> crypto/testmgr.c  | 140 +++--
> crypto/testmgr.h  | 208 +-
> 7 files changed, 627 insertions(+), 9 deletions(-)
> create mode 100644 crypto/dh.c
> create mode 100644 crypto/pkcs3.asn1
> 
> diff --git a/crypto/Kconfig b/crypto/Kconfig
> index f6bfdda..fd5b78d 100644
> --- a/crypto/Kconfig
> +++ b/crypto/Kconfig
> @@ -101,6 +101,14 @@ config CRYPTO_RSA
>   help
> Generic implementation of the RSA public key algorithm.
> 
> +config CRYPTO_DH
> + tristate "Diffie-Hellman algorithm"
> + select CRYPTO_AKCIPHER
> + select MPILIB
> + select ASN1

I really wonder that depending on ASN1 is a good idea here. As mentioned above 
ECDH would make sense to actually have supported from the beginning. The 
Bluetooth subsystem could be then converted to utilize in kernel ECC key 
generation and ECDH shared secret computation. It would be good to show this is 
truly generic DH.

Regards

Marcel

--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] crypto: implement DH primitives under akcipher API

[Stephan Mueller]

Am Dienstag, 1. März 2016, 11:08:34 schrieb Salvatore Benedetto:

Hi Salvatore,

> > > +static int dh_check_params_length(unsigned int p_len)
> > > +{
> > > + switch (p_len) {
> > > + case 768:
> > > + case 1024:
> > > + case 1536:
> > > + case 2048:
> > > + case 3072:
> > > + case 4096:
> > > + return 0;
> > > + }
> > > + return -EINVAL;
> > > +}
> > 
> > What is the reason for restricting the size to 4096?
> 
> Honestly no reason.
> Could not find restrictions in the spec about the params length.

I am just wondering because other DH impls allow longer sizes.

And besides, I would like to disallow all < 2048 right from the start.
> 
> > > +
> > > +static int dh_no_op(struct akcipher_request *req)
> > > +{
> > > + return -ENOPROTOOPT;
> > > +}
> > > +
> > > +static int dh_set_priv_key(struct crypto_akcipher *tfm, const void
> > > *key,
> > > +unsigned int keylen)
> > > +{
> > > + struct dh_params *params = akcipher_tfm_ctx(tfm);
> > 
> > dh_get_params?
> 
> You mean adding a helper function? OK.

Not adding, but using your helper function -- why do you have it there in the 
first place? :-)

Ciao
Stephan
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] crypto: implement DH primitives under akcipher API

[Salvatore Benedetto]

Hi Stephan,

thanks for reviewing.

On Mon, Feb 15, 2016 at 02:57:08PM +0100, Stephan Mueller wrote:
> Am Montag, 15. Februar 2016, 09:01:55 schrieb Salvatore Benedetto:
> 
> Hi Salvatore, Herbert,
> 
> > Implement Diffie-Hellman primitives required by the scheme under the
> > akcipher API. Here is how it works.
> > 1) Call set_pub_key() by passing DH parameters (p,g) in PKCS3 format
> > 2) Call set_priv_key() to set your own private key (xa) in raw format
> > 3) Call decrypt() without passing any data as input to get back the
> >public part which will be computed as g^xa mod p
> > 4) Call encrypt() by passing the counter part public key (yb) in raw format
> >as input to get back the shared secret calculated as zz = yb^xa mod p
> > 
> > A test is included in the patch. Test vector has been generated with
> > openssl
> 
> Herbert, as this is a raw DH operation where the shared secret must be 
> subjected to a KDF, I guess the KDF patch I provided some time ago may become 
> of interest again?
> > 
> > Signed-off-by: Salvatore Benedetto 
> > ---
> >  crypto/Kconfig|   8 ++
> >  crypto/Makefile   |   7 ++
> >  crypto/dh.c   | 264
> > ++ crypto/pkcs3.asn1 | 
> >  5 ++
> >  crypto/tcrypt.c   |   4 +
> >  crypto/testmgr.c  | 140 +++--
> >  crypto/testmgr.h  | 208 +-
> >  7 files changed, 627 insertions(+), 9 deletions(-)
> >  create mode 100644 crypto/dh.c
> >  create mode 100644 crypto/pkcs3.asn1
> > 
> > diff --git a/crypto/Kconfig b/crypto/Kconfig
> > index f6bfdda..fd5b78d 100644
> > --- a/crypto/Kconfig
> > +++ b/crypto/Kconfig
> > @@ -101,6 +101,14 @@ config CRYPTO_RSA
> > help
> >   Generic implementation of the RSA public key algorithm.
> > 
> > +config CRYPTO_DH
> > +   tristate "Diffie-Hellman algorithm"
> > +   select CRYPTO_AKCIPHER
> > +   select MPILIB
> > +   select ASN1
> > +   help
> > + Generic implementation of the Diffie-Hellman algorithm.
> > +
> >  config CRYPTO_MANAGER
> > tristate "Cryptographic algorithm manager"
> > select CRYPTO_MANAGER2
> > diff --git a/crypto/Makefile b/crypto/Makefile
> > index 4f4ef7e..ee73489 100644
> > --- a/crypto/Makefile
> > +++ b/crypto/Makefile
> > @@ -31,6 +31,13 @@ obj-$(CONFIG_CRYPTO_HASH2) += crypto_hash.o
> > 
> >  obj-$(CONFIG_CRYPTO_AKCIPHER2) += akcipher.o
> > 
> > +$(obj)/pkcs3-asn1.o: $(obj)/pkcs3-asn1.c $(obj)/pkcs3-asn1.h
> > +clean-files += pkcs3-asn1.c pkcs3-asn1.h
> > +
> > +dh_generic-y := pkcs3-asn1.o
> > +dh_generic-y += dh.o
> > +obj-$(CONFIG_CRYPTO_DH) += dh_generic.o
> > +
> >  $(obj)/rsapubkey-asn1.o: $(obj)/rsapubkey-asn1.c $(obj)/rsapubkey-asn1.h
> >  $(obj)/rsaprivkey-asn1.o: $(obj)/rsaprivkey-asn1.c $(obj)/rsaprivkey-asn1.h
> > clean-files += rsapubkey-asn1.c rsapubkey-asn1.h
> > diff --git a/crypto/dh.c b/crypto/dh.c
> > new file mode 100644
> > index 000..614c4f1
> > --- /dev/null
> > +++ b/crypto/dh.c
> > @@ -0,0 +1,264 @@
> > +/*  Diffie-Hellman Key Agreement Method [RFC2631]
> > + *
> > + * Copyright (c) 2016, Intel Corporation
> > + * Authors: Salvatore Benedetto 
> > + *
> > + * This program is free software; you can redistribute it and/or
> > + * modify it under the terms of the GNU General Public Licence
> > + * as published by the Free Software Foundation; either version
> > + * 2 of the Licence, or (at your option) any later version.
> > + */
> > +
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include "pkcs3-asn1.h"
> > +
> > +struct dh_params {
> > +   MPI p;
> > +   MPI g;
> > +   MPI xa;
> > +};
> > +
> > +int dh_get_g(void *context, size_t hdrlen, unsigned char tag, const void
> > *value, +size_t vlen)
> > +{
> > +   struct dh_params *params = context;
> > +
> > +   params->g = mpi_read_raw_data(value, vlen);
> > +
> > +   if (!params->g)
> > +   return -ENOMEM;
> > +
> > +   return 0;
> > +}
> > +
> > +int dh_get_p(void *context, size_t hdrlen, unsigned char tag, const void
> > *value, +size_t vlen)
> > +{
> > +   struct dh_params *params = context;
> > +
> > +   params->p = mpi_read_raw_data(value, vlen);
> > +
> > +   if (!params->p)
> > +   return -ENOMEM;
> > +
> > +   return 0;
> > +}
> > +
> > +static int dh_parse_params(struct dh_params *params, const void *key,
> > +  unsigned int keylen)
> > +{
> > +   int ret;
> > +
> > +   mpi_free(params->p);
> > +   mpi_free(params->g);
> > +
> > +   ret = asn1_ber_decoder(_decoder, params, key, keylen);
> > +
> > +   return ret;
> > +}
> > +
> > +static void dh_free_params(struct dh_params *params)
> > +{
> > +   mpi_free(params->p);
> > +   mpi_free(params->g);
> > +   mpi_free(params->xa);
> > +   params->p = NULL;
> > +   params->g = NULL;
> > +   params->xa = NULL;
> > +}
> > +
> > +/*
> > + * Public key generation function [RFC2631 sec 2.1.1]
> > + * ya = g^xa mod 

Re: [PATCH] crypto: implement DH primitives under akcipher API

[Tadeusz Struk]

On 02/16/2016 12:19 PM, Herbert Xu wrote:
> On Mon, Feb 15, 2016 at 09:01:55AM +, Salvatore Benedetto wrote:
>> > Implement Diffie-Hellman primitives required by the scheme under the
>> > akcipher API. Here is how it works.
>> > 1) Call set_pub_key() by passing DH parameters (p,g) in PKCS3 format
>> > 2) Call set_priv_key() to set your own private key (xa) in raw format
>> > 3) Call decrypt() without passing any data as input to get back the
>> >public part which will be computed as g^xa mod p
>> > 4) Call encrypt() by passing the counter part public key (yb) in raw format
>> >as input to get back the shared secret calculated as zz = yb^xa mod p
>> > 
>> > A test is included in the patch. Test vector has been generated with
>> > openssl
>> > 
>> > Signed-off-by: Salvatore Benedetto 
> Who is going to use this?

OpenSSL via PF_ALG. The plan for this is to accelerate TLS handshakes in HW.
This is an RFC to get your opinion on the usage of the API.
Are you ok with the approach?
Thanks,

-- 
TS
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] crypto: implement DH primitives under akcipher API

[Herbert Xu]

On Mon, Feb 15, 2016 at 09:01:55AM +, Salvatore Benedetto wrote:
> Implement Diffie-Hellman primitives required by the scheme under the
> akcipher API. Here is how it works.
> 1) Call set_pub_key() by passing DH parameters (p,g) in PKCS3 format
> 2) Call set_priv_key() to set your own private key (xa) in raw format
> 3) Call decrypt() without passing any data as input to get back the
>public part which will be computed as g^xa mod p
> 4) Call encrypt() by passing the counter part public key (yb) in raw format
>as input to get back the shared secret calculated as zz = yb^xa mod p
> 
> A test is included in the patch. Test vector has been generated with
> openssl
> 
> Signed-off-by: Salvatore Benedetto 

Who is going to use this?

Thanks,
-- 
Email: Herbert Xu 
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] crypto: implement DH primitives under akcipher API

[Stephan Mueller]

Am Montag, 15. Februar 2016, 09:01:55 schrieb Salvatore Benedetto:

Hi Salvatore, Herbert,

> Implement Diffie-Hellman primitives required by the scheme under the
> akcipher API. Here is how it works.
> 1) Call set_pub_key() by passing DH parameters (p,g) in PKCS3 format
> 2) Call set_priv_key() to set your own private key (xa) in raw format
> 3) Call decrypt() without passing any data as input to get back the
>public part which will be computed as g^xa mod p
> 4) Call encrypt() by passing the counter part public key (yb) in raw format
>as input to get back the shared secret calculated as zz = yb^xa mod p
> 
> A test is included in the patch. Test vector has been generated with
> openssl

Herbert, as this is a raw DH operation where the shared secret must be 
subjected to a KDF, I guess the KDF patch I provided some time ago may become 
of interest again?
> 
> Signed-off-by: Salvatore Benedetto 
> ---
>  crypto/Kconfig|   8 ++
>  crypto/Makefile   |   7 ++
>  crypto/dh.c   | 264
> ++ crypto/pkcs3.asn1 | 
>  5 ++
>  crypto/tcrypt.c   |   4 +
>  crypto/testmgr.c  | 140 +++--
>  crypto/testmgr.h  | 208 +-
>  7 files changed, 627 insertions(+), 9 deletions(-)
>  create mode 100644 crypto/dh.c
>  create mode 100644 crypto/pkcs3.asn1
> 
> diff --git a/crypto/Kconfig b/crypto/Kconfig
> index f6bfdda..fd5b78d 100644
> --- a/crypto/Kconfig
> +++ b/crypto/Kconfig
> @@ -101,6 +101,14 @@ config CRYPTO_RSA
>   help
> Generic implementation of the RSA public key algorithm.
> 
> +config CRYPTO_DH
> + tristate "Diffie-Hellman algorithm"
> + select CRYPTO_AKCIPHER
> + select MPILIB
> + select ASN1
> + help
> +   Generic implementation of the Diffie-Hellman algorithm.
> +
>  config CRYPTO_MANAGER
>   tristate "Cryptographic algorithm manager"
>   select CRYPTO_MANAGER2
> diff --git a/crypto/Makefile b/crypto/Makefile
> index 4f4ef7e..ee73489 100644
> --- a/crypto/Makefile
> +++ b/crypto/Makefile
> @@ -31,6 +31,13 @@ obj-$(CONFIG_CRYPTO_HASH2) += crypto_hash.o
> 
>  obj-$(CONFIG_CRYPTO_AKCIPHER2) += akcipher.o
> 
> +$(obj)/pkcs3-asn1.o: $(obj)/pkcs3-asn1.c $(obj)/pkcs3-asn1.h
> +clean-files += pkcs3-asn1.c pkcs3-asn1.h
> +
> +dh_generic-y := pkcs3-asn1.o
> +dh_generic-y += dh.o
> +obj-$(CONFIG_CRYPTO_DH) += dh_generic.o
> +
>  $(obj)/rsapubkey-asn1.o: $(obj)/rsapubkey-asn1.c $(obj)/rsapubkey-asn1.h
>  $(obj)/rsaprivkey-asn1.o: $(obj)/rsaprivkey-asn1.c $(obj)/rsaprivkey-asn1.h
> clean-files += rsapubkey-asn1.c rsapubkey-asn1.h
> diff --git a/crypto/dh.c b/crypto/dh.c
> new file mode 100644
> index 000..614c4f1
> --- /dev/null
> +++ b/crypto/dh.c
> @@ -0,0 +1,264 @@
> +/*  Diffie-Hellman Key Agreement Method [RFC2631]
> + *
> + * Copyright (c) 2016, Intel Corporation
> + * Authors: Salvatore Benedetto 
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public Licence
> + * as published by the Free Software Foundation; either version
> + * 2 of the Licence, or (at your option) any later version.
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include "pkcs3-asn1.h"
> +
> +struct dh_params {
> + MPI p;
> + MPI g;
> + MPI xa;
> +};
> +
> +int dh_get_g(void *context, size_t hdrlen, unsigned char tag, const void
> *value, +  size_t vlen)
> +{
> + struct dh_params *params = context;
> +
> + params->g = mpi_read_raw_data(value, vlen);
> +
> + if (!params->g)
> + return -ENOMEM;
> +
> + return 0;
> +}
> +
> +int dh_get_p(void *context, size_t hdrlen, unsigned char tag, const void
> *value, +  size_t vlen)
> +{
> + struct dh_params *params = context;
> +
> + params->p = mpi_read_raw_data(value, vlen);
> +
> + if (!params->p)
> + return -ENOMEM;
> +
> + return 0;
> +}
> +
> +static int dh_parse_params(struct dh_params *params, const void *key,
> +unsigned int keylen)
> +{
> + int ret;
> +
> + mpi_free(params->p);
> + mpi_free(params->g);
> +
> + ret = asn1_ber_decoder(_decoder, params, key, keylen);
> +
> + return ret;
> +}
> +
> +static void dh_free_params(struct dh_params *params)
> +{
> + mpi_free(params->p);
> + mpi_free(params->g);
> + mpi_free(params->xa);
> + params->p = NULL;
> + params->g = NULL;
> + params->xa = NULL;
> +}
> +
> +/*
> + * Public key generation function [RFC2631 sec 2.1.1]
> + * ya = g^xa mod p;
> + */
> +static int _generate_public_key(const struct dh_params *params, MPI ya)
> +{
> + /* ya = g^xa mod p */
> + return mpi_powm(ya, params->g, params->xa, params->p);
> +}
> +
> +/*
> + * ZZ generation function [RFC2631 sec 2.1.1]
> + * ZZ = yb^xa mod p;
> + */
> +static int _compute_shared_secret(const struct