[PATCH v2 00/10] crypto: AF_ALG: add AEAD and RNG support

[PATCH v2 00/10] crypto: AF_ALG: add AEAD and RNG support

[Stephan Mueller]

Hi,

This patch set adds AEAD and RNG support to the AF_ALG interface
exported by the kernel crypto API. By extending AF_ALG with AEAD and RNG
support, all cipher types the kernel crypto API allows access to are
now accessible from userspace.

The RNG support is stand-alone.

The AEAD implementation is added to algif_skcipher.c to prevent
re-implementation of the memory moving logic.

The extension for the AEAD support can be summarized with the following
types of changes:

        * select the correct crypto API functions (either the ablkcipher
          or the aead functions)

        * apply the additional data needed for AEAD at the right time
          (associated data, authentication tag) -- this includes the addition
          of user space interfaces to allow setting this data.

        * add the calculation for the memory size needed for encryption and
          decryption.

In addition, the patch set adds a getsockopt implementation to skcipher to
allow user space to inquire about properties of the ciphers (IV size,
block size, authentication data size). This extension would be needed for a
generic user space usage of these ciphers.

The new AEAD and RNG interfaces are fully tested with the test application
provided at [1]. That test application exercises all newly added user space
interfaces.

The patch set was tested on x86_64 and i386.

[1] http://www.chronox.de/libkcapi.html

Changes v2:
* rebase to current cryptodev-2.6 tree
* use memzero_explicit to zeroize AEAD associated data
* use sizeof for determining length of AEAD associated data
* update algif_rng.c covering all suggestions from Daniel Borkmann
  <dborkman@redhat.com>
* addition of patch 9: add digestsize interface for hashes
* addition of patch to update documentation covering the userspace interface
* change numbers of getsockopt options: separate them from sendmsg interface
  definitions

Stephan Mueller (10):
  crypto: AF_ALG: add user space interface for AEAD
  crypto: AF_ALG: user space interface for cipher info
  crypto: AF_ALG: extend data structuers for AEAD
  crypto: AF_ALG: crypto API calls to inline functions
  crypto: AF_ALG: add AEAD support
  crypto: AF_ALG: make setkey optional
  crypto: AF_ALG: add random number generator support
  crypto: AF_ALG: enable RNG interface compilation
  crypto: AF_ALG: user space interface for hash info
  crypto: AF_ALG: document the user space interface

 Documentation/crypto/crypto-API-userspace.txt |  95 ++++++-
 crypto/Kconfig                                |   9 +
 crypto/Makefile                               |   1 +
 crypto/af_alg.c                               |  20 ++
 crypto/algif_hash.c                           |  35 ++-
 crypto/algif_rng.c                            | 186 ++++++++++++++
 crypto/algif_skcipher.c                       | 352 +++++++++++++++++++++++---
 include/crypto/if_alg.h                       |   2 +
 include/uapi/linux/if_alg.h                   |  15 ++
 9 files changed, 683 insertions(+), 32 deletions(-)
 create mode 100644 crypto/algif_rng.c

-- 
2.1.0

[PATCH v2 01/10] crypto: AF_ALG: add user space interface for AEAD

[Stephan Mueller]

AEAD requires the following data in addition to normal symmetric
ciphers:

	* Associated authentication data of arbitrary length

	* Authentication tag for decryption

	* Length of authentication tag for encryption

The authentication tag data is communicated as part of the actual
ciphertext as mandated by the kernel crypto API. Therefore we only need
to provide a user space interface for the associated authentication data
as well as for the authentication tag length.

This patch adds both as a setsockopt interface that is identical to the
AF_ALG interface for setting an IV and for selecting the cipher
operation type (encrypt or decrypt).

Signed-off-by: Stephan Mueller <smueller@chronox.de>
---
 crypto/af_alg.c             | 17 +++++++++++++++++
 include/crypto/if_alg.h     |  2 ++
 include/uapi/linux/if_alg.h |  7 +++++++
 3 files changed, 26 insertions(+)

diff --git a/crypto/af_alg.c b/crypto/af_alg.c
index 6a3ad80..635140b 100644
--- a/crypto/af_alg.c
+++ b/crypto/af_alg.c
@@ -421,6 +421,23 @@ int af_alg_cmsg_send(struct msghdr *msg, struct af_alg_control *con)
 			con->op = *(u32 *)CMSG_DATA(cmsg);
 			break;
 
+
+		case ALG_SET_AEAD_AUTHSIZE:
+			if (cmsg->cmsg_len < CMSG_LEN(sizeof(u32)))
+				return -EINVAL;
+			con->aead_authsize = *(u32 *)CMSG_DATA(cmsg);
+			break;
+
+		case ALG_SET_AEAD_ASSOC:
+			if (cmsg->cmsg_len < CMSG_LEN(sizeof(*con->aead_assoc)))
+				return -EINVAL;
+			con->aead_assoc = (void *)CMSG_DATA(cmsg);
+			if (cmsg->cmsg_len <
+				CMSG_LEN(con->aead_assoc->aead_assoclen +
+					 sizeof(*con->aead_assoc)))
+				return -EINVAL;
+			break;
+
 		default:
 			return -EINVAL;
 		}
diff --git a/include/crypto/if_alg.h b/include/crypto/if_alg.h
index d61c111..c741483 100644
--- a/include/crypto/if_alg.h
+++ b/include/crypto/if_alg.h
@@ -41,7 +41,9 @@ struct af_alg_completion {
 
 struct af_alg_control {
 	struct af_alg_iv *iv;
+	struct af_alg_aead_assoc *aead_assoc;
 	int op;
+	unsigned int aead_authsize;
 };
 
 struct af_alg_type {
diff --git a/include/uapi/linux/if_alg.h b/include/uapi/linux/if_alg.h
index 0f9acce..64e7008 100644
--- a/include/uapi/linux/if_alg.h
+++ b/include/uapi/linux/if_alg.h
@@ -28,10 +28,17 @@ struct af_alg_iv {
 	__u8	iv[0];
 };
 
+struct af_alg_aead_assoc {
+	__u32	aead_assoclen;
+	__u8	aead_assoc[0];
+};
+
 /* Socket options */
 #define ALG_SET_KEY			1
 #define ALG_SET_IV			2
 #define ALG_SET_OP			3
+#define ALG_SET_AEAD_ASSOC		4
+#define ALG_SET_AEAD_AUTHSIZE		5
 
 /* Operations */
 #define ALG_OP_DECRYPT			0
-- 
2.1.0

Re: [PATCH v2 01/10] crypto: AF_ALG: add user space interface for AEAD

[Herbert Xu]

On Sun, Nov 16, 2014 at 03:23:50AM +0100, Stephan Mueller wrote:
> AEAD requires the following data in addition to normal symmetric
> ciphers:
> 
> 	* Associated authentication data of arbitrary length
> 
> 	* Authentication tag for decryption
> 
> 	* Length of authentication tag for encryption
> 
> The authentication tag data is communicated as part of the actual
> ciphertext as mandated by the kernel crypto API. Therefore we only need
> to provide a user space interface for the associated authentication data
> as well as for the authentication tag length.
> 
> This patch adds both as a setsockopt interface that is identical to the
> AF_ALG interface for setting an IV and for selecting the cipher
> operation type (encrypt or decrypt).
> 
> Signed-off-by: Stephan Mueller <smueller@chronox.de>

I don't like the fact that we're putting arbitrary limits on
the AD, as well as the fact that the way you're doing it the
AD has to be copied.

How about simply saying that the first X bytes of the input
shall be the AD?

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 v2 01/10] crypto: AF_ALG: add user space interface for AEAD

[Stephan Mueller]

Am Dienstag, 18. November 2014, 22:06:31 schrieb Herbert Xu:

Hi Herbert,

> On Sun, Nov 16, 2014 at 03:23:50AM +0100, Stephan Mueller wrote:
> > AEAD requires the following data in addition to normal symmetric
> > 
> > ciphers:
> > 	* Associated authentication data of arbitrary length
> > 	
> > 	* Authentication tag for decryption
> > 	
> > 	* Length of authentication tag for encryption
> > 
> > The authentication tag data is communicated as part of the actual
> > ciphertext as mandated by the kernel crypto API. Therefore we only need
> > to provide a user space interface for the associated authentication data
> > as well as for the authentication tag length.
> > 
> > This patch adds both as a setsockopt interface that is identical to the
> > AF_ALG interface for setting an IV and for selecting the cipher
> > operation type (encrypt or decrypt).
> > 
> > Signed-off-by: Stephan Mueller <smueller@chronox.de>
> 
> I don't like the fact that we're putting arbitrary limits on
> the AD, as well as the fact that the way you're doing it the
> AD has to be copied.
> 
> How about simply saying that the first X bytes of the input
> shall be the AD?

That is a very good idea.

To cover that approach, the kernel needs to be informed about the length of 
the authentication data size to separate the ciphertext/plaintext from the 
authentication data.

To cover that, I would recommend to simply send a u32 value to the kernel for 
the AD size instead of the AD. The kernel then can adjust the pointers as 
necessary.

I will update the patch in the next days to cover that scenario.

Thanks

-- 
Ciao
Stephan

Re: [PATCH v2 01/10] crypto: AF_ALG: add user space interface for AEAD

[Stephan Mueller]

Am Dienstag, 18. November 2014, 22:06:31 schrieb Herbert Xu:

Hi Herbert,

> On Sun, Nov 16, 2014 at 03:23:50AM +0100, Stephan Mueller wrote:
> > AEAD requires the following data in addition to normal symmetric
> > 
> > ciphers:
> > 	* Associated authentication data of arbitrary length
> > 	
> > 	* Authentication tag for decryption
> > 	
> > 	* Length of authentication tag for encryption
> > 
> > The authentication tag data is communicated as part of the actual
> > ciphertext as mandated by the kernel crypto API. Therefore we only need
> > to provide a user space interface for the associated authentication data
> > as well as for the authentication tag length.
> > 
> > This patch adds both as a setsockopt interface that is identical to the
> > AF_ALG interface for setting an IV and for selecting the cipher
> > operation type (encrypt or decrypt).
> > 
> > Signed-off-by: Stephan Mueller <smueller@chronox.de>
> 
> I don't like the fact that we're putting arbitrary limits on
> the AD, as well as the fact that the way you're doing it the
> AD has to be copied.
> 
> How about simply saying that the first X bytes of the input
> shall be the AD?

When looking deeper into skcipher_sendmsg, I see that the input data is copied 
into the kernel using memcpy_fromiovec. The memory is allocated before the 
memcpy call by skcipher_alloc_sgl.

The AD is input data and therefore needs to be copied into the kernel just 
like plaintext. Of course it is possible to require user space to concatenate 
the AD with the plaintext (or ciphertext in case of decryption). However, I 
see the following drawbacks when we do that:

- either user space has to do a very good memory allocation or it has to copy 
the data into the buffer before the plaintext. Also, if the plaintext buffer 
is not allocated in the right way, even the plaintext has to be copied to the 
newly created buffer that concatenates the AD with the plaintext. So, unless 
user space is very careful, at least some memcpy must be done in user space to 
accommodate the requirement that AD and plaintext is concatenated.

- The kernel function of skcipher_sendmsg would now become very complicated, 
because a similar logic currently applied to plaintext needs to be also 
implemented to copy and track the initial AD into the kernel.

However I see your point as the sendmsg approach to handle AD currently 
implements two memcpy() calls: one is the copy_from_user of the sendmsg system 
call framework to copy in msg and then the memcpy in skcipher_sendmsg.

To avoid the cluttering of the skcipher_sendmsg function (which already is 
complex), may I propose that a setsockopt call is used just like the SET_IV 
call? When using the setsockopt call, I see the following advantages:

- only one memcpy (the copy_from_user) is needed to move the data into kernel 
land -- this would be then en-par with the skcipher_sendmsg approach.

- the implementation of the setsockopt approach would be very clean and small 
as just one copy_from_user call is to be made followed by a 
aead_request_set_assoc call.

- user space memory handling is very clean as user space does not need a very 
specific memory setup to deliver AD. So, if the memory layout is not as 
specific as needed for the AEAD call, with the setsockopt approach, we do not 
need additional memcpy calls in user space.

In any case, we need to set some upper boundary for the maximum size of the 
AD. As I do not know of any standardized upper limit for the AD size, I would 
consider the standard CAVP testing requirements. These tests have the maximum 
AD size of 1<<16. When using the setsockopt call approach we can allocate the 
in-kernel AD memory at the setsockopt call time. IMHO it would be save now to 
limit the maximum AD size to 1<<16 at this point.
> 
> Cheers,


-- 
Ciao
Stephan

Re: [PATCH v2 01/10] crypto: AF_ALG: add user space interface for AEAD

[Herbert Xu]

On Wed, Nov 19, 2014 at 05:20:42AM +0100, Stephan Mueller wrote:
> 
> When looking deeper into skcipher_sendmsg, I see that the input data is copied 
> into the kernel using memcpy_fromiovec. The memory is allocated before the 
> memcpy call by skcipher_alloc_sgl.

Zero-copy is done through sendpage.

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 v2 01/10] crypto: AF_ALG: add user space interface for AEAD

