[PATCH 0/1] crypto: af_alg - add extra parameters for DRBG interface

[PATCH 0/1] crypto: af_alg - add extra parameters for DRBG interface

[Elena Petrova]

This patch extends the userspace RNG interface to make it usable for
CAVS testing. This is achieved by adding ALG_SET_DRBG_ENTROPY
option to the setsockopt interface for specifying the entropy, and using
sendmsg syscall for specifying the additional data.

See libkcapi patch [1] to test the added functionality. The libkcapi
patch is not intended for merging into libkcapi as is: it is only a
quick plug to manually verify that the extended AF_ALG RNG interface
generates the expected output on DRBG800-90A CAVS inputs.

[1] https://github.com/Len0k/libkcapi/commit/6f095d270b982008f419078614c15caa592cb531

Elena Petrova (1):
  crypto: af_alg - add extra parameters for DRBG interface

 Documentation/crypto/userspace-if.rst | 14 +++-
 crypto/af_alg.c                       |  8 +++
 crypto/algif_rng.c                    | 99 +++++++++++++++++++++++++--
 include/crypto/if_alg.h               |  3 +-
 include/uapi/linux/if_alg.h           |  1 +
 5 files changed, 115 insertions(+), 10 deletions(-)

--
2.27.0.383.g050319c2ae-goog

[PATCH 1/1] crypto: af_alg - add extra parameters for DRBG interface

[Elena Petrova]

Extending the userspace RNG interface:
  1. adding ALG_SET_DRBG_ENTROPY setsockopt option for entropy input;
  2. using sendmsg syscall for specifying the additional data.

Signed-off-by: Elena Petrova <lenaptr@google.com>
---
 Documentation/crypto/userspace-if.rst | 14 +++-
 crypto/af_alg.c                       |  8 +++
 crypto/algif_rng.c                    | 99 +++++++++++++++++++++++++--
 include/crypto/if_alg.h               |  3 +-
 include/uapi/linux/if_alg.h           |  1 +
 5 files changed, 115 insertions(+), 10 deletions(-)

diff --git a/Documentation/crypto/userspace-if.rst b/Documentation/crypto/userspace-if.rst
index ff86befa61e0..9ee9e93e9207 100644
--- a/Documentation/crypto/userspace-if.rst
+++ b/Documentation/crypto/userspace-if.rst
@@ -296,15 +296,20 @@ follows:
 
     struct sockaddr_alg sa = {
         .salg_family = AF_ALG,
-        .salg_type = "rng", /* this selects the symmetric cipher */
-        .salg_name = "drbg_nopr_sha256" /* this is the cipher name */
+        .salg_type = "rng", /* this selects the random number generator */
+        .salg_name = "drbg_nopr_sha256" /* this is the RNG name */
     };
 
 
 Depending on the RNG type, the RNG must be seeded. The seed is provided
 using the setsockopt interface to set the key. For example, the
 ansi_cprng requires a seed. The DRBGs do not require a seed, but may be
-seeded.
+seeded. The seed is also known as a *Personalization String* in DRBG800-90A
+standard.
+
+For the purpose of CAVS testing, the concatenation of *Entropy* and *Nonce*
+can be provided to the RNG via ALG_SET_DRBG_ENTROPY setsockopt interface.
+*Additional Data* can be provided using the send()/sendmsg() system calls.
 
 Using the read()/recvmsg() system calls, random numbers can be obtained.
 The kernel generates at most 128 bytes in one call. If user space
@@ -377,6 +382,9 @@ mentioned optname:
    provided ciphertext is assumed to contain an authentication tag of
    the given size (see section about AEAD memory layout below).
 
+-  ALG_SET_DRBG_ENTROPY -- Setting the entropy of the random number generator.
+   This option is applicable to RNG cipher type only.
+
 User space API example
 ----------------------
 
diff --git a/crypto/af_alg.c b/crypto/af_alg.c
index b1cd3535c525..27d6248ca447 100644
--- a/crypto/af_alg.c
+++ b/crypto/af_alg.c
@@ -260,6 +260,14 @@ static int alg_setsockopt(struct socket *sock, int level, int optname,
 		if (!type->setauthsize)
 			goto unlock;
 		err = type->setauthsize(ask->private, optlen);
+		break;
+	case ALG_SET_DRBG_ENTROPY:
+		if (sock->state == SS_CONNECTED)
+			goto unlock;
+		if (!type->setentropy)
+			goto unlock;
+
+		err = type->setentropy(ask->private, optval, optlen);
 	}
 
 unlock:
diff --git a/crypto/algif_rng.c b/crypto/algif_rng.c
index 087c0ad09d38..c3d1667db367 100644
--- a/crypto/algif_rng.c
+++ b/crypto/algif_rng.c
@@ -53,8 +53,24 @@ struct rng_ctx {
 #define MAXSIZE 128
 	unsigned int len;
 	struct crypto_rng *drng;
+	u8 *addtl;
+	size_t addtl_len;
 };
 
