Re: [RFC PATCH 4/5] crypto: Adds user space interface for ALG_SET_KEY_TYPE

2019-01-17 Thread Stephan Mueller
Am Donnerstag, 17. Januar 2019, 08:02:20 CET schrieb Kalyani Akula:

Hi Kalyani,

> ALG_SET_KEY_TYPE requires caller to pass the key_type to be used
> for AES encryption/decryption.
> 
> Sometimes the cipher key will be stored in the
> device's hardware. So, there is a need to specify
> the information about the key to use for AES operations.
> 
> In Xilinx ZynqMP SoC, below key types are available
> 
> 1. Device key, which is flashed in the HW.
> 
> 2. PUF KEK, which can be regenerated using the
>helper data programmed in the HW.
> 
> 3. User supplied key.
> 
> So to choose the AES key to be used, this patch adds key-type attribute.

You expose your particular driver interface to user space. So, user space 
would need the details of you driver to know what to set. If another driver 
has such key type support, user space would need to know about that, too. I do 
not think this is a wise idea.

If we are going to have such a keytype selection, there must be a common user 
space interface for all drivers. I.e. define common key types the drivers then 
can map to their particular key type interface.

Besides, seem to be more a key handling issue. Wouldn't it make sense to 
rather have such issue solved with key rings than in the kernel crypto API?

Ciao
Stephan




[RFC PATCH 4/5] crypto: Adds user space interface for ALG_SET_KEY_TYPE

2019-01-16 Thread Kalyani Akula
ALG_SET_KEY_TYPE requires caller to pass the key_type to be used
for AES encryption/decryption.

Sometimes the cipher key will be stored in the
device's hardware. So, there is a need to specify
the information about the key to use for AES operations.

In Xilinx ZynqMP SoC, below key types are available

1. Device key, which is flashed in the HW.

2. PUF KEK, which can be regenerated using the
   helper data programmed in the HW.

3. User supplied key.

So to choose the AES key to be used, this patch adds key-type attribute.

Signed-off-by: Kalyani Akula 
---
 crypto/af_alg.c |  7 +++
 crypto/algif_skcipher.c |  7 +++
 crypto/blkcipher.c  |  9 +
 crypto/skcipher.c   | 18 ++
 include/crypto/if_alg.h |  2 ++
 include/crypto/skcipher.h   | 10 ++
 include/linux/crypto.h  | 12 
 include/uapi/linux/if_alg.h |  1 +
 8 files changed, 66 insertions(+)

diff --git a/crypto/af_alg.c b/crypto/af_alg.c
index 17eb09d..c3c0660 100644
--- a/crypto/af_alg.c
+++ b/crypto/af_alg.c
@@ -261,6 +261,13 @@ static int alg_setsockopt(struct socket *sock, int level, 
int optname,
if (!type->setauthsize)
goto unlock;
err = type->setauthsize(ask->private, optlen);
+   break;
+   case ALG_SET_KEY_TYPE:
+   if (sock->state == SS_CONNECTED)
+   goto unlock;
+   if (!type->setkeytype)
+   goto unlock;
+   err = type->setkeytype(ask->private, optval, optlen);
}
 
 unlock:
diff --git a/crypto/algif_skcipher.c b/crypto/algif_skcipher.c
index cfdaab2..9911a56 100644
--- a/crypto/algif_skcipher.c
+++ b/crypto/algif_skcipher.c
@@ -320,6 +320,12 @@ static int skcipher_setkey(void *private, const u8 *key, 
unsigned int keylen)
return crypto_skcipher_setkey(private, key, keylen);
 }
 
+static int skcipher_setkeytype(void *private, const u8 *key,
+  unsigned int keylen)
+{
+   return crypto_skcipher_setkeytype(private, key, keylen);
+}
+
 static void skcipher_sock_destruct(struct sock *sk)
 {
struct alg_sock *ask = alg_sk(sk);
@@ -384,6 +390,7 @@ static int skcipher_accept_parent(void *private, struct 
sock *sk)
.bind   =   skcipher_bind,
.release=   skcipher_release,
.setkey =   skcipher_setkey,
+   .setkeytype =   skcipher_setkeytype,
.accept =   skcipher_accept_parent,
.accept_nokey   =   skcipher_accept_parent_nokey,
.ops=   _skcipher_ops,
diff --git a/crypto/blkcipher.c b/crypto/blkcipher.c
index c5398bd..8922f58 100644
--- a/crypto/blkcipher.c
+++ b/crypto/blkcipher.c
@@ -408,6 +408,14 @@ static int setkey(struct crypto_tfm *tfm, const u8 *key, 
unsigned int keylen)
return cipher->setkey(tfm, key, keylen);
 }
 
+static int setkeytype(struct crypto_tfm *tfm, const u8 *key,
+ unsigned int keylen)
+{
+   struct blkcipher_alg *cipher = >__crt_alg->cra_blkcipher;
+
+   return cipher->setkeytype(tfm, key, keylen);
+}
+
 static int async_setkey(struct crypto_ablkcipher *tfm, const u8 *key,
unsigned int keylen)
 {
@@ -478,6 +486,7 @@ static int crypto_init_blkcipher_ops_sync(struct crypto_tfm 
*tfm)
unsigned long addr;
 
crt->setkey = setkey;
+   crt->setkeytype = setkeytype;
crt->encrypt = alg->encrypt;
crt->decrypt = alg->decrypt;
 
diff --git a/crypto/skcipher.c b/crypto/skcipher.c
index 2a96929..6a2a0dd 100644
--- a/crypto/skcipher.c
+++ b/crypto/skcipher.c
@@ -605,6 +605,23 @@ static int skcipher_setkey_blkcipher(struct 
crypto_skcipher *tfm,
return 0;
 }
 
+static int skcipher_setkeytype_blkcipher(struct crypto_skcipher *tfm,
+const u8 *key, unsigned int keylen)
+{
+   struct crypto_blkcipher **ctx = crypto_skcipher_ctx(tfm);
+   struct crypto_blkcipher *blkcipher = *ctx;
+   int err;
+
+   crypto_blkcipher_clear_flags(blkcipher, ~0);
+   crypto_blkcipher_set_flags(blkcipher, crypto_skcipher_get_flags(tfm) &
+   CRYPTO_TFM_REQ_MASK);
+   err = crypto_blkcipher_setkeytype(blkcipher, key, keylen);
+   crypto_skcipher_set_flags(tfm, crypto_blkcipher_get_flags(blkcipher) &
+   CRYPTO_TFM_RES_MASK);
+
+   return err;
+}
+
 static int skcipher_crypt_blkcipher(struct skcipher_request *req,
int (*crypt)(struct blkcipher_desc *,
 struct scatterlist *,
@@ -671,6 +688,7 @@ static int crypto_init_skcipher_ops_blkcipher(struct 
crypto_tfm *tfm)
tfm->exit = crypto_exit_skcipher_ops_blkcipher;
 
skcipher->setkey = skcipher_setkey_blkcipher;
+   skcipher->setkeytype = skcipher_setkeytype_blkcipher;
skcipher->encrypt =