git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: <rsbecker@nexbridge.com>
To: "'brian m. carlson'" <sandals@crustytoothpaste.net>,
	"'Patrick Steinhardt'" <ps@pks.im>
Cc: <git@vger.kernel.org>,
	"'Randall S. Becker'" <randall.becker@nexbridge.ca>
Subject: RE: [PATCH 0/2] reftable/stack: stop dying on exhausted entropy pool
Date: Tue, 7 Jan 2025 18:54:02 -0500	[thread overview]
Message-ID: <00ac01db615f$70b72600$52257200$@nexbridge.com> (raw)
In-Reply-To: <Z323CLrRsnOko1gB@tapette.crustytoothpaste.net>

On January 7, 2025 6:22 PM, brian m. carlson wrote:
>On 2025-01-07 at 15:26:58, Patrick Steinhardt wrote:
>> Hi,
>>
>> this small patch series fixes the issue reported by Randall [1], where
>> an exhausted entropy pool can cause us to die when writing a new table
>> to the reftable stack. I _think_ that this is only an issue with the
>> OpenSSL backend of `csprng_bytes()`:
>>
>>   - `arc4random_buf()` never returns an error.
>>
>>   - `getrandom()` pulls from "/dev/urandom" by default.
>>
>>   - `getentropy()` seems to block when there is not enough randomness
>>     available.
>>
>>   - `GtlGenRandom()` I cannot really tell.
>>
>>   - The fallback reads from "/dev/urandom", which also returns bytes in
>>     case the entropy pool is drained.
>>
>> So OpenSSL's `RAND_bytes()` seems to be the only one that returns an
>> error when the entropy pool is empty. I did wonder whether we even
>> need to introduce the new flag in the first place, or whether we
>> cannot just use `RAND_pseudo_bytes()` unconditionally. But I'm a bit
>> uneasy about it given that OpenSSL has this doc:
>>
>>     RAND_pseudo_bytes() puts num pseudo-random bytes into buf.
>>     Pseudo-random byte sequences generated by RAND_pseudo_bytes() will
>>     be unique if they are of sufficient length, but are not necessarily
>>     unpredictable. They can be used for non-cryptographic purposes and
>>     for certain purposes in cryptographic protocols, but usually not for
>>     key generation etc.
>>
>> It might be too easy to accidentally rely on `csprng_bytes()` where it
>> actually requires strong cryptographic data, so I was erring on the
>> side of caution.
>
>The reason I didn't use RAND_pseudo_bytes is because it's been deprecated since
>OpenSSL 1.1.0 and RAND_bytes uses a CSPRNG just like RAND_pseudo_bytes as of
>that version.  Once it's seeded, it should be able to generate plenty of bytes,
>because I believe it uses a CTR-DRBG, which only needs to be reseeded after 2^48
>bytes (which is far more than we should be using).
>
>We can full well use RAND_pseudo_bytes, but all operating systems should provide
>an appropriate entropy source that can provide 256 bits of entropy on startup.
>arc4random will just kill the process if it can't seed itself, so your changes won't
>actually prevent dying on a lack of entropy.
>
>I don't want an option that chooses "insecure" bytes.  My preference is that we
>require people use a different backend or an up-to-date OpenSSL version that
>shouldn't have this problem.  We can use RAND_pseudo_bytes if we really need to
>support older versions, but there are also no major operating systems which
>require that old of a version (CentOS 7, which is dead, used OpenSSL 1.0.2, and
>CentOS 8 uses 1.1.1k), so it's probably not within our support policy to do that.
>
>Note also that if OpenSSL is being used for TLS, a lack of entropy will result in TLS
>not working, which means that Git will be randomly broken on that system, which is
>not really an experience that we want to encourage, so that should be taken into
>account.
>
>Can we get some more information about what version of OpenSSL is being used
>and what the system entropy source is?

In my situation, OpenSSL 3.0.11 on ia64. 3.4.1 and 3.0.13 on x86 (x86 works fine
Because OpenSSL uses hardware. On ia64, we end up on PRNGD, which does fail.


  reply	other threads:[~2025-01-07 23:54 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-01-07 15:26 [PATCH 0/2] reftable/stack: stop dying on exhausted entropy pool Patrick Steinhardt
2025-01-07 15:26 ` [PATCH 1/2] wrapper: allow generating insecure random bytes Patrick Steinhardt
2025-01-07 15:27 ` [PATCH 2/2] reftable/stack: accept " Patrick Steinhardt
2025-01-07 15:37   ` rsbecker
2025-01-07 20:56   ` Junio C Hamano
2025-01-07 21:03     ` rsbecker
2025-01-07 21:09       ` Junio C Hamano
2025-01-07 21:03     ` Junio C Hamano
2025-01-08  6:51       ` Patrick Steinhardt
2025-01-08 15:39         ` Junio C Hamano
2025-01-08 16:21           ` Patrick Steinhardt
2025-01-08 17:40             ` Junio C Hamano
2025-01-08 18:16               ` Patrick Steinhardt
2025-01-07 23:56   ` rsbecker
2025-01-07 23:21 ` [PATCH 0/2] reftable/stack: stop dying on exhausted entropy pool brian m. carlson
2025-01-07 23:54   ` rsbecker [this message]
2025-01-08  7:18     ` Patrick Steinhardt
2025-01-08 13:50       ` rsbecker
2025-01-08 22:44       ` brian m. carlson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='00ac01db615f$70b72600$52257200$@nexbridge.com' \
    --to=rsbecker@nexbridge.com \
    --cc=git@vger.kernel.org \
    --cc=ps@pks.im \
    --cc=randall.becker@nexbridge.ca \
    --cc=sandals@crustytoothpaste.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).