LinuxLists.cc - [PATCH v2 0/5] crypto: add algif

[permalink] [raw]

Subject: [PATCH v2 5/5] crypto: algif_akcipher - enable compilation

Add the Makefile and Kconfig updates to allow algif_akcipher to be
compiled.

Signed-off-by: Stephan Mueller <[email protected]>
---
crypto/Kconfig | 9 +++++++++
crypto/Makefile | 1 +
2 files changed, 10 insertions(+)

diff --git a/crypto/Kconfig b/crypto/Kconfig
index fc93444..aa5d3aa 100644
--- a/crypto/Kconfig
+++ b/crypto/Kconfig
@@ -1632,6 +1632,15 @@ config CRYPTO_USER_API_AEAD
This option enables the user-spaces interface for AEAD
cipher algorithms.

+config CRYPTO_USER_API_AKCIPHER
+ tristate "User-space interface for asymmetric key cipher algorithms"
+ depends on NET
+ select CRYPTO_AKCIPHER2
+ select CRYPTO_USER_API
+ help
+ This option enables the user-spaces interface for asymmetric
+ key cipher algorithms.
+
config CRYPTO_HASH_INFO
bool

diff --git a/crypto/Makefile b/crypto/Makefile
index d897e0b..3aaa914 100644
--- a/crypto/Makefile
+++ b/crypto/Makefile
@@ -120,6 +120,7 @@ obj-$(CONFIG_CRYPTO_USER_API_HASH) += algif_hash.o
obj-$(CONFIG_CRYPTO_USER_API_SKCIPHER) += algif_skcipher.o
obj-$(CONFIG_CRYPTO_USER_API_RNG) += algif_rng.o
obj-$(CONFIG_CRYPTO_USER_API_AEAD) += algif_aead.o
+obj-$(CONFIG_CRYPTO_USER_API_AKCIPHER) += algif_akcipher.o

#
# generic algorithms and the async_tx api
--
2.5.0

2015-10-18 10:48:28

[permalink] [raw]

Subject: [PATCH v2 4/5] crypto: AF_ALG -- add asymmetric cipher interface

This patch adds the user space interface for asymmetric ciphers. The
interface allows the use of sendmsg as well as vmsplice to provide data.

