Re: [PATCH 1/2] wrapper: add a helper to generate numbers from a CSPRNG

From: Junio C Hamano <gitster@pobox.com>
Date: Nov 18, 2021, 07:19 UTC
Message-ID: <xmqqv90qx76b.fsf@gitster.g>
In-Reply-To: <YZWJy16Emrkr6qVL@camp.crustytoothpaste.net>

"brian m. carlson" <sandals@crustytoothpaste.net> writes:

Show 19 quoted lines

> On 2021-11-17 at 07:39:08, Junio C Hamano wrote:
>> "brian m. carlson" <sandals@crustytoothpaste.net> writes:
>> 
>> > Finally, add a self-test option here to make sure that our buffer
>> > handling is correct and we aren't truncating data.  We simply read 64
>> > KiB and then make sure we've seen each byte.  The probability of this
>> > test failing spuriously is less than 10^-100.
>> 
>> I saw that 10^-100 math in the other message, and have no problem
>> with that, but I am not sure how such a test makes "sure that our
>> buffer handling is correct and we aren't truncating data."  If you
>> thought you are generate 64kiB of random bytes but a bug caused you
>> to actually use 32kiB of random bytes with 32kiB of other garbage,
>> wouldn't you still have enough entropy left that you would be likely
>> to paint all 256 buckets?
>
> True, but our code processes smaller chunks at a time, which means that
> theoretically we'd notice before then.  For example, getentropy(2) won't
> process chunks larger than 256 bytes.

Sorry, you lost me.

> If we don't think there's value, I can just remove it.

It is not that I do not think there is value. I am not sure where this code is getting its value from.

We grab 1k at a time and repeat that 64 times.

Presumably csprn_bytes() grabs bytes from underlying mechanism in smaller chunk, but would not return until it fills the buffer---ah, your "make sure our buffer handling is correct" is primarily about the check that we get full 1k bytes in the loop? We ask 1k chunk 64 times and we must get full 1k chunk every time?

What I was wondering about was the other half of the check, ensuring all buckets[] are painted that gave us the cute 10^-100 math.

+ int buckets[256] = { 0 }; + unsigned char buf[1024]; + unsigned long count = 64 * 1024; + int i; + + while (count) { + if (csprng_bytes(buf, sizeof(buf)) < 0) { + perror("failed to read"); + return 3; + } + for (i = 0; i < sizeof(buf); i++) + buckets[buf[i]]++; + count -= sizeof(buf); + }