Re: [PATCH v2 3/5] crypto: AF_ALG -- add setpubkey setsockopt call

Am Freitag, 30. Oktober 2015, 17:16:47 schrieb Marcel Holtmann:

Hi Marcel,

Hi Stephan,

quoted

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

Signed-off-by: Stephan Mueller <redacted>
---
crypto/af_alg.c         | 14 +++++++++++---
include/crypto/if_alg.h |  1 +
2 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/crypto/af_alg.c b/crypto/af_alg.c
index a8e7aa3..bf6528e 100644
--- a/crypto/af_alg.c
+++ b/crypto/af_alg.c

@@ -173,13 +173,16 @@ static int alg_bind(struct socket *sock, struct

sockaddr *uaddr, int addr_len) }

static int alg_setkey(struct sock *sk, char __user *ukey,
-		      unsigned int keylen)
+		      unsigned int keylen, bool pubkey)
{

	struct alg_sock *ask = alg_sk(sk);
	const struct af_alg_type *type = ask->type;
	u8 *key;
	int err;

+	if (pubkey && !type->setpubkey)
+		return -EOPNOTSUPP;
+

	key = sock_kmalloc(sk, keylen, GFP_KERNEL);
	if (!key)
	
		return -ENOMEM;

@@ -188,7 +191,10 @@ static int alg_setkey(struct sock *sk, char __user

*ukey,> 
	if (copy_from_user(key, ukey, keylen))
	
		goto out;

-	err = type->setkey(ask->private, key, keylen);
+	if (pubkey)
+		err = type->setpubkey(ask->private, key, keylen);
+	else
+		err = type->setkey(ask->private, key, keyless);

why is this kind of hackery needed? Why not just introduce alg_setpubkey to
keep this a lot cleaner.

You are fully correct. I wanted to spare a re-implementation of the setkey 
function. But instead of that hack, having something like the following would 
be much cleaner:

setkey_common() {
...
	heavy lifting
...
}

setpubkey() {
	setkey_common(type->setpubkey)
}

setkey() {
	setkey_common(type->setkey)
}

quoted

out:
	sock_kzfree_s(sk, key, keylen);

@@ -212,12 +218,14 @@ static int alg_setsockopt(struct socket *sock, int

level, int optname,> 
	switch (optname) {

	case ALG_SET_KEY:
+	case ALG_SET_PUBKEY:
		if (sock->state == SS_CONNECTED)
		
			goto unlock;
		
		if (!type->setkey)
		
			goto unlock;

-		err = alg_setkey(sk, optval, optlen);
+		err = alg_setkey(sk, optval, optlen,
+				 (optname == ALG_SET_PUBKEY) ? true : false);

		break;

Same here. Why not give ALG_SET_PUBKEY a separate case statement. Especially
since you have to check type->setkey vs type->setpubkey.

Same here.

quoted

	case ALG_SET_AEAD_AUTHSIZE:
		if (sock->state == SS_CONNECTED)

diff --git a/include/crypto/if_alg.h b/include/crypto/if_alg.h
index 018afb2..ca4dc72 100644
--- a/include/crypto/if_alg.h
+++ b/include/crypto/if_alg.h

@@ -49,6 +49,7 @@ struct af_alg_type {

	void *(*bind)(const char *name, u32 type, u32 mask);
	void (*release)(void *private);
	int (*setkey)(void *private, const u8 *key, unsigned int keylen);

+	int (*setpubkey)(void *private, const u8 *key, unsigned int keylen);

	int (*accept)(void *private, struct sock *sk);
	int (*setauthsize)(void *private, unsigned int authorize);

Regards

Marcel

Thanks.

Ciao
Stephan