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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.