From: Tadeusz Struk <hidden> Date: 2016-05-05 19:51:40
First four patches are a resend of the v3 algif_akcipher from
Stephan Mueller, with minor changes after rebase on top of 4.6-rc1.
The next three patches add support for keys stored in system
keyring subsystem.
First patch adds algif_akcipher nokey hadlers.
Second patch adds generic sign, verify, encrypt, decrypt accessors
functions to the asymmetric key type. These will be defined by
asymmetric subtypes, similarly to how public_key currently defines
the verify_signature function.
Third patch adds support for ALG_SET_KEY_ID and ALG_SET_PUBKEY_ID
commands to AF_ALG and setkeyid operation to the af_alg_type struct.
If the keyid is used then the afalg layer acquires the key for the
keyring subsystem and uses the new asymmetric accessor functions
instead of akcipher api. The asymmetric subtypes can use akcipher
api internally.
This is the same v5 version as before rebased on top of
http://git.kernel.org/cgit/linux/kernel/git/dhowells/linux-fs.git/log/?h=keys-asym-keyctl
v5 changes:
- drop public key changes and use new version provided by David
v4 changes:
- don't use internal public_key struct in af_alg.
- add generic accessor functions to asymmetric key type, which take
the generic struct key type and resolve the specific subtype internally
v3 changes:
- include Stephan's patches (rebased on 4.6-rc1)
- add algif_akcipher nokey hadlers
- add public_key info struct to public_key and helper query functions
- add a check if a key is a software accessible key on af_alg, and
return -ENOKEY if it isn't
v2 changes:
- pass the original skcipher request in ablkcipher.base.data instead of
casting it back from the ablkcipher request.
- rename _req to base_req
- dropped 3/3
---
Stephan Mueller (4):
crypto: AF_ALG -- add sign/verify API
crypto: AF_ALG -- add setpubkey setsockopt call
crypto: AF_ALG -- add asymmetric cipher interface
crypto: algif_akcipher - enable compilation
Tadeusz Struk (2):
crypto: algif_akcipher - add ops_nokey
crypto: AF_ALG - add support for key_id
crypto/Kconfig | 9
crypto/Makefile | 1
crypto/af_alg.c | 28 +
crypto/algif_akcipher.c | 884 +++++++++++++++++++++++++++++++++++++++++++
include/crypto/if_alg.h | 2
include/uapi/linux/if_alg.h | 5
6 files changed, 924 insertions(+), 5 deletions(-)
create mode 100644 crypto/algif_akcipher.c
--
TS
From: Tadeusz Struk <hidden> Date: 2016-05-05 19:51:47
From: Stephan Mueller <redacted>
Add the flags for handling signature generation and signature
verification.
Also, the patch adds the interface for setting a public key.
Signed-off-by: Stephan Mueller <redacted>
Signed-off-by: Tadeusz Struk <redacted>
---
include/uapi/linux/if_alg.h | 3 +++
1 file changed, 3 insertions(+)
From: Tadeusz Struk <hidden> Date: 2016-05-05 19:51:56
From: Stephan Mueller <redacted>
This patch adds the user space interface for asymmetric ciphers. The
interface allows the use of sendmsg as well as vmsplice to provide data.
This version has been rebased on top of 4.6 and a few chackpatch issues
have been fixed.
Signed-off-by: Stephan Mueller <redacted>
Signed-off-by: Tadeusz Struk <redacted>
---
crypto/algif_akcipher.c | 542 +++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 542 insertions(+)
create mode 100644 crypto/algif_akcipher.c
@@ -0,0 +1,542 @@+/*+*algif_akcipher:User-spaceinterfaceforasymmetriccipheralgorithms+*+*Copyright(C)2015,StephanMueller<smueller@chronox.de>+*+*Thisfileprovidestheuser-spaceAPIforasymmetricciphers.+*+*Thisprogramisfreesoftware;youcanredistributeitand/ormodifyit+*underthetermsoftheGNUGeneralPublicLicenseaspublishedbytheFree+*SoftwareFoundation;eitherversion2oftheLicense,or(atyouroption)+*anylaterversion.+*/++#include<crypto/akcipher.h>+#include<crypto/scatterwalk.h>+#include<crypto/if_alg.h>+#include<linux/init.h>+#include<linux/list.h>+#include<linux/kernel.h>+#include<linux/mm.h>+#include<linux/module.h>+#include<linux/net.h>+#include<net/sock.h>++structakcipher_sg_list{+unsignedintcur;+structscatterlistsg[ALG_MAX_PAGES];+};++structakcipher_ctx{+structakcipher_sg_listtsgl;+structaf_alg_sglrsgl[ALG_MAX_PAGES];++structaf_alg_completioncompletion;++unsignedlongused;++unsignedintlen;+boolmore;+boolmerge;+intop;++structakcipher_requestreq;+};++staticinlineintakcipher_sndbuf(structsock*sk)+{+structalg_sock*ask=alg_sk(sk);+structakcipher_ctx*ctx=ask->private;++returnmax_t(int,max_t(int,sk->sk_sndbuf&PAGE_MASK,PAGE_SIZE)-+ctx->used,0);+}++staticinlineboolakcipher_writable(structsock*sk)+{+returnakcipher_sndbuf(sk)>=PAGE_SIZE;+}++staticinlineintakcipher_calcsize(structakcipher_ctx*ctx)+{+returncrypto_akcipher_maxsize(crypto_akcipher_reqtfm(&ctx->req));+}++staticvoidakcipher_put_sgl(structsock*sk)+{+structalg_sock*ask=alg_sk(sk);+structakcipher_ctx*ctx=ask->private;+structakcipher_sg_list*sgl=&ctx->tsgl;+structscatterlist*sg=sgl->sg;+unsignedinti;++for(i=0;i<sgl->cur;i++){+if(!sg_page(sg+i))+continue;++put_page(sg_page(sg+i));+sg_assign_page(sg+i,NULL);+}+sg_init_table(sg,ALG_MAX_PAGES);+sgl->cur=0;+ctx->used=0;+ctx->more=0;+ctx->merge=0;+}++staticvoidakcipher_wmem_wakeup(structsock*sk)+{+structsocket_wq*wq;++if(!akcipher_writable(sk))+return;++rcu_read_lock();+wq=rcu_dereference(sk->sk_wq);+if(wq_has_sleeper(&wq->wait))+wake_up_interruptible_sync_poll(&wq->wait,POLLIN|+POLLRDNORM|+POLLRDBAND);+sk_wake_async(sk,SOCK_WAKE_WAITD,POLL_IN);+rcu_read_unlock();+}++staticintakcipher_wait_for_data(structsock*sk,unsignedintflags)+{+structalg_sock*ask=alg_sk(sk);+structakcipher_ctx*ctx=ask->private;+longtimeout;+DEFINE_WAIT(wait);+interr=-ERESTARTSYS;++if(flags&MSG_DONTWAIT)+return-EAGAIN;++set_bit(SOCKWQ_ASYNC_WAITDATA,&sk->sk_socket->flags);++for(;;){+if(signal_pending(current))+break;+prepare_to_wait(sk_sleep(sk),&wait,TASK_INTERRUPTIBLE);+timeout=MAX_SCHEDULE_TIMEOUT;+if(sk_wait_event(sk,&timeout,!ctx->more)){+err=0;+break;+}+}+finish_wait(sk_sleep(sk),&wait);++clear_bit(SOCKWQ_ASYNC_WAITDATA,&sk->sk_socket->flags);++returnerr;+}++staticvoidakcipher_data_wakeup(structsock*sk)+{+structalg_sock*ask=alg_sk(sk);+structakcipher_ctx*ctx=ask->private;+structsocket_wq*wq;++if(ctx->more)+return;+if(!ctx->used)+return;++rcu_read_lock();+wq=rcu_dereference(sk->sk_wq);+if(wq_has_sleeper(&wq->wait))+wake_up_interruptible_sync_poll(&wq->wait,POLLOUT|+POLLRDNORM|+POLLRDBAND);+sk_wake_async(sk,SOCK_WAKE_SPACE,POLL_OUT);+rcu_read_unlock();+}++staticintakcipher_sendmsg(structsocket*sock,structmsghdr*msg,+size_tsize)+{+structsock*sk=sock->sk;+structalg_sock*ask=alg_sk(sk);+structakcipher_ctx*ctx=ask->private;+structakcipher_sg_list*sgl=&ctx->tsgl;+structaf_alg_controlcon={};+longcopied=0;+intop=0;+boolinit=0;+interr;++if(msg->msg_controllen){+err=af_alg_cmsg_send(msg,&con);+if(err)+returnerr;++init=1;+switch(con.op){+caseALG_OP_VERIFY:+caseALG_OP_SIGN:+caseALG_OP_ENCRYPT:+caseALG_OP_DECRYPT:+op=con.op;+break;+default:+return-EINVAL;+}+}++lock_sock(sk);+if(!ctx->more&&ctx->used)+gotounlock;++if(init)+ctx->op=op;++while(size){+unsignedlonglen=size;+structscatterlist*sg=NULL;++/* use the existing memory in an allocated page */+if(ctx->merge){+sg=sgl->sg+sgl->cur-1;+len=min_t(unsignedlong,len,+PAGE_SIZE-sg->offset-sg->length);+err=memcpy_from_msg(page_address(sg_page(sg))++sg->offset+sg->length,+msg,len);+if(err)+gotounlock;++sg->length+=len;+ctx->merge=(sg->offset+sg->length)&+(PAGE_SIZE-1);++ctx->used+=len;+copied+=len;+size-=len;+continue;+}++if(!akcipher_writable(sk)){+/* user space sent too much data */+akcipher_put_sgl(sk);+err=-EMSGSIZE;+gotounlock;+}++/* allocate a new page */+len=min_t(unsignedlong,size,akcipher_sndbuf(sk));+while(len){+intplen=0;++if(sgl->cur>=ALG_MAX_PAGES){+akcipher_put_sgl(sk);+err=-E2BIG;+gotounlock;+}++sg=sgl->sg+sgl->cur;+plen=min_t(int,len,PAGE_SIZE);++sg_assign_page(sg,alloc_page(GFP_KERNEL));+if(!sg_page(sg)){+err=-ENOMEM;+gotounlock;+}++err=memcpy_from_msg(page_address(sg_page(sg)),+msg,plen);+if(err){+__free_page(sg_page(sg));+sg_assign_page(sg,NULL);+gotounlock;+}++sg->offset=0;+sg->length=plen;+len-=plen;+ctx->used+=plen;+copied+=plen;+sgl->cur++;+size-=plen;+ctx->merge=plen&(PAGE_SIZE-1);+}+}++err=0;++ctx->more=msg->msg_flags&MSG_MORE;++unlock:+akcipher_data_wakeup(sk);+release_sock(sk);++returnerr?:copied;+}++staticssize_takcipher_sendpage(structsocket*sock,structpage*page,+intoffset,size_tsize,intflags)+{+structsock*sk=sock->sk;+structalg_sock*ask=alg_sk(sk);+structakcipher_ctx*ctx=ask->private;+structakcipher_sg_list*sgl=&ctx->tsgl;+interr=0;++if(flags&MSG_SENDPAGE_NOTLAST)+flags|=MSG_MORE;++if(sgl->cur>=ALG_MAX_PAGES)+return-E2BIG;++lock_sock(sk);+if(!ctx->more&&ctx->used)+gotounlock;++if(!size)+gotodone;++if(!akcipher_writable(sk)){+/* user space sent too much data */+akcipher_put_sgl(sk);+err=-EMSGSIZE;+gotounlock;+}++ctx->merge=0;++get_page(page);+sg_set_page(sgl->sg+sgl->cur,page,size,offset);+sgl->cur++;+ctx->used+=size;++done:+ctx->more=flags&MSG_MORE;+unlock:+akcipher_data_wakeup(sk);+release_sock(sk);++returnerr?err:size;+}++staticintakcipher_recvmsg(structsocket*sock,structmsghdr*msg,+size_tignored,intflags)+{+structsock*sk=sock->sk;+structalg_sock*ask=alg_sk(sk);+structakcipher_ctx*ctx=ask->private;+structakcipher_sg_list*sgl=&ctx->tsgl;+unsignedinti=0;+interr;+unsignedlongused=0;+size_tusedpages=0;+unsignedintcnt=0;++/* Limit number of IOV blocks to be accessed below */+if(msg->msg_iter.nr_segs>ALG_MAX_PAGES)+return-ENOMSG;++lock_sock(sk);++if(ctx->more){+err=akcipher_wait_for_data(sk,flags);+if(err)+gotounlock;+}++used=ctx->used;++/* convert iovecs of output buffers into scatterlists */+while(iov_iter_count(&msg->msg_iter)){+/* make one iovec available as scatterlist */+err=af_alg_make_sg(&ctx->rsgl[cnt],&msg->msg_iter,+iov_iter_count(&msg->msg_iter));+if(err<0)+gotounlock;+usedpages+=err;+/* chain the new scatterlist with previous one */+if(cnt)+af_alg_link_sg(&ctx->rsgl[cnt-1],&ctx->rsgl[cnt]);++iov_iter_advance(&msg->msg_iter,err);+cnt++;+}++/* ensure output buffer is sufficiently large */+if(usedpages<akcipher_calcsize(ctx)){+err=-EMSGSIZE;+gotounlock;+}++sg_mark_end(sgl->sg+sgl->cur-1);++akcipher_request_set_crypt(&ctx->req,sgl->sg,ctx->rsgl[0].sg,used,+usedpages);+switch(ctx->op){+caseALG_OP_VERIFY:+err=crypto_akcipher_verify(&ctx->req);+break;+caseALG_OP_SIGN:+err=crypto_akcipher_sign(&ctx->req);+break;+caseALG_OP_ENCRYPT:+err=crypto_akcipher_encrypt(&ctx->req);+break;+caseALG_OP_DECRYPT:+err=crypto_akcipher_decrypt(&ctx->req);+break;+default:+err=-EFAULT;+gotounlock;+}++err=af_alg_wait_for_completion(err,&ctx->completion);++if(err){+/* EBADMSG implies a valid cipher operation took place */+if(err==-EBADMSG)+akcipher_put_sgl(sk);+gotounlock;+}++akcipher_put_sgl(sk);++unlock:+for(i=0;i<cnt;i++)+af_alg_free_sg(&ctx->rsgl[i]);++akcipher_wmem_wakeup(sk);+release_sock(sk);++returnerr?err:ctx->req.dst_len;+}++staticunsignedintakcipher_poll(structfile*file,structsocket*sock,+poll_table*wait)+{+structsock*sk=sock->sk;+structalg_sock*ask=alg_sk(sk);+structakcipher_ctx*ctx=ask->private;+unsignedintmask=0;++sock_poll_wait(file,sk_sleep(sk),wait);++if(!ctx->more)+mask|=POLLIN|POLLRDNORM;++if(akcipher_writable(sk))+mask|=POLLOUT|POLLWRNORM|POLLWRBAND;++returnmask;+}++staticstructproto_opsalgif_akcipher_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,++.release=af_alg_release,+.sendmsg=akcipher_sendmsg,+.sendpage=akcipher_sendpage,+.recvmsg=akcipher_recvmsg,+.poll=akcipher_poll,+};++staticvoid*akcipher_bind(constchar*name,u32type,u32mask)+{+returncrypto_alloc_akcipher(name,type,mask);+}++staticvoidakcipher_release(void*private)+{+crypto_free_akcipher(private);+}++staticintakcipher_setprivkey(void*private,constu8*key,+unsignedintkeylen)+{+returncrypto_akcipher_set_priv_key(private,key,keylen);+}++staticintakcipher_setpubkey(void*private,constu8*key,unsignedintkeylen)+{+returncrypto_akcipher_set_pub_key(private,key,keylen);+}++staticvoidakcipher_sock_destruct(structsock*sk)+{+structalg_sock*ask=alg_sk(sk);+structakcipher_ctx*ctx=ask->private;++akcipher_put_sgl(sk);+sock_kfree_s(sk,ctx,ctx->len);+af_alg_release_parent(sk);+}++staticintakcipher_accept_parent(void*private,structsock*sk)+{+structakcipher_ctx*ctx;+structalg_sock*ask=alg_sk(sk);+unsignedintlen=sizeof(*ctx)+crypto_akcipher_reqsize(private);++ctx=sock_kmalloc(sk,len,GFP_KERNEL);+if(!ctx)+return-ENOMEM;+memset(ctx,0,len);++ctx->len=len;+ctx->used=0;+ctx->more=0;+ctx->merge=0;+ctx->op=0;+ctx->tsgl.cur=0;+af_alg_init_completion(&ctx->completion);+sg_init_table(ctx->tsgl.sg,ALG_MAX_PAGES);++ask->private=ctx;++akcipher_request_set_tfm(&ctx->req,private);+akcipher_request_set_callback(&ctx->req,CRYPTO_TFM_REQ_MAY_BACKLOG,+af_alg_complete,&ctx->completion);++sk->sk_destruct=akcipher_sock_destruct;++return0;+}++staticconststructaf_alg_typealgif_type_akcipher={+.bind=akcipher_bind,+.release=akcipher_release,+.setkey=akcipher_setprivkey,+.setpubkey=akcipher_setpubkey,+.accept=akcipher_accept_parent,+.ops=&algif_akcipher_ops,+.name="akcipher",+.owner=THIS_MODULE+};++staticint__initalgif_akcipher_init(void)+{+returnaf_alg_register_type(&algif_type_akcipher);+}++staticvoid__exitalgif_akcipher_exit(void)+{+interr=af_alg_unregister_type(&algif_type_akcipher);++WARN_ON(err);+}++module_init(algif_akcipher_init);+module_exit(algif_akcipher_exit);+MODULE_LICENSE("GPL");+MODULE_AUTHOR("Stephan Mueller <smueller@chronox.de>");+MODULE_DESCRIPTION("Asymmetric kernel crypto API user space interface");
From: Tadeusz Struk <hidden> Date: 2016-05-05 19:52:04
From: Stephan Mueller <redacted>
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 | 18 +++++++++++++-----
include/crypto/if_alg.h | 1 +
2 files changed, 14 insertions(+), 5 deletions(-)
@@ -247,10 +251,14 @@ static int alg_setsockopt(struct socket *sock, int level, int optname,caseALG_SET_KEY:if(sock->state==SS_CONNECTED)gotounlock;-if(!type->setkey)++err=alg_setkey(sk,optval,optlen,type->setkey);+break;+caseALG_SET_PUBKEY:+if(sock->state==SS_CONNECTED)gotounlock;-err=alg_setkey(sk,optval,optlen);+err=alg_setkey(sk,optval,optlen,type->setpubkey);break;caseALG_SET_AEAD_AUTHSIZE:if(sock->state==SS_CONNECTED)
From: Tadeusz Struk <hidden> Date: 2016-05-05 19:52:10
Similar to algif_skcipher and algif_hash, algif_akcipher needs
to prevent user space from using the interface in an improper way.
This patch adds nokey ops handlers, which do just that.
Signed-off-by: Tadeusz Struk <redacted>
---
crypto/algif_akcipher.c | 159 +++++++++++++++++++++++++++++++++++++++++++++--
1 file changed, 152 insertions(+), 7 deletions(-)
From: Tadeusz Struk <hidden> Date: 2016-05-05 19:52:22
From: Stephan Mueller <redacted>
Add the Makefile and Kconfig updates to allow algif_akcipher to be
compiled.
Signed-off-by: Stephan Mueller <redacted>
Signed-off-by: Tadeusz Struk <redacted>
---
crypto/Kconfig | 9 +++++++++
crypto/Makefile | 1 +
2 files changed, 10 insertions(+)
@@ -121,6 +121,7 @@ obj-$(CONFIG_CRYPTO_USER_API_HASH) += algif_hash.oobj-$(CONFIG_CRYPTO_USER_API_SKCIPHER)+=algif_skcipher.oobj-$(CONFIG_CRYPTO_USER_API_RNG)+=algif_rng.oobj-$(CONFIG_CRYPTO_USER_API_AEAD)+=algif_aead.o+obj-$(CONFIG_CRYPTO_USER_API_AKCIPHER)+=algif_akcipher.o## generic algorithms and the async_tx api
From: Tadeusz Struk <hidden> Date: 2016-05-05 19:53:10
This patch adds support for asymmetric key type to AF_ALG.
It will work as follows: A new PF_ALG socket options are
added on top of existing ALG_SET_KEY and ALG_SET_PUBKEY, namely
ALG_SET_KEY_ID and ALG_SET_PUBKEY_ID for setting public and
private keys respectively. When these new options will be used
the user, instead of providing the key material, will provide a
key id and the key itself will be obtained from kernel keyring
subsystem. The user will use the standard tools (keyctl tool
or the keyctl syscall) for key instantiation and to obtain the
key id. The key id can also be obtained by reading the
/proc/keys file.
When a key corresponding to the given keyid is found, it is stored
in the socket context and subsequent crypto operation invoked by the
user will use the new asymmetric accessor functions instead of akcipher
api. The asymmetric subtype can internally use akcipher api or
invoke operations defined by a given subtype, depending on the
key type.
Signed-off-by: Tadeusz Struk <redacted>
---
crypto/af_alg.c | 10 ++
crypto/algif_akcipher.c | 207 ++++++++++++++++++++++++++++++++++++++++++-
include/crypto/if_alg.h | 1
include/uapi/linux/if_alg.h | 2
4 files changed, 215 insertions(+), 5 deletions(-)
@@ -260,6 +260,16 @@ static int alg_setsockopt(struct socket *sock, int level, int optname,err=alg_setkey(sk,optval,optlen,type->setpubkey);break;++caseALG_SET_KEY_ID:+caseALG_SET_PUBKEY_ID:+/* ALG_SET_KEY_ID is only for akcipher */+if(!strcmp(type->name,"akcipher")||+sock->state==SS_CONNECTED)+gotounlock;++err=alg_setkey(sk,optval,optlen,type->setkeyid);+break;caseALG_SET_AEAD_AUTHSIZE:if(sock->state==SS_CONNECTED)gotounlock;
@@ -322,6 +326,153 @@ unlock:returnerr?err:size;}+staticintasym_key_encrypt(conststructkey*key,structakcipher_request*req)+{+structkernel_pkey_paramsparams={0};+char*src=NULL,*dst=NULL,*in,*out;+intret;++if(!sg_is_last(req->src)){+src=kmalloc(req->src_len,GFP_KERNEL);+if(!src)+return-ENOMEM;+scatterwalk_map_and_copy(src,req->src,0,req->src_len,0);+in=src;+}else{+in=sg_virt(req->src);+}+if(!sg_is_last(req->dst)){+dst=kmalloc(req->dst_len,GFP_KERNEL);+if(!dst){+kfree(src);+return-ENOMEM;+}+out=dst;+}else{+out=sg_virt(req->dst);+}+params.key=(structkey*)key;+params.data_len=req->src_len;+params.enc_len=req->dst_len;+ret=encrypt_blob(¶ms,in,out);+if(ret)+gotofree;++if(dst)+scatterwalk_map_and_copy(dst,req->dst,0,req->dst_len,1);+free:+kfree(src);+kfree(dst);+returnret;+}++staticintasym_key_decrypt(conststructkey*key,structakcipher_request*req)+{+structkernel_pkey_paramsparams={0};+char*src=NULL,*dst=NULL,*in,*out;+intret;++if(!sg_is_last(req->src)){+src=kmalloc(req->src_len,GFP_KERNEL);+if(!src)+return-ENOMEM;+scatterwalk_map_and_copy(src,req->src,0,req->src_len,0);+in=src;+}else{+in=sg_virt(req->src);+}+if(!sg_is_last(req->dst)){+dst=kmalloc(req->dst_len,GFP_KERNEL);+if(!dst){+kfree(src);+return-ENOMEM;+}+out=dst;+}else{+out=sg_virt(req->dst);+}+params.key=(structkey*)key;+params.data_len=req->src_len;+params.enc_len=req->dst_len;+ret=decrypt_blob(¶ms,in,out);+if(ret)+gotofree;++if(dst)+scatterwalk_map_and_copy(dst,req->dst,0,req->dst_len,1);+free:+kfree(src);+kfree(dst);+returnret;+}++staticintasym_key_sign(conststructkey*key,structakcipher_request*req)+{+structkernel_pkey_paramsparams={0};+char*src=NULL,*dst=NULL,*in,*out;+intret;++if(!sg_is_last(req->src)){+src=kmalloc(req->src_len,GFP_KERNEL);+if(!src)+return-ENOMEM;+scatterwalk_map_and_copy(src,req->src,0,req->src_len,0);+in=src;+}else{+in=sg_virt(req->src);+}+if(!sg_is_last(req->dst)){+dst=kmalloc(req->dst_len,GFP_KERNEL);+if(!dst){+kfree(src);+return-ENOMEM;+}+out=dst;+}else{+out=sg_virt(req->dst);+}+params.key=(structkey*)key;+params.data_len=req->src_len;+params.enc_len=req->dst_len;+ret=create_signature(¶ms,in,out);+if(ret)+gotofree;++if(dst)+scatterwalk_map_and_copy(dst,req->dst,0,req->dst_len,1);+free:+kfree(src);+kfree(dst);+returnret;+}++staticintasym_key_verify(conststructkey*key,structakcipher_request*req)+{+structpublic_key_signaturesig;+char*src=NULL,*in;+intret;++if(!sg_is_last(req->src)){+src=kmalloc(req->src_len,GFP_KERNEL);+if(!src)+return-ENOMEM;+scatterwalk_map_and_copy(src,req->src,0,req->src_len,0);+in=src;+}else{+in=sg_virt(req->src);+}+sig.pkey_algo="rsa";+sig.encoding="pkcs1";+/* Need to find a way to pass the hash param */+sig.hash_algo="sha1";+sig.digest_size=20;+sig.s_size=req->src_len;+sig.s=src;+ret=verify_signature(key,NULL,&sig);+kfree(src);+returnret;+}+staticintakcipher_recvmsg(structsocket*sock,structmsghdr*msg,size_tignored,intflags){
@@ -579,6 +742,27 @@ static void akcipher_release(void *private)kfree(tfm);}+staticintakcipher_setkeyid(void*private,constu8*key,unsignedintkeylen)+{+structakcipher_tfm*tfm=private;+structkey*akey;+u32keyid=*((u32*)key);+interr=-ENOKEY;++/* Store the key id and verify that a key with the given id is present.+*Theactualkeywillbeacquiredintheaccept_parentfunction+*/+sprintf(tfm->keyid,"id:%08x",keyid);+akey=request_key(&key_type_asymmetric,tfm->keyid,NULL);+if(IS_ERR(key))+gotoout;++tfm->has_key=true;+key_put(akey);+out:+returnerr;+}+staticintakcipher_setprivkey(void*private,constu8*key,unsignedintkeylen){
From: Stephan Mueller <hidden> Date: 2016-05-06 10:36:33
Am Donnerstag, 5. Mai 2016, 12:50:54 schrieb Tadeusz Struk:
Hi Tadeusz, David,
From: Stephan Mueller <redacted>
Add the flags for handling signature generation and signature
verification.
Also, the patch adds the interface for setting a public key.
Signed-off-by: Stephan Mueller <redacted>
Signed-off-by: Tadeusz Struk <redacted>
All four patches from me:
Acked-by: Stephan Mueller <redacted>
From: Stephan Mueller <hidden> Date: 2016-05-06 14:50:40
Am Donnerstag, 5. Mai 2016, 12:51:20 schrieb Tadeusz Struk:
Hi Tadeusz,
quoted hunk
This patch adds support for asymmetric key type to AF_ALG.
It will work as follows: A new PF_ALG socket options are
added on top of existing ALG_SET_KEY and ALG_SET_PUBKEY, namely
ALG_SET_KEY_ID and ALG_SET_PUBKEY_ID for setting public and
private keys respectively. When these new options will be used
the user, instead of providing the key material, will provide a
key id and the key itself will be obtained from kernel keyring
subsystem. The user will use the standard tools (keyctl tool
or the keyctl syscall) for key instantiation and to obtain the
key id. The key id can also be obtained by reading the
/proc/keys file.
When a key corresponding to the given keyid is found, it is stored
in the socket context and subsequent crypto operation invoked by the
user will use the new asymmetric accessor functions instead of akcipher
api. The asymmetric subtype can internally use akcipher api or
invoke operations defined by a given subtype, depending on the
key type.
Signed-off-by: Tadeusz Struk <redacted>
---
crypto/af_alg.c | 10 ++
crypto/algif_akcipher.c | 207
++++++++++++++++++++++++++++++++++++++++++- include/crypto/if_alg.h |
1
include/uapi/linux/if_alg.h | 2
4 files changed, 215 insertions(+), 5 deletions(-)
*req) +{
+ struct kernel_pkey_params params = {0};
+ char *src = NULL, *dst = NULL, *in, *out;
+ int ret;
+
+ if (!sg_is_last(req->src)) {
+ src = kmalloc(req->src_len, GFP_KERNEL);
+ if (!src)
+ return -ENOMEM;
+ scatterwalk_map_and_copy(src, req->src, 0, req->src_len, 0);
+ in = src;
+ } else {
+ in = sg_virt(req->src);
+ }
+ if (!sg_is_last(req->dst)) {
+ dst = kmalloc(req->dst_len, GFP_KERNEL);
+ if (!dst) {
+ kfree(src);
+ return -ENOMEM;
+ }
+ out = dst;
+ } else {
+ out = sg_virt(req->dst);
+ }
+ params.key = (struct key *)key;
+ params.data_len = req->src_len;
+ params.enc_len = req->dst_len;
+ ret = encrypt_blob(¶ms, in, out);
+ if (ret)
+ goto free;
+
+ if (dst)
+ scatterwalk_map_and_copy(dst, req->dst, 0, req->dst_len, 1);
+free:
+ kfree(src);
+ kfree(dst);
+ return ret;
+}
+
+static int asym_key_decrypt(const struct key *key, struct akcipher_request
*req) +{
+ struct kernel_pkey_params params = {0};
+ char *src = NULL, *dst = NULL, *in, *out;
+ int ret;
+
+ if (!sg_is_last(req->src)) {
+ src = kmalloc(req->src_len, GFP_KERNEL);
+ if (!src)
+ return -ENOMEM;
+ scatterwalk_map_and_copy(src, req->src, 0, req->src_len, 0);
+ in = src;
+ } else {
+ in = sg_virt(req->src);
+ }
+ if (!sg_is_last(req->dst)) {
+ dst = kmalloc(req->dst_len, GFP_KERNEL);
+ if (!dst) {
+ kfree(src);
+ return -ENOMEM;
+ }
+ out = dst;
+ } else {
+ out = sg_virt(req->dst);
+ }
+ params.key = (struct key *)key;
+ params.data_len = req->src_len;
+ params.enc_len = req->dst_len;
+ ret = decrypt_blob(¶ms, in, out);
+ if (ret)
+ goto free;
+
+ if (dst)
+ scatterwalk_map_and_copy(dst, req->dst, 0, req->dst_len, 1);
+free:
+ kfree(src);
+ kfree(dst);
+ return ret;
+}
+
+static int asym_key_sign(const struct key *key, struct akcipher_request
*req) +{
+ struct kernel_pkey_params params = {0};
+ char *src = NULL, *dst = NULL, *in, *out;
+ int ret;
+
+ if (!sg_is_last(req->src)) {
+ src = kmalloc(req->src_len, GFP_KERNEL);
+ if (!src)
+ return -ENOMEM;
+ scatterwalk_map_and_copy(src, req->src, 0, req->src_len, 0);
+ in = src;
+ } else {
+ in = sg_virt(req->src);
+ }
+ if (!sg_is_last(req->dst)) {
+ dst = kmalloc(req->dst_len, GFP_KERNEL);
+ if (!dst) {
+ kfree(src);
+ return -ENOMEM;
+ }
+ out = dst;
+ } else {
+ out = sg_virt(req->dst);
+ }
+ params.key = (struct key *)key;
+ params.data_len = req->src_len;
+ params.enc_len = req->dst_len;
+ ret = create_signature(¶ms, in, out);
+ if (ret)
+ goto free;
+
+ if (dst)
+ scatterwalk_map_and_copy(dst, req->dst, 0, req->dst_len, 1);
+free:
+ kfree(src);
+ kfree(dst);
+ return ret;
+}
+
+static int asym_key_verify(const struct key *key, struct akcipher_request
*req) +{
+ struct public_key_signature sig;
+ char *src = NULL, *in;
+ int ret;
+
+ if (!sg_is_last(req->src)) {
+ src = kmalloc(req->src_len, GFP_KERNEL);
+ if (!src)
+ return -ENOMEM;
+ scatterwalk_map_and_copy(src, req->src, 0, req->src_len, 0);
+ in = src;
+ } else {
+ in = sg_virt(req->src);
+ }
+ sig.pkey_algo = "rsa";
+ sig.encoding = "pkcs1";
+ /* Need to find a way to pass the hash param */
+ sig.hash_algo = "sha1";
This comment shall not hold up any merging with the mainline tree.
I am not yet fully up to speed on the keys framework. But commonly, the
signature's hash type is identical to the hash used for the key. Is there a
way to obtain the key's signature type from the key framework?
@@ -579,6 +742,27 @@ static void akcipher_release(void *private) kfree(tfm); }+static int akcipher_setkeyid(void *private, const u8 *key, unsigned int
keylen) +{
+ struct akcipher_tfm *tfm = private;
+ struct key *akey;
+ u32 keyid = *((u32 *)key);
+ int err = -ENOKEY;
+
+ /* Store the key id and verify that a key with the given id is
present.
quoted hunk
+ * The actual key will be acquired in the accept_parent function
+ */
+ sprintf(tfm->keyid, "id:%08x", keyid);
+ akey = request_key(&key_type_asymmetric, tfm->keyid, NULL);
+ if (IS_ERR(key))
+ goto out;
+
+ tfm->has_key = true;
+ key_put(akey);
+out:
+ return err;
+}
+
static int akcipher_setprivkey(void *private, const u8 *key,
unsigned int keylen)
{
I've just reposted this. The interface you're using should be the same, I
think, but the details underneath have changed.
Also, you can now supply private keys to the kernel if they're PKCS#8 encoded
and keyctls are supplied that do encryption, decryption, signing and
verifying, e.g.:
j=`openssl pkcs8 -in ~/pkcs7/firmwarekey2.priv -topk8 -nocrypt -outform DER | \
keyctl padd asymmetric foo @s`
echo -n abcdefghijklmnopqrst >/tmp/data
keyctl pkey_encrypt $j 0 /tmp/data enc=pkcs1 >/tmp/enc
keyctl pkey_decrypt $j 0 /tmp/enc enc=pkcs1 >/tmp/dec
cmp /tmp/data /tmp/dec
keyctl pkey_sign $j 0 /tmp/data enc=pkcs1 hash=sha1 >/tmp/sig
keyctl pkey_verify $j 0 /tmp/data /tmp/sig enc=pkcs1 hash=sha1
David
The params member names have changed (now in_len and out_len).
+ ret = encrypt_blob(¶ms, in, out);
The encrypt function for the key can now be called with
params.key->type->asym_eds_op(). This also allows you to factor out the
duplication in asym_key_encrypt, asym_key_decrypt, and asym_key_sign. See
keyctl_pkey_e_d_s() in keyctl_pkey.c
+static int asym_key_verify(const struct key *key, struct akcipher_request *req)
...
+ ret = verify_signature(key, NULL, &sig);
key->type->asym_verify_signature() is available as well.
Regards,
--
Mat Martineau
Intel OTC
From: Tadeusz Struk <hidden> Date: 2016-05-15 04:17:51
First four patches are a resend of the v3 algif_akcipher from
Stephan Mueller, with minor changes after rebase on top of 4.6-rc1.
The next three patches add support for keys stored in system
keyring subsystem.
First patch adds algif_akcipher nokey hadlers.
Second patch adds generic sign, verify, encrypt, decrypt accessors
functions to the asymmetric key type. These will be defined by
asymmetric subtypes, similarly to how public_key currently defines
the verify_signature function.
Third patch adds support for ALG_SET_KEY_ID and ALG_SET_PUBKEY_ID
commands to AF_ALG and setkeyid operation to the af_alg_type struct.
If the keyid is used then the afalg layer acquires the key for the
keyring subsystem and uses the new asymmetric accessor functions
instead of akcipher api. The asymmetric subtypes can use akcipher
api internally.
This is the same v5 version as before rebased on top of
http://git.kernel.org/cgit/linux/kernel/git/dhowells/linux-fs.git/log/?h=keys-asym-keyctl
v6 changes:
- update to reflect changes in kernel_pkey_params struct
v5 changes:
- drop public key changes and use new version provided by David
v4 changes:
- don't use internal public_key struct in af_alg.
- add generic accessor functions to asymmetric key type, which take
the generic struct key type and resolve the specific subtype internally
v3 changes:
- include Stephan's patches (rebased on 4.6-rc1)
- add algif_akcipher nokey hadlers
- add public_key info struct to public_key and helper query functions
- add a check if a key is a software accessible key on af_alg, and
return -ENOKEY if it isn't
v2 changes:
- pass the original skcipher request in ablkcipher.base.data instead of
casting it back from the ablkcipher request.
- rename _req to base_req
- dropped 3/3
---
Stephan Mueller (4):
crypto: AF_ALG -- add sign/verify API
crypto: AF_ALG -- add setpubkey setsockopt call
crypto: AF_ALG -- add asymmetric cipher interface
crypto: algif_akcipher - enable compilation
Tadeusz Struk (2):
crypto: algif_akcipher - add ops_nokey
crypto: AF_ALG - add support for key_id
crypto/Kconfig | 9
crypto/Makefile | 1
crypto/af_alg.c | 28 +
crypto/algif_akcipher.c | 884 +++++++++++++++++++++++++++++++++++++++++++
include/crypto/if_alg.h | 2
include/uapi/linux/if_alg.h | 5
6 files changed, 924 insertions(+), 5 deletions(-)
create mode 100644 crypto/algif_akcipher.c
--
TS
From: Tadeusz Struk <hidden> Date: 2016-05-15 04:17:56
From: Stephan Mueller <redacted>
Add the flags for handling signature generation and signature
verification.
Also, the patch adds the interface for setting a public key.
Signed-off-by: Stephan Mueller <redacted>
Signed-off-by: Tadeusz Struk <redacted>
---
include/uapi/linux/if_alg.h | 3 +++
1 file changed, 3 insertions(+)
From: Tadeusz Struk <hidden> Date: 2016-05-15 04:18:13
From: Stephan Mueller <redacted>
Add the Makefile and Kconfig updates to allow algif_akcipher to be
compiled.
Signed-off-by: Stephan Mueller <redacted>
Signed-off-by: Tadeusz Struk <redacted>
---
crypto/Kconfig | 9 +++++++++
crypto/Makefile | 1 +
2 files changed, 10 insertions(+)
@@ -121,6 +121,7 @@ obj-$(CONFIG_CRYPTO_USER_API_HASH) += algif_hash.oobj-$(CONFIG_CRYPTO_USER_API_SKCIPHER)+=algif_skcipher.oobj-$(CONFIG_CRYPTO_USER_API_RNG)+=algif_rng.oobj-$(CONFIG_CRYPTO_USER_API_AEAD)+=algif_aead.o+obj-$(CONFIG_CRYPTO_USER_API_AKCIPHER)+=algif_akcipher.o## generic algorithms and the async_tx api
From: Tadeusz Struk <hidden> Date: 2016-05-15 04:18:14
From: Stephan Mueller <redacted>
This patch adds the user space interface for asymmetric ciphers. The
interface allows the use of sendmsg as well as vmsplice to provide data.
This version has been rebased on top of 4.6 and a few chackpatch issues
have been fixed.
Signed-off-by: Stephan Mueller <redacted>
Signed-off-by: Tadeusz Struk <redacted>
---
crypto/algif_akcipher.c | 542 +++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 542 insertions(+)
create mode 100644 crypto/algif_akcipher.c
@@ -0,0 +1,542 @@+/*+*algif_akcipher:User-spaceinterfaceforasymmetriccipheralgorithms+*+*Copyright(C)2015,StephanMueller<smueller@chronox.de>+*+*Thisfileprovidestheuser-spaceAPIforasymmetricciphers.+*+*Thisprogramisfreesoftware;youcanredistributeitand/ormodifyit+*underthetermsoftheGNUGeneralPublicLicenseaspublishedbytheFree+*SoftwareFoundation;eitherversion2oftheLicense,or(atyouroption)+*anylaterversion.+*/++#include<crypto/akcipher.h>+#include<crypto/scatterwalk.h>+#include<crypto/if_alg.h>+#include<linux/init.h>+#include<linux/list.h>+#include<linux/kernel.h>+#include<linux/mm.h>+#include<linux/module.h>+#include<linux/net.h>+#include<net/sock.h>++structakcipher_sg_list{+unsignedintcur;+structscatterlistsg[ALG_MAX_PAGES];+};++structakcipher_ctx{+structakcipher_sg_listtsgl;+structaf_alg_sglrsgl[ALG_MAX_PAGES];++structaf_alg_completioncompletion;++unsignedlongused;++unsignedintlen;+boolmore;+boolmerge;+intop;++structakcipher_requestreq;+};++staticinlineintakcipher_sndbuf(structsock*sk)+{+structalg_sock*ask=alg_sk(sk);+structakcipher_ctx*ctx=ask->private;++returnmax_t(int,max_t(int,sk->sk_sndbuf&PAGE_MASK,PAGE_SIZE)-+ctx->used,0);+}++staticinlineboolakcipher_writable(structsock*sk)+{+returnakcipher_sndbuf(sk)>=PAGE_SIZE;+}++staticinlineintakcipher_calcsize(structakcipher_ctx*ctx)+{+returncrypto_akcipher_maxsize(crypto_akcipher_reqtfm(&ctx->req));+}++staticvoidakcipher_put_sgl(structsock*sk)+{+structalg_sock*ask=alg_sk(sk);+structakcipher_ctx*ctx=ask->private;+structakcipher_sg_list*sgl=&ctx->tsgl;+structscatterlist*sg=sgl->sg;+unsignedinti;++for(i=0;i<sgl->cur;i++){+if(!sg_page(sg+i))+continue;++put_page(sg_page(sg+i));+sg_assign_page(sg+i,NULL);+}+sg_init_table(sg,ALG_MAX_PAGES);+sgl->cur=0;+ctx->used=0;+ctx->more=0;+ctx->merge=0;+}++staticvoidakcipher_wmem_wakeup(structsock*sk)+{+structsocket_wq*wq;++if(!akcipher_writable(sk))+return;++rcu_read_lock();+wq=rcu_dereference(sk->sk_wq);+if(wq_has_sleeper(&wq->wait))+wake_up_interruptible_sync_poll(&wq->wait,POLLIN|+POLLRDNORM|+POLLRDBAND);+sk_wake_async(sk,SOCK_WAKE_WAITD,POLL_IN);+rcu_read_unlock();+}++staticintakcipher_wait_for_data(structsock*sk,unsignedintflags)+{+structalg_sock*ask=alg_sk(sk);+structakcipher_ctx*ctx=ask->private;+longtimeout;+DEFINE_WAIT(wait);+interr=-ERESTARTSYS;++if(flags&MSG_DONTWAIT)+return-EAGAIN;++set_bit(SOCKWQ_ASYNC_WAITDATA,&sk->sk_socket->flags);++for(;;){+if(signal_pending(current))+break;+prepare_to_wait(sk_sleep(sk),&wait,TASK_INTERRUPTIBLE);+timeout=MAX_SCHEDULE_TIMEOUT;+if(sk_wait_event(sk,&timeout,!ctx->more)){+err=0;+break;+}+}+finish_wait(sk_sleep(sk),&wait);++clear_bit(SOCKWQ_ASYNC_WAITDATA,&sk->sk_socket->flags);++returnerr;+}++staticvoidakcipher_data_wakeup(structsock*sk)+{+structalg_sock*ask=alg_sk(sk);+structakcipher_ctx*ctx=ask->private;+structsocket_wq*wq;++if(ctx->more)+return;+if(!ctx->used)+return;++rcu_read_lock();+wq=rcu_dereference(sk->sk_wq);+if(wq_has_sleeper(&wq->wait))+wake_up_interruptible_sync_poll(&wq->wait,POLLOUT|+POLLRDNORM|+POLLRDBAND);+sk_wake_async(sk,SOCK_WAKE_SPACE,POLL_OUT);+rcu_read_unlock();+}++staticintakcipher_sendmsg(structsocket*sock,structmsghdr*msg,+size_tsize)+{+structsock*sk=sock->sk;+structalg_sock*ask=alg_sk(sk);+structakcipher_ctx*ctx=ask->private;+structakcipher_sg_list*sgl=&ctx->tsgl;+structaf_alg_controlcon={};+longcopied=0;+intop=0;+boolinit=0;+interr;++if(msg->msg_controllen){+err=af_alg_cmsg_send(msg,&con);+if(err)+returnerr;++init=1;+switch(con.op){+caseALG_OP_VERIFY:+caseALG_OP_SIGN:+caseALG_OP_ENCRYPT:+caseALG_OP_DECRYPT:+op=con.op;+break;+default:+return-EINVAL;+}+}++lock_sock(sk);+if(!ctx->more&&ctx->used)+gotounlock;++if(init)+ctx->op=op;++while(size){+unsignedlonglen=size;+structscatterlist*sg=NULL;++/* use the existing memory in an allocated page */+if(ctx->merge){+sg=sgl->sg+sgl->cur-1;+len=min_t(unsignedlong,len,+PAGE_SIZE-sg->offset-sg->length);+err=memcpy_from_msg(page_address(sg_page(sg))++sg->offset+sg->length,+msg,len);+if(err)+gotounlock;++sg->length+=len;+ctx->merge=(sg->offset+sg->length)&+(PAGE_SIZE-1);++ctx->used+=len;+copied+=len;+size-=len;+continue;+}++if(!akcipher_writable(sk)){+/* user space sent too much data */+akcipher_put_sgl(sk);+err=-EMSGSIZE;+gotounlock;+}++/* allocate a new page */+len=min_t(unsignedlong,size,akcipher_sndbuf(sk));+while(len){+intplen=0;++if(sgl->cur>=ALG_MAX_PAGES){+akcipher_put_sgl(sk);+err=-E2BIG;+gotounlock;+}++sg=sgl->sg+sgl->cur;+plen=min_t(int,len,PAGE_SIZE);++sg_assign_page(sg,alloc_page(GFP_KERNEL));+if(!sg_page(sg)){+err=-ENOMEM;+gotounlock;+}++err=memcpy_from_msg(page_address(sg_page(sg)),+msg,plen);+if(err){+__free_page(sg_page(sg));+sg_assign_page(sg,NULL);+gotounlock;+}++sg->offset=0;+sg->length=plen;+len-=plen;+ctx->used+=plen;+copied+=plen;+sgl->cur++;+size-=plen;+ctx->merge=plen&(PAGE_SIZE-1);+}+}++err=0;++ctx->more=msg->msg_flags&MSG_MORE;++unlock:+akcipher_data_wakeup(sk);+release_sock(sk);++returnerr?:copied;+}++staticssize_takcipher_sendpage(structsocket*sock,structpage*page,+intoffset,size_tsize,intflags)+{+structsock*sk=sock->sk;+structalg_sock*ask=alg_sk(sk);+structakcipher_ctx*ctx=ask->private;+structakcipher_sg_list*sgl=&ctx->tsgl;+interr=0;++if(flags&MSG_SENDPAGE_NOTLAST)+flags|=MSG_MORE;++if(sgl->cur>=ALG_MAX_PAGES)+return-E2BIG;++lock_sock(sk);+if(!ctx->more&&ctx->used)+gotounlock;++if(!size)+gotodone;++if(!akcipher_writable(sk)){+/* user space sent too much data */+akcipher_put_sgl(sk);+err=-EMSGSIZE;+gotounlock;+}++ctx->merge=0;++get_page(page);+sg_set_page(sgl->sg+sgl->cur,page,size,offset);+sgl->cur++;+ctx->used+=size;++done:+ctx->more=flags&MSG_MORE;+unlock:+akcipher_data_wakeup(sk);+release_sock(sk);++returnerr?err:size;+}++staticintakcipher_recvmsg(structsocket*sock,structmsghdr*msg,+size_tignored,intflags)+{+structsock*sk=sock->sk;+structalg_sock*ask=alg_sk(sk);+structakcipher_ctx*ctx=ask->private;+structakcipher_sg_list*sgl=&ctx->tsgl;+unsignedinti=0;+interr;+unsignedlongused=0;+size_tusedpages=0;+unsignedintcnt=0;++/* Limit number of IOV blocks to be accessed below */+if(msg->msg_iter.nr_segs>ALG_MAX_PAGES)+return-ENOMSG;++lock_sock(sk);++if(ctx->more){+err=akcipher_wait_for_data(sk,flags);+if(err)+gotounlock;+}++used=ctx->used;++/* convert iovecs of output buffers into scatterlists */+while(iov_iter_count(&msg->msg_iter)){+/* make one iovec available as scatterlist */+err=af_alg_make_sg(&ctx->rsgl[cnt],&msg->msg_iter,+iov_iter_count(&msg->msg_iter));+if(err<0)+gotounlock;+usedpages+=err;+/* chain the new scatterlist with previous one */+if(cnt)+af_alg_link_sg(&ctx->rsgl[cnt-1],&ctx->rsgl[cnt]);++iov_iter_advance(&msg->msg_iter,err);+cnt++;+}++/* ensure output buffer is sufficiently large */+if(usedpages<akcipher_calcsize(ctx)){+err=-EMSGSIZE;+gotounlock;+}++sg_mark_end(sgl->sg+sgl->cur-1);++akcipher_request_set_crypt(&ctx->req,sgl->sg,ctx->rsgl[0].sg,used,+usedpages);+switch(ctx->op){+caseALG_OP_VERIFY:+err=crypto_akcipher_verify(&ctx->req);+break;+caseALG_OP_SIGN:+err=crypto_akcipher_sign(&ctx->req);+break;+caseALG_OP_ENCRYPT:+err=crypto_akcipher_encrypt(&ctx->req);+break;+caseALG_OP_DECRYPT:+err=crypto_akcipher_decrypt(&ctx->req);+break;+default:+err=-EFAULT;+gotounlock;+}++err=af_alg_wait_for_completion(err,&ctx->completion);++if(err){+/* EBADMSG implies a valid cipher operation took place */+if(err==-EBADMSG)+akcipher_put_sgl(sk);+gotounlock;+}++akcipher_put_sgl(sk);++unlock:+for(i=0;i<cnt;i++)+af_alg_free_sg(&ctx->rsgl[i]);++akcipher_wmem_wakeup(sk);+release_sock(sk);++returnerr?err:ctx->req.dst_len;+}++staticunsignedintakcipher_poll(structfile*file,structsocket*sock,+poll_table*wait)+{+structsock*sk=sock->sk;+structalg_sock*ask=alg_sk(sk);+structakcipher_ctx*ctx=ask->private;+unsignedintmask=0;++sock_poll_wait(file,sk_sleep(sk),wait);++if(!ctx->more)+mask|=POLLIN|POLLRDNORM;++if(akcipher_writable(sk))+mask|=POLLOUT|POLLWRNORM|POLLWRBAND;++returnmask;+}++staticstructproto_opsalgif_akcipher_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,++.release=af_alg_release,+.sendmsg=akcipher_sendmsg,+.sendpage=akcipher_sendpage,+.recvmsg=akcipher_recvmsg,+.poll=akcipher_poll,+};++staticvoid*akcipher_bind(constchar*name,u32type,u32mask)+{+returncrypto_alloc_akcipher(name,type,mask);+}++staticvoidakcipher_release(void*private)+{+crypto_free_akcipher(private);+}++staticintakcipher_setprivkey(void*private,constu8*key,+unsignedintkeylen)+{+returncrypto_akcipher_set_priv_key(private,key,keylen);+}++staticintakcipher_setpubkey(void*private,constu8*key,unsignedintkeylen)+{+returncrypto_akcipher_set_pub_key(private,key,keylen);+}++staticvoidakcipher_sock_destruct(structsock*sk)+{+structalg_sock*ask=alg_sk(sk);+structakcipher_ctx*ctx=ask->private;++akcipher_put_sgl(sk);+sock_kfree_s(sk,ctx,ctx->len);+af_alg_release_parent(sk);+}++staticintakcipher_accept_parent(void*private,structsock*sk)+{+structakcipher_ctx*ctx;+structalg_sock*ask=alg_sk(sk);+unsignedintlen=sizeof(*ctx)+crypto_akcipher_reqsize(private);++ctx=sock_kmalloc(sk,len,GFP_KERNEL);+if(!ctx)+return-ENOMEM;+memset(ctx,0,len);++ctx->len=len;+ctx->used=0;+ctx->more=0;+ctx->merge=0;+ctx->op=0;+ctx->tsgl.cur=0;+af_alg_init_completion(&ctx->completion);+sg_init_table(ctx->tsgl.sg,ALG_MAX_PAGES);++ask->private=ctx;++akcipher_request_set_tfm(&ctx->req,private);+akcipher_request_set_callback(&ctx->req,CRYPTO_TFM_REQ_MAY_BACKLOG,+af_alg_complete,&ctx->completion);++sk->sk_destruct=akcipher_sock_destruct;++return0;+}++staticconststructaf_alg_typealgif_type_akcipher={+.bind=akcipher_bind,+.release=akcipher_release,+.setkey=akcipher_setprivkey,+.setpubkey=akcipher_setpubkey,+.accept=akcipher_accept_parent,+.ops=&algif_akcipher_ops,+.name="akcipher",+.owner=THIS_MODULE+};++staticint__initalgif_akcipher_init(void)+{+returnaf_alg_register_type(&algif_type_akcipher);+}++staticvoid__exitalgif_akcipher_exit(void)+{+interr=af_alg_unregister_type(&algif_type_akcipher);++WARN_ON(err);+}++module_init(algif_akcipher_init);+module_exit(algif_akcipher_exit);+MODULE_LICENSE("GPL");+MODULE_AUTHOR("Stephan Mueller <smueller@chronox.de>");+MODULE_DESCRIPTION("Asymmetric kernel crypto API user space interface");
From: Tadeusz Struk <hidden> Date: 2016-05-15 04:18:19
Similar to algif_skcipher and algif_hash, algif_akcipher needs
to prevent user space from using the interface in an improper way.
This patch adds nokey ops handlers, which do just that.
Signed-off-by: Tadeusz Struk <redacted>
---
crypto/algif_akcipher.c | 159 +++++++++++++++++++++++++++++++++++++++++++++--
1 file changed, 152 insertions(+), 7 deletions(-)
From: Tadeusz Struk <hidden> Date: 2016-05-15 04:18:29
This patch adds support for asymmetric key type to AF_ALG.
It will work as follows: A new PF_ALG socket options are
added on top of existing ALG_SET_KEY and ALG_SET_PUBKEY, namely
ALG_SET_KEY_ID and ALG_SET_PUBKEY_ID for setting public and
private keys respectively. When these new options will be used
the user, instead of providing the key material, will provide a
key id and the key itself will be obtained from kernel keyring
subsystem. The user will use the standard tools (keyctl tool
or the keyctl syscall) for key instantiation and to obtain the
key id. The key id can also be obtained by reading the
/proc/keys file.
When a key corresponding to the given keyid is found, it is stored
in the socket context and subsequent crypto operation invoked by the
user will use the new asymmetric accessor functions instead of akcipher
api. The asymmetric subtype can internally use akcipher api or
invoke operations defined by a given subtype, depending on the
key type.
Signed-off-by: Tadeusz Struk <redacted>
---
crypto/af_alg.c | 10 ++
crypto/algif_akcipher.c | 207 ++++++++++++++++++++++++++++++++++++++++++-
include/crypto/if_alg.h | 1
include/uapi/linux/if_alg.h | 2
4 files changed, 215 insertions(+), 5 deletions(-)
@@ -260,6 +260,16 @@ static int alg_setsockopt(struct socket *sock, int level, int optname,err=alg_setkey(sk,optval,optlen,type->setpubkey);break;++caseALG_SET_KEY_ID:+caseALG_SET_PUBKEY_ID:+/* ALG_SET_KEY_ID is only for akcipher */+if(!strcmp(type->name,"akcipher")||+sock->state==SS_CONNECTED)+gotounlock;++err=alg_setkey(sk,optval,optlen,type->setkeyid);+break;caseALG_SET_AEAD_AUTHSIZE:if(sock->state==SS_CONNECTED)gotounlock;
@@ -322,6 +326,153 @@ unlock:returnerr?err:size;}+staticintasym_key_encrypt(conststructkey*key,structakcipher_request*req)+{+structkernel_pkey_paramsparams={0};+char*src=NULL,*dst=NULL,*in,*out;+intret;++if(!sg_is_last(req->src)){+src=kmalloc(req->src_len,GFP_KERNEL);+if(!src)+return-ENOMEM;+scatterwalk_map_and_copy(src,req->src,0,req->src_len,0);+in=src;+}else{+in=sg_virt(req->src);+}+if(!sg_is_last(req->dst)){+dst=kmalloc(req->dst_len,GFP_KERNEL);+if(!dst){+kfree(src);+return-ENOMEM;+}+out=dst;+}else{+out=sg_virt(req->dst);+}+params.key=(structkey*)key;+params.in_len=req->src_len;+params.out_len=req->dst_len;+ret=encrypt_blob(¶ms,in,out);+if(ret)+gotofree;++if(dst)+scatterwalk_map_and_copy(dst,req->dst,0,req->dst_len,1);+free:+kfree(src);+kfree(dst);+returnret;+}++staticintasym_key_decrypt(conststructkey*key,structakcipher_request*req)+{+structkernel_pkey_paramsparams={0};+char*src=NULL,*dst=NULL,*in,*out;+intret;++if(!sg_is_last(req->src)){+src=kmalloc(req->src_len,GFP_KERNEL);+if(!src)+return-ENOMEM;+scatterwalk_map_and_copy(src,req->src,0,req->src_len,0);+in=src;+}else{+in=sg_virt(req->src);+}+if(!sg_is_last(req->dst)){+dst=kmalloc(req->dst_len,GFP_KERNEL);+if(!dst){+kfree(src);+return-ENOMEM;+}+out=dst;+}else{+out=sg_virt(req->dst);+}+params.key=(structkey*)key;+params.in_len=req->src_len;+params.out_len=req->dst_len;+ret=decrypt_blob(¶ms,in,out);+if(ret)+gotofree;++if(dst)+scatterwalk_map_and_copy(dst,req->dst,0,req->dst_len,1);+free:+kfree(src);+kfree(dst);+returnret;+}++staticintasym_key_sign(conststructkey*key,structakcipher_request*req)+{+structkernel_pkey_paramsparams={0};+char*src=NULL,*dst=NULL,*in,*out;+intret;++if(!sg_is_last(req->src)){+src=kmalloc(req->src_len,GFP_KERNEL);+if(!src)+return-ENOMEM;+scatterwalk_map_and_copy(src,req->src,0,req->src_len,0);+in=src;+}else{+in=sg_virt(req->src);+}+if(!sg_is_last(req->dst)){+dst=kmalloc(req->dst_len,GFP_KERNEL);+if(!dst){+kfree(src);+return-ENOMEM;+}+out=dst;+}else{+out=sg_virt(req->dst);+}+params.key=(structkey*)key;+params.in_len=req->src_len;+params.out_len=req->dst_len;+ret=create_signature(¶ms,in,out);+if(ret)+gotofree;++if(dst)+scatterwalk_map_and_copy(dst,req->dst,0,req->dst_len,1);+free:+kfree(src);+kfree(dst);+returnret;+}++staticintasym_key_verify(conststructkey*key,structakcipher_request*req)+{+structpublic_key_signaturesig;+char*src=NULL,*in;+intret;++if(!sg_is_last(req->src)){+src=kmalloc(req->src_len,GFP_KERNEL);+if(!src)+return-ENOMEM;+scatterwalk_map_and_copy(src,req->src,0,req->src_len,0);+in=src;+}else{+in=sg_virt(req->src);+}+sig.pkey_algo="rsa";+sig.encoding="pkcs1";+/* Need to find a way to pass the hash param */+sig.hash_algo="sha1";+sig.digest_size=20;+sig.s_size=req->src_len;+sig.s=src;+ret=verify_signature(key,NULL,&sig);+kfree(src);+returnret;+}+staticintakcipher_recvmsg(structsocket*sock,structmsghdr*msg,size_tignored,intflags){
@@ -579,6 +742,27 @@ static void akcipher_release(void *private)kfree(tfm);}+staticintakcipher_setkeyid(void*private,constu8*key,unsignedintkeylen)+{+structakcipher_tfm*tfm=private;+structkey*akey;+u32keyid=*((u32*)key);+interr=-ENOKEY;++/* Store the key id and verify that a key with the given id is present.+*Theactualkeywillbeacquiredintheaccept_parentfunction+*/+sprintf(tfm->keyid,"id:%08x",keyid);+akey=request_key(&key_type_asymmetric,tfm->keyid,NULL);+if(IS_ERR(key))+gotoout;++tfm->has_key=true;+key_put(akey);+out:+returnerr;+}+staticintakcipher_setprivkey(void*private,constu8*key,unsignedintkeylen){
From: Tadeusz Struk <hidden> Date: 2016-05-15 04:18:58
From: Stephan Mueller <redacted>
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 | 18 +++++++++++++-----
include/crypto/if_alg.h | 1 +
2 files changed, 14 insertions(+), 5 deletions(-)
@@ -247,10 +251,14 @@ static int alg_setsockopt(struct socket *sock, int level, int optname,caseALG_SET_KEY:if(sock->state==SS_CONNECTED)gotounlock;-if(!type->setkey)++err=alg_setkey(sk,optval,optlen,type->setkey);+break;+caseALG_SET_PUBKEY:+if(sock->state==SS_CONNECTED)gotounlock;-err=alg_setkey(sk,optval,optlen);+err=alg_setkey(sk,optval,optlen,type->setpubkey);break;caseALG_SET_AEAD_AUTHSIZE:if(sock->state==SS_CONNECTED)
The params member names have changed (now in_len and out_len).
quoted
+ ret = encrypt_blob(¶ms, in, out);
The encrypt function for the key can now be called with params.key->type->asym_eds_op(). This also allows you to factor out the duplication in asym_key_encrypt, asym_key_decrypt, and asym_key_sign. See keyctl_pkey_e_d_s() in keyctl_pkey.c
quoted
+static int asym_key_verify(const struct key *key, struct akcipher_request *req)
...
quoted
+ ret = verify_signature(key, NULL, &sig);
key->type->asym_verify_signature() is available as well.
Since these operation will be triggered from userspace I think it's better to use the
official interface as defined in crypto/public_key.h instead of direct calls.
Some operation may not be implemented for a given key type and the official interface
performs necessary checks.
Thanks,
--
TS
From: Tadeusz Struk <hidden> Date: 2016-05-16 20:47:15
On 05/14/2016 09:16 PM, Tadeusz Struk wrote:
First four patches are a resend of the v3 algif_akcipher from
Stephan Mueller, with minor changes after rebase on top of 4.6-rc1.
The next three patches add support for keys stored in system
keyring subsystem.
First patch adds algif_akcipher nokey hadlers.
Second patch adds generic sign, verify, encrypt, decrypt accessors
functions to the asymmetric key type. These will be defined by
asymmetric subtypes, similarly to how public_key currently defines
the verify_signature function.
Third patch adds support for ALG_SET_KEY_ID and ALG_SET_PUBKEY_ID
commands to AF_ALG and setkeyid operation to the af_alg_type struct.
If the keyid is used then the afalg layer acquires the key for the
keyring subsystem and uses the new asymmetric accessor functions
instead of akcipher api. The asymmetric subtypes can use akcipher
api internally.
This is the same v5 version as before rebased on top of
http://git.kernel.org/cgit/linux/kernel/git/dhowells/linux-fs.git/log/?h=keys-asym-keyctl
v6 changes:
- update to reflect changes in kernel_pkey_params struct
v5 changes:
- drop public key changes and use new version provided by David
v4 changes:
- don't use internal public_key struct in af_alg.
- add generic accessor functions to asymmetric key type, which take
the generic struct key type and resolve the specific subtype internally
v3 changes:
- include Stephan's patches (rebased on 4.6-rc1)
- add algif_akcipher nokey hadlers
- add public_key info struct to public_key and helper query functions
- add a check if a key is a software accessible key on af_alg, and
return -ENOKEY if it isn't
v2 changes:
- pass the original skcipher request in ablkcipher.base.data instead of
casting it back from the ablkcipher request.
- rename _req to base_req
- dropped 3/3
---
Stephan Mueller (4):
crypto: AF_ALG -- add sign/verify API
crypto: AF_ALG -- add setpubkey setsockopt call
crypto: AF_ALG -- add asymmetric cipher interface
crypto: algif_akcipher - enable compilation
Tadeusz Struk (2):
crypto: algif_akcipher - add ops_nokey
crypto: AF_ALG - add support for key_id
crypto/Kconfig | 9
crypto/Makefile | 1
crypto/af_alg.c | 28 +
crypto/algif_akcipher.c | 884 +++++++++++++++++++++++++++++++++++++++++++
include/crypto/if_alg.h | 2
include/uapi/linux/if_alg.h | 5
6 files changed, 924 insertions(+), 5 deletions(-)
create mode 100644 crypto/algif_akcipher.c
Hi David,
Are you ok to take it into 4.7 via your linux-fs?
Thanks,
--
TS
From: Mat Martineau <hidden> Date: 2016-05-26 00:46:09
On Sat, 14 May 2016, Tadeusz Struk wrote:
quoted hunk
diff --git a/crypto/algif_akcipher.c b/crypto/algif_akcipher.cindex e00793d..6733df1 100644--- a/crypto/algif_akcipher.c+++ b/crypto/algif_akcipher.c
+static int asym_key_verify(const struct key *key, struct akcipher_request *req)
+{
+ struct public_key_signature sig;
+ char *src = NULL, *in;
+ int ret;
+
+ if (!sg_is_last(req->src)) {
+ src = kmalloc(req->src_len, GFP_KERNEL);
+ if (!src)
+ return -ENOMEM;
+ scatterwalk_map_and_copy(src, req->src, 0, req->src_len, 0);
+ in = src;
+ } else {
+ in = sg_virt(req->src);
+ }
+ sig.pkey_algo = "rsa";
+ sig.encoding = "pkcs1";
+ /* Need to find a way to pass the hash param */
Are you referring to sig.digest here? It looks like you will hit a
BUG_ON() in public_key_verify_signature() if sig.digest is 0. However,
sig.digest is unlikely to be 0 because the struct is not cleared - should
fix this, since public_key_verify_signature() will try to follow that
random pointer.
Is the idea to write the signature to the socket, and then read out the
expected digest (the digest comparison being done elsewhere)? Is that
something that will be supported by a future hardware asymmetric key
subtype?
verify_signature() ends up calling public_key_verify_signature(), which
currently expects to get both the digest and signature as input and
returns an error if verification fails. The output of
crypto_akcipher_verify() is discarded before public_key_verify_signature()
returns so nothing ends up in req->dst to read from the socket.
ALG_OP_VERIFY should behave the same whether using ALG_SET_PUBKEY or
ALG_SET_PUBKEY_ID, and they aren't right now.
If sig.digest is 0, verify_signature() could return the expected digest in
the sig structure and skip the digest comparison it currently does. Then
that data could be packaged up in req as if crypto_akcipher_verify() had
been called. I don't know if this change confuses the semantics of
verify_signature() too much, maybe a new function is required with all the
requisite plumbing to the asymmetric key subtype.
From: Tadeusz Struk <hidden> Date: 2016-05-31 17:45:25
Hi Mat,
On 05/25/2016 05:45 PM, Mat Martineau wrote:
On Sat, 14 May 2016, Tadeusz Struk wrote:
quoted
diff --git a/crypto/algif_akcipher.c b/crypto/algif_akcipher.cindex e00793d..6733df1 100644--- a/crypto/algif_akcipher.c+++ b/crypto/algif_akcipher.c
+static int asym_key_verify(const struct key *key, struct akcipher_request *req)
+{
+ struct public_key_signature sig;
+ char *src = NULL, *in;
+ int ret;
+
+ if (!sg_is_last(req->src)) {
+ src = kmalloc(req->src_len, GFP_KERNEL);
+ if (!src)
+ return -ENOMEM;
+ scatterwalk_map_and_copy(src, req->src, 0, req->src_len, 0);
+ in = src;
+ } else {
+ in = sg_virt(req->src);
+ }
+ sig.pkey_algo = "rsa";
+ sig.encoding = "pkcs1";
+ /* Need to find a way to pass the hash param */
Are you referring to sig.digest here? It looks like you will hit a BUG_ON() in public_key_verify_signature() if sig.digest is 0. However, sig.digest is unlikely to be 0 because the struct is not cleared - should fix this, since public_key_verify_signature() will try to follow that random pointer.
Right, I need to have a local buffer for the digest here.
Is the idea to write the signature to the socket, and then read out the expected digest (the digest comparison being done elsewhere)? Is that something that will be supported by a future hardware asymmetric key subtype?
After the verify operation the output will be copied to the user,
and the user needs to verify it.
verify_signature() ends up calling public_key_verify_signature(), which currently expects to get both the digest and signature as input and returns an error if verification fails. The output of crypto_akcipher_verify() is discarded before public_key_verify_signature() returns so nothing ends up in req->dst to read from the socket.
ALG_OP_VERIFY should behave the same whether using ALG_SET_PUBKEY or ALG_SET_PUBKEY_ID, and they aren't right now.
If sig.digest is 0, verify_signature() could return the expected digest in the sig structure and skip the digest comparison it currently does. Then that data could be packaged up in req as if crypto_akcipher_verify() had been called. I don't know if this change confuses the semantics of verify_signature() too much, maybe a new function is required with all the requisite plumbing to the asymmetric key subtype.
We need to copy output to the user to verify because we don't have it.
That will be consistent for both ALG_SET_PUBKEY and ALG_SET_PUBKEY_ID.
Thanks for your comments and sorry for the delayed response. I'll will send v7 soon.
--
TS
From: Mat Martineau <hidden> Date: 2016-06-08 00:28:13
Stephan,
On Sat, 14 May 2016, Tadeusz Struk wrote:
quoted hunk
From: Stephan Mueller <redacted>
This patch adds the user space interface for asymmetric ciphers. The
interface allows the use of sendmsg as well as vmsplice to provide data.
This version has been rebased on top of 4.6 and a few chackpatch issues
have been fixed.
Signed-off-by: Stephan Mueller <redacted>
Signed-off-by: Tadeusz Struk <redacted>
---
diff --git a/crypto/algif_akcipher.c b/crypto/algif_akcipher.cnew file mode 100644index 0000000..6342b6e--- /dev/null+++ b/crypto/algif_akcipher.c
+
+static int akcipher_recvmsg(struct socket *sock, struct msghdr *msg,
+ size_t ignored, int flags)
+{
+ struct sock *sk = sock->sk;
+ struct alg_sock *ask = alg_sk(sk);
+ struct akcipher_ctx *ctx = ask->private;
+ struct akcipher_sg_list *sgl = &ctx->tsgl;
+ unsigned int i = 0;
+ int err;
+ unsigned long used = 0;
+ size_t usedpages = 0;
+ unsigned int cnt = 0;
+
+ /* Limit number of IOV blocks to be accessed below */
+ if (msg->msg_iter.nr_segs > ALG_MAX_PAGES)
+ return -ENOMSG;
+
+ lock_sock(sk);
+
+ if (ctx->more) {
+ err = akcipher_wait_for_data(sk, flags);
+ if (err)
+ goto unlock;
+ }
+
+ used = ctx->used;
+
+ /* convert iovecs of output buffers into scatterlists */
+ while (iov_iter_count(&msg->msg_iter)) {
+ /* make one iovec available as scatterlist */
+ err = af_alg_make_sg(&ctx->rsgl[cnt], &msg->msg_iter,
+ iov_iter_count(&msg->msg_iter));
+ if (err < 0)
+ goto unlock;
+ usedpages += err;
+ /* chain the new scatterlist with previous one */
+ if (cnt)
+ af_alg_link_sg(&ctx->rsgl[cnt - 1], &ctx->rsgl[cnt]);
+
+ iov_iter_advance(&msg->msg_iter, err);
+ cnt++;
+ }
+
+ /* ensure output buffer is sufficiently large */
+ if (usedpages < akcipher_calcsize(ctx)) {
+ err = -EMSGSIZE;
+ goto unlock;
+ }
Why is the size of the output buffer enforced here instead of depending on
the algorithm implementation?
Thanks,
Mat
From: Stephan Mueller <hidden> Date: 2016-06-08 05:31:26
Am Dienstag, 7. Juni 2016, 17:28:07 schrieb Mat Martineau:
Hi Mat,
quoted
+ used = ctx->used;
+
+ /* convert iovecs of output buffers into scatterlists */
+ while (iov_iter_count(&msg->msg_iter)) {
+ /* make one iovec available as scatterlist */
+ err = af_alg_make_sg(&ctx->rsgl[cnt], &msg->msg_iter,
+ iov_iter_count(&msg->msg_iter));
+ if (err < 0)
+ goto unlock;
+ usedpages += err;
+ /* chain the new scatterlist with previous one */
+ if (cnt)
+ af_alg_link_sg(&ctx->rsgl[cnt - 1], &ctx->rsgl[cnt]);
+
+ iov_iter_advance(&msg->msg_iter, err);
+ cnt++;
+ }
+
+ /* ensure output buffer is sufficiently large */
+ if (usedpages < akcipher_calcsize(ctx)) {
+ err = -EMSGSIZE;
+ goto unlock;
+ }
Why is the size of the output buffer enforced here instead of depending on
the algorithm implementation?
akcipher_calcsize calls crypto_akcipher_maxsize to get the maximum size the
algorithm generates as output during its operation.
The code ensures that the caller provided at least that amount of memory for
the kernel to store its data in. This check therefore is present to ensure the
kernel does not overstep memory boundaries in user space.
What is your concern?
Thanks
Ciao
Stephan
From: Mat Martineau <hidden> Date: 2016-06-08 19:14:56
On Wed, 8 Jun 2016, Stephan Mueller wrote:
Am Dienstag, 7. Juni 2016, 17:28:07 schrieb Mat Martineau:
Hi Mat,
quoted
quoted
+ used = ctx->used;
+
+ /* convert iovecs of output buffers into scatterlists */
+ while (iov_iter_count(&msg->msg_iter)) {
+ /* make one iovec available as scatterlist */
+ err = af_alg_make_sg(&ctx->rsgl[cnt], &msg->msg_iter,
+ iov_iter_count(&msg->msg_iter));
+ if (err < 0)
+ goto unlock;
+ usedpages += err;
+ /* chain the new scatterlist with previous one */
+ if (cnt)
+ af_alg_link_sg(&ctx->rsgl[cnt - 1], &ctx->rsgl[cnt]);
+
+ iov_iter_advance(&msg->msg_iter, err);
+ cnt++;
+ }
+
+ /* ensure output buffer is sufficiently large */
+ if (usedpages < akcipher_calcsize(ctx)) {
+ err = -EMSGSIZE;
+ goto unlock;
+ }
Why is the size of the output buffer enforced here instead of depending on
the algorithm implementation?
akcipher_calcsize calls crypto_akcipher_maxsize to get the maximum size the
algorithm generates as output during its operation.
The code ensures that the caller provided at least that amount of memory for
the kernel to store its data in. This check therefore is present to ensure the
kernel does not overstep memory boundaries in user space.
Yes, it's understood that the userspace buffer length must not be
exceeded. But dst_len is part of the akcipher_request struct, so why does
it need to be checked *here* when it is also checked later?
What is your concern?
Userspace must allocate larger buffers than it knows are necessary for
expected results.
It looks like the software rsa implementation handles shorter output
buffers ok (mpi_write_to_sgl will return EOVERFLOW if the the buffer is
too small), however I see at least one hardware rsa driver that requires
the output buffer to be the maximum size. But this inconsistency might be
best addressed within the software cipher or drivers rather than in
recvmsg.
From: Stephan Mueller <hidden> Date: 2016-06-09 09:29:05
Am Mittwoch, 8. Juni 2016, 12:14:49 schrieb Mat Martineau:
Hi Mat,
On Wed, 8 Jun 2016, Stephan Mueller wrote:
quoted
Am Dienstag, 7. Juni 2016, 17:28:07 schrieb Mat Martineau:
Hi Mat,
quoted
quoted
+ used = ctx->used;
+
+ /* convert iovecs of output buffers into scatterlists */
+ while (iov_iter_count(&msg->msg_iter)) {
+ /* make one iovec available as scatterlist */
+ err = af_alg_make_sg(&ctx->rsgl[cnt], &msg->msg_iter,
+ iov_iter_count(&msg->msg_iter));
+ if (err < 0)
+ goto unlock;
+ usedpages += err;
+ /* chain the new scatterlist with previous one */
+ if (cnt)
+ af_alg_link_sg(&ctx->rsgl[cnt - 1], &ctx->rsgl[cnt]);
+
+ iov_iter_advance(&msg->msg_iter, err);
+ cnt++;
+ }
+
+ /* ensure output buffer is sufficiently large */
+ if (usedpages < akcipher_calcsize(ctx)) {
+ err = -EMSGSIZE;
+ goto unlock;
+ }
Why is the size of the output buffer enforced here instead of depending
on
the algorithm implementation?
akcipher_calcsize calls crypto_akcipher_maxsize to get the maximum size
the
algorithm generates as output during its operation.
The code ensures that the caller provided at least that amount of memory
for the kernel to store its data in. This check therefore is present to
ensure the kernel does not overstep memory boundaries in user space.
Yes, it's understood that the userspace buffer length must not be
exceeded. But dst_len is part of the akcipher_request struct, so why does
it need to be checked *here* when it is also checked later?
I am always uneasy when the kernel has a user space interface and expects
layers deep down inside the kernel to check for user space related boundaries.
Note, we do not hand the __user flag down, so sparse and other tools cannot
detect whether a particular cipher implementation has the right checks.
I therefore always would like to check parameters at the interface handling
logic. Cryptographers rightly should worry about their code implementing the
cipher correctly. But I do not think that the cipher implementations should
worry about security implications since they may be called from user space.
quoted
What is your concern?
Userspace must allocate larger buffers than it knows are necessary for
expected results.
It looks like the software rsa implementation handles shorter output
buffers ok (mpi_write_to_sgl will return EOVERFLOW if the the buffer is
too small), however I see at least one hardware rsa driver that requires
the output buffer to be the maximum size. But this inconsistency might be
best addressed within the software cipher or drivers rather than in
recvmsg.
Is your concern that we have a double check check for lengths here? If yes, I
think we can live with an additional if() here.
Or is your concern that the user space interface restricts things too much and
thus prevents a valid use case?
Ciao
Stephan
From: Mat Martineau <hidden> Date: 2016-06-09 18:18:25
On Thu, 9 Jun 2016, Stephan Mueller wrote:
Am Mittwoch, 8. Juni 2016, 12:14:49 schrieb Mat Martineau:
Hi Mat,
quoted
On Wed, 8 Jun 2016, Stephan Mueller wrote:
quoted
Am Dienstag, 7. Juni 2016, 17:28:07 schrieb Mat Martineau:
Hi Mat,
quoted
quoted
+ used = ctx->used;
+
+ /* convert iovecs of output buffers into scatterlists */
+ while (iov_iter_count(&msg->msg_iter)) {
+ /* make one iovec available as scatterlist */
+ err = af_alg_make_sg(&ctx->rsgl[cnt], &msg->msg_iter,
+ iov_iter_count(&msg->msg_iter));
+ if (err < 0)
+ goto unlock;
+ usedpages += err;
+ /* chain the new scatterlist with previous one */
+ if (cnt)
+ af_alg_link_sg(&ctx->rsgl[cnt - 1], &ctx->rsgl[cnt]);
+
+ iov_iter_advance(&msg->msg_iter, err);
+ cnt++;
+ }
+
+ /* ensure output buffer is sufficiently large */
+ if (usedpages < akcipher_calcsize(ctx)) {
+ err = -EMSGSIZE;
+ goto unlock;
+ }
Why is the size of the output buffer enforced here instead of
depending on the algorithm implementation?
akcipher_calcsize calls crypto_akcipher_maxsize to get the maximum
size the algorithm generates as output during its operation.
The code ensures that the caller provided at least that amount of memory
for the kernel to store its data in. This check therefore is present to
ensure the kernel does not overstep memory boundaries in user space.
Yes, it's understood that the userspace buffer length must not be
exceeded. But dst_len is part of the akcipher_request struct, so why does
it need to be checked *here* when it is also checked later?
I am always uneasy when the kernel has a user space interface and expects
layers deep down inside the kernel to check for user space related boundaries.
Note, we do not hand the __user flag down, so sparse and other tools cannot
detect whether a particular cipher implementation has the right checks.
I therefore always would like to check parameters at the interface handling
logic. Cryptographers rightly should worry about their code implementing the
cipher correctly. But I do not think that the cipher implementations should
worry about security implications since they may be called from user space.
Userspace or not, buffer lengths need to be strictly checked.
quoted
quoted
What is your concern?
Userspace must allocate larger buffers than it knows are necessary for
expected results.
It looks like the software rsa implementation handles shorter output
buffers ok (mpi_write_to_sgl will return EOVERFLOW if the the buffer is
too small), however I see at least one hardware rsa driver that requires
the output buffer to be the maximum size. But this inconsistency might be
best addressed within the software cipher or drivers rather than in
recvmsg.
Is your concern that we have a double check check for lengths here? If yes, I
think we can live with an additional if() here.
Or is your concern that the user space interface restricts things too much and
thus prevents a valid use case?
The latter - my primary concern is the constraint this places on userspace
by forcing larger buffer sizes than might be necessary for the operation.
struct akcipher_request has separate members for src_len and dst_len, and
dst_len is documented as needing "to be at least as big as the expected
result depending on the operation". Not the maximum result, the expected
result. It's also documented that the cipher will generate an error if
dst_len is insufficient and update the value with the required size.
I'm updating some userspace TLS code that worked with an earlier, unmerged
patch set for AF_ALG akcipher (from last year). The read calls with
shorter buffers were the main porting problem.
--
Mat Martineau
Intel OTC
From: Stephan Mueller <hidden> Date: 2016-06-09 18:24:25
Am Donnerstag, 9. Juni 2016, 11:18:04 schrieb Mat Martineau:
Hi Mat,
quoted
Or is your concern that the user space interface restricts things too much
and thus prevents a valid use case?
The latter - my primary concern is the constraint this places on userspace
by forcing larger buffer sizes than might be necessary for the operation.
struct akcipher_request has separate members for src_len and dst_len, and
dst_len is documented as needing "to be at least as big as the expected
result depending on the operation". Not the maximum result, the expected
result. It's also documented that the cipher will generate an error if
dst_len is insufficient and update the value with the required size.
I'm updating some userspace TLS code that worked with an earlier, unmerged
patch set for AF_ALG akcipher (from last year). The read calls with
shorter buffers were the main porting problem.
I see -- are you proposing to drop that check entirely?
Ciao
Stephan
From: Mat Martineau <hidden> Date: 2016-06-09 18:27:18
On Thu, 9 Jun 2016, Stephan Mueller wrote:
Am Donnerstag, 9. Juni 2016, 11:18:04 schrieb Mat Martineau:
Hi Mat,
quoted
quoted
Or is your concern that the user space interface restricts things too much
and thus prevents a valid use case?
The latter - my primary concern is the constraint this places on userspace
by forcing larger buffer sizes than might be necessary for the operation.
struct akcipher_request has separate members for src_len and dst_len, and
dst_len is documented as needing "to be at least as big as the expected
result depending on the operation". Not the maximum result, the expected
result. It's also documented that the cipher will generate an error if
dst_len is insufficient and update the value with the required size.
I'm updating some userspace TLS code that worked with an earlier, unmerged
patch set for AF_ALG akcipher (from last year). The read calls with
shorter buffers were the main porting problem.
I see -- are you proposing to drop that check entirely?
From: Stephan Mueller <hidden> Date: 2016-06-09 18:36:35
Am Donnerstag, 9. Juni 2016, 11:27:13 schrieb Mat Martineau:
Hi Mat, Tadeusz,
On Thu, 9 Jun 2016, Stephan Mueller wrote:
quoted
Am Donnerstag, 9. Juni 2016, 11:18:04 schrieb Mat Martineau:
Hi Mat,
quoted
quoted
Or is your concern that the user space interface restricts things too
much
and thus prevents a valid use case?
The latter - my primary concern is the constraint this places on
userspace
by forcing larger buffer sizes than might be necessary for the operation.
struct akcipher_request has separate members for src_len and dst_len, and
dst_len is documented as needing "to be at least as big as the expected
result depending on the operation". Not the maximum result, the expected
result. It's also documented that the cipher will generate an error if
dst_len is insufficient and update the value with the required size.
I'm updating some userspace TLS code that worked with an earlier,
unmerged
patch set for AF_ALG akcipher (from last year). The read calls with
shorter buffers were the main porting problem.
I see -- are you proposing to drop that check entirely?
Yes.
Ok, after checking the code again, I think that dropping that sanity check
should be ok given that this length is part of the akcipher API.
Tadeusz, as you are currently managing that patch set, would you re-spin it
with the following check removed?
+ if (usedpages < akcipher_calcsize(ctx)) {
+ err = -EMSGSIZE;
+ goto unlock;
+ }
Ciao
Stephan
From: Tadeusz Struk <hidden> Date: 2016-06-10 14:43:32
On 06/09/2016 11:36 AM, Stephan Mueller wrote:
Am Donnerstag, 9. Juni 2016, 11:27:13 schrieb Mat Martineau:
Hi Mat, Tadeusz,
quoted
On Thu, 9 Jun 2016, Stephan Mueller wrote:
quoted
Am Donnerstag, 9. Juni 2016, 11:18:04 schrieb Mat Martineau:
Hi Mat,
quoted
quoted
Or is your concern that the user space interface restricts things too
much
and thus prevents a valid use case?
The latter - my primary concern is the constraint this places on
userspace
by forcing larger buffer sizes than might be necessary for the operation.
struct akcipher_request has separate members for src_len and dst_len, and
dst_len is documented as needing "to be at least as big as the expected
result depending on the operation". Not the maximum result, the expected
result. It's also documented that the cipher will generate an error if
dst_len is insufficient and update the value with the required size.
I'm updating some userspace TLS code that worked with an earlier,
unmerged
patch set for AF_ALG akcipher (from last year). The read calls with
shorter buffers were the main porting problem.
I see -- are you proposing to drop that check entirely?
Yes.
Ok, after checking the code again, I think that dropping that sanity check
should be ok given that this length is part of the akcipher API.
Tadeusz, as you are currently managing that patch set, would you re-spin it
with the following check removed?
+ if (usedpages < akcipher_calcsize(ctx)) {
+ err = -EMSGSIZE;
+ goto unlock;
+ }
From: Andrew Zaborowski <hidden> Date: 2016-06-13 22:16:15
Hi,
On 8 June 2016 at 21:14, Mat Martineau
[off-list ref] wrote:
On Wed, 8 Jun 2016, Stephan Mueller wrote:
quoted
What is your concern?
Userspace must allocate larger buffers than it knows are necessary for
expected results.
It looks like the software rsa implementation handles shorter output buffers
ok (mpi_write_to_sgl will return EOVERFLOW if the the buffer is too small),
however I see at least one hardware rsa driver that requires the output
buffer to be the maximum size. But this inconsistency might be best
addressed within the software cipher or drivers rather than in recvmsg.
Should the hardware drivers fix this instead? I've looked at the qat
and caam drivers, they both require the destination buffer size to be
the key size and in both cases there would be no penalty for dropping
this requirement as far as I see. Both do a memmove if the result
ends up being shorter than key size. In case the caller knows it is
expecting a specific output size, the driver will have to use a self
allocated buffer + a memcpy in those same cases where it would later
use memmove instead. Alternatively the sg passed to dma_map_sg can be
prepended with a dummy segment the right size to save the memcpy.
akcipher.h only says:
@dst_len: Size of the output buffer. It needs to be at least as big as
the expected result depending on the operation
Note that for random input data the memmove will be done about 1 in
256 times but with PKCS#1 padding the signature always has a leading
zero.
Requiring buffers bigger than needed makes the added work of dropping
the zero bytes from the sglist and potentially re-adding them in the
client difficult to justify. RSA doing this sets a precedent for a
future pkcs1pad (or other algorithm) implementation to do the same
thing and a portable client having to always know the key size and use
key-sized buffers.
Best regards
From: Stephan Mueller <hidden> Date: 2016-06-14 05:12:34
Am Dienstag, 14. Juni 2016, 00:16:11 schrieb Andrew Zaborowski:
Hi Andrew,
Hi,
On 8 June 2016 at 21:14, Mat Martineau
[off-list ref] wrote:
quoted
On Wed, 8 Jun 2016, Stephan Mueller wrote:
quoted
What is your concern?
Userspace must allocate larger buffers than it knows are necessary for
expected results.
It looks like the software rsa implementation handles shorter output
buffers ok (mpi_write_to_sgl will return EOVERFLOW if the the buffer is
too small), however I see at least one hardware rsa driver that requires
the output buffer to be the maximum size. But this inconsistency might be
best addressed within the software cipher or drivers rather than in
recvmsg.
Should the hardware drivers fix this instead? I've looked at the qat
and caam drivers, they both require the destination buffer size to be
the key size and in both cases there would be no penalty for dropping
this requirement as far as I see. Both do a memmove if the result
ends up being shorter than key size. In case the caller knows it is
expecting a specific output size, the driver will have to use a self
allocated buffer + a memcpy in those same cases where it would later
use memmove instead. Alternatively the sg passed to dma_map_sg can be
prepended with a dummy segment the right size to save the memcpy.
akcipher.h only says:
@dst_len: Size of the output buffer. It needs to be at least as big as
the expected result depending on the operation
Note that for random input data the memmove will be done about 1 in
256 times but with PKCS#1 padding the signature always has a leading
zero.
Requiring buffers bigger than needed makes the added work of dropping
the zero bytes from the sglist and potentially re-adding them in the
client difficult to justify. RSA doing this sets a precedent for a
future pkcs1pad (or other algorithm) implementation to do the same
thing and a portable client having to always know the key size and use
key-sized buffers.
I think we have agreed on dropping the length enforcement at the interface
level.
Ciao
Stephan
From: Andrew Zaborowski <hidden> Date: 2016-06-14 07:42:38
Hi Stephan,
On 14 June 2016 at 07:12, Stephan Mueller [off-list ref] wrote:
Am Dienstag, 14. Juni 2016, 00:16:11 schrieb Andrew Zaborowski:
quoted
On 8 June 2016 at 21:14, Mat Martineau
[off-list ref] wrote:
quoted
On Wed, 8 Jun 2016, Stephan Mueller wrote:
quoted
What is your concern?
Userspace must allocate larger buffers than it knows are necessary for
expected results.
It looks like the software rsa implementation handles shorter output
buffers ok (mpi_write_to_sgl will return EOVERFLOW if the the buffer is
too small), however I see at least one hardware rsa driver that requires
the output buffer to be the maximum size. But this inconsistency might be
best addressed within the software cipher or drivers rather than in
recvmsg.
Should the hardware drivers fix this instead? I've looked at the qat
and caam drivers, they both require the destination buffer size to be
the key size and in both cases there would be no penalty for dropping
this requirement as far as I see. Both do a memmove if the result
ends up being shorter than key size. In case the caller knows it is
expecting a specific output size, the driver will have to use a self
allocated buffer + a memcpy in those same cases where it would later
use memmove instead. Alternatively the sg passed to dma_map_sg can be
prepended with a dummy segment the right size to save the memcpy.
akcipher.h only says:
@dst_len: Size of the output buffer. It needs to be at least as big as
the expected result depending on the operation
Note that for random input data the memmove will be done about 1 in
256 times but with PKCS#1 padding the signature always has a leading
zero.
Requiring buffers bigger than needed makes the added work of dropping
the zero bytes from the sglist and potentially re-adding them in the
client difficult to justify. RSA doing this sets a precedent for a
future pkcs1pad (or other algorithm) implementation to do the same
thing and a portable client having to always know the key size and use
key-sized buffers.
I think we have agreed on dropping the length enforcement at the interface
level.
Separately from this there's a problem with the user being unable to
know if the algorithm is going to fail because of destination buffer
size != key size (including kernel users). For RSA, the qat
implementation will fail while the software implementation won't. For
pkcs1pad(...) there's currently just one implementation but the user
can't assume that.
Best regards
From: Mat Martineau <hidden> Date: 2016-06-14 17:22:27
Stephan,
On Sat, 14 May 2016, Tadeusz Struk wrote:
quoted hunk
From: Stephan Mueller <redacted>
This patch adds the user space interface for asymmetric ciphers. The
interface allows the use of sendmsg as well as vmsplice to provide data.
This version has been rebased on top of 4.6 and a few chackpatch issues
have been fixed.
Signed-off-by: Stephan Mueller <redacted>
Signed-off-by: Tadeusz Struk <redacted>
---
crypto/algif_akcipher.c | 542 +++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 542 insertions(+)
create mode 100644 crypto/algif_akcipher.c
diff --git a/crypto/algif_akcipher.c b/crypto/algif_akcipher.cnew file mode 100644index 0000000..6342b6e--- /dev/null+++ b/crypto/algif_akcipher.c
+
+static int akcipher_sendmsg(struct socket *sock, struct msghdr *msg,
+ size_t size)
+{
+ struct sock *sk = sock->sk;
+ struct alg_sock *ask = alg_sk(sk);
+ struct akcipher_ctx *ctx = ask->private;
+ struct akcipher_sg_list *sgl = &ctx->tsgl;
+ struct af_alg_control con = {};
+ long copied = 0;
+ int op = 0;
+ bool init = 0;
+ int err;
+
+ if (msg->msg_controllen) {
+ err = af_alg_cmsg_send(msg, &con);
+ if (err)
+ return err;
+
+ init = 1;
+ switch (con.op) {
+ case ALG_OP_VERIFY:
+ case ALG_OP_SIGN:
+ case ALG_OP_ENCRYPT:
+ case ALG_OP_DECRYPT:
+ op = con.op;
+ break;
+ default:
+ return -EINVAL;
+ }
+ }
+
+ lock_sock(sk);
+ if (!ctx->more && ctx->used)
+ goto unlock;
err might be uninitialised at this goto. Should it be set to something
like -EALREADY to indicate that data is already queued for a different
crypto op?
<snip>
From: Stephan Mueller <hidden> Date: 2016-06-15 07:04:37
Am Dienstag, 14. Juni 2016, 10:22:15 schrieb Mat Martineau:
Hi Mat,
Stephan,
On Sat, 14 May 2016, Tadeusz Struk wrote:
quoted
From: Stephan Mueller <redacted>
This patch adds the user space interface for asymmetric ciphers. The
interface allows the use of sendmsg as well as vmsplice to provide data.
This version has been rebased on top of 4.6 and a few chackpatch issues
have been fixed.
Signed-off-by: Stephan Mueller <redacted>
Signed-off-by: Tadeusz Struk <redacted>
---
crypto/algif_akcipher.c | 542
+++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 542
insertions(+)
create mode 100644 crypto/algif_akcipher.c
diff --git a/crypto/algif_akcipher.c b/crypto/algif_akcipher.cnew file mode 100644index 0000000..6342b6e--- /dev/null+++ b/crypto/algif_akcipher.c
+
+static int akcipher_sendmsg(struct socket *sock, struct msghdr *msg,
+ size_t size)
+{
+ struct sock *sk = sock->sk;
+ struct alg_sock *ask = alg_sk(sk);
+ struct akcipher_ctx *ctx = ask->private;
+ struct akcipher_sg_list *sgl = &ctx->tsgl;
+ struct af_alg_control con = {};
+ long copied = 0;
+ int op = 0;
+ bool init = 0;
+ int err;
+
+ if (msg->msg_controllen) {
+ err = af_alg_cmsg_send(msg, &con);
+ if (err)
+ return err;
+
+ init = 1;
+ switch (con.op) {
+ case ALG_OP_VERIFY:
+ case ALG_OP_SIGN:
+ case ALG_OP_ENCRYPT:
+ case ALG_OP_DECRYPT:
+ op = con.op;
+ break;
+ default:
+ return -EINVAL;
+ }
+ }
+
+ lock_sock(sk);
+ if (!ctx->more && ctx->used)
+ goto unlock;
err might be uninitialised at this goto. Should it be set to something
like -EALREADY to indicate that data is already queued for a different
crypto op?
Thanks for the hint. Tadeusz, I will provide you with an updated
algif_akcipher.c for your patchset.
I will also have a look at the comment from Andrew.
From: Stephan Mueller <hidden> Date: 2016-06-16 08:05:57
Am Dienstag, 14. Juni 2016, 09:42:34 schrieb Andrew Zaborowski:
Hi Andrew,
quoted
I think we have agreed on dropping the length enforcement at the interface
level.
Separately from this there's a problem with the user being unable to
know if the algorithm is going to fail because of destination buffer
size != key size (including kernel users). For RSA, the qat
implementation will fail while the software implementation won't. For
pkcs1pad(...) there's currently just one implementation but the user
can't assume that.
If I understand your issue correctly, my initial code requiring the caller to
provide sufficient memory would have covered the issue, right? If so, we seem
to have implementations which can handle shorter buffer sizes and some which
do not. Should a caller really try to figure the right buffer size out? Why
not requiring a mandatory buffer size and be done with it? I.e. what is the
gain to allow shorter buffer sizes (as pointed out by Mat)? So, bottom line, I
am wondering whether we should keep the algif_akcipher code to require a
minimum buffer size.
If there is really a good argument to allow shorter buffers, then I guess we
need an in-kernel API call (which should be reported to user space) which
gives us the smallest usable buffer size. I guess that call would only be
valid after a setkey operation as the output size depends on the key size.
Instead of inventing a complete new API call, shouldn't the call
crypto_akcipher_maxsize() be converted for this purpose? I requested that API
call during the time the akcipher API was developed explicitly for getting the
minimum buffer size the caller needs to provide.
Ciao
Stephan
From: Andrew Zaborowski <hidden> Date: 2016-06-16 14:59:36
Hi Stephan,
On 16 June 2016 at 10:05, Stephan Mueller [off-list ref] wrote:
Am Dienstag, 14. Juni 2016, 09:42:34 schrieb Andrew Zaborowski:
Hi Andrew,
quoted
quoted
I think we have agreed on dropping the length enforcement at the interface
level.
Separately from this there's a problem with the user being unable to
know if the algorithm is going to fail because of destination buffer
size != key size (including kernel users). For RSA, the qat
implementation will fail while the software implementation won't. For
pkcs1pad(...) there's currently just one implementation but the user
can't assume that.
If I understand your issue correctly, my initial code requiring the caller to
provide sufficient memory would have covered the issue, right?
This isn't an issue with AF_ALG, I should have changed the subject
line perhaps. In this case it's an inconsistency between some
implementations and the documentation (header comment). It affects
users accessing the cipher through AF_ALG but also directly.
If so, we seem
to have implementations which can handle shorter buffer sizes and some which
do not. Should a caller really try to figure the right buffer size out? Why
not requiring a mandatory buffer size and be done with it? I.e. what is the
gain to allow shorter buffer sizes (as pointed out by Mat)?
It's that client code doesn't need an intermediate layer with an
additional buffer and a memcpy to provide a sensible API. If the code
wants to decrypt a 32-byte Digest Info structure with a given key or a
reference to a key it makes no sense, logically or in terms of
performance, for it to provide a key-sized buffer.
In the case of the userspace interface I think it's also rare for a
recv() or read() on Linux to require a buffer larger than it's going
to use, correct me if i'm wrong. (I.e. fail if given a 32-byte
buffer, return 32 bytes of data anyway) Turning your questino around
is there a gain from requiring larger buffers?
Best regards
From: Stephan Mueller <hidden> Date: 2016-06-16 15:38:22
Am Donnerstag, 16. Juni 2016, 16:59:01 schrieb Andrew Zaborowski:
Hi Andrew,
Hi Stephan,
On 16 June 2016 at 10:05, Stephan Mueller [off-list ref] wrote:
quoted
Am Dienstag, 14. Juni 2016, 09:42:34 schrieb Andrew Zaborowski:
Hi Andrew,
quoted
quoted
I think we have agreed on dropping the length enforcement at the
interface
level.
Separately from this there's a problem with the user being unable to
know if the algorithm is going to fail because of destination buffer
size != key size (including kernel users). For RSA, the qat
implementation will fail while the software implementation won't. For
pkcs1pad(...) there's currently just one implementation but the user
can't assume that.
If I understand your issue correctly, my initial code requiring the caller
to provide sufficient memory would have covered the issue, right?
This isn't an issue with AF_ALG, I should have changed the subject
line perhaps. In this case it's an inconsistency between some
implementations and the documentation (header comment). It affects
users accessing the cipher through AF_ALG but also directly.
As I want to send a new version of the algif_akcipher shortly now (hoping for
an inclusion into 4.8), is there anything you see that I should prepare for
regarding this issue? I.e. do you forsee a potential fix that would change the
API or ABI of algif_akcipher?
quoted
If so, we seem
to have implementations which can handle shorter buffer sizes and some
which do not. Should a caller really try to figure the right buffer size
out? Why not requiring a mandatory buffer size and be done with it? I.e.
what is the gain to allow shorter buffer sizes (as pointed out by Mat)?
It's that client code doesn't need an intermediate layer with an
additional buffer and a memcpy to provide a sensible API. If the code
wants to decrypt a 32-byte Digest Info structure with a given key or a
reference to a key it makes no sense, logically or in terms of
performance, for it to provide a key-sized buffer.
In the case of the userspace interface I think it's also rare for a
recv() or read() on Linux to require a buffer larger than it's going
to use, correct me if i'm wrong. (I.e. fail if given a 32-byte
buffer, return 32 bytes of data anyway) Turning your questino around
is there a gain from requiring larger buffers?
That is a good one :-)
I have that check removed.
Ciao
Stephan
From: Andrew Zaborowski <hidden> Date: 2016-06-17 00:39:51
Hi Stephan,
On 16 June 2016 at 17:38, Stephan Mueller [off-list ref] wrote:
quoted
This isn't an issue with AF_ALG, I should have changed the subject
line perhaps. In this case it's an inconsistency between some
implementations and the documentation (header comment). It affects
users accessing the cipher through AF_ALG but also directly.
As I want to send a new version of the algif_akcipher shortly now (hoping for
an inclusion into 4.8), is there anything you see that I should prepare for
regarding this issue? I.e. do you forsee a potential fix that would change the
API or ABI of algif_akcipher?
No, as far as I understand algif_akcipher will do the right thing now
if the algorithm does the right thing. It's only the two RSA drivers
that would need to align with the software RSA in what buffer length
they accept.
Best regards
From: Mat Martineau <hidden> Date: 2016-06-22 22:46:49
Stephan and Tadeusz,
On Fri, 10 Jun 2016, Tadeusz Struk wrote:
On 06/09/2016 11:36 AM, Stephan Mueller wrote:
quoted
Am Donnerstag, 9. Juni 2016, 11:27:13 schrieb Mat Martineau:
Hi Mat, Tadeusz,
Ok, after checking the code again, I think that dropping that sanity check
should be ok given that this length is part of the akcipher API.
Tadeusz, as you are currently managing that patch set, would you re-spin it
with the following check removed?
+ if (usedpages < akcipher_calcsize(ctx)) {
+ err = -EMSGSIZE;
+ goto unlock;
+ }
Ok, I'll update the patch.
Thanks, that helps (especially with pkcs1pad).
This brings me to another proposal for read buffer sizing: AF_ALG akcipher
can guarantee that partial reads (where the read buffer is shorter than
the output of the crypto op) will work using the same semantics as
SOCK_DGRAM/SOCK_SEQPACKET. With those sockets, as much data as will fit is
copied in to the read buffer and the remainder is discarded.
I realize there's a performance and memory tradeoff, since the crypto
algorithm needs a sufficiently large output buffer that would have to be
created by AF_ALG akcipher. The user could manage that tradeoff by
providing a larger buffer (typically key_size?) if it wants to avoid
allocating and copying intermediate buffers inside the kernel.
--
Mat Martineau
Intel OTC
From: Stephan Mueller <hidden> Date: 2016-06-23 05:07:18
Am Mittwoch, 22. Juni 2016, 15:45:38 schrieb Mat Martineau:
Hi Mat,
quoted
Ok, I'll update the patch.
Thanks, that helps (especially with pkcs1pad).
Tadeusz received the updated patch from me to integrate it into his patch set.
This brings me to another proposal for read buffer sizing: AF_ALG akcipher
can guarantee that partial reads (where the read buffer is shorter than
the output of the crypto op) will work using the same semantics as
SOCK_DGRAM/SOCK_SEQPACKET. With those sockets, as much data as will fit is
copied in to the read buffer and the remainder is discarded.
I realize there's a performance and memory tradeoff, since the crypto
algorithm needs a sufficiently large output buffer that would have to be
created by AF_ALG akcipher. The user could manage that tradeoff by
providing a larger buffer (typically key_size?) if it wants to avoid
allocating and copying intermediate buffers inside the kernel.
How shall the user know that something got truncated or that the kernel
created memory?
Ciao
Stephan
This brings me to another proposal for read buffer sizing: AF_ALG akcipher
can guarantee that partial reads (where the read buffer is shorter than
the output of the crypto op) will work using the same semantics as
SOCK_DGRAM/SOCK_SEQPACKET. With those sockets, as much data as will fit is
copied in to the read buffer and the remainder is discarded.
I realize there's a performance and memory tradeoff, since the crypto
algorithm needs a sufficiently large output buffer that would have to be
created by AF_ALG akcipher. The user could manage that tradeoff by
providing a larger buffer (typically key_size?) if it wants to avoid
allocating and copying intermediate buffers inside the kernel.
How shall the user know that something got truncated or that the kernel
created memory?
To the former point, recall the signature of recv:
ssize_t recv(int sockfd, void *buf, size_t len, int flags);
Traditionally, userspace apps can know that the buffer provided to recv
was too small in two ways:
The return value from recv / recvmsg was >= len.
In the case of recvmsg, the MSG_TRUNC flag is set.
To quote man recv:
"All three calls return the length of the message on successful comple‐
tion. If a message is too long to fit in the supplied buffer, excess
bytes may be discarded depending on the type of socket the message is
received from."
and
"MSG_TRUNC (since Linux 2.2)
For raw (AF_PACKET), Internet datagram (since Linux
2.4.27/2.6.8), netlink (since Linux 2.6.22), and UNIX datagram
(since Linux 3.4) sockets: return the real length of the packet
or datagram, even when it was longer than the passed buffer.
"
Regards,
-Denis