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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <4767831.y2tiDqZFiq@tauon.chronox.de>
Date:   Wed, 08 Dec 2021 08:16:18 +0100
From:   Stephan Mueller <smueller@...onox.de>
To:     Nicolai Stange <nstange@...e.de>
Cc:     Herbert Xu <herbert@...dor.apana.org.au>,
        "David S. Miller" <davem@...emloft.net>,
        Nicolai Stange <nstange@...e.de>,
        Hannes Reinecke <hare@...e.de>, Torsten Duwe <duwe@...e.de>,
        Zaibo Xu <xuzaibo@...wei.com>,
        Giovanni Cabiddu <giovanni.cabiddu@...el.com>,
        David Howells <dhowells@...hat.com>,
        Jarkko Sakkinen <jarkko@...nel.org>,
        linux-crypto@...r.kernel.org, linux-kernel@...r.kernel.org,
        qat-linux@...el.com, keyrings@...r.kernel.org
Subject: Re: [PATCH 09/18] crypto: dh - implement private key generation primitive

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