linux-kernel - Re: [GIT PULL] KEYS: Fixes and crypto fixes

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 for Android: free password hash cracker in your pocket

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

Message-ID: <20170928001529.GA120911@gmail.com>
Date:   Wed, 27 Sep 2017 17:15:29 -0700
From:   Eric Biggers <ebiggers3@...il.com>
To:     James Morris <jmorris@...ei.org>
Cc:     David Howells <dhowells@...hat.com>,
        Eric Biggers <ebiggers@...gle.com>,
        "Jason A. Donenfeld" <Jason@...c4.com>,
        Michael Halcrow <mhalcrow@...gle.com>,
        keyrings@...r.kernel.org, linux-security-module@...r.kernel.org,
        linux-crypto@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [GIT PULL] KEYS: Fixes and crypto fixes

On Thu, Sep 28, 2017 at 09:14:58AM +1000, James Morris wrote:
> On Wed, 27 Sep 2017, David Howells wrote:
> 
> >  (2) Fixing big_key to use safe crypto from Jason A. Donenfeld.
> > 
> 
> I'm concerned about the lack of crypto review mentioned by Jason -- I 
> wonder if we can get this rewrite any more review from crypto folk.
> 
> Also, are there any tests for this code?  If not, it would be good to make 
> some.
> 

There is a test for the big_key key type in the keyutils test suite.  I also
manually tested Jason's change.  And as far as I can tell there isn't actually a
whole lot to test besides adding a big_key larger than BIG_KEY_FILE_THRESHOLD
bytes, reading it back, and verifying that the data is unchanged --- since that
covers the code that was changed.  An earlier version of the patch produced a
warning with CONFIG_DEBUG_SG=y since it put the aead_request on the stack, but
that's been fixed.

It would be great if someone else would comment on the crypto too, but for what
it's worth I'm satisfied with the crypto changes.  GCM is a much better choice
than ECB as long as we don't repeat (key, IV) pairs --- which we don't.  And in
any case ECB mode makes no sense in this context; you'd need a *very* good
reason to actually choose to encrypt something with ECB mode.  Unfortunately it
tends to be a favorite of people who don't understand encryption modes...

Plus, getting all the randomness at boot time didn't make sense because that's
when entropy is the most scarce.

Eric