Received-SPF: pass (google.com: domain of linux-crypto-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
From:   Stephan =?ISO-8859-1?Q?M=FCller?= <smueller@chronox.de>
To:     Herbert Xu <herbert@gondor.apana.org.au>,
        "David S. Miller" <davem@davemloft.net>,
        Nicolai Stange <nstange@suse.de>
Cc:     Torsten Duwe <duwe@suse.de>, linux-crypto@vger.kernel.org,
        linux-kernel@vger.kernel.org, Nicolai Stange <nstange@suse.de>
Subject: Re: [PATCH 0/6] crypto: DRBG - improve 'nopr' reseeding
Date:   Tue, 26 Oct 2021 10:33:05 +0200
Message-ID: <2120606.3HGXcN3vsr@positron.chronox.de>
In-Reply-To: <20211025092525.12805-1-nstange@suse.de>
References: <20211025092525.12805-1-nstange@suse.de>
MIME-Version: 1.0
Content-Transfer-Encoding: 7Bit
Content-Type: text/plain; charset="us-ascii"
Precedence: bulk

Am Montag, 25. Oktober 2021, 11:25:19 CEST schrieb Nicolai Stange:

Hi Nicolai,

> Hi all,
> 
> this patchset aims at (hopefully) improving the DRBG code related to
> reseeding from get_random_bytes() a bit:

Thanks for sharing your patches.

> - Replace the asynchronous random_ready_callback based DRBG reseeding
>   logic with a synchronous solution leveraging rng_is_initialized().

Could you please help me why replacing an async method with a sync method is 
helpful? Which problems do you see with the async method that are alleviated 
with the swtich to the sync method? In general, an async method is more 
powerful, though it requires a bit more code.

>   This
>   move simplifies the code IMO and, as a side-effect, would enable DRBG
>   users to rely on wait_for_random_bytes() to sync properly with
>   drbg_generate(), if desired. Implemented by patches 1-5/6.
> - Make the 'nopr' DRBGs to reseed themselves every 5min from
>   get_random_bytes(). This achieves at least kind of a partial prediction
>   resistance over the time domain at almost no extra cost. Implemented
>   by patch 6/6, the preceding patches in this series are a prerequisite
>   for this.

Just as a side note not against your ideas and patches, but in general: IMHO 
it is a failure of all of us that the quite sensitive (re)seeding of RNGs and 
entropy management is handled in multiple places in the kernel - and each case 
only handles a subset of considerations around that topic. Note, (re)seeding 
may be needed in other occasions than the elapse of a timer or the reaching of 
maximum number of generate operations. Seeding belongs to a central place 
where it is done right once and usable for differnent RNGs as proposed with my 
LRNG patch set and the published todo list to get rid of the entire seeding 
logic in the DRBG code base.

That said, your patch of adding the timer-based reseeding seems appropriate 
and thus should be considered for the current code base.

Ciao
Stephan