| Message ID | 56DDF682.5070008@intel.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Delegated to: | Herbert Xu |
| Headers | show |
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 try to find out how exactly this will work together. > > Also please note that there already exists an af_alg engine for openSSL for > symmetric ciphers. It would be more logical to add support for asymmetric ciphers > to the existing engine, than to create a new one, which uses keyctl. Of course keyctl should use akcipher and also a new keyexch to work with keys that do not have a hardware crypto backing them up. >> 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 then be easily referenced by skcipher for ease of use. >> >> And I did mention this before, this would also solve the problem where you might have to use a private key that is part of a TPM or secure enclave. The case where the kernel does not even know the key, just its existence. Use it to generate the session key and keep the session key in the kernel. > > 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. Regards Marcel -- To unsubscribe from this list: send the line "unsubscribe linux-crypto" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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,
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 majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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