From: <rsbecker@nexbridge.com>
To: "'Patrick Steinhardt'" <ps@pks.im>, <git@vger.kernel.org>
Subject: RE: [PATCH 2/2] reftable/stack: accept insecure random bytes
Date: Tue, 7 Jan 2025 18:56:38 -0500 [thread overview]
Message-ID: <00ad01db615f$ce9b6740$6bd235c0$@nexbridge.com> (raw)
In-Reply-To: <20250107-b4-pks-reftable-csprng-v1-2-6109a54a8756@pks.im>
>-----Original Message-----
>From: Patrick Steinhardt <ps@pks.im>
>Sent: January 7, 2025 10:27 AM
>To: git@vger.kernel.org
>Cc: Randall S. Becker <randall.becker@nexbridge.ca>
>Subject: [PATCH 2/2] reftable/stack: accept insecure random bytes
>
>The reftable library uses randomness in two call paths:
>
> - When reading a stack in case some of the referenced tables
> disappears. The randomness is used to delay the next read by a
> couple of milliseconds.
>
> - When writing a new table, where the randomness gets appended to the
> table name (e.g. "0x000000000001-0x000000000002-0b1d8ddf.ref").
>
>In neither of these cases do we need strong randomness.
>
>Unfortunately though, we have observed test failures caused by the former case. In
>t0610 we have a test that spawns a 100 processes at once, all of which try to write
>a new table to the stack. And given that all of the processes will require
>randomness, it can happen that these processes make the entropy pool run dry,
>which will then cause us to
>die:
>
> + test_seq 100
> + printf %s commit\trefs/heads/branch-%s\n
> 68d032e9edd3481ac96382786ececc37ec28709e 1
> + printf %s commit\trefs/heads/branch-%s\n
> 68d032e9edd3481ac96382786ececc37ec28709e 2
> ...
> + git update-ref refs/heads/branch-98 HEAD
> + git update-ref refs/heads/branch-97 HEAD
> + git update-ref refs/heads/branch-99 HEAD
> + git update-ref refs/heads/branch-100 HEAD
> fatal: unable to get random bytes
> fatal: unable to get random bytes
> fatal: unable to get random bytes
> fatal: unable to get random bytes
> fatal: unable to get random bytes
> fatal: unable to get random bytes
> fatal: unable to get random bytes
>
>The report was for NonStop, which uses OpenSSL as the backend for randomness.
>In the preceding commit we have adapted that backend to also return randomness
>in case the entropy pool is empty and the caller passes the
>`CSPRNG_BYTES_INSECURE` flag. Do so to fix the issue.
>
>Reported-by: Randall S. Becker <rsbecker@nexbridge.com>
>Signed-off-by: Patrick Steinhardt <ps@pks.im>
>---
> reftable/stack.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
>diff --git a/reftable/stack.c b/reftable/stack.c index
>6d0aa774e7e29d5366ed55df19725944f8eef792..572a74e00f9ed6040534e060
>652e72c26641749d 100644
>--- a/reftable/stack.c
>+++ b/reftable/stack.c
>@@ -493,7 +493,7 @@ static int reftable_stack_reload_maybe_reuse(struct
>reftable_stack *st,
> close(fd);
> fd = -1;
>
>- delay = delay + (delay * rand()) / RAND_MAX + 1;
>+ delay = delay + (delay * git_rand(CSPRNG_BYTES_INSECURE)) /
>+UINT32_MAX + 1;
> sleep_millisec(delay);
> }
>
>@@ -659,7 +659,7 @@ int reftable_stack_add(struct reftable_stack *st, static int
>format_name(struct reftable_buf *dest, uint64_t min, uint64_t max) {
> char buf[100];
>- uint32_t rnd = (uint32_t)git_rand(0);
>+ uint32_t rnd = git_rand(CSPRNG_BYTES_INSECURE);
> snprintf(buf, sizeof(buf), "0x%012" PRIx64 "-0x%012" PRIx64 "-%08x",
> min, max, rnd);
> reftable_buf_reset(dest);
>
>--
>2.48.0.rc1.245.gb3e6e7acbc.dirty
The tests that are all impacted on ia64 (using PRNGD) are:
t0610-reftable-basics
t1092-sparse-checkout-compatibility
t1300-config
t1404-update-ref-errors
t1410-reflog
t1414-reflog-walk
t1415-worktree-refs
t1450-fsck
t1460-refs-migrate
t1500-rev-parse
t2006-checkout-index-basic
t2009-checkout-statinfo
t2013-checkout-submodule
t2017-checkout-orphan
t2030-unresolve-info
t2400-worktree-add
Everything passes on x86.
next prev parent reply other threads:[~2025-01-07 23:56 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 [this message]
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
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='00ad01db615f$ce9b6740$6bd235c0$@nexbridge.com' \
--to=rsbecker@nexbridge.com \
--cc=git@vger.kernel.org \
--cc=ps@pks.im \
/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).