linux-kernel - Re: [PATCH v2] crypto: algif_skcipher

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Sat, 02 Jan 2016 21:03:41 +0100
From:	Stephan Mueller <smueller@...onox.de>
To:	Milan Broz <gmazyland@...il.com>
Cc:	Herbert Xu <herbert@...dor.apana.org.au>,
	Dmitry Vyukov <dvyukov@...gle.com>, syzkaller@...glegroups.com,
	davem@...emloft.net, linux-crypto@...r.kernel.org,
	linux-kernel@...r.kernel.org, kcc@...gle.com, glider@...gle.com,
	edumazet@...gle.com, sasha.levin@...cle.com, keescook@...gle.com
Subject: Re: [PATCH v2] crypto: algif_skcipher - Require setkey before accept(2)

Am Samstag, 2. Januar 2016, 15:41:34 schrieb Milan Broz:

Hi Milan,

> On 01/02/2016 12:52 PM, Milan Broz wrote:
> > On 12/25/2015 08:40 AM, Herbert Xu wrote:
> >> Dmitry Vyukov <dvyukov@...gle.com> wrote:
> >>> I am testing with your two patches:
> >>> crypto: algif_skcipher - Use new skcipher interface
> >>> crypto: algif_skcipher - Require setkey before accept(2)
> >>> on top of a88164345b81292b55a8d4829fdd35c8d611cd7d (Dec 23).
> >> 
> >> You sent the email to everyone on the original CC list except me.
> >> Please don't do that.
> >> 
> >>> Now the following program causes a bunch of use-after-frees and them
> >> 
> >>> kills kernel:
> >> Yes there is an obvious bug in the patch that Julia Lawall has
> >> responded to in another thread.  Here is a fixed version.
> >> 
> >> ---8<--
> >> 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.
> > 
> > Hi Herbert,
> > 
> > this patch breaks userspace in cryptsetup...
> > 
> > We use algif_skcipher in cryptsetup (for years, even before
> > there was Stephan's library) and with this patch applied
> > I see fail in ALG_SET_IV call (patch from your git).
> 
> (Obviously this was because of failing accept() call here, not set_iv.)
> 
> > I can fix it upstream, but for thousands of installations it will
> > be broken (for LUKS there is a fallback, cor TrueCrypt compatible devices
> > it will be unusable. Also people who configured kernel crypto API as
> > default backend will have non-working cryptsetup).
> > 
> > Is it really thing for stable branch?
> 
> Also how it is supposed to work for cipher_null, where there is no key?
> Why it should call set_key if it is noop? (and set key length 0 is not
> possible).
> 
> (We are using cipher_null for testing and for offline re-encryption tool
> to create temporary "fake" header for not-yet encrypted device...)

The change implies that any setkey or set IV operations (i.e. any operations 
on the tfmfd) are done before the opfd(s) are created with one or more accept 
calls.

Thus, after a bind that returns the tfmfd, the setkey and setiv operations 
shall be called. This is followed by accept. If you change the order of 
invocations in your code, it should work.
> 
> Milan


-- 
Ciao
Stephan
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/