Messages in this thread Patch in this message | | | From | Stephan Mueller <> | Subject | Re: [PATCH v2 3/5] crypto: AF_ALG -- add setpubkey setsockopt call | Date | Fri, 30 Oct 2015 09:42:27 +0100 |
| |
Am Freitag, 30. Oktober 2015, 17:16:47 schrieb Marcel Holtmann:
Hi Marcel,
>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@chronox.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.
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) }
>> 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.
>> 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
| |