[PATCH v8 0/4] crypto: add algif_akcipher user space API

[PATCH v8 0/4] crypto: add algif_akcipher user space API

[Stephan Müller]

Hi,

This patch set adds the AF_ALG user space API to externalize the
asymmetric cipher API recently added to the kernel crypto API.

The patch set is tested with the user space library of libkcapi [1].
Use [1] test/test.sh for a full test run. The test covers the
following scenarios:

	* sendmsg of one IOVEC

	* sendmsg of 16 IOVECs with non-linear buffer

	* vmsplice of one IOVEC

	* vmsplice of 15 IOVECs with non-linear buffer

	* invoking multiple separate cipher operations with one
	  open cipher handle

	* encryption with private key (using vector from testmgr.h)

	* encryption with public key (using vector from testmgr.h)

	* decryption with private key (using vector from testmgr.h)

Note, to enable the test, edit line [2] from "4 99" to "4 13".

[1] http://www.chronox.de/libkcapi.html
[2] https://github.com/smuellerDD/libkcapi/blob/master/test/test.sh#L1452

Changes v8:
 * port to kernel 4.13
 * port to consolidated AF_ALG code

Stephan Mueller (4):
  crypto: AF_ALG -- add sign/verify API
  crypto: AF_ALG -- add setpubkey setsockopt call
  crypto: AF_ALG -- add asymmetric cipher
  crypto: algif_akcipher - enable compilation

 crypto/Kconfig              |   9 +
 crypto/Makefile             |   1 +
 crypto/af_alg.c             |  28 ++-
 crypto/algif_aead.c         |  36 ++--
 crypto/algif_akcipher.c     | 466 ++++++++++++++++++++++++++++++++++++++++++++
 crypto/algif_skcipher.c     |  26 ++-
 include/crypto/if_alg.h     |   7 +-
 include/uapi/linux/if_alg.h |   3 +
 8 files changed, 543 insertions(+), 33 deletions(-)
 create mode 100644 crypto/algif_akcipher.c

-- 
2.13.4

[PATCH v8 1/4] crypto: AF_ALG -- add sign/verify API

[Stephan Müller]

Add the flags for handling signature generation and signature
verification.

The af_alg helper code as well as the algif_skcipher and algif_aead code
must be changed from a boolean indicating the cipher operation to an
integer because there are now 4 different cipher operations that are
defined. Yet, the algif_aead and algif_skcipher code still only allows
encryption and decryption cipher operations.

Signed-off-by: Stephan Mueller <smueller@chronox.de>
Signed-off-by: Tadeusz Struk <tadeusz.struk@intel.com>
---
 crypto/af_alg.c             | 10 +++++-----
 crypto/algif_aead.c         | 36 ++++++++++++++++++++++++------------
 crypto/algif_skcipher.c     | 26 +++++++++++++++++---------
 include/crypto/if_alg.h     |  4 ++--
 include/uapi/linux/if_alg.h |  2 ++
 5 files changed, 50 insertions(+), 28 deletions(-)