Signed-off-by: Stephan Mueller <[email protected]>
---
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.c
new file mode 100644
index 0000000..7c58e9b
--- /dev/null
+++ b/crypto/algif_akcipher.c
@@ -0,0 +1,542 @@
+/*
+ * algif_akcipher: User-space interface for asymmetric cipher algorithms
+ *
+ * Copyright (C) 2015, Stephan Mueller <[email protected]>
+ *
+ * This file provides the user-space API for asymmetric ciphers.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the Free
+ * Software Foundation; either version 2 of the License, or (at your option)
+ * any later version.
+ */
+
+#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>
+
+struct akcipher_sg_list {
+ unsigned int cur;
+ struct scatterlist sg[ALG_MAX_PAGES];
+};
+
+struct akcipher_ctx {
+ struct akcipher_sg_list tsgl;
+ struct af_alg_sgl rsgl[ALG_MAX_PAGES];
+
+ struct af_alg_completion completion;
+
+ unsigned long used;
+
+ unsigned int len;
+ bool more;
+ bool merge;
+ int op;
+
+ struct akcipher_request req;
+};
+
+static inline int akcipher_sndbuf(struct sock *sk)
+{
+ struct alg_sock *ask = alg_sk(sk);
+ struct akcipher_ctx *ctx = ask->private;
+
+ return max_t(int, max_t(int, sk->sk_sndbuf & PAGE_MASK, PAGE_SIZE) -
+ ctx->used, 0);
+}
+
+static inline bool akcipher_writable(struct sock *sk)
+{
+ return PAGE_SIZE <= akcipher_sndbuf(sk);
+}
+
+static inline int akcipher_calcsize(struct akcipher_ctx *ctx)
+{
+ return crypto_akcipher_maxsize(crypto_akcipher_reqtfm(&ctx->req));
+}
+
+static void akcipher_put_sgl(struct sock *sk)
+{
+ struct alg_sock *ask = alg_sk(sk);
+ struct akcipher_ctx *ctx = ask->private;
+ struct akcipher_sg_list *sgl = &ctx->tsgl;
+ struct scatterlist *sg = sgl->sg;
+ unsigned int i;
+
+ 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;
+}
+
+static void akcipher_wmem_wakeup(struct sock *sk)
+{
+ struct socket_wq *wq;
+
+ if (!akcipher_writable(sk))
+ return;
+
+ rcu_read_lock();
+ wq = rcu_dereference(sk->sk_wq);
+ if (wq_has_sleeper(wq))
+ wake_up_interruptible_sync_poll(&wq->wait, POLLIN |
+ POLLRDNORM |
+ POLLRDBAND);
+ sk_wake_async(sk, SOCK_WAKE_WAITD, POLL_IN);
+ rcu_read_unlock();
+}
+
+static int akcipher_wait_for_data(struct sock *sk, unsigned flags)
+{
+ struct alg_sock *ask = alg_sk(sk);
+ struct akcipher_ctx *ctx = ask->private;
+ long timeout;
+ DEFINE_WAIT(wait);
+ int err = -ERESTARTSYS;
+
+ if (flags & MSG_DONTWAIT)
+ return -EAGAIN;
+
+ set_bit(SOCK_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(SOCK_ASYNC_WAITDATA, &sk->sk_socket->flags);
+
+ return err;
+}
+
+static void akcipher_data_wakeup(struct sock *sk)
+{
+ struct alg_sock *ask = alg_sk(sk);
+ struct akcipher_ctx *ctx = ask->private;
+ struct socket_wq *wq;
+
+ if (ctx->more)
+ return;
+ if (!ctx->used)
+ return;
+
+ rcu_read_lock();
+ wq = rcu_dereference(sk->sk_wq);
+ if (wq_has_sleeper(wq))
+ wake_up_interruptible_sync_poll(&wq->wait, POLLOUT |
+ POLLRDNORM |
+ POLLRDBAND);
+ sk_wake_async(sk, SOCK_WAKE_SPACE, POLL_OUT);
+ rcu_read_unlock();
+}
+
+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;
+
+ if (init)
+ ctx->op = op;
+
+ while (size) {
+ unsigned long len = size;
+ struct scatterlist *sg = NULL;
+
+ /* use the existing memory in an allocated page */
+ if (ctx->merge) {
+ sg = sgl->sg + sgl->cur - 1;
+ len = min_t(unsigned long, len,
+ PAGE_SIZE - sg->offset - sg->length);
+ err = memcpy_from_msg(page_address(sg_page(sg)) +
+ sg->offset + sg->length,
+ msg, len);
+ if (err)
+ goto unlock;
+
+ 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;
+ goto unlock;
+ }
+
+ /* allocate a new page */
+ len = min_t(unsigned long, size, akcipher_sndbuf(sk));
+ while (len) {
+ int plen = 0;
+
+ if (sgl->cur >= ALG_MAX_PAGES) {
+ akcipher_put_sgl(sk);
+ err = -E2BIG;
+ goto unlock;
+ }
+
+ 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;
+ goto unlock;
+ }
+
+ err = memcpy_from_msg(page_address(sg_page(sg)),
+ msg, plen);
+ if (err) {
+ __free_page(sg_page(sg));
+ sg_assign_page(sg, NULL);
+ goto unlock;
+ }
+
+ 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);
+
+ return err ?: copied;
+}
+
+static ssize_t akcipher_sendpage(struct socket *sock, struct page *page,
+ int offset, size_t size, 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;
+ int err = 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)
+ goto unlock;
+
+ if (!size)
+ goto done;
+
+ if (!akcipher_writable(sk)) {
+ /* user space sent too much data */
+ akcipher_put_sgl(sk);
+ err = -EMSGSIZE;
+ goto unlock;
+ }
+
+ 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);
+
+ return err ? err : size;
+}
+
+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;
+ }
+
+ 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) {
+ case ALG_OP_VERIFY:
+ err = crypto_akcipher_verify(&ctx->req);
+ break;
+ case ALG_OP_SIGN:
+ err = crypto_akcipher_sign(&ctx->req);
+ break;
+ case ALG_OP_ENCRYPT:
+ err = crypto_akcipher_encrypt(&ctx->req);
+ break;
+ case ALG_OP_DECRYPT:
+ err = crypto_akcipher_decrypt(&ctx->req);
+ break;
+ default:
+ err = -EFAULT;
+ goto unlock;
+ }
+
+ 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);
+ goto unlock;
+ }
+
+ 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);
+
+ return err ? err : ctx->req.dst_len;
+}
+
+static unsigned int akcipher_poll(struct file *file, struct socket *sock,
+ poll_table *wait)
+{
+ struct sock *sk = sock->sk;
+ struct alg_sock *ask = alg_sk(sk);
+ struct akcipher_ctx *ctx = ask->private;
+ unsigned int mask = 0;
+
+ sock_poll_wait(file, sk_sleep(sk), wait);
+
+ if (!ctx->more)
+ mask |= POLLIN | POLLRDNORM;
+
+ if (akcipher_writable(sk))
+ mask |= POLLOUT | POLLWRNORM | POLLWRBAND;
+
+ return mask;
+}
+
+static struct proto_ops algif_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,
+};
+
+static void *akcipher_bind(const char *name, u32 type, u32 mask)
+{
+ return crypto_alloc_akcipher(name, type, mask);
+}
+
+static void akcipher_release(void *private)
+{
+ crypto_free_akcipher(private);
+}
+
+static int akcipher_setprivkey(void *private, const u8 *key,
+ unsigned int keylen)
+{
+ return crypto_akcipher_set_priv_key(private, key, keylen);
+}
+
+static int akcipher_setpubkey(void *private, const u8 *key, unsigned int keylen)
+{
+ return crypto_akcipher_set_pub_key(private, key, keylen);
+}
+
+static void akcipher_sock_destruct(struct sock *sk)
+{
+ struct alg_sock *ask = alg_sk(sk);
+ struct akcipher_ctx *ctx = ask->private;
+
+ akcipher_put_sgl(sk);
+ sock_kfree_s(sk, ctx, ctx->len);
+ af_alg_release_parent(sk);
+}
+
+static int akcipher_accept_parent(void *private, struct sock *sk)
+{
+ struct akcipher_ctx *ctx;
+ struct alg_sock *ask = alg_sk(sk);
+ unsigned int len = 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;
+
+ return 0;
+}
+
+static const struct af_alg_type algif_type_akcipher = {
+ .bind = akcipher_bind,
+ .release = akcipher_release,
+ .setkey = akcipher_setprivkey,
+ .setpubkey = akcipher_setpubkey,
+ .setauthsize = NULL,
+ .accept = akcipher_accept_parent,
+ .ops = &algif_akcipher_ops,
+ .name = "akcipher",
+ .owner = THIS_MODULE
+};
+
+static int __init algif_akcipher_init(void)
+{
+ return af_alg_register_type(&algif_type_akcipher);
+}
+
+static void __exit algif_akcipher_exit(void)
+{
+ int err = af_alg_unregister_type(&algif_type_akcipher);
+ BUG_ON(err);
+}
+
+module_init(algif_akcipher_init);
+module_exit(algif_akcipher_exit);
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Stephan Mueller <[email protected]>");
+MODULE_DESCRIPTION("Asymmetric kernel crypto API user space interface");
--
2.5.0

2015-10-18 10:46:45

[permalink] [raw]

Subject: [PATCH v2 2/5] crypto: AF_ALG -- add sign/verify API

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 <[email protected]>
---
include/uapi/linux/if_alg.h | 3 +++
1 file changed, 3 insertions(+)