+struct rng_parent_ctx {
+	struct crypto_rng *drng;
+	u8 *entropy;
+};
+
+static void reset_addtl(struct rng_ctx *ctx)
+{
+	if (ctx->addtl) {
+		kzfree(ctx->addtl);
+		ctx->addtl = NULL;
+	}
+	ctx->addtl_len = 0;
+}
+
 static int rng_recvmsg(struct socket *sock, struct msghdr *msg, size_t len,
 		       int flags)
 {
@@ -82,16 +98,39 @@ static int rng_recvmsg(struct socket *sock, struct msghdr *msg, size_t len,
 	 * seeding as they automatically seed. The X9.31 DRNG will return
 	 * an error if it was not seeded properly.
 	 */
-	genlen = crypto_rng_get_bytes(ctx->drng, result, len);
+	genlen = crypto_rng_generate(ctx->drng, ctx->addtl, ctx->addtl_len,
+				     result, len);
 	if (genlen < 0)
 		return genlen;
 
 	err = memcpy_to_msg(msg, result, len);
 	memzero_explicit(result, len);
+	reset_addtl(ctx);
 
 	return err ? err : len;
 }
 
+static int rng_sendmsg(struct socket *sock, struct msghdr *msg, size_t len)
+{
+	int err;
+	struct alg_sock *ask = alg_sk(sock->sk);
+	struct rng_ctx *ctx = ask->private;
+
+	reset_addtl(ctx);
+	ctx->addtl = kzalloc(len, GFP_KERNEL);
+	if (!ctx->addtl)
+		return -ENOMEM;
+
+	err = memcpy_from_msg(ctx->addtl, msg, len);
+	if (err) {
+		reset_addtl(ctx);
+		return err;
+	}
+	ctx->addtl_len = len;
+
+	return 0;
+}
+
 static struct proto_ops algif_rng_ops = {
 	.family		=	PF_ALG,
 
@@ -106,21 +145,40 @@ static struct proto_ops algif_rng_ops = {
 	.bind		=	sock_no_bind,
 	.accept		=	sock_no_accept,
 	.setsockopt	=	sock_no_setsockopt,
-	.sendmsg	=	sock_no_sendmsg,
 	.sendpage	=	sock_no_sendpage,
 
 	.release	=	af_alg_release,
 	.recvmsg	=	rng_recvmsg,
+	.sendmsg	=	rng_sendmsg,
 };
 
 static void *rng_bind(const char *name, u32 type, u32 mask)
 {
-	return crypto_alloc_rng(name, type, mask);
+	struct rng_parent_ctx *pctx;
+	void *err_ptr;
+
+	pctx = kzalloc(sizeof(*pctx), GFP_KERNEL);
+	if (!pctx)
+		return ERR_PTR(-ENOMEM);
+
+	pctx->drng = crypto_alloc_rng(name, type, mask);
+	if (!IS_ERR(pctx->drng))
+		return pctx;
+
+	err_ptr = pctx->drng;
+	kfree(pctx);
+	return err_ptr;
 }
 
 static void rng_release(void *private)
 {
-	crypto_free_rng(private);
+	struct rng_parent_ctx *pctx = private;
+	if (unlikely(!pctx))
+		return;
+	crypto_free_rng(pctx->drng);
+	if (pctx->entropy)
+		kzfree(pctx->entropy);
+	kzfree(pctx);
 }
 
 static void rng_sock_destruct(struct sock *sk)
@@ -128,6 +186,7 @@ static void rng_sock_destruct(struct sock *sk)
 	struct alg_sock *ask = alg_sk(sk);
 	struct rng_ctx *ctx = ask->private;
 
+	reset_addtl(ctx);
 	sock_kfree_s(sk, ctx, ctx->len);
 	af_alg_release_parent(sk);
 }
@@ -135,6 +194,7 @@ static void rng_sock_destruct(struct sock *sk)
 static int rng_accept_parent(void *private, struct sock *sk)
 {
 	struct rng_ctx *ctx;
+	struct rng_parent_ctx *pctx = private;
 	struct alg_sock *ask = alg_sk(sk);
 	unsigned int len = sizeof(*ctx);
 
@@ -150,7 +210,9 @@ static int rng_accept_parent(void *private, struct sock *sk)
 	 * state of the RNG.
 	 */
 
-	ctx->drng = private;
+	ctx->drng = pctx->drng;
+	ctx->addtl = NULL;
+	ctx->addtl_len = 0;
 	ask->private = ctx;
 	sk->sk_destruct = rng_sock_destruct;
 
@@ -159,11 +221,35 @@ static int rng_accept_parent(void *private, struct sock *sk)
 
 static int rng_setkey(void *private, const u8 *seed, unsigned int seedlen)
 {
+	struct rng_parent_ctx *pctx = private;
 	/*
 	 * Check whether seedlen is of sufficient size is done in RNG
 	 * implementations.
 	 */
-	return crypto_rng_reset(private, seed, seedlen);
+	return crypto_rng_reset(pctx->drng, seed, seedlen);
+}
+
+static int rng_setentropy(void *private, const u8 *entropy, unsigned int len)
+{
+	struct rng_parent_ctx *pctx = private;
+	u8 *kentropy = NULL;
+
+	if (pctx->entropy)
+		return -EINVAL;
+
+	if (entropy && len) {
+		kentropy = kzalloc(len, GFP_KERNEL);
+		if (!kentropy)
+			return -ENOMEM;
+		if (copy_from_user(kentropy, entropy, len)) {
+			kzfree(kentropy);
+			return -EFAULT;
+		}
+	}
+
+	crypto_rng_alg(pctx->drng)->set_ent(pctx->drng, kentropy, len);
+	pctx->entropy = kentropy;
+	return 0;
 }
 
 static const struct af_alg_type algif_type_rng = {
@@ -171,6 +257,7 @@ static const struct af_alg_type algif_type_rng = {
 	.release	=	rng_release,
 	.accept		=	rng_accept_parent,
 	.setkey		=	rng_setkey,
+	.setentropy	=	rng_setentropy,
 	.ops		=	&algif_rng_ops,
 	.name		=	"rng",
 	.owner		=	THIS_MODULE
diff --git a/include/crypto/if_alg.h b/include/crypto/if_alg.h
index 56527c85d122..312fdb3469cf 100644
--- a/include/crypto/if_alg.h
+++ b/include/crypto/if_alg.h
@@ -46,6 +46,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 (*setentropy)(void *private, const u8 *entropy, unsigned int len);
 	int (*accept)(void *private, struct sock *sk);
 	int (*accept_nokey)(void *private, struct sock *sk);
 	int (*setauthsize)(void *private, unsigned int authsize);
@@ -123,7 +124,7 @@ struct af_alg_async_req {
  * @tsgl_list:		Link to TX SGL
  * @iv:			IV for cipher operation
  * @aead_assoclen:	Length of AAD for AEAD cipher operations
- * @completion:		Work queue for synchronous operation
+ * @wait:		Wait on completion of async crypto ops
  * @used:		TX bytes sent to kernel. This variable is used to
  *			ensure that user space cannot cause the kernel
  *			to allocate too much memory in sendmsg operation.
diff --git a/include/uapi/linux/if_alg.h b/include/uapi/linux/if_alg.h
index bc2bcdec377b..60b7c2efd921 100644
--- a/include/uapi/linux/if_alg.h
+++ b/include/uapi/linux/if_alg.h
@@ -35,6 +35,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_DRBG_ENTROPY		6
 
 /* Operations */
 #define ALG_OP_DECRYPT			0
-- 
2.27.0.383.g050319c2ae-goog

Re: [PATCH 1/1] crypto: af_alg - add extra parameters for DRBG interface

[Eric Biggers]

Just some bike-shedding:

On Mon, Jul 13, 2020 at 05:48:57PM +0100, Elena Petrova wrote:
> Extending the userspace RNG interface:
>   1. adding ALG_SET_DRBG_ENTROPY setsockopt option for entropy input;
>   2. using sendmsg syscall for specifying the additional data.
> 
> Signed-off-by: Elena Petrova <lenaptr@google.com>

A cover letter shouldn't really be used for a single patch.
Just put the details here in the commit message.

> diff --git a/crypto/algif_rng.c b/crypto/algif_rng.c
> index 087c0ad09d38..c3d1667db367 100644
> --- a/crypto/algif_rng.c
> +++ b/crypto/algif_rng.c
> @@ -53,8 +53,24 @@ struct rng_ctx {
>  #define MAXSIZE 128
>  	unsigned int len;
>  	struct crypto_rng *drng;
> +	u8 *addtl;
> +	size_t addtl_len;
>  };
>  
> +struct rng_parent_ctx {
> +	struct crypto_rng *drng;
> +	u8 *entropy;
> +};
> +
> +static void reset_addtl(struct rng_ctx *ctx)
> +{
> +	if (ctx->addtl) {
> +		kzfree(ctx->addtl);
> +		ctx->addtl = NULL;
> +	}
> +	ctx->addtl_len = 0;
> +}

It's recommended to prefix function names.  So, reset_addtl => rng_reset_addtl.

> +static int rng_sendmsg(struct socket *sock, struct msghdr *msg, size_t len)
> +{
> +	int err;
> +	struct alg_sock *ask = alg_sk(sock->sk);
> +	struct rng_ctx *ctx = ask->private;
> +
> +	reset_addtl(ctx);
> +	ctx->addtl = kzalloc(len, GFP_KERNEL);
> +	if (!ctx->addtl)
> +		return -ENOMEM;

Shouldn't the length be limited here?

Also, kmalloc would be sufficient since the memcpy_from_msg() immediately below
initializes the memory.

> +
> +	err = memcpy_from_msg(ctx->addtl, msg, len);
> +	if (err) {
> +		reset_addtl(ctx);
> +		return err;
> +	}
> +	ctx->addtl_len = len;
> +
> +	return 0;
> +}

>  static void *rng_bind(const char *name, u32 type, u32 mask)
>  {
> -	return crypto_alloc_rng(name, type, mask);
> +	struct rng_parent_ctx *pctx;
> +	void *err_ptr;
> +
> +	pctx = kzalloc(sizeof(*pctx), GFP_KERNEL);
> +	if (!pctx)
> +		return ERR_PTR(-ENOMEM);
> +
> +	pctx->drng = crypto_alloc_rng(name, type, mask);
> +	if (!IS_ERR(pctx->drng))
> +		return pctx;
> +
> +	err_ptr = pctx->drng;
> +	kfree(pctx);
> +	return err_ptr;
>  }

The error handling here is weird.  It would be more conventional to do something
like:

static void *rng_bind(const char *name, u32 type, u32 mask)
{
	struct rng_parent_ctx *pctx;
	struct crypto_rng *rng;

	pctx = kzalloc(sizeof(*pctx), GFP_KERNEL);
	if (!pctx)
		return ERR_PTR(-ENOMEM);

	rng = crypto_alloc_rng(name, type, mask);
	if (IS_ERR(rng)) {
		kfree(pctx);
		return ERR_CAST(rng);
	}

	pctx->drng = rng;
	return pctx;
}

>  
>  static void rng_release(void *private)
>  {
> -	crypto_free_rng(private);
> +	struct rng_parent_ctx *pctx = private;
> +	if (unlikely(!pctx))
> +		return;

There should be a blank line between declarations and statements.

> +	crypto_free_rng(pctx->drng);
> +	if (pctx->entropy)
> +		kzfree(pctx->entropy);

No need to check for NULL before calling kzfree().

> +static int rng_setentropy(void *private, const u8 *entropy, unsigned int len)
> +{
> +	struct rng_parent_ctx *pctx = private;
> +	u8 *kentropy = NULL;
> +
> +	if (pctx->entropy)
> +		return -EINVAL;
> +
> +	if (entropy && len) {

Best to check just 'len', so that users get an error as expected if they
accidentally pass entry=NULL len=nonzero.

> +		kentropy = kzalloc(len, GFP_KERNEL);
> +		if (!kentropy)
> +			return -ENOMEM;
> +		if (copy_from_user(kentropy, entropy, len)) {
> +			kzfree(kentropy);
> +			return -EFAULT;
> +		}

This can use memdup_user().  Also, should there be a length limit?

> +	}
> +
> +	crypto_rng_alg(pctx->drng)->set_ent(pctx->drng, kentropy, len);
> +	pctx->entropy = kentropy;

pctx->entropy could just be a bool 'has_entropy', right?  The actual value
doesn't need to be saved.

- Eric

Re: [PATCH 1/1] crypto: af_alg - add extra parameters for DRBG interface

[Elena Petrova]

Thank you for the review, Eric, I'll address your comments in v2.

On Mon, 13 Jul 2020 at 18:10, Eric Biggers <ebiggers@kernel.org> wrote:
>
> Just some bike-shedding:
>
> On Mon, Jul 13, 2020 at 05:48:57PM +0100, Elena Petrova wrote:
> > Extending the userspace RNG interface:
> >   1. adding ALG_SET_DRBG_ENTROPY setsockopt option for entropy input;
> >   2. using sendmsg syscall for specifying the additional data.
> >
> > Signed-off-by: Elena Petrova <lenaptr@google.com>
>
> A cover letter shouldn't really be used for a single patch.
> Just put the details here in the commit message.

Ack

> > diff --git a/crypto/algif_rng.c b/crypto/algif_rng.c
> > index 087c0ad09d38..c3d1667db367 100644
> > --- a/crypto/algif_rng.c
> > +++ b/crypto/algif_rng.c
> > @@ -53,8 +53,24 @@ struct rng_ctx {
> >  #define MAXSIZE 128
> >       unsigned int len;
> >       struct crypto_rng *drng;
> > +     u8 *addtl;
> > +     size_t addtl_len;
> >  };
> >
> > +struct rng_parent_ctx {
> > +     struct crypto_rng *drng;
> > +     u8 *entropy;
> > +};
> > +
> > +static void reset_addtl(struct rng_ctx *ctx)
> > +{
> > +     if (ctx->addtl) {
> > +             kzfree(ctx->addtl);
> > +             ctx->addtl = NULL;
> > +     }
> > +     ctx->addtl_len = 0;
> > +}
>
> It's recommended to prefix function names.  So, reset_addtl => rng_reset_addtl.

Ack

> > +static int rng_sendmsg(struct socket *sock, struct msghdr *msg, size_t len)
> > +{
> > +     int err;
> > +     struct alg_sock *ask = alg_sk(sock->sk);
> > +     struct rng_ctx *ctx = ask->private;
> > +
> > +     reset_addtl(ctx);
> > +     ctx->addtl = kzalloc(len, GFP_KERNEL);
> > +     if (!ctx->addtl)
> > +             return -ENOMEM;
>
> Shouldn't the length be limited here?
>
> Also, kmalloc would be sufficient since the memcpy_from_msg() immediately below
> initializes the memory.

Good point, I'll use the same limit as for the recv(). Ack kzalloc/kmalloc.

> > +
> > +     err = memcpy_from_msg(ctx->addtl, msg, len);
> > +     if (err) {
> > +             reset_addtl(ctx);
> > +             return err;
> > +     }
> > +     ctx->addtl_len = len;
> > +
> > +     return 0;
> > +}
>
> >  static void *rng_bind(const char *name, u32 type, u32 mask)
> >  {
> > -     return crypto_alloc_rng(name, type, mask);
> > +     struct rng_parent_ctx *pctx;
> > +     void *err_ptr;
> > +
> > +     pctx = kzalloc(sizeof(*pctx), GFP_KERNEL);
> > +     if (!pctx)
> > +             return ERR_PTR(-ENOMEM);
> > +
> > +     pctx->drng = crypto_alloc_rng(name, type, mask);
> > +     if (!IS_ERR(pctx->drng))
> > +             return pctx;
> > +
> > +     err_ptr = pctx->drng;
> > +     kfree(pctx);
> > +     return err_ptr;
> >  }
>
> The error handling here is weird.  It would be more conventional to do something
> like:
>
> static void *rng_bind(const char *name, u32 type, u32 mask)
> {
>         struct rng_parent_ctx *pctx;
>         struct crypto_rng *rng;
>
>         pctx = kzalloc(sizeof(*pctx), GFP_KERNEL);
>         if (!pctx)
>                 return ERR_PTR(-ENOMEM);
>
>         rng = crypto_alloc_rng(name, type, mask);
>         if (IS_ERR(rng)) {
>                 kfree(pctx);
>                 return ERR_CAST(rng);
>         }
>
>         pctx->drng = rng;
>         return pctx;
> }

Thanks, I will use your variant.

> >  static void rng_release(void *private)
> >  {
> > -     crypto_free_rng(private);
> > +     struct rng_parent_ctx *pctx = private;
> > +     if (unlikely(!pctx))
> > +             return;
>
> There should be a blank line between declarations and statements.

Ack

> > +     crypto_free_rng(pctx->drng);
> > +     if (pctx->entropy)
> > +             kzfree(pctx->entropy);
>
> No need to check for NULL before calling kzfree().

Ack

> > +static int rng_setentropy(void *private, const u8 *entropy, unsigned int len)
> > +{
> > +     struct rng_parent_ctx *pctx = private;
> > +     u8 *kentropy = NULL;
> > +
> > +     if (pctx->entropy)
> > +             return -EINVAL;
> > +
> > +     if (entropy && len) {
>
> Best to check just 'len', so that users get an error as expected if they
> accidentally pass entry=NULL len=nonzero.

Ack

> > +             kentropy = kzalloc(len, GFP_KERNEL);
> > +             if (!kentropy)
> > +                     return -ENOMEM;
> > +             if (copy_from_user(kentropy, entropy, len)) {
> > +                     kzfree(kentropy);
> > +                     return -EFAULT;
> > +             }
>
> This can use memdup_user().  Also, should there be a length limit?

Alright, changed to memdup_user() and added the same limit as in send and recv.

> > +     }
> > +
> > +     crypto_rng_alg(pctx->drng)->set_ent(pctx->drng, kentropy, len);
> > +     pctx->entropy = kentropy;
>
> pctx->entropy could just be a bool 'has_entropy', right?  The actual value
> doesn't need to be saved.

I need to keep the pointer to free it after use. DRBG saves the
pointer in one of its internal structures, but doesn't do any memory
management. So I had to either change drbg code to deal with the
memory, or save the pointer somewhere inside the socket. I opted for
the latter.

> > +static int rng_sendmsg(struct socket *sock, struct msghdr *msg, size_t len)
> > +{
> > +     int err;
> > +     struct alg_sock *ask = alg_sk(sock->sk);
> > +     struct rng_ctx *ctx = ask->private;
> > +
> > +     reset_addtl(ctx);
> > +     ctx->addtl = kzalloc(len, GFP_KERNEL);
> > +     if (!ctx->addtl)
> > +             return -ENOMEM;
> > +
> > +     err = memcpy_from_msg(ctx->addtl, msg, len);
> > +     if (err) {
> > +             reset_addtl(ctx);
> > +             return err;
> > +     }
> > +     ctx->addtl_len = len;
> > +
> > +     return 0;
> > +}
>
> This is also missing any sort of locking, both between concurrent calls to
> rng_sendmsg(), and between rng_sendmsg() and rng_recvmsg().
>
> lock_sock() would solve the former.  I'm not sure what should be done about
> rng_recvmsg().  It apparently relies on the crypto_rng doing its own locking,
> but maybe it should just use lock_sock() too.

Thanks, I've added lock_sock() to both.

> - Eric

[PATCH v2] crypto: af_alg - add extra parameters for DRBG interface

[Elena Petrova]

Extending the userspace RNG interface:
  1. adding ALG_SET_DRBG_ENTROPY setsockopt option for entropy input;
  2. using sendmsg syscall for specifying the additional data.

Signed-off-by: Elena Petrova <lenaptr@google.com>
---

libkcapi patch for testing:
  https://github.com/Len0k/libkcapi/commit/6f095d270b982008f419078614c15caa592cb531

Updates in v2:
  1) Adding CONFIG_CRYPTO_CAVS_DRBG around setentropy.
  2) Requiring CAP_SYS_ADMIN for entropy reset.
  3) Locking for send and recv.
  4) Length checks added for send and setentropy; send and setentropy now return
     number of bytes accepted.
  5) Minor code style corrections.

 Documentation/crypto/userspace-if.rst |  17 +++-
 crypto/Kconfig                        |   8 ++
 crypto/af_alg.c                       |   8 ++
 crypto/algif_rng.c                    | 112 ++++++++++++++++++++++++--
 include/crypto/if_alg.h               |   3 +-
 include/uapi/linux/if_alg.h           |   1 +
 6 files changed, 139 insertions(+), 10 deletions(-)

diff --git a/Documentation/crypto/userspace-if.rst b/Documentation/crypto/userspace-if.rst
index ff86befa61e0..c3695d2c7e0b 100644
--- a/Documentation/crypto/userspace-if.rst
+++ b/Documentation/crypto/userspace-if.rst
@@ -296,15 +296,23 @@ follows:
 
     struct sockaddr_alg sa = {
         .salg_family = AF_ALG,
-        .salg_type = "rng", /* this selects the symmetric cipher */
-        .salg_name = "drbg_nopr_sha256" /* this is the cipher name */
+        .salg_type = "rng", /* this selects the random number generator */
+        .salg_name = "drbg_nopr_sha256" /* this is the RNG name */
     };
 
 
 Depending on the RNG type, the RNG must be seeded. The seed is provided
 using the setsockopt interface to set the key. For example, the
 ansi_cprng requires a seed. The DRBGs do not require a seed, but may be
-seeded.
+seeded. The seed is also known as a *Personalization String* in DRBG800-90A
+standard.
+
+For the purpose of CAVS testing, the concatenation of *Entropy* and *Nonce*
+can be provided to the RNG via ALG_SET_DRBG_ENTROPY setsockopt interface. This
+requires a kernel built with CONFIG_CRYPTO_CAVS_DRBG, and CAP_SYS_ADMIN
+permission.
+
+*Additional Data* can be provided using the send()/sendmsg() system calls.
 
 Using the read()/recvmsg() system calls, random numbers can be obtained.
 The kernel generates at most 128 bytes in one call. If user space
@@ -377,6 +385,9 @@ mentioned optname:
    provided ciphertext is assumed to contain an authentication tag of
    the given size (see section about AEAD memory layout below).
 
+-  ALG_SET_DRBG_ENTROPY -- Setting the entropy of the random number generator.
+   This option is applicable to RNG cipher type only.
+
 User space API example
 ----------------------
 
diff --git a/crypto/Kconfig b/crypto/Kconfig
index 091c0a0bbf26..8484617596d1 100644
--- a/crypto/Kconfig
+++ b/crypto/Kconfig
@@ -1896,6 +1896,14 @@ config CRYPTO_STATS
 config CRYPTO_HASH_INFO
 	bool
 
+config CRYPTO_CAVS_DRBG
+	tristate "Enable CAVS testing of DRBG"
+	depends on CRYPTO_USER_API_RNG && CRYPTO_DRBG
+	help
+	  This option enables the resetting of DRBG entropy via the user-space
+	  interface. This should only be enabled for CAVS testing. You should say
+	  no unless you know what this is.
+
 source "lib/crypto/Kconfig"
 source "drivers/crypto/Kconfig"
 source "crypto/asymmetric_keys/Kconfig"
diff --git a/crypto/af_alg.c b/crypto/af_alg.c
index b1cd3535c525..27d6248ca447 100644
--- a/crypto/af_alg.c
+++ b/crypto/af_alg.c
@@ -260,6 +260,14 @@ static int alg_setsockopt(struct socket *sock, int level, int optname,
 		if (!type->setauthsize)
 			goto unlock;
 		err = type->setauthsize(ask->private, optlen);
+		break;
+	case ALG_SET_DRBG_ENTROPY:
+		if (sock->state == SS_CONNECTED)
+			goto unlock;
+		if (!type->setentropy)
+			goto unlock;
+
+		err = type->setentropy(ask->private, optval, optlen);
 	}
 
 unlock:
diff --git a/crypto/algif_rng.c b/crypto/algif_rng.c
index 087c0ad09d38..8a3b0eb45a85 100644
--- a/crypto/algif_rng.c
+++ b/crypto/algif_rng.c
@@ -38,6 +38,7 @@
  * DAMAGE.
  */
 
+#include <linux/capability.h>
 #include <linux/module.h>
 #include <crypto/rng.h>
 #include <linux/random.h>
@@ -53,8 +54,22 @@ struct rng_ctx {
 #define MAXSIZE 128
 	unsigned int len;
 	struct crypto_rng *drng;
+	u8 *addtl;
+	size_t addtl_len;
 };
 
+struct rng_parent_ctx {
+	struct crypto_rng *drng;
+	u8 *entropy;
+};
+
+static void rng_reset_addtl(struct rng_ctx *ctx)
+{
+	kzfree(ctx->addtl);
+	ctx->addtl = NULL;
+	ctx->addtl_len = 0;
+}
+
 static int rng_recvmsg(struct socket *sock, struct msghdr *msg, size_t len,
 		       int flags)
 {
@@ -65,6 +80,7 @@ static int rng_recvmsg(struct socket *sock, struct msghdr *msg, size_t len,
 	int genlen = 0;
 	u8 result[MAXSIZE];
 
+	lock_sock(sock->sk);
 	if (len == 0)
 		return 0;
 	if (len > MAXSIZE)
@@ -82,16 +98,45 @@ static int rng_recvmsg(struct socket *sock, struct msghdr *msg, size_t len,
 	 * seeding as they automatically seed. The X9.31 DRNG will return
 	 * an error if it was not seeded properly.
 	 */
-	genlen = crypto_rng_get_bytes(ctx->drng, result, len);
+	genlen = crypto_rng_generate(ctx->drng, ctx->addtl, ctx->addtl_len,
+				     result, len);
 	if (genlen < 0)
 		return genlen;
 
 	err = memcpy_to_msg(msg, result, len);
 	memzero_explicit(result, len);
+	rng_reset_addtl(ctx);
+	release_sock(sock->sk);
 
 	return err ? err : len;
 }
 
+static int rng_sendmsg(struct socket *sock, struct msghdr *msg, size_t len)
+{
+	int err;
+	struct alg_sock *ask = alg_sk(sock->sk);
+	struct rng_ctx *ctx = ask->private;
+
+	lock_sock(sock->sk);
+	if (len > MAXSIZE)
+		len = MAXSIZE;
+
+	rng_reset_addtl(ctx);
+	ctx->addtl = kmalloc(len, GFP_KERNEL);
+	if (!ctx->addtl)
+		return -ENOMEM;
+
+	err = memcpy_from_msg(ctx->addtl, msg, len);
+	if (err) {
+		rng_reset_addtl(ctx);
+		return err;
+	}
+	ctx->addtl_len = len;
+	release_sock(sock->sk);
+
+	return len;
+}
+
 static struct proto_ops algif_rng_ops = {
 	.family		=	PF_ALG,
 
@@ -106,21 +151,41 @@ static struct proto_ops algif_rng_ops = {
 	.bind		=	sock_no_bind,
 	.accept		=	sock_no_accept,
 	.setsockopt	=	sock_no_setsockopt,
-	.sendmsg	=	sock_no_sendmsg,
 	.sendpage	=	sock_no_sendpage,
 
 	.release	=	af_alg_release,
 	.recvmsg	=	rng_recvmsg,
+	.sendmsg	=	rng_sendmsg,
 };
 
 static void *rng_bind(const char *name, u32 type, u32 mask)
 {
-	return crypto_alloc_rng(name, type, mask);
+	struct rng_parent_ctx *pctx;
+	struct crypto_rng *rng;
+
+	pctx = kzalloc(sizeof(*pctx), GFP_KERNEL);
+	if (!pctx)
+		return ERR_PTR(-ENOMEM);
+
+	rng = crypto_alloc_rng(name, type, mask);
+	if (IS_ERR(rng)) {
+		kfree(pctx);
+		return ERR_CAST(rng);
+	}
+
+	pctx->drng = rng;
+	return pctx;
 }
 
 static void rng_release(void *private)
 {
-	crypto_free_rng(private);
+	struct rng_parent_ctx *pctx = private;
+
+	if (unlikely(!pctx))
+		return;
+	crypto_free_rng(pctx->drng);
+	kzfree(pctx->entropy);
+	kzfree(pctx);
 }
 
 static void rng_sock_destruct(struct sock *sk)
@@ -128,6 +193,7 @@ static void rng_sock_destruct(struct sock *sk)
 	struct alg_sock *ask = alg_sk(sk);
 	struct rng_ctx *ctx = ask->private;
 
+	rng_reset_addtl(ctx);
 	sock_kfree_s(sk, ctx, ctx->len);
 	af_alg_release_parent(sk);
 }
@@ -135,6 +201,7 @@ static void rng_sock_destruct(struct sock *sk)
 static int rng_accept_parent(void *private, struct sock *sk)
 {
 	struct rng_ctx *ctx;
+	struct rng_parent_ctx *pctx = private;
 	struct alg_sock *ask = alg_sk(sk);
 	unsigned int len = sizeof(*ctx);
 
@@ -150,7 +217,9 @@ static int rng_accept_parent(void *private, struct sock *sk)
 	 * state of the RNG.
 	 */
 
-	ctx->drng = private;
+	ctx->drng = pctx->drng;
+	ctx->addtl = NULL;
+	ctx->addtl_len = 0;
 	ask->private = ctx;
 	sk->sk_destruct = rng_sock_destruct;
 
@@ -159,18 +228,49 @@ static int rng_accept_parent(void *private, struct sock *sk)
 
 static int rng_setkey(void *private, const u8 *seed, unsigned int seedlen)
 {
+	struct rng_parent_ctx *pctx = private;
 	/*
 	 * Check whether seedlen is of sufficient size is done in RNG
 	 * implementations.
 	 */
-	return crypto_rng_reset(private, seed, seedlen);
+	return crypto_rng_reset(pctx->drng, seed, seedlen);
+}
+
+#ifdef CONFIG_CRYPTO_CAVS_DRBG
+static int rng_setentropy(void *private, const u8 *entropy, unsigned int len)
+{
+	struct rng_parent_ctx *pctx = private;
+	u8 *kentropy = NULL;
+
+	if (!capable(CAP_SYS_ADMIN))
+		return -EPERM;
+
+	if (pctx->entropy)
+		return -EINVAL;
+
+	if (len > MAXSIZE)
+		len = MAXSIZE;
+
+	if (len) {
+		kentropy = memdup_user(entropy, len);
+		if (IS_ERR(kentropy))
+			return PTR_ERR(kentropy);
+	}
+
+	crypto_rng_alg(pctx->drng)->set_ent(pctx->drng, kentropy, len);
+	pctx->entropy = kentropy;
+	return len;
 }
+#endif
 
 static const struct af_alg_type algif_type_rng = {
 	.bind		=	rng_bind,
 	.release	=	rng_release,
 	.accept		=	rng_accept_parent,
 	.setkey		=	rng_setkey,
+#ifdef CONFIG_CRYPTO_CAVS_DRBG
+	.setentropy	=	rng_setentropy,
+#endif
 	.ops		=	&algif_rng_ops,
 	.name		=	"rng",
 	.owner		=	THIS_MODULE
diff --git a/include/crypto/if_alg.h b/include/crypto/if_alg.h
index 56527c85d122..312fdb3469cf 100644
--- a/include/crypto/if_alg.h
+++ b/include/crypto/if_alg.h
@@ -46,6 +46,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 (*setentropy)(void *private, const u8 *entropy, unsigned int len);
 	int (*accept)(void *private, struct sock *sk);
 	int (*accept_nokey)(void *private, struct sock *sk);
 	int (*setauthsize)(void *private, unsigned int authsize);
@@ -123,7 +124,7 @@ struct af_alg_async_req {
  * @tsgl_list:		Link to TX SGL
  * @iv:			IV for cipher operation
  * @aead_assoclen:	Length of AAD for AEAD cipher operations
- * @completion:		Work queue for synchronous operation
+ * @wait:		Wait on completion of async crypto ops
  * @used:		TX bytes sent to kernel. This variable is used to
  *			ensure that user space cannot cause the kernel
  *			to allocate too much memory in sendmsg operation.
diff --git a/include/uapi/linux/if_alg.h b/include/uapi/linux/if_alg.h
index bc2bcdec377b..60b7c2efd921 100644
--- a/include/uapi/linux/if_alg.h
+++ b/include/uapi/linux/if_alg.h
@@ -35,6 +35,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_DRBG_ENTROPY		6
 
 /* Operations */
 #define ALG_OP_DECRYPT			0
-- 
2.27.0.389.gc38d7665816-goog

Re: [PATCH v2] crypto: af_alg - add extra parameters for DRBG interface

[Stephan Mueller]

Am Donnerstag, 16. Juli 2020, 18:40:28 CEST schrieb Elena Petrova:

Hi Elena,

sorry for the delay in answering.

> Extending the userspace RNG interface:
>   1. adding ALG_SET_DRBG_ENTROPY setsockopt option for entropy input;
>   2. using sendmsg syscall for specifying the additional data.
> 
> Signed-off-by: Elena Petrova <lenaptr@google.com>
> ---
> 
> libkcapi patch for testing:
>  
> https://github.com/Len0k/libkcapi/commit/6f095d270b982008f419078614c15caa59
> 2cb531

If that kernel patch is taken, I would be happy to take the libkcapi patch 
with the following suggested changes:

- add full documentation of kcapi_rng_set_entropy and kcapi_rng_send_addtl to 
kcapi.h

- move test_cavp into either test/kcapi-main.c or its own application inside 
test/ where the caller provides the entropy_input, personalization string, 
additional inputs
> 
> Updates in v2:
>   1) Adding CONFIG_CRYPTO_CAVS_DRBG around setentropy.
>   2) Requiring CAP_SYS_ADMIN for entropy reset.
>   3) Locking for send and recv.
>   4) Length checks added for send and setentropy; send and setentropy now
> return number of bytes accepted.
>   5) Minor code style corrections.
> 
>  Documentation/crypto/userspace-if.rst |  17 +++-
>  crypto/Kconfig                        |   8 ++
>  crypto/af_alg.c                       |   8 ++
>  crypto/algif_rng.c                    | 112 ++++++++++++++++++++++++--
>  include/crypto/if_alg.h               |   3 +-
>  include/uapi/linux/if_alg.h           |   1 +
>  6 files changed, 139 insertions(+), 10 deletions(-)
> 
> diff --git a/Documentation/crypto/userspace-if.rst
> b/Documentation/crypto/userspace-if.rst index ff86befa61e0..c3695d2c7e0b
> 100644
> --- a/Documentation/crypto/userspace-if.rst
> +++ b/Documentation/crypto/userspace-if.rst
> @@ -296,15 +296,23 @@ follows:
> 
>      struct sockaddr_alg sa = {
>          .salg_family = AF_ALG,
> -        .salg_type = "rng", /* this selects the symmetric cipher */
> -        .salg_name = "drbg_nopr_sha256" /* this is the cipher name */
> +        .salg_type = "rng", /* this selects the random number generator */
> +        .salg_name = "drbg_nopr_sha256" /* this is the RNG name */
>      };
> 
> 
>  Depending on the RNG type, the RNG must be seeded. The seed is provided
>  using the setsockopt interface to set the key. For example, the
>  ansi_cprng requires a seed. The DRBGs do not require a seed, but may be
> -seeded.
> +seeded. The seed is also known as a *Personalization String* in DRBG800-90A
> +standard.
> +
> +For the purpose of CAVS testing, the concatenation of *Entropy* and *Nonce*
> +can be provided to the RNG via ALG_SET_DRBG_ENTROPY setsockopt interface.
> This +requires a kernel built with CONFIG_CRYPTO_CAVS_DRBG, and
> CAP_SYS_ADMIN +permission.
> +
> +*Additional Data* can be provided using the send()/sendmsg() system calls.
> 
>  Using the read()/recvmsg() system calls, random numbers can be obtained.
>  The kernel generates at most 128 bytes in one call. If user space
> @@ -377,6 +385,9 @@ mentioned optname:
>     provided ciphertext is assumed to contain an authentication tag of
>     the given size (see section about AEAD memory layout below).
> 
> +-  ALG_SET_DRBG_ENTROPY -- Setting the entropy of the random number
> generator. +   This option is applicable to RNG cipher type only.
> +
>  User space API example
>  ----------------------
> 
> diff --git a/crypto/Kconfig b/crypto/Kconfig
> index 091c0a0bbf26..8484617596d1 100644
> --- a/crypto/Kconfig
> +++ b/crypto/Kconfig
> @@ -1896,6 +1896,14 @@ config CRYPTO_STATS
>  config CRYPTO_HASH_INFO
>  	bool
> 
> +config CRYPTO_CAVS_DRBG

CAVS is dead, long live ACVT :-) So, maybe use CAVP or ACVT as abbreviations?

As the config option applies to AF_ALG, wouldn't it be better to use an 
appropriate name here like CRYPTO_USER_API_CAVP_DRBG or similar?

Note, if indeed we would add akcipher or even KPP with CAVP support, maybe we 
need additional config options here. So, I would recommend to use this 
separate name space.


> +	tristate "Enable CAVS testing of DRBG"

Dto: replace CAVS

> +	depends on CRYPTO_USER_API_RNG && CRYPTO_DRBG
> +	help
> +	  This option enables the resetting of DRBG entropy via the user-space
> +	  interface. This should only be enabled for CAVS testing. You should 
say
> +	  no unless you know what this is.
> +
>  source "lib/crypto/Kconfig"
>  source "drivers/crypto/Kconfig"
>  source "crypto/asymmetric_keys/Kconfig"
> diff --git a/crypto/af_alg.c b/crypto/af_alg.c
> index b1cd3535c525..27d6248ca447 100644
> --- a/crypto/af_alg.c
> +++ b/crypto/af_alg.c
> @@ -260,6 +260,14 @@ static int alg_setsockopt(struct socket *sock, int
> level, int optname, if (!type->setauthsize)
>  			goto unlock;
>  		err = type->setauthsize(ask->private, optlen);
> +		break;
> +	case ALG_SET_DRBG_ENTROPY:
> +		if (sock->state == SS_CONNECTED)
> +			goto unlock;
> +		if (!type->setentropy)
> +			goto unlock;
> +
> +		err = type->setentropy(ask->private, optval, optlen);
>  	}
> 
>  unlock:
> diff --git a/crypto/algif_rng.c b/crypto/algif_rng.c
> index 087c0ad09d38..8a3b0eb45a85 100644
> --- a/crypto/algif_rng.c
> +++ b/crypto/algif_rng.c
> @@ -38,6 +38,7 @@
>   * DAMAGE.
>   */
> 
> +#include <linux/capability.h>
>  #include <linux/module.h>
>  #include <crypto/rng.h>
>  #include <linux/random.h>
> @@ -53,8 +54,22 @@ struct rng_ctx {
>  #define MAXSIZE 128
>  	unsigned int len;
>  	struct crypto_rng *drng;
> +	u8 *addtl;
> +	size_t addtl_len;
>  };
> 
> +struct rng_parent_ctx {
> +	struct crypto_rng *drng;
> +	u8 *entropy;
> +};
> +
> +static void rng_reset_addtl(struct rng_ctx *ctx)
> +{
> +	kzfree(ctx->addtl);
> +	ctx->addtl = NULL;
> +	ctx->addtl_len = 0;
> +}
> +
>  static int rng_recvmsg(struct socket *sock, struct msghdr *msg, size_t len,
> int flags)
>  {
> @@ -65,6 +80,7 @@ static int rng_recvmsg(struct socket *sock, struct msghdr
> *msg, size_t len, int genlen = 0;
>  	u8 result[MAXSIZE];
> 
> +	lock_sock(sock->sk);
>  	if (len == 0)
>  		return 0;
>  	if (len > MAXSIZE)
> @@ -82,16 +98,45 @@ static int rng_recvmsg(struct socket *sock, struct
> msghdr *msg, size_t len, * seeding as they automatically seed. The X9.31
> DRNG will return * an error if it was not seeded properly.
>  	 */
> -	genlen = crypto_rng_get_bytes(ctx->drng, result, len);
> +	genlen = crypto_rng_generate(ctx->drng, ctx->addtl, ctx->addtl_len,
> +				     result, len);
>  	if (genlen < 0)
>  		return genlen;
> 
>  	err = memcpy_to_msg(msg, result, len);
>  	memzero_explicit(result, len);
> +	rng_reset_addtl(ctx);
> +	release_sock(sock->sk);
> 
>  	return err ? err : len;
>  }
> 
> +static int rng_sendmsg(struct socket *sock, struct msghdr *msg, size_t len)
> +{
> +	int err;
> +	struct alg_sock *ask = alg_sk(sock->sk);
> +	struct rng_ctx *ctx = ask->private;
> +
> +	lock_sock(sock->sk);
> +	if (len > MAXSIZE)
> +		len = MAXSIZE;
> +
> +	rng_reset_addtl(ctx);
> +	ctx->addtl = kmalloc(len, GFP_KERNEL);
> +	if (!ctx->addtl)
> +		return -ENOMEM;
> +
> +	err = memcpy_from_msg(ctx->addtl, msg, len);
> +	if (err) {
> +		rng_reset_addtl(ctx);
> +		return err;
> +	}
> +	ctx->addtl_len = len;
> +	release_sock(sock->sk);
> +
> +	return len;
> +}
> +
>  static struct proto_ops algif_rng_ops = {
>  	.family		=	PF_ALG,
> 
> @@ -106,21 +151,41 @@ static struct proto_ops algif_rng_ops = {
>  	.bind		=	sock_no_bind,
>  	.accept		=	sock_no_accept,
>  	.setsockopt	=	sock_no_setsockopt,
> -	.sendmsg	=	sock_no_sendmsg,
>  	.sendpage	=	sock_no_sendpage,
> 
>  	.release	=	af_alg_release,
>  	.recvmsg	=	rng_recvmsg,
> +	.sendmsg	=	rng_sendmsg,
>  };
> 
>  static void *rng_bind(const char *name, u32 type, u32 mask)
>  {
> -	return crypto_alloc_rng(name, type, mask);
> +	struct rng_parent_ctx *pctx;
> +	struct crypto_rng *rng;
> +
> +	pctx = kzalloc(sizeof(*pctx), GFP_KERNEL);
> +	if (!pctx)
> +		return ERR_PTR(-ENOMEM);
> +
> +	rng = crypto_alloc_rng(name, type, mask);
> +	if (IS_ERR(rng)) {
> +		kfree(pctx);
> +		return ERR_CAST(rng);
> +	}
> +
> +	pctx->drng = rng;
> +	return pctx;
>  }
> 
>  static void rng_release(void *private)
>  {
> -	crypto_free_rng(private);
> +	struct rng_parent_ctx *pctx = private;
> +
> +	if (unlikely(!pctx))
> +		return;
> +	crypto_free_rng(pctx->drng);
> +	kzfree(pctx->entropy);
> +	kzfree(pctx);
>  }
> 
>  static void rng_sock_destruct(struct sock *sk)
> @@ -128,6 +193,7 @@ static void rng_sock_destruct(struct sock *sk)
>  	struct alg_sock *ask = alg_sk(sk);
>  	struct rng_ctx *ctx = ask->private;
> 
> +	rng_reset_addtl(ctx);
>  	sock_kfree_s(sk, ctx, ctx->len);
>  	af_alg_release_parent(sk);
>  }
> @@ -135,6 +201,7 @@ static void rng_sock_destruct(struct sock *sk)
>  static int rng_accept_parent(void *private, struct sock *sk)
>  {
>  	struct rng_ctx *ctx;
> +	struct rng_parent_ctx *pctx = private;
>  	struct alg_sock *ask = alg_sk(sk);
>  	unsigned int len = sizeof(*ctx);
> 
> @@ -150,7 +217,9 @@ static int rng_accept_parent(void *private, struct sock
> *sk) * state of the RNG.
>  	 */
> 
> -	ctx->drng = private;
> +	ctx->drng = pctx->drng;
> +	ctx->addtl = NULL;
> +	ctx->addtl_len = 0;
>  	ask->private = ctx;
>  	sk->sk_destruct = rng_sock_destruct;
> 
> @@ -159,18 +228,49 @@ static int rng_accept_parent(void *private, struct
> sock *sk)
> 
>  static int rng_setkey(void *private, const u8 *seed, unsigned int seedlen)
>  {
> +	struct rng_parent_ctx *pctx = private;
>  	/*
>  	 * Check whether seedlen is of sufficient size is done in RNG
>  	 * implementations.
>  	 */
> -	return crypto_rng_reset(private, seed, seedlen);
> +	return crypto_rng_reset(pctx->drng, seed, seedlen);
> +}
> +
> +#ifdef CONFIG_CRYPTO_CAVS_DRBG
> +static int rng_setentropy(void *private, const u8 *entropy, unsigned int
> len) +{
> +	struct rng_parent_ctx *pctx = private;
> +	u8 *kentropy = NULL;
> +
> +	if (!capable(CAP_SYS_ADMIN))
> +		return -EPERM;
> +
> +	if (pctx->entropy)
> +		return -EINVAL;
> +
> +	if (len > MAXSIZE)
> +		len = MAXSIZE;
> +
> +	if (len) {
> +		kentropy = memdup_user(entropy, len);
> +		if (IS_ERR(kentropy))
> +			return PTR_ERR(kentropy);
> +	}
> +
> +	crypto_rng_alg(pctx->drng)->set_ent(pctx->drng, kentropy, len);
> +	pctx->entropy = kentropy;

Why do you need to keep kentropy around? For the check above whether entropy 
was set, wouldn't a boolean suffice?

> +	return len;
>  }
> +#endif
> 
>  static const struct af_alg_type algif_type_rng = {
>  	.bind		=	rng_bind,
>  	.release	=	rng_release,
>  	.accept		=	rng_accept_parent,
>  	.setkey		=	rng_setkey,
> +#ifdef CONFIG_CRYPTO_CAVS_DRBG
> +	.setentropy	=	rng_setentropy,
> +#endif
>  	.ops		=	&algif_rng_ops,
>  	.name		=	"rng",
>  	.owner		=	THIS_MODULE
> diff --git a/include/crypto/if_alg.h b/include/crypto/if_alg.h
> index 56527c85d122..312fdb3469cf 100644
> --- a/include/crypto/if_alg.h
> +++ b/include/crypto/if_alg.h
> @@ -46,6 +46,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 (*setentropy)(void *private, const u8 *entropy, unsigned int len);
>  	int (*accept)(void *private, struct sock *sk);
>  	int (*accept_nokey)(void *private, struct sock *sk);
>  	int (*setauthsize)(void *private, unsigned int authsize);
> @@ -123,7 +124,7 @@ struct af_alg_async_req {
>   * @tsgl_list:		Link to TX SGL
>   * @iv:			IV for cipher operation
>   * @aead_assoclen:	Length of AAD for AEAD cipher operations
> - * @completion:		Work queue for synchronous operation
> + * @wait:		Wait on completion of async crypto ops
>   * @used:		TX bytes sent to kernel. This variable is used to
>   *			ensure that user space cannot cause the kernel
>   *			to allocate too much memory in sendmsg operation.
> diff --git a/include/uapi/linux/if_alg.h b/include/uapi/linux/if_alg.h
> index bc2bcdec377b..60b7c2efd921 100644
> --- a/include/uapi/linux/if_alg.h
> +++ b/include/uapi/linux/if_alg.h
> @@ -35,6 +35,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_DRBG_ENTROPY		6
> 
>  /* Operations */
>  #define ALG_OP_DECRYPT			0


Ciao
Stephan

Re: [PATCH v2] crypto: af_alg - add extra parameters for DRBG interface

[Elena Petrova]

Hi Stephan,

On Mon, 20 Jul 2020 at 18:35, Stephan Mueller <smueller@chronox.de> wrote:
>
> Am Donnerstag, 16. Juli 2020, 18:40:28 CEST schrieb Elena Petrova:
>
> Hi Elena,
>
> sorry for the delay in answering.
>
> > Extending the userspace RNG interface:
> >   1. adding ALG_SET_DRBG_ENTROPY setsockopt option for entropy input;
> >   2. using sendmsg syscall for specifying the additional data.
> >
> > Signed-off-by: Elena Petrova <lenaptr@google.com>
> > ---
> >
> > libkcapi patch for testing:
> >
> > https://github.com/Len0k/libkcapi/commit/6f095d270b982008f419078614c15caa59
> > 2cb531
>
> If that kernel patch is taken, I would be happy to take the libkcapi patch
> with the following suggested changes:
>
> - add full documentation of kcapi_rng_set_entropy and kcapi_rng_send_addtl to
> kcapi.h
>
> - move test_cavp into either test/kcapi-main.c or its own application inside
> test/ where the caller provides the entropy_input, personalization string,
> additional inputs

Ok, thanks!

> >
> > diff --git a/crypto/Kconfig b/crypto/Kconfig
> > index 091c0a0bbf26..8484617596d1 100644
> > --- a/crypto/Kconfig
> > +++ b/crypto/Kconfig
> > @@ -1896,6 +1896,14 @@ config CRYPTO_STATS
> >  config CRYPTO_HASH_INFO
> >       bool
> >
> > +config CRYPTO_CAVS_DRBG
>
> CAVS is dead, long live ACVT :-) So, maybe use CAVP or ACVT as abbreviations?

Ok, let's use CAVP then.

> As the config option applies to AF_ALG, wouldn't it be better to use an
> appropriate name here like CRYPTO_USER_API_CAVP_DRBG or similar?
>
> Note, if indeed we would add akcipher or even KPP with CAVP support, maybe we
> need additional config options here. So, I would recommend to use this
> separate name space.

Yes, that makes sense, thanks.

> > +     tristate "Enable CAVS testing of DRBG"
>
> Dto: replace CAVS

Ack

> > +#ifdef CONFIG_CRYPTO_CAVS_DRBG
> > +static int rng_setentropy(void *private, const u8 *entropy, unsigned int
> > len) +{
> > +     struct rng_parent_ctx *pctx = private;
> > +     u8 *kentropy = NULL;
> > +
> > +     if (!capable(CAP_SYS_ADMIN))
> > +             return -EPERM;
> > +
> > +     if (pctx->entropy)
> > +             return -EINVAL;
> > +
> > +     if (len > MAXSIZE)
> > +             len = MAXSIZE;
> > +
> > +     if (len) {
> > +             kentropy = memdup_user(entropy, len);
> > +             if (IS_ERR(kentropy))
> > +                     return PTR_ERR(kentropy);
> > +     }
> > +
> > +     crypto_rng_alg(pctx->drng)->set_ent(pctx->drng, kentropy, len);
> > +     pctx->entropy = kentropy;
>
> Why do you need to keep kentropy around? For the check above whether entropy
> was set, wouldn't a boolean suffice?

I need to keep the pointer to free it after use. Unlike the setting of
the key, DRBG saves the entropy pointer in one of its internal
structures, but doesn't do any memory
management. I had only two ideas on how to prevent memory leaks:
either change drbg code to deal with the memory, or save the pointer
somewhere inside the socket. I opted for the latter. But if you know a
better approach I'm happy to rework my code accordingly.

> > diff --git a/include/crypto/if_alg.h b/include/crypto/if_alg.h
> > index 56527c85d122..312fdb3469cf 100644
> > --- a/include/crypto/if_alg.h
> > +++ b/include/crypto/if_alg.h
> > @@ -46,6 +46,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 (*setentropy)(void *private, const u8 *entropy, unsigned int len);
> >       int (*accept)(void *private, struct sock *sk);
> >       int (*accept_nokey)(void *private, struct sock *sk);
> >       int (*setauthsize)(void *private, unsigned int authsize);
> > @@ -123,7 +124,7 @@ struct af_alg_async_req {
> >   * @tsgl_list:               Link to TX SGL
> >   * @iv:                      IV for cipher operation
> >   * @aead_assoclen:   Length of AAD for AEAD cipher operations
> > - * @completion:              Work queue for synchronous operation
> > + * @wait:            Wait on completion of async crypto ops
>
> What is this change about? I am not sure it relates to the changes above.

I noticed that the documentation for the function differs from the
function declaration, so I thought I'd fix that, since I touched the
header. But yeah, it doesn't relate to the changes.

> Ciao
> Stephan

Thanks,
Elena

Re: [PATCH v2] crypto: af_alg - add extra parameters for DRBG interface

[Stephan Mueller]

Am Dienstag, 21. Juli 2020, 14:55:14 CEST schrieb Elena Petrova:

Hi Elena,

> > > +#ifdef CONFIG_CRYPTO_CAVS_DRBG
> > > +static int rng_setentropy(void *private, const u8 *entropy, unsigned
> > > int
> > > len) +{
> > > +     struct rng_parent_ctx *pctx = private;
> > > +     u8 *kentropy = NULL;
> > > +
> > > +     if (!capable(CAP_SYS_ADMIN))
> > > +             return -EPERM;
> > > +
> > > +     if (pctx->entropy)
> > > +             return -EINVAL;
> > > +
> > > +     if (len > MAXSIZE)
> > > +             len = MAXSIZE;
> > > +
> > > +     if (len) {
> > > +             kentropy = memdup_user(entropy, len);
> > > +             if (IS_ERR(kentropy))
> > > +                     return PTR_ERR(kentropy);
> > > +     }
> > > +
> > > +     crypto_rng_alg(pctx->drng)->set_ent(pctx->drng, kentropy, len);
> > > +     pctx->entropy = kentropy;
> > 
> > Why do you need to keep kentropy around? For the check above whether
> > entropy was set, wouldn't a boolean suffice?
> 
> I need to keep the pointer to free it after use. Unlike the setting of
> the key, DRBG saves the entropy pointer in one of its internal
> structures, but doesn't do any memory
> management. I had only two ideas on how to prevent memory leaks:
> either change drbg code to deal with the memory, or save the pointer
> somewhere inside the socket. I opted for the latter. But if you know a
> better approach I'm happy to rework my code accordingly.

I was thinking of calling crypto_rng_alg(pctx->drng)->seed() directly after 
set_ent. This call performs a DRBG instantatiate where the entropy buffer is 
used. See crypto_drbg_reset_test for the approach.

But maybe you are right, the test "entropy" buffer inside the DRBG currently 
cannot be reset. So, for sanity purposes, you need to keep it around.

Ciao
Stephan

Re: [PATCH v2] crypto: af_alg - add extra parameters for DRBG interface

[Elena Petrova]

Hi Stephan,

On Tue, 21 Jul 2020 at 14:19, Stephan Mueller <smueller@chronox.de> wrote:
>
> Am Dienstag, 21. Juli 2020, 14:55:14 CEST schrieb Elena Petrova:
>
> Hi Elena,
>
> > > > +#ifdef CONFIG_CRYPTO_CAVS_DRBG
> > > > +static int rng_setentropy(void *private, const u8 *entropy, unsigned
> > > > int
> > > > len) +{
> > > > +     struct rng_parent_ctx *pctx = private;
> > > > +     u8 *kentropy = NULL;
> > > > +
> > > > +     if (!capable(CAP_SYS_ADMIN))
> > > > +             return -EPERM;
> > > > +
> > > > +     if (pctx->entropy)
> > > > +             return -EINVAL;
> > > > +
> > > > +     if (len > MAXSIZE)
> > > > +             len = MAXSIZE;
> > > > +
> > > > +     if (len) {
> > > > +             kentropy = memdup_user(entropy, len);
> > > > +             if (IS_ERR(kentropy))
> > > > +                     return PTR_ERR(kentropy);
> > > > +     }
> > > > +
> > > > +     crypto_rng_alg(pctx->drng)->set_ent(pctx->drng, kentropy, len);
> > > > +     pctx->entropy = kentropy;
> > >
> > > Why do you need to keep kentropy around? For the check above whether
> > > entropy was set, wouldn't a boolean suffice?
> >
> > I need to keep the pointer to free it after use. Unlike the setting of
> > the key, DRBG saves the entropy pointer in one of its internal
> > structures, but doesn't do any memory
> > management. I had only two ideas on how to prevent memory leaks:
> > either change drbg code to deal with the memory, or save the pointer
> > somewhere inside the socket. I opted for the latter. But if you know a
> > better approach I'm happy to rework my code accordingly.
>
> I was thinking of calling crypto_rng_alg(pctx->drng)->seed() directly after
> set_ent. This call performs a DRBG instantatiate where the entropy buffer is
> used. See crypto_drbg_reset_test for the approach.
>
> But maybe you are right, the test "entropy" buffer inside the DRBG currently
> cannot be reset. So, for sanity purposes, you need to keep it around.

I looked into this, and afaik `->seed()` needs the seed buffer (a.k.a.
key); and seed() is also invoked on ALG_SET_KEY setsockopt. So we
would need both entropy and seed values at the same time. To avoid
complicating the matters, I decided to leave the code as is. I added a
comment in v3 [https://lore.kernel.org/linux-crypto/20200728155159.2156480-1-lenaptr@google.com/]
explaining why the `kentropy` pointer is saved.

> Ciao
> Stephan

Regards,
Elena

Re: [PATCH v2] crypto: af_alg - add extra parameters for DRBG interface

[Stephan Müller]

Am Donnerstag, 16. Juli 2020, 18:40:28 CEST schrieb Elena Petrova:

Hi Elena,

> diff --git a/include/crypto/if_alg.h b/include/crypto/if_alg.h
> index 56527c85d122..312fdb3469cf 100644
> --- a/include/crypto/if_alg.h
> +++ b/include/crypto/if_alg.h
> @@ -46,6 +46,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 (*setentropy)(void *private, const u8 *entropy, unsigned int len);
>  	int (*accept)(void *private, struct sock *sk);
>  	int (*accept_nokey)(void *private, struct sock *sk);
>  	int (*setauthsize)(void *private, unsigned int authsize);
> @@ -123,7 +124,7 @@ struct af_alg_async_req {
>   * @tsgl_list:		Link to TX SGL
>   * @iv:			IV for cipher operation
>   * @aead_assoclen:	Length of AAD for AEAD cipher operations
> - * @completion:		Work queue for synchronous operation
> + * @wait:		Wait on completion of async crypto ops

What is this change about? I am not sure it relates to the changes above.

>   * @used:		TX bytes sent to kernel. This variable is used to
>   *			ensure that user space cannot cause the kernel
>   *			to allocate too much memory in sendmsg operation.
> diff --git a/include/uapi/linux/if_alg.h b/include/uapi/linux/if_alg.h


Ciao
Stephan

Re: [PATCH v2] crypto: af_alg - add extra parameters for DRBG interface

[Eric Biggers]

On Thu, Jul 16, 2020 at 05:40:28PM +0100, Elena Petrova wrote:
> Extending the userspace RNG interface:
>   1. adding ALG_SET_DRBG_ENTROPY setsockopt option for entropy input;
>   2. using sendmsg syscall for specifying the additional data.
> 
> Signed-off-by: Elena Petrova <lenaptr@google.com>

Can you add more details to the commit message?  E.g. why this is needed.

Also please use imperative tense, e.g. "Extend the userspace RNG interface".

>  static int rng_recvmsg(struct socket *sock, struct msghdr *msg, size_t len,
>  		       int flags)
>  {
> @@ -65,6 +80,7 @@ static int rng_recvmsg(struct socket *sock, struct msghdr *msg, size_t len,
>  	int genlen = 0;
>  	u8 result[MAXSIZE];
>  
> +	lock_sock(sock->sk);
>  	if (len == 0)
>  		return 0;

This returns without unlocking the socket.

>  	if (len > MAXSIZE)
> @@ -82,16 +98,45 @@ static int rng_recvmsg(struct socket *sock, struct msghdr *msg, size_t len,
>  	 * seeding as they automatically seed. The X9.31 DRNG will return
>  	 * an error if it was not seeded properly.
>  	 */
> -	genlen = crypto_rng_get_bytes(ctx->drng, result, len);
> +	genlen = crypto_rng_generate(ctx->drng, ctx->addtl, ctx->addtl_len,
> +				     result, len);
>  	if (genlen < 0)
>  		return genlen;

Likewise.

>  
>  	err = memcpy_to_msg(msg, result, len);
>  	memzero_explicit(result, len);
> +	rng_reset_addtl(ctx);
> +	release_sock(sock->sk);
>  
>  	return err ? err : len;
>  }
>  
> +static int rng_sendmsg(struct socket *sock, struct msghdr *msg, size_t len)
> +{
> +	int err;
> +	struct alg_sock *ask = alg_sk(sock->sk);
> +	struct rng_ctx *ctx = ask->private;
> +
> +	lock_sock(sock->sk);
> +	if (len > MAXSIZE)
> +		len = MAXSIZE;
> +
> +	rng_reset_addtl(ctx);
> +	ctx->addtl = kmalloc(len, GFP_KERNEL);
> +	if (!ctx->addtl)
> +		return -ENOMEM;

Likewise.

> +
> +	err = memcpy_from_msg(ctx->addtl, msg, len);
> +	if (err) {
> +		rng_reset_addtl(ctx);
> +		return err;

Likewise.

[PATCH v3] crypto: af_alg - add extra parameters for DRBG interface

[Elena Petrova]

Extend the user-space RNG interface:
  1. Add entropy input via ALG_SET_DRBG_ENTROPY setsockopt option;
  2. Add additional data input via sendmsg syscall.

This allows DRBG to be tested with test vectors, for example for the
purpose of CAVP testing, which otherwise isn't possible.

To prevent erroneous use of entropy input, it is hidden under
CRYPTO_USER_API_CAVP_DRBG config option and requires CAP_SYS_ADMIN to
succeed.

Signed-off-by: Elena Petrova <lenaptr@google.com>
---

libkcapi patch for testing:
  https://github.com/Len0k/libkcapi/commit/6f095d270b982008f419078614c15caa592cb531

Updates in v3:
  1) More details in commit message;
  2) config option name is now CRYPTO_USER_API_CAVP_DRBG;
  3) fixed a bug of not releasing socket locks.

Updates in v2:
  1) Adding CONFIG_CRYPTO_CAVS_DRBG around setentropy.
  2) Requiring CAP_SYS_ADMIN for entropy reset.
  3) Locking for send and recv.
  4) Length checks added for send and setentropy; send and setentropy now return
     number of bytes accepted.
  5) Minor code style corrections.

 Documentation/crypto/userspace-if.rst |  17 +++-
 crypto/Kconfig                        |   8 ++
 crypto/af_alg.c                       |   8 ++
 crypto/algif_rng.c                    | 130 ++++++++++++++++++++++++--
 include/crypto/if_alg.h               |   1 +
 include/uapi/linux/if_alg.h           |   1 +
 6 files changed, 152 insertions(+), 13 deletions(-)

diff --git a/Documentation/crypto/userspace-if.rst b/Documentation/crypto/userspace-if.rst
index ff86befa61e0..ef7132802c2d 100644
--- a/Documentation/crypto/userspace-if.rst
+++ b/Documentation/crypto/userspace-if.rst
@@ -296,15 +296,23 @@ follows:
 
     struct sockaddr_alg sa = {
         .salg_family = AF_ALG,
-        .salg_type = "rng", /* this selects the symmetric cipher */
-        .salg_name = "drbg_nopr_sha256" /* this is the cipher name */
+        .salg_type = "rng", /* this selects the random number generator */
+        .salg_name = "drbg_nopr_sha256" /* this is the RNG name */
     };
 
 
 Depending on the RNG type, the RNG must be seeded. The seed is provided
 using the setsockopt interface to set the key. For example, the
 ansi_cprng requires a seed. The DRBGs do not require a seed, but may be
-seeded.
+seeded. The seed is also known as a *Personalization String* in DRBG800-90A
+standard.
+
+For the purpose of CAVP testing, the concatenation of *Entropy* and *Nonce*
+can be provided to the RNG via ALG_SET_DRBG_ENTROPY setsockopt interface. This
+requires a kernel built with CONFIG_CRYPTO_USER_API_CAVP_DRBG, and
+CAP_SYS_ADMIN permission.
+
+*Additional Data* can be provided using the send()/sendmsg() system calls.
 
 Using the read()/recvmsg() system calls, random numbers can be obtained.
 The kernel generates at most 128 bytes in one call. If user space
@@ -377,6 +385,9 @@ mentioned optname:
    provided ciphertext is assumed to contain an authentication tag of
    the given size (see section about AEAD memory layout below).
 
+-  ALG_SET_DRBG_ENTROPY -- Setting the entropy of the random number generator.
+   This option is applicable to RNG cipher type only.
+
 User space API example
 ----------------------
 
diff --git a/crypto/Kconfig b/crypto/Kconfig
index 091c0a0bbf26..aa2b3085a431 100644
--- a/crypto/Kconfig
+++ b/crypto/Kconfig
@@ -1896,6 +1896,14 @@ config CRYPTO_STATS
 config CRYPTO_HASH_INFO
 	bool
 
+config CRYPTO_USER_API_CAVP_DRBG
+	tristate "Enable CAVP testing of DRBG"
+	depends on CRYPTO_USER_API_RNG && CRYPTO_DRBG
+	help
+	  This option enables the resetting of DRBG entropy via the user-space
+	  interface. This should only be enabled for CAVP testing. You should say
+	  no unless you know what this is.
+
 source "lib/crypto/Kconfig"
 source "drivers/crypto/Kconfig"
 source "crypto/asymmetric_keys/Kconfig"
diff --git a/crypto/af_alg.c b/crypto/af_alg.c
index b1cd3535c525..27d6248ca447 100644
--- a/crypto/af_alg.c
+++ b/crypto/af_alg.c
@@ -260,6 +260,14 @@ static int alg_setsockopt(struct socket *sock, int level, int optname,
 		if (!type->setauthsize)
 			goto unlock;
 		err = type->setauthsize(ask->private, optlen);
+		break;
+	case ALG_SET_DRBG_ENTROPY:
+		if (sock->state == SS_CONNECTED)
+			goto unlock;
+		if (!type->setentropy)
+			goto unlock;
+
+		err = type->setentropy(ask->private, optval, optlen);
 	}
 
 unlock:
diff --git a/crypto/algif_rng.c b/crypto/algif_rng.c
index 087c0ad09d38..3a88f4fca2a9 100644
--- a/crypto/algif_rng.c
+++ b/crypto/algif_rng.c
@@ -38,6 +38,7 @@
  * DAMAGE.
  */
 
+#include <linux/capability.h>
 #include <linux/module.h>
 #include <crypto/rng.h>
 #include <linux/random.h>
@@ -53,20 +54,35 @@ struct rng_ctx {
 #define MAXSIZE 128
 	unsigned int len;
 	struct crypto_rng *drng;
+	u8 *addtl;
+	size_t addtl_len;
 };
 
+struct rng_parent_ctx {
+	struct crypto_rng *drng;
+	u8 *entropy;
+};
+
+static void rng_reset_addtl(struct rng_ctx *ctx)
+{
+	kzfree(ctx->addtl);
+	ctx->addtl = NULL;
+	ctx->addtl_len = 0;
+}
+
 static int rng_recvmsg(struct socket *sock, struct msghdr *msg, size_t len,
 		       int flags)
 {
 	struct sock *sk = sock->sk;
 	struct alg_sock *ask = alg_sk(sk);
 	struct rng_ctx *ctx = ask->private;
-	int err;
+	int err = 0;
 	int genlen = 0;
 	u8 result[MAXSIZE];
 
+	lock_sock(sock->sk);
 	if (len == 0)
-		return 0;
+		goto unlock;
 	if (len > MAXSIZE)
 		len = MAXSIZE;
 
@@ -82,16 +98,51 @@ static int rng_recvmsg(struct socket *sock, struct msghdr *msg, size_t len,
 	 * seeding as they automatically seed. The X9.31 DRNG will return
 	 * an error if it was not seeded properly.
 	 */
-	genlen = crypto_rng_get_bytes(ctx->drng, result, len);
-	if (genlen < 0)
-		return genlen;
+	genlen = crypto_rng_generate(ctx->drng, ctx->addtl, ctx->addtl_len,
+				     result, len);
+	if (genlen < 0) {
+		err = genlen;
+		goto unlock;
+	}
 
 	err = memcpy_to_msg(msg, result, len);
 	memzero_explicit(result, len);
+	rng_reset_addtl(ctx);
 
+unlock:
+	release_sock(sock->sk);
 	return err ? err : len;
 }
 
+static int rng_sendmsg(struct socket *sock, struct msghdr *msg, size_t len)
+{
+	int err;
+	struct alg_sock *ask = alg_sk(sock->sk);
+	struct rng_ctx *ctx = ask->private;
+
+	lock_sock(sock->sk);
+	if (len > MAXSIZE)
+		len = MAXSIZE;
+
+	rng_reset_addtl(ctx);
+	ctx->addtl = kmalloc(len, GFP_KERNEL);
+	if (!ctx->addtl) {
+		err = -ENOMEM;
+		goto unlock;
+	}
+
+	err = memcpy_from_msg(ctx->addtl, msg, len);
+	if (err) {
+		rng_reset_addtl(ctx);
+		goto unlock;
+	}
+	ctx->addtl_len = len;
+
+unlock:
+	release_sock(sock->sk);
+	return len;
+}
+
 static struct proto_ops algif_rng_ops = {
 	.family		=	PF_ALG,
 
@@ -106,21 +157,41 @@ static struct proto_ops algif_rng_ops = {
 	.bind		=	sock_no_bind,
 	.accept		=	sock_no_accept,
 	.setsockopt	=	sock_no_setsockopt,
-	.sendmsg	=	sock_no_sendmsg,
 	.sendpage	=	sock_no_sendpage,
 
 	.release	=	af_alg_release,
 	.recvmsg	=	rng_recvmsg,
+	.sendmsg	=	rng_sendmsg,
 };
 
 static void *rng_bind(const char *name, u32 type, u32 mask)
 {
-	return crypto_alloc_rng(name, type, mask);
+	struct rng_parent_ctx *pctx;
+	struct crypto_rng *rng;
+
+	pctx = kzalloc(sizeof(*pctx), GFP_KERNEL);
+	if (!pctx)
+		return ERR_PTR(-ENOMEM);
+
+	rng = crypto_alloc_rng(name, type, mask);
+	if (IS_ERR(rng)) {
+		kfree(pctx);
+		return ERR_CAST(rng);
+	}
+
+	pctx->drng = rng;
+	return pctx;
 }
 
 static void rng_release(void *private)
 {
-	crypto_free_rng(private);
+	struct rng_parent_ctx *pctx = private;
+
+	if (unlikely(!pctx))
+		return;
+	crypto_free_rng(pctx->drng);
+	kzfree(pctx->entropy);
+	kzfree(pctx);
 }
 
 static void rng_sock_destruct(struct sock *sk)
@@ -128,6 +199,7 @@ static void rng_sock_destruct(struct sock *sk)
 	struct alg_sock *ask = alg_sk(sk);
 	struct rng_ctx *ctx = ask->private;
 
+	rng_reset_addtl(ctx);
 	sock_kfree_s(sk, ctx, ctx->len);
 	af_alg_release_parent(sk);
 }
@@ -135,6 +207,7 @@ static void rng_sock_destruct(struct sock *sk)
 static int rng_accept_parent(void *private, struct sock *sk)
 {
 	struct rng_ctx *ctx;
+	struct rng_parent_ctx *pctx = private;
 	struct alg_sock *ask = alg_sk(sk);
 	unsigned int len = sizeof(*ctx);
 
@@ -150,7 +223,9 @@ static int rng_accept_parent(void *private, struct sock *sk)
 	 * state of the RNG.
 	 */
 
-	ctx->drng = private;
+	ctx->drng = pctx->drng;
+	ctx->addtl = NULL;
+	ctx->addtl_len = 0;
 	ask->private = ctx;
 	sk->sk_destruct = rng_sock_destruct;
 
@@ -159,18 +234,53 @@ static int rng_accept_parent(void *private, struct sock *sk)
 
 static int rng_setkey(void *private, const u8 *seed, unsigned int seedlen)
 {
+	struct rng_parent_ctx *pctx = private;
 	/*
 	 * Check whether seedlen is of sufficient size is done in RNG
 	 * implementations.
 	 */
-	return crypto_rng_reset(private, seed, seedlen);
+	return crypto_rng_reset(pctx->drng, seed, seedlen);
+}
+
+#ifdef CONFIG_CRYPTO_USER_API_CAVP_DRBG
+static int rng_setentropy(void *private, const u8 *entropy, unsigned int len)
+{
+	struct rng_parent_ctx *pctx = private;
+	u8 *kentropy = NULL;
+
+	if (!capable(CAP_SYS_ADMIN))
+		return -EPERM;
+
+	if (pctx->entropy)
+		return -EINVAL;
+
+	if (len > MAXSIZE)
+		len = MAXSIZE;
+
+	if (len) {
+		kentropy = memdup_user(entropy, len);
+		if (IS_ERR(kentropy))
+			return PTR_ERR(kentropy);
+	}
+
+	crypto_rng_alg(pctx->drng)->set_ent(pctx->drng, kentropy, len);
+	/*
+	 * Since rng doesn't perform any memory management for the entropy
+	 * buffer, save kentropy pointer to pctx now to free it after use.
+	 */
+	pctx->entropy = kentropy;
+	return len;
 }
+#endif
 
 static const struct af_alg_type algif_type_rng = {
 	.bind		=	rng_bind,
 	.release	=	rng_release,
 	.accept		=	rng_accept_parent,
 	.setkey		=	rng_setkey,
+#ifdef CONFIG_CRYPTO_USER_API_CAVP_DRBG
+	.setentropy	=	rng_setentropy,
+#endif
 	.ops		=	&algif_rng_ops,
 	.name		=	"rng",
 	.owner		=	THIS_MODULE
diff --git a/include/crypto/if_alg.h b/include/crypto/if_alg.h
index 56527c85d122..9e5c8ac53c59 100644
--- a/include/crypto/if_alg.h
+++ b/include/crypto/if_alg.h
@@ -46,6 +46,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 (*setentropy)(void *private, const u8 *entropy, unsigned int len);
 	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 bc2bcdec377b..60b7c2efd921 100644
--- a/include/uapi/linux/if_alg.h
+++ b/include/uapi/linux/if_alg.h
@@ -35,6 +35,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_DRBG_ENTROPY		6
 
 /* Operations */
 #define ALG_OP_DECRYPT			0
-- 
2.28.0.rc0.142.g3c755180ce-goog

Re: [PATCH v3] crypto: af_alg - add extra parameters for DRBG interface

[Eric Biggers]

On Tue, Jul 28, 2020 at 04:51:59PM +0100, Elena Petrova wrote:
> +static int rng_sendmsg(struct socket *sock, struct msghdr *msg, size_t len)
> +{
> +	int err;
> +	struct alg_sock *ask = alg_sk(sock->sk);
> +	struct rng_ctx *ctx = ask->private;
> +
> +	lock_sock(sock->sk);
> +	if (len > MAXSIZE)
> +		len = MAXSIZE;
> +
> +	rng_reset_addtl(ctx);
> +	ctx->addtl = kmalloc(len, GFP_KERNEL);
> +	if (!ctx->addtl) {
> +		err = -ENOMEM;
> +		goto unlock;

This error code isn't actually returned.

> +	}
> +
> +	err = memcpy_from_msg(ctx->addtl, msg, len);
> +	if (err) {
> +		rng_reset_addtl(ctx);
> +		goto unlock;

Likewise.

> +#ifdef CONFIG_CRYPTO_USER_API_CAVP_DRBG
> +static int rng_setentropy(void *private, const u8 *entropy, unsigned int len)
> +{
> +	struct rng_parent_ctx *pctx = private;
> +	u8 *kentropy = NULL;
> +
> +	if (!capable(CAP_SYS_ADMIN))
> +		return -EPERM;

This should be EACCES, not EPERM.  EACCES means the operation will succeed if
you acquire the needed privileges.  EPERM means it will never succeed.

> +	if (len > MAXSIZE)
> +		len = MAXSIZE;

Truncating the length is error prone.  Shouldn't this instead return an error
(EMSGSIZE?) if the length is too long, and 0 on success?  Remember this is
setsockopt(), not write().

- Eric

[PATCH v4] crypto: af_alg - add extra parameters for DRBG interface

[Elena Petrova]

Extend the user-space RNG interface:
  1. Add entropy input via ALG_SET_DRBG_ENTROPY setsockopt option;
  2. Add additional data input via sendmsg syscall.

This allows DRBG to be tested with test vectors, for example for the
purpose of CAVP testing, which otherwise isn't possible.

To prevent erroneous use of entropy input, it is hidden under
CRYPTO_USER_API_CAVP_DRBG config option and requires CAP_SYS_ADMIN to
succeed.

Signed-off-by: Elena Petrova <lenaptr@google.com>
---

Updates in v4:
  1) setentropy returns 0 or error code (used to return length);
  2) bigfixes suggested by Eric.

Updates in v3:
  1) More details in commit message;
  2) config option name is now CRYPTO_USER_API_CAVP_DRBG;
  3) fixed a bug of not releasing socket locks.

Updates in v2:
  1) Adding CONFIG_CRYPTO_CAVS_DRBG around setentropy.
  2) Requiring CAP_SYS_ADMIN for entropy reset.
  3) Locking for send and recv.
  4) Length checks added for send and setentropy; send and setentropy now return
     number of bytes accepted.
  5) Minor code style corrections.

libkcapi patch for testing:
  https://github.com/Len0k/libkcapi/commit/6f095d270b982008f419078614c15caa592cb531

 Documentation/crypto/userspace-if.rst |  17 +++-
 crypto/Kconfig                        |   8 ++
 crypto/af_alg.c                       |   8 ++
 crypto/algif_rng.c                    | 130 ++++++++++++++++++++++++--
 include/crypto/if_alg.h               |   1 +
 include/uapi/linux/if_alg.h           |   1 +
 6 files changed, 152 insertions(+), 13 deletions(-)

diff --git a/Documentation/crypto/userspace-if.rst b/Documentation/crypto/userspace-if.rst
index ff86befa61e0..ef7132802c2d 100644
--- a/Documentation/crypto/userspace-if.rst
+++ b/Documentation/crypto/userspace-if.rst
@@ -296,15 +296,23 @@ follows:
 
     struct sockaddr_alg sa = {
         .salg_family = AF_ALG,
-        .salg_type = "rng", /* this selects the symmetric cipher */
-        .salg_name = "drbg_nopr_sha256" /* this is the cipher name */
+        .salg_type = "rng", /* this selects the random number generator */
+        .salg_name = "drbg_nopr_sha256" /* this is the RNG name */
     };
 
 
 Depending on the RNG type, the RNG must be seeded. The seed is provided
 using the setsockopt interface to set the key. For example, the
 ansi_cprng requires a seed. The DRBGs do not require a seed, but may be
-seeded.
+seeded. The seed is also known as a *Personalization String* in DRBG800-90A
+standard.
+
+For the purpose of CAVP testing, the concatenation of *Entropy* and *Nonce*
+can be provided to the RNG via ALG_SET_DRBG_ENTROPY setsockopt interface. This
+requires a kernel built with CONFIG_CRYPTO_USER_API_CAVP_DRBG, and
+CAP_SYS_ADMIN permission.
+
+*Additional Data* can be provided using the send()/sendmsg() system calls.
 
 Using the read()/recvmsg() system calls, random numbers can be obtained.
 The kernel generates at most 128 bytes in one call. If user space
@@ -377,6 +385,9 @@ mentioned optname:
    provided ciphertext is assumed to contain an authentication tag of
    the given size (see section about AEAD memory layout below).
 
+-  ALG_SET_DRBG_ENTROPY -- Setting the entropy of the random number generator.
+   This option is applicable to RNG cipher type only.
+
 User space API example
 ----------------------
 
diff --git a/crypto/Kconfig b/crypto/Kconfig
index 091c0a0bbf26..aa2b3085a431 100644
--- a/crypto/Kconfig
+++ b/crypto/Kconfig
@@ -1896,6 +1896,14 @@ config CRYPTO_STATS
 config CRYPTO_HASH_INFO
 	bool
 
+config CRYPTO_USER_API_CAVP_DRBG
+	tristate "Enable CAVP testing of DRBG"
+	depends on CRYPTO_USER_API_RNG && CRYPTO_DRBG
+	help
+	  This option enables the resetting of DRBG entropy via the user-space
+	  interface. This should only be enabled for CAVP testing. You should say
+	  no unless you know what this is.
+
 source "lib/crypto/Kconfig"
 source "drivers/crypto/Kconfig"
 source "crypto/asymmetric_keys/Kconfig"
diff --git a/crypto/af_alg.c b/crypto/af_alg.c
index b1cd3535c525..27d6248ca447 100644
--- a/crypto/af_alg.c
+++ b/crypto/af_alg.c
@@ -260,6 +260,14 @@ static int alg_setsockopt(struct socket *sock, int level, int optname,
 		if (!type->setauthsize)
 			goto unlock;
 		err = type->setauthsize(ask->private, optlen);
+		break;
+	case ALG_SET_DRBG_ENTROPY:
+		if (sock->state == SS_CONNECTED)
+			goto unlock;
+		if (!type->setentropy)
+			goto unlock;
+
+		err = type->setentropy(ask->private, optval, optlen);
 	}
 
 unlock:
diff --git a/crypto/algif_rng.c b/crypto/algif_rng.c
index 087c0ad09d38..21e8201844bf 100644
--- a/crypto/algif_rng.c
+++ b/crypto/algif_rng.c
@@ -38,6 +38,7 @@
  * DAMAGE.
  */
 
+#include <linux/capability.h>
 #include <linux/module.h>
 #include <crypto/rng.h>
 #include <linux/random.h>
@@ -53,20 +54,35 @@ struct rng_ctx {
 #define MAXSIZE 128
 	unsigned int len;
 	struct crypto_rng *drng;
+	u8 *addtl;
+	size_t addtl_len;
 };
 
+struct rng_parent_ctx {
+	struct crypto_rng *drng;
+	u8 *entropy;
+};
+
+static void rng_reset_addtl(struct rng_ctx *ctx)
+{
+	kzfree(ctx->addtl);
+	ctx->addtl = NULL;
+	ctx->addtl_len = 0;
+}
+
 static int rng_recvmsg(struct socket *sock, struct msghdr *msg, size_t len,
 		       int flags)
 {
 	struct sock *sk = sock->sk;
 	struct alg_sock *ask = alg_sk(sk);
 	struct rng_ctx *ctx = ask->private;
-	int err;
+	int err = 0;
 	int genlen = 0;
 	u8 result[MAXSIZE];
 
+	lock_sock(sock->sk);
 	if (len == 0)
-		return 0;
+		goto unlock;
 	if (len > MAXSIZE)
 		len = MAXSIZE;
 
@@ -82,13 +98,48 @@ static int rng_recvmsg(struct socket *sock, struct msghdr *msg, size_t len,
 	 * seeding as they automatically seed. The X9.31 DRNG will return
 	 * an error if it was not seeded properly.
 	 */
-	genlen = crypto_rng_get_bytes(ctx->drng, result, len);
-	if (genlen < 0)
-		return genlen;
+	genlen = crypto_rng_generate(ctx->drng, ctx->addtl, ctx->addtl_len,
+				     result, len);
+	if (genlen < 0) {
+		err = genlen;
+		goto unlock;
+	}
 
 	err = memcpy_to_msg(msg, result, len);
 	memzero_explicit(result, len);
+	rng_reset_addtl(ctx);
 
+unlock:
+	release_sock(sock->sk);
+	return err ? err : len;
+}
+
+static int rng_sendmsg(struct socket *sock, struct msghdr *msg, size_t len)
+{
+	int err;
+	struct alg_sock *ask = alg_sk(sock->sk);
+	struct rng_ctx *ctx = ask->private;
+
+	lock_sock(sock->sk);
+	if (len > MAXSIZE)
+		len = MAXSIZE;
+
+	rng_reset_addtl(ctx);
+	ctx->addtl = kmalloc(len, GFP_KERNEL);
+	if (!ctx->addtl) {
+		err = -ENOMEM;
+		goto unlock;
+	}
+
+	err = memcpy_from_msg(ctx->addtl, msg, len);
+	if (err) {
+		rng_reset_addtl(ctx);
+		goto unlock;
+	}
+	ctx->addtl_len = len;
+
+unlock:
+	release_sock(sock->sk);
 	return err ? err : len;
 }
 
@@ -106,21 +157,41 @@ static struct proto_ops algif_rng_ops = {
 	.bind		=	sock_no_bind,
 	.accept		=	sock_no_accept,
 	.setsockopt	=	sock_no_setsockopt,
-	.sendmsg	=	sock_no_sendmsg,
 	.sendpage	=	sock_no_sendpage,
 
 	.release	=	af_alg_release,
 	.recvmsg	=	rng_recvmsg,
+	.sendmsg	=	rng_sendmsg,
 };
 
 static void *rng_bind(const char *name, u32 type, u32 mask)
 {
-	return crypto_alloc_rng(name, type, mask);
+	struct rng_parent_ctx *pctx;
+	struct crypto_rng *rng;
+
+	pctx = kzalloc(sizeof(*pctx), GFP_KERNEL);
+	if (!pctx)
+		return ERR_PTR(-ENOMEM);
+
+	rng = crypto_alloc_rng(name, type, mask);
+	if (IS_ERR(rng)) {
+		kfree(pctx);
+		return ERR_CAST(rng);
+	}
+
+	pctx->drng = rng;
+	return pctx;
 }
 
 static void rng_release(void *private)
 {
-	crypto_free_rng(private);
+	struct rng_parent_ctx *pctx = private;
+
+	if (unlikely(!pctx))
+		return;
+	crypto_free_rng(pctx->drng);
+	kzfree(pctx->entropy);
+	kzfree(pctx);
 }
 
 static void rng_sock_destruct(struct sock *sk)
@@ -128,6 +199,7 @@ static void rng_sock_destruct(struct sock *sk)
 	struct alg_sock *ask = alg_sk(sk);
 	struct rng_ctx *ctx = ask->private;
 
+	rng_reset_addtl(ctx);
 	sock_kfree_s(sk, ctx, ctx->len);
 	af_alg_release_parent(sk);
 }
@@ -135,6 +207,7 @@ static void rng_sock_destruct(struct sock *sk)
 static int rng_accept_parent(void *private, struct sock *sk)
 {
 	struct rng_ctx *ctx;
+	struct rng_parent_ctx *pctx = private;
 	struct alg_sock *ask = alg_sk(sk);
 	unsigned int len = sizeof(*ctx);
 
@@ -150,7 +223,9 @@ static int rng_accept_parent(void *private, struct sock *sk)
 	 * state of the RNG.
 	 */
 
-	ctx->drng = private;
+	ctx->drng = pctx->drng;
+	ctx->addtl = NULL;
+	ctx->addtl_len = 0;
 	ask->private = ctx;
 	sk->sk_destruct = rng_sock_destruct;
 
@@ -159,18 +234,53 @@ static int rng_accept_parent(void *private, struct sock *sk)
 
 static int rng_setkey(void *private, const u8 *seed, unsigned int seedlen)
 {
+	struct rng_parent_ctx *pctx = private;
 	/*
 	 * Check whether seedlen is of sufficient size is done in RNG
 	 * implementations.
 	 */
-	return crypto_rng_reset(private, seed, seedlen);
+	return crypto_rng_reset(pctx->drng, seed, seedlen);
+}
+
+#ifdef CONFIG_CRYPTO_USER_API_CAVP_DRBG
+static int rng_setentropy(void *private, const u8 *entropy, unsigned int len)
+{
+	struct rng_parent_ctx *pctx = private;
+	u8 *kentropy = NULL;
+
+	if (!capable(CAP_SYS_ADMIN))
+		return -EACCES;
+
+	if (pctx->entropy)
+		return -EINVAL;
+
+	if (len > MAXSIZE)
+		return -EMSGSIZE;
+
+	if (len) {
+		kentropy = memdup_user(entropy, len);
+		if (IS_ERR(kentropy))
+			return PTR_ERR(kentropy);
+	}
+
+	crypto_rng_alg(pctx->drng)->set_ent(pctx->drng, kentropy, len);
+	/*
+	 * Since rng doesn't perform any memory management for the entropy
+	 * buffer, save kentropy pointer to pctx now to free it after use.
+	 */
+	pctx->entropy = kentropy;
+	return 0;
 }
+#endif
 
 static const struct af_alg_type algif_type_rng = {
 	.bind		=	rng_bind,
 	.release	=	rng_release,
 	.accept		=	rng_accept_parent,
 	.setkey		=	rng_setkey,
+#ifdef CONFIG_CRYPTO_USER_API_CAVP_DRBG
+	.setentropy	=	rng_setentropy,
+#endif
 	.ops		=	&algif_rng_ops,
 	.name		=	"rng",
 	.owner		=	THIS_MODULE
diff --git a/include/crypto/if_alg.h b/include/crypto/if_alg.h
index 56527c85d122..9e5c8ac53c59 100644
--- a/include/crypto/if_alg.h
+++ b/include/crypto/if_alg.h
@@ -46,6 +46,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 (*setentropy)(void *private, const u8 *entropy, unsigned int len);
 	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 bc2bcdec377b..60b7c2efd921 100644
--- a/include/uapi/linux/if_alg.h
+++ b/include/uapi/linux/if_alg.h
@@ -35,6 +35,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_DRBG_ENTROPY		6
 
 /* Operations */
 #define ALG_OP_DECRYPT			0
-- 
2.28.0.rc0.142.g3c755180ce-goog

Re: [PATCH v4] crypto: af_alg - add extra parameters for DRBG interface

[Stephan Müller]

Am Mittwoch, 29. Juli 2020, 17:45:01 CEST schrieb Elena Petrova:

Hi Elena,

> Extend the user-space RNG interface:
>   1. Add entropy input via ALG_SET_DRBG_ENTROPY setsockopt option;
>   2. Add additional data input via sendmsg syscall.
> 
> This allows DRBG to be tested with test vectors, for example for the
> purpose of CAVP testing, which otherwise isn't possible.
> 
> To prevent erroneous use of entropy input, it is hidden under
> CRYPTO_USER_API_CAVP_DRBG config option and requires CAP_SYS_ADMIN to
> succeed.
> 
> Signed-off-by: Elena Petrova <lenaptr@google.com>

Acked-by: Stephan Müller <smueller@chronox.de>

Ciao
Stephan

Re: [PATCH v4] crypto: af_alg - add extra parameters for DRBG interface

[Herbert Xu]

Elena Petrova <lenaptr@google.com> wrote:
>
> +#ifdef CONFIG_CRYPTO_USER_API_CAVP_DRBG
> +static int rng_setentropy(void *private, const u8 *entropy, unsigned int len)

Please use __maybe_unused instead of the ifdef.

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

Re: [PATCH v4] crypto: af_alg - add extra parameters for DRBG interface

[Elena Petrova]

Hi Herbert,

> > +#ifdef CONFIG_CRYPTO_USER_API_CAVP_DRBG
> > +static int rng_setentropy(void *private, const u8 *entropy, unsigned int len)
>
> Please use __maybe_unused instead of the ifdef.

Ack

On Fri, 31 Jul 2020 at 08:27, Herbert Xu <herbert@gondor.apana.org.au> wrote:
>
> Eric Biggers <ebiggers@kernel.org> wrote:
> >
> > lock_sock() would solve the former.  I'm not sure what should be done about
> > rng_recvmsg().  It apparently relies on the crypto_rng doing its own locking,
> > but maybe it should just use lock_sock() too.
>
> The lock_sock is only needed if you're doing testing.  What I'd
> prefer is to have a completely different code-path for testing.

sendmsg is used for "Additional Data" input, and unlike entropy, it
could be useful outside of testing. But if you confirm it's not
useful, then yes, I can decouple the testing parts.

> How about you fork the code in rng_accept_parent so that you have
> a separate proto_ops for the test path that is used only if
> setentropy has been called? That way all of this code could
> magically go away if the CONFIG option wasn't set.

Depends on the comment above, but otherwise, my only concern is that
the testing variant of rng_recvmsg would be largely copy-pasted from
the normal rng_recvmsg, apart from a few lines of lock/release and
crypto_rng_generate/crypto_rng_get_bytes.


Thanks!
Elena

Re: [PATCH v4] crypto: af_alg - add extra parameters for DRBG interface

[Stephan Mueller]

Am Montag, 3. August 2020, 16:48:02 CEST schrieb Elena Petrova:

Hi Elena,

> On Fri, 31 Jul 2020 at 08:27, Herbert Xu <herbert@gondor.apana.org.au> 
wrote:
> > Eric Biggers <ebiggers@kernel.org> wrote:
> > > lock_sock() would solve the former.  I'm not sure what should be done
> > > about
> > > rng_recvmsg().  It apparently relies on the crypto_rng doing its own
> > > locking, but maybe it should just use lock_sock() too.
> > 
> > The lock_sock is only needed if you're doing testing.  What I'd
> > prefer is to have a completely different code-path for testing.
> 
> sendmsg is used for "Additional Data" input, and unlike entropy, it
> could be useful outside of testing. But if you confirm it's not
> useful, then yes, I can decouple the testing parts.

Nobody has requested it for now - so why not only compiling it when the DRBG 
test config value is set? If for some reason there is a request to allow 
setting the additional data from user space, we may simply take the ifdef 
away.

My approach is to have only interfaces into the kernel that are truly 
requested and needed.

Ciao
Stephan

Re: [PATCH v4] crypto: af_alg - add extra parameters for DRBG interface

[Elena Petrova]

Hi Stephan,

On Mon, 3 Aug 2020 at 16:10, Stephan Mueller <smueller@chronox.de> wrote:
>
> Am Montag, 3. August 2020, 16:48:02 CEST schrieb Elena Petrova:
>
> Hi Elena,
>
> > On Fri, 31 Jul 2020 at 08:27, Herbert Xu <herbert@gondor.apana.org.au>
> wrote:
> > > Eric Biggers <ebiggers@kernel.org> wrote:
> > > > lock_sock() would solve the former.  I'm not sure what should be done
> > > > about
> > > > rng_recvmsg().  It apparently relies on the crypto_rng doing its own
> > > > locking, but maybe it should just use lock_sock() too.
> > >
> > > The lock_sock is only needed if you're doing testing.  What I'd
> > > prefer is to have a completely different code-path for testing.
> >
> > sendmsg is used for "Additional Data" input, and unlike entropy, it
> > could be useful outside of testing. But if you confirm it's not
> > useful, then yes, I can decouple the testing parts.
>
> Nobody has requested it for now - so why not only compiling it when the DRBG
> test config value is set? If for some reason there is a request to allow
> setting the additional data from user space, we may simply take the ifdef
> away.
>
> My approach is to have only interfaces into the kernel that are truly
> requested and needed.

Ok, makes sense, thanks!

> Ciao
> Stephan
>
>

Re: [PATCH v4] crypto: af_alg - add extra parameters for DRBG interface

[Herbert Xu]

On Mon, Aug 03, 2020 at 03:48:02PM +0100, Elena Petrova wrote:
>
> sendmsg is used for "Additional Data" input, and unlike entropy, it
> could be useful outside of testing. But if you confirm it's not
> useful, then yes, I can decouple the testing parts.

Unless there is someone asking for it then I'd rather not export
it to user-space.

> Depends on the comment above, but otherwise, my only concern is that
> the testing variant of rng_recvmsg would be largely copy-pasted from
> the normal rng_recvmsg, apart from a few lines of lock/release and
> crypto_rng_generate/crypto_rng_get_bytes.

They could certainly share code through the use of functions.

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

Re: [PATCH 1/1] crypto: af_alg - add extra parameters for DRBG interface

[Eric Biggers]

On Mon, Jul 13, 2020 at 05:48:57PM +0100, Elena Petrova wrote:
> +static int rng_sendmsg(struct socket *sock, struct msghdr *msg, size_t len)
> +{
> +	int err;
> +	struct alg_sock *ask = alg_sk(sock->sk);
> +	struct rng_ctx *ctx = ask->private;
> +
> +	reset_addtl(ctx);
> +	ctx->addtl = kzalloc(len, GFP_KERNEL);
> +	if (!ctx->addtl)
> +		return -ENOMEM;
> +
> +	err = memcpy_from_msg(ctx->addtl, msg, len);
> +	if (err) {
> +		reset_addtl(ctx);
> +		return err;
> +	}
> +	ctx->addtl_len = len;
> +
> +	return 0;
> +}

This is also missing any sort of locking, both between concurrent calls to
rng_sendmsg(), and between rng_sendmsg() and rng_recvmsg().

lock_sock() would solve the former.  I'm not sure what should be done about
rng_recvmsg().  It apparently relies on the crypto_rng doing its own locking,
but maybe it should just use lock_sock() too.

- Eric

Re: [PATCH 1/1] crypto: af_alg - add extra parameters for DRBG interface

[Herbert Xu]

Eric Biggers <ebiggers@kernel.org> wrote:
>
> lock_sock() would solve the former.  I'm not sure what should be done about
> rng_recvmsg().  It apparently relies on the crypto_rng doing its own locking,
> but maybe it should just use lock_sock() too.

The lock_sock is only needed if you're doing testing.  What I'd
prefer is to have a completely different code-path for testing.

How about you fork the code in rng_accept_parent so that you have
a separate proto_ops for the test path that is used only if
setentropy has been called? That way all of this code could
magically go away if the CONFIG option wasn't set.

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

Re: [PATCH 1/1] crypto: af_alg - add extra parameters for DRBG interface

[Elena Petrova]

Hi Herbert,

On Fri, 31 Jul 2020 at 08:27, Herbert Xu <herbert@gondor.apana.org.au> wrote:
>
> How about you fork the code in rng_accept_parent so that you have
> a separate proto_ops for the test path that is used only if
> setentropy has been called? That way all of this code could
> magically go away if the CONFIG option wasn't set.

I tried doing `ask->type->ops = &algif_rng_test_ops;` in
accept_parent, to which the compiler complained "error: assignment of
member ‘ops’ in read-only object". So I'm setting `.ops` accordingly
at compile-time instead, together with `.setentropy`, in v5.

[PATCH v4] crypto: af_alg - add extra parameters for DRBG interface

[Elena Petrova]

Extend the user-space RNG interface:
  1. Add entropy input via ALG_SET_DRBG_ENTROPY setsockopt option;
  2. Add additional data input via sendmsg syscall.

This allows DRBG to be tested with test vectors, for example for the
purpose of CAVP testing, which otherwise isn't possible.

To prevent erroneous use of entropy input, it is hidden under
CRYPTO_USER_API_CAVP_DRBG config option and requires CAP_SYS_ADMIN to
succeed.

Signed-off-by: Elena Petrova <lenaptr@google.com>
---

Updates in v4:
  1) setentropy returns 0 or error code (used to return length);
  2) bigfixes suggested by Eric.

Updates in v3:
  1) More details in commit message;
  2) config option name is now CRYPTO_USER_API_CAVP_DRBG;
  3) fixed a bug of not releasing socket locks.

Updates in v2:
  1) Adding CONFIG_CRYPTO_CAVS_DRBG around setentropy.
  2) Requiring CAP_SYS_ADMIN for entropy reset.
  3) Locking for send and recv.
  4) Length checks added for send and setentropy; send and setentropy now return
     number of bytes accepted.
  5) Minor code style corrections.

