On Tue, 2021-07-20 at 13:31 +0200, Hannes Reinecke wrote: > On 7/20/21 12:49 PM, Simo Sorce wrote: > > On Tue, 2021-07-20 at 12:14 +0200, Hannes Reinecke wrote: > > > On 7/19/21 1:52 PM, Stephan Mueller wrote: > > > > Am Montag, dem 19.07.2021 um 13:10 +0200 schrieb Hannes Reinecke: > > > > > On 7/19/21 12:19 PM, Stephan Mueller wrote: > > > > > > Am Montag, dem 19.07.2021 um 11:57 +0200 schrieb Hannes Reinecke: > > > > > > > On 7/19/21 10:51 AM, Stephan Mueller wrote: > > > [ .. ] > > > > > > > > > > > > > > > > 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. > > > 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. > > > > 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. > > > > 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
