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

2017-09-20 Thread Jason A. Donenfeld
On Tue, Sep 19, 2017 at 5:38 PM, David Howells wrote: > Jason A. Donenfeld wrote: > >> And, some error paths forgot to zero out sensitive >> material, so this patch changes a kfree into a kzfree. > > Can you split that out into a separate preparatory patch?

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

2017-09-20 Thread Jason A. Donenfeld
On Tue, Sep 19, 2017 at 5:38 PM, David Howells wrote: > Jason A. Donenfeld wrote: > >> And, some error paths forgot to zero out sensitive >> material, so this patch changes a kfree into a kzfree. > > Can you split that out into a separate preparatory patch? > > Also, I recommend limiting the

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

2017-09-20 Thread Jason A. Donenfeld
On Wed, Sep 20, 2017 at 4:06 PM, Stephan Mueller wrote: >> Section 3 shows an attack with repeated nonces, which we don't do here. > > Maybe I miss a point here, but zero IVs is no repetition of nonces? If there's a fresh key each time, then no, it's not a repetition. This

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

2017-09-20 Thread Jason A. Donenfeld
On Wed, Sep 20, 2017 at 4:06 PM, Stephan Mueller wrote: >> Section 3 shows an attack with repeated nonces, which we don't do here. > > Maybe I miss a point here, but zero IVs is no repetition of nonces? If there's a fresh key each time, then no, it's not a repetition. This patch uses a fresh

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

2017-09-20 Thread Stephan Mueller
Am Mittwoch, 20. September 2017, 16:01:21 CEST schrieb Jason A. Donenfeld: Hi Jason, > > Section 3 shows an attack with repeated nonces, which we don't do here. Maybe I miss a point here, but zero IVs is no repetition of nonces? Ciao Stephan

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

2017-09-20 Thread Stephan Mueller
Am Mittwoch, 20. September 2017, 16:01:21 CEST schrieb Jason A. Donenfeld: Hi Jason, > > Section 3 shows an attack with repeated nonces, which we don't do here. Maybe I miss a point here, but zero IVs is no repetition of nonces? Ciao Stephan

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

2017-09-20 Thread Jason A. Donenfeld
On Wed, Sep 20, 2017 at 3:45 PM, Stephan Mueller wrote: > http://csrc.nist.gov/groups/ST/toolkit/BCM/documents/Joux_comments.pdf Section 3 shows an attack with repeated nonces, which we don't do here. Section 4 shows an attack using a non-96-bit nonce, which we also don't

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

2017-09-20 Thread Jason A. Donenfeld
On Wed, Sep 20, 2017 at 3:45 PM, Stephan Mueller wrote: > http://csrc.nist.gov/groups/ST/toolkit/BCM/documents/Joux_comments.pdf Section 3 shows an attack with repeated nonces, which we don't do here. Section 4 shows an attack using a non-96-bit nonce, which we also don't do here.

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

2017-09-20 Thread Stephan Mueller
Am Mittwoch, 20. September 2017, 12:52:21 CEST schrieb Jason A. Donenfeld: Hi Jason, > > This sounds incorrect to me. Choosing a fresh, random, one-time-use > 256-bit key and rolling with a zero nonce is a totally legitimate way > of using GCM. There's no possible reuse of the key stream this

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

2017-09-20 Thread Stephan Mueller
Am Mittwoch, 20. September 2017, 12:52:21 CEST schrieb Jason A. Donenfeld: Hi Jason, > > This sounds incorrect to me. Choosing a fresh, random, one-time-use > 256-bit key and rolling with a zero nonce is a totally legitimate way > of using GCM. There's no possible reuse of the key stream this

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

2017-09-20 Thread Jason A. Donenfeld
On Wed, Sep 20, 2017 at 7:30 AM, Stephan Mueller wrote: > The use of GCM with the implementtion here is just as challenging. The > implementation uses a NULL IV. GCM is a very brittle cipher where the > construction of the IV is of special importance. SP800-38D section 8.2.1

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

2017-09-20 Thread Jason A. Donenfeld
On Wed, Sep 20, 2017 at 7:30 AM, Stephan Mueller wrote: > The use of GCM with the implementtion here is just as challenging. The > implementation uses a NULL IV. GCM is a very brittle cipher where the > construction of the IV is of special importance. SP800-38D section 8.2.1 and > 8.2.2 outlines

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

2017-09-19 Thread Stephan Mueller
Am Sonntag, 17. September 2017, 13:52:17 CEST schrieb Jason A. Donenfeld: Hi Jason, > * Use of ECB mode, allowing an attacker to trivially swap blocks or > compare identical plaintext blocks. The use of GCM with the implementtion here is just as challenging. The implementation uses a

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

2017-09-19 Thread Stephan Mueller
Am Sonntag, 17. September 2017, 13:52:17 CEST schrieb Jason A. Donenfeld: Hi Jason, > * Use of ECB mode, allowing an attacker to trivially swap blocks or > compare identical plaintext blocks. The use of GCM with the implementtion here is just as challenging. The implementation uses a

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

2017-09-19 Thread David Howells
Jason A. Donenfeld wrote: > And, some error paths forgot to zero out sensitive > material, so this patch changes a kfree into a kzfree. Can you split that out into a separate preparatory patch? Also, I recommend limiting the lines in your patch description to 75 chars lest you

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

2017-09-19 Thread David Howells
Jason A. Donenfeld wrote: > And, some error paths forgot to zero out sensitive > material, so this patch changes a kfree into a kzfree. Can you split that out into a separate preparatory patch? Also, I recommend limiting the lines in your patch description to 75 chars lest you get people who

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

2017-09-17 Thread Jason A. Donenfeld
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

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

2017-09-17 Thread Jason A. Donenfeld
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