libkcapi patch for testing:
  https://github.com/Len0k/libkcapi/commit/6f095d270b982008f419078614c15caa592cb531

 Documentation/crypto/userspace-if.rst |  17 +++-
 crypto/Kconfig                        |   8 ++
 crypto/af_alg.c                       |   8 ++
 crypto/algif_rng.c                    | 130 ++++++++++++++++++++++++--
 include/crypto/if_alg.h               |   1 +
 include/uapi/linux/if_alg.h           |   1 +
 6 files changed, 152 insertions(+), 13 deletions(-)

diff --git a/Documentation/crypto/userspace-if.rst b/Documentation/crypto/userspace-if.rst
index ff86befa61e0..ef7132802c2d 100644
--- a/Documentation/crypto/userspace-if.rst
+++ b/Documentation/crypto/userspace-if.rst
@@ -296,15 +296,23 @@ follows:
 
     struct sockaddr_alg sa = {
         .salg_family = AF_ALG,
-        .salg_type = "rng", /* this selects the symmetric cipher */
-        .salg_name = "drbg_nopr_sha256" /* this is the cipher name */
+        .salg_type = "rng", /* this selects the random number generator */
+        .salg_name = "drbg_nopr_sha256" /* this is the RNG name */
     };
 
 
 Depending on the RNG type, the RNG must be seeded. The seed is provided
 using the setsockopt interface to set the key. For example, the
 ansi_cprng requires a seed. The DRBGs do not require a seed, but may be