diff --git a/crypto/af_alg.c b/crypto/af_alg.c
index d6936c0e08d9..a35a9f854a04 100644
--- a/crypto/af_alg.c
+++ b/crypto/af_alg.c
@@ -859,7 +859,7 @@ int af_alg_sendmsg(struct socket *sock, struct msghdr *msg, size_t size,
 	struct af_alg_tsgl *sgl;
 	struct af_alg_control con = {};
 	long copied = 0;
-	bool enc = 0;
+	int op = 0;
 	bool init = 0;
 	int err = 0;
 
@@ -870,11 +870,11 @@ int af_alg_sendmsg(struct socket *sock, struct msghdr *msg, size_t size,
 
 		init = 1;
 		switch (con.op) {
+		case ALG_OP_VERIFY:
+		case ALG_OP_SIGN:
 		case ALG_OP_ENCRYPT:
-			enc = 1;
-			break;
 		case ALG_OP_DECRYPT:
-			enc = 0;
+			op = con.op;
 			break;
 		default:
 			return -EINVAL;
@@ -891,7 +891,7 @@ int af_alg_sendmsg(struct socket *sock, struct msghdr *msg, size_t size,
 	}
 
 	if (init) {
-		ctx->enc = enc;
+		ctx->op = op;
 		if (con.iv)
 			memcpy(ctx->iv, con.iv->iv, ivsize);
 
diff --git a/crypto/algif_aead.c b/crypto/algif_aead.c
index 516b38c3a169..77abc04cf942 100644
--- a/crypto/algif_aead.c
+++ b/crypto/algif_aead.c
@@ -60,7 +60,7 @@ static inline bool aead_sufficient_data(struct sock *sk)
 	 * The minimum amount of memory needed for an AEAD cipher is
 	 * the AAD and in case of decryption the tag.
 	 */
-	return ctx->used >= ctx->aead_assoclen + (ctx->enc ? 0 : as);
+	return ctx->used >= ctx->aead_assoclen + (ctx->op ? 0 : as);
 }
 
 static int aead_sendmsg(struct socket *sock, struct msghdr *msg, size_t size)
@@ -137,7 +137,7 @@ static int _aead_recvmsg(struct socket *sock, struct msghdr *msg,
 	 * buffer provides the tag which is consumed resulting in only the
 	 * plaintext without a buffer for the tag returned to the caller.
 	 */
-	if (ctx->enc)
+	if (ctx->op)
 		outlen = used + as;
 	else
 		outlen = used - as;
@@ -196,7 +196,7 @@ static int _aead_recvmsg(struct socket *sock, struct msghdr *msg,
 	/* Use the RX SGL as source (and destination) for crypto op. */
 	src = areq->first_rsgl.sgl.sg;
 
-	if (ctx->enc) {
+	if (ctx->op == ALG_OP_ENCRYPT) {
 		/*
 		 * Encryption operation - The in-place cipher operation is
 		 * achieved by the following operation:
@@ -212,7 +212,7 @@ static int _aead_recvmsg(struct socket *sock, struct msghdr *msg,
 		if (err)
 			goto free;
 		af_alg_pull_tsgl(sk, processed, NULL, 0);
-	} else {
+	} else if (ctx->op == ALG_OP_DECRYPT) {
 		/*
 		 * Decryption operation - To achieve an in-place cipher
 		 * operation, the following  SGL structure is used:
@@ -258,6 +258,9 @@ static int _aead_recvmsg(struct socket *sock, struct msghdr *msg,
 		} else
 			/* no RX SGL present (e.g. authentication only) */
 			src = areq->tsgl;
+	} else {
+		err = -EOPNOTSUPP;
+		goto free;
 	}
 
 	/* Initialize the crypto operation */
@@ -272,19 +275,28 @@ static int _aead_recvmsg(struct socket *sock, struct msghdr *msg,
 		aead_request_set_callback(&areq->cra_u.aead_req,
 					  CRYPTO_TFM_REQ_MAY_BACKLOG,
 					  af_alg_async_cb, areq);
-		err = ctx->enc ? crypto_aead_encrypt(&areq->cra_u.aead_req) :
-				 crypto_aead_decrypt(&areq->cra_u.aead_req);
-	} else {
+	} else
 		/* Synchronous operation */
 		aead_request_set_callback(&areq->cra_u.aead_req,
 					  CRYPTO_TFM_REQ_MAY_BACKLOG,
 					  af_alg_complete, &ctx->completion);
-		err = af_alg_wait_for_completion(ctx->enc ?
-				crypto_aead_encrypt(&areq->cra_u.aead_req) :
-				crypto_aead_decrypt(&areq->cra_u.aead_req),
-						 &ctx->completion);
+
+	switch (ctx->op) {
+	case ALG_OP_ENCRYPT:
+		err = crypto_aead_encrypt(&areq->cra_u.aead_req);
+		break;
+	case ALG_OP_DECRYPT:
+		err = crypto_aead_decrypt(&areq->cra_u.aead_req);
+		break;
+	default:
+		err = -EOPNOTSUPP;
+		goto free;
 	}
 
+	/* Wait for synchronous operation completion */
+	if (msg->msg_iocb && !is_sync_kiocb(msg->msg_iocb))
+		err = af_alg_wait_for_completion(err, &ctx->completion);
+
 	/* AIO operation in progress */
 	if (err == -EINPROGRESS) {
 		sock_hold(sk);
@@ -552,7 +564,7 @@ static int aead_accept_parent_nokey(void *private, struct sock *sk)
 	ctx->rcvused = 0;
 	ctx->more = 0;
 	ctx->merge = 0;
-	ctx->enc = 0;
+	ctx->op = 0;
 	ctx->aead_assoclen = 0;
 	af_alg_init_completion(&ctx->completion);
 
diff --git a/crypto/algif_skcipher.c b/crypto/algif_skcipher.c
index 8ae4170aaeb4..3bf761868689 100644
--- a/crypto/algif_skcipher.c
+++ b/crypto/algif_skcipher.c
@@ -121,22 +121,30 @@ static int _skcipher_recvmsg(struct socket *sock, struct msghdr *msg,
 		skcipher_request_set_callback(&areq->cra_u.skcipher_req,
 					      CRYPTO_TFM_REQ_MAY_SLEEP,
 					      af_alg_async_cb, areq);
-		err = ctx->enc ?
-			crypto_skcipher_encrypt(&areq->cra_u.skcipher_req) :
-			crypto_skcipher_decrypt(&areq->cra_u.skcipher_req);
-	} else {
+	} else
 		/* Synchronous operation */
 		skcipher_request_set_callback(&areq->cra_u.skcipher_req,
 					      CRYPTO_TFM_REQ_MAY_SLEEP |
 					      CRYPTO_TFM_REQ_MAY_BACKLOG,
 					      af_alg_complete,
 					      &ctx->completion);
-		err = af_alg_wait_for_completion(ctx->enc ?
-			crypto_skcipher_encrypt(&areq->cra_u.skcipher_req) :
-			crypto_skcipher_decrypt(&areq->cra_u.skcipher_req),
-						 &ctx->completion);
+
+	switch (ctx->op) {
+	case ALG_OP_ENCRYPT:
+		err = crypto_skcipher_encrypt(&areq->cra_u.skcipher_req);
+		break;
+	case ALG_OP_DECRYPT:
+		err = crypto_skcipher_decrypt(&areq->cra_u.skcipher_req);
+		break;
+	default:
+		err = -EOPNOTSUPP;
+		goto free;
 	}
 
+	/* Wait for synchronous operation completion */
+	if (msg->msg_iocb && !is_sync_kiocb(msg->msg_iocb))
+		err = af_alg_wait_for_completion(err, &ctx->completion);
+
 	/* AIO operation in progress */
 	if (err == -EINPROGRESS) {
 		sock_hold(sk);
@@ -387,7 +395,7 @@ static int skcipher_accept_parent_nokey(void *private, struct sock *sk)
 	ctx->rcvused = 0;
 	ctx->more = 0;
 	ctx->merge = 0;
-	ctx->enc = 0;
+	ctx->op = 0;
 	af_alg_init_completion(&ctx->completion);
 
 	ask->private = ctx;
diff --git a/include/crypto/if_alg.h b/include/crypto/if_alg.h
index 75ec9c662268..50a21488f3ba 100644
--- a/include/crypto/if_alg.h
+++ b/include/crypto/if_alg.h
@@ -142,7 +142,7 @@ struct af_alg_async_req {
  * @more:		More data to be expected from user space?
  * @merge:		Shall new data from user space be merged into existing
  *			SG?
- * @enc:		Cryptographic operation to be performed when
+ * @op:			Cryptographic operation to be performed when
  *			recvmsg is invoked.
  * @len:		Length of memory allocated for this data structure.
  */
@@ -159,7 +159,7 @@ struct af_alg_ctx {
 
 	bool more;
 	bool merge;
-	bool enc;
+	int op;
 
 	unsigned int len;
 };
diff --git a/include/uapi/linux/if_alg.h b/include/uapi/linux/if_alg.h
index f2acd2fde1f3..d81dcca5bdd7 100644
--- a/include/uapi/linux/if_alg.h
+++ b/include/uapi/linux/if_alg.h
@@ -38,5 +38,7 @@ struct af_alg_iv {
 /* Operations */
 #define ALG_OP_DECRYPT			0
 #define ALG_OP_ENCRYPT			1
+#define ALG_OP_SIGN			2
+#define ALG_OP_VERIFY			3
 
 #endif	/* _LINUX_IF_ALG_H */
-- 
2.13.4

Re: [PATCH v8 1/4] crypto: AF_ALG -- add sign/verify API

[Tudor Ambarus]

Hi, Stephan,

On 08/10/2017 09:39 AM, Stephan Müller wrote:
> Add the flags for handling signature generation and signature
> verification.
> 
> The af_alg helper code as well as the algif_skcipher and algif_aead code
> must be changed from a boolean indicating the cipher operation to an
> integer because there are now 4 different cipher operations that are
> defined. Yet, the algif_aead and algif_skcipher code still only allows
> encryption and decryption cipher operations.
> 
> Signed-off-by: Stephan Mueller <smueller@chronox.de>
> Signed-off-by: Tadeusz Struk <tadeusz.struk@intel.com>
> ---
>   crypto/af_alg.c             | 10 +++++-----
>   crypto/algif_aead.c         | 36 ++++++++++++++++++++++++------------
>   crypto/algif_skcipher.c     | 26 +++++++++++++++++---------
>   include/crypto/if_alg.h     |  4 ++--
>   include/uapi/linux/if_alg.h |  2 ++
>   5 files changed, 50 insertions(+), 28 deletions(-)
> 
> diff --git a/crypto/af_alg.c b/crypto/af_alg.c
> index d6936c0e08d9..a35a9f854a04 100644
> --- a/crypto/af_alg.c
> +++ b/crypto/af_alg.c
> @@ -859,7 +859,7 @@ int af_alg_sendmsg(struct socket *sock, struct msghdr *msg, size_t size,
>   	struct af_alg_tsgl *sgl;
>   	struct af_alg_control con = {};
>   	long copied = 0;
> -	bool enc = 0;
> +	int op = 0;
>   	bool init = 0;
>   	int err = 0;
>   
> @@ -870,11 +870,11 @@ int af_alg_sendmsg(struct socket *sock, struct msghdr *msg, size_t size,
>   
>   		init = 1;
>   		switch (con.op) {
> +		case ALG_OP_VERIFY:
> +		case ALG_OP_SIGN:
>   		case ALG_OP_ENCRYPT:
> -			enc = 1;
> -			break;
>   		case ALG_OP_DECRYPT:
> -			enc = 0;
> +			op = con.op;
>   			break;
>   		default:
>   			return -EINVAL;
> @@ -891,7 +891,7 @@ int af_alg_sendmsg(struct socket *sock, struct msghdr *msg, size_t size,
>   	}
>   
>   	if (init) {
> -		ctx->enc = enc;
> +		ctx->op = op;
>   		if (con.iv)
>   			memcpy(ctx->iv, con.iv->iv, ivsize);
>   
> diff --git a/crypto/algif_aead.c b/crypto/algif_aead.c
> index 516b38c3a169..77abc04cf942 100644
> --- a/crypto/algif_aead.c
> +++ b/crypto/algif_aead.c
> @@ -60,7 +60,7 @@ static inline bool aead_sufficient_data(struct sock *sk)
>   	 * The minimum amount of memory needed for an AEAD cipher is
>   	 * the AAD and in case of decryption the tag.
>   	 */
> -	return ctx->used >= ctx->aead_assoclen + (ctx->enc ? 0 : as);
> +	return ctx->used >= ctx->aead_assoclen + (ctx->op ? 0 : as);
>   }
>   
>   static int aead_sendmsg(struct socket *sock, struct msghdr *msg, size_t size)
> @@ -137,7 +137,7 @@ static int _aead_recvmsg(struct socket *sock, struct msghdr *msg,
>   	 * buffer provides the tag which is consumed resulting in only the
>   	 * plaintext without a buffer for the tag returned to the caller.
>   	 */
> -	if (ctx->enc)
> +	if (ctx->op)
>   		outlen = used + as;
>   	else
>   		outlen = used - as;
> @@ -196,7 +196,7 @@ static int _aead_recvmsg(struct socket *sock, struct msghdr *msg,
>   	/* Use the RX SGL as source (and destination) for crypto op. */
>   	src = areq->first_rsgl.sgl.sg;
>   
> -	if (ctx->enc) {
> +	if (ctx->op == ALG_OP_ENCRYPT) {
>   		/*
>   		 * Encryption operation - The in-place cipher operation is
>   		 * achieved by the following operation:
> @@ -212,7 +212,7 @@ static int _aead_recvmsg(struct socket *sock, struct msghdr *msg,
>   		if (err)
>   			goto free;
>   		af_alg_pull_tsgl(sk, processed, NULL, 0);
> -	} else {
> +	} else if (ctx->op == ALG_OP_DECRYPT) {
>   		/*
>   		 * Decryption operation - To achieve an in-place cipher
>   		 * operation, the following  SGL structure is used:
> @@ -258,6 +258,9 @@ static int _aead_recvmsg(struct socket *sock, struct msghdr *msg,
>   		} else
>   			/* no RX SGL present (e.g. authentication only) */
>   			src = areq->tsgl;
> +	} else {
> +		err = -EOPNOTSUPP;
> +		goto free;
>   	}
>   
>   	/* Initialize the crypto operation */
> @@ -272,19 +275,28 @@ static int _aead_recvmsg(struct socket *sock, struct msghdr *msg,
>   		aead_request_set_callback(&areq->cra_u.aead_req,
>   					  CRYPTO_TFM_REQ_MAY_BACKLOG,
>   					  af_alg_async_cb, areq);
> -		err = ctx->enc ? crypto_aead_encrypt(&areq->cra_u.aead_req) :
> -				 crypto_aead_decrypt(&areq->cra_u.aead_req);
> -	} else {
> +	} else

Unbalanced braces around else statement.

>   		/* Synchronous operation */
>   		aead_request_set_callback(&areq->cra_u.aead_req,
>   					  CRYPTO_TFM_REQ_MAY_BACKLOG,
>   					  af_alg_complete, &ctx->completion);
> -		err = af_alg_wait_for_completion(ctx->enc ?
> -				crypto_aead_encrypt(&areq->cra_u.aead_req) :
> -				crypto_aead_decrypt(&areq->cra_u.aead_req),
> -						 &ctx->completion);
> +
> +	switch (ctx->op) {
> +	case ALG_OP_ENCRYPT:
> +		err = crypto_aead_encrypt(&areq->cra_u.aead_req);
> +		break;
> +	case ALG_OP_DECRYPT:
> +		err = crypto_aead_decrypt(&areq->cra_u.aead_req);
> +		break;
> +	default:
> +		err = -EOPNOTSUPP;
> +		goto free;
>   	}
>   
> +	/* Wait for synchronous operation completion */
> +	if (msg->msg_iocb && !is_sync_kiocb(msg->msg_iocb))
> +		err = af_alg_wait_for_completion(err, &ctx->completion);
> +
>   	/* AIO operation in progress */
>   	if (err == -EINPROGRESS) {
>   		sock_hold(sk);
> @@ -552,7 +564,7 @@ static int aead_accept_parent_nokey(void *private, struct sock *sk)
>   	ctx->rcvused = 0;
>   	ctx->more = 0;
>   	ctx->merge = 0;
> -	ctx->enc = 0;
> +	ctx->op = 0;

This implies decryption. Should we change the value of ALG_OP_DECRYPT?

>   	ctx->aead_assoclen = 0;
>   	af_alg_init_completion(&ctx->completion);
>   
> diff --git a/crypto/algif_skcipher.c b/crypto/algif_skcipher.c
> index 8ae4170aaeb4..3bf761868689 100644
> --- a/crypto/algif_skcipher.c
> +++ b/crypto/algif_skcipher.c
> @@ -121,22 +121,30 @@ static int _skcipher_recvmsg(struct socket *sock, struct msghdr *msg,
>   		skcipher_request_set_callback(&areq->cra_u.skcipher_req,
>   					      CRYPTO_TFM_REQ_MAY_SLEEP,
>   					      af_alg_async_cb, areq);
> -		err = ctx->enc ?
> -			crypto_skcipher_encrypt(&areq->cra_u.skcipher_req) :
> -			crypto_skcipher_decrypt(&areq->cra_u.skcipher_req);
> -	} else {
> +	} else

Unbalanced braces around else statement.

Cheers,
ta

>   		/* Synchronous operation */
>   		skcipher_request_set_callback(&areq->cra_u.skcipher_req,
>   					      CRYPTO_TFM_REQ_MAY_SLEEP |
>   					      CRYPTO_TFM_REQ_MAY_BACKLOG,
>   					      af_alg_complete,
>   					      &ctx->completion);
> -		err = af_alg_wait_for_completion(ctx->enc ?
> -			crypto_skcipher_encrypt(&areq->cra_u.skcipher_req) :
> -			crypto_skcipher_decrypt(&areq->cra_u.skcipher_req),
> -						 &ctx->completion);
> +
> +	switch (ctx->op) {
> +	case ALG_OP_ENCRYPT:
> +		err = crypto_skcipher_encrypt(&areq->cra_u.skcipher_req);
> +		break;
> +	case ALG_OP_DECRYPT:
> +		err = crypto_skcipher_decrypt(&areq->cra_u.skcipher_req);
> +		break;
> +	default:
> +		err = -EOPNOTSUPP;
> +		goto free;
>   	}
>   
> +	/* Wait for synchronous operation completion */
> +	if (msg->msg_iocb && !is_sync_kiocb(msg->msg_iocb))
> +		err = af_alg_wait_for_completion(err, &ctx->completion);
> +
>   	/* AIO operation in progress */
>   	if (err == -EINPROGRESS) {
>   		sock_hold(sk);
> @@ -387,7 +395,7 @@ static int skcipher_accept_parent_nokey(void *private, struct sock *sk)
>   	ctx->rcvused = 0;
>   	ctx->more = 0;
>   	ctx->merge = 0;
> -	ctx->enc = 0;
> +	ctx->op = 0;
>   	af_alg_init_completion(&ctx->completion);
>   
>   	ask->private = ctx;
> diff --git a/include/crypto/if_alg.h b/include/crypto/if_alg.h
> index 75ec9c662268..50a21488f3ba 100644
> --- a/include/crypto/if_alg.h
> +++ b/include/crypto/if_alg.h
> @@ -142,7 +142,7 @@ struct af_alg_async_req {
>    * @more:		More data to be expected from user space?
>    * @merge:		Shall new data from user space be merged into existing
>    *			SG?
> - * @enc:		Cryptographic operation to be performed when
> + * @op:			Cryptographic operation to be performed when
>    *			recvmsg is invoked.
>    * @len:		Length of memory allocated for this data structure.
>    */
> @@ -159,7 +159,7 @@ struct af_alg_ctx {
>   
>   	bool more;
>   	bool merge;
> -	bool enc;
> +	int op;
>   
>   	unsigned int len;
>   };
> diff --git a/include/uapi/linux/if_alg.h b/include/uapi/linux/if_alg.h
> index f2acd2fde1f3..d81dcca5bdd7 100644
> --- a/include/uapi/linux/if_alg.h
> +++ b/include/uapi/linux/if_alg.h
> @@ -38,5 +38,7 @@ struct af_alg_iv {
>   /* Operations */
>   #define ALG_OP_DECRYPT			0
>   #define ALG_OP_ENCRYPT			1
> +#define ALG_OP_SIGN			2
> +#define ALG_OP_VERIFY			3
>   
>   #endif	/* _LINUX_IF_ALG_H */
> 

Re: [PATCH v8 1/4] crypto: AF_ALG -- add sign/verify API

[Stephan Mueller]

Am Donnerstag, 10. August 2017, 14:49:39 CEST schrieb Tudor Ambarus:

Hi Tudor,

thanks for reviewing

> > 
> > -		err = ctx->enc ? crypto_aead_encrypt(&areq->cra_u.aead_req) :
> > -				 crypto_aead_decrypt(&areq->cra_u.aead_req);
> > -	} else {
> > +	} else
> 
> Unbalanced braces around else statement.

Is there a style requirement for that? checkpatch.pl does not complain. I 
thought that one liners in a conditional should not have braces?

> > -	ctx->enc = 0;
> > +	ctx->op = 0;
> 
> This implies decryption. Should we change the value of ALG_OP_DECRYPT?

ALG_OP_DECRYPT is a user space interface, so we cannot change it.

Do you see harm in leaving it as is? Note, I did not want to introduce 
functional changes that have no bearing on the addition of the sign/verify 
API. If you think this is problematic, I would like to add another patch that 
is dedicated to fix this.

> > -		err = ctx->enc ?
> > -			crypto_skcipher_encrypt(&areq->cra_u.skcipher_req) :
> > -			crypto_skcipher_decrypt(&areq->cra_u.skcipher_req);
> > -	} else {
> > +	} else
> 
> Unbalanced braces around else statement.

Same as above.

Thanks a lot!

Ciao
Stephan

Re: [PATCH v8 1/4] crypto: AF_ALG -- add sign/verify API

[Tudor Ambarus]

On 08/10/2017 04:03 PM, Stephan Mueller wrote:
> Is there a style requirement for that? checkpatch.pl does not complain. I
> thought that one liners in a conditional should not have braces?

Linux coding style requires braces in both branches when you have a
branch with a statement and the other with multiple statements.

Checkpatch complains about this when you run it with --strict option.

Cheers,
ta

Re: [PATCH v8 1/4] crypto: AF_ALG -- add sign/verify API

[Stephan Müller]

Am Donnerstag, 10. August 2017, 15:59:33 CEST schrieb Tudor Ambarus:

Hi Tudor,

> On 08/10/2017 04:03 PM, Stephan Mueller wrote:
> > Is there a style requirement for that? checkpatch.pl does not complain. I
> > thought that one liners in a conditional should not have braces?
> 
> Linux coding style requires braces in both branches when you have a
> branch with a statement and the other with multiple statements.
> 
> Checkpatch complains about this when you run it with --strict option.

Ok, then I will add it.

Thanks

Ciao
Stephan

[PATCH v8 2/4] crypto: AF_ALG -- add setpubkey setsockopt call

[Stephan Müller]

For supporting asymmetric ciphers, user space must be able to set the
public key. The patch adds a new setsockopt call for setting the public
key.

Signed-off-by: Stephan Mueller <smueller@chronox.de>
---
 crypto/af_alg.c             | 18 +++++++++++++-----
 include/crypto/if_alg.h     |  1 +
 include/uapi/linux/if_alg.h |  1 +
 3 files changed, 15 insertions(+), 5 deletions(-)

diff --git a/crypto/af_alg.c b/crypto/af_alg.c
index a35a9f854a04..176921d7593a 100644
--- a/crypto/af_alg.c
+++ b/crypto/af_alg.c
@@ -203,13 +203,17 @@ static int alg_bind(struct socket *sock, struct sockaddr *uaddr, int addr_len)
 }
 
 static int alg_setkey(struct sock *sk, char __user *ukey,
-		      unsigned int keylen)
+		      unsigned int keylen,
+		      int (*setkey)(void *private, const u8 *key,
+				    unsigned int keylen))
 {
 	struct alg_sock *ask = alg_sk(sk);
-	const struct af_alg_type *type = ask->type;
 	u8 *key;
 	int err;
 
+	if (!setkey)
+		return -ENOPROTOOPT;
+
 	key = sock_kmalloc(sk, keylen, GFP_KERNEL);
 	if (!key)
 		return -ENOMEM;
@@ -218,7 +222,7 @@ static int alg_setkey(struct sock *sk, char __user *ukey,
 	if (copy_from_user(key, ukey, keylen))
 		goto out;
 
-	err = type->setkey(ask->private, key, keylen);
+	err = setkey(ask->private, key, keylen);
 
 out:
 	sock_kzfree_s(sk, key, keylen);
@@ -248,10 +252,14 @@ static int alg_setsockopt(struct socket *sock, int level, int optname,
 	case ALG_SET_KEY:
 		if (sock->state == SS_CONNECTED)
 			goto unlock;
-		if (!type->setkey)
+
+		err = alg_setkey(sk, optval, optlen, type->setkey);
+		break;
+	case ALG_SET_PUBKEY:
+		if (sock->state == SS_CONNECTED)
 			goto unlock;
 
-		err = alg_setkey(sk, optval, optlen);
+		err = alg_setkey(sk, optval, optlen, type->setpubkey);
 		break;
 	case ALG_SET_AEAD_AUTHSIZE:
 		if (sock->state == SS_CONNECTED)
diff --git a/include/crypto/if_alg.h b/include/crypto/if_alg.h
index 50a21488f3ba..d1de8ed3e77b 100644
--- a/include/crypto/if_alg.h
+++ b/include/crypto/if_alg.h
@@ -55,6 +55,7 @@ struct af_alg_type {
 	void *(*bind)(const char *name, u32 type, u32 mask);
 	void (*release)(void *private);
 	int (*setkey)(void *private, const u8 *key, unsigned int keylen);
+	int (*setpubkey)(void *private, const u8 *key, unsigned int keylen);
 	int (*accept)(void *private, struct sock *sk);
 	int (*accept_nokey)(void *private, struct sock *sk);
 	int (*setauthsize)(void *private, unsigned int authsize);
diff --git a/include/uapi/linux/if_alg.h b/include/uapi/linux/if_alg.h
index d81dcca5bdd7..02e61627e089 100644
--- a/include/uapi/linux/if_alg.h
+++ b/include/uapi/linux/if_alg.h
@@ -34,6 +34,7 @@ struct af_alg_iv {
 #define ALG_SET_OP			3
 #define ALG_SET_AEAD_ASSOCLEN		4
 #define ALG_SET_AEAD_AUTHSIZE		5
+#define ALG_SET_PUBKEY			6
 
 /* Operations */
 #define ALG_OP_DECRYPT			0
-- 
2.13.4

[PATCH v8 3/4] crypto: AF_ALG -- add asymmetric cipher

[Stephan Müller]

This patch adds the user space interface for asymmetric ciphers. The
interface allows the use of sendmsg as well as vmsplice to provide data.

The akcipher interface implementation uses the common AF_ALG interface
code regarding TX and RX SGL handling.

Signed-off-by: Stephan Mueller <smueller@chronox.de>
---
 crypto/algif_akcipher.c | 466 ++++++++++++++++++++++++++++++++++++++++++++++++
 include/crypto/if_alg.h |   2 +
 2 files changed, 468 insertions(+)
 create mode 100644 crypto/algif_akcipher.c

diff --git a/crypto/algif_akcipher.c b/crypto/algif_akcipher.c
new file mode 100644
index 000000000000..1b36eb0b6e8f
--- /dev/null
+++ b/crypto/algif_akcipher.c
@@ -0,0 +1,466 @@
+/*
+ * algif_akcipher: User-space interface for asymmetric cipher algorithms
+ *
+ * Copyright (C) 2017, Stephan Mueller <smueller@chronox.de>
+ *
+ * This file provides the user-space API for asymmetric ciphers.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the Free
+ * Software Foundation; either version 2 of the License, or (at your option)
+ * any later version.
+ *
+ * The following concept of the memory management is used:
+ *
+ * The kernel maintains two SGLs, the TX SGL and the RX SGL. The TX SGL is
+ * filled by user space with the data submitted via sendpage/sendmsg. Filling
+ * up the TX SGL does not cause a crypto operation -- the data will only be
+ * tracked by the kernel. Upon receipt of one recvmsg call, the caller must
+ * provide a buffer which is tracked with the RX SGL.
+ *
+ * During the processing of the recvmsg operation, the cipher request is
+ * allocated and prepared. As part of the recvmsg operation, the processed
+ * TX buffers are extracted from the TX SGL into a separate SGL.
+ *
+ * After the completion of the crypto operation, the RX SGL and the cipher
+ * request is released. The extracted TX SGL parts are released together with
+ * the RX SGL release.
+ */
+
+#include <crypto/akcipher.h>
+#include <crypto/scatterwalk.h>
+#include <crypto/if_alg.h>
+#include <linux/init.h>
+#include <linux/list.h>
+#include <linux/kernel.h>
+#include <linux/mm.h>
+#include <linux/module.h>
+#include <linux/net.h>
+#include <net/sock.h>
+
+struct akcipher_tfm {
+	struct crypto_akcipher *akcipher;
+	bool has_key;
+};
+
+static int akcipher_sendmsg(struct socket *sock, struct msghdr *msg,
+			    size_t size)
+{
+	return af_alg_sendmsg(sock, msg, size, 0);
+}
+
+static int _akcipher_recvmsg(struct socket *sock, struct msghdr *msg,
+			     size_t ignored, int flags)
+{
+	struct sock *sk = sock->sk;
+	struct alg_sock *ask = alg_sk(sk);
+	struct sock *psk = ask->parent;
+	struct alg_sock *pask = alg_sk(psk);
+	struct af_alg_ctx *ctx = ask->private;
+	struct akcipher_tfm *akc = pask->private;
+	struct crypto_akcipher *tfm = akc->akcipher;
+	struct af_alg_async_req *areq;
+	int err = 0;
+	int maxsize;
+	size_t len = 0;
+	size_t used = 0;
+
+	maxsize = crypto_akcipher_maxsize(tfm);
+	if (maxsize < 0)
+		return maxsize;
+
+	/* Allocate cipher request for current operation. */
+	areq = af_alg_alloc_areq(sk, sizeof(struct af_alg_async_req) +
+				     crypto_akcipher_reqsize(tfm));
+	if (IS_ERR(areq))
+		return PTR_ERR(areq);
+
+	/* convert iovecs of output buffers into RX SGL */
+	err = af_alg_get_rsgl(sk, msg, flags, areq, maxsize, &len);
+	if (err)
+		goto free;
+
+	/* ensure output buffer is sufficiently large */
+	if (len < maxsize) {
+		err = -EMSGSIZE;
+		goto free;
+	}
+
+	/*
+	 * Create a per request TX SGL for this request which tracks the
+	 * SG entries from the global TX SGL.
+	 */
+	used = ctx->used;
+	areq->tsgl_entries = af_alg_count_tsgl(sk, used, 0);
+	if (!areq->tsgl_entries)
+		areq->tsgl_entries = 1;
+	areq->tsgl = sock_kmalloc(sk, sizeof(*areq->tsgl) * areq->tsgl_entries,
+				  GFP_KERNEL);
+	if (!areq->tsgl) {
+		err = -ENOMEM;
+		goto free;
+	}
+	sg_init_table(areq->tsgl, areq->tsgl_entries);
+	af_alg_pull_tsgl(sk, used, areq->tsgl, 0);
+
+	/* Initialize the crypto operation */
+	akcipher_request_set_tfm(&areq->cra_u.akcipher_req, tfm);
+	akcipher_request_set_crypt(&areq->cra_u.akcipher_req, areq->tsgl,
+				   areq->first_rsgl.sgl.sg, used, len);
+
+	if (msg->msg_iocb && !is_sync_kiocb(msg->msg_iocb)) {
+		/* AIO operation */
+		areq->iocb = msg->msg_iocb;
+		akcipher_request_set_callback(&areq->cra_u.akcipher_req,
+					      CRYPTO_TFM_REQ_MAY_SLEEP,
+					      af_alg_async_cb, areq);
+	} else
+		/* Synchronous operation */
+		akcipher_request_set_callback(&areq->cra_u.akcipher_req,
+					      CRYPTO_TFM_REQ_MAY_SLEEP |
+					      CRYPTO_TFM_REQ_MAY_BACKLOG,
+					      af_alg_complete,
+					      &ctx->completion);
+
+	switch (ctx->op) {
+	case ALG_OP_ENCRYPT:
+		err = crypto_akcipher_encrypt(&areq->cra_u.akcipher_req);
+		break;
+	case ALG_OP_DECRYPT:
+		err = crypto_akcipher_decrypt(&areq->cra_u.akcipher_req);
+		break;
+	case ALG_OP_SIGN:
+		err = crypto_akcipher_sign(&areq->cra_u.akcipher_req);
+		break;
+	case ALG_OP_VERIFY:
+		err = crypto_akcipher_verify(&areq->cra_u.akcipher_req);
+		break;
+	default:
+		err = -EOPNOTSUPP;
+		goto free;
+	}
+
+	/* Wait for synchronous operation completion */
+	if (msg->msg_iocb && !is_sync_kiocb(msg->msg_iocb))
+		err = af_alg_wait_for_completion(err, &ctx->completion);
+
+	/* AIO operation in progress */
+	if (err == -EINPROGRESS) {
+		sock_hold(sk);
+
+		/* Remember output size that will be generated. */
+		areq->outlen = areq->cra_u.akcipher_req.dst_len;
+
+		return -EIOCBQUEUED;
+	}
+
+free:
+	af_alg_free_areq_sgls(areq);
+	sock_kfree_s(sk, areq, areq->areqlen);
+
+	return err ? err : areq->cra_u.akcipher_req.dst_len;
+}
+
+static int akcipher_recvmsg(struct socket *sock, struct msghdr *msg,
+			    size_t ignored, int flags)
+{
+	struct sock *sk = sock->sk;
+	struct alg_sock *ask = alg_sk(sk);
+	struct sock *psk = ask->parent;
+	struct alg_sock *pask = alg_sk(psk);
+	struct akcipher_tfm *akc = pask->private;
+	struct crypto_akcipher *tfm = akc->akcipher;
+
+	int ret = 0;
+
+	lock_sock(sk);
+
+	while (msg_data_left(msg)) {
+		int err = _akcipher_recvmsg(sock, msg, ignored, flags);
+
+		/*
+		 * This error covers -EIOCBQUEUED which implies that we can
+		 * only handle one AIO request. If the caller wants to have
+		 * multiple AIO requests in parallel, he must make multiple
+		 * separate AIO calls.
+		 */
+		if (err <= 0) {
+			if (err == -EIOCBQUEUED || err == -EBADMSG || !ret)
+				ret = err;
+			goto out;
+		}
+
+		ret += err;
+
+		/*
+		 * The caller must provide crypto_akcipher_maxsize per request.
+		 * If he provides more, we conclude that multiple akcipher
+		 * operations are requested.
+		 */
+		iov_iter_advance(&msg->msg_iter,
+				 crypto_akcipher_maxsize(tfm) - err);
+	}
+
+out:
+	af_alg_wmem_wakeup(sk);
+	release_sock(sk);
+	return ret;
+}
+
+static struct proto_ops algif_akcipher_ops = {
+	.family		=	PF_ALG,
+
+	.connect	=	sock_no_connect,
+	.socketpair	=	sock_no_socketpair,
+	.getname	=	sock_no_getname,
+	.ioctl		=	sock_no_ioctl,
+	.listen		=	sock_no_listen,
+	.shutdown	=	sock_no_shutdown,
+	.getsockopt	=	sock_no_getsockopt,
+	.mmap		=	sock_no_mmap,
+	.bind		=	sock_no_bind,
+	.accept		=	sock_no_accept,
+	.setsockopt	=	sock_no_setsockopt,
+
+	.release	=	af_alg_release,
+	.sendmsg	=	akcipher_sendmsg,
+	.sendpage	=	af_alg_sendpage,
+	.recvmsg	=	akcipher_recvmsg,
+	.poll		=	af_alg_poll,
+};
+
+static int akcipher_check_key(struct socket *sock)
+{
+	int err = 0;
+	struct sock *psk;
+	struct alg_sock *pask;
+	struct akcipher_tfm *tfm;
+	struct sock *sk = sock->sk;
+	struct alg_sock *ask = alg_sk(sk);
+
+	lock_sock(sk);
+	if (ask->refcnt)
+		goto unlock_child;
+
+	psk = ask->parent;
+	pask = alg_sk(ask->parent);
+	tfm = pask->private;
+
+	err = -ENOKEY;
+	lock_sock_nested(psk, SINGLE_DEPTH_NESTING);
+	if (!tfm->has_key)
+		goto unlock;
+
+	if (!pask->refcnt++)
+		sock_hold(psk);
+
+	ask->refcnt = 1;
+	sock_put(psk);
+
+	err = 0;
+
+unlock:
+	release_sock(psk);
+unlock_child:
+	release_sock(sk);
+
+	return err;
+}
+
+static int akcipher_sendmsg_nokey(struct socket *sock, struct msghdr *msg,
+				  size_t size)
+{
+	int err;
+
+	err = akcipher_check_key(sock);
+	if (err)
+		return err;
+
+	return akcipher_sendmsg(sock, msg, size);
+}
+
+static ssize_t akcipher_sendpage_nokey(struct socket *sock, struct page *page,
+				       int offset, size_t size, int flags)
+{
+	int err;
+
+	err = akcipher_check_key(sock);
+	if (err)
+		return err;
+
+	return af_alg_sendpage(sock, page, offset, size, flags);
+}
+
+static int akcipher_recvmsg_nokey(struct socket *sock, struct msghdr *msg,
+				  size_t ignored, int flags)
+{
+	int err;
+
+	err = akcipher_check_key(sock);
+	if (err)
+		return err;
+
+	return akcipher_recvmsg(sock, msg, ignored, flags);
+}
+
+static struct proto_ops algif_akcipher_ops_nokey = {
+	.family		=	PF_ALG,
+
+	.connect	=	sock_no_connect,
+	.socketpair	=	sock_no_socketpair,
+	.getname	=	sock_no_getname,
+	.ioctl		=	sock_no_ioctl,
+	.listen		=	sock_no_listen,
+	.shutdown	=	sock_no_shutdown,
+	.getsockopt	=	sock_no_getsockopt,
+	.mmap		=	sock_no_mmap,
+	.bind		=	sock_no_bind,
+	.accept		=	sock_no_accept,
+	.setsockopt	=	sock_no_setsockopt,
+
+	.release	=	af_alg_release,
+	.sendmsg	=	akcipher_sendmsg_nokey,
+	.sendpage	=	akcipher_sendpage_nokey,
+	.recvmsg	=	akcipher_recvmsg_nokey,
+	.poll		=	af_alg_poll,
+};
+
+static void *akcipher_bind(const char *name, u32 type, u32 mask)
+{
+	struct akcipher_tfm *tfm;
+	struct crypto_akcipher *akcipher;
+
+	tfm = kzalloc(sizeof(*tfm), GFP_KERNEL);
+	if (!tfm)
+		return ERR_PTR(-ENOMEM);
+
+	akcipher = crypto_alloc_akcipher(name, type, mask);
+	if (IS_ERR(akcipher)) {
+		kfree(tfm);
+		return ERR_CAST(akcipher);
+	}
+
+	tfm->akcipher = akcipher;
+
+	return tfm;
+}
+
+static void akcipher_release(void *private)
+{
+	struct akcipher_tfm *tfm = private;
+	struct crypto_akcipher *akcipher = tfm->akcipher;
+
+	crypto_free_akcipher(akcipher);
+	kfree(tfm);
+}
+
+static int akcipher_setprivkey(void *private, const u8 *key,
+			       unsigned int keylen)
+{
+	struct akcipher_tfm *tfm = private;
+	struct crypto_akcipher *akcipher = tfm->akcipher;
+	int err;
+
+	err = crypto_akcipher_set_priv_key(akcipher, key, keylen);
+	tfm->has_key = !err;
+
+	/* Return the maximum size of the akcipher operation. */
+	if (!err)
+		err = crypto_akcipher_maxsize(akcipher);
+
+	return err;
+}
+
+static int akcipher_setpubkey(void *private, const u8 *key, unsigned int keylen)
+{
+	struct akcipher_tfm *tfm = private;
+	struct crypto_akcipher *akcipher = tfm->akcipher;
+	int err;
+
+	err = crypto_akcipher_set_pub_key(akcipher, key, keylen);
+	tfm->has_key = !err;
+
+	/* Return the maximum size of the akcipher operation. */
+	if (!err)
+		err = crypto_akcipher_maxsize(akcipher);
+
+	return err;
+}
+
+static void akcipher_sock_destruct(struct sock *sk)
+{
+	struct alg_sock *ask = alg_sk(sk);
+	struct af_alg_ctx *ctx = ask->private;
+
+	af_alg_pull_tsgl(sk, ctx->used, NULL, 0);
+	sock_kfree_s(sk, ctx, ctx->len);
+	af_alg_release_parent(sk);
+}
+
+static int akcipher_accept_parent_nokey(void *private, struct sock *sk)
+{
+	struct af_alg_ctx *ctx;
+	struct alg_sock *ask = alg_sk(sk);
+	unsigned int len = sizeof(*ctx);
+
+	ctx = sock_kmalloc(sk, len, GFP_KERNEL);
+	if (!ctx)
+		return -ENOMEM;
+	memset(ctx, 0, len);
+
+	INIT_LIST_HEAD(&ctx->tsgl_list);
+	ctx->len = len;
+	ctx->used = 0;
+	ctx->rcvused = 0;
+	ctx->more = 0;
+	ctx->merge = 0;
+	ctx->op = 0;
+	af_alg_init_completion(&ctx->completion);
+
+	ask->private = ctx;
+
+	sk->sk_destruct = akcipher_sock_destruct;
+
+	return 0;
+}
+
+static int akcipher_accept_parent(void *private, struct sock *sk)
+{
+	struct akcipher_tfm *tfm = private;
+
+	if (!tfm->has_key)
+		return -ENOKEY;
+
+	return akcipher_accept_parent_nokey(private, sk);
+}
+
+static const struct af_alg_type algif_type_akcipher = {
+	.bind		=	akcipher_bind,
+	.release	=	akcipher_release,
+	.setkey		=	akcipher_setprivkey,
+	.setpubkey	=	akcipher_setpubkey,
+	.setauthsize	=	NULL,
+	.accept		=	akcipher_accept_parent,
+	.accept_nokey	=	akcipher_accept_parent_nokey,
+	.ops		=	&algif_akcipher_ops,
+	.ops_nokey	=	&algif_akcipher_ops_nokey,
+	.name		=	"akcipher",
+	.owner		=	THIS_MODULE
+};
+
+static int __init algif_akcipher_init(void)
+{
+	return af_alg_register_type(&algif_type_akcipher);
+}
+
+static void __exit algif_akcipher_exit(void)
+{
+	int err = af_alg_unregister_type(&algif_type_akcipher);
+	BUG_ON(err);
+}
+
+module_init(algif_akcipher_init);
+module_exit(algif_akcipher_exit);
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Stephan Mueller <smueller@chronox.de>");
+MODULE_DESCRIPTION("Asymmetric kernel crypto API user space interface");
diff --git a/include/crypto/if_alg.h b/include/crypto/if_alg.h
index d1de8ed3e77b..a2b396756e24 100644
--- a/include/crypto/if_alg.h
+++ b/include/crypto/if_alg.h
@@ -22,6 +22,7 @@
 
 #include <crypto/aead.h>
 #include <crypto/skcipher.h>
+#include <crypto/akcipher.h>
 
 #define ALG_MAX_PAGES			16
 
@@ -119,6 +120,7 @@ struct af_alg_async_req {
 	union {
 		struct aead_request aead_req;
 		struct skcipher_request skcipher_req;
+		struct akcipher_request akcipher_req;
 	} cra_u;
 
 	/* req ctx trails this struct */
-- 
2.13.4

Re: [PATCH v8 3/4] crypto: AF_ALG -- add asymmetric cipher

[Tudor Ambarus]

Hi, Stephan,

On 08/10/2017 09:40 AM, Stephan Müller wrote:
> This patch adds the user space interface for asymmetric ciphers. The
> interface allows the use of sendmsg as well as vmsplice to provide data.
> 
> The akcipher interface implementation uses the common AF_ALG interface
> code regarding TX and RX SGL handling.
> 
> Signed-off-by: Stephan Mueller <smueller@chronox.de>
> ---
>   crypto/algif_akcipher.c | 466 ++++++++++++++++++++++++++++++++++++++++++++++++
>   include/crypto/if_alg.h |   2 +
>   2 files changed, 468 insertions(+)
>   create mode 100644 crypto/algif_akcipher.c
> 
> diff --git a/crypto/algif_akcipher.c b/crypto/algif_akcipher.c
> new file mode 100644
> index 000000000000..1b36eb0b6e8f
> --- /dev/null
> +++ b/crypto/algif_akcipher.c
> @@ -0,0 +1,466 @@
> +/*
> + * algif_akcipher: User-space interface for asymmetric cipher algorithms
> + *
> + * Copyright (C) 2017, Stephan Mueller <smueller@chronox.de>
> + *
> + * This file provides the user-space API for asymmetric ciphers.
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License as published by the Free
> + * Software Foundation; either version 2 of the License, or (at your option)
> + * any later version.
> + *
> + * The following concept of the memory management is used:
> + *
> + * The kernel maintains two SGLs, the TX SGL and the RX SGL. The TX SGL is
> + * filled by user space with the data submitted via sendpage/sendmsg. Filling
> + * up the TX SGL does not cause a crypto operation -- the data will only be
> + * tracked by the kernel. Upon receipt of one recvmsg call, the caller must
> + * provide a buffer which is tracked with the RX SGL.
> + *
> + * During the processing of the recvmsg operation, the cipher request is
> + * allocated and prepared. As part of the recvmsg operation, the processed
> + * TX buffers are extracted from the TX SGL into a separate SGL.
> + *
> + * After the completion of the crypto operation, the RX SGL and the cipher
> + * request is released. The extracted TX SGL parts are released together with
> + * the RX SGL release.
> + */
> +
> +#include <crypto/akcipher.h>
> +#include <crypto/scatterwalk.h>
> +#include <crypto/if_alg.h>
> +#include <linux/init.h>
> +#include <linux/list.h>
> +#include <linux/kernel.h>
> +#include <linux/mm.h>
> +#include <linux/module.h>
> +#include <linux/net.h>
> +#include <net/sock.h>

I like that you ordered these alphabetically. Is there a reason why
crypto/scatterwalk.h is not after crypto/if_alg.h?

> +
> +struct akcipher_tfm {
> +	struct crypto_akcipher *akcipher;
> +	bool has_key;
> +};
> +
> +static int akcipher_sendmsg(struct socket *sock, struct msghdr *msg,
> +			    size_t size)
> +{
> +	return af_alg_sendmsg(sock, msg, size, 0);
> +}
> +
> +static int _akcipher_recvmsg(struct socket *sock, struct msghdr *msg,
> +			     size_t ignored, int flags)
> +{
> +	struct sock *sk = sock->sk;
> +	struct alg_sock *ask = alg_sk(sk);
> +	struct sock *psk = ask->parent;
> +	struct alg_sock *pask = alg_sk(psk);
> +	struct af_alg_ctx *ctx = ask->private;
> +	struct akcipher_tfm *akc = pask->private;
> +	struct crypto_akcipher *tfm = akc->akcipher;
> +	struct af_alg_async_req *areq;
> +	int err = 0;

initialization not needed.

> +	int maxsize;
> +	size_t len = 0;

initialization not needed. len will be initialized in af_alg_get_rsgl.

Also, size_t could be 32 or 64 bit wide. Could you declare the size_t
variables before the int ones? This will avoid stack padding on 64bit
systems if one adds a new int variable in the same place where the ints
are now.

> +	size_t used = 0;

initialization to zero not needed. You can directly initialize to
ctx->used or don't initialize at all.

> +
> +	maxsize = crypto_akcipher_maxsize(tfm);
> +	if (maxsize < 0)
> +		return maxsize;
> +
> +	/* Allocate cipher request for current operation. */
> +	areq = af_alg_alloc_areq(sk, sizeof(struct af_alg_async_req) +
> +				     crypto_akcipher_reqsize(tfm));
> +	if (IS_ERR(areq))
> +		return PTR_ERR(areq);
> +
> +	/* convert iovecs of output buffers into RX SGL */
> +	err = af_alg_get_rsgl(sk, msg, flags, areq, maxsize, &len);
> +	if (err)
> +		goto free;
> +
> +	/* ensure output buffer is sufficiently large */
> +	if (len < maxsize) {
> +		err = -EMSGSIZE;
> +		goto free;
> +	}
> +
> +	/*
> +	 * Create a per request TX SGL for this request which tracks the
> +	 * SG entries from the global TX SGL.
> +	 */
> +	used = ctx->used;
> +	areq->tsgl_entries = af_alg_count_tsgl(sk, used, 0);
> +	if (!areq->tsgl_entries)
> +		areq->tsgl_entries = 1;
> +	areq->tsgl = sock_kmalloc(sk, sizeof(*areq->tsgl) * areq->tsgl_entries,
> +				  GFP_KERNEL);
> +	if (!areq->tsgl) {
> +		err = -ENOMEM;
> +		goto free;
> +	}
> +	sg_init_table(areq->tsgl, areq->tsgl_entries);
> +	af_alg_pull_tsgl(sk, used, areq->tsgl, 0);
> +
> +	/* Initialize the crypto operation */
> +	akcipher_request_set_tfm(&areq->cra_u.akcipher_req, tfm);
> +	akcipher_request_set_crypt(&areq->cra_u.akcipher_req, areq->tsgl,
> +				   areq->first_rsgl.sgl.sg, used, len);
> +
> +	if (msg->msg_iocb && !is_sync_kiocb(msg->msg_iocb)) {
> +		/* AIO operation */
> +		areq->iocb = msg->msg_iocb;
> +		akcipher_request_set_callback(&areq->cra_u.akcipher_req,
> +					      CRYPTO_TFM_REQ_MAY_SLEEP,
> +					      af_alg_async_cb, areq);
> +	} else

Unbalanced braces around else statement.

> +		/* Synchronous operation */
> +		akcipher_request_set_callback(&areq->cra_u.akcipher_req,
> +					      CRYPTO_TFM_REQ_MAY_SLEEP |
> +					      CRYPTO_TFM_REQ_MAY_BACKLOG,
> +					      af_alg_complete,
> +					      &ctx->completion);
> +
> +	switch (ctx->op) {
> +	case ALG_OP_ENCRYPT:
> +		err = crypto_akcipher_encrypt(&areq->cra_u.akcipher_req);
> +		break;
> +	case ALG_OP_DECRYPT:
> +		err = crypto_akcipher_decrypt(&areq->cra_u.akcipher_req);
> +		break;
> +	case ALG_OP_SIGN:
> +		err = crypto_akcipher_sign(&areq->cra_u.akcipher_req);
> +		break;
> +	case ALG_OP_VERIFY:
> +		err = crypto_akcipher_verify(&areq->cra_u.akcipher_req);
> +		break;
> +	default:
> +		err = -EOPNOTSUPP;
> +		goto free;
> +	}
> +
> +	/* Wait for synchronous operation completion */
> +	if (msg->msg_iocb && !is_sync_kiocb(msg->msg_iocb))
> +		err = af_alg_wait_for_completion(err, &ctx->completion);
> +
> +	/* AIO operation in progress */
> +	if (err == -EINPROGRESS) {
> +		sock_hold(sk);
> +
> +		/* Remember output size that will be generated. */
> +		areq->outlen = areq->cra_u.akcipher_req.dst_len;

don't you have the dst_len value in len variable?

> +
> +		return -EIOCBQUEUED;
> +	}
> +
> +free:
> +	af_alg_free_areq_sgls(areq);
> +	sock_kfree_s(sk, areq, areq->areqlen);
> +
> +	return err ? err : areq->cra_u.akcipher_req.dst_len;
> +}
> +
> +static int akcipher_recvmsg(struct socket *sock, struct msghdr *msg,
> +			    size_t ignored, int flags)
> +{
> +	struct sock *sk = sock->sk;
> +	struct alg_sock *ask = alg_sk(sk);
> +	struct sock *psk = ask->parent;
> +	struct alg_sock *pask = alg_sk(psk);
> +	struct akcipher_tfm *akc = pask->private;
> +	struct crypto_akcipher *tfm = akc->akcipher;
> +

blank line

> +	int ret = 0;
> +
> +	lock_sock(sk);
> +
> +	while (msg_data_left(msg)) {
> +		int err = _akcipher_recvmsg(sock, msg, ignored, flags);

If the compiler will not make any optimization, you will end up in
allocating an int on the stack for each iteration.

> +
> +		/*
> +		 * This error covers -EIOCBQUEUED which implies that we can
> +		 * only handle one AIO request. If the caller wants to have
> +		 * multiple AIO requests in parallel, he must make multiple
> +		 * separate AIO calls.
> +		 */
> +		if (err <= 0) {

why the equal?

> +			if (err == -EIOCBQUEUED || err == -EBADMSG || !ret)
> +				ret = err;
> +			goto out;
> +		}
> +
> +		ret += err;
> +
> +		/*
> +		 * The caller must provide crypto_akcipher_maxsize per request.
> +		 * If he provides more, we conclude that multiple akcipher
> +		 * operations are requested.
> +		 */
> +		iov_iter_advance(&msg->msg_iter,
> +				 crypto_akcipher_maxsize(tfm) - err);
> +	}
> +
> +out:
> +	af_alg_wmem_wakeup(sk);
> +	release_sock(sk);
> +	return ret;
> +}
> +
> +static struct proto_ops algif_akcipher_ops = {

static const struct?

> +	.family		=	PF_ALG,
> +
> +	.connect	=	sock_no_connect,
> +	.socketpair	=	sock_no_socketpair,
> +	.getname	=	sock_no_getname,
> +	.ioctl		=	sock_no_ioctl,
> +	.listen		=	sock_no_listen,
> +	.shutdown	=	sock_no_shutdown,
> +	.getsockopt	=	sock_no_getsockopt,
> +	.mmap		=	sock_no_mmap,
> +	.bind		=	sock_no_bind,
> +	.accept		=	sock_no_accept,
> +	.setsockopt	=	sock_no_setsockopt,
> +
> +	.release	=	af_alg_release,
> +	.sendmsg	=	akcipher_sendmsg,
> +	.sendpage	=	af_alg_sendpage,
> +	.recvmsg	=	akcipher_recvmsg,
> +	.poll		=	af_alg_poll,
> +};
> +
> +static int akcipher_check_key(struct socket *sock)
> +{
> +	int err = 0;

initialization not needed.
You can avoid stack padding on 64bit systems if you move the int after
pointers.

> +	struct sock *psk;
> +	struct alg_sock *pask;
> +	struct akcipher_tfm *tfm;
> +	struct sock *sk = sock->sk;
> +	struct alg_sock *ask = alg_sk(sk);
> +
> +	lock_sock(sk);
> +	if (ask->refcnt)
> +		goto unlock_child;
> +
> +	psk = ask->parent;
> +	pask = alg_sk(ask->parent);
> +	tfm = pask->private;
> +
> +	err = -ENOKEY;

see https://rusty.ozlabs.org/?p=232

> +	lock_sock_nested(psk, SINGLE_DEPTH_NESTING);
> +	if (!tfm->has_key)
> +		goto unlock;
> +
> +	if (!pask->refcnt++)
> +		sock_hold(psk);
> +
> +	ask->refcnt = 1;
> +	sock_put(psk);
> +
> +	err = 0;
> +
> +unlock:
> +	release_sock(psk);
> +unlock_child:
> +	release_sock(sk);
> +
> +	return err;
> +}
> +
> +static int akcipher_sendmsg_nokey(struct socket *sock, struct msghdr *msg,
> +				  size_t size)
> +{
> +	int err;
> +
> +	err = akcipher_check_key(sock);
> +	if (err)
> +		return err;
> +
> +	return akcipher_sendmsg(sock, msg, size);
> +}
> +
> +static ssize_t akcipher_sendpage_nokey(struct socket *sock, struct page *page,
> +				       int offset, size_t size, int flags)
> +{
> +	int err;
> +
> +	err = akcipher_check_key(sock);
> +	if (err)
> +		return err;
> +
> +	return af_alg_sendpage(sock, page, offset, size, flags);
> +}
> +
> +static int akcipher_recvmsg_nokey(struct socket *sock, struct msghdr *msg,
> +				  size_t ignored, int flags)
> +{
> +	int err;
> +
> +	err = akcipher_check_key(sock);
> +	if (err)
> +		return err;
> +
> +	return akcipher_recvmsg(sock, msg, ignored, flags);
> +}
> +
> +static struct proto_ops algif_akcipher_ops_nokey = {

static const struct?

> +	.family		=	PF_ALG,
> +
> +	.connect	=	sock_no_connect,
> +	.socketpair	=	sock_no_socketpair,
> +	.getname	=	sock_no_getname,
> +	.ioctl		=	sock_no_ioctl,
> +	.listen		=	sock_no_listen,
> +	.shutdown	=	sock_no_shutdown,
> +	.getsockopt	=	sock_no_getsockopt,
> +	.mmap		=	sock_no_mmap,
> +	.bind		=	sock_no_bind,
> +	.accept		=	sock_no_accept,
> +	.setsockopt	=	sock_no_setsockopt,
> +
> +	.release	=	af_alg_release,
> +	.sendmsg	=	akcipher_sendmsg_nokey,
> +	.sendpage	=	akcipher_sendpage_nokey,
> +	.recvmsg	=	akcipher_recvmsg_nokey,
> +	.poll		=	af_alg_poll,
> +};
> +
> +static void *akcipher_bind(const char *name, u32 type, u32 mask)
> +{
> +	struct akcipher_tfm *tfm;
> +	struct crypto_akcipher *akcipher;
> +
> +	tfm = kzalloc(sizeof(*tfm), GFP_KERNEL);
> +	if (!tfm)
> +		return ERR_PTR(-ENOMEM);
> +
> +	akcipher = crypto_alloc_akcipher(name, type, mask);
> +	if (IS_ERR(akcipher)) {
> +		kfree(tfm);
> +		return ERR_CAST(akcipher);
> +	}
> +
> +	tfm->akcipher = akcipher;

tfm->akcipher was initialized to zero and now you overwrite it.
Maybe it's better to use kmalloc and tfm->has_key = false so you
can get rid of the superfluous initialization.

> +
> +	return tfm;
> +}
> +
> +static void akcipher_release(void *private)
> +{
> +	struct akcipher_tfm *tfm = private;
> +	struct crypto_akcipher *akcipher = tfm->akcipher;
> +
> +	crypto_free_akcipher(akcipher);
> +	kfree(tfm);
> +}
> +
> +static int akcipher_setprivkey(void *private, const u8 *key,
> +			       unsigned int keylen)
> +{
> +	struct akcipher_tfm *tfm = private;
> +	struct crypto_akcipher *akcipher = tfm->akcipher;
> +	int err;
> +
> +	err = crypto_akcipher_set_priv_key(akcipher, key, keylen);
> +	tfm->has_key = !err;
> +
> +	/* Return the maximum size of the akcipher operation. */
> +	if (!err)
> +		err = crypto_akcipher_maxsize(akcipher);

crypto subsystem returns zero when setkey is successful and introduces
a new function for determining the maxsize. Should we comply with that?

> +
> +	return err;
> +}
> +
> +static int akcipher_setpubkey(void *private, const u8 *key, unsigned int keylen)
> +{
> +	struct akcipher_tfm *tfm = private;
> +	struct crypto_akcipher *akcipher = tfm->akcipher;
> +	int err;
> +
> +	err = crypto_akcipher_set_pub_key(akcipher, key, keylen);
> +	tfm->has_key = !err;
> +
> +	/* Return the maximum size of the akcipher operation. */
> +	if (!err)
> +		err = crypto_akcipher_maxsize(akcipher);
> +
> +	return err;
> +}
> +
> +static void akcipher_sock_destruct(struct sock *sk)
> +{
> +	struct alg_sock *ask = alg_sk(sk);
> +	struct af_alg_ctx *ctx = ask->private;
> +
> +	af_alg_pull_tsgl(sk, ctx->used, NULL, 0);
> +	sock_kfree_s(sk, ctx, ctx->len);
> +	af_alg_release_parent(sk);
> +}
> +
> +static int akcipher_accept_parent_nokey(void *private, struct sock *sk)
> +{
> +	struct af_alg_ctx *ctx;
> +	struct alg_sock *ask = alg_sk(sk);
> +	unsigned int len = sizeof(*ctx);
> +
> +	ctx = sock_kmalloc(sk, len, GFP_KERNEL);
> +	if (!ctx)
> +		return -ENOMEM;
> +	memset(ctx, 0, len);

do you really need the memset? you can initialize the members that you
will use as you did below.

> +
> +	INIT_LIST_HEAD(&ctx->tsgl_list);
> +	ctx->len = len;
> +	ctx->used = 0;
> +	ctx->rcvused = 0;
> +	ctx->more = 0;
> +	ctx->merge = 0;
> +	ctx->op = 0;
> +	af_alg_init_completion(&ctx->completion);
> +
> +	ask->private = ctx;
> +
> +	sk->sk_destruct = akcipher_sock_destruct;
> +
> +	return 0;
> +}
> +
> +static int akcipher_accept_parent(void *private, struct sock *sk)
> +{
> +	struct akcipher_tfm *tfm = private;
> +
> +	if (!tfm->has_key)
> +		return -ENOKEY;
> +
> +	return akcipher_accept_parent_nokey(private, sk);
> +}
> +
> +static const struct af_alg_type algif_type_akcipher = {
> +	.bind		=	akcipher_bind,
> +	.release	=	akcipher_release,
> +	.setkey		=	akcipher_setprivkey,
> +	.setpubkey	=	akcipher_setpubkey,
> +	.setauthsize	=	NULL,
> +	.accept		=	akcipher_accept_parent,
> +	.accept_nokey	=	akcipher_accept_parent_nokey,
> +	.ops		=	&algif_akcipher_ops,
> +	.ops_nokey	=	&algif_akcipher_ops_nokey,
> +	.name		=	"akcipher",
> +	.owner		=	THIS_MODULE
> +};
> +
> +static int __init algif_akcipher_init(void)
> +{
> +	return af_alg_register_type(&algif_type_akcipher);
> +}
> +
> +static void __exit algif_akcipher_exit(void)
> +{
> +	int err = af_alg_unregister_type(&algif_type_akcipher);

checkpatch suggests to insert a blank line after declaration.

Cheers,
ta

> +	BUG_ON(err);
> +}
> +
> +module_init(algif_akcipher_init);
> +module_exit(algif_akcipher_exit);
> +MODULE_LICENSE("GPL");
> +MODULE_AUTHOR("Stephan Mueller <smueller@chronox.de>");
> +MODULE_DESCRIPTION("Asymmetric kernel crypto API user space interface");
> diff --git a/include/crypto/if_alg.h b/include/crypto/if_alg.h
> index d1de8ed3e77b..a2b396756e24 100644
> --- a/include/crypto/if_alg.h
> +++ b/include/crypto/if_alg.h
> @@ -22,6 +22,7 @@
>   
>   #include <crypto/aead.h>
>   #include <crypto/skcipher.h>
> +#include <crypto/akcipher.h>
>   
>   #define ALG_MAX_PAGES			16
>   
> @@ -119,6 +120,7 @@ struct af_alg_async_req {
>   	union {
>   		struct aead_request aead_req;
>   		struct skcipher_request skcipher_req;
> +		struct akcipher_request akcipher_req;
>   	} cra_u;
>   
>   	/* req ctx trails this struct */
> 

Re: [PATCH v8 3/4] crypto: AF_ALG -- add asymmetric cipher

[Stephan Müller]

Am Freitag, 11. August 2017, 14:51:10 CEST schrieb Tudor Ambarus:

Hi Tudor,

I have covered all your requests
> 
> > +	size_t used = 0;
> 
> initialization to zero not needed. You can directly initialize to
> ctx->used or don't initialize at all.

It is not initialized now. We cannot use ctx->used here as the socket (and 
thus the ctx data structure) is not locked yet.

> > +
> > +		/*
> > +		 * This error covers -EIOCBQUEUED which implies that we can
> > +		 * only handle one AIO request. If the caller wants to have
> > +		 * multiple AIO requests in parallel, he must make multiple
> > +		 * separate AIO calls.
> > +		 */
> > +		if (err <= 0) {
> 
> why the equal?

We must get something out of the cipher operation as otherwise something is 
wrong. In this case I would like to error out to prevent an endless loop here.

> > +static int akcipher_setprivkey(void *private, const u8 *key,
> > +			       unsigned int keylen)
> > +{
> > +	struct akcipher_tfm *tfm = private;
> > +	struct crypto_akcipher *akcipher = tfm->akcipher;
> > +	int err;
> > +
> > +	err = crypto_akcipher_set_priv_key(akcipher, key, keylen);
> > +	tfm->has_key = !err;
> > +
> > +	/* Return the maximum size of the akcipher operation. */
> > +	if (!err)
> > +		err = crypto_akcipher_maxsize(akcipher);
> 
> crypto subsystem returns zero when setkey is successful and introduces
> a new function for determining the maxsize. Should we comply with that?

The idea is that only when the the setting of the priv key fails, it returns 
the size of the expected privkey.

Which new function are you referring to?

Ciao
Stephan

Re: [PATCH v8 3/4] crypto: AF_ALG -- add asymmetric cipher

[Tudor Ambarus]

Hi, Stephan,

>>> +static int akcipher_setprivkey(void *private, const u8 *key,
>>> +			       unsigned int keylen)
>>> +{
>>> +	struct akcipher_tfm *tfm = private;
>>> +	struct crypto_akcipher *akcipher = tfm->akcipher;
>>> +	int err;
>>> +
>>> +	err = crypto_akcipher_set_priv_key(akcipher, key, keylen);
>>> +	tfm->has_key = !err;
>>> +
>>> +	/* Return the maximum size of the akcipher operation. */
>>> +	if (!err)
>>> +		err = crypto_akcipher_maxsize(akcipher);
>>
>> crypto subsystem returns zero when setkey is successful and introduces
>> a new function for determining the maxsize. Should we comply with that?
> 
> The idea is that only when the the setting of the priv key fails, it returns
> the size of the expected privkey.
> 
> Which new function are you referring to?

I was referring to crypto_akcipher_maxsize. When
crypto_akcipher_set_priv_key fails, you are overwriting it's return
value with the value of crypto_akcipher_maxsize, hiding the cause of
the error.

crypto akcipher uses a dedicated function for determining the length of
the output buffer, crypto_akcipher_maxsize. Should we add a new function
pointer in struct af_alg_type that returns the maxsize?

Thanks,
ta

Re: [PATCH v8 3/4] crypto: AF_ALG -- add asymmetric cipher

[Tudor Ambarus]

On 08/21/2017 11:55 AM, Tudor Ambarus wrote:
> Hi, Stephan,
> 
>>>> +static int akcipher_setprivkey(void *private, const u8 *key,
>>>> +                   unsigned int keylen)
>>>> +{
>>>> +    struct akcipher_tfm *tfm = private;
>>>> +    struct crypto_akcipher *akcipher = tfm->akcipher;
>>>> +    int err;
>>>> +
>>>> +    err = crypto_akcipher_set_priv_key(akcipher, key, keylen);
>>>> +    tfm->has_key = !err;
>>>> +
>>>> +    /* Return the maximum size of the akcipher operation. */
>>>> +    if (!err)
>>>> +        err = crypto_akcipher_maxsize(akcipher);
>>>
>>> crypto subsystem returns zero when setkey is successful and introduces
>>> a new function for determining the maxsize. Should we comply with that?
>>
>> The idea is that only when the the setting of the priv key fails, it 
>> returns
>> the size of the expected privkey.
>>
>> Which new function are you referring to?
> 
> I was referring to crypto_akcipher_maxsize. When
> crypto_akcipher_set_priv_key fails, you are overwriting it's return
> value with the value of crypto_akcipher_maxsize, hiding the cause of
> the error.

Oops, I missed the negation. When crypto_akcipher_set_priv_key succeeds
you return the akcipher_maxsize. Not a bad idea, you save few cpu
cycles.

> 
> crypto akcipher uses a dedicated function for determining the length of
> the output buffer, crypto_akcipher_maxsize. Should we add a new function
> pointer in struct af_alg_type that returns the maxsize?

Your API is different from crypto's akcipher. Should we make them
identical?

Cheers,
ta

Re: [PATCH v8 3/4] crypto: AF_ALG -- add asymmetric cipher

[Stephan Mueller]

Am Montag, 21. August 2017, 11:23:55 CEST schrieb Tudor Ambarus:

Hi Tudor,

> 
> Oops, I missed the negation. When crypto_akcipher_set_priv_key succeeds
> you return the akcipher_maxsize. Not a bad idea, you save few cpu
> cycles.

I was hoping to save some context switches.
> 
> > crypto akcipher uses a dedicated function for determining the length of
> > the output buffer, crypto_akcipher_maxsize. Should we add a new function
> > pointer in struct af_alg_type that returns the maxsize?
> 
> Your API is different from crypto's akcipher. Should we make them
> identical?

In the early days of the akcipher API it used to be the way algif_akcipher 
implements it today.

Do you see a case where user space wants to deliberately ask for this value? 
As this value never changes after setting a key, I thought we can skip it for 
the user space interface.

Ciao
Stephan

[PATCH v8 4/4] crypto: algif_akcipher - enable compilation

[Stephan Müller]

Add the Makefile and Kconfig updates to allow algif_akcipher to be
compiled.

Signed-off-by: Stephan Mueller <smueller@chronox.de>
---
 crypto/Kconfig  | 9 +++++++++
 crypto/Makefile | 1 +
 2 files changed, 10 insertions(+)

diff --git a/crypto/Kconfig b/crypto/Kconfig
index 0a121f9ddf8e..fdcec68545f3 100644
--- a/crypto/Kconfig
+++ b/crypto/Kconfig
@@ -1760,6 +1760,15 @@ config CRYPTO_USER_API_AEAD
 	  This option enables the user-spaces interface for AEAD
 	  cipher algorithms.
 
+config CRYPTO_USER_API_AKCIPHER
+	tristate "User-space interface for asymmetric key cipher algorithms"
+	depends on NET
+	select CRYPTO_AKCIPHER2
+	select CRYPTO_USER_API
+	help
+	  This option enables the user-spaces interface for asymmetric
+	  key cipher algorithms.
+
 config CRYPTO_HASH_INFO
 	bool
 
diff --git a/crypto/Makefile b/crypto/Makefile
index d41f0331b085..12dbf2c5fe7c 100644
--- a/crypto/Makefile
+++ b/crypto/Makefile
@@ -133,6 +133,7 @@ obj-$(CONFIG_CRYPTO_USER_API_HASH) += algif_hash.o
 obj-$(CONFIG_CRYPTO_USER_API_SKCIPHER) += algif_skcipher.o
 obj-$(CONFIG_CRYPTO_USER_API_RNG) += algif_rng.o
 obj-$(CONFIG_CRYPTO_USER_API_AEAD) += algif_aead.o
+obj-$(CONFIG_CRYPTO_USER_API_AKCIPHER) += algif_akcipher.o
 
 ecdh_generic-y := ecc.o
 ecdh_generic-y += ecdh.o
-- 
2.13.4

Re: [PATCH v8 4/4] crypto: algif_akcipher - enable compilation

[Tudor Ambarus]

Hi, Stephan,

On 08/10/2017 09:40 AM, Stephan Müller wrote:
> Add the Makefile and Kconfig updates to allow algif_akcipher to be
> compiled.
> 
> Signed-off-by: Stephan Mueller<smueller@chronox.de>
> ---
>   crypto/Kconfig  | 9 +++++++++
>   crypto/Makefile | 1 +
>   2 files changed, 10 insertions(+)

Any reason why you keep this patch separately?
I think you can squash this patch with patch 3/4.

Cheers,
ta

Re: [PATCH v8 4/4] crypto: algif_akcipher - enable compilation

[Stephan Mueller]

Am Freitag, 11. August 2017, 14:56:18 CEST schrieb Tudor Ambarus:

Hi Tudor,

> Any reason why you keep this patch separately?
> I think you can squash this patch with patch 3/4.

There is no real reason. If you think it makes sense to add it to patch 3, I 
will do that.

Thanks.


Ciao
Stephan

Re: [PATCH v8 0/4] crypto: add algif_akcipher user space API

[Mat Martineau]

[-- Attachment #1: Type: text/plain, Size: 2099 bytes --]


Hi Stephan,

On Thu, 10 Aug 2017, Stephan Müller wrote:

> Hi,
>
> This patch set adds the AF_ALG user space API to externalize the
> asymmetric cipher API recently added to the kernel crypto API.

...

> Changes v8:
> * port to kernel 4.13
> * port to consolidated AF_ALG code
>
> Stephan Mueller (4):
>  crypto: AF_ALG -- add sign/verify API
>  crypto: AF_ALG -- add setpubkey setsockopt call
>  crypto: AF_ALG -- add asymmetric cipher
>  crypto: algif_akcipher - enable compilation
>
> crypto/Kconfig              |   9 +
> crypto/Makefile             |   1 +
> crypto/af_alg.c             |  28 ++-
> crypto/algif_aead.c         |  36 ++--
> crypto/algif_akcipher.c     | 466 ++++++++++++++++++++++++++++++++++++++++++++
> crypto/algif_skcipher.c     |  26 ++-
> include/crypto/if_alg.h     |   7 +-
> include/uapi/linux/if_alg.h |   3 +
> 8 files changed, 543 insertions(+), 33 deletions(-)
> create mode 100644 crypto/algif_akcipher.c
>
> -- 
> 2.13.4

The last round of reviews for AF_ALG akcipher left off at an impasse 
around a year ago: the consensus was that hardware key support was needed, 
but that requirement was in conflict with the "always have a software 
fallback" rule for the crypto subsystem. For example, a private key 
securely generated by and stored in a TPM could not be copied out for use 
by a software algorithm. Has anything come about to resolve this impasse?

There were some patches around to add keyring support by associating a key 
ID with an akcipher socket, but that approach ran in to a mismatch between 
the proposed keyring API for the verify operation and the semantics of 
AF_ALG verify.

AF_ALG is best suited for crypto use cases where a socket is set up once 
and there are lots of reads and writes to justify the setup cost. With 
asymmetric crypto, the setup cost is high when you might only use the 
socket for a brief time to do one verify or encrypt operation.

Given the efficiency and hardware key issues, AF_ALG seems to be 
mismatched with asymmetric crypto. Have you looked at the proposed 
keyctl() support for crypto operations?

Thanks,

Re: [PATCH v8 0/4] crypto: add algif_akcipher user space API

[Marcel Holtmann]

Hi Mat,

>> This patch set adds the AF_ALG user space API to externalize the
>> asymmetric cipher API recently added to the kernel crypto API.
> 
> ...
> 
>> Changes v8:
>> * port to kernel 4.13
>> * port to consolidated AF_ALG code
>> 
>> Stephan Mueller (4):
>> crypto: AF_ALG -- add sign/verify API
>> crypto: AF_ALG -- add setpubkey setsockopt call
>> crypto: AF_ALG -- add asymmetric cipher
>> crypto: algif_akcipher - enable compilation
>> 
>> crypto/Kconfig              |   9 +
>> crypto/Makefile             |   1 +
>> crypto/af_alg.c             |  28 ++-
>> crypto/algif_aead.c         |  36 ++--
>> crypto/algif_akcipher.c     | 466 ++++++++++++++++++++++++++++++++++++++++++++
>> crypto/algif_skcipher.c     |  26 ++-
>> include/crypto/if_alg.h     |   7 +-
>> include/uapi/linux/if_alg.h |   3 +
>> 8 files changed, 543 insertions(+), 33 deletions(-)
>> create mode 100644 crypto/algif_akcipher.c
>> 
>> -- 
>> 2.13.4
> 
> The last round of reviews for AF_ALG akcipher left off at an impasse around a year ago: the consensus was that hardware key support was needed, but that requirement was in conflict with the "always have a software fallback" rule for the crypto subsystem. For example, a private key securely generated by and stored in a TPM could not be copied out for use by a software algorithm. Has anything come about to resolve this impasse?
> 
> There were some patches around to add keyring support by associating a key ID with an akcipher socket, but that approach ran in to a mismatch between the proposed keyring API for the verify operation and the semantics of AF_ALG verify.
> 
> AF_ALG is best suited for crypto use cases where a socket is set up once and there are lots of reads and writes to justify the setup cost. With asymmetric crypto, the setup cost is high when you might only use the socket for a brief time to do one verify or encrypt operation.
> 
> Given the efficiency and hardware key issues, AF_ALG seems to be mismatched with asymmetric crypto. Have you looked at the proposed keyctl() support for crypto operations?

we have also seen hardware now where the private key will never leave the crypto hardware. They public and private key is only generated for key exchange purposes and later on discarded again. Asymmetric ciphers are really not a good fit for AF_ALG and they should be solely supported via keyctl.

Regards

Marcel

Re: [PATCH v8 0/4] crypto: add algif_akcipher user space API

[Stephan Müller]

Am Freitag, 11. August 2017, 07:13:30 CEST schrieb Marcel Holtmann:

Hi Marcel,

> > 
> > The last round of reviews for AF_ALG akcipher left off at an impasse
> > around a year ago: the consensus was that hardware key support was
> > needed, but that requirement was in conflict with the "always have a
> > software fallback" rule for the crypto subsystem. For example, a private
> > key securely generated by and stored in a TPM could not be copied out for
> > use by a software algorithm. Has anything come about to resolve this
> > impasse?
> > 
> > There were some patches around to add keyring support by associating a key
> > ID with an akcipher socket, but that approach ran in to a mismatch
> > between the proposed keyring API for the verify operation and the
> > semantics of AF_ALG verify.
> > 
> > AF_ALG is best suited for crypto use cases where a socket is set up once
> > and there are lots of reads and writes to justify the setup cost. With
> > asymmetric crypto, the setup cost is high when you might only use the
> > socket for a brief time to do one verify or encrypt operation.
> > 
> > Given the efficiency and hardware key issues, AF_ALG seems to be
> > mismatched with asymmetric crypto. Have you looked at the proposed
> > keyctl() support for crypto operations?
> we have also seen hardware now where the private key will never leave the
> crypto hardware. They public and private key is only generated for key
> exchange purposes and later on discarded again. Asymmetric ciphers are
> really not a good fit for AF_ALG and they should be solely supported via
> keyctl.

Thanks for the reminder. I have looked at that but I am unsure about whether 
this one covers asym crypto appropriately, too.

The issue is that some hardware may only offer accelerators without a full 
blown RSA siggen/ver logic (that pulls in PKCS or OAEP or others). How do you 
propose to cover raw primitives with keyctl?

Ciao
Stephan

Re: [PATCH v8 0/4] crypto: add algif_akcipher user space API

[Marcel Holtmann]

Hi Stephan,

>>> The last round of reviews for AF_ALG akcipher left off at an impasse
>>> around a year ago: the consensus was that hardware key support was
>>> needed, but that requirement was in conflict with the "always have a
>>> software fallback" rule for the crypto subsystem. For example, a private
>>> key securely generated by and stored in a TPM could not be copied out for
>>> use by a software algorithm. Has anything come about to resolve this
>>> impasse?
>>> 
>>> There were some patches around to add keyring support by associating a key
>>> ID with an akcipher socket, but that approach ran in to a mismatch
>>> between the proposed keyring API for the verify operation and the
>>> semantics of AF_ALG verify.
>>> 
>>> AF_ALG is best suited for crypto use cases where a socket is set up once
>>> and there are lots of reads and writes to justify the setup cost. With
>>> asymmetric crypto, the setup cost is high when you might only use the
>>> socket for a brief time to do one verify or encrypt operation.
>>> 
>>> Given the efficiency and hardware key issues, AF_ALG seems to be
>>> mismatched with asymmetric crypto. Have you looked at the proposed
>>> keyctl() support for crypto operations?
>> we have also seen hardware now where the private key will never leave the
>> crypto hardware. They public and private key is only generated for key
>> exchange purposes and later on discarded again. Asymmetric ciphers are
>> really not a good fit for AF_ALG and they should be solely supported via
>> keyctl.
> 
> Thanks for the reminder. I have looked at that but I am unsure about whether 
> this one covers asym crypto appropriately, too.
> 
> The issue is that some hardware may only offer accelerators without a full 
> blown RSA siggen/ver logic (that pulls in PKCS or OAEP or others). How do you 
> propose to cover raw primitives with keyctl?

where is such a hardware? And what is the usage of it? Look at what we are using asymmetric crypto for at the moment. It is either for sign and verify with secure boot etc. Or it is for key exchange purposes.

The asymmetric crypto is a means to an end. If it is not for certification verification, then it for is creating a symmetric key to be used with a symmetric cipher. We have the the asymmetric_keys subsystem for representing the nature of this crypto. Also the list of asymmetric ciphers is a lot smaller than the symmetric ones.

What raw primitives? When we are using for example ECDH for Bluetooth where you need to create a pairwise symmetric key, then what you really want from the cryptographic primitives is this:

1) Create public/private key pair
2) Give public key to applications and store the private key safely
3) Retrieve public key from remote side and challenge
4) Compute key exchange magic (like DH) from remote public key
5) Tell the key generator to throw away the private key

So I do not want to load any private key into the kernel. I want the kernel to give me a public key and allow me to feed the key exchange primitive with the remote public key. This is clearly not AF_ALG. We had this discussion during the KPP design and I made it clear multiple times that this is totally different.

This is clearly keyctl area of interfaces since the main operation is key generation. I am not saying that keyctl is perfect just yet, but we are working on it. We however have to accept that it is more suitable than AF_ALG. You will never have stream based data feed into an asymmetric cipher. That is what stream ciphers are for.

Regards

Marcel

Re: [PATCH v8 0/4] crypto: add algif_akcipher user space API

[Stephan Mueller]

Am Freitag, 11. August 2017, 18:02:55 CEST schrieb Marcel Holtmann:

Hi Marcel,

> > Thanks for the reminder. I have looked at that but I am unsure about
> > whether this one covers asym crypto appropriately, too.
> > 
> > The issue is that some hardware may only offer accelerators without a full
> > blown RSA siggen/ver logic (that pulls in PKCS or OAEP or others). How do
> > you propose to cover raw primitives with keyctl?
> 
> where is such a hardware?

Strangely, I see such support all the time in embedded devices where 
asymmetric acceleration is really necessary.

> And what is the usage of it? Look at what we are
> using asymmetric crypto for at the moment. It is either for sign and verify
> with secure boot etc. Or it is for key exchange purposes.

I understand that this is the core purpose of asymmetric ciphers. Yet, 
asymmetric ciphers are complex and we as kernel developers do not know (or 
shall not mandate?) where the complexity shall be implemented. By forcing all 
into the keyctl, we would insist that the entire complexity of the full-blown 
asym cipher is in the kernel without an option that user space may implement 
it.

What we are currently seemingly discuss is the choice of

- keyctl: having all complexity of the entire asym logic including its key 
handling in the kernel with the benefit of the kernel protection of the 
private key

- algif_akcipher (maybe with a link to keyctl): only exporting the cipher 
support and allow user space to decide where the complexity lies

Just to give you an example: A full blown RSA operation (excluding the hashing 
part for signatures) consists of padding and the asymmetric operation. For the 
asymmetric operation, we have sign/verify and encrypt/decrypt (keywrap). There 
are a gazillion padding types out there:

- PKCS1

- OAEP

- SP800-56B: RSAEP, RSADP, RSASVE, RSA-OAEP, RSA-KEM-KWS

And there may be others.

When we talk about encryption/decryption we have to consider the KDFs 
(SP800-108, RFC5689, SP800-56A).

When we consider the KDFs, we have to think of the KDF data styles (ASN.1, 
concatenation)

With keyctl to me it seems that we need to integrate all that logic into the 
kernel. As all of that is just processing logic, securing it in the kernel may 
not be the right way as this code does not need the elevated privileges in the 
kernel for that.

Yet, some hardware may provide some/all of this logic. And we want to make 
that available to user space.
> 
> The asymmetric crypto is a means to an end. If it is not for certification
> verification, then it for is creating a symmetric key to be used with a
> symmetric cipher. We have the the asymmetric_keys subsystem for
> representing the nature of this crypto. Also the list of asymmetric ciphers
> is a lot smaller than the symmetric ones.
> 
> What raw primitives? When we are using for example ECDH for Bluetooth where
> you need to create a pairwise symmetric key, then what you really want from
> the cryptographic primitives is this:
> 
> 1) Create public/private key pair

See above, it is my opinion that with asym ciphers, there is a lot of 
complexity and lots of options. I do not think that the kernel API should be a 
limiting factor here, because the kernel simply does not implement a specific 
cipher type.

> 2) Give public key to applications and store the private key safely
> 3) Retrieve public key from remote side and challenge

This assumes that always the Linux kernel is the manager of keys (or the 
gatekeeper to the key store). Are we sure that this is always the case?

Ciao
Stephan

Re: [PATCH v8 0/4] crypto: add algif_akcipher user space API

[Marcel Holtmann]

Hi Stephan,

>>> Thanks for the reminder. I have looked at that but I am unsure about
>>> whether this one covers asym crypto appropriately, too.
>>> 
>>> The issue is that some hardware may only offer accelerators without a full
>>> blown RSA siggen/ver logic (that pulls in PKCS or OAEP or others). How do
>>> you propose to cover raw primitives with keyctl?
>> 
>> where is such a hardware?
> 
> Strangely, I see such support all the time in embedded devices where 
> asymmetric acceleration is really necessary.

and where are the support patches to integrate them with KPP in the kernel. Maybe we should first have these before exposing anything to userspace.

I said this in another thread before, we have Bluetooth Security Manager which uses ECDH. It has been re-written to use KPP and once we have that fully supporting hardware based ECC key generation and ECDH operation, I would be a lot more confident that we understand how asymmetric crypto accelerators actually do their job. Especially in the embedded world.

>> And what is the usage of it? Look at what we are
>> using asymmetric crypto for at the moment. It is either for sign and verify
>> with secure boot etc. Or it is for key exchange purposes.
> 
> I understand that this is the core purpose of asymmetric ciphers. Yet, 
> asymmetric ciphers are complex and we as kernel developers do not know (or 
> shall not mandate?) where the complexity shall be implemented. By forcing all 
> into the keyctl, we would insist that the entire complexity of the full-blown 
> asym cipher is in the kernel without an option that user space may implement 
> it.
> 
> What we are currently seemingly discuss is the choice of
> 
> - keyctl: having all complexity of the entire asym logic including its key 
> handling in the kernel with the benefit of the kernel protection of the 
> private key

Which we already have anyway since the kernel supports Secure Boot.

> - algif_akcipher (maybe with a link to keyctl): only exporting the cipher 
> support and allow user space to decide where the complexity lies
> 
> Just to give you an example: A full blown RSA operation (excluding the hashing 
> part for signatures) consists of padding and the asymmetric operation. For the 
> asymmetric operation, we have sign/verify and encrypt/decrypt (keywrap). There 
> are a gazillion padding types out there:
> 
> - PKCS1
> 
> - OAEP
> 
> - SP800-56B: RSAEP, RSADP, RSASVE, RSA-OAEP, RSA-KEM-KWS
> 
> And there may be others.
> 
> When we talk about encryption/decryption we have to consider the KDFs 
> (SP800-108, RFC5689, SP800-56A).
> 
> When we consider the KDFs, we have to think of the KDF data styles (ASN.1, 
> concatenation)
> 
> With keyctl to me it seems that we need to integrate all that logic into the 
> kernel. As all of that is just processing logic, securing it in the kernel may 
> not be the right way as this code does not need the elevated privileges in the 
> kernel for that.

The keyring subsystem has extensive user permissions available. Actually that is even more of a reason to not expose anything with private keys via AF_ALG. If I can use AF_ALG to access (meaning use a private key) that I am not suppose to use, then we have a real security nightmare on our hands. The keyring subsystem has permission that can be restricted on a per-thread level.

On a side note, have a look at ELL and how it uses Mat’s crypto tree. For fun look at iwd and how it does WPA-Enterprise with mostly only kernel primitives.

> Yet, some hardware may provide some/all of this logic. And we want to make 
> that available to user space.

I don’t get the raw access to RSA for example. Nobody uses it in raw mode. There is always some sort of padding involved and needed. Otherwise RSA would be pretty insecure.

It is also funny how RSA centric this is. There is more than RSA since especially ECDH and its friends are getting more and more traction in the IoT spaces. Secure key exchange without the usage of certificates is actually a valid use case as well.

>> The asymmetric crypto is a means to an end. If it is not for certification
>> verification, then it for is creating a symmetric key to be used with a
>> symmetric cipher. We have the the asymmetric_keys subsystem for
>> representing the nature of this crypto. Also the list of asymmetric ciphers
>> is a lot smaller than the symmetric ones.
>> 
>> What raw primitives? When we are using for example ECDH for Bluetooth where
>> you need to create a pairwise symmetric key, then what you really want from
>> the cryptographic primitives is this:
>> 
>> 1) Create public/private key pair
> 
> See above, it is my opinion that with asym ciphers, there is a lot of 
> complexity and lots of options. I do not think that the kernel API should be a 
> limiting factor here, because the kernel simply does not implement a specific 
> cipher type.

What has cipher type to do with this? This confuses me. If the kernel supports any KPP backed algorithm, it should be able to generate key pairs for it. The complexity if the private key is in hardware is not what I care about. Frankly if the public is not leaving the kernel space, that is a huge bonus in my mind. The constant copying and with that securing of keys is always an attack vector.

>> 2) Give public key to applications and store the private key safely
>> 3) Retrieve public key from remote side and challenge
> 
> This assumes that always the Linux kernel is the manager of keys (or the 
> gatekeeper to the key store). Are we sure that this is always the case?

Why would it not be? How can you do a better job in userspace? And if you have hardware backend for storage, then it can be easily abstracted with the keyring subsystem. See TPM for example.

Regards

Marcel

Re: [PATCH v8 0/4] crypto: add algif_akcipher user space API

[Stephan Mueller]

Am Freitag, 11. August 2017, 02:48:37 CEST schrieb Mat Martineau:

Hi Mat,

> 
> AF_ALG is best suited for crypto use cases where a socket is set up once
> and there are lots of reads and writes to justify the setup cost. With
> asymmetric crypto, the setup cost is high when you might only use the
> socket for a brief time to do one verify or encrypt operation.

To me, the entire AF_ALG purpose is solely to export hardware support to user 
space. That said, if user space wants an accelerator, a socket would be opened 
once followed by numerous read/write requests.

Besides, I am aware of Tadeusz' patch to link algif_akcipher to the keyring 
and I planned to port it to the current implementation. But I thought I offer 
a small patch focusing on the externalization of the akcipher API first.

I think the keyctl and AF_ALG are no opponents, but rather are orthogonal to 
each other. The statement I made for the KPP AF_ALG RFC applies here too:

"""
I am aware and in fact supported development of the DH support in the keys
subsystem. The question will be raised whether the AF_ALG KPP interface is
superfluous in light of the keys DH support. My answer is that AF_ALG KPP is
orthogonal to the keys DH support. The keys DH support use case is that
the keys are managed inside the kernel and thus has the focus on the
key management. Contrary, AF_ALG KPP does not really focus on key management
but simply externalizes the DH/ECDH support found in the kernel including
hardware acceleration. User space is in full control of the key life cycle.
This way, AF_ALG could be used to complement user-space network protocol
implementations like TLS or IKE to offload the resource intense DH
calculations to accelerators.
"""

Ciao
Stephan

Re: [PATCH v8 0/4] crypto: add algif_akcipher user space API

[Marcel Holtmann]

Hi Stephan,

>> AF_ALG is best suited for crypto use cases where a socket is set up once
>> and there are lots of reads and writes to justify the setup cost. With
>> asymmetric crypto, the setup cost is high when you might only use the
>> socket for a brief time to do one verify or encrypt operation.
> 
> To me, the entire AF_ALG purpose is solely to export hardware support to user 
> space. That said, if user space wants an accelerator, a socket would be opened 
> once followed by numerous read/write requests.
> 
> Besides, I am aware of Tadeusz' patch to link algif_akcipher to the keyring 
> and I planned to port it to the current implementation. But I thought I offer 
> a small patch focusing on the externalization of the akcipher API first.
> 
> I think the keyctl and AF_ALG are no opponents, but rather are orthogonal to 
> each other. The statement I made for the KPP AF_ALG RFC applies here too:
> 
> """
> I am aware and in fact supported development of the DH support in the keys
> subsystem. The question will be raised whether the AF_ALG KPP interface is
> superfluous in light of the keys DH support. My answer is that AF_ALG KPP is
> orthogonal to the keys DH support. The keys DH support use case is that
> the keys are managed inside the kernel and thus has the focus on the
> key management. Contrary, AF_ALG KPP does not really focus on key management
> but simply externalizes the DH/ECDH support found in the kernel including
> hardware acceleration. User space is in full control of the key life cycle.
> This way, AF_ALG could be used to complement user-space network protocol
> implementations like TLS or IKE to offload the resource intense DH
> calculations to accelerators.
> “""

we do not need two interfaces for doing the same thing. Especially not one that can not handle hardware backed keys. And more important if you can not abstract an accelerator that doesn’t expose the private key at all.

If you only have a hammer, everything looks like a nail. Luckily we have more tools than just a hammer :)

Regards

Marcel

Re: [PATCH v8 0/4] crypto: add algif_akcipher user space API

[Gilad Ben-Yossef]

On Fri, Aug 11, 2017 at 7:05 PM, Marcel Holtmann <marcel@holtmann.org> wrote:
> Hi Stephan,
>
>>> AF_ALG is best suited for crypto use cases where a socket is set up once
>>> and there are lots of reads and writes to justify the setup cost. With
>>> asymmetric crypto, the setup cost is high when you might only use the
>>> socket for a brief time to do one verify or encrypt operation.
>>
>> To me, the entire AF_ALG purpose is solely to export hardware support to user
>> space. That said, if user space wants an accelerator, a socket would be opened
>> once followed by numerous read/write requests.
>>
>> Besides, I am aware of Tadeusz' patch to link algif_akcipher to the keyring
>> and I planned to port it to the current implementation. But I thought I offer
>> a small patch focusing on the externalization of the akcipher API first.
>>
>> I think the keyctl and AF_ALG are no opponents, but rather are orthogonal to
>> each other. The statement I made for the KPP AF_ALG RFC applies here too:
>>
>> """
>> I am aware and in fact supported development of the DH support in the keys
>> subsystem. The question will be raised whether the AF_ALG KPP interface is
>> superfluous in light of the keys DH support. My answer is that AF_ALG KPP is
>> orthogonal to the keys DH support. The keys DH support use case is that
>> the keys are managed inside the kernel and thus has the focus on the
>> key management. Contrary, AF_ALG KPP does not really focus on key management
>> but simply externalizes the DH/ECDH support found in the kernel including
>> hardware acceleration. User space is in full control of the key life cycle.
>> This way, AF_ALG could be used to complement user-space network protocol
>> implementations like TLS or IKE to offload the resource intense DH
>> calculations to accelerators.
>> “""
>
> we do not need two interfaces for doing the same thing. Especially not one that can not handle hardware backed keys. And more important if you can not abstract an accelerator that doesn’t expose the private key at all.

While I don't have anything to contribute to the choice between
keyctl() vs ALG_IF as interfaces for asymmetric  cryptography, I would
like to point out that there is both interest and HW support for
private symmetric key operations as well, for example for storage
encryption via DM-Crypt and fscrypt, so I do hope (and will work on)
adding some sort of HW key support the crypto API, community
acceptance withstanding of course.

So I hope we won't treat the idea of crypto API lack of support for HW
keys as a long standing immutable argument.

Thanks,
Gilad


-- 
Gilad Ben-Yossef
Chief Coffee Drinker

"If you take a class in large-scale robotics, can you end up in a
situation where the homework eats your dog?"
 -- Jean-Baptiste Queru

Re: [PATCH v8 0/4] crypto: add algif_akcipher user space API

[Stephan Mueller]

Am Sonntag, 13. August 2017, 10:52:00 CEST schrieb Gilad Ben-Yossef:

Hi Gilad,

> While I don't have anything to contribute to the choice between
> keyctl() vs ALG_IF as interfaces for asymmetric  cryptography, I would
> like to point out that there is both interest and HW support for
> private symmetric key operations as well, for example for storage
> encryption via DM-Crypt and fscrypt, so I do hope (and will work on)
> adding some sort of HW key support the crypto API, community
> acceptance withstanding of course.
> 
> So I hope we won't treat the idea of crypto API lack of support for HW
> keys as a long standing immutable argument.

See the patch set that was offered by Tudor regarding the in-kernel or in-
hardware generation of the ECDH private keys. There is nothing that prevents 
us having such API for akcipher. In fact, it would even be more or less a 
copy-n-paste job.

Exporting that logic to user space could be done as follows:

- keyctl API is used to trigger the key generation process and to obtain a 
handle

- AF_ALG to perform the asym operation where the key handle from keyctl is 
handed into the kernel. I am aware that this link between AF_ALG and keyctl is 
yet missing. But it on my desk and I am willing to integrate it. The 
integration should even not be specific to algif_akcipher, but to all cipher 
types.

Ciao
Stephan

Re: [PATCH v8 0/4] crypto: add algif_akcipher user space API

[Tudor Ambarus]

Hi, all,

On 08/11/2017 07:05 PM, Marcel Holtmann wrote:
> Hi Stephan,
> 
>>> AF_ALG is best suited for crypto use cases where a socket is set up once
>>> and there are lots of reads and writes to justify the setup cost. With
>>> asymmetric crypto, the setup cost is high when you might only use the
>>> socket for a brief time to do one verify or encrypt operation.
>>
>> To me, the entire AF_ALG purpose is solely to export hardware support to user
>> space. That said, if user space wants an accelerator, a socket would be opened
>> once followed by numerous read/write requests.
>>
>> Besides, I am aware of Tadeusz' patch to link algif_akcipher to the keyring
>> and I planned to port it to the current implementation. But I thought I offer
>> a small patch focusing on the externalization of the akcipher API first.
>>
>> I think the keyctl and AF_ALG are no opponents, but rather are orthogonal to
>> each other. The statement I made for the KPP AF_ALG RFC applies here too:
>>
>> """
>> I am aware and in fact supported development of the DH support in the keys
>> subsystem. The question will be raised whether the AF_ALG KPP interface is
>> superfluous in light of the keys DH support. My answer is that AF_ALG KPP is
>> orthogonal to the keys DH support. The keys DH support use case is that
>> the keys are managed inside the kernel and thus has the focus on the
>> key management. Contrary, AF_ALG KPP does not really focus on key management
>> but simply externalizes the DH/ECDH support found in the kernel including
>> hardware acceleration. User space is in full control of the key life cycle.
>> This way, AF_ALG could be used to complement user-space network protocol
>> implementations like TLS or IKE to offload the resource intense DH
>> calculations to accelerators.
>> “""
> 
> we do not need two interfaces for doing the same thing. Especially not one that can not handle hardware backed keys. And more important if you can not abstract an accelerator that doesn’t expose the private key at all.

I'm working with a crypto accelerator (find it at [1]) that is capable
of generating random ecc private keys internally within the device that
are never revealed outside of it. The keys can be further used for ECDH
and ECDSA.

The simplest way to access my device from user-space is to go through
af_alg. We can permit the users to provide NULL keys, and if so, we can
generate the keys inside the kernel/hardware. If hardware supports
key generation and retention, it will use it, else the keys
will be generated inside the kernel. Either way it's a win, we don't
reveal the private keys to user-space. Going through the keyring
subsystem seems superfluous in this case.

My use case compared with the one of using keyring subsystem to access
keys from TPMs or Intel's sgx enclave seem orthogonal. What do you
think?

Cheers,
ta

[1] http://www.microchip.com/wwwproducts/en/ATECC508A
The driver can be found in drivers/crypto/atmel-ecc* in Herbert's tree.

Re: [PATCH v8 0/4] crypto: add algif_akcipher user space API

[Tudor Ambarus]

Hi, Herbert, all,

akcipher can work with its own internal keys, now that we have crypto
accelerators that can generate keys that never leave the hardware. Going
through the kernel's key subsystem seems superfluous in this case.

I also understand the need of going through the kernel's key subsystem
when the user wants to refer to a key which exists elsewhere, such as in
TPM or within an SGX software enclave, but this seems orthogonal with
crypto accelerators with key generation and retention support.

How should we interface akcipher/kpp with user-space?

Thanks,
ta

On 08/17/2017 04:17 PM, Tudor Ambarus wrote:
> Hi, all,
> 
> On 08/11/2017 07:05 PM, Marcel Holtmann wrote:
>> Hi Stephan,
>>
>>>> AF_ALG is best suited for crypto use cases where a socket is set up 
>>>> once
>>>> and there are lots of reads and writes to justify the setup cost. With
>>>> asymmetric crypto, the setup cost is high when you might only use the
>>>> socket for a brief time to do one verify or encrypt operation.
>>>
>>> To me, the entire AF_ALG purpose is solely to export hardware support 
>>> to user
>>> space. That said, if user space wants an accelerator, a socket would 
>>> be opened
>>> once followed by numerous read/write requests.
>>>
>>> Besides, I am aware of Tadeusz' patch to link algif_akcipher to the 
>>> keyring
>>> and I planned to port it to the current implementation. But I thought 
>>> I offer
>>> a small patch focusing on the externalization of the akcipher API first.
>>>
>>> I think the keyctl and AF_ALG are no opponents, but rather are 
>>> orthogonal to
>>> each other. The statement I made for the KPP AF_ALG RFC applies here 
>>> too:
>>>
>>> """
>>> I am aware and in fact supported development of the DH support in the 
>>> keys
>>> subsystem. The question will be raised whether the AF_ALG KPP 
>>> interface is
>>> superfluous in light of the keys DH support. My answer is that AF_ALG 
>>> KPP is
>>> orthogonal to the keys DH support. The keys DH support use case is that
>>> the keys are managed inside the kernel and thus has the focus on the
>>> key management. Contrary, AF_ALG KPP does not really focus on key 
>>> management
>>> but simply externalizes the DH/ECDH support found in the kernel 
>>> including
>>> hardware acceleration. User space is in full control of the key life 
>>> cycle.
>>> This way, AF_ALG could be used to complement user-space network protocol
>>> implementations like TLS or IKE to offload the resource intense DH
>>> calculations to accelerators.
>>> “""
>>
>> we do not need two interfaces for doing the same thing. Especially not 
>> one that can not handle hardware backed keys. And more important if 
>> you can not abstract an accelerator that doesn’t expose the private 
>> key at all.
> 
> I'm working with a crypto accelerator (find it at [1]) that is capable
> of generating random ecc private keys internally within the device that
> are never revealed outside of it. The keys can be further used for ECDH
> and ECDSA.
> 
> The simplest way to access my device from user-space is to go through
> af_alg. We can permit the users to provide NULL keys, and if so, we can
> generate the keys inside the kernel/hardware. If hardware supports
> key generation and retention, it will use it, else the keys
> will be generated inside the kernel. Either way it's a win, we don't
> reveal the private keys to user-space. Going through the keyring
> subsystem seems superfluous in this case.
> 
> My use case compared with the one of using keyring subsystem to access
> keys from TPMs or Intel's sgx enclave seem orthogonal. What do you
> think?
> 
> Cheers,
> ta
> 
> [1] http://www.microchip.com/wwwproducts/en/ATECC508A
> The driver can be found in drivers/crypto/atmel-ecc* in Herbert's tree.

Re: [PATCH v8 0/4] crypto: add algif_akcipher user space API

[Marcel Holtmann]

Hi Tudor,

> akcipher can work with its own internal keys, now that we have crypto
> accelerators that can generate keys that never leave the hardware. Going
> through the kernel's key subsystem seems superfluous in this case.
> 
> I also understand the need of going through the kernel's key subsystem
> when the user wants to refer to a key which exists elsewhere, such as in
> TPM or within an SGX software enclave, but this seems orthogonal with
> crypto accelerators with key generation and retention support.
> 
> How should we interface akcipher/kpp with user-space?

you still need to get the public key out of the kernel if you want to use it from user space. Or feed the remote public key if you plan to use some sort of key derivation function.

I am saying this again, if you only have a hammer, everything looks like a nail. What about actually looking at how this would be used from user space in real crypto cases.

My point is that the usages here are key generation, some sort of key-exchange-agreement (aka DH) and key derivation into a symmetric key. Frankly the focus with asymmetric ciphers are the keys and the key derivation. They are not encryption and decryption of massive amounts of data.

Regards

Marcel

Re: [PATCH v8 0/4] crypto: add algif_akcipher user space API

[Tudor Ambarus]

Hi, Marcel,

On 08/30/2017 10:21 AM, Marcel Holtmann wrote:
> you still need to get the public key out of the kernel if you want to use it from user space. Or feed the remote public key if you plan to use some sort of key derivation function.
> 

The crypto hardware that I'm working on, generates the private key
internally within the device and never reveals it to software and
immediately returns the public key pair. The user can retrieve the
public key from hardware.

> I am saying this again, if you only have a hammer, everything looks like a nail. What about actually looking at how this would be used from user space in real crypto cases.
> 
> My point is that the usages here are key generation, some sort of key-exchange-agreement (aka DH) and key derivation into a symmetric key. Frankly the focus with asymmetric ciphers are the keys and the key derivation. They are not encryption and decryption of massive amounts of data.

The hardware uses it's own private key and the public key received from
the other end and computes the ecdh shared secret. The hardware computed
shared secret can then be used for key derivation.

Cheers,
ta

Re: [PATCH v8 0/4] crypto: add algif_akcipher user space API

[Marcel Holtmann]

Hi Tudor,

>> you still need to get the public key out of the kernel if you want to use it from user space. Or feed the remote public key if you plan to use some sort of key derivation function.
> 
> The crypto hardware that I'm working on, generates the private key
> internally within the device and never reveals it to software and
> immediately returns the public key pair. The user can retrieve the
> public key from hardware.

and don’t we want some sort of access control here. Only the user / process that requested the private key and has access to the public key is allowed to keep using the private key?

>> I am saying this again, if you only have a hammer, everything looks like a nail. What about actually looking at how this would be used from user space in real crypto cases.
>> My point is that the usages here are key generation, some sort of key-exchange-agreement (aka DH) and key derivation into a symmetric key. Frankly the focus with asymmetric ciphers are the keys and the key derivation. They are not encryption and decryption of massive amounts of data.
> 
> The hardware uses it's own private key and the public key received from
> the other end and computes the ecdh shared secret. The hardware computed
> shared secret can then be used for key derivation.

And that is normally the case. Get your local public key, send it to the other side, get the other sides public key, give it to the hardware and get shared secret.

So how is AF_ALG a good fit here?

Regards

Marcel

Re: [PATCH v8 0/4] crypto: add algif_akcipher user space API

[Andrew Zaborowski]

HI,

On 11 August 2017 at 02:48, Mat Martineau
<mathew.j.martineau@linux.intel.com> wrote:
> The last round of reviews for AF_ALG akcipher left off at an impasse around
> a year ago: the consensus was that hardware key support was needed, but that
> requirement was in conflict with the "always have a software fallback" rule
> for the crypto subsystem. For example, a private key securely generated by
> and stored in a TPM could not be copied out for use by a software algorithm.
> Has anything come about to resolve this impasse?
>
> There were some patches around to add keyring support by associating a key
> ID with an akcipher socket, but that approach ran in to a mismatch between
> the proposed keyring API for the verify operation and the semantics of
> AF_ALG verify.
>
> AF_ALG is best suited for crypto use cases where a socket is set up once and
> there are lots of reads and writes to justify the setup cost. With
> asymmetric crypto, the setup cost is high when you might only use the socket
> for a brief time to do one verify or encrypt operation.

Would that time be shorter when going through the keyctl API?

In any case there will be situations, similar to the lightweight TLS
implementation use case, where this isn't a factor.

>
> Given the efficiency and hardware key issues, AF_ALG seems to be mismatched
> with asymmetric crypto.

The hardware key support would obviously be a benefit but it's
orthogonal to this I believe.  That issue is not specific to akcipher
either, there will be hardware-only symmetric keys that can't be used
with current ALG_IF.

The ALG_IF API provides a slightly lower level access to the
algorithms listed in /proc/crypto than the keyctl API and I don't see
the reason that some of those algorithms should not be available.

Best regards

Re: [PATCH v8 0/4] crypto: add algif_akcipher user space API

[Mat Martineau]

On Fri, 11 Aug 2017, Andrew Zaborowski wrote:

> HI,
>
> On 11 August 2017 at 02:48, Mat Martineau
> <mathew.j.martineau@linux.intel.com> wrote:
>> The last round of reviews for AF_ALG akcipher left off at an impasse around
>> a year ago: the consensus was that hardware key support was needed, but that
>> requirement was in conflict with the "always have a software fallback" rule
>> for the crypto subsystem. For example, a private key securely generated by
>> and stored in a TPM could not be copied out for use by a software algorithm.
>> Has anything come about to resolve this impasse?
>>
>> There were some patches around to add keyring support by associating a key
>> ID with an akcipher socket, but that approach ran in to a mismatch between
>> the proposed keyring API for the verify operation and the semantics of
>> AF_ALG verify.
>>
>> AF_ALG is best suited for crypto use cases where a socket is set up once and
>> there are lots of reads and writes to justify the setup cost. With
>> asymmetric crypto, the setup cost is high when you might only use the socket
>> for a brief time to do one verify or encrypt operation.
>
> Would that time be shorter when going through the keyctl API?
>

There's a certain amount of work to be done for a one-off crypto operation 
no matter what the API is: load a key, set up the operation, provide the 
input, read the output, and clean up. keyctl() handles that "one-off" case 
with a smaller sequence of system calls, but is not as good for streaming.

> In any case there will be situations, similar to the lightweight TLS
> implementation use case, where this isn't a factor.
>
>>
>> Given the efficiency and hardware key issues, AF_ALG seems to be mismatched
>> with asymmetric crypto.
>
> The hardware key support would obviously be a benefit but it's
> orthogonal to this I believe.  That issue is not specific to akcipher
> either, there will be hardware-only symmetric keys that can't be used
> with current ALG_IF.
>
> The ALG_IF API provides a slightly lower level access to the
> algorithms listed in /proc/crypto than the keyctl API and I don't see
> the reason that some of those algorithms should not be available.

I also would like to have more of those algorithms exposed to userspace, 
and I'd like to make sure the API is a good fit. There was extensive 
discussion of AF_ALG akcipher last year. v8 of the patch set updates the 
implementation but doesn't address the API concerns that kept the previous 
versions from being merged, so we seem to be at just as much of an impasse 
as before.

Re: [PATCH v8 0/4] crypto: add algif_akcipher user space API

[Stephan Mueller]

Am Freitag, 11. August 2017, 21:43:15 CEST schrieb Mat Martineau:

Hi Mat,

> 
> I also would like to have more of those algorithms exposed to userspace,
> and I'd like to make sure the API is a good fit. There was extensive
> discussion of AF_ALG akcipher last year. v8 of the patch set updates the
> implementation but doesn't address the API concerns that kept the previous
> versions from being merged, so we seem to be at just as much of an impasse
> as before.

During last year's discussion, I think we have concluded (and please correct 
me if I miss something), that the export of the asymmetric HW implementations 
to user space should

- allow a streaming mode where the user space uses the kernel as an 
accelerator (maybe user space has another way of storing keys)

- allow the private key to be retained in kernel space (or even in hardware 
only) -- i.e. user space only has a handle to the key for a full asym 
operation

The first part is clearly where AF_ALG fits and keyctl does not. This is 
provided with the current patch set. As the keyctl API only handles, well, 
keys, access to the raw ciphers may not be possible through this API. And let 
us face it, a lot of user space code shall support many different OSes. Thus, 
if you have a crypto lib in user space who has its own key management (which 
is a core element of such libraries and thus cannot be put into an 
architecture-dependent code part), having only the keyctl API on Linux for 
accelerated asym support may not be helpful.

The second part is a keyctl domain. I see in-kernel support for this scenario, 
but have not yet seen the kernel/user interface nor the user space support.

Both options are orthogonal, IMHO.

Tadeusz Struck provided a patch to link the kernel crypto API / algif_akcipher 
with the keys subsystem to allow the second requirement to be implemented in 
algif_akcipher. That patch is on my desk and I plan to integrate it and even 
make it generic to allow its use for all different cipher types. I have not 
yet integrated it to allow a small patch set to be reviewed. If it is the will 
of the council, I can surely add that code to the initial patch set and 
resubmit.


Ciao
Stephan

Re: [PATCH v8 0/4] crypto: add algif_akcipher user space API

[Marcel Holtmann]

Hi Stephan,

>> I also would like to have more of those algorithms exposed to userspace,
>> and I'd like to make sure the API is a good fit. There was extensive
>> discussion of AF_ALG akcipher last year. v8 of the patch set updates the
>> implementation but doesn't address the API concerns that kept the previous
>> versions from being merged, so we seem to be at just as much of an impasse
>> as before.
> 
> During last year's discussion, I think we have concluded (and please correct 
> me if I miss something), that the export of the asymmetric HW implementations 
> to user space should
> 
> - allow a streaming mode where the user space uses the kernel as an 
> accelerator (maybe user space has another way of storing keys)

explain to me what a streaming mode with an asymmetric cipher is? They are no good for these kind of operations. If you want streaming mode, then you want a symmetric cipher.

The keyring subsystem is actually a good storage for keys. If the keyring subsystem can use hardware storage for the keys, even better. However right now all the certificates and its associated keys are stored perfectly fine in a keyring.

> - allow the private key to be retained in kernel space (or even in hardware 
> only) -- i.e. user space only has a handle to the key for a full asym 
> operation

And that is exactly my point. Even if userspace has the key, let it load into the kernel as a key inside a keyring. We do not need two ways for loading asymmetric keys. They are by nature more complex then any symmetric key.

> The first part is clearly where AF_ALG fits and keyctl does not. This is 
> provided with the current patch set. As the keyctl API only handles, well, 
> keys, access to the raw ciphers may not be possible through this API. And let 
> us face it, a lot of user space code shall support many different OSes. Thus, 
> if you have a crypto lib in user space who has its own key management (which 
> is a core element of such libraries and thus cannot be put into an 
> architecture-dependent code part), having only the keyctl API on Linux for 
> accelerated asym support may not be helpful.

That argument is just non-sense. The crypto libraries that run on multiple OSes have already enough abstraction layers to deal with this. And frankly there it would be preferred if hardware backed keys are handled properly. Since that is what is really needed.

Also I have to repeat my comment from above. The asymmetric ciphers are really bad for any kind of streaming operation. Using them in that way is almost always going to end badly.

David Howells has patches for sign/verify/encrypt/decrypt operation. Keep in mind that all the parameters of the asymmetric keys are really bound to its cipher. So it is pretty obvious that the key itself defines what cipher is used. I do not get the wish for raw access to RSA or ECDH for example. You need the right set of keys and parameters first. Otherwise it is all garbage and not even guaranteed to be cryptographically secure.

> The second part is a keyctl domain. I see in-kernel support for this scenario, 
> but have not yet seen the kernel/user interface nor the user space support.

Actually have you looked at Mat’s kernel tree and the support for it in ELL.

> Both options are orthogonal, IMHO.

I don’t agree with that. As explained above, asymmetric ciphers are bound to their keys. For me this all feels like a hammer approach. You want to treat things as nails since that is the only thing you have. However that is not the case. We have the keyring subsystem.

> Tadeusz Struck provided a patch to link the kernel crypto API / algif_akcipher 
> with the keys subsystem to allow the second requirement to be implemented in 
> algif_akcipher. That patch is on my desk and I plan to integrate it and even 
> make it generic to allow its use for all different cipher types. I have not 
> yet integrated it to allow a small patch set to be reviewed. If it is the will 
> of the council, I can surely add that code to the initial patch set and 
> resubmit.

For symmetric ciphers this is awesome. For asymmetric ciphers I have no idea on how this would work. Once you provided the key handle, then the crypto subsystem would have to select the cipher. However that is not how it is designed. You are binding a cipher early one. In addition, depending on the key, you would need to choose the right hardware to execute on (in case of hardware bound keys). It is also not designed for this either.

So I have no idea how you want to overcome the design limitations / choices of AF_ALG when it comes to supporting assymetric ciphers correctly.

Regards

Marcel

Re: [PATCH v8 0/4] crypto: add algif_akcipher user space API

[Stephan Mueller]

Am Montag, 14. August 2017, 08:26:22 CEST schrieb Marcel Holtmann:

Hi Marcel,

> > The first part is clearly where AF_ALG fits and keyctl does not. This is
> > provided with the current patch set. As the keyctl API only handles, well,
> > keys, access to the raw ciphers may not be possible through this API. And
> > let us face it, a lot of user space code shall support many different
> > OSes. Thus, if you have a crypto lib in user space who has its own key
> > management (which is a core element of such libraries and thus cannot be
> > put into an architecture-dependent code part), having only the keyctl API
> > on Linux for accelerated asym support may not be helpful.
> 
> That argument is just non-sense.

How interesting. For example, what about NSS with its own key database?

Ciao
Stephan

Re: [PATCH v8 0/4] crypto: add algif_akcipher user space API

[Marcel Holtmann]

Hi Stephan,

>>> The first part is clearly where AF_ALG fits and keyctl does not. This is
>>> provided with the current patch set. As the keyctl API only handles, well,
>>> keys, access to the raw ciphers may not be possible through this API. And
>>> let us face it, a lot of user space code shall support many different
>>> OSes. Thus, if you have a crypto lib in user space who has its own key
>>> management (which is a core element of such libraries and thus cannot be
>>> put into an architecture-dependent code part), having only the keyctl API
>>> on Linux for accelerated asym support may not be helpful.
>> 
>> That argument is just non-sense.
> 
> How interesting. For example, what about NSS with its own key database?

a lot of applications create their own key or certificate database. It also means they need to reload and reload them over and over again for each process. A lot of things are possible, but why keep doing things more complicated than they need to be. As I said before, if you only have a hammer ..

Regards

Marcel

Re: [PATCH v8 0/4] crypto: add algif_akcipher user space API

[Tudor Ambarus]

Hi, all,

On 08/10/2017 09:39 AM, Stephan Müller wrote:
> Hi,
> 
> This patch set adds the AF_ALG user space API to externalize the
> asymmetric cipher API recently added to the kernel crypto API.

Do we have enough pros and cons so we can decide which interface to use
for exporting akcipher/kpp to user-space?

I'm willing to invest some time to make this happen.

Cheers,
ta

Re: [PATCH v8 0/4] crypto: add algif_akcipher user space API

[Mat Martineau]

[-- Attachment #1: Type: text/plain, Size: 1780 bytes --]


Hi Tudor -

On Mon, 2 Oct 2017, Tudor Ambarus wrote:

> Hi, all,
>
> On 08/10/2017 09:39 AM, Stephan Müller wrote:
>> Hi,
>> 
>> This patch set adds the AF_ALG user space API to externalize the
>> asymmetric cipher API recently added to the kernel crypto API.
>
> Do we have enough pros and cons so we can decide which interface to use
> for exporting akcipher/kpp to user-space?
>
> I'm willing to invest some time to make this happen.

David Howells has a branch implementing asymmetric crypto operations using 
keyctl:

https://git.kernel.org/pub/scm/linux/kernel/git/dhowells/linux-fs.git/log/?h=keys-asym-keyctl

That's based on v4.8-rc1, but I've been merging it with more recent 
kernels, and it's been working fine:

https://git.kernel.org/pub/scm/linux/kernel/git/martineau/linux.git/log/?h=ell-key-crypto

As far as I know, these keyctl operations are not upstream yet because 
there isn't yet a hardware (TPM) asymmetric key subtype that would allow 
the key subsystem to choose between akcipher or TPM crypto depending on 
the key subtype in use.

I'd like to see the functionality merged even with only akcipher support. 
The only issue I've had with the existing patch set is that RSA 
verification can't unpad certain results before comparing because the 
necessary functionality was removed from rsa-pkcs1pad in the kernel. This 
isn't a limitation of the keyctl patch set, though.

A handful of the people on this thread had agreed to move ahead with the 
keyctl API because it could handle key material well and choose between 
crypto engines (including engines that did not support software fallback) 
- that's when David put together his patches. David, is there help you 
could use with the hardware asymmetric key subtype or other aspects of 
keyctl crypto?