diff --git a/include/uapi/linux/if_alg.h b/include/uapi/linux/if_alg.h
index f2acd2f..02e6162 100644
--- a/include/uapi/linux/if_alg.h
+++ b/include/uapi/linux/if_alg.h
@@ -34,9 +34,12 @@ struct af_alg_iv {
#define ALG_SET_OP 3
#define ALG_SET_AEAD_ASSOCLEN 4
#define ALG_SET_AEAD_AUTHSIZE 5
+#define ALG_SET_PUBKEY 6

/* Operations */
#define ALG_OP_DECRYPT 0
#define ALG_OP_ENCRYPT 1
+#define ALG_OP_SIGN 2
+#define ALG_OP_VERIFY 3

#endif /* _LINUX_IF_ALG_H */
--
2.5.0

2015-10-18 10:45:18

[permalink] [raw]

Subject: [PATCH v2 1/5] MPI: fix off by one in mpi_read_raw_from_sgl

The patch fixes the analysis of the input data which contains an off
by one.

The issue is visible when the SGL contains one byte per SG entry.
The code for checking for zero bytes does not operate on the data byte.

Signed-off-by: Stephan Mueller <[email protected]>
---
lib/mpi/mpicoder.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/lib/mpi/mpicoder.c b/lib/mpi/mpicoder.c
index c20ef27..c7e0a70 100644
--- a/lib/mpi/mpicoder.c
+++ b/lib/mpi/mpicoder.c
@@ -446,8 +446,11 @@ MPI mpi_read_raw_from_sgl(struct scatterlist *sgl, unsigned int len)
const u8 *buff = sg_virt(sg);
int len = sg->length;

- while (len-- && !*buff++)
+ while (len && !*buff) {
lzeros++;
+ len--;
+ buff++;
+ }

if (len && *buff)
break;
--
2.5.0

2015-10-18 10:47:41

[permalink] [raw]

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

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 <[email protected]>
---
crypto/af_alg.c | 14 +++++++++++---
include/crypto/if_alg.h | 1 +
2 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/crypto/af_alg.c b/crypto/af_alg.c
index a8e7aa3..bf6528e 100644
--- a/crypto/af_alg.c
+++ b/crypto/af_alg.c
@@ -173,13 +173,16 @@ static int alg_bind(struct socket *sock, struct sockaddr *uaddr, int addr_len)
}

