Re: [PATCH] Re: Broken userspace crypto in linux-4.1.18
On 02/24/2016 06:12 PM, Greg KH wrote: > On Wed, Feb 24, 2016 at 09:54:48AM +0100, Milan Broz wrote: >> On 02/24/2016 09:32 AM, Jiri Slaby wrote: + af_alg_release_parent(sk); >>> >>> and this occurs to me like a double release? >> >> yes, my copy&paste mistake. > > Which is why I want the real patches backported please. Whenever we do > a "just this smaller patch" for a stable kernel, it is ALWAYS wrong. I think that it was clear that I do not want you to directly include this patch, just it points to the direction where is the problem. Anyway, seems the problem is only in 4.1.18. > Please backport the patches in a correct way so that we can apply > them... Not sure if it is still needed, but I'll reply to this thread with my git version of backported patches for 4.1.18. (Resp. only the first need changes, other then applied cleanly from upstream). Thanks, Milan -- To unsubscribe from this list: send the line "unsubscribe linux-crypto" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/8] akcipher: Move the RSA DER encoding to the crypto layer
Andrew Zaborowski wrote: > Without overhauling akcipher you could modify pkcs1pad so that sign > takes the hash as input, adds the DER struct in front of it to build > the signature, and the verify operation could at most check that the > DER string matches the hash type and return the hash. But I think > RFC2437 suggests that you rather compare the signatures, not the > hashes. Whilst that is true about what RFC2437 shows, I wonder how strict it wants to be about that rather than it just being a convenient way of describing the algorithm. The advantage of doing it the way the RFC suggests is that you get to use the EMSA-PKCS1-V1_5-ENCODE operation twice, thereby saving code and only having one place for bugs to occur instead of two - but you can argue this either way. That said, I would be okay with it returning just the message hash with the padding stripped off, providing the padding is validated in the crypto layer, if that's necessary. David -- To unsubscribe from this list: send the line "unsubscribe linux-crypto" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/4] crypto: algif_skcipher - Require setkey before accept(2)
From: Herbert Xu commit dd504589577d8e8e70f51f997ad487a4cb6c026f upstream. Some cipher implementations will crash if you try to use them without calling setkey first. This patch adds a check so that the accept(2) call will fail with -ENOKEY if setkey hasn't been done on the socket yet. Cc: sta...@vger.kernel.org Reported-by: Dmitry Vyukov Signed-off-by: Herbert Xu Tested-by: Dmitry Vyukov [backported to 4.1 by Milan Broz ] --- crypto/algif_skcipher.c | 48 +--- 1 file changed, 41 insertions(+), 7 deletions(-) diff --git a/crypto/algif_skcipher.c b/crypto/algif_skcipher.c index 5bc42f9..1c9879d 100644 --- a/crypto/algif_skcipher.c +++ b/crypto/algif_skcipher.c @@ -31,6 +31,11 @@ struct skcipher_sg_list { struct scatterlist sg[0]; }; +struct skcipher_tfm { + struct crypto_ablkcipher *skcipher; + bool has_key; +}; + struct skcipher_ctx { struct list_head tsgl; struct af_alg_sgl rsgl; @@ -752,17 +757,41 @@ static struct proto_ops algif_skcipher_ops = { static void *skcipher_bind(const char *name, u32 type, u32 mask) { - return crypto_alloc_ablkcipher(name, type, mask); + struct skcipher_tfm *tfm; + struct crypto_ablkcipher *skcipher; + + tfm = kzalloc(sizeof(*tfm), GFP_KERNEL); + if (!tfm) + return ERR_PTR(-ENOMEM); + + skcipher = crypto_alloc_ablkcipher(name, type, mask); + if (IS_ERR(skcipher)) { + kfree(tfm); + return ERR_CAST(skcipher); + } + + tfm->skcipher = skcipher; + + return tfm; } static void skcipher_release(void *private) { - crypto_free_ablkcipher(private); + struct skcipher_tfm *tfm = private; + + crypto_free_ablkcipher(tfm->skcipher); + kfree(tfm); } static int skcipher_setkey(void *private, const u8 *key, unsigned int keylen) { - return crypto_ablkcipher_setkey(private, key, keylen); + struct skcipher_tfm *tfm = private; + int err; + + err = crypto_ablkcipher_setkey(tfm->skcipher, key, keylen); + tfm->has_key = !err; + + return err; } static void skcipher_wait(struct sock *sk) @@ -794,20 +823,25 @@ static int skcipher_accept_parent(void *private, struct sock *sk) { struct skcipher_ctx *ctx; struct alg_sock *ask = alg_sk(sk); - unsigned int len = sizeof(*ctx) + crypto_ablkcipher_reqsize(private); + struct skcipher_tfm *tfm = private; + struct crypto_ablkcipher *skcipher = tfm->skcipher; + unsigned int len = sizeof(*ctx) + crypto_ablkcipher_reqsize(skcipher); + + if (!tfm->has_key) + return -ENOKEY; ctx = sock_kmalloc(sk, len, GFP_KERNEL); if (!ctx) return -ENOMEM; - ctx->iv = sock_kmalloc(sk, crypto_ablkcipher_ivsize(private), + ctx->iv = sock_kmalloc(sk, crypto_ablkcipher_ivsize(skcipher), GFP_KERNEL); if (!ctx->iv) { sock_kfree_s(sk, ctx, len); return -ENOMEM; } - memset(ctx->iv, 0, crypto_ablkcipher_ivsize(private)); + memset(ctx->iv, 0, crypto_ablkcipher_ivsize(skcipher)); INIT_LIST_HEAD(&ctx->tsgl); ctx->len = len; @@ -820,7 +854,7 @@ static int skcipher_accept_parent(void *private, struct sock *sk) ask->private = ctx; - ablkcipher_request_set_tfm(&ctx->req, private); + ablkcipher_request_set_tfm(&ctx->req, skcipher); ablkcipher_request_set_callback(&ctx->req, CRYPTO_TFM_REQ_MAY_BACKLOG, af_alg_complete, &ctx->completion); -- 2.7.0 -- To unsubscribe from this list: send the line "unsubscribe linux-crypto" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/4] crypto: algif_skcipher - Add nokey compatibility path
From: Herbert Xu commit a0fa2d037129a9849918a92d91b79ed6c7bd2818 upstream. This patch adds a compatibility path to support old applications that do acept(2) before setkey. Cc: sta...@vger.kernel.org Signed-off-by: Herbert Xu --- crypto/algif_skcipher.c | 149 ++-- 1 file changed, 144 insertions(+), 5 deletions(-) diff --git a/crypto/algif_skcipher.c b/crypto/algif_skcipher.c index 1c9879d..566df2c 100644 --- a/crypto/algif_skcipher.c +++ b/crypto/algif_skcipher.c @@ -755,6 +755,99 @@ static struct proto_ops algif_skcipher_ops = { .poll = skcipher_poll, }; +static int skcipher_check_key(struct socket *sock) +{ + int err; + struct sock *psk; + struct alg_sock *pask; + struct skcipher_tfm *tfm; + struct sock *sk = sock->sk; + struct alg_sock *ask = alg_sk(sk); + + if (ask->refcnt) + return 0; + + psk = ask->parent; + pask = alg_sk(ask->parent); + tfm = pask->private; + + err = -ENOKEY; + lock_sock(psk); + if (!tfm->has_key) + goto unlock; + + if (!pask->refcnt++) + sock_hold(psk); + + ask->refcnt = 1; + sock_put(psk); + + err = 0; + +unlock: + release_sock(psk); + + return err; +} + +static int skcipher_sendmsg_nokey(struct socket *sock, struct msghdr *msg, + size_t size) +{ + int err; + + err = skcipher_check_key(sock); + if (err) + return err; + + return skcipher_sendmsg(sock, msg, size); +} + +static ssize_t skcipher_sendpage_nokey(struct socket *sock, struct page *page, + int offset, size_t size, int flags) +{ + int err; + + err = skcipher_check_key(sock); + if (err) + return err; + + return skcipher_sendpage(sock, page, offset, size, flags); +} + +static int skcipher_recvmsg_nokey(struct socket *sock, struct msghdr *msg, + size_t ignored, int flags) +{ + int err; + + err = skcipher_check_key(sock); + if (err) + return err; + + return skcipher_recvmsg(sock, msg, ignored, flags); +} + +static struct proto_ops algif_skcipher_ops_nokey = { + .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= skcipher_sendmsg_nokey, + .sendpage = skcipher_sendpage_nokey, + .recvmsg= skcipher_recvmsg_nokey, + .poll = skcipher_poll, +}; + static void *skcipher_bind(const char *name, u32 type, u32 mask) { struct skcipher_tfm *tfm; @@ -804,7 +897,7 @@ static void skcipher_wait(struct sock *sk) msleep(100); } -static void skcipher_sock_destruct(struct sock *sk) +static void skcipher_sock_destruct_common(struct sock *sk) { struct alg_sock *ask = alg_sk(sk); struct skcipher_ctx *ctx = ask->private; @@ -816,10 +909,33 @@ static void skcipher_sock_destruct(struct sock *sk) skcipher_free_sgl(sk); sock_kzfree_s(sk, ctx->iv, crypto_ablkcipher_ivsize(tfm)); sock_kfree_s(sk, ctx, ctx->len); +} + +static void skcipher_sock_destruct(struct sock *sk) +{ + skcipher_sock_destruct_common(sk); af_alg_release_parent(sk); } -static int skcipher_accept_parent(void *private, struct sock *sk) +static void skcipher_release_parent_nokey(struct sock *sk) +{ + struct alg_sock *ask = alg_sk(sk); + + if (!ask->refcnt) { + sock_put(ask->parent); + return; + } + + af_alg_release_parent(sk); +} + +static void skcipher_sock_destruct_nokey(struct sock *sk) +{ + skcipher_sock_destruct_common(sk); + skcipher_release_parent_nokey(sk); +} + +static int skcipher_accept_parent_common(void *private, struct sock *sk) { struct skcipher_ctx *ctx; struct alg_sock *ask = alg_sk(sk); @@ -827,9 +943,6 @@ static int skcipher_accept_parent(void *private, struct sock *sk) struct crypto_ablkcipher *skcipher = tfm->skcipher; unsigned int len = sizeof(*ctx) + crypto_ablkcipher_reqsize(skcipher); - if (!tfm->has_key) - return -ENOKEY; - ctx = sock_kmalloc(sk, len, GFP_KERNEL); if (!ctx) return -ENOMEM; @@ -863,12 +976,38 @@ static
[PATCH 3/4] crypto: algif_skcipher - Remove custom release parent function
From: Herbert Xu commit d7b65aee1e7b4c87922b0232eaba56a8a143a4a0 upstream. This patch removes the custom release parent function as the generic af_alg_release_parent now works for nokey sockets too. Cc: sta...@vger.kernel.org Signed-off-by: Herbert Xu --- crypto/algif_skcipher.c | 43 +++ 1 file changed, 3 insertions(+), 40 deletions(-) diff --git a/crypto/algif_skcipher.c b/crypto/algif_skcipher.c index 566df2c..83bcf75 100644 --- a/crypto/algif_skcipher.c +++ b/crypto/algif_skcipher.c @@ -897,7 +897,7 @@ static void skcipher_wait(struct sock *sk) msleep(100); } -static void skcipher_sock_destruct_common(struct sock *sk) +static void skcipher_sock_destruct(struct sock *sk) { struct alg_sock *ask = alg_sk(sk); struct skcipher_ctx *ctx = ask->private; @@ -909,33 +909,10 @@ static void skcipher_sock_destruct_common(struct sock *sk) skcipher_free_sgl(sk); sock_kzfree_s(sk, ctx->iv, crypto_ablkcipher_ivsize(tfm)); sock_kfree_s(sk, ctx, ctx->len); -} - -static void skcipher_sock_destruct(struct sock *sk) -{ - skcipher_sock_destruct_common(sk); - af_alg_release_parent(sk); -} - -static void skcipher_release_parent_nokey(struct sock *sk) -{ - struct alg_sock *ask = alg_sk(sk); - - if (!ask->refcnt) { - sock_put(ask->parent); - return; - } - af_alg_release_parent(sk); } -static void skcipher_sock_destruct_nokey(struct sock *sk) -{ - skcipher_sock_destruct_common(sk); - skcipher_release_parent_nokey(sk); -} - -static int skcipher_accept_parent_common(void *private, struct sock *sk) +static int skcipher_accept_parent_nokey(void *private, struct sock *sk) { struct skcipher_ctx *ctx; struct alg_sock *ask = alg_sk(sk); @@ -983,21 +960,7 @@ static int skcipher_accept_parent(void *private, struct sock *sk) if (!tfm->has_key) return -ENOKEY; - return skcipher_accept_parent_common(private, sk); -} - -static int skcipher_accept_parent_nokey(void *private, struct sock *sk) -{ - int err; - - err = skcipher_accept_parent_common(private, sk); - if (err) - goto out; - - sk->sk_destruct = skcipher_sock_destruct_nokey; - -out: - return err; + return skcipher_accept_parent_nokey(private, sk); } static const struct af_alg_type algif_type_skcipher = { -- 2.7.0 -- To unsubscribe from this list: send the line "unsubscribe linux-crypto" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 4/4] crypto: algif_skcipher - Fix race condition in skcipher_check_key
From: Herbert Xu commit 1822793a523e5d5730b19cc21160ff1717421bc8 upstream. We need to lock the child socket in skcipher_check_key as otherwise two simultaneous calls can cause the parent socket to be freed. Cc: sta...@vger.kernel.org Signed-off-by: Herbert Xu --- crypto/algif_skcipher.c | 9 ++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/crypto/algif_skcipher.c b/crypto/algif_skcipher.c index 83bcf75..c0f0356 100644 --- a/crypto/algif_skcipher.c +++ b/crypto/algif_skcipher.c @@ -757,22 +757,23 @@ static struct proto_ops algif_skcipher_ops = { static int skcipher_check_key(struct socket *sock) { - int err; + int err = 0; struct sock *psk; struct alg_sock *pask; struct skcipher_tfm *tfm; struct sock *sk = sock->sk; struct alg_sock *ask = alg_sk(sk); + lock_sock(sk); if (ask->refcnt) - return 0; + goto unlock_child; psk = ask->parent; pask = alg_sk(ask->parent); tfm = pask->private; err = -ENOKEY; - lock_sock(psk); + lock_sock_nested(psk, SINGLE_DEPTH_NESTING); if (!tfm->has_key) goto unlock; @@ -786,6 +787,8 @@ static int skcipher_check_key(struct socket *sock) unlock: release_sock(psk); +unlock_child: + release_sock(sk); return err; } -- 2.7.0 -- To unsubscribe from this list: send the line "unsubscribe linux-crypto" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/2] lib/mpi: avoid assembler warning
A wrapper around the umull assembly instruction might reuse the input register as an output, which is undefined on some ARM machines, as pointed out by this assembler warning: CC lib/mpi/generic_mpih-mul1.o /tmp/ccxJuxIy.s: Assembler messages: /tmp/ccxJuxIy.s:53: rdhi, rdlo and rm must all be different CC lib/mpi/generic_mpih-mul2.o /tmp/ccI0scAD.s: Assembler messages: /tmp/ccI0scAD.s:53: rdhi, rdlo and rm must all be different CC lib/mpi/generic_mpih-mul3.o /tmp/ccMvVQcp.s: Assembler messages: /tmp/ccMvVQcp.s:53: rdhi, rdlo and rm must all be different This changes the constraints to force different registers to be used as output. Signed-off-by: Arnd Bergmann --- lib/mpi/longlong.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/mpi/longlong.h b/lib/mpi/longlong.h index b90e255c2a68..93336502af08 100644 --- a/lib/mpi/longlong.h +++ b/lib/mpi/longlong.h @@ -216,7 +216,7 @@ extern UDItype __udiv_qrnnd(UDItype *, UDItype, UDItype, UDItype); __asm__ ("%@ Inlined umul_ppmm\n" \ "umull %r1, %r0, %r2, %r3" \ : "=&r" ((USItype)(xh)), \ - "=r" ((USItype)(xl)) \ + "=&r" ((USItype)(xl)) \ : "r" ((USItype)(a)), \ "r" ((USItype)(b)) \ : "r0", "r1") -- 2.7.0 -- To unsubscribe from this list: send the line "unsubscribe linux-crypto" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/2] lib/mpi: use "static inline" instead of "extern inline"
When we use CONFIG_PROFILE_ALL_BRANCHES, every 'if()' introduces a static variable, but that is not allowed in 'extern inline' functions: mpi-inline.h:116:204: warning: '__f' is static but declared in inline function 'mpihelp_sub' which is not static mpi-inline.h:113:184: warning: '__f' is static but declared in inline function 'mpihelp_sub' which is not static mpi-inline.h:70:184: warning: '__f' is static but declared in inline function 'mpihelp_add' which is not static mpi-inline.h:56:204: warning: '__f' is static but declared in inline function 'mpihelp_add_1' which is not static This changes the MPI code to use 'static inline' instead, to get rid of hundreds of warnings. Signed-off-by: Arnd Bergmann --- lib/mpi/mpi-inline.h | 2 +- lib/mpi/mpi-internal.h | 8 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/lib/mpi/mpi-inline.h b/lib/mpi/mpi-inline.h index e2b39852b30a..c245ea31f785 100644 --- a/lib/mpi/mpi-inline.h +++ b/lib/mpi/mpi-inline.h @@ -30,7 +30,7 @@ #define G10_MPI_INLINE_H #ifndef G10_MPI_INLINE_DECL -#define G10_MPI_INLINE_DECL extern inline +#define G10_MPI_INLINE_DECL static inline #endif G10_MPI_INLINE_DECL mpi_limb_t diff --git a/lib/mpi/mpi-internal.h b/lib/mpi/mpi-internal.h index c65dd1bff45a..7eceeddb3fb8 100644 --- a/lib/mpi/mpi-internal.h +++ b/lib/mpi/mpi-internal.h @@ -168,19 +168,19 @@ void mpi_rshift_limbs(MPI a, unsigned int count); int mpi_lshift_limbs(MPI a, unsigned int count); /*-- mpihelp-add.c --*/ -mpi_limb_t mpihelp_add_1(mpi_ptr_t res_ptr, mpi_ptr_t s1_ptr, +static inline mpi_limb_t mpihelp_add_1(mpi_ptr_t res_ptr, mpi_ptr_t s1_ptr, mpi_size_t s1_size, mpi_limb_t s2_limb); mpi_limb_t mpihelp_add_n(mpi_ptr_t res_ptr, mpi_ptr_t s1_ptr, mpi_ptr_t s2_ptr, mpi_size_t size); -mpi_limb_t mpihelp_add(mpi_ptr_t res_ptr, mpi_ptr_t s1_ptr, mpi_size_t s1_size, +static inline mpi_limb_t mpihelp_add(mpi_ptr_t res_ptr, mpi_ptr_t s1_ptr, mpi_size_t s1_size, mpi_ptr_t s2_ptr, mpi_size_t s2_size); /*-- mpihelp-sub.c --*/ -mpi_limb_t mpihelp_sub_1(mpi_ptr_t res_ptr, mpi_ptr_t s1_ptr, +static inline mpi_limb_t mpihelp_sub_1(mpi_ptr_t res_ptr, mpi_ptr_t s1_ptr, mpi_size_t s1_size, mpi_limb_t s2_limb); mpi_limb_t mpihelp_sub_n(mpi_ptr_t res_ptr, mpi_ptr_t s1_ptr, mpi_ptr_t s2_ptr, mpi_size_t size); -mpi_limb_t mpihelp_sub(mpi_ptr_t res_ptr, mpi_ptr_t s1_ptr, mpi_size_t s1_size, +static inline mpi_limb_t mpihelp_sub(mpi_ptr_t res_ptr, mpi_ptr_t s1_ptr, mpi_size_t s1_size, mpi_ptr_t s2_ptr, mpi_size_t s2_size); /*-- mpihelp-cmp.c --*/ -- 2.7.0 -- To unsubscribe from this list: send the line "unsubscribe linux-crypto" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] X.509: Fix test for self-signed certificate
On 2016-02-24 15:54, David Howells wrote: > Hi Michal, > > I have the attached patch already in my queue. > > David > --- > commit d19fcb825912c67e09e0575b95accaa42899e07f > Author: David Howells > Date: Wed Feb 24 14:37:54 2016 + > > X.509: Don't treat self-signed keys specially Hi David, this solves my problem too, obviously. I thought the signature check for self-signed certificates was a sort of consistency check. Thanks, Michal -- To unsubscribe from this list: send the line "unsubscribe linux-crypto" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] crypto: remove padding logic from rsa.c
Tadeusz Struk wrote: > + if (memcmp(sig->digest, output, sig->digest_size) || I've added " != 0" after the memcmp(). > + req->dst_len != sig->digest_size) > + ret = -EBADMSG; Btw, this has to be -EKEYREJECTED; -EBADMSG would indicate that the container is unparseable. I wonder if we should propose a -ESIGREJECTED error... David -- To unsubscribe from this list: send the line "unsubscribe linux-crypto" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] crypto: remove padding logic from rsa.c
Tadeusz Struk wrote: > + if (memcmp(sig->digest, output, sig->digest_size) || > + req->dst_len != sig->digest_size) You must always do the size check first! David -- To unsubscribe from this list: send the line "unsubscribe linux-crypto" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/3] crypto: Add hash param to pkcs1pad
From: Tadeusz Struk This adds hash param to pkcs1pad. The pkcs1pad template can work with or without the hash. When hash param is provided then the verify operation will also verify the output against the known digest. Signed-off-by: Tadeusz Struk Signed-off-by: David Howells --- crypto/rsa-pkcs1pad.c | 182 ++--- 1 file changed, 156 insertions(+), 26 deletions(-) diff --git a/crypto/rsa-pkcs1pad.c b/crypto/rsa-pkcs1pad.c index 50f5c97e1087..1cea67d43e1d 100644 --- a/crypto/rsa-pkcs1pad.c +++ b/crypto/rsa-pkcs1pad.c @@ -18,12 +18,89 @@ #include #include +/* + * Hash algorithm OIDs plus ASN.1 DER wrappings [RFC4880 sec 5.2.2]. + */ +static const u8 rsa_digest_info_md5[] = { + 0x30, 0x20, 0x30, 0x0c, 0x06, 0x08, + 0x2a, 0x86, 0x48, 0x86, 0xf7, 0x0d, 0x02, 0x05, /* OID */ + 0x05, 0x00, 0x04, 0x10 +}; + +static const u8 rsa_digest_info_sha1[] = { + 0x30, 0x21, 0x30, 0x09, 0x06, 0x05, + 0x2b, 0x0e, 0x03, 0x02, 0x1a, + 0x05, 0x00, 0x04, 0x14 +}; + +static const u8 rsa_digest_info_rmd160[] = { + 0x30, 0x21, 0x30, 0x09, 0x06, 0x05, + 0x2b, 0x24, 0x03, 0x02, 0x01, + 0x05, 0x00, 0x04, 0x14 +}; + +static const u8 rsa_digest_info_sha224[] = { + 0x30, 0x2d, 0x30, 0x0d, 0x06, 0x09, + 0x60, 0x86, 0x48, 0x01, 0x65, 0x03, 0x04, 0x02, 0x04, + 0x05, 0x00, 0x04, 0x1c +}; + +static const u8 rsa_digest_info_sha256[] = { + 0x30, 0x31, 0x30, 0x0d, 0x06, 0x09, + 0x60, 0x86, 0x48, 0x01, 0x65, 0x03, 0x04, 0x02, 0x01, + 0x05, 0x00, 0x04, 0x20 +}; + +static const u8 rsa_digest_info_sha384[] = { + 0x30, 0x41, 0x30, 0x0d, 0x06, 0x09, + 0x60, 0x86, 0x48, 0x01, 0x65, 0x03, 0x04, 0x02, 0x02, + 0x05, 0x00, 0x04, 0x30 +}; + +static const u8 rsa_digest_info_sha512[] = { + 0x30, 0x51, 0x30, 0x0d, 0x06, 0x09, + 0x60, 0x86, 0x48, 0x01, 0x65, 0x03, 0x04, 0x02, 0x03, + 0x05, 0x00, 0x04, 0x40 +}; + +static const struct rsa_asn1_template { + const char *name; + const u8*data; + size_t size; +} rsa_asn1_templates[] = { +#define _(X) { #X, rsa_digest_info_##X, sizeof(rsa_digest_info_##X) } + _(md5), + _(sha1), + _(rmd160), + _(sha256), + _(sha384), + _(sha512), + _(sha224), + { NULL } +#undef _ +}; + +static const struct rsa_asn1_template *rsa_lookup_asn1(const char *name) +{ + const struct rsa_asn1_template *p; + + for (p = rsa_asn1_templates; p->name; p++) + if (strcmp(name, p->name) == 0) + return p; + return NULL; +} + struct pkcs1pad_ctx { struct crypto_akcipher *child; - + const char *hash_name; unsigned int key_size; }; +struct pkcs1pad_inst_ctx { + struct crypto_akcipher_spawn spawn; + const char *hash_name; +}; + struct pkcs1pad_request { struct akcipher_request child_req; @@ -339,13 +416,22 @@ static int pkcs1pad_sign(struct akcipher_request *req) struct crypto_akcipher *tfm = crypto_akcipher_reqtfm(req); struct pkcs1pad_ctx *ctx = akcipher_tfm_ctx(tfm); struct pkcs1pad_request *req_ctx = akcipher_request_ctx(req); + const struct rsa_asn1_template *digest_info = NULL; int err; - unsigned int ps_end; + unsigned int ps_end, digest_size = 0; if (!ctx->key_size) return -EINVAL; - if (req->src_len > ctx->key_size - 11) + if (ctx->hash_name) { + digest_info = rsa_lookup_asn1(ctx->hash_name); + if (!digest_info) + return -EINVAL; + + digest_size = digest_info->size; + } + + if (req->src_len + digest_size > ctx->key_size - 11) return -EOVERFLOW; if (req->dst_len < ctx->key_size) { @@ -371,11 +457,16 @@ static int pkcs1pad_sign(struct akcipher_request *req) if (!req_ctx->in_buf) return -ENOMEM; - ps_end = ctx->key_size - req->src_len - 2; + ps_end = ctx->key_size - digest_size - req->src_len - 2; req_ctx->in_buf[0] = 0x01; memset(req_ctx->in_buf + 1, 0xff, ps_end - 1); req_ctx->in_buf[ps_end] = 0x00; + if (digest_info) { + memcpy(req_ctx->in_buf + ps_end + 1, digest_info->data, + digest_info->size); + } + pkcs1pad_sg_set_buf(req_ctx->in_sg, req_ctx->in_buf, ctx->key_size - 1 - req->src_len, req->src); @@ -408,6 +499,7 @@ static int pkcs1pad_verify_complete(struct akcipher_request *req, int err) struct crypto_akcipher *tfm = crypto_akcipher_reqtfm(req); struct pkcs1pad_ctx *ctx = akcipher_tfm_ctx(tfm); struct pkcs1pad_request *req_ctx = akcipher_request_ctx(req); + const struct rsa_asn1_template *digest_info; unsigned int pos; if (err == -EOVERFLOW) @@ -422,20 +514,33 @@ static int pkcs1pad_verify_comple
[PATCH 0/3] KEYS: Use crypto rsa pkcs1pad module for software public keys
Here's a set of patches that change the software public key asymmetric key subtype to use the RSA pkcs1pad module in the crypto layer. I've merged together mine and Tadeusz's patches. I have not included Tadeusz's original three patches that converted to using the raw rsa module in this set, but they're prerequisites for this. David --- David Howells (2): akcipher: Move the RSA DER encoding check to the crypto layer X.509: Make algo identifiers text instead of enum Tadeusz Struk (1): crypto: Add hash param to pkcs1pad crypto/asymmetric_keys/Kconfig|7 - crypto/asymmetric_keys/Makefile |1 crypto/asymmetric_keys/mscode_parser.c| 14 +- crypto/asymmetric_keys/pkcs7_parser.c | 18 +- crypto/asymmetric_keys/pkcs7_verify.c |8 - crypto/asymmetric_keys/public_key.c | 120 crypto/asymmetric_keys/rsa.c | 224 - crypto/asymmetric_keys/verify_pefile.c|4 - crypto/asymmetric_keys/verify_pefile.h|2 crypto/asymmetric_keys/x509_cert_parser.c | 26 ++- crypto/asymmetric_keys/x509_public_key.c | 17 +- crypto/rsa-pkcs1pad.c | 182 include/crypto/public_key.h | 32 +--- init/Kconfig |2 kernel/module_signing.c |6 + security/integrity/digsig_asymmetric.c|7 - 16 files changed, 305 insertions(+), 365 deletions(-) delete mode 100644 crypto/asymmetric_keys/rsa.c -- To unsubscribe from this list: send the line "unsubscribe linux-crypto" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/3] crypto: Add hash param to pkcs1pad
From: Tadeusz Struk This adds hash param to pkcs1pad. The pkcs1pad template can work with or without the hash. When hash param is provided then the verify operation will also verify the output against the known digest. Signed-off-by: Tadeusz Struk Signed-off-by: David Howells --- crypto/rsa-pkcs1pad.c | 182 ++--- 1 file changed, 156 insertions(+), 26 deletions(-) diff --git a/crypto/rsa-pkcs1pad.c b/crypto/rsa-pkcs1pad.c index 50f5c97e1087..1cea67d43e1d 100644 --- a/crypto/rsa-pkcs1pad.c +++ b/crypto/rsa-pkcs1pad.c @@ -18,12 +18,89 @@ #include #include +/* + * Hash algorithm OIDs plus ASN.1 DER wrappings [RFC4880 sec 5.2.2]. + */ +static const u8 rsa_digest_info_md5[] = { + 0x30, 0x20, 0x30, 0x0c, 0x06, 0x08, + 0x2a, 0x86, 0x48, 0x86, 0xf7, 0x0d, 0x02, 0x05, /* OID */ + 0x05, 0x00, 0x04, 0x10 +}; + +static const u8 rsa_digest_info_sha1[] = { + 0x30, 0x21, 0x30, 0x09, 0x06, 0x05, + 0x2b, 0x0e, 0x03, 0x02, 0x1a, + 0x05, 0x00, 0x04, 0x14 +}; + +static const u8 rsa_digest_info_rmd160[] = { + 0x30, 0x21, 0x30, 0x09, 0x06, 0x05, + 0x2b, 0x24, 0x03, 0x02, 0x01, + 0x05, 0x00, 0x04, 0x14 +}; + +static const u8 rsa_digest_info_sha224[] = { + 0x30, 0x2d, 0x30, 0x0d, 0x06, 0x09, + 0x60, 0x86, 0x48, 0x01, 0x65, 0x03, 0x04, 0x02, 0x04, + 0x05, 0x00, 0x04, 0x1c +}; + +static const u8 rsa_digest_info_sha256[] = { + 0x30, 0x31, 0x30, 0x0d, 0x06, 0x09, + 0x60, 0x86, 0x48, 0x01, 0x65, 0x03, 0x04, 0x02, 0x01, + 0x05, 0x00, 0x04, 0x20 +}; + +static const u8 rsa_digest_info_sha384[] = { + 0x30, 0x41, 0x30, 0x0d, 0x06, 0x09, + 0x60, 0x86, 0x48, 0x01, 0x65, 0x03, 0x04, 0x02, 0x02, + 0x05, 0x00, 0x04, 0x30 +}; + +static const u8 rsa_digest_info_sha512[] = { + 0x30, 0x51, 0x30, 0x0d, 0x06, 0x09, + 0x60, 0x86, 0x48, 0x01, 0x65, 0x03, 0x04, 0x02, 0x03, + 0x05, 0x00, 0x04, 0x40 +}; + +static const struct rsa_asn1_template { + const char *name; + const u8*data; + size_t size; +} rsa_asn1_templates[] = { +#define _(X) { #X, rsa_digest_info_##X, sizeof(rsa_digest_info_##X) } + _(md5), + _(sha1), + _(rmd160), + _(sha256), + _(sha384), + _(sha512), + _(sha224), + { NULL } +#undef _ +}; + +static const struct rsa_asn1_template *rsa_lookup_asn1(const char *name) +{ + const struct rsa_asn1_template *p; + + for (p = rsa_asn1_templates; p->name; p++) + if (strcmp(name, p->name) == 0) + return p; + return NULL; +} + struct pkcs1pad_ctx { struct crypto_akcipher *child; - + const char *hash_name; unsigned int key_size; }; +struct pkcs1pad_inst_ctx { + struct crypto_akcipher_spawn spawn; + const char *hash_name; +}; + struct pkcs1pad_request { struct akcipher_request child_req; @@ -339,13 +416,22 @@ static int pkcs1pad_sign(struct akcipher_request *req) struct crypto_akcipher *tfm = crypto_akcipher_reqtfm(req); struct pkcs1pad_ctx *ctx = akcipher_tfm_ctx(tfm); struct pkcs1pad_request *req_ctx = akcipher_request_ctx(req); + const struct rsa_asn1_template *digest_info = NULL; int err; - unsigned int ps_end; + unsigned int ps_end, digest_size = 0; if (!ctx->key_size) return -EINVAL; - if (req->src_len > ctx->key_size - 11) + if (ctx->hash_name) { + digest_info = rsa_lookup_asn1(ctx->hash_name); + if (!digest_info) + return -EINVAL; + + digest_size = digest_info->size; + } + + if (req->src_len + digest_size > ctx->key_size - 11) return -EOVERFLOW; if (req->dst_len < ctx->key_size) { @@ -371,11 +457,16 @@ static int pkcs1pad_sign(struct akcipher_request *req) if (!req_ctx->in_buf) return -ENOMEM; - ps_end = ctx->key_size - req->src_len - 2; + ps_end = ctx->key_size - digest_size - req->src_len - 2; req_ctx->in_buf[0] = 0x01; memset(req_ctx->in_buf + 1, 0xff, ps_end - 1); req_ctx->in_buf[ps_end] = 0x00; + if (digest_info) { + memcpy(req_ctx->in_buf + ps_end + 1, digest_info->data, + digest_info->size); + } + pkcs1pad_sg_set_buf(req_ctx->in_sg, req_ctx->in_buf, ctx->key_size - 1 - req->src_len, req->src); @@ -408,6 +499,7 @@ static int pkcs1pad_verify_complete(struct akcipher_request *req, int err) struct crypto_akcipher *tfm = crypto_akcipher_reqtfm(req); struct pkcs1pad_ctx *ctx = akcipher_tfm_ctx(tfm); struct pkcs1pad_request *req_ctx = akcipher_request_ctx(req); + const struct rsa_asn1_template *digest_info; unsigned int pos; if (err == -EOVERFLOW) @@ -422,20 +514,33 @@ static int pkcs1pad_verify_comple
[PATCH 3/3] X.509: Make algo identifiers text instead of enum
Make the identifier public key and digest algorithm fields text instead of enum. Signed-off-by: David Howells --- crypto/asymmetric_keys/mscode_parser.c| 14 +++--- crypto/asymmetric_keys/pkcs7_parser.c | 18 - crypto/asymmetric_keys/pkcs7_verify.c |8 +++- crypto/asymmetric_keys/public_key.c | 24 --- crypto/asymmetric_keys/verify_pefile.c|4 ++-- crypto/asymmetric_keys/verify_pefile.h|2 +- crypto/asymmetric_keys/x509_cert_parser.c | 26 + crypto/asymmetric_keys/x509_public_key.c | 17 include/crypto/public_key.h | 30 - kernel/module_signing.c |6 ++ security/integrity/digsig_asymmetric.c|8 +++- 11 files changed, 59 insertions(+), 98 deletions(-) diff --git a/crypto/asymmetric_keys/mscode_parser.c b/crypto/asymmetric_keys/mscode_parser.c index adcef59eec0b..3242cbfaeaa2 100644 --- a/crypto/asymmetric_keys/mscode_parser.c +++ b/crypto/asymmetric_keys/mscode_parser.c @@ -86,25 +86,25 @@ int mscode_note_digest_algo(void *context, size_t hdrlen, oid = look_up_OID(value, vlen); switch (oid) { case OID_md4: - ctx->digest_algo = HASH_ALGO_MD4; + ctx->digest_algo = "md4"; break; case OID_md5: - ctx->digest_algo = HASH_ALGO_MD5; + ctx->digest_algo = "md5"; break; case OID_sha1: - ctx->digest_algo = HASH_ALGO_SHA1; + ctx->digest_algo = "sha1"; break; case OID_sha256: - ctx->digest_algo = HASH_ALGO_SHA256; + ctx->digest_algo = "sha256"; break; case OID_sha384: - ctx->digest_algo = HASH_ALGO_SHA384; + ctx->digest_algo = "sha384"; break; case OID_sha512: - ctx->digest_algo = HASH_ALGO_SHA512; + ctx->digest_algo = "sha512"; break; case OID_sha224: - ctx->digest_algo = HASH_ALGO_SHA224; + ctx->digest_algo = "sha224"; break; case OID__NR: diff --git a/crypto/asymmetric_keys/pkcs7_parser.c b/crypto/asymmetric_keys/pkcs7_parser.c index 3ef62dac9771..1c4f715efdde 100644 --- a/crypto/asymmetric_keys/pkcs7_parser.c +++ b/crypto/asymmetric_keys/pkcs7_parser.c @@ -218,25 +218,25 @@ int pkcs7_sig_note_digest_algo(void *context, size_t hdrlen, switch (ctx->last_oid) { case OID_md4: - ctx->sinfo->sig.pkey_hash_algo = HASH_ALGO_MD4; + ctx->sinfo->sig.hash_algo = "md4"; break; case OID_md5: - ctx->sinfo->sig.pkey_hash_algo = HASH_ALGO_MD5; + ctx->sinfo->sig.hash_algo = "md5"; break; case OID_sha1: - ctx->sinfo->sig.pkey_hash_algo = HASH_ALGO_SHA1; + ctx->sinfo->sig.hash_algo = "sha1"; break; case OID_sha256: - ctx->sinfo->sig.pkey_hash_algo = HASH_ALGO_SHA256; + ctx->sinfo->sig.hash_algo = "sha256"; break; case OID_sha384: - ctx->sinfo->sig.pkey_hash_algo = HASH_ALGO_SHA384; + ctx->sinfo->sig.hash_algo = "sha384"; break; case OID_sha512: - ctx->sinfo->sig.pkey_hash_algo = HASH_ALGO_SHA512; + ctx->sinfo->sig.hash_algo = "sha512"; break; case OID_sha224: - ctx->sinfo->sig.pkey_hash_algo = HASH_ALGO_SHA224; + ctx->sinfo->sig.hash_algo = "sha224"; default: printk("Unsupported digest algo: %u\n", ctx->last_oid); return -ENOPKG; @@ -255,7 +255,7 @@ int pkcs7_sig_note_pkey_algo(void *context, size_t hdrlen, switch (ctx->last_oid) { case OID_rsaEncryption: - ctx->sinfo->sig.pkey_algo = PKEY_ALGO_RSA; + ctx->sinfo->sig.pkey_algo = "rsa"; break; default: printk("Unsupported pkey algo: %u\n", ctx->last_oid); @@ -615,8 +615,6 @@ int pkcs7_sig_note_signature(void *context, size_t hdrlen, { struct pkcs7_parse_context *ctx = context; - BUG_ON(ctx->sinfo->sig.pkey_algo != PKEY_ALGO_RSA); - ctx->sinfo->sig.s = kmemdup(value, vlen, GFP_KERNEL); if (!ctx->sinfo->sig.s) return -ENOMEM; diff --git a/crypto/asymmetric_keys/pkcs7_verify.c b/crypto/asymmetric_keys/pkcs7_verify.c index f5db1378c096..50be2a15e531 100644 --- a/crypto/asymmetric_keys/pkcs7_verify.c +++ b/crypto/asymmetric_keys/pkcs7_verify.c @@ -31,17 +31,15 @@ static int pkcs7_digest(struct pkcs7_message *pkcs7, void *digest; int ret; - kenter(",%u,%u", sinfo->index, sinfo->sig.pkey_hash_algo); + kenter(",%u,%s", sinfo->index, sin
[PATCH 2/3] akcipher: Move the RSA DER encoding check to the crypto layer
Move the RSA EMSA-PKCS1-v1_5 encoding from the asymmetric-key public_key subtype to the rsa crypto module's pkcs1pad template. This means that the public_key subtype no longer has any dependencies on public key type. To make this work, the following changes have been made: (1) The rsa pkcs1pad template is now used for RSA keys. This strips off the padding and returns just the message hash. (2) In a previous patch, the pkcs1pad template gained an optional second parameter that, if given, specifies the hash used. We now give this, and pkcs1pad checks the encoded message E(M) for the EMSA-PKCS1-v1_5 encoding and verifies that the correct digest OID is present. (3) The crypto driver in crypto/asymmetric_keys/rsa.c is now reduced to something that doesn't care about what the encryption actually does and and has been merged into public_key.c. (4) CONFIG_PUBLIC_KEY_ALGO_RSA is gone. Module signing must set CONFIG_CRYPTO_RSA=y instead. Thoughts: (*) Should the encoding style (eg. raw, EMSA-PKCS1-v1_5) also be passed to the padding template? Should there be multiple padding templates registered that share most of the code? Signed-off-by: David Howells Signed-off-by: Tadeusz Struk --- crypto/asymmetric_keys/Kconfig |7 - crypto/asymmetric_keys/Makefile|1 crypto/asymmetric_keys/public_key.c| 104 +-- crypto/asymmetric_keys/rsa.c | 224 include/crypto/public_key.h|2 init/Kconfig |2 security/integrity/digsig_asymmetric.c |1 7 files changed, 95 insertions(+), 246 deletions(-) delete mode 100644 crypto/asymmetric_keys/rsa.c diff --git a/crypto/asymmetric_keys/Kconfig b/crypto/asymmetric_keys/Kconfig index 905d745c2f85..91a7e047a765 100644 --- a/crypto/asymmetric_keys/Kconfig +++ b/crypto/asymmetric_keys/Kconfig @@ -12,7 +12,6 @@ if ASYMMETRIC_KEY_TYPE config ASYMMETRIC_PUBLIC_KEY_SUBTYPE tristate "Asymmetric public-key crypto algorithm subtype" select MPILIB - select PUBLIC_KEY_ALGO_RSA select CRYPTO_HASH_INFO help This option provides support for asymmetric public key type handling. @@ -20,12 +19,6 @@ config ASYMMETRIC_PUBLIC_KEY_SUBTYPE appropriate hash algorithms (such as SHA-1) must be available. ENOPKG will be reported if the requisite algorithm is unavailable. -config PUBLIC_KEY_ALGO_RSA - tristate "RSA public-key algorithm" - select CRYPTO_RSA - help - This option enables support for the RSA algorithm (PKCS#1, RFC3447). - config X509_CERTIFICATE_PARSER tristate "X.509 certificate parser" depends on ASYMMETRIC_PUBLIC_KEY_SUBTYPE diff --git a/crypto/asymmetric_keys/Makefile b/crypto/asymmetric_keys/Makefile index b78a194ea014..f90486256f01 100644 --- a/crypto/asymmetric_keys/Makefile +++ b/crypto/asymmetric_keys/Makefile @@ -7,7 +7,6 @@ obj-$(CONFIG_ASYMMETRIC_KEY_TYPE) += asymmetric_keys.o asymmetric_keys-y := asymmetric_type.o signature.o obj-$(CONFIG_ASYMMETRIC_PUBLIC_KEY_SUBTYPE) += public_key.o -obj-$(CONFIG_PUBLIC_KEY_ALGO_RSA) += rsa.o # # X.509 Certificate handling diff --git a/crypto/asymmetric_keys/public_key.c b/crypto/asymmetric_keys/public_key.c index b383629b9e62..27ebc2f44394 100644 --- a/crypto/asymmetric_keys/public_key.c +++ b/crypto/asymmetric_keys/public_key.c @@ -17,8 +17,10 @@ #include #include #include +#include #include #include +#include MODULE_LICENSE("GPL"); @@ -35,12 +37,6 @@ const char *const pkey_id_type_name[PKEY_ID_TYPE__LAST] = { }; EXPORT_SYMBOL_GPL(pkey_id_type_name); -static int (*alg_verify[PKEY_ALGO__LAST])(const struct public_key *pkey, - const struct public_key_signature *sig) = { - NULL, - rsa_verify_signature -}; - /* * Provide a part of a description of the key for /proc/keys. */ @@ -68,24 +64,110 @@ void public_key_destroy(void *payload) } EXPORT_SYMBOL_GPL(public_key_destroy); +struct public_key_completion { + struct completion completion; + int err; +}; + +static void public_key_verify_done(struct crypto_async_request *req, int err) +{ + struct public_key_completion *compl = req->data; + + if (err == -EINPROGRESS) + return; + + compl->err = err; + complete(&compl->completion); +} + /* * Verify a signature using a public key. */ int public_key_verify_signature(const struct public_key *pkey, const struct public_key_signature *sig) { + struct public_key_completion compl; + struct crypto_akcipher *tfm; + struct akcipher_request *req; + struct scatterlist sig_sg, digest_sg; + const char *alg_name; + char alg_name_buf[CRYPTO_MAX_ALG_NAME]; + void *output; + unsigned int outlen; + int ret = -ENOMEM; + + pr_devel("==>%s()\n", __func__); + BUG_ON(!pk
Re: [PATCH] Re: Broken userspace crypto in linux-4.1.18
On 02/26/2016 06:25 AM, Milan Broz wrote: > On 02/24/2016 06:12 PM, Greg KH wrote: >> On Wed, Feb 24, 2016 at 09:54:48AM +0100, Milan Broz wrote: >>> On 02/24/2016 09:32 AM, Jiri Slaby wrote: > + af_alg_release_parent(sk); and this occurs to me like a double release? >>> >>> yes, my copy&paste mistake. >> >> Which is why I want the real patches backported please. Whenever we do >> a "just this smaller patch" for a stable kernel, it is ALWAYS wrong. > > I think that it was clear that I do not want you to directly include > this patch, just it points to the direction where is the problem. > > Anyway, seems the problem is only in 4.1.18. > >> Please backport the patches in a correct way so that we can apply >> them... > > Not sure if it is still needed, but I'll reply to this thread with my git > version > of backported patches for 4.1.18. > (Resp. only the first need changes, other then applied cleanly from upstream). Please do. Thanks, Sasha -- To unsubscribe from this list: send the line "unsubscribe linux-crypto" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Is a crypto_ahash_init required before invoking crypto_ahash_import?
On Thu, Feb 25, 2016 at 04:26:54PM -0600, Tom Lendacky wrote: > > Thanks for the clarification Herbert. I'll send in a patch to perform > the memset during the import. I know it's late in the cycle, but will > you be able to apply it to the current cryptodev-2.6 tree which contains > the ccp import/export patches? Yes I will add it on top. Thanks, -- Email: Herbert Xu Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt -- To unsubscribe from this list: send the line "unsubscribe linux-crypto" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html