-seeded.
+seeded. The seed is also known as a *Personalization String* in DRBG800-90A
+standard.
+
+For the purpose of CAVP testing, the concatenation of *Entropy* and *Nonce*
+can be provided to the RNG via ALG_SET_DRBG_ENTROPY setsockopt interface. This
+requires a kernel built with CONFIG_CRYPTO_USER_API_CAVP_DRBG, and
+CAP_SYS_ADMIN permission.
+
+*Additional Data* can be provided using the send()/sendmsg() system calls.
 
 Using the read()/recvmsg() system calls, random numbers can be obtained.
 The kernel generates at most 128 bytes in one call. If user space
@@ -377,6 +385,9 @@ mentioned optname:
    provided ciphertext is assumed to contain an authentication tag of
    the given size (see section about AEAD memory layout below).
 
+-  ALG_SET_DRBG_ENTROPY -- Setting the entropy of the random number generator.
+   This option is applicable to RNG cipher type only.
+
 User space API example
 ----------------------
 
diff --git a/crypto/Kconfig b/crypto/Kconfig
index 091c0a0bbf26..aa2b3085a431 100644
--- a/crypto/Kconfig
+++ b/crypto/Kconfig
@@ -1896,6 +1896,14 @@ config CRYPTO_STATS
 config CRYPTO_HASH_INFO
 	bool
 
+config CRYPTO_USER_API_CAVP_DRBG
+	tristate "Enable CAVP testing of DRBG"
+	depends on CRYPTO_USER_API_RNG && CRYPTO_DRBG
+	help
+	  This option enables the resetting of DRBG entropy via the user-space
+	  interface. This should only be enabled for CAVP testing. You should say
+	  no unless you know what this is.
+
 source "lib/crypto/Kconfig"
 source "drivers/crypto/Kconfig"
 source "crypto/asymmetric_keys/Kconfig"
diff --git a/crypto/af_alg.c b/crypto/af_alg.c
index b1cd3535c525..27d6248ca447 100644
--- a/crypto/af_alg.c
+++ b/crypto/af_alg.c
@@ -260,6 +260,14 @@ static int alg_setsockopt(struct socket *sock, int level, int optname,
 		if (!type->setauthsize)
 			goto unlock;
 		err = type->setauthsize(ask->private, optlen);
+		break;
+	case ALG_SET_DRBG_ENTROPY:
+		if (sock->state == SS_CONNECTED)
+			goto unlock;
+		if (!type->setentropy)
+			goto unlock;
+
+		err = type->setentropy(ask->private, optval, optlen);
 	}
 
 unlock:
diff --git a/crypto/algif_rng.c b/crypto/algif_rng.c
index 087c0ad09d38..21e8201844bf 100644
--- a/crypto/algif_rng.c
+++ b/crypto/algif_rng.c
@@ -38,6 +38,7 @@
  * DAMAGE.
  */
 
+#include <linux/capability.h>
 #include <linux/module.h>
 #include <crypto/rng.h>
 #include <linux/random.h>
@@ -53,20 +54,35 @@ struct rng_ctx {
 #define MAXSIZE 128
 	unsigned int len;
 	struct crypto_rng *drng;
+	u8 *addtl;
+	size_t addtl_len;
 };
 
+struct rng_parent_ctx {
+	struct crypto_rng *drng;
+	u8 *entropy;
+};
+
+static void rng_reset_addtl(struct rng_ctx *ctx)
+{
+	kzfree(ctx->addtl);
+	ctx->addtl = NULL;
+	ctx->addtl_len = 0;
+}
+
 static int rng_recvmsg(struct socket *sock, struct msghdr *msg, size_t len,
 		       int flags)
 {
 	struct sock *sk = sock->sk;
 	struct alg_sock *ask = alg_sk(sk);
 	struct rng_ctx *ctx = ask->private;
-	int err;
+	int err = 0;
 	int genlen = 0;
 	u8 result[MAXSIZE];
 
+	lock_sock(sock->sk);
 	if (len == 0)
-		return 0;
+		goto unlock;
 	if (len > MAXSIZE)
 		len = MAXSIZE;
 
@@ -82,13 +98,48 @@ static int rng_recvmsg(struct socket *sock, struct msghdr *msg, size_t len,
 	 * seeding as they automatically seed. The X9.31 DRNG will return
 	 * an error if it was not seeded properly.
 	 */
-	genlen = crypto_rng_get_bytes(ctx->drng, result, len);
-	if (genlen < 0)
-		return genlen;
+	genlen = crypto_rng_generate(ctx->drng, ctx->addtl, ctx->addtl_len,
+				     result, len);
+	if (genlen < 0) {
+		err = genlen;
+		goto unlock;
+	}
 
 	err = memcpy_to_msg(msg, result, len);
 	memzero_explicit(result, len);
+	rng_reset_addtl(ctx);
 
+unlock:
+	release_sock(sock->sk);
+	return err ? err : len;
+}
+
+static int rng_sendmsg(struct socket *sock, struct msghdr *msg, size_t len)
+{
+	int err;
+	struct alg_sock *ask = alg_sk(sock->sk);
+	struct rng_ctx *ctx = ask->private;
+
+	lock_sock(sock->sk);
+	if (len > MAXSIZE)
+		len = MAXSIZE;
+
+	rng_reset_addtl(ctx);
+	ctx->addtl = kmalloc(len, GFP_KERNEL);
+	if (!ctx->addtl) {
+		err = -ENOMEM;
+		goto unlock;
+	}
+
+	err = memcpy_from_msg(ctx->addtl, msg, len);
+	if (err) {
+		rng_reset_addtl(ctx);
+		goto unlock;
+	}
+	ctx->addtl_len = len;
+
+unlock:
+	release_sock(sock->sk);
 	return err ? err : len;
 }
 
@@ -106,21 +157,41 @@ static struct proto_ops algif_rng_ops = {
 	.bind		=	sock_no_bind,
 	.accept		=	sock_no_accept,
 	.setsockopt	=	sock_no_setsockopt,
-	.sendmsg	=	sock_no_sendmsg,
 	.sendpage	=	sock_no_sendpage,
 
 	.release	=	af_alg_release,
 	.recvmsg	=	rng_recvmsg,
+	.sendmsg	=	rng_sendmsg,
 };
 
 static void *rng_bind(const char *name, u32 type, u32 mask)
 {
-	return crypto_alloc_rng(name, type, mask);
+	struct rng_parent_ctx *pctx;
+	struct crypto_rng *rng;
+
+	pctx = kzalloc(sizeof(*pctx), GFP_KERNEL);
+	if (!pctx)
+		return ERR_PTR(-ENOMEM);
+
+	rng = crypto_alloc_rng(name, type, mask);
+	if (IS_ERR(rng)) {
+		kfree(pctx);
+		return ERR_CAST(rng);
+	}
+
+	pctx->drng = rng;
+	return pctx;
 }
 
 static void rng_release(void *private)
 {
-	crypto_free_rng(private);
+	struct rng_parent_ctx *pctx = private;
+
+	if (unlikely(!pctx))
+		return;
+	crypto_free_rng(pctx->drng);
+	kzfree(pctx->entropy);
+	kzfree(pctx);
 }
 
 static void rng_sock_destruct(struct sock *sk)
@@ -128,6 +199,7 @@ static void rng_sock_destruct(struct sock *sk)
 	struct alg_sock *ask = alg_sk(sk);
 	struct rng_ctx *ctx = ask->private;
 
+	rng_reset_addtl(ctx);
 	sock_kfree_s(sk, ctx, ctx->len);
 	af_alg_release_parent(sk);
 }
@@ -135,6 +207,7 @@ static void rng_sock_destruct(struct sock *sk)
 static int rng_accept_parent(void *private, struct sock *sk)
 {
 	struct rng_ctx *ctx;
+	struct rng_parent_ctx *pctx = private;
 	struct alg_sock *ask = alg_sk(sk);
 	unsigned int len = sizeof(*ctx);
 
@@ -150,7 +223,9 @@ static int rng_accept_parent(void *private, struct sock *sk)
 	 * state of the RNG.
 	 */
 
-	ctx->drng = private;
+	ctx->drng = pctx->drng;
+	ctx->addtl = NULL;
+	ctx->addtl_len = 0;
 	ask->private = ctx;
 	sk->sk_destruct = rng_sock_destruct;
 
@@ -159,18 +234,53 @@ static int rng_accept_parent(void *private, struct sock *sk)
 
 static int rng_setkey(void *private, const u8 *seed, unsigned int seedlen)
 {
+	struct rng_parent_ctx *pctx = private;
 	/*
 	 * Check whether seedlen is of sufficient size is done in RNG
 	 * implementations.
 	 */
-	return crypto_rng_reset(private, seed, seedlen);
+	return crypto_rng_reset(pctx->drng, seed, seedlen);
+}
+
+#ifdef CONFIG_CRYPTO_USER_API_CAVP_DRBG
+static int rng_setentropy(void *private, const u8 *entropy, unsigned int len)
+{
+	struct rng_parent_ctx *pctx = private;
+	u8 *kentropy = NULL;
+
+	if (!capable(CAP_SYS_ADMIN))
+		return -EACCES;
+
+	if (pctx->entropy)
+		return -EINVAL;
+
+	if (len > MAXSIZE)
+		return -EMSGSIZE;
+
+	if (len) {
+		kentropy = memdup_user(entropy, len);
+		if (IS_ERR(kentropy))
+			return PTR_ERR(kentropy);
+	}
+
+	crypto_rng_alg(pctx->drng)->set_ent(pctx->drng, kentropy, len);
+	/*
+	 * Since rng doesn't perform any memory management for the entropy
+	 * buffer, save kentropy pointer to pctx now to free it after use.
+	 */
+	pctx->entropy = kentropy;
+	return 0;
 }
+#endif
 
 static const struct af_alg_type algif_type_rng = {
 	.bind		=	rng_bind,
 	.release	=	rng_release,
 	.accept		=	rng_accept_parent,
 	.setkey		=	rng_setkey,
+#ifdef CONFIG_CRYPTO_USER_API_CAVP_DRBG
+	.setentropy	=	rng_setentropy,
+#endif
 	.ops		=	&algif_rng_ops,
 	.name		=	"rng",
 	.owner		=	THIS_MODULE
diff --git a/include/crypto/if_alg.h b/include/crypto/if_alg.h
index 56527c85d122..9e5c8ac53c59 100644
--- a/include/crypto/if_alg.h
+++ b/include/crypto/if_alg.h
@@ -46,6 +46,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 (*setentropy)(void *private, const u8 *entropy, unsigned int len);
 	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 bc2bcdec377b..60b7c2efd921 100644
--- a/include/uapi/linux/if_alg.h
+++ b/include/uapi/linux/if_alg.h
@@ -35,6 +35,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_DRBG_ENTROPY		6
 
 /* Operations */
 #define ALG_OP_DECRYPT			0
-- 
2.28.0.rc0.142.g3c755180ce-goog

Re: [PATCH v4] crypto: af_alg - add extra parameters for DRBG interface

[Elena Petrova]

Err, I sent the old patch file, sorry. Real v5 coming shortly...

On Thu, 13 Aug 2020 at 17:02, Elena Petrova <lenaptr@google.com> wrote:
>
> Extend the user-space RNG interface:
>   1. Add entropy input via ALG_SET_DRBG_ENTROPY setsockopt option;
>   2. Add additional data input via sendmsg syscall.
>
> This allows DRBG to be tested with test vectors, for example for the
> purpose of CAVP testing, which otherwise isn't possible.
>
> To prevent erroneous use of entropy input, it is hidden under
> CRYPTO_USER_API_CAVP_DRBG config option and requires CAP_SYS_ADMIN to
> succeed.
>
> Signed-off-by: Elena Petrova <lenaptr@google.com>
> ---
>
> Updates in v4:
>   1) setentropy returns 0 or error code (used to return length);
>   2) bigfixes suggested by Eric.
>
> Updates in v3:
>   1) More details in commit message;
>   2) config option name is now CRYPTO_USER_API_CAVP_DRBG;
>   3) fixed a bug of not releasing socket locks.
>
> Updates in v2:
>   1) Adding CONFIG_CRYPTO_CAVS_DRBG around setentropy.
>   2) Requiring CAP_SYS_ADMIN for entropy reset.
>   3) Locking for send and recv.
>   4) Length checks added for send and setentropy; send and setentropy now return
>      number of bytes accepted.
>   5) Minor code style corrections.
>
> libkcapi patch for testing:
>   https://github.com/Len0k/libkcapi/commit/6f095d270b982008f419078614c15caa592cb531
>
>  Documentation/crypto/userspace-if.rst |  17 +++-
>  crypto/Kconfig                        |   8 ++
>  crypto/af_alg.c                       |   8 ++
>  crypto/algif_rng.c                    | 130 ++++++++++++++++++++++++--
>  include/crypto/if_alg.h               |   1 +
>  include/uapi/linux/if_alg.h           |   1 +
>  6 files changed, 152 insertions(+), 13 deletions(-)
>
> diff --git a/Documentation/crypto/userspace-if.rst b/Documentation/crypto/userspace-if.rst
> index ff86befa61e0..ef7132802c2d 100644
> --- a/Documentation/crypto/userspace-if.rst
> +++ b/Documentation/crypto/userspace-if.rst
> @@ -296,15 +296,23 @@ follows:
>
>      struct sockaddr_alg sa = {
>          .salg_family = AF_ALG,
> -        .salg_type = "rng", /* this selects the symmetric cipher */
> -        .salg_name = "drbg_nopr_sha256" /* this is the cipher name */
> +        .salg_type = "rng", /* this selects the random number generator */
> +        .salg_name = "drbg_nopr_sha256" /* this is the RNG name */
>      };
>
>
>  Depending on the RNG type, the RNG must be seeded. The seed is provided
>  using the setsockopt interface to set the key. For example, the
>  ansi_cprng requires a seed. The DRBGs do not require a seed, but may be
> -seeded.
> +seeded. The seed is also known as a *Personalization String* in DRBG800-90A
> +standard.
> +
> +For the purpose of CAVP testing, the concatenation of *Entropy* and *Nonce*
> +can be provided to the RNG via ALG_SET_DRBG_ENTROPY setsockopt interface. This
> +requires a kernel built with CONFIG_CRYPTO_USER_API_CAVP_DRBG, and
> +CAP_SYS_ADMIN permission.
> +
> +*Additional Data* can be provided using the send()/sendmsg() system calls.
>
>  Using the read()/recvmsg() system calls, random numbers can be obtained.
>  The kernel generates at most 128 bytes in one call. If user space
> @@ -377,6 +385,9 @@ mentioned optname:
>     provided ciphertext is assumed to contain an authentication tag of
>     the given size (see section about AEAD memory layout below).
>
> +-  ALG_SET_DRBG_ENTROPY -- Setting the entropy of the random number generator.
> +   This option is applicable to RNG cipher type only.
> +
>  User space API example
>  ----------------------
>
> diff --git a/crypto/Kconfig b/crypto/Kconfig
> index 091c0a0bbf26..aa2b3085a431 100644
> --- a/crypto/Kconfig
> +++ b/crypto/Kconfig
> @@ -1896,6 +1896,14 @@ config CRYPTO_STATS
>  config CRYPTO_HASH_INFO
>         bool
>
> +config CRYPTO_USER_API_CAVP_DRBG
> +       tristate "Enable CAVP testing of DRBG"
> +       depends on CRYPTO_USER_API_RNG && CRYPTO_DRBG
> +       help
> +         This option enables the resetting of DRBG entropy via the user-space
> +         interface. This should only be enabled for CAVP testing. You should say
> +         no unless you know what this is.
> +
>  source "lib/crypto/Kconfig"
>  source "drivers/crypto/Kconfig"
>  source "crypto/asymmetric_keys/Kconfig"
> diff --git a/crypto/af_alg.c b/crypto/af_alg.c
> index b1cd3535c525..27d6248ca447 100644
> --- a/crypto/af_alg.c
> +++ b/crypto/af_alg.c
> @@ -260,6 +260,14 @@ static int alg_setsockopt(struct socket *sock, int level, int optname,
>                 if (!type->setauthsize)
>                         goto unlock;
>                 err = type->setauthsize(ask->private, optlen);
> +               break;
> +       case ALG_SET_DRBG_ENTROPY:
> +               if (sock->state == SS_CONNECTED)
> +                       goto unlock;
> +               if (!type->setentropy)
> +                       goto unlock;
> +
> +               err = type->setentropy(ask->private, optval, optlen);
>         }
>
>  unlock:
> diff --git a/crypto/algif_rng.c b/crypto/algif_rng.c
> index 087c0ad09d38..21e8201844bf 100644
> --- a/crypto/algif_rng.c
> +++ b/crypto/algif_rng.c
> @@ -38,6 +38,7 @@
>   * DAMAGE.
>   */
>
> +#include <linux/capability.h>
>  #include <linux/module.h>
>  #include <crypto/rng.h>
>  #include <linux/random.h>
> @@ -53,20 +54,35 @@ struct rng_ctx {
>  #define MAXSIZE 128
>         unsigned int len;
>         struct crypto_rng *drng;
> +       u8 *addtl;
> +       size_t addtl_len;
>  };
>
> +struct rng_parent_ctx {
> +       struct crypto_rng *drng;
> +       u8 *entropy;
> +};
> +
> +static void rng_reset_addtl(struct rng_ctx *ctx)
> +{
> +       kzfree(ctx->addtl);
> +       ctx->addtl = NULL;
> +       ctx->addtl_len = 0;
> +}
> +
>  static int rng_recvmsg(struct socket *sock, struct msghdr *msg, size_t len,
>                        int flags)
>  {
>         struct sock *sk = sock->sk;
>         struct alg_sock *ask = alg_sk(sk);
>         struct rng_ctx *ctx = ask->private;
> -       int err;
> +       int err = 0;
>         int genlen = 0;
>         u8 result[MAXSIZE];
>
> +       lock_sock(sock->sk);
>         if (len == 0)
> -               return 0;
> +               goto unlock;
>         if (len > MAXSIZE)
>                 len = MAXSIZE;
>
> @@ -82,13 +98,48 @@ static int rng_recvmsg(struct socket *sock, struct msghdr *msg, size_t len,
>          * seeding as they automatically seed. The X9.31 DRNG will return
>          * an error if it was not seeded properly.
>          */
> -       genlen = crypto_rng_get_bytes(ctx->drng, result, len);
> -       if (genlen < 0)
> -               return genlen;
> +       genlen = crypto_rng_generate(ctx->drng, ctx->addtl, ctx->addtl_len,
> +                                    result, len);
> +       if (genlen < 0) {
> +               err = genlen;
> +               goto unlock;
> +       }
>
>         err = memcpy_to_msg(msg, result, len);
>         memzero_explicit(result, len);
> +       rng_reset_addtl(ctx);
>
> +unlock:
> +       release_sock(sock->sk);
> +       return err ? err : len;
> +}
> +
> +static int rng_sendmsg(struct socket *sock, struct msghdr *msg, size_t len)
> +{
> +       int err;
> +       struct alg_sock *ask = alg_sk(sock->sk);
> +       struct rng_ctx *ctx = ask->private;
> +
> +       lock_sock(sock->sk);
> +       if (len > MAXSIZE)
> +               len = MAXSIZE;
> +
> +       rng_reset_addtl(ctx);
> +       ctx->addtl = kmalloc(len, GFP_KERNEL);
> +       if (!ctx->addtl) {
> +               err = -ENOMEM;
> +               goto unlock;
> +       }
> +
> +       err = memcpy_from_msg(ctx->addtl, msg, len);
> +       if (err) {
> +               rng_reset_addtl(ctx);
> +               goto unlock;
> +       }
> +       ctx->addtl_len = len;
> +
> +unlock:
> +       release_sock(sock->sk);
>         return err ? err : len;
>  }
>
> @@ -106,21 +157,41 @@ static struct proto_ops algif_rng_ops = {
>         .bind           =       sock_no_bind,
>         .accept         =       sock_no_accept,
>         .setsockopt     =       sock_no_setsockopt,
> -       .sendmsg        =       sock_no_sendmsg,
>         .sendpage       =       sock_no_sendpage,
>
>         .release        =       af_alg_release,
>         .recvmsg        =       rng_recvmsg,
> +       .sendmsg        =       rng_sendmsg,
>  };
>
>  static void *rng_bind(const char *name, u32 type, u32 mask)
>  {
> -       return crypto_alloc_rng(name, type, mask);
> +       struct rng_parent_ctx *pctx;
> +       struct crypto_rng *rng;
> +
> +       pctx = kzalloc(sizeof(*pctx), GFP_KERNEL);
> +       if (!pctx)
> +               return ERR_PTR(-ENOMEM);
> +
> +       rng = crypto_alloc_rng(name, type, mask);
> +       if (IS_ERR(rng)) {
> +               kfree(pctx);
> +               return ERR_CAST(rng);
> +       }
> +
> +       pctx->drng = rng;
> +       return pctx;
>  }
>
>  static void rng_release(void *private)
>  {
> -       crypto_free_rng(private);
> +       struct rng_parent_ctx *pctx = private;
> +
> +       if (unlikely(!pctx))
> +               return;
> +       crypto_free_rng(pctx->drng);
> +       kzfree(pctx->entropy);
> +       kzfree(pctx);
>  }
>
>  static void rng_sock_destruct(struct sock *sk)
> @@ -128,6 +199,7 @@ static void rng_sock_destruct(struct sock *sk)
>         struct alg_sock *ask = alg_sk(sk);
>         struct rng_ctx *ctx = ask->private;
>
> +       rng_reset_addtl(ctx);
>         sock_kfree_s(sk, ctx, ctx->len);
>         af_alg_release_parent(sk);
>  }
> @@ -135,6 +207,7 @@ static void rng_sock_destruct(struct sock *sk)
>  static int rng_accept_parent(void *private, struct sock *sk)
>  {
>         struct rng_ctx *ctx;
> +       struct rng_parent_ctx *pctx = private;
>         struct alg_sock *ask = alg_sk(sk);
>         unsigned int len = sizeof(*ctx);
>
> @@ -150,7 +223,9 @@ static int rng_accept_parent(void *private, struct sock *sk)
>          * state of the RNG.
>          */
>
> -       ctx->drng = private;
> +       ctx->drng = pctx->drng;
> +       ctx->addtl = NULL;
> +       ctx->addtl_len = 0;
>         ask->private = ctx;
>         sk->sk_destruct = rng_sock_destruct;
>
> @@ -159,18 +234,53 @@ static int rng_accept_parent(void *private, struct sock *sk)
>
>  static int rng_setkey(void *private, const u8 *seed, unsigned int seedlen)
>  {
> +       struct rng_parent_ctx *pctx = private;
>         /*
>          * Check whether seedlen is of sufficient size is done in RNG
>          * implementations.
>          */
> -       return crypto_rng_reset(private, seed, seedlen);
> +       return crypto_rng_reset(pctx->drng, seed, seedlen);
> +}
> +
> +#ifdef CONFIG_CRYPTO_USER_API_CAVP_DRBG
> +static int rng_setentropy(void *private, const u8 *entropy, unsigned int len)
> +{
> +       struct rng_parent_ctx *pctx = private;
> +       u8 *kentropy = NULL;
> +
> +       if (!capable(CAP_SYS_ADMIN))
> +               return -EACCES;
> +
> +       if (pctx->entropy)
> +               return -EINVAL;
> +
> +       if (len > MAXSIZE)
> +               return -EMSGSIZE;
> +
> +       if (len) {
> +               kentropy = memdup_user(entropy, len);
> +               if (IS_ERR(kentropy))
> +                       return PTR_ERR(kentropy);
> +       }
> +
> +       crypto_rng_alg(pctx->drng)->set_ent(pctx->drng, kentropy, len);
> +       /*
> +        * Since rng doesn't perform any memory management for the entropy
> +        * buffer, save kentropy pointer to pctx now to free it after use.
> +        */
> +       pctx->entropy = kentropy;
> +       return 0;
>  }
> +#endif
>
>  static const struct af_alg_type algif_type_rng = {
>         .bind           =       rng_bind,
>         .release        =       rng_release,
>         .accept         =       rng_accept_parent,
>         .setkey         =       rng_setkey,
> +#ifdef CONFIG_CRYPTO_USER_API_CAVP_DRBG
> +       .setentropy     =       rng_setentropy,
> +#endif
>         .ops            =       &algif_rng_ops,
>         .name           =       "rng",
>         .owner          =       THIS_MODULE
> diff --git a/include/crypto/if_alg.h b/include/crypto/if_alg.h
> index 56527c85d122..9e5c8ac53c59 100644
> --- a/include/crypto/if_alg.h
> +++ b/include/crypto/if_alg.h
> @@ -46,6 +46,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 (*setentropy)(void *private, const u8 *entropy, unsigned int len);
>         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 bc2bcdec377b..60b7c2efd921 100644
> --- a/include/uapi/linux/if_alg.h
> +++ b/include/uapi/linux/if_alg.h
> @@ -35,6 +35,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_DRBG_ENTROPY           6
>
>  /* Operations */
>  #define ALG_OP_DECRYPT                 0
> --
> 2.28.0.rc0.142.g3c755180ce-goog
>

[PATCH v5] crypto: af_alg - add extra parameters for DRBG interface

[Elena Petrova]

Extend the user-space RNG interface:
  1. Add entropy input via ALG_SET_DRBG_ENTROPY setsockopt option;
  2. Add additional data input via sendmsg syscall.

This allows DRBG to be tested with test vectors, for example for the
purpose of CAVP testing, which otherwise isn't possible.

To prevent erroneous use of entropy input, it is hidden under
CRYPTO_USER_API_CAVP_DRBG config option and requires CAP_SYS_ADMIN to
succeed.

Signed-off-by: Elena Petrova <lenaptr@google.com>
Acked-by: Stephan Müller <smueller@chronox.de>
---

Updates in v5:
  1) use __maybe_unused instead of #ifdef;
  2) separate code path for a testing mode;
  3) only allow Additional Data input in a testing mode.