static int alg_setkey(struct sock *sk, char __user *ukey,
- unsigned int keylen)
+ unsigned int keylen, bool pubkey)
{
struct alg_sock *ask = alg_sk(sk);
const struct af_alg_type *type = ask->type;
u8 *key;
int err;

+ if (pubkey && !type->setpubkey)
+ return -EOPNOTSUPP;
+
key = sock_kmalloc(sk, keylen, GFP_KERNEL);
if (!key)
return -ENOMEM;
@@ -188,7 +191,10 @@ static int alg_setkey(struct sock *sk, char __user *ukey,
if (copy_from_user(key, ukey, keylen))
goto out;

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

out:
sock_kzfree_s(sk, key, keylen);
@@ -212,12 +218,14 @@ static int alg_setsockopt(struct socket *sock, int level, int optname,

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

- err = alg_setkey(sk, optval, optlen);
+ err = alg_setkey(sk, optval, optlen,
+ (optname == ALG_SET_PUBKEY) ? true : false);
break;
case ALG_SET_AEAD_AUTHSIZE:
if (sock->state == SS_CONNECTED)
diff --git a/include/crypto/if_alg.h b/include/crypto/if_alg.h
index 018afb2..ca4dc72 100644
--- a/include/crypto/if_alg.h
+++ b/include/crypto/if_alg.h
@@ -49,6 +49,7 @@ struct af_alg_type {
void *(*bind)(const char *name, u32 type, u32 mask);
void (*release)(void *private);
int (*setkey)(void *private, const u8 *key, unsigned int keylen);
+ int (*setpubkey)(void *private, const u8 *key, unsigned int keylen);
int (*accept)(void *private, struct sock *sk);
int (*setauthsize)(void *private, unsigned int authsize);

--
2.5.0

2015-10-19 01:32:30

by Herbert Xu

[permalink] [raw]

Subject: Re: [PATCH v2 0/5] crypto: add algif_akcipher user space API

On Sun, Oct 18, 2015 at 12:44:00PM +0200, Stephan Mueller wrote:
> Hi,
>
> This patch set adds the AF_ALG user space API to externalize the
> asymmetric cipher API recently added to the kernel crypto API.
>
> The patch set is tested with the user space library of libkcapi [1].
> Use [1] test/test.sh for a full test run. The test covers the
> following scenarios:

Thanks Stephan. But I would prefer to defer this til after we have
completed the conversion of current in-kernel users. This is because
changing the kernel interface is easy while changing the user-space
interface is not.

Cheers,
--
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

2015-10-19 07:14:15

[permalink] [raw]

Subject: Re: [PATCH v2 0/5] crypto: add algif_akcipher user space API

Am Montag, 19. Oktober 2015, 09:32:30 schrieb Herbert Xu:

Hi Herbert,

> On Sun, Oct 18, 2015 at 12:44:00PM +0200, Stephan Mueller wrote:
> > Hi,
> >
> > This patch set adds the AF_ALG user space API to externalize the
> > asymmetric cipher API recently added to the kernel crypto API.
> >
> > The patch set is tested with the user space library of libkcapi [1].
> > Use [1] test/test.sh for a full test run. The test covers the
>
> > following scenarios:
> Thanks Stephan. But I would prefer to defer this til after we have
> completed the conversion of current in-kernel users. This is because
> changing the kernel interface is easy while changing the user-space
> interface is not.

Sure, let us wait.

However, I would suggest that you pull patch 1/5 as this is a bug fix that may
affect even other users.

--
Ciao
Stephan

2015-10-19 07:27:00

by Herbert Xu

[permalink] [raw]

Subject: Re: [PATCH v2 0/5] crypto: add algif_akcipher user space API

On Mon, Oct 19, 2015 at 09:14:09AM +0200, Stephan Mueller wrote:
>
> However, I would suggest that you pull patch 1/5 as this is a bug fix that may
> affect even other users.

Sure I'll look into it.

Thanks,
--
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

2015-10-19 23:27:56

by Tadeusz Struk

[permalink] [raw]

Subject: Re: [PATCH v2 1/5] MPI: fix off by one in mpi_read_raw_from_sgl

On 10/18/2015 03:45 AM, Stephan Mueller wrote:
> The patch fixes the analysis of the input data which contains an off
> by one.
>
> The issue is visible when the SGL contains one byte per SG entry.
> The code for checking for zero bytes does not operate on the data byte.
>
> Signed-off-by: Stephan Mueller <[email protected]>
> ---
> lib/mpi/mpicoder.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/lib/mpi/mpicoder.c b/lib/mpi/mpicoder.c
> index c20ef27..c7e0a70 100644
> --- a/lib/mpi/mpicoder.c
> +++ b/lib/mpi/mpicoder.c
> @@ -446,8 +446,11 @@ MPI mpi_read_raw_from_sgl(struct scatterlist *sgl, unsigned int len)
> const u8 *buff = sg_virt(sg);
> int len = sg->length;
>
> - while (len-- && !*buff++)
> + while (len && !*buff) {
> lzeros++;
> + len--;
> + buff++;
> + }
>
> if (len && *buff)
> break;

ACK. Thanks for testing this Stephan.

2015-10-20 14:20:22

by Herbert Xu

[permalink] [raw]

Subject: Re: [PATCH v2 1/5] MPI: fix off by one in mpi_read_raw_from_sgl

On Sun, Oct 18, 2015 at 12:45:18PM +0200, Stephan Mueller wrote:
> The patch fixes the analysis of the input data which contains an off
> by one.
>
> The issue is visible when the SGL contains one byte per SG entry.
> The code for checking for zero bytes does not operate on the data byte.
>
> Signed-off-by: Stephan Mueller <[email protected]>

Patch applied. Thanks.
--
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

2015-10-27 04:54:41

[permalink] [raw]

Subject: Re: [PATCH v2 0/5] crypto: add algif_akcipher user space API

Hi Stephan,

> This patch set adds the AF_ALG user space API to externalize the
> asymmetric cipher API recently added to the kernel crypto API.
>
> The patch set is tested with the user space library of libkcapi [1].
> Use [1] test/test.sh for a full test run. The test covers the
> following scenarios:
>
> * sendmsg of one IOVEC
>
> * sendmsg of 16 IOVECs with non-linear buffer
>
> * vmsplice of one IOVEC
>
> * vmsplice of 15 IOVECs with non-linear buffer
>
> * invoking multiple separate cipher operations with one
> open cipher handle
>
> * encryption with private key (using vector from testmgr.h)
>
> * encryption with public key (using vector from testmgr.h)
>
> * decryption with private key (using vector from testmgr.h)

after having discussions with David Howells and David Woodhouse, I don't think we should expose akcipher via AF_ALG at all. I think the akcipher operations for sign/verify/encrypt/decrypt should operate on asymmetric keys in the first place. With akcipher you are pretty much bound to public and private keys and the key is the important part and not the akcipher itself. Especially since we want to support private keys in hardware (like TPM for example).

It seems more appropriate to use keyctl to derive the symmetric session key from your asymmetric key. And then use the symmetric session key id with skcipher via AF_ALG. Especially once symmetric key type has been introduced this seems to be trivial then.

I am not really in favor of having two userspace facing APIs for asymmetric cipher usage. And we need to have an API that is capable to work with hardware keys.

Regards

Marcel

2015-10-27 09:12:06

[permalink] [raw]

Subject: Re: [PATCH v2 0/5] crypto: add algif_akcipher user space API

Am Dienstag, 27. Oktober 2015, 13:54:30 schrieb Marcel Holtmann:

Hi Marcel,

>Hi Stephan,
>
>> This patch set adds the AF_ALG user space API to externalize the
>> asymmetric cipher API recently added to the kernel crypto API.
>>
>> The patch set is tested with the user space library of libkcapi [1].
>> Use [1] test/test.sh for a full test run. The test covers the
>>
>> following scenarios:
>> * sendmsg of one IOVEC
>>
>> * sendmsg of 16 IOVECs with non-linear buffer
>>
>> * vmsplice of one IOVEC
>>
>> * vmsplice of 15 IOVECs with non-linear buffer
>>
>> * invoking multiple separate cipher operations with one
>>
>> open cipher handle
>>
>> * encryption with private key (using vector from testmgr.h)
>>
>> * encryption with public key (using vector from testmgr.h)
>>
>> * decryption with private key (using vector from testmgr.h)
>
>after having discussions with David Howells and David Woodhouse, I don't
>think we should expose akcipher via AF_ALG at all. I think the akcipher
>operations for sign/verify/encrypt/decrypt should operate on asymmetric keys
>in the first place. With akcipher you are pretty much bound to public and
>private keys and the key is the important part and not the akcipher itself.
>Especially since we want to support private keys in hardware (like TPM for
>example).
>
>It seems more appropriate to use keyctl to derive the symmetric session key

Are you saying that you consider importing parts of TLS into the kernel?
Considering the use case where akcipher would be used to speed up network
protocols, I would imply that your comment refers to importing parts of that
network protocol into the kernel.

The key derivation that you mention here would be: RSA-based key exchange plus
the TLS KDF. Do we really want to load that into the kernel given that TLS is
one protocol and there are many others?

>from your asymmetric key. And then use the symmetric session key id with
>skcipher via AF_ALG. Especially once symmetric key type has been introduced
>this seems to be trivial then.
>
>I am not really in favor of having two userspace facing APIs for asymmetric
>cipher usage. And we need to have an API that is capable to work with
>hardware keys.
>
>Regards
>
>Marcel
>
>--
>To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
>the body of a message to [email protected]
>More majordomo info at http://vger.kernel.org/majordomo-info.html

Ciao
Stephan

2015-10-27 09:19:01

[permalink] [raw]

Subject: Re: [PATCH v2 0/5] crypto: add algif_akcipher user space API

On Tue, 2015-10-27 at 10:12 +0100, Stephan Mueller wrote:
>
> >after having discussions with David Howells and David Woodhouse, I don't
> >think we should expose akcipher via AF_ALG at all. I think the akcipher
> >operations for sign/verify/encrypt/decrypt should operate on asymmetric keys
> >in the first place. With akcipher you are pretty much bound to public and
> >private keys and the key is the important part and not the akcipher itself.
> >Especially since we want to support private keys in hardware (like TPM for
> >example).
> >
> >It seems more appropriate to use keyctl to derive the symmetric session key
>
> Are you saying that you consider importing parts of TLS into the kernel?
> Considering the use case where akcipher would be used to speed up network
> protocols, I would imply that your comment refers to importing parts of that
> network protocol into the kernel.
>
> The key derivation that you mention here would be: RSA-based key exchange plus
> the TLS KDF. Do we really want to load that into the kernel given that TLS is
> one protocol and there are many others?

That's largely orthogonal to the point Marcel was making.

The point is that akcipher is limited to using keys for which we have
the private key material available directly in software. We cannot
expose that critically limited API to userspace. We need to expose an
API which supports hardware keys, and basically that means using the
kernel's key subsystem.

For a key which *happens* to be in software, the key subsystem may end
up *using* akcipher behind the scenes. But the API we expose to
userspace cannot simply be based on akcipher.

--
dwmw2

Attachments:

smime.p7s (5.56 kB)

2015-10-27 10:50:04

[permalink] [raw]

Subject: Re: [PATCH v2 0/5] crypto: add algif_akcipher user space API

Am Dienstag, 27. Oktober 2015, 18:19:01 schrieb David Woodhouse:

Hi David,
>
>That's largely orthogonal to the point Marcel was making.
>
>The point is that akcipher is limited to using keys for which we have
>the private key material available directly in software. We cannot

Agreed.

>expose that critically limited API to userspace. We need to expose an
>API which supports hardware keys, and basically that means using the
>kernel's key subsystem.

Agreed. But at the same time, that interface should be able to support the use
case where the software wants to be in control (just take OpenSSL as the
simple example where you can add an engine for the Linux kernel backed RSA
operation).

Note, the goal with AF_ALG is simply to expose asymmetric hardware support to
user space for unspecified use cases to make user space faster.
>
>For a key which *happens* to be in software, the key subsystem may end
>up *using* akcipher behind the scenes. But the API we expose to
>userspace cannot simply be based on akcipher.

So, how do you propose that would work? Based on what I understand the
suggested logic flow for a simple OpenSSL-like approach would be:

1. OpenSSL opens the interface with the kernel key subsystem and sends the
pub/priv key along

2. the key subsystem hooks the key in

3. the key subsystem sees that there is a simple RSA operation to be made

4. the key subsystem initiates an akcipher operation and sends the keys along

5. the akcipher returns the cipher result to the key subsystem

6. the key subsystem sends the data to user space

Currently I fail to understand why that key subsystem would help me in that
common use case (note, I totally understand to use the key subsystem when you
have some key management schema in place).

Also, I fail to see that this logic flow would be fast to warrant a detour
from user land into kernel land for an RSA operation.

Thanks
Stephan

2015-10-27 15:16:53

by Tadeusz Struk

[permalink] [raw]

Subject: Re: [PATCH v2 0/5] crypto: add algif_akcipher user space API

Hi Marcel,
On 10/26/2015 09:54 PM, Marcel Holtmann wrote:
> after having discussions with David Howells and David Woodhouse, I don't think we should expose akcipher via AF_ALG at all. I think the akcipher operations for sign/verify/encrypt/decrypt should operate on asymmetric keys in the first place. With akcipher you are pretty much bound to public and private keys and the key is the important part and not the akcipher itself. Especially since we want to support private keys in hardware (like TPM for example).
>
> It seems more appropriate to use keyctl to derive the symmetric session key from your asymmetric key. And then use the symmetric session key id with skcipher via AF_ALG. Especially once symmetric key type has been introduced this seems to be trivial then.
>
> I am not really in favor of having two userspace facing APIs for asymmetric cipher usage. And we need to have an API that is capable to work with hardware keys.

The main use case for algif_akcipher will be to allow a web server, which needs to handle
tens of thousand TLS handshakes per second, to offload the RSA operation to a HW accelerator.
Do you think we can use keyctl for this?
Thanks,
T

2015-10-27 23:15:35

[permalink] [raw]

Subject: Re: [PATCH v2 0/5] crypto: add algif_akcipher user space API

On Tue, 2015-10-27 at 11:50 +0100, Stephan Mueller wrote:
>
> >expose that critically limited API to userspace. We need to expose an
> >API which supports hardware keys, and basically that means using the
> >kernel's key subsystem.
>
> Agreed. But at the same time, that interface should be able to support the use
> case where the software wants to be in control (just take OpenSSL as the
> simple example where you can add an engine for the Linux kernel backed RSA
> operation).

Absolutely. The interface needs to support *both*.

I've spent a lot of time chasing through userspace stacks, fixing
broken assumptions that we will *always* have the actual key material
in a file — and making libraries and applications accept PKCS#11 URIs
referring to keys in hardware as well as filenames.

I am violently opposed to exposing an API from the kernel which makes
that *same* fundamental mistake, and ties its users to using *only*
software keys.

FROM THE BEGINNING, users of this new API should be able to cope with
the fact that they might not *have* the key material, and that they
might simply be referring to a key which exists elsewhere. Such as in
the TPM, or within an SGX software enclave.

--
dwmw2

Attachments:

smime.p7s (5.56 kB)

2015-10-27 23:35:35

[permalink] [raw]

Subject: Re: [PATCH v2 0/5] crypto: add algif_akcipher user space API

Am Mittwoch, 28. Oktober 2015, 08:15:16 schrieb David Woodhouse:

Hi David,
>
>Absolutely. The interface needs to support *both*.
>
>I've spent a lot of time chasing through userspace stacks, fixing
>broken assumptions that we will *always* have the actual key material
>in a file — and making libraries and applications accept PKCS#11 URIs
>referring to keys in hardware as well as filenames.
>
>I am violently opposed to exposing an API from the kernel which makes
>that *same* fundamental mistake, and ties its users to using *only*
>software keys.
>
>FROM THE BEGINNING, users of this new API should be able to cope with
>the fact that they might not *have* the key material, and that they
>might simply be referring to a key which exists elsewhere. Such as in
>the TPM, or within an SGX software enclave.

Albeit that all sounds like the crown jewel, how do you propose that shall
happen?

Assume that you have a web server that has a pub and priv key in its current
configuration -- I guess that is the vast majority of configs.

Can you please elaborate how the process for such a web server shall really
work?

Ciao
Stephan

2015-10-27 23:43:16

[permalink] [raw]

Subject: Re: [PATCH v2 0/5] crypto: add algif_akcipher user space API

On Wed, 2015-10-28 at 00:35 +0100, Stephan Mueller wrote:
> Am Mittwoch, 28. Oktober 2015, 08:15:16 schrieb David Woodhouse:
>
> Hi David,
> >
> > Absolutely. The interface needs to support *both*.
> >
> > I've spent a lot of time chasing through userspace stacks, fixing
> > broken assumptions that we will *always* have the actual key material
> > in a file — and making libraries and applications accept PKCS#11 URIs
> > referring to keys in hardware as well as filenames.
> >
> > I am violently opposed to exposing an API from the kernel which makes
> > that *same* fundamental mistake, and ties its users to using *only*
> > software keys.
> >
> > FROM THE BEGINNING, users of this new API should be able to cope with
> > the fact that they might not *have* the key material, and that they
> > might simply be referring to a key which exists elsewhere. Such as in
> > the TPM, or within an SGX software enclave.
>
> Albeit that all sounds like the crown jewel, how do you propose that shall
> happen?
>
> Assume that you have a web server that has a pub and priv key in its current
> configuration -- I guess that is the vast majority of configs.
>
> Can you please elaborate how the process for such a web server shall really
> work?

1. Create a kernel-side key.
2. Use it.

That may require adding an API similar to the one you're proposing, but
working with kernel keys instead of directly with akcipher. Or perhaps
the key subsystem can already offer what you need in userspace. David?

--
dwmw2

Attachments:

smime.p7s (5.56 kB)

2015-10-27 23:47:53

[permalink] [raw]

Subject: Re: [PATCH v2 0/5] crypto: add algif_akcipher user space API

Am Mittwoch, 28. Oktober 2015, 08:43:16 schrieb David Woodhouse:

Hi David,

> > Albeit that all sounds like the crown jewel, how do you propose that shall
> > happen?
> >
> > Assume that you have a web server that has a pub and priv key in its
> > current configuration -- I guess that is the vast majority of configs.
> >
> > Can you please elaborate how the process for such a web server shall
> > really
> > work?
>
> 1. Create a kernel-side key.
> 2. Use it.
>
> That may require adding an API similar to the one you're proposing, but
> working with kernel keys instead of directly with akcipher. Or perhaps
> the key subsystem can already offer what you need in userspace. David?

Ohh, I see. So, you are saying that there should not be a setpub/privkey for
the akcipher AF_ALG interface?!

If somebody wants to use akcipher, he shall set the key via the keyring and
akcipher shall obtain it from the keyring?

However, for the actual data shoveling, AF_ALG should still be used?

--
Ciao
Stephan

2015-10-28 00:37:11

[permalink] [raw]

Subject: Re: [PATCH v2 0/5] crypto: add algif_akcipher user space API

On Wed, 2015-10-28 at 00:47 +0100, Stephan Mueller wrote:
>
> Ohh, I see. So, you are saying that there should not be a setpub/privkey for
> the akcipher AF_ALG interface?!
>
> If somebody wants to use akcipher, he shall set the key via the keyring and
> akcipher shall obtain it from the keyring?
>
> However, for the actual data shoveling, AF_ALG should still be used?

That might seem ideal, but Herbert doesn't want that — he wants
akcipher to work *only* with its own internal keys, not with keys from
the kernel's key subsystem.

David has patches (not upstream yet; used for testing) which expose a
verify operation for kernel keys through sys_keyctl(). Adding the other
three operations would be feasible.

Perhaps if we *really* want this to appear through AF_ALG, the key
subsystem could register its own RSA (and other) implementation(s) with
the crypto subsystem. Then we could find some way that we can pass the
key_serial_t through alg_setkey() instead of the actual key data
without it (or Herbert) noticing that's what we're doing. But... ick.
And I don't think we can select algorithms based on the actual key
rather than the type. We should do it properly, if at all. And define
the API as taking key IDs instead of having it be a nasty special-case
hack for a specific (but really, very generic) algorithm provider.

--
dwmw2

Attachments:

smime.p7s (5.56 kB)

2015-10-28 00:46:51

[permalink] [raw]

Subject: Re: [PATCH v2 0/5] crypto: add algif_akcipher user space API

Hi Stephan,

>>> Albeit that all sounds like the crown jewel, how do you propose that shall
>>> happen?
>>>
>>> Assume that you have a web server that has a pub and priv key in its
>>> current configuration -- I guess that is the vast majority of configs.
>>>
>>> Can you please elaborate how the process for such a web server shall
>>> really
>>> work?
>>
>> 1. Create a kernel-side key.
>> 2. Use it.
>>
>> That may require adding an API similar to the one you're proposing, but
>> working with kernel keys instead of directly with akcipher. Or perhaps
>> the key subsystem can already offer what you need in userspace. David?
>
> Ohh, I see. So, you are saying that there should not be a setpub/privkey for
> the akcipher AF_ALG interface?!

I tested support for adding ALG_SET_KEY_ID which takes the 32-bit key serial and then using AF_ALG with an skcipher. That works so far so fine. For making this super clean and get it upstream, it needs a symmetric key type (which is planned to be added by David). I can post my current patch as RFC since it is dead simple actually.

> If somebody wants to use akcipher, he shall set the key via the keyring and
> akcipher shall obtain it from the keyring?
>
> However, for the actual data shoveling, AF_ALG should still be used?

There is no massive data shoveling by an akcipher. All data shoveling for TLS is done via the symmetric session key that is negotiated. Actually with asymmetric ciphers, your keys will be normally larger than your clear text anyway.

So if a server has public/private key pair, then the first thing that should the server do is load this key pair into the kernel and retrieve a key serial for it. And then use this key id to derive the session key. That session key can then be used with AF_ALG and skcipher for the data shoveling.

However that all said, the keys should never leave the kernel. Neither the private key nor the session key. There is no point in sending keys through userspace. We actually do not want this at all. That is especially important if your actual private/public key pair is in hardware. So maybe your RSA accelerator might expose secure storage for the keys. Loading them over and over again from userspace makes no sense.

As David mentioned, we need to take a deep look at what the userspace API for asymmetric cipher suites (and we also have needs for ECDH etc. and not just RSA) should look like. Just exposing akcipher via AF_ALG is premature. If we expose it now, it is not an API that we can take back. Having two userspace APIs for the exactly the same functionality is a bad thing. Especially if one is limited to software only keys.

We also need to look at the larger picture here. And that is TLS support in the kernel. Potentially via AF_KCM or something similar.

And with TLS in mind, we actually do want to just load the whole X.509 certificate into the key subsystem. It will then allow you retrieve the key id, validate the certificate and use the key. Current kernels already support this kind of functionality and we want to enable this and extend it. Having random pieces in userspace and/or kernel space to extract keys from the containers is a pretty bad idea. We really want this centralized. Here the same goal applies, to not have multiple userspace APIs doing the same thing.

Regards

Marcel

2015-10-28 01:18:35

[permalink] [raw]

Subject: Re: [PATCH v2 0/5] crypto: add algif_akcipher user space API

Am Mittwoch, 28. Oktober 2015, 09:37:02 schrieb David Woodhouse:

Hi David,

> On Wed, 2015-10-28 at 00:47 +0100, Stephan Mueller wrote:
> > Ohh, I see. So, you are saying that there should not be a setpub/privkey
> > for the akcipher AF_ALG interface?!
> >
> > If somebody wants to use akcipher, he shall set the key via the keyring
> > and
> > akcipher shall obtain it from the keyring?
> >
> > However, for the actual data shoveling, AF_ALG should still be used?
>
> That might seem ideal, but Herbert doesn't want that — he wants
> akcipher to work *only* with its own internal keys, not with keys from
> the kernel's key subsystem.
>
> David has patches (not upstream yet; used for testing) which expose a
> verify operation for kernel keys through sys_keyctl(). Adding the other
> three operations would be feasible.
>
> Perhaps if we *really* want this to appear through AF_ALG, the key
> subsystem could register its own RSA (and other) implementation(s) with
> the crypto subsystem. Then we could find some way that we can pass the
> key_serial_t through alg_setkey() instead of the actual key data
> without it (or Herbert) noticing that's what we're doing. But... ick.
> And I don't think we can select algorithms based on the actual key
> rather than the type. We should do it properly, if at all. And define
> the API as taking key IDs instead of having it be a nasty special-case
> hack for a specific (but really, very generic) algorithm provider.

First of all, I personally would not insist on an AF_ALG for pure akcipher
externalization. Integrating it with the key subsystem looks like a fine idea.

But that would imply that we tie the crypto API quite hard with the key
retention system which I guess may not be warranted.

Note, I was playing with the idea of using the key retention system for the
kernel crypto API myself (hence also my patch to add key wrapping support
which has its roots there).

But having a tie between both, the kernel crypto API and the key system, that
cannot be cut any more is something I am not sure about. Both should and would
work in isolation of each other as both serve different needs.

But I agree that when having both and the user wants proper key management,
the key system should be used. But isn't that already a policy decision of the
user/administrator? IIRC, the kernel should not hard-wire policies that may or
may not be wanted by user space. Hence, the decision about connecting both
systems should rest in user space. And the kernel should support a joint
operation of both.

--
Ciao
Stephan

2015-10-28 01:29:59

[permalink] [raw]

Subject: Re: [PATCH v2 0/5] crypto: add algif_akcipher user space API

Am Mittwoch, 28. Oktober 2015, 09:46:51 schrieb Marcel Holtmann:

Hi Marcel,

> So if a server has public/private key pair, then the first thing that should
> the server do is load this key pair into the kernel and retrieve a key
> serial for it. And then use this key id to derive the session key. That
> session key can then be used with AF_ALG and skcipher for the data
> shoveling.
>
> However that all said, the keys should never leave the kernel. Neither the

I personally do not fully agree here. For our day-to-day desktops and servers
I would fully and completely agree. But I see other use cases of Linux in
routers or other embedded systems where there may be other checks and balances
in place where this hard demand is not warranted.

Thus, I feel that this is a policy decision to be made in user space (see my
other email -- please answer on that topic there to keep a single thread).

> private key nor the session key. There is no point in sending keys through
> userspace. We actually do not want this at all. That is especially
> important if your actual private/public key pair is in hardware. So maybe
> your RSA accelerator might expose secure storage for the keys. Loading them
> over and over again from userspace makes no sense.
>
> As David mentioned, we need to take a deep look at what the userspace API
> for asymmetric cipher suites (and we also have needs for ECDH etc. and not
> just RSA) should look like. Just exposing akcipher via AF_ALG is premature.
> If we expose it now, it is not an API that we can take back. Having two
> userspace APIs for the exactly the same functionality is a bad thing.
> Especially if one is limited to software only keys.

Do not get me wrong, my patch is shall be there for all to comment. I have no
issues when we find a better solution. And I also do not like multiple
interfaces that would not be needed if we would have thought better.
>
> We also need to look at the larger picture here. And that is TLS support in
> the kernel. Potentially via AF_KCM or something similar.

With all due respect, I would object here. When we say yes to TLS (even if it
is parts of TLS up to the point where the KDF happens), we invite all higher
level crypto implementations: IKE, SNMP, SSH -- I would not want to go down
that path that started by simply supporting accelerated asymmetric ciphers.

Look at user space crypto libs: where is the most fuzz happening? Not in the
cipher implementations, but in the network protocols.

--
Ciao
Stephan

2015-10-28 01:36:43

[permalink] [raw]

Subject: Re: [PATCH v2 0/5] crypto: add algif_akcipher user space API

On Wed, 2015-10-28 at 02:18 +0100, Stephan Mueller wrote:
>
> But having a tie between both, the kernel crypto API and the key system, that
> cannot be cut any more is something I am not sure about. Both should and would
> work in isolation of each other as both serve different needs.

Sure, let people load keys directly without having to instantiate keys
and then reference them. My point is that only an API which permits
*both* models is acceptable. Otherwise, people build bogus assumptions
all the way up the stack.

Having both ALG_SET_KEY and ALG_SET_KEY_ID in parallel seems ideal.

--
dwmw2

Attachments:

smime.p7s (5.56 kB)

2015-10-28 02:57:08

[permalink] [raw]

Subject: Re: [PATCH v2 0/5] crypto: add algif_akcipher user space API

Hi Stephan,

>> So if a server has public/private key pair, then the first thing that should
>> the server do is load this key pair into the kernel and retrieve a key
>> serial for it. And then use this key id to derive the session key. That
>> session key can then be used with AF_ALG and skcipher for the data
>> shoveling.
>>
>> However that all said, the keys should never leave the kernel. Neither the
>
> I personally do not fully agree here. For our day-to-day desktops and servers
> I would fully and completely agree. But I see other use cases of Linux in
> routers or other embedded systems where there may be other checks and balances
> in place where this hard demand is not warranted.

actually in embedded devices (especially the tiny IoT stuff I am looking after) I really want keys in a central place and do not have to copy them around all the time. Keeping keys secure is actually a hard job. Doing that multiple places in the kernel and in userspace seems not a good idea. Especially not for an embedded device.

The keys subsystem is great since it includes access control to your keys. If you have the right permissions you can get the keys back out if you want to. However if the key is locked down, you will not be able to read it again.

I mean the keys subsystem can operate on per user, per process and per thread keys and access rights. So the policy that you are after is already done nicely via the keys subsystem. I really don't think we are limiting anything here. We are actually giving users more choice for policy.

> Thus, I feel that this is a policy decision to be made in user space (see my
> other email -- please answer on that topic there to keep a single thread).
>
>> private key nor the session key. There is no point in sending keys through
>> userspace. We actually do not want this at all. That is especially
>> important if your actual private/public key pair is in hardware. So maybe
>> your RSA accelerator might expose secure storage for the keys. Loading them
>> over and over again from userspace makes no sense.
>>
>> As David mentioned, we need to take a deep look at what the userspace API
>> for asymmetric cipher suites (and we also have needs for ECDH etc. and not
>> just RSA) should look like. Just exposing akcipher via AF_ALG is premature.
>> If we expose it now, it is not an API that we can take back. Having two
>> userspace APIs for the exactly the same functionality is a bad thing.
>> Especially if one is limited to software only keys.
>
> Do not get me wrong, my patch is shall be there for all to comment. I have no
> issues when we find a better solution. And I also do not like multiple
> interfaces that would not be needed if we would have thought better.
>>
>> We also need to look at the larger picture here. And that is TLS support in
>> the kernel. Potentially via AF_KCM or something similar.
>
> With all due respect, I would object here. When we say yes to TLS (even if it
> is parts of TLS up to the point where the KDF happens), we invite all higher
> level crypto implementations: IKE, SNMP, SSH -- I would not want to go down
> that path that started by simply supporting accelerated asymmetric ciphers.

Reality is that TLS in the kernel is happening. Reality is also that we do signing and certificate handling in the kernel. How much of the cipher negotiations and key derivation happens in kernel vs userspace is something to be still discussed. And there will be userspace involved for sure. However that does not mean that we keep moving keys through userspace to just load it back into another subsystem. That kind of API really needs to stop.

One other note I make is that we are looking at such tiny systems that including OpenSSL or GnuTLS is not an option at all. However we do use secure boot and need to be able to handle certificates and TLS.

> Look at user space crypto libs: where is the most fuzz happening? Not in the
> cipher implementations, but in the network protocols.

Ciphers itself are easy of course. What we are saying here is that we do not have the cipher as the main important input on what to do. We have the key itself. The key decides what options for ciphers or fallback ciphers we have. So let me repeat this, we need to think really hard about supporting hardware keys. We need to see the bigger picture. Just looking at ciphers is not enough.

Regards

Marcel

2015-10-28 10:12:05

[permalink] [raw]

Subject: Re: [PATCH v2 0/5] crypto: add algif_akcipher user space API

Am Mittwoch, 28. Oktober 2015, 11:56:59 schrieb Marcel Holtmann:

Hi Marcel,
>>
>> With all due respect, I would object here. When we say yes to TLS (even if
>> it is parts of TLS up to the point where the KDF happens), we invite all
>> higher level crypto implementations: IKE, SNMP, SSH -- I would not want to
>> go down that path that started by simply supporting accelerated asymmetric
>> ciphers.
>Reality is that TLS in the kernel is happening. Reality is also that we do

This is simply wrong IMHO for anything beyond the key handling. But that is
not the topic here, I guess.

Ciao
Stephan

2015-10-30 08:16:47

[permalink] [raw]

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

Hi Stephan,

> For supporting asymmetric ciphers, user space must be able to set the
> public key. The patch adds a new setsockopt call for setting the public
> key.
>
> Signed-off-by: Stephan Mueller <[email protected]>
> ---
> crypto/af_alg.c | 14 +++++++++++---
> include/crypto/if_alg.h | 1 +
> 2 files changed, 12 insertions(+), 3 deletions(-)
>
> diff --git a/crypto/af_alg.c b/crypto/af_alg.c
> index a8e7aa3..bf6528e 100644
> --- a/crypto/af_alg.c
> +++ b/crypto/af_alg.c
> @@ -173,13 +173,16 @@ static int alg_bind(struct socket *sock, struct sockaddr *uaddr, int addr_len)
> }
>
> static int alg_setkey(struct sock *sk, char __user *ukey,
> - unsigned int keylen)
> + unsigned int keylen, bool pubkey)
> {
> struct alg_sock *ask = alg_sk(sk);
> const struct af_alg_type *type = ask->type;
> u8 *key;
> int err;
>
> + if (pubkey && !type->setpubkey)
> + return -EOPNOTSUPP;
> +
> key = sock_kmalloc(sk, keylen, GFP_KERNEL);
> if (!key)
> return -ENOMEM;
> @@ -188,7 +191,10 @@ static int alg_setkey(struct sock *sk, char __user *ukey,
> if (copy_from_user(key, ukey, keylen))
> goto out;
>
> - err = type->setkey(ask->private, key, keylen);
> + if (pubkey)
> + err = type->setpubkey(ask->private, key, keylen);
> + else
> + err = type->setkey(ask->private, key, keyless);

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

>
> out:
> sock_kzfree_s(sk, key, keylen);
> @@ -212,12 +218,14 @@ static int alg_setsockopt(struct socket *sock, int level, int optname,
>
> switch (optname) {
> case ALG_SET_KEY:
> + case ALG_SET_PUBKEY:
> if (sock->state == SS_CONNECTED)
> goto unlock;
> if (!type->setkey)
> goto unlock;
>
> - err = alg_setkey(sk, optval, optlen);
> + err = alg_setkey(sk, optval, optlen,
> + (optname == ALG_SET_PUBKEY) ? true : false);
> break;

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

> case ALG_SET_AEAD_AUTHSIZE:
> if (sock->state == SS_CONNECTED)
> diff --git a/include/crypto/if_alg.h b/include/crypto/if_alg.h
> index 018afb2..ca4dc72 100644
> --- a/include/crypto/if_alg.h
> +++ b/include/crypto/if_alg.h
> @@ -49,6 +49,7 @@ struct af_alg_type {
> void *(*bind)(const char *name, u32 type, u32 mask);
> void (*release)(void *private);
> int (*setkey)(void *private, const u8 *key, unsigned int keylen);
> + int (*setpubkey)(void *private, const u8 *key, unsigned int keylen);
> int (*accept)(void *private, struct sock *sk);
> int (*setauthsize)(void *private, unsigned int authorize);

Regards

Marcel

2015-10-30 08:42:33