Re: [PATCH 4.9] security/keys: rewrite all of big_key crypto

[Greg KH <greg@kroah.com>]

On Mon, Oct 02, 2017 at 12:52:56PM +0200, Jason A. Donenfeld wrote:
> commit 428490e38b2e352812e0b765d8bceafab0ec441d upstream.
> 
> This started out as just replacing the use of crypto/rng with
> get_random_bytes_wait, so that we wouldn't use bad randomness at boot
> time. But, upon looking further, it appears that there were even deeper
> underlying cryptographic problems, and that this seems to have been
> committed with very little crypto review. So, I rewrote the whole thing,
> trying to keep to the conventions introduced by the previous author, to
> fix these cryptographic flaws.
> 
> It makes no sense to seed crypto/rng at boot time and then keep
> using it like this, when in fact there's already get_random_bytes_wait,
> which can ensure there's enough entropy and be a much more standard way
> of generating keys. Since this sensitive material is being stored
> untrusted, using ECB and no authentication is simply not okay at all. I
> find it surprising and a bit horrifying that this code even made it past
> basic crypto review, which perhaps points to some larger issues. This
> patch moves from using AES-ECB to using AES-GCM. Since keys are uniquely
> generated each time, we can set the nonce to zero. There was also a race
> condition in which the same key would be reused at the same time in
> different threads. A mutex fixes this issue now.
> 
> So, to summarize, this commit fixes the following vulnerabilities:
> 
>   * Low entropy key generation, allowing an attacker to potentially
>     guess or predict keys.
>   * Unauthenticated encryption, allowing an attacker to modify the
>     cipher text in particular ways in order to manipulate the plaintext,
>     which is is even more frightening considering the next point.
>   * Use of ECB mode, allowing an attacker to trivially swap blocks or
>     compare identical plaintext blocks.
>   * Key re-use.
>   * Faulty memory zeroing.
> 
> [Note that in backporting this commit to 4.9, get_random_bytes_wait was
> replaced with get_random_bytes, since 4.9 does not have the former
> function. This might result in slightly worse entropy in key generation,
> but common use cases of big_keys makes that likely not a huge deal. And,
> this is the best we can do with this old kernel. Alas.]

Thanks for the backport, I've now taken this one instead.

greg k-h