Updates in v4:
  1) setentropy returns 0 or error code (used to return length);
  2) bigfixes suggested by Eric.

Updates in v3:
  1) More details in commit message;
  2) config option name is now CRYPTO_USER_API_CAVP_DRBG;
  3) fixed a bug of not releasing socket locks.

Updates in v2:
  1) Adding CONFIG_CRYPTO_CAVS_DRBG around setentropy.
  2) Requiring CAP_SYS_ADMIN for entropy reset.
  3) Locking for send and recv.
  4) Length checks added for send and setentropy; send and setentropy now return
     number of bytes accepted.
  5) Minor code style corrections.

libkcapi patch for testing:
  https://github.com/Len0k/libkcapi/commit/6f095d270b982008f419078614c15caa592cb531

 Documentation/crypto/userspace-if.rst |  17 ++-
 crypto/Kconfig                        |   9 ++
 crypto/af_alg.c                       |   8 ++
 crypto/algif_rng.c                    | 172 ++++++++++++++++++++++++--
 include/crypto/if_alg.h               |   1 +
 include/uapi/linux/if_alg.h           |   1 +
 6 files changed, 193 insertions(+), 15 deletions(-)

diff --git a/Documentation/crypto/userspace-if.rst b/Documentation/crypto/userspace-if.rst
index ff86befa61e0..ef7132802c2d 100644
--- a/Documentation/crypto/userspace-if.rst
+++ b/Documentation/crypto/userspace-if.rst
@@ -296,15 +296,23 @@ follows:
 
     struct sockaddr_alg sa = {
         .salg_family = AF_ALG,
-        .salg_type = "rng", /* this selects the symmetric cipher */
-        .salg_name = "drbg_nopr_sha256" /* this is the cipher name */
+        .salg_type = "rng", /* this selects the random number generator */
+        .salg_name = "drbg_nopr_sha256" /* this is the RNG name */
     };
 
 
 Depending on the RNG type, the RNG must be seeded. The seed is provided
 using the setsockopt interface to set the key. For example, the
 ansi_cprng requires a seed. The DRBGs do not require a seed, but may be
-seeded.
+seeded. The seed is also known as a *Personalization String* in DRBG800-90A
+standard.
+
+For the purpose of CAVP testing, the concatenation of *Entropy* and *Nonce*
+can be provided to the RNG via ALG_SET_DRBG_ENTROPY setsockopt interface. This
+requires a kernel built with CONFIG_CRYPTO_USER_API_CAVP_DRBG, and
+CAP_SYS_ADMIN permission.
+
+*Additional Data* can be provided using the send()/sendmsg() system calls.
 
 Using the read()/recvmsg() system calls, random numbers can be obtained.
 The kernel generates at most 128 bytes in one call. If user space
@@ -377,6 +385,9 @@ mentioned optname:
    provided ciphertext is assumed to contain an authentication tag of
    the given size (see section about AEAD memory layout below).
 
+-  ALG_SET_DRBG_ENTROPY -- Setting the entropy of the random number generator.
+   This option is applicable to RNG cipher type only.
+
 User space API example
 ----------------------
 
diff --git a/crypto/Kconfig b/crypto/Kconfig
index 091c0a0bbf26..7c8736f71681 100644
--- a/crypto/Kconfig
+++ b/crypto/Kconfig
@@ -1896,6 +1896,15 @@ config CRYPTO_STATS
 config CRYPTO_HASH_INFO
 	bool
 
+config CRYPTO_USER_API_CAVP_DRBG
+	tristate "Enable CAVP testing of DRBG"
+	depends on CRYPTO_USER_API_RNG && CRYPTO_DRBG
+	help
+	  This option enables extra API for CAVP testing via the user-space
+	  interface: resetting of DRBG entropy, and providing Additional Data.
+	  This should only be enabled for CAVP testing. You should say
+	  no unless you know what this is.
+
 source "lib/crypto/Kconfig"
 source "drivers/crypto/Kconfig"
 source "crypto/asymmetric_keys/Kconfig"
diff --git a/crypto/af_alg.c b/crypto/af_alg.c
index b1cd3535c525..27d6248ca447 100644
--- a/crypto/af_alg.c
+++ b/crypto/af_alg.c
@@ -260,6 +260,14 @@ static int alg_setsockopt(struct socket *sock, int level, int optname,
 		if (!type->setauthsize)
 			goto unlock;
 		err = type->setauthsize(ask->private, optlen);
+		break;
+	case ALG_SET_DRBG_ENTROPY:
+		if (sock->state == SS_CONNECTED)
+			goto unlock;
+		if (!type->setentropy)
+			goto unlock;
+
+		err = type->setentropy(ask->private, optval, optlen);
 	}
 
 unlock:
diff --git a/crypto/algif_rng.c b/crypto/algif_rng.c
index 087c0ad09d38..6ec78c444206 100644
--- a/crypto/algif_rng.c
+++ b/crypto/algif_rng.c
@@ -38,6 +38,7 @@
  * DAMAGE.
  */
 
+#include <linux/capability.h>
 #include <linux/module.h>
 #include <crypto/rng.h>
 #include <linux/random.h>
@@ -53,15 +54,26 @@ struct rng_ctx {
 #define MAXSIZE 128
 	unsigned int len;
 	struct crypto_rng *drng;
+	u8 *addtl;
+	size_t addtl_len;
 };
 
-static int rng_recvmsg(struct socket *sock, struct msghdr *msg, size_t len,
-		       int flags)
+struct rng_parent_ctx {
+	struct crypto_rng *drng;
+	u8 *entropy;
+};
+
+static void rng_reset_addtl(struct rng_ctx *ctx)
 {
-	struct sock *sk = sock->sk;
-	struct alg_sock *ask = alg_sk(sk);
-	struct rng_ctx *ctx = ask->private;
-	int err;
+	kzfree(ctx->addtl);
+	ctx->addtl = NULL;
+	ctx->addtl_len = 0;
+}
+
+static int _rng_recvmsg(struct crypto_rng *drng, struct msghdr *msg, size_t len,
+			u8 *addtl, size_t addtl_len)
+{
+	int err = 0;
 	int genlen = 0;
 	u8 result[MAXSIZE];
 
@@ -82,7 +94,7 @@ static int rng_recvmsg(struct socket *sock, struct msghdr *msg, size_t len,
 	 * seeding as they automatically seed. The X9.31 DRNG will return
 	 * an error if it was not seeded properly.
 	 */
-	genlen = crypto_rng_get_bytes(ctx->drng, result, len);
+	genlen = crypto_rng_generate(drng, addtl, addtl_len, result, len);
 	if (genlen < 0)
 		return genlen;
 
@@ -92,7 +104,62 @@ static int rng_recvmsg(struct socket *sock, struct msghdr *msg, size_t len,
 	return err ? err : len;
 }
 
-static struct proto_ops algif_rng_ops = {
+static int rng_recvmsg(struct socket *sock, struct msghdr *msg, size_t len,
+		       int flags)
+{
+	struct sock *sk = sock->sk;
+	struct alg_sock *ask = alg_sk(sk);
+	struct rng_ctx *ctx = ask->private;
+
+	return _rng_recvmsg(ctx->drng, msg, len, NULL, 0);
+}
+
+static int rng_test_recvmsg(struct socket *sock, struct msghdr *msg, size_t len,
+			    int flags)
+{
+	struct sock *sk = sock->sk;
+	struct alg_sock *ask = alg_sk(sk);
+	struct rng_ctx *ctx = ask->private;
+	int err;
+
+	lock_sock(sock->sk);
+	err = _rng_recvmsg(ctx->drng, msg, len, ctx->addtl, ctx->addtl_len);
+	rng_reset_addtl(ctx);
+	release_sock(sock->sk);
+
+	return err ? err : len;
+}
+
+static int rng_test_sendmsg(struct socket *sock, struct msghdr *msg, size_t len)
+{
+	int err;
+	struct alg_sock *ask = alg_sk(sock->sk);
+	struct rng_ctx *ctx = ask->private;
+
+	lock_sock(sock->sk);
+	if (len > MAXSIZE)
+		len = MAXSIZE;
+
+	rng_reset_addtl(ctx);
+	ctx->addtl = kmalloc(len, GFP_KERNEL);
+	if (!ctx->addtl) {
+		err = -ENOMEM;
+		goto unlock;
+	}
+
+	err = memcpy_from_msg(ctx->addtl, msg, len);
+	if (err) {
+		rng_reset_addtl(ctx);
+		goto unlock;
+	}
+	ctx->addtl_len = len;
+
+unlock:
+	release_sock(sock->sk);
+	return err ? err : len;
+}
+
+static struct proto_ops __maybe_unused algif_rng_ops = {
 	.family		=	PF_ALG,
 
 	.connect	=	sock_no_connect,
@@ -113,14 +180,55 @@ static struct proto_ops algif_rng_ops = {
 	.recvmsg	=	rng_recvmsg,
 };
 
+static struct proto_ops __maybe_unused algif_rng_test_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,
+	.sendpage	=	sock_no_sendpage,
+
+	.release	=	af_alg_release,
+	.recvmsg	=	rng_test_recvmsg,
+	.sendmsg	=	rng_test_sendmsg,
+};
+
 static void *rng_bind(const char *name, u32 type, u32 mask)
 {
-	return crypto_alloc_rng(name, type, mask);
+	struct rng_parent_ctx *pctx;
+	struct crypto_rng *rng;
+
+	pctx = kzalloc(sizeof(*pctx), GFP_KERNEL);
+	if (!pctx)
+		return ERR_PTR(-ENOMEM);
+
+	rng = crypto_alloc_rng(name, type, mask);
+	if (IS_ERR(rng)) {
+		kfree(pctx);
+		return ERR_CAST(rng);
+	}
+
+	pctx->drng = rng;
+	return pctx;
 }
 
 static void rng_release(void *private)
 {
-	crypto_free_rng(private);
+	struct rng_parent_ctx *pctx = private;
+
+	if (unlikely(!pctx))
+		return;
+	crypto_free_rng(pctx->drng);
+	kzfree(pctx->entropy);
+	kzfree(pctx);
 }
 
 static void rng_sock_destruct(struct sock *sk)
@@ -128,6 +236,7 @@ static void rng_sock_destruct(struct sock *sk)
 	struct alg_sock *ask = alg_sk(sk);
 	struct rng_ctx *ctx = ask->private;
 
+	rng_reset_addtl(ctx);
 	sock_kfree_s(sk, ctx, ctx->len);
 	af_alg_release_parent(sk);
 }
@@ -135,6 +244,7 @@ static void rng_sock_destruct(struct sock *sk)
 static int rng_accept_parent(void *private, struct sock *sk)
 {
 	struct rng_ctx *ctx;
+	struct rng_parent_ctx *pctx = private;
 	struct alg_sock *ask = alg_sk(sk);
 	unsigned int len = sizeof(*ctx);
 
@@ -143,6 +253,8 @@ static int rng_accept_parent(void *private, struct sock *sk)
 		return -ENOMEM;
 
 	ctx->len = len;
+	ctx->addtl = NULL;
+	ctx->addtl_len = 0;
 
 	/*
 	 * No seeding done at that point -- if multiple accepts are
@@ -150,7 +262,7 @@ static int rng_accept_parent(void *private, struct sock *sk)
 	 * state of the RNG.
 	 */
 
-	ctx->drng = private;
+	ctx->drng = pctx->drng;
 	ask->private = ctx;
 	sk->sk_destruct = rng_sock_destruct;
 
@@ -159,11 +271,42 @@ static int rng_accept_parent(void *private, struct sock *sk)
 
 static int rng_setkey(void *private, const u8 *seed, unsigned int seedlen)
 {
+	struct rng_parent_ctx *pctx = private;
 	/*
 	 * Check whether seedlen is of sufficient size is done in RNG
 	 * implementations.
 	 */
-	return crypto_rng_reset(private, seed, seedlen);
+	return crypto_rng_reset(pctx->drng, seed, seedlen);
+}
+
+static int __maybe_unused rng_setentropy(void *private, const u8 *entropy,
+					 unsigned int len)
+{
+	struct rng_parent_ctx *pctx = private;
+	u8 *kentropy = NULL;
+
+	if (!capable(CAP_SYS_ADMIN))
+		return -EACCES;
+
+	if (pctx->entropy)
+		return -EINVAL;
+
+	if (len > MAXSIZE)
+		return -EMSGSIZE;
+
+	if (len) {
+		kentropy = memdup_user(entropy, len);
+		if (IS_ERR(kentropy))
+			return PTR_ERR(kentropy);
+	}
+
+	crypto_rng_alg(pctx->drng)->set_ent(pctx->drng, kentropy, len);
+	/*
+	 * Since rng doesn't perform any memory management for the entropy
+	 * buffer, save kentropy pointer to pctx now to free it after use.
+	 */
+	pctx->entropy = kentropy;
+	return 0;
 }
 
 static const struct af_alg_type algif_type_rng = {
@@ -171,7 +314,12 @@ static const struct af_alg_type algif_type_rng = {
 	.release	=	rng_release,
 	.accept		=	rng_accept_parent,
 	.setkey		=	rng_setkey,
+#if IS_ENABLED(CONFIG_CRYPTO_USER_API_CAVP_DRBG)
+	.setentropy	=	rng_setentropy,
+	.ops		=	&algif_rng_test_ops,
+#else
 	.ops		=	&algif_rng_ops,
+#endif
 	.name		=	"rng",
 	.owner		=	THIS_MODULE
 };
diff --git a/include/crypto/if_alg.h b/include/crypto/if_alg.h
index 56527c85d122..9e5c8ac53c59 100644
--- a/include/crypto/if_alg.h
+++ b/include/crypto/if_alg.h
@@ -46,6 +46,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 (*setentropy)(void *private, const u8 *entropy, unsigned int len);
 	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 bc2bcdec377b..60b7c2efd921 100644
--- a/include/uapi/linux/if_alg.h
+++ b/include/uapi/linux/if_alg.h
@@ -35,6 +35,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_DRBG_ENTROPY		6
 
 /* Operations */
 #define ALG_OP_DECRYPT			0
-- 
2.28.0.220.ged08abb693-goog

Re: [PATCH v5] crypto: af_alg - add extra parameters for DRBG interface

[Eric Biggers]

On Thu, Aug 13, 2020 at 05:08:11PM +0100, Elena Petrova wrote:
> Extend the user-space RNG interface:
>   1. Add entropy input via ALG_SET_DRBG_ENTROPY setsockopt option;
>   2. Add additional data input via sendmsg syscall.
> 
> This allows DRBG to be tested with test vectors, for example for the
> purpose of CAVP testing, which otherwise isn't possible.
> 
> To prevent erroneous use of entropy input, it is hidden under
> CRYPTO_USER_API_CAVP_DRBG config option and requires CAP_SYS_ADMIN to
> succeed.
> 
> Signed-off-by: Elena Petrova <lenaptr@google.com>
> Acked-by: Stephan Müller <smueller@chronox.de>
> ---
> 
> Updates in v5:
>   1) use __maybe_unused instead of #ifdef;
>   2) separate code path for a testing mode;
>   3) only allow Additional Data input in a testing mode.
> 
> Updates in v4:
>   1) setentropy returns 0 or error code (used to return length);
>   2) bigfixes suggested by Eric.
> 
> Updates in v3:
>   1) More details in commit message;
>   2) config option name is now CRYPTO_USER_API_CAVP_DRBG;
>   3) fixed a bug of not releasing socket locks.
> 
> Updates in v2:
>   1) Adding CONFIG_CRYPTO_CAVS_DRBG around setentropy.
>   2) Requiring CAP_SYS_ADMIN for entropy reset.
>   3) Locking for send and recv.
>   4) Length checks added for send and setentropy; send and setentropy now return
>      number of bytes accepted.
>   5) Minor code style corrections.
> 
> libkcapi patch for testing:
>   https://github.com/Len0k/libkcapi/commit/6f095d270b982008f419078614c15caa592cb531
> 
>  Documentation/crypto/userspace-if.rst |  17 ++-
>  crypto/Kconfig                        |   9 ++
>  crypto/af_alg.c                       |   8 ++
>  crypto/algif_rng.c                    | 172 ++++++++++++++++++++++++--
>  include/crypto/if_alg.h               |   1 +
>  include/uapi/linux/if_alg.h           |   1 +
>  6 files changed, 193 insertions(+), 15 deletions(-)
> 
> diff --git a/Documentation/crypto/userspace-if.rst b/Documentation/crypto/userspace-if.rst
> index ff86befa61e0..ef7132802c2d 100644
> --- a/Documentation/crypto/userspace-if.rst
> +++ b/Documentation/crypto/userspace-if.rst
> @@ -296,15 +296,23 @@ follows:
>  
>      struct sockaddr_alg sa = {
>          .salg_family = AF_ALG,
> -        .salg_type = "rng", /* this selects the symmetric cipher */
> -        .salg_name = "drbg_nopr_sha256" /* this is the cipher name */
> +        .salg_type = "rng", /* this selects the random number generator */
> +        .salg_name = "drbg_nopr_sha256" /* this is the RNG name */
>      };
>  
>  
>  Depending on the RNG type, the RNG must be seeded. The seed is provided
>  using the setsockopt interface to set the key. For example, the
>  ansi_cprng requires a seed. The DRBGs do not require a seed, but may be
> -seeded.
> +seeded. The seed is also known as a *Personalization String* in DRBG800-90A
> +standard.

Isn't the standard called "NIST SP 800-90A"?
"DRBG800-90A" doesn't return many hits on Google.

> +For the purpose of CAVP testing, the concatenation of *Entropy* and *Nonce*
> +can be provided to the RNG via ALG_SET_DRBG_ENTROPY setsockopt interface. This
> +requires a kernel built with CONFIG_CRYPTO_USER_API_CAVP_DRBG, and
> +CAP_SYS_ADMIN permission.
> +
> +*Additional Data* can be provided using the send()/sendmsg() system calls.

This doesn't make it clear whether the support for "Additional Data" is
dependent on CONFIG_CRYPTO_USER_API_CAVP_DRBG or not.

> diff --git a/crypto/Kconfig b/crypto/Kconfig
> index 091c0a0bbf26..7c8736f71681 100644
> --- a/crypto/Kconfig
> +++ b/crypto/Kconfig
> @@ -1896,6 +1896,15 @@ config CRYPTO_STATS
>  config CRYPTO_HASH_INFO
>  	bool
>  
> +config CRYPTO_USER_API_CAVP_DRBG
> +	tristate "Enable CAVP testing of DRBG"
> +	depends on CRYPTO_USER_API_RNG && CRYPTO_DRBG
> +	help
> +	  This option enables extra API for CAVP testing via the user-space
> +	  interface: resetting of DRBG entropy, and providing Additional Data.
> +	  This should only be enabled for CAVP testing. You should say
> +	  no unless you know what this is.

Using "tristate" here is incorrect because this option is not a module itself.
It's an option *for* a module.  So it needs to be "bool" instead.

Also, since this is an option to refine CRYPTO_USER_API_RNG, how about renaming
it to "CRYPTO_USER_API_RNG_CAVP", and moving it to just below the definition of
"CRYPTO_USER_API_RNG" so that they show up adjacent to each other?

> +static int rng_test_recvmsg(struct socket *sock, struct msghdr *msg, size_t len,
> +			    int flags)
> +{
> +	struct sock *sk = sock->sk;
> +	struct alg_sock *ask = alg_sk(sk);
> +	struct rng_ctx *ctx = ask->private;
> +	int err;
> +
> +	lock_sock(sock->sk);
> +	err = _rng_recvmsg(ctx->drng, msg, len, ctx->addtl, ctx->addtl_len);
> +	rng_reset_addtl(ctx);
> +	release_sock(sock->sk);
> +
> +	return err ? err : len;

Shouldn't this just return the value that _rng_recvmsg() returned?

Also 'err' is conventionally just for 0 or -errno codes.  Use 'ret' if the
variable can also hold a length.

> +static struct proto_ops __maybe_unused algif_rng_test_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,
> +	.sendpage	=	sock_no_sendpage,
> +
> +	.release	=	af_alg_release,
> +	.recvmsg	=	rng_test_recvmsg,
> +	.sendmsg	=	rng_test_sendmsg,
> +};
[...]
>  static const struct af_alg_type algif_type_rng = {
> @@ -171,7 +314,12 @@ static const struct af_alg_type algif_type_rng = {
>  	.release	=	rng_release,
>  	.accept		=	rng_accept_parent,
>  	.setkey		=	rng_setkey,
> +#if IS_ENABLED(CONFIG_CRYPTO_USER_API_CAVP_DRBG)
> +	.setentropy	=	rng_setentropy,
> +	.ops		=	&algif_rng_test_ops,
> +#else
>  	.ops		=	&algif_rng_ops,
> +#endif
>  	.name		=	"rng",
>  	.owner		=	THIS_MODULE
>  };

I think that switching to the separate proto_ops structs made the patch worse.
Now there's duplicated code.

Since proto_ops are almost identical, and only one is used in a given kernel
build, why not just do:

static struct proto_ops algif_rng_ops = {
	...
#ifdef CONFIG_CRYPTO_USER_API_RNG_CAVP
	.sendmsg	= rng_sendmsg,
#else
	.sendmsg	= sock_no_sendmsg,
#endif
	...
};

Similarly for .recvmsg(), although I don't understand what's wrong with just
adding the lock_sock() instead...  The RNG algorithms do locking anyway, so it's
not like that would regress the ability to recvmsg() in parallel.  Also,
conditional locking depending on the kernel config makes it more difficult to
find kernel bugs like deadlocks.

- Eric

Re: [PATCH v5] crypto: af_alg - add extra parameters for DRBG interface

[Herbert Xu]

Eric Biggers <ebiggers@kernel.org> wrote:
>
> Since proto_ops are almost identical, and only one is used in a given kernel
> build, why not just do:
> 
> static struct proto_ops algif_rng_ops = {
>        ...
> #ifdef CONFIG_CRYPTO_USER_API_RNG_CAVP
>        .sendmsg        = rng_sendmsg,
> #else
>        .sendmsg        = sock_no_sendmsg,
> #endif
>        ...
> };
> 
> Similarly for .recvmsg(), although I don't understand what's wrong with just
> adding the lock_sock() instead...  The RNG algorithms do locking anyway, so it's
> not like that would regress the ability to recvmsg() in parallel.  Also,
> conditional locking depending on the kernel config makes it more difficult to
> find kernel bugs like deadlocks.

I want this to have minimal impact on anyone who's not using it.
After all, this is something that only Google is asking for.

Anyway, I wasn't looking for a compile-time ops switch, but a
run-time one.

I think what we can do is move the new newsock->ops assignment
in af_alg_accept up above the type->accept call which would then
allow it to be overridden by the accept call.

After that you could just change newsock->ops depending on whether
pctx->entropy is NULL or not in rng_accept_parent.

As for the proto_ops duplication I don't think it's that big a
deal, but if you're really bothered just create a macro for the
identical bits in the struct.

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

[PATCH v6] crypto: af_alg - add extra parameters for DRBG interface

[Elena Petrova]

Extend the user-space RNG interface:
  1. Add entropy input via ALG_SET_DRBG_ENTROPY setsockopt option;
  2. Add additional data input via sendmsg syscall.

This allows DRBG to be tested with test vectors, for example for the
purpose of CAVP testing, which otherwise isn't possible.

To prevent erroneous use of entropy input, it is hidden under
CRYPTO_USER_API_RNG_CAVP config option and requires CAP_SYS_ADMIN to
succeed.

Signed-off-by: Elena Petrova <lenaptr@google.com>
Acked-by: Stephan Müller <smueller@chronox.de>
---

Updates in v6:
  1) Kconfig option renamed to CRYPTO_USER_API_RNG_CAVP and is now bool instead
     of tristate;
  2) run-time switch of proto_ops depending on whether the entropy was set;
  3) corrected the NIST standard name;
  4) rebased onto the tip of the tree;
  5) documentation clarified;

Updates in v5:
  1) use __maybe_unused instead of #ifdef;
  2) separate code path for a testing mode;
  3) only allow Additional Data input in a testing mode.

Updates in v4:
  1) setentropy returns 0 or error code (used to return length);
  2) bigfixes suggested by Eric.

Updates in v3:
  1) More details in commit message;
  2) config option name is now CRYPTO_USER_API_CAVP_DRBG;
  3) fixed a bug of not releasing socket locks.

Updates in v2:
  1) Adding CONFIG_CRYPTO_CAVS_DRBG around setentropy.
  2) Requiring CAP_SYS_ADMIN for entropy reset.
  3) Locking for send and recv.
  4) Length checks added for send and setentropy; send and setentropy now return
     number of bytes accepted.
  5) Minor code style corrections.

libkcapi patch for testing:
  https://github.com/Len0k/libkcapi/commit/6f095d270b982008f419078614c15caa592cb531

 Documentation/crypto/userspace-if.rst |  19 ++-
 crypto/Kconfig                        |   9 ++
 crypto/af_alg.c                       |  14 ++-
 crypto/algif_rng.c                    | 175 ++++++++++++++++++++++++--
 include/crypto/if_alg.h               |   1 +
 include/uapi/linux/if_alg.h           |   1 +
 6 files changed, 204 insertions(+), 15 deletions(-)

diff --git a/Documentation/crypto/userspace-if.rst b/Documentation/crypto/userspace-if.rst
index 52019e905900..6a542b0cf22c 100644
--- a/Documentation/crypto/userspace-if.rst
+++ b/Documentation/crypto/userspace-if.rst
@@ -296,15 +296,16 @@ follows:
 
     struct sockaddr_alg sa = {
         .salg_family = AF_ALG,
-        .salg_type = "rng", /* this selects the symmetric cipher */
-        .salg_name = "drbg_nopr_sha256" /* this is the cipher name */
+        .salg_type = "rng", /* this selects the random number generator */
+        .salg_name = "drbg_nopr_sha256" /* this is the RNG name */
     };
 
 
 Depending on the RNG type, the RNG must be seeded. The seed is provided
 using the setsockopt interface to set the key. For example, the
 ansi_cprng requires a seed. The DRBGs do not require a seed, but may be
-seeded.
+seeded. The seed is also known as a *Personalization String* in NIST SP 800-90A
+standard.
 
 Using the read()/recvmsg() system calls, random numbers can be obtained.
 The kernel generates at most 128 bytes in one call. If user space
@@ -314,6 +315,15 @@ WARNING: The user space caller may invoke the initially mentioned accept
 system call multiple times. In this case, the returned file descriptors
 have the same state.
 
+Following CAVP testing interfaces are enabled when kernel is built with
+CRYPTO_USER_API_RNG_CAVP option:
+
+-  the concatenation of *Entropy* and *Nonce* can be provided to the RNG via
+   ALG_SET_DRBG_ENTROPY setsockopt interface. Setting the entropy requires
+   CAP_SYS_ADMIN permission.
+
+-  *Additional Data* can be provided using the send()/sendmsg() system calls.
+
 Zero-Copy Interface
 -------------------
 
