Re: [PATCH 09/18] crypto: dh - implement private key generation primitive

[Stephan Mueller <smueller@chronox.de>]

Am Mittwoch, 8. Dezember 2021, 07:20:43 CET schrieb Nicolai Stange:

Hi Nicolai,

> >> +		return -EINVAL;
> >> +
> >> +	/*
> >> +	 * 5.6.1.1.1: choose key length N such that
> >> +	 * 2 * ->max_strength <= N <= log2(q) + 1 = ->p_size * 8 - 1
> >> +	 * with q = (p - 1) / 2 for the safe-prime groups.
> >> +	 * Choose the lower bound's next power of two for N in order to
> >> +	 * avoid excessively large private keys while still
> >> +	 * maintaining some extra reserve beyond the bare minimum in
> >> +	 * most cases. Note that for each entry in safe_prime_groups[],
> >> +	 * the following holds for such N:
> >> +	 * - N >= 256, in particular it is a multiple of 2^6 = 64
> >> +	 *   bits and
> >> +	 * - N < log2(q) + 1, i.e. N respects the upper bound.
> >> +	 */
> >> +	n = roundup_pow_of_two(2 * g->max_strength);
> >> +	WARN_ON_ONCE(n & ((1u << 6) - 1));
> >> +	n >>= 6; /* Convert N into units of u64. */
> > 
> > Couldn't we pre-compute that value for each of the safeprime groups? This
> > value should be static for each of them.
> 
> Can you elaborate why this would be better? As long as the value
> calculated above is considered reasonable for every usecase, I don't see
> the advantage of storing it somewhere.

Well, I usually try to avoid using CPU resources if I have information a-
priori. And as we have only known domain parameters in this code path, I 
thought we can spare a few CPU cycles.

> 
> OTOH, calculating the value on the fly
> - enforces conformance to 5.6.1.1.1 (>= twice the sec strength)
> - and guarantees that it is a multiple of 64 bits, as required
>   by the implementation,
> whereas you'd had to examine each and every individual group's setting
> for correctness when storing precomputed values alongside the other,
> "primary" group parameters.

You are right, but when we reach this code path we only have well-known 
parameters. Hence my suggestion.

Ciao
Stephan