[Stephan Mueller]

Am Mittwoch, 19. November 2014, 12:27:04 schrieb Herbert Xu:

Hi Herbert,

> On Wed, Nov 19, 2014 at 05:20:42AM +0100, Stephan Mueller wrote:
> > When looking deeper into skcipher_sendmsg, I see that the input data is
> > copied into the kernel using memcpy_fromiovec. The memory is allocated
> > before the memcpy call by skcipher_alloc_sgl.
> 
> Zero-copy is done through sendpage.

I am slightly at a loss here -- if you could give me a hint on how you think 
it can be implemented, I would be grateful.

Let us assume the AD || plaintext buffer is known to the kernel, either 
through sendpage or sendmsg. The entire buffer is split into chunks of 
scatterlists with ctx->tsgl. After processing one scatterlist from ctx->tsgl, 
that scatterlist is released via skcipher_pull_sgl.

Now, for AD, we need to consider:

- AD can span multiple ctx->tsgl chunks

- these AD scatterlist chunks cannot be released after a normal encryption 
operation. The associated data must be available for multiple operations. So, 
while plaintext data is still flowing in, we need to keep operating with the 
same AD.

Thus I am wondering how the rather static nature of the AD can fit with the 
dynamic nature of the plaintext given the current implementation on how 
plaintext is handled in the kernel.

To me, AD in league with an IV considering its rather static nature. Having 
said that, the IV is also not transported via the plaintext interface, but via 
a setsockopt. Shouldn't the AD be handled the same way?
> 
> Cheers,


-- 
Ciao
Stephan

Re: [PATCH v2 01/10] crypto: AF_ALG: add user space interface for AEAD

[Herbert Xu]

On Wed, Nov 19, 2014 at 07:30:52AM +0100, Stephan Mueller wrote:
> 
> - these AD scatterlist chunks cannot be released after a normal encryption 
> operation. The associated data must be available for multiple operations. So, 
> while plaintext data is still flowing in, we need to keep operating with the 
> same AD.

We don't start an AEAD operation until the entire input has been
received.  Unlike ciphers you cannot process AEAD requests as you
go.

So there is no need to special-case AD chunks since you will have
everything at your disposal before you can feed the request to the
crypto API.

> Thus I am wondering how the rather static nature of the AD can fit with the 
> dynamic nature of the plaintext given the current implementation on how 
> plaintext is handled in the kernel.
> 
> To me, AD in league with an IV considering its rather static nature. Having 
> said that, the IV is also not transported via the plaintext interface, but via 
> a setsockopt. Shouldn't the AD be handled the same way?

AD is not like an IV at all.  An IV is a fixed-size (and small)
input while AD can be of any length.

Think about how this is used in real life.  For IPsec AD is the part
of the packet that we don't encrypt.  So there is nothing fundamentally
different between AD and the plain-text that we do encrypt except
that you don't encrypt it :)

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

[PATCH v2 02/10] crypto: AF_ALG: user space interface for cipher info

[Stephan Mueller]

The AF_ALG interface allows normal cipher (hash, encrypt, decrypt).
However, it does not allow user space to obtain the following generic
information about the currently active cipher:

	* block size of the cipher

	* IV size of the cipher

	* for AEAD, the maximum authentication tag size

The patch adds a getsockopt interface for the symmetric ciphers to
answer such information requests from user space.

The kernel crypto API function calls are used to obtain the real data.
As all data are simple integer values, the getsockopt handler function
uses put_user() to return the integer value to user space in the
*optval parameter of getsockopt.

Signed-off-by: Stephan Mueller <smueller@chronox.de>
---
 crypto/algif_skcipher.c     | 46 ++++++++++++++++++++++++++++++++++++++++++++-
 include/uapi/linux/if_alg.h |  7 +++++++
 2 files changed, 52 insertions(+), 1 deletion(-)

diff --git a/crypto/algif_skcipher.c b/crypto/algif_skcipher.c
index 85e3bdb..2f5d663 100644
--- a/crypto/algif_skcipher.c
+++ b/crypto/algif_skcipher.c
@@ -522,6 +522,50 @@ static unsigned int skcipher_poll(struct file *file, struct socket *sock,
 	return mask;
 }
 
+static int skcipher_getsockopt(struct socket *sock, int level, int optname,
+			       char __user *optval, int __user *optlen)
+{
+	struct sock *sk = sock->sk;
+	struct alg_sock *ask = alg_sk(sk);
+	struct skcipher_ctx *ctx = ask->private;
+	const struct af_alg_type *type;
+	int len = 0;
+	int err = -EOPNOTSUPP;
+
+	lock_sock(sk);
+	type = ask->type;
+
+	if (level != SOL_ALG || !type)
+		goto unlock;
+
+	switch (optname) {
+	case ALG_GET_BLOCKSIZE:
+		len = skcipher_crypto_blocksize(ctx);
+		err = 0;
+		break;
+	case ALG_GET_IVSIZE:
+		len = skcipher_crypto_ivsize_ctx(ctx);
+		err = 0;
+		break;
+	case ALG_GET_AEAD_AUTHSIZE:
+		if (ctx->aead) {
+			len = crypto_aead_authsize(crypto_aead_reqtfm(
+						   &ctx->u.aead_req));
+			err = 0;
+		}
+		break;
+	default:
+		break;
+	}
+
+unlock:
+	release_sock(sk);
+	if (err >= 0)
+		err = put_user(len, optlen);
+
+	return err;
+}
+
 static struct proto_ops algif_skcipher_ops = {
 	.family		=	PF_ALG,
 
@@ -531,7 +575,6 @@ static struct proto_ops algif_skcipher_ops = {
 	.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,
@@ -542,6 +585,7 @@ static struct proto_ops algif_skcipher_ops = {
 	.sendpage	=	skcipher_sendpage,
 	.recvmsg	=	skcipher_recvmsg,
 	.poll		=	skcipher_poll,
+	.getsockopt	=	skcipher_getsockopt,
 };
 
 static void *skcipher_bind(const char *name, u32 type, u32 mask)
diff --git a/include/uapi/linux/if_alg.h b/include/uapi/linux/if_alg.h
index 64e7008..b8fb714 100644
--- a/include/uapi/linux/if_alg.h
+++ b/include/uapi/linux/if_alg.h
@@ -40,8 +40,15 @@ struct af_alg_aead_assoc {
 #define ALG_SET_AEAD_ASSOC		4
 #define ALG_SET_AEAD_AUTHSIZE		5
 
+/* getsockopt options */
+#define ALG_GET_BLOCKSIZE		1
+#define ALG_GET_IVSIZE			2
+#define ALG_GET_AEAD_AUTHSIZE		3
+
 /* Operations */
 #define ALG_OP_DECRYPT			0
 #define ALG_OP_ENCRYPT			1
 
+
+
 #endif	/* _LINUX_IF_ALG_H */
-- 
2.1.0

Re: [PATCH v2 02/10] crypto: AF_ALG: user space interface for cipher info

[Herbert Xu]

On Sun, Nov 16, 2014 at 03:24:25AM +0100, Stephan Mueller wrote:
> The AF_ALG interface allows normal cipher (hash, encrypt, decrypt).
> However, it does not allow user space to obtain the following generic
> information about the currently active cipher:
> 
> 	* block size of the cipher
> 
> 	* IV size of the cipher
> 
> 	* for AEAD, the maximum authentication tag size
> 
> The patch adds a getsockopt interface for the symmetric ciphers to
> answer such information requests from user space.
> 
> The kernel crypto API function calls are used to obtain the real data.
> As all data are simple integer values, the getsockopt handler function
> uses put_user() to return the integer value to user space in the
> *optval parameter of getsockopt.
> 
> Signed-off-by: Stephan Mueller <smueller@chronox.de>

We already have crypto_user so you should be extending that to
cover what's missing.

PS These paramters should not vary depending on the implementation,
if they do then one of the implementations must be buggy.

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 v2 02/10] crypto: AF_ALG: user space interface for cipher info

[Stephan Mueller]

Am Dienstag, 18. November 2014, 22:08:23 schrieb Herbert Xu:

Hi Herbert,

> On Sun, Nov 16, 2014 at 03:24:25AM +0100, Stephan Mueller wrote:
> > The AF_ALG interface allows normal cipher (hash, encrypt, decrypt).
> > However, it does not allow user space to obtain the following generic
> > 
> > information about the currently active cipher:
> > 	* block size of the cipher
> > 	
> > 	* IV size of the cipher
> > 	
> > 	* for AEAD, the maximum authentication tag size
> > 
> > The patch adds a getsockopt interface for the symmetric ciphers to
> > answer such information requests from user space.
> > 
> > The kernel crypto API function calls are used to obtain the real data.
> > As all data are simple integer values, the getsockopt handler function
> > uses put_user() to return the integer value to user space in the
> > *optval parameter of getsockopt.
> > 
> > Signed-off-by: Stephan Mueller <smueller@chronox.de>
> 
> We already have crypto_user so you should be extending that to
> cover what's missing.

Looking into that, I think nothing is missing -- thanks for the pointer. Thus 
I think I can withdraw that patch and just simply update libkcapi to use that 
user space netlink interface.

Though, I yet have to try using that interface ;-)

I am wondering why cryptouser.h is in include/linux -- shouldn't it be in 
include/uapi/linux? Aren't the definitions in that header file needed for 
userspace to talk to the netlink socket? I guess that is also the reason why I 
do not see the interface API details in /usr/linux on my F21 system.
> 
> PS These paramters should not vary depending on the implementation,
> if they do then one of the implementations must be buggy.
> 
> Cheers,


-- 
Ciao
Stephan

Re: [PATCH v2 02/10] crypto: AF_ALG: user space interface for cipher info

[Herbert Xu]

On Wed, Nov 19, 2014 at 02:02:33AM +0100, Stephan Mueller wrote:
>
> I am wondering why cryptouser.h is in include/linux -- shouldn't it be in 
> include/uapi/linux? Aren't the definitions in that header file needed for 
> userspace to talk to the netlink socket? I guess that is also the reason why I 

It should be.  I think crypto_user predates the uapi split so
that's probably why it's in the wrong place.

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 v2 02/10] crypto: AF_ALG: user space interface for cipher info