@@ -377,6 +387,9 @@ mentioned optname:
    provided ciphertext is assumed to contain an authentication tag of
    the given size (see section about AEAD memory layout below).
 
+-  ALG_SET_DRBG_ENTROPY -- Setting the entropy of the random number generator.
+   This option is applicable to RNG cipher type only.
+
 User space API example
 ----------------------
 
diff --git a/crypto/Kconfig b/crypto/Kconfig
index 1b57419fa2e7..070a88ec1ba8 100644
--- a/crypto/Kconfig
+++ b/crypto/Kconfig
@@ -1870,6 +1870,15 @@ config CRYPTO_USER_API_RNG
 	  This option enables the user-spaces interface for random
 	  number generator algorithms.
 
+config CRYPTO_USER_API_RNG_CAVP
+	bool "Enable CAVP testing of DRBG"
+	depends on CRYPTO_USER_API_RNG && CRYPTO_DRBG
+	help
+	  This option enables extra API for CAVP testing via the user-space
+	  interface: resetting of DRBG entropy, and providing Additional Data.
+	  This should only be enabled for CAVP testing. You should say
+	  no unless you know what this is.
+
 config CRYPTO_USER_API_AEAD
 	tristate "User-space interface for AEAD cipher algorithms"
 	depends on NET
diff --git a/crypto/af_alg.c b/crypto/af_alg.c
index 8be8bec07cdd..d11db80d24cd 100644
--- a/crypto/af_alg.c
+++ b/crypto/af_alg.c
@@ -254,6 +254,14 @@ static int alg_setsockopt(struct socket *sock, int level, int optname,
 		if (!type->setauthsize)
 			goto unlock;
 		err = type->setauthsize(ask->private, optlen);
+		break;
+	case ALG_SET_DRBG_ENTROPY:
+		if (sock->state == SS_CONNECTED)
+			goto unlock;
+		if (!type->setentropy)
+			goto unlock;
+
+		err = type->setentropy(ask->private, optval, optlen);
 	}
 
 unlock:
@@ -286,6 +294,11 @@ int af_alg_accept(struct sock *sk, struct socket *newsock, bool kern)
 	security_sock_graft(sk2, newsock);
 	security_sk_clone(sk, sk2);
 
+	/*
+	 * newsock->ops assigned here to allow type->accept call to override
+	 * them when required.
+	 */
+	newsock->ops = type->ops;
 	err = type->accept(ask->private, sk2);
 
 	nokey = err == -ENOKEY;
@@ -304,7 +317,6 @@ int af_alg_accept(struct sock *sk, struct socket *newsock, bool kern)
 	alg_sk(sk2)->parent = sk;
 	alg_sk(sk2)->type = type;
 
-	newsock->ops = type->ops;
 	newsock->state = SS_CONNECTED;
 
 	if (nokey)
diff --git a/crypto/algif_rng.c b/crypto/algif_rng.c
index 6300e0566dc5..6b1f01ca8e31 100644
--- a/crypto/algif_rng.c
+++ b/crypto/algif_rng.c
@@ -38,6 +38,7 @@
  * DAMAGE.
  */
 
+#include <linux/capability.h>
 #include <linux/module.h>
 #include <crypto/rng.h>
 #include <linux/random.h>
@@ -53,15 +54,26 @@ struct rng_ctx {
 #define MAXSIZE 128
 	unsigned int len;
 	struct crypto_rng *drng;
+	u8 *addtl;
+	size_t addtl_len;
 };
 
-static int rng_recvmsg(struct socket *sock, struct msghdr *msg, size_t len,
-		       int flags)
+struct rng_parent_ctx {
+	struct crypto_rng *drng;
+	u8 *entropy;
+};
+
+static void rng_reset_addtl(struct rng_ctx *ctx)
 {
-	struct sock *sk = sock->sk;
-	struct alg_sock *ask = alg_sk(sk);
-	struct rng_ctx *ctx = ask->private;
-	int err;
+	kzfree(ctx->addtl);
+	ctx->addtl = NULL;
+	ctx->addtl_len = 0;
+}
+
+static int _rng_recvmsg(struct crypto_rng *drng, struct msghdr *msg, size_t len,
+			u8 *addtl, size_t addtl_len)
+{
+	int err = 0;
 	int genlen = 0;
 	u8 result[MAXSIZE];
 
@@ -82,7 +94,7 @@ static int rng_recvmsg(struct socket *sock, struct msghdr *msg, size_t len,
 	 * seeding as they automatically seed. The X9.31 DRNG will return
 	 * an error if it was not seeded properly.
 	 */
-	genlen = crypto_rng_get_bytes(ctx->drng, result, len);
+	genlen = crypto_rng_generate(drng, addtl, addtl_len, result, len);
 	if (genlen < 0)
 		return genlen;
 
@@ -92,6 +104,61 @@ static int rng_recvmsg(struct socket *sock, struct msghdr *msg, size_t len,
 	return err ? err : len;
 }
 
+static int rng_recvmsg(struct socket *sock, struct msghdr *msg, size_t len,
+		       int flags)
+{
+	struct sock *sk = sock->sk;
+	struct alg_sock *ask = alg_sk(sk);
+	struct rng_ctx *ctx = ask->private;
+
+	return _rng_recvmsg(ctx->drng, msg, len, NULL, 0);
+}
+
+static int rng_test_recvmsg(struct socket *sock, struct msghdr *msg, size_t len,
+			    int flags)
+{
+	struct sock *sk = sock->sk;
+	struct alg_sock *ask = alg_sk(sk);
+	struct rng_ctx *ctx = ask->private;
+	int ret;
+
+	lock_sock(sock->sk);
+	ret = _rng_recvmsg(ctx->drng, msg, len, ctx->addtl, ctx->addtl_len);
+	rng_reset_addtl(ctx);
+	release_sock(sock->sk);
+
+	return ret;
+}
+
+static int rng_test_sendmsg(struct socket *sock, struct msghdr *msg, size_t len)
+{
+	int err;
+	struct alg_sock *ask = alg_sk(sock->sk);
+	struct rng_ctx *ctx = ask->private;
+
+	lock_sock(sock->sk);
+	if (len > MAXSIZE)
+		len = MAXSIZE;
+
+	rng_reset_addtl(ctx);
+	ctx->addtl = kmalloc(len, GFP_KERNEL);
+	if (!ctx->addtl) {
+		err = -ENOMEM;
+		goto unlock;
+	}
+
+	err = memcpy_from_msg(ctx->addtl, msg, len);
+	if (err) {
+		rng_reset_addtl(ctx);
+		goto unlock;
+	}
+	ctx->addtl_len = len;
+
+unlock:
+	release_sock(sock->sk);
+	return err ? err : len;
+}
+
 static struct proto_ops algif_rng_ops = {
 	.family		=	PF_ALG,
 
@@ -111,14 +178,55 @@ static struct proto_ops algif_rng_ops = {
 	.recvmsg	=	rng_recvmsg,
 };
 
+static struct proto_ops __maybe_unused algif_rng_test_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,
+	.sendpage	=	sock_no_sendpage,
+
+	.release	=	af_alg_release,
+	.recvmsg	=	rng_test_recvmsg,
+	.sendmsg	=	rng_test_sendmsg,
+};
+
 static void *rng_bind(const char *name, u32 type, u32 mask)
 {
-	return crypto_alloc_rng(name, type, mask);
+	struct rng_parent_ctx *pctx;
+	struct crypto_rng *rng;
+
+	pctx = kzalloc(sizeof(*pctx), GFP_KERNEL);
+	if (!pctx)
+		return ERR_PTR(-ENOMEM);
+
+	rng = crypto_alloc_rng(name, type, mask);
+	if (IS_ERR(rng)) {
+		kfree(pctx);
+		return ERR_CAST(rng);
+	}
+
+	pctx->drng = rng;
+	return pctx;
 }
 
 static void rng_release(void *private)
 {
-	crypto_free_rng(private);
+	struct rng_parent_ctx *pctx = private;
+
+	if (unlikely(!pctx))
+		return;
+	crypto_free_rng(pctx->drng);
+	kzfree(pctx->entropy);
+	kzfree(pctx);
 }
 
 static void rng_sock_destruct(struct sock *sk)
@@ -126,6 +234,7 @@ static void rng_sock_destruct(struct sock *sk)
 	struct alg_sock *ask = alg_sk(sk);
 	struct rng_ctx *ctx = ask->private;
 
+	rng_reset_addtl(ctx);
 	sock_kfree_s(sk, ctx, ctx->len);
 	af_alg_release_parent(sk);
 }
@@ -133,6 +242,7 @@ static void rng_sock_destruct(struct sock *sk)
 static int rng_accept_parent(void *private, struct sock *sk)
 {
 	struct rng_ctx *ctx;
+	struct rng_parent_ctx *pctx = private;
 	struct alg_sock *ask = alg_sk(sk);
 	unsigned int len = sizeof(*ctx);
 
@@ -141,6 +251,8 @@ static int rng_accept_parent(void *private, struct sock *sk)
 		return -ENOMEM;
 
 	ctx->len = len;
+	ctx->addtl = NULL;
+	ctx->addtl_len = 0;
 
 	/*
 	 * No seeding done at that point -- if multiple accepts are
@@ -148,20 +260,58 @@ static int rng_accept_parent(void *private, struct sock *sk)
 	 * state of the RNG.
 	 */
 
-	ctx->drng = private;
+	ctx->drng = pctx->drng;
 	ask->private = ctx;
 	sk->sk_destruct = rng_sock_destruct;
 
+	/*
+	 * Non NULL pctx->entropy means that CAVP test has been initiated on
+	 * this socket, replace proto_ops algif_rng_ops with algif_rng_test_ops.
+	 */
+	if (pctx->entropy)
+		sk->sk_socket->ops = algif_rng_test_ops;
+
 	return 0;
 }
 
 static int rng_setkey(void *private, const u8 *seed, unsigned int seedlen)
 {
+	struct rng_parent_ctx *pctx = private;
 	/*
 	 * Check whether seedlen is of sufficient size is done in RNG
 	 * implementations.
 	 */
-	return crypto_rng_reset(private, seed, seedlen);
+	return crypto_rng_reset(pctx->drng, seed, seedlen);
+}
+
+static int __maybe_unused rng_setentropy(void *private, const u8 *entropy,
+					 unsigned int len)
+{
+	struct rng_parent_ctx *pctx = private;
+	u8 *kentropy = NULL;
+
+	if (!capable(CAP_SYS_ADMIN))
+		return -EACCES;
+
+	if (pctx->entropy)
+		return -EINVAL;
+
+	if (len > MAXSIZE)
+		return -EMSGSIZE;
+
+	if (len) {
+		kentropy = memdup_user(entropy, len);
+		if (IS_ERR(kentropy))
+			return PTR_ERR(kentropy);
+	}
+
+	crypto_rng_alg(pctx->drng)->set_ent(pctx->drng, kentropy, len);
+	/*
+	 * Since rng doesn't perform any memory management for the entropy
+	 * buffer, save kentropy pointer to pctx now to free it after use.
+	 */
+	pctx->entropy = kentropy;
+	return 0;
 }
 
 static const struct af_alg_type algif_type_rng = {
@@ -169,6 +319,9 @@ static const struct af_alg_type algif_type_rng = {
 	.release	=	rng_release,
 	.accept		=	rng_accept_parent,
 	.setkey		=	rng_setkey,
+#if IS_ENABLED(CONFIG_CRYPTO_USER_API_RNG_CAVP)
+	.setentropy	=	rng_setentropy,
+#endif
 	.ops		=	&algif_rng_ops,
 	.name		=	"rng",
 	.owner		=	THIS_MODULE
diff --git a/include/crypto/if_alg.h b/include/crypto/if_alg.h
index ee6412314f8f..e6f4a342417d 100644
--- a/include/crypto/if_alg.h
+++ b/include/crypto/if_alg.h
@@ -46,6 +46,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 (*setentropy)(void *private, const u8 *entropy, unsigned int len);
 	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 bc2bcdec377b..60b7c2efd921 100644
--- a/include/uapi/linux/if_alg.h
+++ b/include/uapi/linux/if_alg.h
@@ -35,6 +35,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_DRBG_ENTROPY		6
 
 /* Operations */
 #define ALG_OP_DECRYPT			0
-- 
2.28.0.526.ge36021eeef-goog

Re: [PATCH v6] crypto: af_alg - add extra parameters for DRBG interface

[Eric Biggers]

On Tue, Sep 08, 2020 at 06:04:03PM +0100, Elena Petrova wrote:
> Extend the user-space RNG interface:
>   1. Add entropy input via ALG_SET_DRBG_ENTROPY setsockopt option;
>   2. Add additional data input via sendmsg syscall.
> 
> This allows DRBG to be tested with test vectors, for example for the
> purpose of CAVP testing, which otherwise isn't possible.
> 
> To prevent erroneous use of entropy input, it is hidden under
> CRYPTO_USER_API_RNG_CAVP config option and requires CAP_SYS_ADMIN to
> succeed.
> 
> Signed-off-by: Elena Petrova <lenaptr@google.com>
> Acked-by: Stephan Müller <smueller@chronox.de>

This doesn't compile for me.  Can you rebase this onto the latest
"master" branch from
https://git.kernel.org/pub/scm/linux/kernel/git/herbert/cryptodev-2.6.git?

> +static void rng_reset_addtl(struct rng_ctx *ctx)
>  {
> -	struct sock *sk = sock->sk;
> -	struct alg_sock *ask = alg_sk(sk);
> -	struct rng_ctx *ctx = ask->private;
> -	int err;
> +	kzfree(ctx->addtl);
> +	ctx->addtl = NULL;
> +	ctx->addtl_len = 0;
> +}

kzfree() has been renamed to kfree_sensitive(); see commit 453431a54934
("mm, treewide: rename kzfree() to kfree_sensitive()").
So please use kfree_sensitive() rather than kzfree(), in all three places.

Note, kzfree() won't actually cause a compilation error since it's still
#define'd to kfree_sensitive().  But that #define probably will go away soon.

> +static int rng_test_sendmsg(struct socket *sock, struct msghdr *msg, size_t len)
> +{
> +	int err;
> +	struct alg_sock *ask = alg_sk(sock->sk);
> +	struct rng_ctx *ctx = ask->private;
> +
> +	lock_sock(sock->sk);
> +	if (len > MAXSIZE)
> +		len = MAXSIZE;

Since this function only supports providing the additional data all at once, not
incrementally, shouldn't it return an error code if the length is too long,
rather than truncate the length?

> +	/*
> +	 * Non NULL pctx->entropy means that CAVP test has been initiated on
> +	 * this socket, replace proto_ops algif_rng_ops with algif_rng_test_ops.
> +	 */
> +	if (pctx->entropy)
> +		sk->sk_socket->ops = algif_rng_test_ops;

This means that providing additional data on a "request socket" via sendmsg will
only work if ALG_SET_DRBG_ENTROPY was used on the "algorithm socket" earlier.
If that's intentional, it needs to be mentioned in the documentation.

>  static const struct af_alg_type algif_type_rng = {
> @@ -169,6 +319,9 @@ static const struct af_alg_type algif_type_rng = {
>  	.release	=	rng_release,
>  	.accept		=	rng_accept_parent,
>  	.setkey		=	rng_setkey,
> +#if IS_ENABLED(CONFIG_CRYPTO_USER_API_RNG_CAVP)
> +	.setentropy	=	rng_setentropy,
> +#endif

Since CRYPTO_USER_API_RNG_CAVP is now a bool rather than a tristate, this should
use '#ifdef CONFIG_CRYPTO_USER_API_RNG_CAVP' instead of
'IS_ENABLED(CONFIG_CRYPTO_USER_API_RNG_CAVP)'.

- Eric

[PATCH v7] crypto: af_alg - add extra parameters for DRBG interface

[Elena Petrova]

Extend the user-space RNG interface:
  1. Add entropy input via ALG_SET_DRBG_ENTROPY setsockopt option;
  2. Add additional data input via sendmsg syscall.

This allows DRBG to be tested with test vectors, for example for the
purpose of CAVP testing, which otherwise isn't possible.

To prevent erroneous use of entropy input, it is hidden under
CRYPTO_USER_API_RNG_CAVP config option and requires CAP_SYS_ADMIN to
succeed.

Signed-off-by: Elena Petrova <lenaptr@google.com>
Acked-by: Stephan Müller <smueller@chronox.de>
---

Updates in v7:
  1) rebased onto the latest at cryptodev-2.6.git, fixed compiler errors;
  2) replaced kzfree with kfree_sensitive;
  3) changed rng_test_sendmsg to return an error if len > MAXSIZE;
  4) updated documentation to say when can Additional Data be provided.

Updates in v6:
  1) Kconfig option renamed to CRYPTO_USER_API_RNG_CAVP and is now bool instead
     of tristate;
  2) run-time switch of proto_ops depending on whether the entropy was set;
  3) corrected the NIST standard name;
  4) rebased onto the tip of the tree;
  5) documentation clarified;

Updates in v5:
  1) use __maybe_unused instead of #ifdef;
  2) separate code path for a testing mode;
  3) only allow Additional Data input in a testing mode.

Updates in v4:
  1) setentropy returns 0 or error code (used to return length);
  2) bigfixes suggested by Eric.

Updates in v3:
  1) More details in commit message;
  2) config option name is now CRYPTO_USER_API_CAVP_DRBG;
  3) fixed a bug of not releasing socket locks.

Updates in v2:
  1) Adding CONFIG_CRYPTO_CAVS_DRBG around setentropy.
  2) Requiring CAP_SYS_ADMIN for entropy reset.
  3) Locking for send and recv.
  4) Length checks added for send and setentropy; send and setentropy now return
     number of bytes accepted.
  5) Minor code style corrections.

libkcapi patch for testing:
  https://github.com/Len0k/libkcapi/commit/6f095d270b982008f419078614c15caa592cb531

 Documentation/crypto/userspace-if.rst |  20 ++-
 crypto/Kconfig                        |   9 ++
 crypto/af_alg.c                       |  14 ++-
 crypto/algif_rng.c                    | 175 ++++++++++++++++++++++++--
 include/crypto/if_alg.h               |   1 +
 include/uapi/linux/if_alg.h           |   1 +
 6 files changed, 205 insertions(+), 15 deletions(-)

diff --git a/Documentation/crypto/userspace-if.rst b/Documentation/crypto/userspace-if.rst
index 52019e905900..b45dabbf69d6 100644
--- a/Documentation/crypto/userspace-if.rst
+++ b/Documentation/crypto/userspace-if.rst
@@ -296,15 +296,16 @@ follows:
 
     struct sockaddr_alg sa = {
         .salg_family = AF_ALG,
-        .salg_type = "rng", /* this selects the symmetric cipher */
-        .salg_name = "drbg_nopr_sha256" /* this is the cipher name */
+        .salg_type = "rng", /* this selects the random number generator */
+        .salg_name = "drbg_nopr_sha256" /* this is the RNG name */
     };
 
 
 Depending on the RNG type, the RNG must be seeded. The seed is provided
 using the setsockopt interface to set the key. For example, the
 ansi_cprng requires a seed. The DRBGs do not require a seed, but may be
-seeded.
+seeded. The seed is also known as a *Personalization String* in NIST SP 800-90A
+standard.
 
 Using the read()/recvmsg() system calls, random numbers can be obtained.
 The kernel generates at most 128 bytes in one call. If user space
@@ -314,6 +315,16 @@ WARNING: The user space caller may invoke the initially mentioned accept
 system call multiple times. In this case, the returned file descriptors
 have the same state.
 
+Following CAVP testing interfaces are enabled when kernel is built with
+CRYPTO_USER_API_RNG_CAVP option:
+
+-  the concatenation of *Entropy* and *Nonce* can be provided to the RNG via
+   ALG_SET_DRBG_ENTROPY setsockopt interface. Setting the entropy requires
+   CAP_SYS_ADMIN permission.
+
+-  *Additional Data* can be provided using the send()/sendmsg() system calls,
+   but only after the entropy has been set.
+
 Zero-Copy Interface
 -------------------
 
@@ -377,6 +388,9 @@ mentioned optname:
    provided ciphertext is assumed to contain an authentication tag of
    the given size (see section about AEAD memory layout below).
 
+-  ALG_SET_DRBG_ENTROPY -- Setting the entropy of the random number generator.
+   This option is applicable to RNG cipher type only.
+
 User space API example
 ----------------------
 
diff --git a/crypto/Kconfig b/crypto/Kconfig
index 1b57419fa2e7..070a88ec1ba8 100644
--- a/crypto/Kconfig
+++ b/crypto/Kconfig
@@ -1870,6 +1870,15 @@ config CRYPTO_USER_API_RNG
 	  This option enables the user-spaces interface for random
 	  number generator algorithms.
 
+config CRYPTO_USER_API_RNG_CAVP
+	bool "Enable CAVP testing of DRBG"
+	depends on CRYPTO_USER_API_RNG && CRYPTO_DRBG
+	help
+	  This option enables extra API for CAVP testing via the user-space
+	  interface: resetting of DRBG entropy, and providing Additional Data.
+	  This should only be enabled for CAVP testing. You should say
+	  no unless you know what this is.
+
 config CRYPTO_USER_API_AEAD
 	tristate "User-space interface for AEAD cipher algorithms"
 	depends on NET
diff --git a/crypto/af_alg.c b/crypto/af_alg.c
index a6f581ab200c..8535cb03b484 100644
--- a/crypto/af_alg.c
+++ b/crypto/af_alg.c
@@ -253,6 +253,14 @@ static int alg_setsockopt(struct socket *sock, int level, int optname,
 		if (!type->setauthsize)
 			goto unlock;
 		err = type->setauthsize(ask->private, optlen);
+		break;
+	case ALG_SET_DRBG_ENTROPY:
+		if (sock->state == SS_CONNECTED)
+			goto unlock;
+		if (!type->setentropy)
+			goto unlock;
+
+		err = type->setentropy(ask->private, optval, optlen);
 	}
 
 unlock:
@@ -285,6 +293,11 @@ int af_alg_accept(struct sock *sk, struct socket *newsock, bool kern)
 	security_sock_graft(sk2, newsock);
 	security_sk_clone(sk, sk2);
 
+	/*
+	 * newsock->ops assigned here to allow type->accept call to override
+	 * them when required.
+	 */
+	newsock->ops = type->ops;
 	err = type->accept(ask->private, sk2);
 
 	nokey = err == -ENOKEY;
@@ -303,7 +316,6 @@ int af_alg_accept(struct sock *sk, struct socket *newsock, bool kern)
 	alg_sk(sk2)->parent = sk;
 	alg_sk(sk2)->type = type;
 
-	newsock->ops = type->ops;
 	newsock->state = SS_CONNECTED;
 
 	if (nokey)
diff --git a/crypto/algif_rng.c b/crypto/algif_rng.c
index 6300e0566dc5..3ca571b10b08 100644
--- a/crypto/algif_rng.c
+++ b/crypto/algif_rng.c
@@ -38,6 +38,7 @@
  * DAMAGE.
  */
 
+#include <linux/capability.h>
 #include <linux/module.h>
 #include <crypto/rng.h>
 #include <linux/random.h>
@@ -53,15 +54,26 @@ struct rng_ctx {
 #define MAXSIZE 128
 	unsigned int len;
 	struct crypto_rng *drng;
+	u8 *addtl;
+	size_t addtl_len;
 };
 
-static int rng_recvmsg(struct socket *sock, struct msghdr *msg, size_t len,
-		       int flags)
+struct rng_parent_ctx {
+	struct crypto_rng *drng;
+	u8 *entropy;
+};
+
+static void rng_reset_addtl(struct rng_ctx *ctx)
 {
-	struct sock *sk = sock->sk;
-	struct alg_sock *ask = alg_sk(sk);
-	struct rng_ctx *ctx = ask->private;
-	int err;
+	kfree_sensitive(ctx->addtl);
+	ctx->addtl = NULL;
+	ctx->addtl_len = 0;
+}
+
+static int _rng_recvmsg(struct crypto_rng *drng, struct msghdr *msg, size_t len,
+			u8 *addtl, size_t addtl_len)
+{
+	int err = 0;
 	int genlen = 0;
 	u8 result[MAXSIZE];
 
@@ -82,7 +94,7 @@ static int rng_recvmsg(struct socket *sock, struct msghdr *msg, size_t len,
 	 * seeding as they automatically seed. The X9.31 DRNG will return
 	 * an error if it was not seeded properly.
 	 */
-	genlen = crypto_rng_get_bytes(ctx->drng, result, len);
+	genlen = crypto_rng_generate(drng, addtl, addtl_len, result, len);
 	if (genlen < 0)
 		return genlen;
 
@@ -92,6 +104,63 @@ static int rng_recvmsg(struct socket *sock, struct msghdr *msg, size_t len,
 	return err ? err : len;
 }
 
+static int rng_recvmsg(struct socket *sock, struct msghdr *msg, size_t len,
+		       int flags)
+{
+	struct sock *sk = sock->sk;
+	struct alg_sock *ask = alg_sk(sk);
+	struct rng_ctx *ctx = ask->private;
+
+	return _rng_recvmsg(ctx->drng, msg, len, NULL, 0);
+}
+
+static int rng_test_recvmsg(struct socket *sock, struct msghdr *msg, size_t len,
+			    int flags)
+{
+	struct sock *sk = sock->sk;
+	struct alg_sock *ask = alg_sk(sk);
+	struct rng_ctx *ctx = ask->private;
+	int ret;
+
+	lock_sock(sock->sk);
+	ret = _rng_recvmsg(ctx->drng, msg, len, ctx->addtl, ctx->addtl_len);
+	rng_reset_addtl(ctx);
+	release_sock(sock->sk);
+
+	return ret;
+}
+
+static int rng_test_sendmsg(struct socket *sock, struct msghdr *msg, size_t len)
+{
+	int err;
+	struct alg_sock *ask = alg_sk(sock->sk);
+	struct rng_ctx *ctx = ask->private;
+
+	lock_sock(sock->sk);
+	if (len > MAXSIZE) {
+		err = -EMSGSIZE;
+		goto unlock;
+	}
+
+	rng_reset_addtl(ctx);
+	ctx->addtl = kmalloc(len, GFP_KERNEL);
+	if (!ctx->addtl) {
+		err = -ENOMEM;
+		goto unlock;
+	}
+
+	err = memcpy_from_msg(ctx->addtl, msg, len);
+	if (err) {
+		rng_reset_addtl(ctx);
+		goto unlock;
+	}
+	ctx->addtl_len = len;
+
+unlock:
+	release_sock(sock->sk);
+	return err ? err : len;
+}
+
 static struct proto_ops algif_rng_ops = {
 	.family		=	PF_ALG,
 
@@ -111,14 +180,53 @@ static struct proto_ops algif_rng_ops = {
 	.recvmsg	=	rng_recvmsg,
 };
 
+static struct proto_ops __maybe_unused algif_rng_test_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,
+	.mmap		=	sock_no_mmap,
+	.bind		=	sock_no_bind,
+	.accept		=	sock_no_accept,
+	.sendpage	=	sock_no_sendpage,
+
+	.release	=	af_alg_release,
+	.recvmsg	=	rng_test_recvmsg,
+	.sendmsg	=	rng_test_sendmsg,
+};
+
 static void *rng_bind(const char *name, u32 type, u32 mask)
 {
-	return crypto_alloc_rng(name, type, mask);
+	struct rng_parent_ctx *pctx;
+	struct crypto_rng *rng;
+
+	pctx = kzalloc(sizeof(*pctx), GFP_KERNEL);
+	if (!pctx)
+		return ERR_PTR(-ENOMEM);
+
+	rng = crypto_alloc_rng(name, type, mask);
+	if (IS_ERR(rng)) {
+		kfree(pctx);
+		return ERR_CAST(rng);
+	}
+
+	pctx->drng = rng;
+	return pctx;
 }
 
 static void rng_release(void *private)
 {
-	crypto_free_rng(private);
+	struct rng_parent_ctx *pctx = private;
+
+	if (unlikely(!pctx))
+		return;
+	crypto_free_rng(pctx->drng);
+	kfree_sensitive(pctx->entropy);
+	kfree_sensitive(pctx);
 }
 
 static void rng_sock_destruct(struct sock *sk)
@@ -126,6 +234,7 @@ static void rng_sock_destruct(struct sock *sk)
 	struct alg_sock *ask = alg_sk(sk);
 	struct rng_ctx *ctx = ask->private;
 
+	rng_reset_addtl(ctx);
 	sock_kfree_s(sk, ctx, ctx->len);
 	af_alg_release_parent(sk);
 }
@@ -133,6 +242,7 @@ static void rng_sock_destruct(struct sock *sk)
 static int rng_accept_parent(void *private, struct sock *sk)
 {
 	struct rng_ctx *ctx;
+	struct rng_parent_ctx *pctx = private;
 	struct alg_sock *ask = alg_sk(sk);
 	unsigned int len = sizeof(*ctx);
 
@@ -141,6 +251,8 @@ static int rng_accept_parent(void *private, struct sock *sk)
 		return -ENOMEM;
 
 	ctx->len = len;
+	ctx->addtl = NULL;
+	ctx->addtl_len = 0;
 
 	/*
 	 * No seeding done at that point -- if multiple accepts are
@@ -148,20 +260,58 @@ static int rng_accept_parent(void *private, struct sock *sk)
 	 * state of the RNG.
 	 */
 
-	ctx->drng = private;
+	ctx->drng = pctx->drng;
 	ask->private = ctx;
 	sk->sk_destruct = rng_sock_destruct;
 
+	/*
+	 * Non NULL pctx->entropy means that CAVP test has been initiated on
+	 * this socket, replace proto_ops algif_rng_ops with algif_rng_test_ops.
+	 */
+	if (pctx->entropy)
+		sk->sk_socket->ops = &algif_rng_test_ops;
+
 	return 0;
 }
 
 static int rng_setkey(void *private, const u8 *seed, unsigned int seedlen)
 {
+	struct rng_parent_ctx *pctx = private;
 	/*
 	 * Check whether seedlen is of sufficient size is done in RNG
 	 * implementations.
 	 */
-	return crypto_rng_reset(private, seed, seedlen);
+	return crypto_rng_reset(pctx->drng, seed, seedlen);
+}
+
+static int __maybe_unused rng_setentropy(void *private, sockptr_t entropy,
+					 unsigned int len)
+{
+	struct rng_parent_ctx *pctx = private;
+	u8 *kentropy = NULL;
+
+	if (!capable(CAP_SYS_ADMIN))
+		return -EACCES;
+
+	if (pctx->entropy)
+		return -EINVAL;
+
+	if (len > MAXSIZE)
+		return -EMSGSIZE;
+
+	if (len) {
+		kentropy = memdup_sockptr(entropy, len);
+		if (IS_ERR(kentropy))
+			return PTR_ERR(kentropy);
+	}
+
+	crypto_rng_alg(pctx->drng)->set_ent(pctx->drng, kentropy, len);
+	/*
+	 * Since rng doesn't perform any memory management for the entropy
+	 * buffer, save kentropy pointer to pctx now to free it after use.
+	 */
+	pctx->entropy = kentropy;
+	return 0;
 }
 
 static const struct af_alg_type algif_type_rng = {
@@ -169,6 +319,9 @@ static const struct af_alg_type algif_type_rng = {
 	.release	=	rng_release,
 	.accept		=	rng_accept_parent,
 	.setkey		=	rng_setkey,
+#ifdef CONFIG_CRYPTO_USER_API_RNG_CAVP
+	.setentropy	=	rng_setentropy,
+#endif
 	.ops		=	&algif_rng_ops,
 	.name		=	"rng",
 	.owner		=	THIS_MODULE
diff --git a/include/crypto/if_alg.h b/include/crypto/if_alg.h
index ee6412314f8f..a5db86670bdf 100644
--- a/include/crypto/if_alg.h
+++ b/include/crypto/if_alg.h
@@ -46,6 +46,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 (*setentropy)(void *private, sockptr_t entropy, unsigned int len);
 	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 bc2bcdec377b..60b7c2efd921 100644
--- a/include/uapi/linux/if_alg.h
+++ b/include/uapi/linux/if_alg.h
@@ -35,6 +35,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_DRBG_ENTROPY		6
 
 /* Operations */
 #define ALG_OP_DECRYPT			0
-- 
2.28.0.526.ge36021eeef-goog

Re: [PATCH v7] crypto: af_alg - add extra parameters for DRBG interface

[Eric Biggers]

On Wed, Sep 09, 2020 at 07:29:47PM +0100, Elena Petrova wrote:
> Extend the user-space RNG interface:
>   1. Add entropy input via ALG_SET_DRBG_ENTROPY setsockopt option;
>   2. Add additional data input via sendmsg syscall.
> 
> This allows DRBG to be tested with test vectors, for example for the
> purpose of CAVP testing, which otherwise isn't possible.
> 
> To prevent erroneous use of entropy input, it is hidden under
> CRYPTO_USER_API_RNG_CAVP config option and requires CAP_SYS_ADMIN to
> succeed.
> 
> Signed-off-by: Elena Petrova <lenaptr@google.com>
> Acked-by: Stephan Müller <smueller@chronox.de>

Reviewed-by: Eric Biggers <ebiggers@google.com>

[PATCH v8] crypto: af_alg - add extra parameters for DRBG interface

[Elena Petrova]

Extend the user-space RNG interface:
  1. Add entropy input via ALG_SET_DRBG_ENTROPY setsockopt option;
  2. Add additional data input via sendmsg syscall.

This allows DRBG to be tested with test vectors, for example for the
purpose of CAVP testing, which otherwise isn't possible.

To prevent erroneous use of entropy input, it is hidden under
CRYPTO_USER_API_RNG_CAVP config option and requires CAP_SYS_ADMIN to
succeed.

Signed-off-by: Elena Petrova <lenaptr@google.com>
Acked-by: Stephan Müller <smueller@chronox.de>
Reviewed-by: Eric Biggers <ebiggers@google.com>
---

Herbert, can you please include this for 5.10?

Updates in v8:
  Added Reviewed-by tag to the description.

Updates in v7:
  1) rebased onto the latest at cryptodev-2.6.git, fixed compiler errors;
  2) replaced kzfree with kfree_sensitive;
  3) changed rng_test_sendmsg to return an error if len > MAXSIZE;
  4) updated documentation to say when can Additional Data be provided.

