From: Eric Biggers <ebiggers3@gmail.com>
To: James Morris <jmorris@namei.org>
Cc: David Howells <dhowells@redhat.com>,
Eric Biggers <ebiggers@google.com>,
"Jason A. Donenfeld" <Jason@zx2c4.com>,
Michael Halcrow <mhalcrow@google.com>,
keyrings@vger.kernel.org, linux-security-module@vger.kernel.org,
linux-crypto@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [GIT PULL] KEYS: Fixes and crypto fixes
Date: Wed, 27 Sep 2017 17:15:29 -0700 [thread overview]
Message-ID: <20170928001529.GA120911@gmail.com> (raw)
In-Reply-To: <alpine.LRH.2.21.1709280912400.12069@namei.org>
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
next prev parent reply other threads:[~2017-09-28 0:15 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-09-27 21:19 [GIT PULL] KEYS: Fixes and crypto fixes David Howells
2017-09-27 23:14 ` James Morris
2017-09-28 0:15 ` Eric Biggers [this message]
2017-09-28 2:08 ` James Morris
2017-09-28 10:34 ` Herbert Xu
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=20170928001529.GA120911@gmail.com \
--to=ebiggers3@gmail.com \
--cc=Jason@zx2c4.com \
--cc=dhowells@redhat.com \
--cc=ebiggers@google.com \
--cc=jmorris@namei.org \
--cc=keyrings@vger.kernel.org \
--cc=linux-crypto@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-security-module@vger.kernel.org \
--cc=mhalcrow@google.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