linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Milan Broz <gmazyland@gmail.com>
To: Herbert Xu <herbert@gondor.apana.org.au>
Cc: Stephan Mueller <smueller@chronox.de>,
	Dmitry Vyukov <dvyukov@google.com>,
	syzkaller@googlegroups.com, davem@davemloft.net,
	linux-crypto@vger.kernel.org, linux-kernel@vger.kernel.org,
	kcc@google.com, glider@google.com, edumazet@google.com,
	sasha.levin@oracle.com, keescook@google.com,
	Ondrej Kozina <okozina@redhat.com>
Subject: Re: [PATCH 1/2] crypto: af_alg - Add nokey compatibility path
Date: Mon, 4 Jan 2016 13:33:54 +0100	[thread overview]
Message-ID: <568A66B2.4090307@gmail.com> (raw)
In-Reply-To: <20160104043518.GA4411@gondor.apana.org.au>


On 01/04/2016 05:35 AM, Herbert Xu wrote:
> On Sun, Jan 03, 2016 at 10:42:28AM +0100, Milan Broz wrote:
>>
>> yes, basically it prepares socket()/bind()/accept() and then it calls setkey once.
>> (I'll try to fix in next releases to call setkey first though.)
> 
> OK please try these two patches (warning, totally untested).

Well, it is not much better.

I had to apply another two patches that are not mentioned and are not in your tree yet
before it:
  crypto: af_alg - Disallow bind_setkey_... after accept(2)
  crypto: use-after-free in alg_bind

then it at least compiles correctly.

skcipher works, But I still see two compatibility problems:

- hmac() is now failing the same way (SETKEY after accept())
(I initially tested without two patches above, these are not in linux-next yet.)
This breaks all cryptsetup TrueCrypt support (and moreover all systems if
kernel crypto API is set as a default vcrypto backend - but that's not default).

- cipher_null before worked without setkey, now it requires to set key
(either before or after accept().
This was actually probably bad workaround in cryptsetup, anyway it will now cause
old cryptsetup-reencrypt tool failing with your patches.
(Try "cryptsetup benchmark -c cipher_null-ecb". I am not sure what to do here,
but not requiring setkey for "cipher" that has no key internally seems ok for me...)

As I said, I'll fix it in cryptsetup upstream but we are breaking  a lot of existing systems.
Isn't there better way, how to fix it?

Milan

  parent reply	other threads:[~2016-01-04 12:34 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-12-17 12:59 GPF in lrw_crypt Dmitry Vyukov
2015-12-21 22:58 ` Stephan Mueller
2015-12-24  9:39 ` Herbert Xu
2015-12-24 11:03   ` Dmitry Vyukov
2015-12-25  7:40     ` [PATCH v2] crypto: algif_skcipher - Require setkey before accept(2) Herbert Xu
2015-12-28 13:39       ` Dmitry Vyukov
2015-12-29 13:24         ` Herbert Xu
2016-01-02 11:52       ` Milan Broz
2016-01-02 14:41         ` Milan Broz
2016-01-02 20:03           ` Stephan Mueller
2016-01-02 20:18             ` Milan Broz
2016-01-03  1:31               ` Herbert Xu
2016-01-03  9:42                 ` Milan Broz
2016-01-04  4:35                   ` [PATCH 1/2] crypto: af_alg - Add nokey compatibility path Herbert Xu
2016-01-04  4:36                     ` [PATCH 2/2] crypto: algif_skcipher " Herbert Xu
2016-01-04 12:33                     ` Milan Broz [this message]
2016-01-08 12:48                       ` [PATCH 1/2] crypto: af_alg " Herbert Xu
2016-01-08 18:21                         ` Milan Broz
2016-01-09  5:41                           ` Herbert Xu
2016-01-09 10:14                             ` Milan Broz
2016-01-11 13:26                               ` [PATCH 1/2] crypto: skcipher - Add crypto_skcipher_has_setkey Herbert Xu
2016-01-11 13:29                                 ` [PATCH 2/2] crypto: algif_skcipher - Add key check exception for cipher_null Herbert Xu
2016-01-11 14:59                                   ` Milan Broz
2016-01-08 13:28                       ` [PATCH 1/2] crypto: hash - Add crypto_ahash_has_setkey Herbert Xu
2016-01-08 13:31                         ` [PATCH 2/2] crypto: algif_hash - Require setkey before accept(2) Herbert Xu
2016-01-08 13:54                           ` kbuild test robot
2016-01-09 10:15                           ` Milan Broz

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=568A66B2.4090307@gmail.com \
    --to=gmazyland@gmail.com \
    --cc=davem@davemloft.net \
    --cc=dvyukov@google.com \
    --cc=edumazet@google.com \
    --cc=glider@google.com \
    --cc=herbert@gondor.apana.org.au \
    --cc=kcc@google.com \
    --cc=keescook@google.com \
    --cc=linux-crypto@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=okozina@redhat.com \
    --cc=sasha.levin@oracle.com \
    --cc=smueller@chronox.de \
    --cc=syzkaller@googlegroups.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).