Updates in v6:
  1) Kconfig option renamed to CRYPTO_USER_API_RNG_CAVP and is now bool instead
     of tristate;
  2) run-time switch of proto_ops depending on whether the entropy was set;
  3) corrected the NIST standard name;
  4) rebased onto the tip of the tree;
  5) documentation clarified;

Updates in v5:
  1) use __maybe_unused instead of #ifdef;
  2) separate code path for a testing mode;
  3) only allow Additional Data input in a testing mode.

Updates in v4:
  1) setentropy returns 0 or error code (used to return length);
  2) bigfixes suggested by Eric.

Updates in v3:
  1) More details in commit message;
  2) config option name is now CRYPTO_USER_API_CAVP_DRBG;
  3) fixed a bug of not releasing socket locks.

Updates in v2:
  1) Adding CONFIG_CRYPTO_CAVS_DRBG around setentropy.
  2) Requiring CAP_SYS_ADMIN for entropy reset.
  3) Locking for send and recv.
  4) Length checks added for send and setentropy; send and setentropy now return
     number of bytes accepted.
  5) Minor code style corrections.

libkcapi patch for testing:
  https://github.com/Len0k/libkcapi/commit/6f095d270b982008f419078614c15caa592cb531

 Documentation/crypto/userspace-if.rst |  20 ++-
 crypto/Kconfig                        |   9 ++
 crypto/af_alg.c                       |  14 ++-
 crypto/algif_rng.c                    | 175 ++++++++++++++++++++++++--
 include/crypto/if_alg.h               |   1 +
 include/uapi/linux/if_alg.h           |   1 +
 6 files changed, 205 insertions(+), 15 deletions(-)

diff --git a/Documentation/crypto/userspace-if.rst b/Documentation/crypto/userspace-if.rst
index 52019e905900..b45dabbf69d6 100644
--- a/Documentation/crypto/userspace-if.rst
+++ b/Documentation/crypto/userspace-if.rst
@@ -296,15 +296,16 @@ follows:
 
     struct sockaddr_alg sa = {
         .salg_family = AF_ALG,
-        .salg_type = "rng", /* this selects the symmetric cipher */
-        .salg_name = "drbg_nopr_sha256" /* this is the cipher name */
+        .salg_type = "rng", /* this selects the random number generator */
+        .salg_name = "drbg_nopr_sha256" /* this is the RNG name */
     };
 
 
 Depending on the RNG type, the RNG must be seeded. The seed is provided
 using the setsockopt interface to set the key. For example, the
 ansi_cprng requires a seed. The DRBGs do not require a seed, but may be
-seeded.
+seeded. The seed is also known as a *Personalization String* in NIST SP 800-90A
+standard.
 
 Using the read()/recvmsg() system calls, random numbers can be obtained.
 The kernel generates at most 128 bytes in one call. If user space
@@ -314,6 +315,16 @@ WARNING: The user space caller may invoke the initially mentioned accept
 system call multiple times. In this case, the returned file descriptors
 have the same state.
 
+Following CAVP testing interfaces are enabled when kernel is built with
+CRYPTO_USER_API_RNG_CAVP option:
+
+-  the concatenation of *Entropy* and *Nonce* can be provided to the RNG via
+   ALG_SET_DRBG_ENTROPY setsockopt interface. Setting the entropy requires
+   CAP_SYS_ADMIN permission.
+
+-  *Additional Data* can be provided using the send()/sendmsg() system calls,
+   but only after the entropy has been set.
+
 Zero-Copy Interface
 -------------------
 
@@ -377,6 +388,9 @@ mentioned optname:
    provided ciphertext is assumed to contain an authentication tag of
    the given size (see section about AEAD memory layout below).
 
+-  ALG_SET_DRBG_ENTROPY -- Setting the entropy of the random number generator.
+   This option is applicable to RNG cipher type only.
+
 User space API example
 ----------------------
 
diff --git a/crypto/Kconfig b/crypto/Kconfig
index 1b57419fa2e7..070a88ec1ba8 100644
--- a/crypto/Kconfig
+++ b/crypto/Kconfig
@@ -1870,6 +1870,15 @@ config CRYPTO_USER_API_RNG
 	  This option enables the user-spaces interface for random
 	  number generator algorithms.
 
+config CRYPTO_USER_API_RNG_CAVP
+	bool "Enable CAVP testing of DRBG"
+	depends on CRYPTO_USER_API_RNG && CRYPTO_DRBG
+	help
+	  This option enables extra API for CAVP testing via the user-space
+	  interface: resetting of DRBG entropy, and providing Additional Data.
+	  This should only be enabled for CAVP testing. You should say
+	  no unless you know what this is.
+
 config CRYPTO_USER_API_AEAD
 	tristate "User-space interface for AEAD cipher algorithms"
 	depends on NET
diff --git a/crypto/af_alg.c b/crypto/af_alg.c
index a6f581ab200c..8535cb03b484 100644
--- a/crypto/af_alg.c
+++ b/crypto/af_alg.c
@@ -253,6 +253,14 @@ static int alg_setsockopt(struct socket *sock, int level, int optname,
 		if (!type->setauthsize)
 			goto unlock;
 		err = type->setauthsize(ask->private, optlen);
+		break;
+	case ALG_SET_DRBG_ENTROPY:
+		if (sock->state == SS_CONNECTED)
+			goto unlock;
+		if (!type->setentropy)
+			goto unlock;
+
+		err = type->setentropy(ask->private, optval, optlen);
 	}
 
 unlock:
@@ -285,6 +293,11 @@ int af_alg_accept(struct sock *sk, struct socket *newsock, bool kern)
 	security_sock_graft(sk2, newsock);
 	security_sk_clone(sk, sk2);
 
+	/*
+	 * newsock->ops assigned here to allow type->accept call to override
+	 * them when required.
+	 */
+	newsock->ops = type->ops;
 	err = type->accept(ask->private, sk2);
 
 	nokey = err == -ENOKEY;
@@ -303,7 +316,6 @@ int af_alg_accept(struct sock *sk, struct socket *newsock, bool kern)
 	alg_sk(sk2)->parent = sk;
 	alg_sk(sk2)->type = type;
 
-	newsock->ops = type->ops;
 	newsock->state = SS_CONNECTED;
 
 	if (nokey)
diff --git a/crypto/algif_rng.c b/crypto/algif_rng.c
index 6300e0566dc5..3ca571b10b08 100644
--- a/crypto/algif_rng.c
+++ b/crypto/algif_rng.c
@@ -38,6 +38,7 @@
  * DAMAGE.
  */
 
+#include <linux/capability.h>
 #include <linux/module.h>
 #include <crypto/rng.h>
 #include <linux/random.h>
@@ -53,15 +54,26 @@ struct rng_ctx {
 #define MAXSIZE 128
 	unsigned int len;
 	struct crypto_rng *drng;
+	u8 *addtl;
+	size_t addtl_len;
 };
 
-static int rng_recvmsg(struct socket *sock, struct msghdr *msg, size_t len,
-		       int flags)
+struct rng_parent_ctx {
+	struct crypto_rng *drng;
+	u8 *entropy;
+};
+
+static void rng_reset_addtl(struct rng_ctx *ctx)
 {
-	struct sock *sk = sock->sk;
-	struct alg_sock *ask = alg_sk(sk);
-	struct rng_ctx *ctx = ask->private;
-	int err;
+	kfree_sensitive(ctx->addtl);
+	ctx->addtl = NULL;
+	ctx->addtl_len = 0;
+}
+
+static int _rng_recvmsg(struct crypto_rng *drng, struct msghdr *msg, size_t len,
+			u8 *addtl, size_t addtl_len)
+{
+	int err = 0;
 	int genlen = 0;
 	u8 result[MAXSIZE];
 
@@ -82,7 +94,7 @@ static int rng_recvmsg(struct socket *sock, struct msghdr *msg, size_t len,
 	 * seeding as they automatically seed. The X9.31 DRNG will return
 	 * an error if it was not seeded properly.
 	 */
-	genlen = crypto_rng_get_bytes(ctx->drng, result, len);
+	genlen = crypto_rng_generate(drng, addtl, addtl_len, result, len);
 	if (genlen < 0)
 		return genlen;
 
@@ -92,6 +104,63 @@ static int rng_recvmsg(struct socket *sock, struct msghdr *msg, size_t len,
 	return err ? err : len;
 }
 
+static int rng_recvmsg(struct socket *sock, struct msghdr *msg, size_t len,
+		       int flags)
+{
+	struct sock *sk = sock->sk;
+	struct alg_sock *ask = alg_sk(sk);
+	struct rng_ctx *ctx = ask->private;
+
+	return _rng_recvmsg(ctx->drng, msg, len, NULL, 0);
+}
+
+static int rng_test_recvmsg(struct socket *sock, struct msghdr *msg, size_t len,
+			    int flags)
+{
+	struct sock *sk = sock->sk;
+	struct alg_sock *ask = alg_sk(sk);
+	struct rng_ctx *ctx = ask->private;
+	int ret;
+
+	lock_sock(sock->sk);
+	ret = _rng_recvmsg(ctx->drng, msg, len, ctx->addtl, ctx->addtl_len);
+	rng_reset_addtl(ctx);
+	release_sock(sock->sk);
+
+	return ret;
+}
+
+static int rng_test_sendmsg(struct socket *sock, struct msghdr *msg, size_t len)
+{
+	int err;
+	struct alg_sock *ask = alg_sk(sock->sk);
+	struct rng_ctx *ctx = ask->private;
+
+	lock_sock(sock->sk);
+	if (len > MAXSIZE) {
+		err = -EMSGSIZE;
+		goto unlock;
+	}
+
+	rng_reset_addtl(ctx);
+	ctx->addtl = kmalloc(len, GFP_KERNEL);
+	if (!ctx->addtl) {
+		err = -ENOMEM;
+		goto unlock;
+	}
+
+	err = memcpy_from_msg(ctx->addtl, msg, len);
+	if (err) {
+		rng_reset_addtl(ctx);
+		goto unlock;
+	}
+	ctx->addtl_len = len;
+
+unlock:
+	release_sock(sock->sk);
+	return err ? err : len;
+}
+
 static struct proto_ops algif_rng_ops = {
 	.family		=	PF_ALG,
 
@@ -111,14 +180,53 @@ static struct proto_ops algif_rng_ops = {
 	.recvmsg	=	rng_recvmsg,
 };
 
+static struct proto_ops __maybe_unused algif_rng_test_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,
+	.mmap		=	sock_no_mmap,
+	.bind		=	sock_no_bind,
+	.accept		=	sock_no_accept,
+	.sendpage	=	sock_no_sendpage,
+
+	.release	=	af_alg_release,
+	.recvmsg	=	rng_test_recvmsg,
+	.sendmsg	=	rng_test_sendmsg,
+};
+
 static void *rng_bind(const char *name, u32 type, u32 mask)
 {
-	return crypto_alloc_rng(name, type, mask);
+	struct rng_parent_ctx *pctx;
+	struct crypto_rng *rng;
+
+	pctx = kzalloc(sizeof(*pctx), GFP_KERNEL);
+	if (!pctx)
+		return ERR_PTR(-ENOMEM);
+
+	rng = crypto_alloc_rng(name, type, mask);
+	if (IS_ERR(rng)) {
+		kfree(pctx);
+		return ERR_CAST(rng);
+	}
+
+	pctx->drng = rng;
+	return pctx;
 }
 
 static void rng_release(void *private)
 {
-	crypto_free_rng(private);
+	struct rng_parent_ctx *pctx = private;
+
+	if (unlikely(!pctx))
+		return;
+	crypto_free_rng(pctx->drng);
+	kfree_sensitive(pctx->entropy);
+	kfree_sensitive(pctx);
 }
 
 static void rng_sock_destruct(struct sock *sk)
@@ -126,6 +234,7 @@ static void rng_sock_destruct(struct sock *sk)
 	struct alg_sock *ask = alg_sk(sk);
 	struct rng_ctx *ctx = ask->private;
 
+	rng_reset_addtl(ctx);
 	sock_kfree_s(sk, ctx, ctx->len);
 	af_alg_release_parent(sk);
 }
@@ -133,6 +242,7 @@ static void rng_sock_destruct(struct sock *sk)
 static int rng_accept_parent(void *private, struct sock *sk)
 {
 	struct rng_ctx *ctx;
+	struct rng_parent_ctx *pctx = private;
 	struct alg_sock *ask = alg_sk(sk);
 	unsigned int len = sizeof(*ctx);
 
@@ -141,6 +251,8 @@ static int rng_accept_parent(void *private, struct sock *sk)
 		return -ENOMEM;
 
 	ctx->len = len;
+	ctx->addtl = NULL;
+	ctx->addtl_len = 0;
 
 	/*
 	 * No seeding done at that point -- if multiple accepts are
@@ -148,20 +260,58 @@ static int rng_accept_parent(void *private, struct sock *sk)
 	 * state of the RNG.
 	 */
 
-	ctx->drng = private;
+	ctx->drng = pctx->drng;
 	ask->private = ctx;
 	sk->sk_destruct = rng_sock_destruct;
 
+	/*
+	 * Non NULL pctx->entropy means that CAVP test has been initiated on
+	 * this socket, replace proto_ops algif_rng_ops with algif_rng_test_ops.
+	 */
+	if (pctx->entropy)
+		sk->sk_socket->ops = &algif_rng_test_ops;
+
 	return 0;
 }
 
 static int rng_setkey(void *private, const u8 *seed, unsigned int seedlen)
 {
+	struct rng_parent_ctx *pctx = private;
 	/*
 	 * Check whether seedlen is of sufficient size is done in RNG
 	 * implementations.
 	 */
-	return crypto_rng_reset(private, seed, seedlen);
+	return crypto_rng_reset(pctx->drng, seed, seedlen);
+}
+
+static int __maybe_unused rng_setentropy(void *private, sockptr_t entropy,
+					 unsigned int len)
+{
+	struct rng_parent_ctx *pctx = private;
+	u8 *kentropy = NULL;
+
+	if (!capable(CAP_SYS_ADMIN))
+		return -EACCES;
+
+	if (pctx->entropy)
+		return -EINVAL;
+
+	if (len > MAXSIZE)
+		return -EMSGSIZE;
+
+	if (len) {
+		kentropy = memdup_sockptr(entropy, len);
+		if (IS_ERR(kentropy))
+			return PTR_ERR(kentropy);
+	}
+
+	crypto_rng_alg(pctx->drng)->set_ent(pctx->drng, kentropy, len);
+	/*
+	 * Since rng doesn't perform any memory management for the entropy
+	 * buffer, save kentropy pointer to pctx now to free it after use.
+	 */
+	pctx->entropy = kentropy;
+	return 0;
 }
 
 static const struct af_alg_type algif_type_rng = {
@@ -169,6 +319,9 @@ static const struct af_alg_type algif_type_rng = {
 	.release	=	rng_release,
 	.accept		=	rng_accept_parent,
 	.setkey		=	rng_setkey,
+#ifdef CONFIG_CRYPTO_USER_API_RNG_CAVP
+	.setentropy	=	rng_setentropy,
+#endif
 	.ops		=	&algif_rng_ops,
 	.name		=	"rng",
 	.owner		=	THIS_MODULE
diff --git a/include/crypto/if_alg.h b/include/crypto/if_alg.h
index ee6412314f8f..a5db86670bdf 100644
--- a/include/crypto/if_alg.h
+++ b/include/crypto/if_alg.h
@@ -46,6 +46,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 (*setentropy)(void *private, sockptr_t entropy, unsigned int len);
 	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 bc2bcdec377b..60b7c2efd921 100644
--- a/include/uapi/linux/if_alg.h
+++ b/include/uapi/linux/if_alg.h
@@ -35,6 +35,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_DRBG_ENTROPY		6
 
 /* Operations */
 #define ALG_OP_DECRYPT			0
-- 
2.28.0.526.ge36021eeef-goog

Re: [PATCH v8] crypto: af_alg - add extra parameters for DRBG interface

[Herbert Xu]

On Wed, Sep 16, 2020 at 12:07:31PM +0100, Elena Petrova wrote:
>
> @@ -148,20 +260,58 @@ static int rng_accept_parent(void *private, struct sock *sk)
>  	 * state of the RNG.
>  	 */
>  
> -	ctx->drng = private;
> +	ctx->drng = pctx->drng;
>  	ask->private = ctx;
>  	sk->sk_destruct = rng_sock_destruct;
>  
> +	/*
> +	 * Non NULL pctx->entropy means that CAVP test has been initiated on
> +	 * this socket, replace proto_ops algif_rng_ops with algif_rng_test_ops.
> +	 */
> +	if (pctx->entropy)
> +		sk->sk_socket->ops = &algif_rng_test_ops;
> +

Please make that

	if (IS_ENABLED(CONFIG_CRYPTO_USER_API_RNG_CAVP) && pctx->entropy)

so that this and the rest of the new code simply disappears when
the Kconfig option is off.

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

[PATCH v9] crypto: af_alg - add extra parameters for DRBG interface

[Elena Petrova]

Extend the user-space RNG interface:
  1. Add entropy input via ALG_SET_DRBG_ENTROPY setsockopt option;
  2. Add additional data input via sendmsg syscall.

This allows DRBG to be tested with test vectors, for example for the
purpose of CAVP testing, which otherwise isn't possible.

To prevent erroneous use of entropy input, it is hidden under
CRYPTO_USER_API_RNG_CAVP config option and requires CAP_SYS_ADMIN to
succeed.

Signed-off-by: Elena Petrova <lenaptr@google.com>
Acked-by: Stephan Müller <smueller@chronox.de>
Reviewed-by: Eric Biggers <ebiggers@google.com>
---

Updates in v9:
  Add IS_ENABLED(CONFIG_CRYPTO_USER_API_RNG_CAVP) condition for replacing
  proto_ops.

Updates in v8:
  Added Reviewed-by tag to the description.

Updates in v7:
  1) rebased onto the latest at cryptodev-2.6.git, fixed compiler errors;
  2) replaced kzfree with kfree_sensitive;
  3) changed rng_test_sendmsg to return an error if len > MAXSIZE;
  4) updated documentation to say when can Additional Data be provided.

Updates in v6:
  1) Kconfig option renamed to CRYPTO_USER_API_RNG_CAVP and is now bool instead
     of tristate;
  2) run-time switch of proto_ops depending on whether the entropy was set;
  3) corrected the NIST standard name;
  4) rebased onto the tip of the tree;
  5) documentation clarified;

Updates in v5:
  1) use __maybe_unused instead of #ifdef;
  2) separate code path for a testing mode;
  3) only allow Additional Data input in a testing mode.

Updates in v4:
  1) setentropy returns 0 or error code (used to return length);
  2) bigfixes suggested by Eric.

Updates in v3:
  1) More details in commit message;
  2) config option name is now CRYPTO_USER_API_CAVP_DRBG;
  3) fixed a bug of not releasing socket locks.

Updates in v2:
  1) Adding CONFIG_CRYPTO_CAVS_DRBG around setentropy.
  2) Requiring CAP_SYS_ADMIN for entropy reset.
  3) Locking for send and recv.
  4) Length checks added for send and setentropy; send and setentropy now return
     number of bytes accepted.
  5) Minor code style corrections.

libkcapi patch for testing:
  https://github.com/Len0k/libkcapi/commit/6f095d270b982008f419078614c15caa592cb531

 Documentation/crypto/userspace-if.rst |  20 ++-
 crypto/Kconfig                        |   9 ++
 crypto/af_alg.c                       |  14 ++-
 crypto/algif_rng.c                    | 175 ++++++++++++++++++++++++--
 include/crypto/if_alg.h               |   1 +
 include/uapi/linux/if_alg.h           |   1 +
 6 files changed, 205 insertions(+), 15 deletions(-)

diff --git a/Documentation/crypto/userspace-if.rst b/Documentation/crypto/userspace-if.rst
index 52019e905900..b45dabbf69d6 100644
--- a/Documentation/crypto/userspace-if.rst
+++ b/Documentation/crypto/userspace-if.rst
@@ -296,15 +296,16 @@ follows:
 
     struct sockaddr_alg sa = {
         .salg_family = AF_ALG,
-        .salg_type = "rng", /* this selects the symmetric cipher */
-        .salg_name = "drbg_nopr_sha256" /* this is the cipher name */
+        .salg_type = "rng", /* this selects the random number generator */
+        .salg_name = "drbg_nopr_sha256" /* this is the RNG name */
     };
 
 
 Depending on the RNG type, the RNG must be seeded. The seed is provided
 using the setsockopt interface to set the key. For example, the
 ansi_cprng requires a seed. The DRBGs do not require a seed, but may be
-seeded.
+seeded. The seed is also known as a *Personalization String* in NIST SP 800-90A
+standard.
 
 Using the read()/recvmsg() system calls, random numbers can be obtained.
 The kernel generates at most 128 bytes in one call. If user space
@@ -314,6 +315,16 @@ WARNING: The user space caller may invoke the initially mentioned accept
 system call multiple times. In this case, the returned file descriptors
 have the same state.
 
+Following CAVP testing interfaces are enabled when kernel is built with
+CRYPTO_USER_API_RNG_CAVP option:
+
+-  the concatenation of *Entropy* and *Nonce* can be provided to the RNG via
+   ALG_SET_DRBG_ENTROPY setsockopt interface. Setting the entropy requires
+   CAP_SYS_ADMIN permission.
+
+-  *Additional Data* can be provided using the send()/sendmsg() system calls,
+   but only after the entropy has been set.
+
 Zero-Copy Interface
 -------------------
 
@@ -377,6 +388,9 @@ mentioned optname:
    provided ciphertext is assumed to contain an authentication tag of
    the given size (see section about AEAD memory layout below).
 
+-  ALG_SET_DRBG_ENTROPY -- Setting the entropy of the random number generator.
+   This option is applicable to RNG cipher type only.
+
 User space API example
 ----------------------
 
diff --git a/crypto/Kconfig b/crypto/Kconfig
index 1b57419fa2e7..070a88ec1ba8 100644
--- a/crypto/Kconfig
+++ b/crypto/Kconfig
@@ -1870,6 +1870,15 @@ config CRYPTO_USER_API_RNG
 	  This option enables the user-spaces interface for random
 	  number generator algorithms.
 
+config CRYPTO_USER_API_RNG_CAVP
+	bool "Enable CAVP testing of DRBG"
+	depends on CRYPTO_USER_API_RNG && CRYPTO_DRBG
+	help
+	  This option enables extra API for CAVP testing via the user-space
+	  interface: resetting of DRBG entropy, and providing Additional Data.
+	  This should only be enabled for CAVP testing. You should say
+	  no unless you know what this is.
+
 config CRYPTO_USER_API_AEAD
 	tristate "User-space interface for AEAD cipher algorithms"
 	depends on NET
diff --git a/crypto/af_alg.c b/crypto/af_alg.c
index a6f581ab200c..8535cb03b484 100644
--- a/crypto/af_alg.c
+++ b/crypto/af_alg.c
@@ -253,6 +253,14 @@ static int alg_setsockopt(struct socket *sock, int level, int optname,
 		if (!type->setauthsize)
 			goto unlock;
 		err = type->setauthsize(ask->private, optlen);
+		break;
+	case ALG_SET_DRBG_ENTROPY:
+		if (sock->state == SS_CONNECTED)
+			goto unlock;
+		if (!type->setentropy)
+			goto unlock;
+
+		err = type->setentropy(ask->private, optval, optlen);
 	}
 
 unlock:
@@ -285,6 +293,11 @@ int af_alg_accept(struct sock *sk, struct socket *newsock, bool kern)
 	security_sock_graft(sk2, newsock);
 	security_sk_clone(sk, sk2);
 
+	/*
+	 * newsock->ops assigned here to allow type->accept call to override
+	 * them when required.
+	 */
+	newsock->ops = type->ops;
 	err = type->accept(ask->private, sk2);
 
 	nokey = err == -ENOKEY;
@@ -303,7 +316,6 @@ int af_alg_accept(struct sock *sk, struct socket *newsock, bool kern)
 	alg_sk(sk2)->parent = sk;
 	alg_sk(sk2)->type = type;
 
-	newsock->ops = type->ops;
 	newsock->state = SS_CONNECTED;
 
 	if (nokey)
diff --git a/crypto/algif_rng.c b/crypto/algif_rng.c
index 6300e0566dc5..407408c43730 100644
--- a/crypto/algif_rng.c
+++ b/crypto/algif_rng.c
@@ -38,6 +38,7 @@
  * DAMAGE.
  */
 
+#include <linux/capability.h>
 #include <linux/module.h>
 #include <crypto/rng.h>
 #include <linux/random.h>
@@ -53,15 +54,26 @@ struct rng_ctx {
 #define MAXSIZE 128
 	unsigned int len;
 	struct crypto_rng *drng;
+	u8 *addtl;
+	size_t addtl_len;
 };
 
