On Tue, 2021-07-20 at 13:31 +0200, Hannes Reinecke wrote:

On 7/20/21 12:49 PM, Simo Sorce wrote:

quoted

On Tue, 2021-07-20 at 12:14 +0200, Hannes Reinecke wrote:

quoted

On 7/19/21 1:52 PM, Stephan Mueller wrote:

quoted

Am Montag, dem 19.07.2021 um 13:10 +0200 schrieb Hannes Reinecke:

quoted

On 7/19/21 12:19 PM, Stephan Mueller wrote:

quoted

Am Montag, dem 19.07.2021 um 11:57 +0200 schrieb Hannes Reinecke:

quoted

On 7/19/21 10:51 AM, Stephan Mueller wrote:

[ .. ]

quoted

quoted

quoted

quoted

quoted

Thank you for clarifying that. It sounds to me that there is no
defined protocol (or if there, I would be wondering how the code would have
worked
with a different implementation). Would it make sense to first specify
a protocol for authentication and have it discussed? I personally think
it is a bit difficult to fully understand the protocol from the code and
discuss protocol-level items based on the code.

Oh, the protocol _is_ specified:

 
https://nvmexpress.org/wp-content/uploads/NVM-Express-Base-Specification-2_0-2021.06.02-Ratified-5.pdf

It's just that I have issues translating that spec onto what the kernel
provides.

according to the naming conventions there in figures 447 and following:

- x and y: DH private key (kernel calls it secret set with dh_set_secret
or
encoded into param.key)

But that's were I got confused; one needs a private key here, but there
is no obvious candidate for it. But reading it more closely I guess the
private key is just a random number (cf the spec: g^y mod p, where y is
a random number selected by the host that shall be at least 256 bits
long). So I'll fix it up with the next round.

Here comes the crux: the kernel has an ECC private key generation function
ecdh_set_secret triggered with crypto_kpp_set_secret using a NULL key, but it
has no FFC-DH counterpart.

That said, generating a random number is the most obvious choice, but not the
right one.

The correct one would be following SP800-56A rev 3 and here either section
5.6.1.1.3 or 5.6.1.1.4.

Hmm. Okay. But after having read section 5.6.1.1.4, I still do have some
questions.

Assume we will be using a bit length of 512 for FFDHE, then we will
trivially pass Step 2 for all supported FFDHE groups (the maximum
symmetric-equivalent strength for ffdhe8192 is 192 bits).

N = 512 is not a good choice, minimum length these days for DH should
be 2048 or more.

According to RFC7919:
Peers using ffdhe8192 that want to optimize their key exchange with a
short exponent (Section 5.2) should choose a secret key of at least
400 bits.

So what is wrong with 512 bits?

RFC7519 is TLS Specific.
I do not know if short-exponents are safe to use in all use cases.

If it is safe, your choice is fine and your arguments will follow, but
then a comment that explains the choice and warns about key checks if
it is changed would be a good idea.

Otherwise the default should be to use N = len(q), which implies the
proper checks need to be applied.

Simo.

quoted

quoted

From my understanding, the random number generator will fill out all
available bytes in the string (and nothing more), so we trivially
satisfy step 3 and 4.

And as q is always larger than the random number, step 6 reduces to
'if (c > 2^N - 2)',

Where is this coming from ?
It seem you assume M = 2^N but M = min(2^N, q)

The point here is to make sure the number X you return is:
0 < X < (q-1)

Which is what I've tried to argue. For 512 bits private key and the
smallest possible FFDHE group (which has 2048 bits, with the top bit
non-zero) 2^N is always smaller than (q - 1).
As the other FFHDE groups are using even larger 'q' values, this is true
for all FFHDE groups.

quoted

quoted

 ie we just need to check if the random number is a
string of 0xff characters. Which hardly is a random number at all, so
it'll be impossible to get this.

Which then would mean that our 'x' is simply the random number + 1,

This is an artifact due to the random number being 0 <= c < 2^N - 1,
therefore 1 needs to be added to make sure you never return 0.

And my argument here is that all zeros (and all ones) are not a value I
would expect from our RNG.

quoted

quoted

which arguably is slightly pointless (one more than a random number is
as random as the number itself), so I do feel justified with just
returning a random number here.

Am I wrong with that reasoning?

Looks to me you are not accounting for the fact that N = 512 is too
small and a random number falling in the interval (q - 2) < X < 2^N is
unsuitable?

Only if (q - 2) < 2^N. And my point is that it's not.

Cheers,

Hannes

-- 
Simo Sorce
RHEL Crypto Team
Red Hat, Inc