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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Fri, 30 Oct 2015 17:16:47 +0900
From:	Marcel Holtmann <marcel@...tmann.org>
To:	Stephan Mueller <smueller@...onox.de>
Cc:	Herbert Xu <herbert@...dor.apana.org.au>,
	linux-crypto@...r.kernel.org, linux-kernel@...r.kernel.org,
	linux-api@...r.kernel.org
Subject: Re: [PATCH v2 3/5] crypto: AF_ALG -- add setpubkey setsockopt call

Hi Stephan,

> For supporting asymmetric ciphers, user space must be able to set the
> public key. The patch adds a new setsockopt call for setting the public
> key.
> 
> Signed-off-by: Stephan Mueller <smueller@...onox.de>
> ---
> 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.

> 
> 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.

> 	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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/