-static int rng_recvmsg(struct socket *sock, struct msghdr *msg, size_t len,
-		       int flags)
+struct rng_parent_ctx {
+	struct crypto_rng *drng;
+	u8 *entropy;
+};
+
+static void rng_reset_addtl(struct rng_ctx *ctx)
 {
-	struct sock *sk = sock->sk;
-	struct alg_sock *ask = alg_sk(sk);
-	struct rng_ctx *ctx = ask->private;
-	int err;
+	kfree_sensitive(ctx->addtl);
+	ctx->addtl = NULL;
+	ctx->addtl_len = 0;
+}
+
+static int _rng_recvmsg(struct crypto_rng *drng, struct msghdr *msg, size_t len,
+			u8 *addtl, size_t addtl_len)
+{
+	int err = 0;
 	int genlen = 0;
 	u8 result[MAXSIZE];
 
@@ -82,7 +94,7 @@ static int rng_recvmsg(struct socket *sock, struct msghdr *msg, size_t len,
 	 * seeding as they automatically seed. The X9.31 DRNG will return
 	 * an error if it was not seeded properly.
 	 */
-	genlen = crypto_rng_get_bytes(ctx->drng, result, len);
+	genlen = crypto_rng_generate(drng, addtl, addtl_len, result, len);
 	if (genlen < 0)
 		return genlen;
 
@@ -92,6 +104,63 @@ static int rng_recvmsg(struct socket *sock, struct msghdr *msg, size_t len,
 	return err ? err : len;
 }
 
+static int rng_recvmsg(struct socket *sock, struct msghdr *msg, size_t len,
+		       int flags)
+{
+	struct sock *sk = sock->sk;
+	struct alg_sock *ask = alg_sk(sk);
+	struct rng_ctx *ctx = ask->private;
+
+	return _rng_recvmsg(ctx->drng, msg, len, NULL, 0);
+}
+
+static int rng_test_recvmsg(struct socket *sock, struct msghdr *msg, size_t len,
+			    int flags)
+{
+	struct sock *sk = sock->sk;
+	struct alg_sock *ask = alg_sk(sk);
+	struct rng_ctx *ctx = ask->private;
+	int ret;
+
+	lock_sock(sock->sk);
+	ret = _rng_recvmsg(ctx->drng, msg, len, ctx->addtl, ctx->addtl_len);
+	rng_reset_addtl(ctx);
+	release_sock(sock->sk);
+
+	return ret;
+}
+
+static int rng_test_sendmsg(struct socket *sock, struct msghdr *msg, size_t len)
+{
+	int err;
+	struct alg_sock *ask = alg_sk(sock->sk);
+	struct rng_ctx *ctx = ask->private;
+
+	lock_sock(sock->sk);
+	if (len > MAXSIZE) {
+		err = -EMSGSIZE;
+		goto unlock;
+	}
+
+	rng_reset_addtl(ctx);
+	ctx->addtl = kmalloc(len, GFP_KERNEL);
+	if (!ctx->addtl) {
+		err = -ENOMEM;
+		goto unlock;
+	}
+
+	err = memcpy_from_msg(ctx->addtl, msg, len);
+	if (err) {
+		rng_reset_addtl(ctx);
+		goto unlock;
+	}
+	ctx->addtl_len = len;
+
+unlock:
+	release_sock(sock->sk);
+	return err ? err : len;
+}
+
 static struct proto_ops algif_rng_ops = {
 	.family		=	PF_ALG,
 
@@ -111,14 +180,53 @@ static struct proto_ops algif_rng_ops = {
 	.recvmsg	=	rng_recvmsg,
 };
 
+static struct proto_ops __maybe_unused algif_rng_test_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,
+	.mmap		=	sock_no_mmap,
+	.bind		=	sock_no_bind,
+	.accept		=	sock_no_accept,
+	.sendpage	=	sock_no_sendpage,
+
+	.release	=	af_alg_release,
+	.recvmsg	=	rng_test_recvmsg,
+	.sendmsg	=	rng_test_sendmsg,
+};
+
 static void *rng_bind(const char *name, u32 type, u32 mask)
 {
-	return crypto_alloc_rng(name, type, mask);
+	struct rng_parent_ctx *pctx;
+	struct crypto_rng *rng;
+
+	pctx = kzalloc(sizeof(*pctx), GFP_KERNEL);
+	if (!pctx)
+		return ERR_PTR(-ENOMEM);
+
+	rng = crypto_alloc_rng(name, type, mask);
+	if (IS_ERR(rng)) {
+		kfree(pctx);
+		return ERR_CAST(rng);
+	}
+
+	pctx->drng = rng;
+	return pctx;
 }
 
 static void rng_release(void *private)
 {
-	crypto_free_rng(private);
+	struct rng_parent_ctx *pctx = private;
+
+	if (unlikely(!pctx))
+		return;
+	crypto_free_rng(pctx->drng);
+	kfree_sensitive(pctx->entropy);
+	kfree_sensitive(pctx);
 }
 
 static void rng_sock_destruct(struct sock *sk)
@@ -126,6 +234,7 @@ static void rng_sock_destruct(struct sock *sk)
 	struct alg_sock *ask = alg_sk(sk);
 	struct rng_ctx *ctx = ask->private;
 
+	rng_reset_addtl(ctx);
 	sock_kfree_s(sk, ctx, ctx->len);
 	af_alg_release_parent(sk);
 }
@@ -133,6 +242,7 @@ static void rng_sock_destruct(struct sock *sk)
 static int rng_accept_parent(void *private, struct sock *sk)
 {
 	struct rng_ctx *ctx;
+	struct rng_parent_ctx *pctx = private;
 	struct alg_sock *ask = alg_sk(sk);
 	unsigned int len = sizeof(*ctx);
 
@@ -141,6 +251,8 @@ static int rng_accept_parent(void *private, struct sock *sk)
 		return -ENOMEM;
 
 	ctx->len = len;
+	ctx->addtl = NULL;
+	ctx->addtl_len = 0;
 
 	/*
 	 * No seeding done at that point -- if multiple accepts are
@@ -148,20 +260,58 @@ static int rng_accept_parent(void *private, struct sock *sk)
 	 * state of the RNG.
 	 */
 
-	ctx->drng = private;
+	ctx->drng = pctx->drng;
 	ask->private = ctx;
 	sk->sk_destruct = rng_sock_destruct;
 
+	/*
+	 * Non NULL pctx->entropy means that CAVP test has been initiated on
+	 * this socket, replace proto_ops algif_rng_ops with algif_rng_test_ops.
+	 */
+	if (IS_ENABLED(CONFIG_CRYPTO_USER_API_RNG_CAVP) && pctx->entropy)
+		sk->sk_socket->ops = &algif_rng_test_ops;
+
 	return 0;
 }
 
 static int rng_setkey(void *private, const u8 *seed, unsigned int seedlen)
 {
+	struct rng_parent_ctx *pctx = private;
 	/*
 	 * Check whether seedlen is of sufficient size is done in RNG
 	 * implementations.
 	 */
-	return crypto_rng_reset(private, seed, seedlen);
+	return crypto_rng_reset(pctx->drng, seed, seedlen);
+}
+
+static int __maybe_unused rng_setentropy(void *private, sockptr_t entropy,
+					 unsigned int len)
+{
+	struct rng_parent_ctx *pctx = private;
+	u8 *kentropy = NULL;
+
+	if (!capable(CAP_SYS_ADMIN))
+		return -EACCES;
+
+	if (pctx->entropy)
+		return -EINVAL;
+
+	if (len > MAXSIZE)
+		return -EMSGSIZE;
+
+	if (len) {
+		kentropy = memdup_sockptr(entropy, len);
+		if (IS_ERR(kentropy))
+			return PTR_ERR(kentropy);
+	}
+
+	crypto_rng_alg(pctx->drng)->set_ent(pctx->drng, kentropy, len);
+	/*
+	 * Since rng doesn't perform any memory management for the entropy
+	 * buffer, save kentropy pointer to pctx now to free it after use.
+	 */
+	pctx->entropy = kentropy;
+	return 0;
 }
 
 static const struct af_alg_type algif_type_rng = {
@@ -169,6 +319,9 @@ static const struct af_alg_type algif_type_rng = {
 	.release	=	rng_release,
 	.accept		=	rng_accept_parent,
 	.setkey		=	rng_setkey,
+#ifdef CONFIG_CRYPTO_USER_API_RNG_CAVP
+	.setentropy	=	rng_setentropy,
+#endif
 	.ops		=	&algif_rng_ops,
 	.name		=	"rng",
 	.owner		=	THIS_MODULE
diff --git a/include/crypto/if_alg.h b/include/crypto/if_alg.h
index ee6412314f8f..a5db86670bdf 100644
--- a/include/crypto/if_alg.h
+++ b/include/crypto/if_alg.h
@@ -46,6 +46,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 (*setentropy)(void *private, sockptr_t entropy, unsigned int len);
 	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 bc2bcdec377b..60b7c2efd921 100644
--- a/include/uapi/linux/if_alg.h
+++ b/include/uapi/linux/if_alg.h
@@ -35,6 +35,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_DRBG_ENTROPY		6
 
 /* Operations */
 #define ALG_OP_DECRYPT			0
-- 
2.28.0.681.g6f77f65b4e-goog

Re: [PATCH v9] crypto: af_alg - add extra parameters for DRBG interface

[Herbert Xu]

On Fri, Sep 18, 2020 at 04:42:16PM +0100, Elena Petrova wrote:
> Extend the user-space RNG interface:
>   1. Add entropy input via ALG_SET_DRBG_ENTROPY setsockopt option;
>   2. Add additional data input via sendmsg syscall.
> 
> This allows DRBG to be tested with test vectors, for example for the
> purpose of CAVP testing, which otherwise isn't possible.
> 
> To prevent erroneous use of entropy input, it is hidden under
> CRYPTO_USER_API_RNG_CAVP config option and requires CAP_SYS_ADMIN to
> succeed.
> 
> Signed-off-by: Elena Petrova <lenaptr@google.com>
> Acked-by: Stephan Müller <smueller@chronox.de>
> Reviewed-by: Eric Biggers <ebiggers@google.com>
> ---
> 
> Updates in v9:
>   Add IS_ENABLED(CONFIG_CRYPTO_USER_API_RNG_CAVP) condition for replacing
>   proto_ops.
> 
> Updates in v8:
>   Added Reviewed-by tag to the description.
> 
> Updates in v7:
>   1) rebased onto the latest at cryptodev-2.6.git, fixed compiler errors;
>   2) replaced kzfree with kfree_sensitive;
>   3) changed rng_test_sendmsg to return an error if len > MAXSIZE;
>   4) updated documentation to say when can Additional Data be provided.
> 
> Updates in v6:
>   1) Kconfig option renamed to CRYPTO_USER_API_RNG_CAVP and is now bool instead
>      of tristate;
>   2) run-time switch of proto_ops depending on whether the entropy was set;
>   3) corrected the NIST standard name;
>   4) rebased onto the tip of the tree;
>   5) documentation clarified;
> 
> Updates in v5:
>   1) use __maybe_unused instead of #ifdef;
>   2) separate code path for a testing mode;
>   3) only allow Additional Data input in a testing mode.
> 
> Updates in v4:
>   1) setentropy returns 0 or error code (used to return length);
>   2) bigfixes suggested by Eric.
> 
> Updates in v3:
>   1) More details in commit message;
>   2) config option name is now CRYPTO_USER_API_CAVP_DRBG;
>   3) fixed a bug of not releasing socket locks.
> 
> Updates in v2:
>   1) Adding CONFIG_CRYPTO_CAVS_DRBG around setentropy.
>   2) Requiring CAP_SYS_ADMIN for entropy reset.
>   3) Locking for send and recv.
>   4) Length checks added for send and setentropy; send and setentropy now return
>      number of bytes accepted.
>   5) Minor code style corrections.
> 
> libkcapi patch for testing:
>   https://github.com/Len0k/libkcapi/commit/6f095d270b982008f419078614c15caa592cb531
> 
>  Documentation/crypto/userspace-if.rst |  20 ++-
>  crypto/Kconfig                        |   9 ++
>  crypto/af_alg.c                       |  14 ++-
>  crypto/algif_rng.c                    | 175 ++++++++++++++++++++++++--
>  include/crypto/if_alg.h               |   1 +
>  include/uapi/linux/if_alg.h           |   1 +
>  6 files changed, 205 insertions(+), 15 deletions(-)

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

Re: [PATCH v5] crypto: af_alg - add extra parameters for DRBG interface

[Elena Petrova]

On Fri, 21 Aug 2020 at 05:24, Herbert Xu <herbert@gondor.apana.org.au> wrote:
>
> Eric Biggers <ebiggers@kernel.org> wrote:
> >
> > Since proto_ops are almost identical, and only one is used in a given kernel
> > build, why not just do:
> >
> > static struct proto_ops algif_rng_ops = {
> >        ...
> > #ifdef CONFIG_CRYPTO_USER_API_RNG_CAVP
> >        .sendmsg        = rng_sendmsg,
> > #else
> >        .sendmsg        = sock_no_sendmsg,
> > #endif
> >        ...
> > };
> >
> > Similarly for .recvmsg(), although I don't understand what's wrong with just
> > adding the lock_sock() instead...  The RNG algorithms do locking anyway, so it's
> > not like that would regress the ability to recvmsg() in parallel.  Also,
> > conditional locking depending on the kernel config makes it more difficult to
> > find kernel bugs like deadlocks.
>
> I want this to have minimal impact on anyone who's not using it.
> After all, this is something that only Google is asking for.
>
> Anyway, I wasn't looking for a compile-time ops switch, but a
> run-time one.
>
> I think what we can do is move the new newsock->ops assignment
> in af_alg_accept up above the type->accept call which would then
> allow it to be overridden by the accept call.
>
> After that you could just change newsock->ops depending on whether
> pctx->entropy is NULL or not in rng_accept_parent.

Ack, done in v6.

> As for the proto_ops duplication I don't think it's that big a
> deal, but if you're really bothered just create a macro for the
> identical bits in the struct.

I didn't create a macro to avoid complicating the code.

Thanks,
Elena

Re: [PATCH v5] crypto: af_alg - add extra parameters for DRBG interface

[Elena Petrova]

On Thu, 13 Aug 2020 at 20:32, Eric Biggers <ebiggers@kernel.org> wrote:

> >  Depending on the RNG type, the RNG must be seeded. The seed is provided
> >  using the setsockopt interface to set the key. For example, the
> >  ansi_cprng requires a seed. The DRBGs do not require a seed, but may be
> > -seeded.
> > +seeded. The seed is also known as a *Personalization String* in DRBG800-90A
> > +standard.
>
> Isn't the standard called "NIST SP 800-90A"?
> "DRBG800-90A" doesn't return many hits on Google.

Fixed, thanks!

> > +For the purpose of CAVP testing, the concatenation of *Entropy* and *Nonce*
> > +can be provided to the RNG via ALG_SET_DRBG_ENTROPY setsockopt interface. This
> > +requires a kernel built with CONFIG_CRYPTO_USER_API_CAVP_DRBG, and
> > +CAP_SYS_ADMIN permission.
> > +
> > +*Additional Data* can be provided using the send()/sendmsg() system calls.
>
> This doesn't make it clear whether the support for "Additional Data" is
> dependent on CONFIG_CRYPTO_USER_API_CAVP_DRBG or not.

Fixed.

> > +config CRYPTO_USER_API_CAVP_DRBG
> > +     tristate "Enable CAVP testing of DRBG"
> > +     depends on CRYPTO_USER_API_RNG && CRYPTO_DRBG
> > +     help
> > +       This option enables extra API for CAVP testing via the user-space
> > +       interface: resetting of DRBG entropy, and providing Additional Data.
> > +       This should only be enabled for CAVP testing. You should say
> > +       no unless you know what this is.
>
> Using "tristate" here is incorrect because this option is not a module itself.
> It's an option *for* a module.  So it needs to be "bool" instead.

Done.

> Also, since this is an option to refine CRYPTO_USER_API_RNG, how about renaming
> it to "CRYPTO_USER_API_RNG_CAVP", and moving it to just below the definition of
> "CRYPTO_USER_API_RNG" so that they show up adjacent to each other?

Sounds good, done.

> > +static int rng_test_recvmsg(struct socket *sock, struct msghdr *msg, size_t len,
> > +                         int flags)
> > +{
> > +     struct sock *sk = sock->sk;
> > +     struct alg_sock *ask = alg_sk(sk);
> > +     struct rng_ctx *ctx = ask->private;
> > +     int err;
> > +
> > +     lock_sock(sock->sk);
> > +     err = _rng_recvmsg(ctx->drng, msg, len, ctx->addtl, ctx->addtl_len);
> > +     rng_reset_addtl(ctx);
> > +     release_sock(sock->sk);
> > +
> > +     return err ? err : len;
>
> Shouldn't this just return the value that _rng_recvmsg() returned?
>
> Also 'err' is conventionally just for 0 or -errno codes.  Use 'ret' if the
> variable can also hold a length.

Done.

Re: [PATCH 0/1] crypto: af_alg - add extra parameters for DRBG interface

[Stephan Mueller]

Am Montag, 13. Juli 2020, 18:48:56 CEST schrieb Elena Petrova:

Hi Elena,

> This patch extends the userspace RNG interface to make it usable for
> CAVS testing. This is achieved by adding ALG_SET_DRBG_ENTROPY
> option to the setsockopt interface for specifying the entropy, and using
> sendmsg syscall for specifying the additional data.
> 
> See libkcapi patch [1] to test the added functionality. The libkcapi
> patch is not intended for merging into libkcapi as is: it is only a
> quick plug to manually verify that the extended AF_ALG RNG interface
> generates the expected output on DRBG800-90A CAVS inputs.

As I am responsible for developing such CAVS/ACVP harness as well, I played 
with the idea of going through AF_ALG. I discarded it because I do not see the 
benefit why we should add an interface solely for the purpose of testing. 
Further, it is a potentially dangerous one because the created instance of the 
DRBG is "seeded" from data provided by the caller.

Thus, I do not see the benefit from adding that extension, widening a user 
space interface solely for the purpose of CAVS testing. I would not see any 
other benefit we have with this extension. In particular, this interface would 
then be always there. What I could live with is an interface that can be 
enabled at compile time for those who want it.

Besides, when you want to do CAVS testing, the following ciphers are still not 
testable and thus this patch would only be a partial solution to get the 
testing covered:

- AES KW (you cannot get the final IV out of the kernel - I played with the 
idea to return the IV through AF_ALG, but discarded it because of the concern 
above)

- OFB/CFB MCT testing (you need the IV from the last round - same issue as for 
AES KW)

- RSA

- DH

- ECDH

With these issues, I would assume you are better off creating your own kernel 
module just like I did that externalizes the crypto API to user space but is 
only available on your test kernel and will not affect all other users.

Ciao
Stephan

Re: [PATCH 0/1] crypto: af_alg - add extra parameters for DRBG interface

[Elena Petrova]

Hi Stephan,

On Tue, 14 Jul 2020 at 06:17, Stephan Mueller <smueller@chronox.de> wrote:
>
> Am Montag, 13. Juli 2020, 18:48:56 CEST schrieb Elena Petrova:
>
> Hi Elena,
>
> > This patch extends the userspace RNG interface to make it usable for
> > CAVS testing. This is achieved by adding ALG_SET_DRBG_ENTROPY
> > option to the setsockopt interface for specifying the entropy, and using
> > sendmsg syscall for specifying the additional data.
> >
> > See libkcapi patch [1] to test the added functionality. The libkcapi
> > patch is not intended for merging into libkcapi as is: it is only a
> > quick plug to manually verify that the extended AF_ALG RNG interface
> > generates the expected output on DRBG800-90A CAVS inputs.
>
> As I am responsible for developing such CAVS/ACVP harness as well, I played
> with the idea of going through AF_ALG. I discarded it because I do not see the
> benefit why we should add an interface solely for the purpose of testing.
> Further, it is a potentially dangerous one because the created instance of the
> DRBG is "seeded" from data provided by the caller.
>
> Thus, I do not see the benefit from adding that extension, widening a user
> space interface solely for the purpose of CAVS testing. I would not see any
> other benefit we have with this extension. In particular, this interface would
> then be always there. What I could live with is an interface that can be
> enabled at compile time for those who want it.

Thanks for reviewing this patch. I understand your concern about the erroneous
use of the entropy input by non-testing applications. This was an approach that
I had discussed with Ard. I should have included you, my apologies. I'll  post
v2 with the CAVS testing stuff hidden under CONFIG_ option with appropriate help
text.

With regards to the usefulness, let me elaborate. This effort of extending the
drbg interface is driven by Android needs for having the kernel crypto
certified. I started from having an out-of-tree chrdev driver for Google Pixel
kernels that was exposing the required crypto functionality, and it wasn't ideal
in the following ways:
 * it primarily consisted of copypasted code from testmgr.c
 * it was hard for me to keep the code up to date because I'm not aware of
   ongoing changes to crypto
 * it is hard for other people and/or organisations to re-use it, hense a lot of
   duplicated effort is going into CAVS: you have a private driver, I have mine,
   Jaya from HP <jayalakshmi.bhat@hp.com>, who's been asking linux-crypto a few
   CAVS questions, has to develop theirs...

In general Android is trying to eliminate out-of-tree code. CAVS testing
functionality in particular needs to be upstream because we are switching all
Android devices to using a Generic Kernel Image (GKI)
[https://lwn.net/Articles/771974/] based on the upstream kernel.

> Besides, when you want to do CAVS testing, the following ciphers are still not
> testable and thus this patch would only be a partial solution to get the
> testing covered:
>
> - AES KW (you cannot get the final IV out of the kernel - I played with the
> idea to return the IV through AF_ALG, but discarded it because of the concern
> above)
>
> - OFB/CFB MCT testing (you need the IV from the last round - same issue as for
> AES KW)
>
> - RSA
>
> - DH
>
> - ECDH

For Android certification purposes, we only need to test the modules which are
actually being used. In our case it's what af_alg already exposes plus DRBG.
If, say, HP would need RSA, they could submit their own patch.

As for exposing bits of the internal state for some algorithms, I hope guarding
the testing functionality with a CONFIG_ option would solve the security part of
the problem.

> With these issues, I would assume you are better off creating your own kernel
> module just like I did that externalizes the crypto API to user space but is
> only available on your test kernel and will not affect all other users.

I considered publishing my kernel driver on GitHub, but there appears to be a
sufficiently large number of users to justify having this functionality
upstream.

Hope that addresses your concerns.

> Ciao
> Stephan

Thanks,
Elena

Re: [PATCH 0/1] crypto: af_alg - add extra parameters for DRBG interface

[Stephan Mueller]

Am Dienstag, 14. Juli 2020, 17:23:05 CEST schrieb Elena Petrova:

Hi Elena,

> Hi Stephan,
> 
> On Tue, 14 Jul 2020 at 06:17, Stephan Mueller <smueller@chronox.de> wrote:
> > Am Montag, 13. Juli 2020, 18:48:56 CEST schrieb Elena Petrova:
> > 
> > Hi Elena,
> > 
> > > This patch extends the userspace RNG interface to make it usable for
> > > CAVS testing. This is achieved by adding ALG_SET_DRBG_ENTROPY
> > > option to the setsockopt interface for specifying the entropy, and using
> > > sendmsg syscall for specifying the additional data.
> > > 
> > > See libkcapi patch [1] to test the added functionality. The libkcapi
> > > patch is not intended for merging into libkcapi as is: it is only a
> > > quick plug to manually verify that the extended AF_ALG RNG interface
> > > generates the expected output on DRBG800-90A CAVS inputs.
> > 
> > As I am responsible for developing such CAVS/ACVP harness as well, I
> > played
> > with the idea of going through AF_ALG. I discarded it because I do not see
> > the benefit why we should add an interface solely for the purpose of
> > testing. Further, it is a potentially dangerous one because the created
> > instance of the DRBG is "seeded" from data provided by the caller.
> > 
> > Thus, I do not see the benefit from adding that extension, widening a user
> > space interface solely for the purpose of CAVS testing. I would not see
> > any
> > other benefit we have with this extension. In particular, this interface
> > would then be always there. What I could live with is an interface that
> > can be enabled at compile time for those who want it.
> 
> Thanks for reviewing this patch. I understand your concern about the
> erroneous use of the entropy input by non-testing applications. This was an
> approach that I had discussed with Ard. I should have included you, my
> apologies. I'll  post v2 with the CAVS testing stuff hidden under CONFIG_
> option with appropriate help text.
> 
> With regards to the usefulness, let me elaborate. This effort of extending
> the drbg interface is driven by Android needs for having the kernel crypto
> certified. I started from having an out-of-tree chrdev driver for Google
> Pixel kernels that was exposing the required crypto functionality, and it
> wasn't ideal in the following ways:
>  * it primarily consisted of copypasted code from testmgr.c
>  * it was hard for me to keep the code up to date because I'm not aware of
>    ongoing changes to crypto
>  * it is hard for other people and/or organisations to re-use it, hense a
> lot of duplicated effort is going into CAVS: you have a private driver, I
> have mine, Jaya from HP <jayalakshmi.bhat@hp.com>, who's been asking
> linux-crypto a few CAVS questions, has to develop theirs...
> 
> In general Android is trying to eliminate out-of-tree code. CAVS testing
> functionality in particular needs to be upstream because we are switching
> all Android devices to using a Generic Kernel Image (GKI)
> [https://lwn.net/Articles/771974/] based on the upstream kernel.

Thank you for the explanation.
> 
> > Besides, when you want to do CAVS testing, the following ciphers are still
> > not testable and thus this patch would only be a partial solution to get
> > the testing covered:
> > 
> > - AES KW (you cannot get the final IV out of the kernel - I played with
> > the
> > idea to return the IV through AF_ALG, but discarded it because of the
> > concern above)
> > 
> > - OFB/CFB MCT testing (you need the IV from the last round - same issue as
> > for AES KW)
> > 
> > - RSA
> > 
> > - DH
> > 
> > - ECDH
> 
> For Android certification purposes, we only need to test the modules which
> are actually being used. In our case it's what af_alg already exposes plus
> DRBG. If, say, HP would need RSA, they could submit their own patch.
> 
> As for exposing bits of the internal state for some algorithms, I hope
> guarding the testing functionality with a CONFIG_ option would solve the
> security part of the problem.

Yes, for all other users.

But if you are planning to enable this option for all Android devices across 
the board I am not sure here. In this case, wouldn't it make sense to require 
capable(CAP_SYS_ADMIN) for the DRBG reset operation just as an additional 
precaution? Note, the issue with the reset is that you loose all previous 
state (which is good for testing, but bad for security as I guess you agree 
:-) ).
> 
> > With these issues, I would assume you are better off creating your own
> > kernel module just like I did that externalizes the crypto API to user
> > space but is only available on your test kernel and will not affect all
> > other users.
> I considered publishing my kernel driver on GitHub, but there appears to be
> a sufficiently large number of users to justify having this functionality
> upstream.

So, I should then dust off my AF_ALG KPP and AF_ALG akcipher patches then? :-D
> 
> Hope that addresses your concerns.
> 
> > Ciao
> > Stephan
> 
> Thanks,
> Elena


Ciao
Stephan

Re: [PATCH 0/1] crypto: af_alg - add extra parameters for DRBG interface

[Elena Petrova]

Hi Stephan,

On Tue, 14 Jul 2020 at 16:34, Stephan Mueller <smueller@chronox.de> wrote:
>
> Am Dienstag, 14. Juli 2020, 17:23:05 CEST schrieb Elena Petrova:
>
> Hi Elena,
>
> > Hi Stephan,
> >
> > On Tue, 14 Jul 2020 at 06:17, Stephan Mueller <smueller@chronox.de> wrote:
> > > Am Montag, 13. Juli 2020, 18:48:56 CEST schrieb Elena Petrova:
> > >
> > > Hi Elena,
> > >
> > > > This patch extends the userspace RNG interface to make it usable for
> > > > CAVS testing. This is achieved by adding ALG_SET_DRBG_ENTROPY
> > > > option to the setsockopt interface for specifying the entropy, and using
> > > > sendmsg syscall for specifying the additional data.
> > > >
> > > > See libkcapi patch [1] to test the added functionality. The libkcapi
> > > > patch is not intended for merging into libkcapi as is: it is only a
> > > > quick plug to manually verify that the extended AF_ALG RNG interface
> > > > generates the expected output on DRBG800-90A CAVS inputs.
> > >
> > > As I am responsible for developing such CAVS/ACVP harness as well, I
> > > played
> > > with the idea of going through AF_ALG. I discarded it because I do not see
> > > the benefit why we should add an interface solely for the purpose of
> > > testing. Further, it is a potentially dangerous one because the created
> > > instance of the DRBG is "seeded" from data provided by the caller.
> > >
> > > Thus, I do not see the benefit from adding that extension, widening a user
> > > space interface solely for the purpose of CAVS testing. I would not see
> > > any
> > > other benefit we have with this extension. In particular, this interface
> > > would then be always there. What I could live with is an interface that
> > > can be enabled at compile time for those who want it.
> >
> > Thanks for reviewing this patch. I understand your concern about the
> > erroneous use of the entropy input by non-testing applications. This was an
> > approach that I had discussed with Ard. I should have included you, my
> > apologies. I'll  post v2 with the CAVS testing stuff hidden under CONFIG_
> > option with appropriate help text.
> >
> > With regards to the usefulness, let me elaborate. This effort of extending
> > the drbg interface is driven by Android needs for having the kernel crypto
> > certified. I started from having an out-of-tree chrdev driver for Google
> > Pixel kernels that was exposing the required crypto functionality, and it
> > wasn't ideal in the following ways:
> >  * it primarily consisted of copypasted code from testmgr.c
> >  * it was hard for me to keep the code up to date because I'm not aware of
> >    ongoing changes to crypto
> >  * it is hard for other people and/or organisations to re-use it, hense a
> > lot of duplicated effort is going into CAVS: you have a private driver, I
> > have mine, Jaya from HP <jayalakshmi.bhat@hp.com>, who's been asking
> > linux-crypto a few CAVS questions, has to develop theirs...
> >
> > In general Android is trying to eliminate out-of-tree code. CAVS testing
> > functionality in particular needs to be upstream because we are switching
> > all Android devices to using a Generic Kernel Image (GKI)
> > [https://lwn.net/Articles/771974/] based on the upstream kernel.
>
> Thank you for the explanation.
> >
> > > Besides, when you want to do CAVS testing, the following ciphers are still
> > > not testable and thus this patch would only be a partial solution to get
> > > the testing covered:
> > >
> > > - AES KW (you cannot get the final IV out of the kernel - I played with
> > > the
> > > idea to return the IV through AF_ALG, but discarded it because of the
> > > concern above)
> > >
> > > - OFB/CFB MCT testing (you need the IV from the last round - same issue as
> > > for AES KW)
> > >
> > > - RSA
> > >
> > > - DH
> > >
> > > - ECDH
> >
> > For Android certification purposes, we only need to test the modules which
> > are actually being used. In our case it's what af_alg already exposes plus
> > DRBG. If, say, HP would need RSA, they could submit their own patch.
> >
> > As for exposing bits of the internal state for some algorithms, I hope
> > guarding the testing functionality with a CONFIG_ option would solve the
> > security part of the problem.
>
> Yes, for all other users.
>
> But if you are planning to enable this option for all Android devices across
> the board I am not sure here. In this case, wouldn't it make sense to require
> capable(CAP_SYS_ADMIN) for the DRBG reset operation just as an additional
> precaution? Note, the issue with the reset is that you loose all previous
> state (which is good for testing, but bad for security as I guess you agree
> :-) ).

I believe that for Android, since CAVS is a one-off on demand
operation, we can create a separate build with CONFIG_CRYPTO_CAVS_DRBG
enabled, which won't go to the users. Thanks for suggesting
capabilities check, happy to add `capable(CAP_SYS_ADMIN)` regardless.

> > > With these issues, I would assume you are better off creating your own
> > > kernel module just like I did that externalizes the crypto API to user
> > > space but is only available on your test kernel and will not affect all
> > > other users.
> > I considered publishing my kernel driver on GitHub, but there appears to be
> > a sufficiently large number of users to justify having this functionality
> > upstream.
>
> So, I should then dust off my AF_ALG KPP and AF_ALG akcipher patches then? :-D

Sure :)

> >
> > Hope that addresses your concerns.
> >
> > > Ciao
> > > Stephan
> >
> > Thanks,
> > Elena
>
>
> Ciao
> Stephan
>
>

Re: [PATCH 0/1] crypto: af_alg - add extra parameters for DRBG interface

[Stephan Mueller]

Am Donnerstag, 16. Juli 2020, 16:41:26 CEST schrieb Elena Petrova:

Hi Herbert,

> > > > With these issues, I would assume you are better off creating your own
> > > > kernel module just like I did that externalizes the crypto API to user
> > > > space but is only available on your test kernel and will not affect
> > > > all
> > > > other users.
> > > 
> > > I considered publishing my kernel driver on GitHub, but there appears to
> > > be
> > > a sufficiently large number of users to justify having this
> > > functionality
> > > upstream.
> > 
> > So, I should then dust off my AF_ALG KPP and AF_ALG akcipher patches then?
> > :-D
> Sure :)

Long time ago when I released the patches now found in [1] and [2] they where 
rejected as it was said, the official route to access the RSA/ECDSA and the 
DH/ECDH ciphers is through the keyring.

Obviously this interface of the keyring is not suitable for testing these 
algorithms. Considering the request that the kernel crypto API ciphers should 
be testable with arbitrary test vectors, would the dusted-off patches for 
AF_ALG KPP and akcipher be accepted?

Ciao
Stephan

[1] https://github.com/smuellerDD/libkcapi/tree/master/kernel-patches/4.15-rc3/asym

[2] https://github.com/smuellerDD/libkcapi/tree/master/kernel-patches/4.15-rc3/kpp

Re: [PATCH 0/1] crypto: af_alg - add extra parameters for DRBG interface

[Stephan Mueller]

Am Donnerstag, 16. Juli 2020, 16:49:52 CEST schrieb Stephan Mueller:

Hi Herbert,

(resent, adding Herbert to the list and fix the keyrings address)

> Am Donnerstag, 16. Juli 2020, 16:41:26 CEST schrieb Elena Petrova:
> 
> Hi Herbert,
> 
> > > > > With these issues, I would assume you are better off creating your
> > > > > own
> > > > > kernel module just like I did that externalizes the crypto API to
> > > > > user
> > > > > space but is only available on your test kernel and will not affect
> > > > > all
> > > > > other users.
> > > > 
> > > > I considered publishing my kernel driver on GitHub, but there appears
> > > > to
> > > > be
> > > > a sufficiently large number of users to justify having this
> > > > functionality
> > > > upstream.
> > > 
> > > So, I should then dust off my AF_ALG KPP and AF_ALG akcipher patches
> > > then?
> > > 
> > > :-D
> > 
> > Sure :)
> 
> Long time ago when I released the patches now found in [1] and [2] they
> where rejected as it was said, the official route to access the RSA/ECDSA
> and the DH/ECDH ciphers is through the keyring.
> 
> Obviously this interface of the keyring is not suitable for testing these
> algorithms. Considering the request that the kernel crypto API ciphers
> should be testable with arbitrary test vectors, would the dusted-off
> patches for AF_ALG KPP and akcipher be accepted?
> 
> Ciao
> Stephan
> 
> [1]
> https://github.com/smuellerDD/libkcapi/tree/master/kernel-patches/4.15-rc3/
> asym
> 
> [2]
> https://github.com/smuellerDD/libkcapi/tree/master/kernel-patches/4.15-rc3/
> kpp


Ciao
Stephan