[Stephan Mueller]

Am Dienstag, 18. November 2014, 22:08:23 schrieb Herbert Xu:

Hi Herbert, Steffen,

> On Sun, Nov 16, 2014 at 03:24:25AM +0100, Stephan Mueller wrote:
> > The AF_ALG interface allows normal cipher (hash, encrypt, decrypt).
> > However, it does not allow user space to obtain the following generic
> > 
> > information about the currently active cipher:
> > 	* block size of the cipher
> > 	
> > 	* IV size of the cipher
> > 	
> > 	* for AEAD, the maximum authentication tag size
> > 
> > The patch adds a getsockopt interface for the symmetric ciphers to
> > answer such information requests from user space.
> > 
> > The kernel crypto API function calls are used to obtain the real data.
> > As all data are simple integer values, the getsockopt handler function
> > uses put_user() to return the integer value to user space in the
> > *optval parameter of getsockopt.
> > 
> > Signed-off-by: Stephan Mueller <smueller@chronox.de>
> 
> We already have crypto_user so you should be extending that to
> cover what's missing.

After playing a bit with the interface, I think it falls short supporting 
AF_ALG in the following way:

crypto_user cannot be applied to the currently active cipher that one has open 
with AF_ALG. For getting information, one has to call crypto_user with the 
cra_driver_name of a cipher. (Why is that limitation, btw (see crypto_report 
and the use of cru_driver_name?)

When we open AF_ALG with the typical approach of simply a cra_name, you have 
no idea which cipher is selected. User space has no way to obtain the 
information on which particular cipher implementation is used with 
crypto_user.

That means, to use crypto_user, we would first have to translate a cra_name 
into a cra_driver_name. Granted, any cra_driver_name for the given cra_name 
would work. But how would such a resolution be implemented? The only way would 
be via /proc/crypto. But that file does not contain all cipher / block 
chaining permutations. For example, ccm(aes) is not listed in /proc/crypto at 
all (even after using it via the kernel crypto API -- i.e. there is an 
accessible ccm(aes) implementation). Therefore, there is no way to resolve 
ccm(aes) to a cra_driver_name. 

Btw: is there an example that uses that interface? The ordering of data 
structures in the netlink message is not really clear from looking at the 
code.
> 
> PS These paramters should not vary depending on the implementation,
> if they do then one of the implementations must be buggy.
> 
> Cheers,


-- 
Ciao
Stephan

Re: [PATCH v2 02/10] crypto: AF_ALG: user space interface for cipher info

[Herbert Xu]

On Thu, Nov 20, 2014 at 05:03:24AM +0100, Stephan Mueller wrote:
>
> crypto_user cannot be applied to the currently active cipher that one has open 
> with AF_ALG. For getting information, one has to call crypto_user with the 
> cra_driver_name of a cipher. (Why is that limitation, btw (see crypto_report 
> and the use of cru_driver_name?)

It doesn't matter because every implementation must supply exactly
the same parameters as the reference implementation or it's buggy.
IOW block size, IV size, supported key sizes etc. are intrinsic
properties of the algorithm and not the implementation.

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 v2 02/10] crypto: AF_ALG: user space interface for cipher info

[Stephan Mueller]

Am Donnerstag, 20. November 2014, 12:07:48 schrieb Herbert Xu:

Hi Herbert,

> On Thu, Nov 20, 2014 at 05:03:24AM +0100, Stephan Mueller wrote:
> > crypto_user cannot be applied to the currently active cipher that one has
> > open with AF_ALG. For getting information, one has to call crypto_user
> > with the cra_driver_name of a cipher. (Why is that limitation, btw (see
> > crypto_report and the use of cru_driver_name?)
> 
> It doesn't matter because every implementation must supply exactly
> the same parameters as the reference implementation or it's buggy.
> IOW block size, IV size, supported key sizes etc. are intrinsic
> properties of the algorithm and not the implementation.
> 
I totally understand that. But you cannot supply a cra_name to crypto-user to 
get infos of that cipher as it is currently written. You have to use a 
cra_driver_name.

So, when I use a cra_name with AF_ALG, where would I get the cra_driver_name 
from to query the details from crypto_user?

Or we have to patch crypto_user to allow the use of cra_name -- which is 
currently explicitly disabled.

> Cheers,


-- 
Ciao
Stephan

Re: [PATCH v2 02/10] crypto: AF_ALG: user space interface for cipher info

[Herbert Xu]

On Thu, Nov 20, 2014 at 05:14:49AM +0100, Stephan Mueller wrote:
> 
> Or we have to patch crypto_user to allow the use of cra_name -- which is 
> currently explicitly disabled.

In what way is it disabled?

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 v2 02/10] crypto: AF_ALG: user space interface for cipher info

[Stephan Mueller]

Am Donnerstag, 20. November 2014, 12:18:24 schrieb Herbert Xu:

Hi Herbert,

> On Thu, Nov 20, 2014 at 05:14:49AM +0100, Stephan Mueller wrote:
> > Or we have to patch crypto_user to allow the use of cra_name -- which is
> > currently explicitly disabled.
> 
> In what way is it disabled?

Here is the code:

static int crypto_report(struct sk_buff *in_skb, struct nlmsghdr *in_nlh,
			 struct nlattr **attrs)
{
...
	if (!p->cru_driver_name[0])
		return -EINVAL;

	alg = crypto_alg_match(p, 1);
	if (!alg)
		return -ENOENT;
...

And crypto_alg_match is implemented as:

static struct crypto_alg *crypto_alg_match(struct crypto_user_alg *p, int 
exact)
{
...
		if (strlen(p->cru_driver_name))
			match = !strcmp(q->cra_driver_name,
					p->cru_driver_name);
		else if (!exact)
			match = !strcmp(q->cra_name, p->cru_name);
...
> 
> Cheers,


-- 
Ciao
Stephan

crypto: user - Allow get request with empty driver name

[Herbert Xu]

On Thu, Nov 20, 2014 at 05:23:23AM +0100, Stephan Mueller wrote:
> 
> Here is the code:
> 
> static int crypto_report(struct sk_buff *in_skb, struct nlmsghdr *in_nlh,
> 			 struct nlattr **attrs)
> {
> ...
> 	if (!p->cru_driver_name[0])
> 		return -EINVAL;

OK let's fix this.

crypto: user - Allow get request with empty driver name
    
Currently all get requests with an empty driver name fail with
EINVAL.  Since most users actually want to supply an empty driver
name this patch removes this check.

Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>

diff --git a/crypto/crypto_user.c b/crypto/crypto_user.c
index e2a34fe..0bb30ac 100644
--- a/crypto/crypto_user.c
+++ b/crypto/crypto_user.c
@@ -201,10 +201,7 @@ static int crypto_report(struct sk_buff *in_skb, struct nlmsghdr *in_nlh,
 	if (!null_terminated(p->cru_name) || !null_terminated(p->cru_driver_name))
 		return -EINVAL;
 
-	if (!p->cru_driver_name[0])
-		return -EINVAL;
-
-	alg = crypto_alg_match(p, 1);
+	alg = crypto_alg_match(p, 0);
 	if (!alg)
 		return -ENOENT;
 
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: crypto: user - Allow get request with empty driver name

[Steffen Klassert]

On Thu, Nov 20, 2014 at 12:46:50PM +0800, Herbert Xu wrote:
> On Thu, Nov 20, 2014 at 05:23:23AM +0100, Stephan Mueller wrote:
> > 
> > Here is the code:
> > 
> > static int crypto_report(struct sk_buff *in_skb, struct nlmsghdr *in_nlh,
> > 			 struct nlattr **attrs)
> > {
> > ...
> > 	if (!p->cru_driver_name[0])
> > 		return -EINVAL;
> 
> OK let's fix this.
> 
> crypto: user - Allow get request with empty driver name
>     
> Currently all get requests with an empty driver name fail with
> EINVAL.  Since most users actually want to supply an empty driver
> name this patch removes this check.
> 
> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
> 
> diff --git a/crypto/crypto_user.c b/crypto/crypto_user.c
> index e2a34fe..0bb30ac 100644
> --- a/crypto/crypto_user.c
> +++ b/crypto/crypto_user.c
> @@ -201,10 +201,7 @@ static int crypto_report(struct sk_buff *in_skb, struct nlmsghdr *in_nlh,
>  	if (!null_terminated(p->cru_name) || !null_terminated(p->cru_driver_name))
>  		return -EINVAL;
>  
> -	if (!p->cru_driver_name[0])
> -		return -EINVAL;
> -
> -	alg = crypto_alg_match(p, 1);
> +	alg = crypto_alg_match(p, 0);

I think this is not sufficient, crypto_alg_match() will now return the first
algorithm in crypto_alg_list that matches cra_name. We would need to extend
crypto_alg_match() to return the algorithm with the highest priority in that
case.

Re: crypto: user - Allow get request with empty driver name

[Herbert Xu]

On Thu, Nov 20, 2014 at 08:11:42AM +0100, Steffen Klassert wrote:
>
> I think this is not sufficient, crypto_alg_match() will now return the first
> algorithm in crypto_alg_list that matches cra_name. We would need to extend
> crypto_alg_match() to return the algorithm with the highest priority in that
> case.

It doesn't really matter because all implementations must provide
exactly the same set of parameters for a given algorithm so it's
good enough for what Stephan wants to do.

But yes an interface to grab the highest priority algorithm would
be useful too so patches are welcome :)

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: crypto: user - Allow get request with empty driver name

[Steffen Klassert]

On Thu, Nov 20, 2014 at 03:45:26PM +0800, Herbert Xu wrote:
> On Thu, Nov 20, 2014 at 08:11:42AM +0100, Steffen Klassert wrote:
> >
> > I think this is not sufficient, crypto_alg_match() will now return the first
> > algorithm in crypto_alg_list that matches cra_name. We would need to extend
> > crypto_alg_match() to return the algorithm with the highest priority in that
> > case.
> 
> It doesn't really matter because all implementations must provide
> exactly the same set of parameters for a given algorithm so it's
> good enough for what Stephan wants to do.

Ok, I see.

> But yes an interface to grab the highest priority algorithm would
> be useful too so patches are welcome :)

Should be not too hard to return the highest priority algorithm
instead of the first match with the existing interface, I'll see
what I can do.

Re: crypto: user - Allow get request with empty driver name

[Stephan Mueller]

Am Donnerstag, 20. November 2014, 09:04:06 schrieb Steffen Klassert:

Hi Steffen,

> On Thu, Nov 20, 2014 at 03:45:26PM +0800, Herbert Xu wrote:
> > On Thu, Nov 20, 2014 at 08:11:42AM +0100, Steffen Klassert wrote:
> > > I think this is not sufficient, crypto_alg_match() will now return the
> > > first algorithm in crypto_alg_list that matches cra_name. We would need
> > > to extend crypto_alg_match() to return the algorithm with the highest
> > > priority in that case.
> > 
> > It doesn't really matter because all implementations must provide
> > exactly the same set of parameters for a given algorithm so it's
> > good enough for what Stephan wants to do.
> 
> Ok, I see.
> 
> > But yes an interface to grab the highest priority algorithm would
> > be useful too so patches are welcome :)
> 
> Should be not too hard to return the highest priority algorithm
> instead of the first match with the existing interface, I'll see
> what I can do.

I think that for the purpose of using crypto-user in an AF_ALG scenario, even 
searching for the highest priporty will not necessarily give you the reference 
to the cipher used in AF_ALG. In the time between AF_ALG initialized a cipher 
and the time crypto-user is called, a new driver could register that may have 
even a higher priority than the one driver currently active for AF_ALG.

Therefore, from my perspective of AF_ALG and considering that all drivers of 
the same name should be identical, I would not suggest add more code to 
resolve the highest priority as I do not see the value of it.

-- 
Ciao
Stephan

Re: crypto: user - Allow get request with empty driver name

[Stephan Mueller]

Am Donnerstag, 20. November 2014, 12:46:50 schrieb Herbert Xu:

Hi Herbert,

> On Thu, Nov 20, 2014 at 05:23:23AM +0100, Stephan Mueller wrote:
> > Here is the code:
> > 
> > static int crypto_report(struct sk_buff *in_skb, struct nlmsghdr *in_nlh,
> > 
> > 			 struct nlattr **attrs)
> > 
> > {
> > ...
> > 
> > 	if (!p->cru_driver_name[0])
> > 	
> > 		return -EINVAL;
> 
> OK let's fix this.
> 
> crypto: user - Allow get request with empty driver name
> 
> Currently all get requests with an empty driver name fail with
> EINVAL.  Since most users actually want to supply an empty driver
> name this patch removes this check.
> 
> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>

Acked-by: Stephan Mueller <smueller@chronox.de>
> 
> diff --git a/crypto/crypto_user.c b/crypto/crypto_user.c
> index e2a34fe..0bb30ac 100644
> --- a/crypto/crypto_user.c
> +++ b/crypto/crypto_user.c
> @@ -201,10 +201,7 @@ static int crypto_report(struct sk_buff *in_skb, struct
> nlmsghdr *in_nlh, if (!null_terminated(p->cru_name) ||
> !null_terminated(p->cru_driver_name)) return -EINVAL;
> 
> -	if (!p->cru_driver_name[0])
> -		return -EINVAL;
> -
> -	alg = crypto_alg_match(p, 1);
> +	alg = crypto_alg_match(p, 0);
>  	if (!alg)
>  		return -ENOENT;
> 
> Cheers,


-- 
Ciao
Stephan

Re: crypto: user - Allow get request with empty driver name

[Stephan Mueller]

Am Donnerstag, 20. November 2014, 14:02:21 schrieb Stephan Mueller:

Hi Stephan,

> Am Donnerstag, 20. November 2014, 12:46:50 schrieb Herbert Xu:
> 
> Hi Herbert,
> 
> > On Thu, Nov 20, 2014 at 05:23:23AM +0100, Stephan Mueller wrote:
> > > Here is the code:
> > > 
> > > static int crypto_report(struct sk_buff *in_skb, struct nlmsghdr
> > > *in_nlh,
> > > 
> > > 			 struct nlattr **attrs)
> > > 
> > > {
> > > ...
> > > 
> > > 	if (!p->cru_driver_name[0])
> > > 	
> > > 		return -EINVAL;
> > 
> > OK let's fix this.
> > 
> > crypto: user - Allow get request with empty driver name
> > 
> > Currently all get requests with an empty driver name fail with
> > EINVAL.  Since most users actually want to supply an empty driver
> > name this patch removes this check.
> > 
> > Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
> 
> Acked-by: Stephan Mueller <smueller@chronox.de>

Although I acked the patch, it just occurred that your change modifies the 
user space interface such that you now cannot use a driver name any more.
> 
> > diff --git a/crypto/crypto_user.c b/crypto/crypto_user.c
> > index e2a34fe..0bb30ac 100644
> > --- a/crypto/crypto_user.c
> > +++ b/crypto/crypto_user.c
> > @@ -201,10 +201,7 @@ static int crypto_report(struct sk_buff *in_skb,
> > struct nlmsghdr *in_nlh, if (!null_terminated(p->cru_name) ||
> > !null_terminated(p->cru_driver_name)) return -EINVAL;
> > 
> > -	if (!p->cru_driver_name[0])
> > -		return -EINVAL;
> > -
> > -	alg = crypto_alg_match(p, 1);
> > +	alg = crypto_alg_match(p, 0);

What about the following

if (p->cru_driver_name[0]
	alg = crypto_alg_match(p, 1);
else
	alg = crypto_alg_match(p, 0);

> > 
> >  	if (!alg)
> >  	
> >  		return -ENOENT;
> > 
> > Cheers,


-- 
Ciao
Stephan

Re: crypto: user - Allow get request with empty driver name

[Herbert Xu]

On Thu, Nov 20, 2014 at 02:10:00PM +0100, Stephan Mueller wrote:
>
> What about the following
> 
> if (p->cru_driver_name[0]
> 	alg = crypto_alg_match(p, 1);
> else
> 	alg = crypto_alg_match(p, 0);

If cru_driver_name is not empty then exact will never be used, no?

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: crypto: user - Allow get request with empty driver name

[Stephan Mueller]

Am Donnerstag, 20. November 2014, 21:40:29 schrieb Herbert Xu:

Hi Herbert,

> On Thu, Nov 20, 2014 at 02:10:00PM +0100, Stephan Mueller wrote:
> > What about the following
> > 
> > if (p->cru_driver_name[0]

If the driver name is not empty
> > 
> > 	alg = crypto_alg_match(p, 1);

Do an exact match using the driver name
> > 
> > else
> > 
> > 	alg = crypto_alg_match(p, 0);

Otherise use the generic name for a fuzzy match.
> 
> If cru_driver_name is not empty then exact will never be used, no?

If the code suggestion does not follow my words outlined above, I want my 
words to be considered ;-)

Yet, I am unsure where the code deviates from my words.

> 
> Cheers,


-- 
Ciao
Stephan

Re: crypto: user - Allow get request with empty driver name

[Herbert Xu]

On Thu, Nov 20, 2014 at 05:08:42PM +0100, Stephan Mueller wrote:
> Am Donnerstag, 20. November 2014, 21:40:29 schrieb Herbert Xu:
> 
> Hi Herbert,
> 
> > On Thu, Nov 20, 2014 at 02:10:00PM +0100, Stephan Mueller wrote:
> > > What about the following
> > > 
> > > if (p->cru_driver_name[0]
> 
> If the driver name is not empty
> > > 
> > > 	alg = crypto_alg_match(p, 1);
> 
> Do an exact match using the driver name
> > > 
> > > else
> > > 
> > > 	alg = crypto_alg_match(p, 0);
> 
> Otherise use the generic name for a fuzzy match.
> > 
> > If cru_driver_name is not empty then exact will never be used, no?
> 
> If the code suggestion does not follow my words outlined above, I want my 
> words to be considered ;-)
> 
> Yet, I am unsure where the code deviates from my words.

No I am asking how is this different from my patch?

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: crypto: user - Allow get request with empty driver name

[Stephan Mueller]

Am Freitag, 21. November 2014, 10:31:31 schrieb Herbert Xu:

Hi Herbert,

> On Thu, Nov 20, 2014 at 05:08:42PM +0100, Stephan Mueller wrote:
> > Am Donnerstag, 20. November 2014, 21:40:29 schrieb Herbert Xu:
> > 
> > Hi Herbert,
> > 
> > > On Thu, Nov 20, 2014 at 02:10:00PM +0100, Stephan Mueller wrote:
> > > > What about the following
> > > > 
> > > > if (p->cru_driver_name[0]
> > 
> > If the driver name is not empty
> > 
> > > > 	alg = crypto_alg_match(p, 1);
> > 
> > Do an exact match using the driver name
> > 
> > > > else
> > > > 
> > > > 	alg = crypto_alg_match(p, 0);
> > 
> > Otherise use the generic name for a fuzzy match.
> > 
> > > If cru_driver_name is not empty then exact will never be used, no?
> > 
> > If the code suggestion does not follow my words outlined above, I want my
> > words to be considered ;-)
> > 
> > Yet, I am unsure where the code deviates from my words.
> 
> No I am asking how is this different from my patch?

Sorry, I withdraw my comment. Your patch is good as is after rechecking 
crypto_alg_match.

> 
> Cheers,


-- 
Ciao
Stephan

Re: crypto: user - Allow get request with empty driver name

[Stephan Mueller]

Am Donnerstag, 20. November 2014, 14:02:21 schrieb Stephan Mueller:

Hi Stephan,

> Am Donnerstag, 20. November 2014, 12:46:50 schrieb Herbert Xu:
> 
> Hi Herbert,
> 
> > On Thu, Nov 20, 2014 at 05:23:23AM +0100, Stephan Mueller wrote:
> > > Here is the code:
> > > 
> > > static int crypto_report(struct sk_buff *in_skb, struct nlmsghdr
> > > *in_nlh,
> > > 
> > > 			 struct nlattr **attrs)
> > > 
> > > {
> > > ...
> > > 
> > > 	if (!p->cru_driver_name[0])
> > > 	
> > > 		return -EINVAL;
> > 
> > OK let's fix this.
> > 
> > crypto: user - Allow get request with empty driver name
> > 
> > Currently all get requests with an empty driver name fail with
> > EINVAL.  Since most users actually want to supply an empty driver
> > name this patch removes this check.
> > 
> > Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
> 
> Acked-by: Stephan Mueller <smueller@chronox.de>
> 
> > diff --git a/crypto/crypto_user.c b/crypto/crypto_user.c
> > index e2a34fe..0bb30ac 100644
> > --- a/crypto/crypto_user.c
> > +++ b/crypto/crypto_user.c
> > @@ -201,10 +201,7 @@ static int crypto_report(struct sk_buff *in_skb,
> > struct nlmsghdr *in_nlh, if (!null_terminated(p->cru_name) ||
> > !null_terminated(p->cru_driver_name)) return -EINVAL;
> > 
> > -	if (!p->cru_driver_name[0])
> > -		return -EINVAL;
> > -
> > -	alg = crypto_alg_match(p, 1);
> > +	alg = crypto_alg_match(p, 0);

Btw: I tested that patch and it works as expected.

> > 
> >  	if (!alg)
> >  	
> >  		return -ENOENT;
> > 
> > Cheers,


-- 
Ciao
Stephan

Re: [PATCH v2 02/10] crypto: AF_ALG: user space interface for cipher info

[Steffen Klassert]

On Thu, Nov 20, 2014 at 05:03:24AM +0100, Stephan Mueller wrote:
> 
> Btw: is there an example that uses that interface? The ordering of data 
> structures in the netlink message is not really clear from looking at the 
> code.

I wrote to tool that uses this API some time ago, it is still
a bit rudimentary but should work. You can find it at:
https://sourceforge.net/projects/crconf/

Re: [PATCH v2 02/10] crypto: AF_ALG: user space interface for cipher info

[Steffen Klassert]

On Thu, Nov 20, 2014 at 05:03:24AM +0100, Stephan Mueller wrote:
> Am Dienstag, 18. November 2014, 22:08:23 schrieb Herbert Xu:
> 
> Hi Herbert, Steffen,
> 
> > 
> > We already have crypto_user so you should be extending that to
> > cover what's missing.
> 
> After playing a bit with the interface, I think it falls short supporting 
> AF_ALG in the following way:
> 
> crypto_user cannot be applied to the currently active cipher that one has open 
> with AF_ALG. For getting information, one has to call crypto_user with the 
> cra_driver_name of a cipher. (Why is that limitation, btw (see crypto_report 
> and the use of cru_driver_name?)

crypto_report() was intended to provide informations of one implementation
of a algorithm, so it was required to specify this algorithm exactly with
cru_driver_name.

We could extend crypto_report() to provide informations of the algorithm
with the highest priority that matches cra_name.

Or, we also have crypto_dump_report(). This basically provides informations
on all instantiated algorithms, similar to /proc/crypto. We could extend
this in a way that you can provide a cra_name. Then it can dump out the
informations of all algorithms that match cra_name.

[PATCH v2 03/10] crypto: AF_ALG: extend data structuers for AEAD

[Stephan Mueller]

The data structure holding the state of an ongoing symmetric cipher
operation is extended by the data variables needed for AEAD.

The request data structures are encapsulated by a union as the symmetric
cipher implementation is either exclusively used for "normal" symmetric
ciphers or for AEAD ciphers.

The define MAX_AEAD_ASSOCLEN restricts the size of the associated
authentication data. The kernel must allocate memory for this data to be
stored for the cipher operation. To prevent an excessive use of memory,
it is limited to 128 bytes, which is considered to be a sensible size.

Signed-off-by: Stephan Mueller <smueller@chronox.de>
---
 crypto/algif_skcipher.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/crypto/algif_skcipher.c b/crypto/algif_skcipher.c
index 2f5d663..483ff97 100644
--- a/crypto/algif_skcipher.c
+++ b/crypto/algif_skcipher.c
@@ -46,7 +46,15 @@ struct skcipher_ctx {
 	bool merge;
 	bool enc;
 
-	struct ablkcipher_request req;
+	bool aead;
+	void *aead_assoc;
+	/* define arbitrary maximum length of associated data */
+	#define MAX_AEAD_ASSOCLEN 128
+	struct scatterlist sg_aead_assoc;
+	union {
+		struct ablkcipher_request ablkcipher_req;
+		struct aead_request aead_req;
+	} u;
 };
 
 #define MAX_SGL_ENTS ((4096 - sizeof(struct skcipher_sg_list)) / \
-- 
2.1.0

[PATCH v2 04/10] crypto: AF_ALG: crypto API calls to inline functions

[Stephan Mueller]

To avoid excessive branches and cluttering the code, all kernel crypto
API calls are extracted into separate inline functions. These functions
invoke either the ablkcipher or the aead crypto API function calls, as
necessary.

Signed-off-by: Stephan Mueller <smueller@chronox.de>
---
 crypto/algif_skcipher.c | 143 ++++++++++++++++++++++++++++++++++++++++++------
 1 file changed, 125 insertions(+), 18 deletions(-)

diff --git a/crypto/algif_skcipher.c b/crypto/algif_skcipher.c
index 483ff97..d0e31ab 100644
--- a/crypto/algif_skcipher.c
+++ b/crypto/algif_skcipher.c
@@ -247,14 +247,121 @@ static void skcipher_data_wakeup(struct sock *sk)
 	rcu_read_unlock();
 }
 
+static inline bool skcipher_is_aead(struct crypto_tfm *tfm)
+{
+	return ((crypto_tfm_alg_type(tfm) & CRYPTO_ALG_TYPE_MASK) ==
+		CRYPTO_ALG_TYPE_AEAD);
+}
+
+static inline unsigned int skcipher_crypto_ivsize(void *private)
+{
+	if (skcipher_is_aead(private))
+		return crypto_aead_ivsize(private);
+	else
+		return crypto_ablkcipher_ivsize(private);
+}
+
+static inline unsigned int skcipher_crypto_ivsize_ctx(struct skcipher_ctx *ctx)
+{
+	if (ctx->aead)
+		return crypto_aead_ivsize(crypto_aead_reqtfm(&ctx->u.aead_req));
+	else
+		return crypto_ablkcipher_ivsize(
+			crypto_ablkcipher_reqtfm(&ctx->u.ablkcipher_req));
+}
+
+static inline unsigned int skcipher_crypto_blocksize(struct skcipher_ctx *ctx)
+{
+	if (ctx->aead)
+		return crypto_aead_blocksize(
+			crypto_aead_reqtfm(&ctx->u.aead_req));
+	else
+		return crypto_ablkcipher_blocksize(
+			crypto_ablkcipher_reqtfm(&ctx->u.ablkcipher_req));
+}
+
+static inline unsigned int skcipher_crypto_reqsize(void *private)
+{
+	if (skcipher_is_aead(private))
+		return crypto_aead_reqsize(private);
+	else
+		return crypto_ablkcipher_reqsize(private);
+}
+
+static inline unsigned int skcipher_crypto_setkey(void *private, const u8 *key,
+						  unsigned int keylen)
+{
+	if (skcipher_is_aead(private))
+		return crypto_aead_setkey(private, key, keylen);
+	else
+		return crypto_ablkcipher_setkey(private, key, keylen);
+}
+
+static inline void skcipher_crypto_free(void *private)
+{
+	if (skcipher_is_aead(private))
+		crypto_free_aead(private);
+	else
+		crypto_free_ablkcipher(private);
+}
+
+static inline void skcipher_request_set_tfm(struct skcipher_ctx *ctx, void *tfm)
+{
+	if (ctx->aead)
+		aead_request_set_tfm(&ctx->u.aead_req, tfm);
+	else
+		ablkcipher_request_set_tfm(&ctx->u.ablkcipher_req, tfm);
+}
+
+static inline int skcipher_crypto_encrypt(struct skcipher_ctx *ctx)
+{
+	if (ctx->aead)
+		return crypto_aead_encrypt(&ctx->u.aead_req);
+	else
+		return crypto_ablkcipher_encrypt(&ctx->u.ablkcipher_req);
+}
+
+static inline int skcipher_crypto_decrypt(struct skcipher_ctx *ctx)
+{
+	if (ctx->aead)
+		return crypto_aead_decrypt(&ctx->u.aead_req);
+	else
+		return crypto_ablkcipher_decrypt(&ctx->u.ablkcipher_req);
+}
+
+static inline void skcipher_crypto_set_crypt(struct skcipher_ctx *ctx,
+					     struct scatterlist *src,
+					     struct scatterlist *dst,
+					     unsigned int cryptlen, u8 *iv)
+{
+	if (ctx->aead)
+		return aead_request_set_crypt(&ctx->u.aead_req, src, dst,
+					      cryptlen, iv);
+	else
+		return ablkcipher_request_set_crypt(&ctx->u.ablkcipher_req, src,
+						    dst, cryptlen, iv);
+}
+
+static inline void skcipher_request_set_callback(struct skcipher_ctx *ctx,
+						 u32 flags,
+						 crypto_completion_t complete,
+						 void *data)
+{
+	if (ctx->aead)
+		aead_request_set_callback(&ctx->u.aead_req, flags, complete,
+					  data);
+	else
+		ablkcipher_request_set_callback(&ctx->u.ablkcipher_req, flags,
+						complete, data);
+}
+
 static int skcipher_sendmsg(struct kiocb *unused, struct socket *sock,
 			    struct msghdr *msg, size_t size)
 {
 	struct sock *sk = sock->sk;
 	struct alg_sock *ask = alg_sk(sk);
 	struct skcipher_ctx *ctx = ask->private;
-	struct crypto_ablkcipher *tfm = crypto_ablkcipher_reqtfm(&ctx->req);
-	unsigned ivsize = crypto_ablkcipher_ivsize(tfm);
+	unsigned ivsize = skcipher_crypto_ivsize_ctx(ctx);
 	struct skcipher_sg_list *sgl;
 	struct af_alg_control con = {};
 	long copied = 0;
@@ -432,8 +539,7 @@ static int skcipher_recvmsg(struct kiocb *unused, struct socket *sock,
 	struct sock *sk = sock->sk;
 	struct alg_sock *ask = alg_sk(sk);
 	struct skcipher_ctx *ctx = ask->private;
-	unsigned bs = crypto_ablkcipher_blocksize(crypto_ablkcipher_reqtfm(
-		&ctx->req));
+	unsigned bs = skcipher_crypto_blocksize(ctx);
 	struct skcipher_sg_list *sgl;
 	struct scatterlist *sg;
 	unsigned long iovlen;
@@ -483,8 +589,8 @@ static int skcipher_recvmsg(struct kiocb *unused, struct socket *sock,
 
 			err = af_alg_wait_for_completion(
 				ctx->enc ?
-					crypto_ablkcipher_encrypt(&ctx->req) :
-					crypto_ablkcipher_decrypt(&ctx->req),
+					skcipher_crypto_encrypt(ctx) :
+					skcipher_crypto_decrypt(ctx),
 				&ctx->completion);
 
 free:
@@ -603,23 +709,23 @@ static void *skcipher_bind(const char *name, u32 type, u32 mask)
 
 static void skcipher_release(void *private)
 {
-	crypto_free_ablkcipher(private);
+	skcipher_crypto_free(private);
 }
 
 static int skcipher_setkey(void *private, const u8 *key, unsigned int keylen)
 {
-	return crypto_ablkcipher_setkey(private, key, keylen);
+	return skcipher_crypto_setkey(private, key, keylen);
 }
 
 static void skcipher_sock_destruct(struct sock *sk)
 {
 	struct alg_sock *ask = alg_sk(sk);
 	struct skcipher_ctx *ctx = ask->private;
-	struct crypto_ablkcipher *tfm = crypto_ablkcipher_reqtfm(&ctx->req);
+	unsigned int ivlen = skcipher_crypto_ivsize_ctx(ctx);
 
 	skcipher_free_sgl(sk);
-	memzero_explicit(ctx->iv, crypto_ablkcipher_ivsize(tfm));
-	sock_kfree_s(sk, ctx->iv, crypto_ablkcipher_ivsize(tfm));
+	memzero_explicit(ctx->iv, ivlen);
+	sock_kfree_s(sk, ctx->iv, ivlen);
 	sock_kfree_s(sk, ctx, ctx->len);
 	af_alg_release_parent(sk);
 }
@@ -628,20 +734,20 @@ static int skcipher_accept_parent(void *private, struct sock *sk)
 {
 	struct skcipher_ctx *ctx;
 	struct alg_sock *ask = alg_sk(sk);
-	unsigned int len = sizeof(*ctx) + crypto_ablkcipher_reqsize(private);
+	unsigned int len = sizeof(*ctx) + skcipher_crypto_reqsize(private);
+	unsigned int ivlen = skcipher_crypto_ivsize(private);
 
 	ctx = sock_kmalloc(sk, len, GFP_KERNEL);
 	if (!ctx)
 		return -ENOMEM;
 
-	ctx->iv = sock_kmalloc(sk, crypto_ablkcipher_ivsize(private),
-			       GFP_KERNEL);
+	ctx->iv = sock_kmalloc(sk, ivlen, GFP_KERNEL);
 	if (!ctx->iv) {
 		sock_kfree_s(sk, ctx, len);
 		return -ENOMEM;
 	}
 
-	memset(ctx->iv, 0, crypto_ablkcipher_ivsize(private));
+	memset(ctx->iv, 0, ivlen);
 
 	INIT_LIST_HEAD(&ctx->tsgl);
 	ctx->len = len;
@@ -649,13 +755,14 @@ static int skcipher_accept_parent(void *private, struct sock *sk)
 	ctx->more = 0;
 	ctx->merge = 0;
 	ctx->enc = 0;
+	ctx->aead = skcipher_is_aead(private);
 	af_alg_init_completion(&ctx->completion);
 
 	ask->private = ctx;
 
-	ablkcipher_request_set_tfm(&ctx->req, private);
-	ablkcipher_request_set_callback(&ctx->req, CRYPTO_TFM_REQ_MAY_BACKLOG,
-					af_alg_complete, &ctx->completion);
+	skcipher_request_set_tfm(ctx, private);
+	skcipher_request_set_callback(ctx, CRYPTO_TFM_REQ_MAY_BACKLOG,
+				      af_alg_complete, &ctx->completion);
 
 	sk->sk_destruct = skcipher_sock_destruct;
 
-- 
2.1.0

[PATCH v2 05/10] crypto: AF_ALG: add AEAD support

[Stephan Mueller]

This patch adds the AEAD support for AF_ALG.

The AEAD implementation uses the entire memory handling and
infrastructure of the existing skcipher implementation.

To use AEAD, the user space consumer has to use the salg_type named
"aead". The AEAD extension only uses the bind callback as the key
differentiator. The previously added functions that select whether to
use AEAD or ablkcipher crypto API functions depend on the TFM type
allocated during the bind() call.

The addition of AEAD brings a bit of overhead to calculate the size of
the ciphertext, because the AEAD implementation of the kernel crypto API
makes implied assumption on the location of the authentication tag. When
performing an encryption, the tag will be added to the created
ciphertext (note, the tag is placed adjacent to the ciphertext). For
decryption, the caller must hand in the ciphertext with the tag appended
to the ciphertext. Therefore, the selection of the used memory
needs to add/subtract the tag size from the source/destination buffers
depending on the encryption type. The code is provided with comments
explainint when and how that operation is performed.

Note: The AF_ALG interface does not support zero length plaintext or
zero length ciphertext. Such zero length input data may be used if one
wants to access the hash implementation of an AEAD directly (e.g. the
GHASH of GCM or CMAC for CCM). However, this is a use case that is not
of interest. GHASH or CMAC is directly available via the hash AF_ALG
interface and we therefore do not need to take precautions for this
use case.

A fully working example using all aspects of AEAD is provided at
http://www.chronox.de/libkcapi.html

Signed-off-by: Stephan Mueller <smueller@chronox.de>
---
 crypto/algif_skcipher.c | 155 ++++++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 145 insertions(+), 10 deletions(-)

diff --git a/crypto/algif_skcipher.c b/crypto/algif_skcipher.c
index d0e31ab..4bb7556 100644
--- a/crypto/algif_skcipher.c
+++ b/crypto/algif_skcipher.c
@@ -49,7 +49,7 @@ struct skcipher_ctx {
 	bool aead;
 	void *aead_assoc;
 	/* define arbitrary maximum length of associated data */
-	#define MAX_AEAD_ASSOCLEN 128
+#define MAX_AEAD_ASSOCLEN 128
 	struct scatterlist sg_aead_assoc;
 	union {
 		struct ablkcipher_request ablkcipher_req;
@@ -387,6 +387,17 @@ static int skcipher_sendmsg(struct kiocb *unused, struct socket *sock,
 
 		if (con.iv && con.iv->ivlen != ivsize)
 			return -EINVAL;
+
+		/*
+		 * AEAD associated data is limited to a sensible size
+		 * Size limit is set to some arbitrary length to avoid
+		 * user space eating up memory
+		 */
+		if (ctx->aead &&
+		    (con.aead_assoc->aead_assoclen > MAX_AEAD_ASSOCLEN ||
+		     !con.aead_assoc->aead_assoclen ||
+		     !con.aead_assoc || !con.aead_authsize))
+			return -EINVAL;
 	}
 
 	err = -EINVAL;
@@ -399,6 +410,25 @@ static int skcipher_sendmsg(struct kiocb *unused, struct socket *sock,
 		ctx->enc = enc;
 		if (con.iv)
 			memcpy(ctx->iv, con.iv->iv, ivsize);
+		/* AEAD authentication data handling */
+		if (ctx->aead) {
+			if (con.aead_authsize)
+				err = crypto_aead_setauthsize(
+					crypto_aead_reqtfm(&ctx->u.aead_req),
+							   con.aead_authsize);
+			if (err)
+				goto unlock;
+			/* set associated data */
+			memcpy(ctx->aead_assoc,
+			       con.aead_assoc->aead_assoc,
+			       con.aead_assoc->aead_assoclen);
+			sg_init_one(&ctx->sg_aead_assoc,
+				    ctx->aead_assoc,
+				    con.aead_assoc->aead_assoclen);
+			aead_request_set_assoc(&ctx->u.aead_req,
+					       &ctx->sg_aead_assoc,
+					       con.aead_assoc->aead_assoclen);
+		}
 	}
 
 	while (size) {
@@ -547,10 +577,41 @@ static int skcipher_recvmsg(struct kiocb *unused, struct socket *sock,
 	int err = -EAGAIN;
 	int used;
 	long copied = 0;
+	unsigned int aead_authsize_enc = 0;
+	unsigned int aead_authsize_dec = 0;
 
 	lock_sock(sk);
+	/*
+	* AEAD memory structure: For encryption, the tag is appended to the
+	* ciphertext which implies that the memory allocated for the ciphertext
+	* must be increased by the tag length. For decryption, the tag
+	* is expected to be concatenated to the ciphertext. The plaintext
+	* therefore has a memory size of the ciphertext minus the tag length.
+	*
+	* Note: this memory calculation only works because we require the
+	* user space caller to:
+	*	* perform encryption by invoking the recv function with a buffer
+	*	  length of ciphertext + tag size -- the send function can be
+	*	  invoked normally with just the plaintext.
+	*	* perform a decryption by invoking the the write function with
+	*	  a buffer holding the ciphertext + tag (and setting the
+	*	  buffer size accordingly) -- the recv function can be invoked
+	*	  normally with just the space needed for the ciphertext.
+	*	  Though, the caller should check for EBADMSG to catch integiry
+	*	  violations.
+	*/
+	if (ctx->aead) {
+		if (ctx->enc)
+			aead_authsize_enc = crypto_aead_authsize(
+					crypto_aead_reqtfm(&ctx->u.aead_req));
+		else
+			aead_authsize_dec = crypto_aead_authsize(
+					crypto_aead_reqtfm(&ctx->u.aead_req));
+	}
+
 	for (iov = msg->msg_iov, iovlen = msg->msg_iovlen; iovlen > 0;
 	     iovlen--, iov++) {
+		/* size of the output data memory */
 		unsigned long seglen = iov->iov_len;
 		char __user *from = iov->iov_base;
 
@@ -562,6 +623,7 @@ static int skcipher_recvmsg(struct kiocb *unused, struct socket *sock,
 			while (!sg->length)
 				sg++;
 
+			/* size of the input data memory */
 			used = ctx->used;
 			if (!used) {
 				err = skcipher_wait_for_data(sk, flags);
@@ -569,7 +631,28 @@ static int skcipher_recvmsg(struct kiocb *unused, struct socket *sock,
 					goto unlock;
 			}
 
-			used = min_t(unsigned long, used, seglen);
+			used = min_t(unsigned long,
+					     /*
+					      * In case of encryption, add
+					      * the memory needed for the tag
+					      * to the input data length to
+					      * give the cipher the necessary
+					      * space to add the tag.
+					      */
+					     used + aead_authsize_enc,
+					     /*
+					      * In case of decryption, add the
+					      * memory needed for the tag
+					      * calculations to the output
+					      * buffer.
+					      */
+					     seglen + aead_authsize_dec);
+
+			if (used < aead_authsize_enc ||
+			    seglen < aead_authsize_dec) {
+				err = -ENOMEM;
+				goto unlock;
+			}
 
 			used = af_alg_make_sg(&ctx->rsgl, from, used, 1);
 			err = used;
@@ -583,9 +666,16 @@ static int skcipher_recvmsg(struct kiocb *unused, struct socket *sock,
 			if (!used)
 				goto free;
 
-			ablkcipher_request_set_crypt(&ctx->req, sg,
-						     ctx->rsgl.sg, used,
-						     ctx->iv);
+			/*
+			 * See API specification of the AEAD API: for
+			 * encryption, we need to tell the encrypt function
+			 * what the size of the plaintext is. But we have
+			 * ensured that we have sufficient memory allocated for
+			 * the operation.
+			 */
+			skcipher_crypto_set_crypt(ctx, sg, ctx->rsgl.sg,
+						  used - aead_authsize_enc,
+						  ctx->iv);
 
 			err = af_alg_wait_for_completion(
 				ctx->enc ?
@@ -599,10 +689,19 @@ free:
 			if (err)
 				goto unlock;
 
-			copied += used;
-			from += used;
-			seglen -= used;
-			skcipher_pull_sgl(sk, used);
+			/*
+			 * Adjust the output buffer counters by only the size
+			 * needed for the plaintext in case of a decryption
+			 */
+			copied += (used - aead_authsize_dec);
+			from += (used - aead_authsize_dec);
+			seglen -= (used - aead_authsize_dec);
+			/*
+			 * Adjust the input buffer by how much we have encrypted
+			 * or decrypted. In case of encryption, we only credit
+			 * the memory of the plaintext.
+			 */
+			skcipher_pull_sgl(sk, used - aead_authsize_enc);
 		}
 	}
 
@@ -724,6 +823,10 @@ static void skcipher_sock_destruct(struct sock *sk)
 	unsigned int ivlen = skcipher_crypto_ivsize_ctx(ctx);
 
 	skcipher_free_sgl(sk);
+	if (ctx->aead) {
+		memzero_explicit(ctx->aead_assoc, MAX_AEAD_ASSOCLEN);
+		sock_kfree_s(sk, ctx->aead_assoc, MAX_AEAD_ASSOCLEN);
+	}
 	memzero_explicit(ctx->iv, ivlen);
 	sock_kfree_s(sk, ctx->iv, ivlen);
 	sock_kfree_s(sk, ctx, ctx->len);
@@ -749,6 +852,17 @@ static int skcipher_accept_parent(void *private, struct sock *sk)
 
 	memset(ctx->iv, 0, ivlen);
 
+	if (skcipher_is_aead(private)) {
+		ctx->aead_assoc = sock_kmalloc(sk, MAX_AEAD_ASSOCLEN,
+					       GFP_KERNEL);
+		if (!ctx->aead_assoc) {
+			sock_kfree_s(sk, ctx->iv, ivlen);
+			sock_kfree_s(sk, ctx, len);
+			return -ENOMEM;
+		}
+		memset(ctx->aead_assoc, 0, MAX_AEAD_ASSOCLEN);
+	}
+
 	INIT_LIST_HEAD(&ctx->tsgl);
 	ctx->len = len;
 	ctx->used = 0;
@@ -779,15 +893,36 @@ static const struct af_alg_type algif_type_skcipher = {
 	.owner		=	THIS_MODULE
 };
 
+static void *aead_bind(const char *name, u32 type, u32 mask)
+{
+	return crypto_alloc_aead(name, type, mask);
+}
+
+static const struct af_alg_type algif_type_aead = {
+	.bind		=	aead_bind,
+	.release	=	skcipher_release,
+	.setkey		=	skcipher_setkey,
+	.accept		=	skcipher_accept_parent,
+	.ops		=	&algif_skcipher_ops,
+	.name		=	"aead",
+	.owner		=	THIS_MODULE
+};
+
 static int __init algif_skcipher_init(void)
 {
-	return af_alg_register_type(&algif_type_skcipher);
+	int ret = af_alg_register_type(&algif_type_skcipher);
+
+	if (ret)
+		return ret;
+	return af_alg_register_type(&algif_type_aead);
 }
 
 static void __exit algif_skcipher_exit(void)
 {
 	int err = af_alg_unregister_type(&algif_type_skcipher);
 	BUG_ON(err);
+	err = af_alg_unregister_type(&algif_type_aead);
+	BUG_ON(err);
 }
 
 module_init(algif_skcipher_init);
-- 
2.1.0

[PATCH v2 06/10] crypto: AF_ALG: make setkey optional

[Stephan Mueller]

The current AF_ALG implementation requires that a userspace interface
implementation must provide a callback for setkey. Such a call is not
appliable to random number generators.

To prepare AF_ALG for the addition of a random number generator user
space interface, this function callback invocation is made optional.

Signed-off-by: Stephan Mueller <smueller@chronox.de>
---
 crypto/af_alg.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/crypto/af_alg.c b/crypto/af_alg.c
index 635140b..47a199c 100644
--- a/crypto/af_alg.c
+++ b/crypto/af_alg.c
@@ -177,6 +177,9 @@ static int alg_setkey(struct sock *sk, char __user *ukey,
 	u8 *key;
 	int err;
 
+	if (!type->setkey)
+		return -EOPNOTSUPP;
+
 	key = sock_kmalloc(sk, keylen, GFP_KERNEL);
 	if (!key)
 		return -ENOMEM;
-- 
2.1.0

Re: [PATCH v2 06/10] crypto: AF_ALG: make setkey optional

[Herbert Xu]

On Sun, Nov 16, 2014 at 03:26:58AM +0100, Stephan Mueller wrote:
> The current AF_ALG implementation requires that a userspace interface
> implementation must provide a callback for setkey. Such a call is not
> appliable to random number generators.
> 
> To prepare AF_ALG for the addition of a random number generator user
> space interface, this function callback invocation is made optional.
> 
> Signed-off-by: Stephan Mueller <smueller@chronox.de>

Did you actually try this? AFAICS setkey is already optional.

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 v2 06/10] crypto: AF_ALG: make setkey optional

[Stephan Mueller]

Am Dienstag, 18. November 2014, 22:10:13 schrieb Herbert Xu:

Hi Herbert,

> On Sun, Nov 16, 2014 at 03:26:58AM +0100, Stephan Mueller wrote:
> > The current AF_ALG implementation requires that a userspace interface
> > implementation must provide a callback for setkey. Such a call is not
> > appliable to random number generators.
> > 
> > To prepare AF_ALG for the addition of a random number generator user
> > space interface, this function callback invocation is made optional.
> > 
> > Signed-off-by: Stephan Mueller <smueller@chronox.de>
> 
> Did you actually try this? AFAICS setkey is already optional.

You are correct. I tested the kernel without my patch and the setkey on the 
RNG handle is rejected. I now also see the check already present in the 
alg_setkey function.

This patch will be removed from a new patchset.
> 
> Cheers,


-- 
Ciao
Stephan

[PATCH v2 07/10] crypto: AF_ALG: add random number generator support

[Stephan Mueller]

This patch adds the random number generator support for AF_ALG.

A random number generator's purpose is to generate data without
requiring the caller to provide any data. Therefore, the AF_ALG
interface handler for RNGs only implements a callback handler for
recvmsg.

The following parameters provided with a recvmsg are processed by the
RNG callback handler:

	* sock - to resolve the RNG context data structure accessing the
	  RNG instance private to the socket

	* len - this parameter allows userspace callers to specify how
	  many random bytes the RNG shall produce and return. As the
	  kernel context for the RNG allocates a buffer of 128 bytes to
	  store random numbers before copying them to userspace, the len
	  parameter is checked that it is not larger than 128. If a
	  caller wants more random numbers, a new request for recvmsg
	  shall be made.

The size of 128 bytes is chose because of the following considerations:

	* to increase the memory footprint of the kernel too much (note,
	  that would be 128 bytes per open socket)

	* 128 is divisible by any typical cryptographic block size an
	  RNG may have

	* A request for random numbers typically only shall supply small
	  amount of data like for keys or IVs that should only require
	  one invocation of the recvmsg function.

Note, during instantiation of the RNG, the code checks whether the RNG
implementation requires seeding. If so, the RNG is seeded with output
from get_random_bytes.

A fully working example using all aspects of the RNG interface is
provided at http://www.chronox.de/libkcapi.html

Signed-off-by: Stephan Mueller <smueller@chronox.de>
---
 crypto/algif_rng.c | 186 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 186 insertions(+)
 create mode 100644 crypto/algif_rng.c

diff --git a/crypto/algif_rng.c b/crypto/algif_rng.c
new file mode 100644
index 0000000..fc25869
--- /dev/null
+++ b/crypto/algif_rng.c
@@ -0,0 +1,186 @@
+/*
+ * algif_rng: User-space interface for random number generators
+ *
+ * This file provides the user-space API for random number generators.
+ *
+ * Copyright (C) 2014, Stephan Mueller <smueller@chronox.de>
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ * 1. Redistributions of source code must retain the above copyright
+ *    notice, and the entire permission notice in its entirety,
+ *    including the disclaimer of warranties.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ *    notice, this list of conditions and the following disclaimer in the
+ *    documentation and/or other materials provided with the distribution.
+ * 3. The name of the author may not be used to endorse or promote
+ *    products derived from this software without specific prior
+ *    written permission.
+ *
+ * ALTERNATIVELY, this product may be distributed under the terms of
+ * the GNU General Public License, in which case the provisions of the GPL2
+ * are required INSTEAD OF the above restrictions.  (This clause is
+ * necessary due to a potential bad interaction between the GPL and
+ * the restrictions contained in a BSD-style copyright.)
+ *
+ * THIS SOFTWARE IS PROVIDED ``AS IS'' AND ANY EXPRESS OR IMPLIED
+ * WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES
+ * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE, ALL OF
+ * WHICH ARE HEREBY DISCLAIMED.  IN NO EVENT SHALL THE AUTHOR BE
+ * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
+ * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT
+ * OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR
+ * BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF
+ * LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE
+ * USE OF THIS SOFTWARE, EVEN IF NOT ADVISED OF THE POSSIBILITY OF SUCH
+ * DAMAGE.
+ */
+
+#include <linux/module.h>
+#include <crypto/rng.h>
+#include <linux/random.h>
+#include <crypto/if_alg.h>
+#include <linux/net.h>
+#include <net/sock.h>
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Stephan Mueller <smueller@chronox.de>");
+MODULE_DESCRIPTION("User-space interface for random number generators");
+
+struct rng_ctx {
+#define MAXSIZE 128
+	u8 result[MAXSIZE];
+	unsigned int len;
+	struct crypto_rng *drng;
+};
+
+static int rng_recvmsg(struct kiocb *unused, 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 = -EFAULT;
+
+	if (len == 0)
+		return 0;
+	if (len > MAXSIZE)
+		len = MAXSIZE;
+
+	lock_sock(sk);
+	len = crypto_rng_get_bytes(ctx->drng, ctx->result, len);
+	if (len < 0)
+		goto unlock;
+
+	err = memcpy_toiovec(msg->msg_iov, ctx->result, len);
+	memzero_explicit(ctx->result, len);
+
+unlock:
+	release_sock(sk);
+
+	return err ? err : len;
+}
+
+static struct proto_ops algif_rng_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,
+	.poll		=	sock_no_poll,
+	.sendmsg	=	sock_no_sendmsg,
+	.sendpage	=	sock_no_sendpage,
+
+	.release	=	af_alg_release,
+	.recvmsg	=	rng_recvmsg,
+};
+
+static void *rng_bind(const char *name, u32 type, u32 mask)
+{
+	return crypto_alloc_rng(name, type, mask);
+}
+
+static void rng_release(void *private)
+{
+	crypto_free_rng(private);
+}
+
+static void rng_sock_destruct(struct sock *sk)
+{
+	struct alg_sock *ask = alg_sk(sk);
+	struct rng_ctx *ctx = ask->private;
+
+	memzero_explicit(ctx->result, sizeof(ctx->result));
+	sock_kfree_s(sk, ctx, ctx->len);
+	af_alg_release_parent(sk);
+}
+
+static int rng_accept_parent(void *private, struct sock *sk)
+{
+	struct rng_ctx *ctx;
+	struct alg_sock *ask = alg_sk(sk);
+	unsigned int len = sizeof(*ctx);
+	int seedsize = crypto_rng_seedsize(private);
+	int ret = -ENOMEM;
+
+	ctx = sock_kmalloc(sk, len, GFP_KERNEL);
+	if (!ctx)
+		return -ENOMEM;
+	memset(ctx->result, 0, sizeof(ctx->result));
+
+	ctx->len = len;
+
+	if (seedsize) {
+		u8 *buf = kmalloc(seedsize, GFP_KERNEL);
+		if (!buf)
+			goto err;
+		get_random_bytes(buf, seedsize);
+		ret = crypto_rng_reset(private, buf, len);
+		kzfree(buf);
+		if (ret)
+			goto err;
+	}
+
+	ctx->drng = private;
+	ask->private = ctx;
+	sk->sk_destruct = rng_sock_destruct;
+
+	return 0;
+
+err:
+	sock_kfree_s(sk, ctx, len);
+	return ret;
+}
+
+static const struct af_alg_type algif_type_rng = {
+	.bind		=	rng_bind,
+	.release	=	rng_release,
+	.accept		=	rng_accept_parent,
+	.ops		=	&algif_rng_ops,
+	.name		=	"rng",
+	.owner		=	THIS_MODULE
+};
+
+static int __init rng_init(void)
+{
+	return af_alg_register_type(&algif_type_rng);
+}
+
+void __exit rng_exit(void)
+{
+	int err = af_alg_unregister_type(&algif_type_rng);
+	BUG_ON(err);
+}
+
+module_init(rng_init);
+module_exit(rng_exit);
-- 
2.1.0

[PATCH v2 08/10] crypto: AF_ALG: enable RNG interface compilation

[Stephan Mueller]

Enable compilation of the RNG AF_ALG support and provide a Kconfig
option to compile the RNG AF_ALG support.

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

diff --git a/crypto/Kconfig b/crypto/Kconfig
index 87bbc9c..e127323 100644
--- a/crypto/Kconfig
+++ b/crypto/Kconfig
@@ -1505,6 +1505,15 @@ config CRYPTO_USER_API_SKCIPHER
 	  This option enables the user-spaces interface for symmetric
 	  key cipher algorithms.
 
+config CRYPTO_USER_API_RNG
+	tristate "User-space interface for random number generator algorithms"
+	depends on NET
+	select CRYPTO_RNG
+	select CRYPTO_USER_API
+	help
+	  This option enables the user-spaces interface for random
+	  number generator algorithms.
+
 config CRYPTO_HASH_INFO
 	bool
 
diff --git a/crypto/Makefile b/crypto/Makefile
index 1445b91..ba19465 100644
--- a/crypto/Makefile
+++ b/crypto/Makefile
@@ -99,6 +99,7 @@ obj-$(CONFIG_CRYPTO_GHASH) += ghash-generic.o
 obj-$(CONFIG_CRYPTO_USER_API) += af_alg.o
 obj-$(CONFIG_CRYPTO_USER_API_HASH) += algif_hash.o
 obj-$(CONFIG_CRYPTO_USER_API_SKCIPHER) += algif_skcipher.o
+obj-$(CONFIG_CRYPTO_USER_API_RNG) += algif_rng.o
 
 #
 # generic algorithms and the async_tx api
-- 
2.1.0

[PATCH v2 09/10] crypto: AF_ALG: user space interface for hash info

[Stephan Mueller]

The AF_ALG interface allows normal cipher (hash, encrypt, decrypt).
However, it does not allow user space to obtain the following generic
information about the currently active hash:

	* digestsize

The patch adds a getsockopt interface for the hash ciphers to
answer such information requests from user space.

The kernel crypto API function calls are used to obtain the real data.
As all data are simple integer values, the getsockopt handler function
uses put_user() to return the integer value to user space in the
*optval parameter of getsockopt.

A fully working example using the digestsize interface is provided at
http://www.chronox.de/libkcapi.html

Signed-off-by: Stephan Mueller <smueller@chronox.de>
---
 crypto/algif_hash.c         | 35 ++++++++++++++++++++++++++++++++++-
 include/uapi/linux/if_alg.h |  1 +
 2 files changed, 35 insertions(+), 1 deletion(-)

diff --git a/crypto/algif_hash.c b/crypto/algif_hash.c
index f75db4c..68ae34c 100644
--- a/crypto/algif_hash.c
+++ b/crypto/algif_hash.c
@@ -216,6 +216,39 @@ static int hash_accept(struct socket *sock, struct socket *newsock, int flags)
 	return err;
 }
 
+static int hash_getsockopt(struct socket *sock, int level, int optname,
+			   char __user *optval, int __user *optlen)
+{
+	struct sock *sk = sock->sk;
+	struct alg_sock *ask = alg_sk(sk);
+	struct hash_ctx *ctx = ask->private;
+	const struct af_alg_type *type;
+	int len = 0;
+	int err = -EOPNOTSUPP;
+
+	lock_sock(sk);
+	type = ask->type;
+
+	if (level != SOL_ALG || !type)
+		goto unlock;
+
+	switch (optname) {
+	case ALG_GET_DIGESTSIZE:
+		len = crypto_ahash_digestsize(crypto_ahash_reqtfm(&ctx->req));
+		err = 0;
+		break;
+	default:
+		break;
+	}
+
+unlock:
+	release_sock(sk);
+	if (err >= 0)
+		err = put_user(len, optlen);
+
+	return err;
+}
+
 static struct proto_ops algif_hash_ops = {
 	.family		=	PF_ALG,
 
@@ -225,7 +258,6 @@ static struct proto_ops algif_hash_ops = {
 	.ioctl		=	sock_no_ioctl,
 	.listen		=	sock_no_listen,
 	.shutdown	=	sock_no_shutdown,
-	.getsockopt	=	sock_no_getsockopt,
 	.mmap		=	sock_no_mmap,
 	.bind		=	sock_no_bind,
 	.setsockopt	=	sock_no_setsockopt,
@@ -236,6 +268,7 @@ static struct proto_ops algif_hash_ops = {
 	.sendpage	=	hash_sendpage,
 	.recvmsg	=	hash_recvmsg,
 	.accept		=	hash_accept,
+	.getsockopt	=	hash_getsockopt,
 };
 
 static void *hash_bind(const char *name, u32 type, u32 mask)
diff --git a/include/uapi/linux/if_alg.h b/include/uapi/linux/if_alg.h
index b8fb714..3759d3c 100644
--- a/include/uapi/linux/if_alg.h
+++ b/include/uapi/linux/if_alg.h
@@ -44,6 +44,7 @@ struct af_alg_aead_assoc {
 #define ALG_GET_BLOCKSIZE		1
 #define ALG_GET_IVSIZE			2
 #define ALG_GET_AEAD_AUTHSIZE		3
+#define ALG_GET_DIGESTSIZE		4
 
 /* Operations */
 #define ALG_OP_DECRYPT			0
-- 
2.1.0

[PATCH v2 10/10] crypto: AF_ALG: document the user space interface

[Stephan Mueller]

The extension of the user space interface documentation covers all
aspects of the patchset, including:

	* AEAD cipher interface

	* RNG cipher interface

	* getsockopt interface

Signed-off-by: Stephan Mueller <smueller@chronox.de>
---
 Documentation/crypto/crypto-API-userspace.txt | 95 ++++++++++++++++++++++++++-
 1 file changed, 93 insertions(+), 2 deletions(-)

diff --git a/Documentation/crypto/crypto-API-userspace.txt b/Documentation/crypto/crypto-API-userspace.txt
index ac619cd..5090b05 100644
--- a/Documentation/crypto/crypto-API-userspace.txt
+++ b/Documentation/crypto/crypto-API-userspace.txt
@@ -30,8 +30,9 @@ ciphers are accessible:
 
 	* Symmetric ciphers
 
-Note, AEAD ciphers are currently not supported via the symmetric cipher
-interface.
+	* AEAD ciphers
+
+	* Random number generators
 
 The interface is provided via Netlink using the type AF_ALG. In addition, the
 setsockopt option type is SOL_ALG. In case the user space header files do not
@@ -85,6 +86,7 @@ If a consumer on the other hand wants to maintain the plaintext and the
 ciphertext in different memory locations, all a consumer needs to do is to
 provide different memory pointers for the encryption and decryption operation.
 
+
 Message digest API
 ==================
 
@@ -169,6 +171,69 @@ as large as to hold all blocks of the encrypted or decrypted data. If the output
 data size is smaller, only as many blocks are returned that fit into that
 output buffer size.
 
+
+AEAD cipher API
+===============
+
+The operation is identical to the symmetric cipher API. However, an AEAD
+cipher requires additional information, such as the authentication tag and
+the associated data. This data is to be supplied in addition to the normal
+symmetric cipher data like key and IV discussed for the symmetric ciphers.
+
+During initialization, the struct sockaddr data structure must be filled as
+follows:
+
+struct sockaddr_alg sa = {
+	.salg_family = AF_ALG,
+	.salg_type = "aead", /* this selects the AEAD cipher */
+	.salg_name = "gcm(aes)" /* this is the cipher name */
+};
+
+The discussion about the sendmsg given for the symmetric cipher applies for
+the AEAD interface as well. In addition to the plaintext / ciphertext data and
+the IV, the following data must be supplied with the cmsghdr data structure:
+
+	* The AEAD authentication tag size is set with the flag
+	  ALG_SET_AEAD_AUTHSIZE. The integer value of the authentication tag
+	  size must be provided in the data field of the cmsghdr structure.
+
+	* The AEAD associated data is set with the flag ALG_SET_AEAD_ASSOC.
+	  The data is set the same way as for the IV by supplying the associated
+	  data in the data field of the cmsghdr structure.
+
+The authentication tag itself, however, is handled in a different way to comply
+with the specifics of the kernel crypto API and to avoid copying the
+authentication tag around in memory. The authentication tag is added to the
+memory that immediately follows the ciphertext.
+
+	* When performing an encryption operation, the resulting ciphertext
+	  buffer will hold the tag as follows: ciphertext || tag.  The consumer
+	  must ensure that the ciphertext buffer is large enough to hold the
+	  ciphertext together with the tag of the size set by the consumer using
+	  the ALG_SET_AEAD_AUTHSIZE cmsghdr flag as discussed above.
+
+	* When performing a decryption operation, the initial ciphertext buffer
+	  must hold the tag as follows: ciphertext || tag. The resulting
+	  plaintext has the same size as the ciphertext.
+
+Note: Authentication errors during decryption are marked with a failing
+read/recv system call whose errno is set to EBADMSG.
+
+
+Random number generator API
+===========================
+
+Compared to the symmetric ciphers, the random number generator API is simple:
+it only supports the system calls of read/recv.
+
+The consumer must observe the returned size of the read/recv system calls and
+potentially make subsequent calls if the returned length of random numbers is
+smaller than the expected length.
+
+When initializing a random number generator instance, the AF_ALG interface
+handler ensures that it is appropriately seeded.
+
+
 Setsockopt interface
 ====================
 
@@ -190,6 +255,32 @@ optname:
 
 		- the hash cipher type (keyed message digests)
 
+
+Getsockopt interface
+====================
+
+To allow consumers to obtain information about the allocated cipher, the
+following getsockopt calls with the optval set to the listed flags are provided.
+
+The getsockopt system call must be invoked with the file descriptor of the open
+cipher (i.e. the file descriptor returned by the accept system call).
+
+	* ALG_GET_BLOCKSIZE -- Return the block size of the cipher in the
+	  optlen field of getsockopt. This call is only applicable to
+	  symmetric and AEAD ciphers.
+
+	* ALG_GET_IVSIZE -- Return the IV size of the cipher in the optlen field
+	  of getsockopt. This call is only applicable to symmetric and AEAD
+	  ciphers.
+
+	* ALG_GET_AEAD_AUTHSIZE -- Return the maximum supported authentication
+	  tag size in the optlen field of getsockopt. This call is only
+	  applicable to AEAD ciphers.
+
+	* ALG_GET_DIGESTSIZE -- Return the digest size in the optlen field of
+	  getsockopt. This call is only applicable to message digests.
+
+
 User space API example
 ======================
 
-- 
2.1.0