Skip to content

Commit

Permalink
Merge branch 'ps/reftable-get-random-fix' into next
Browse files Browse the repository at this point in the history
The code to compute "unique" name used git_rand() which can fail or
get stuck; the callsite does not require cryptographic security.
Introduce the "insecure" mode and use it appropriately.

* ps/reftable-get-random-fix:
  reftable/stack: accept insecure random bytes
  wrapper: allow generating insecure random bytes
  • Loading branch information
gitster committed Jan 9, 2025
2 parents 1d584ee + 0b4f8af commit bc024b7
Show file tree
Hide file tree
Showing 6 changed files with 33 additions and 21 deletions.
2 changes: 1 addition & 1 deletion builtin/gc.c
Original file line number Diff line number Diff line change
Expand Up @@ -1909,7 +1909,7 @@ static int get_random_minute(void)
if (getenv("GIT_TEST_MAINT_SCHEDULER"))
return 13;

return git_rand() % 60;
return git_rand(0) % 60;
}

static int is_launchctl_available(void)
Expand Down
4 changes: 2 additions & 2 deletions reftable/stack.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand Down Expand Up @@ -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();
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);
Expand Down
2 changes: 1 addition & 1 deletion t/helper/test-csprng.c
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ int cmd__csprng(int argc, const char **argv)

while (count) {
unsigned long chunk = count < sizeof(buf) ? count : sizeof(buf);
if (csprng_bytes(buf, chunk) < 0) {
if (csprng_bytes(buf, chunk, 0) < 0) {
perror("failed to read");
return 5;
}
Expand Down
6 changes: 3 additions & 3 deletions t/unit-tests/t-reftable-readwrite.c
Original file line number Diff line number Diff line change
Expand Up @@ -108,8 +108,8 @@ static void t_log_buffer_size(void)
hash, to ensure that the compressed part is larger than the original.
*/
for (i = 0; i < REFTABLE_HASH_SIZE_SHA1; i++) {
log.value.update.old_hash[i] = (uint8_t)(git_rand() % 256);
log.value.update.new_hash[i] = (uint8_t)(git_rand() % 256);
log.value.update.old_hash[i] = (uint8_t)(git_rand(0) % 256);
log.value.update.new_hash[i] = (uint8_t)(git_rand(0) % 256);
}
reftable_writer_set_limits(w, update_index, update_index);
err = reftable_writer_add_log(w, &log);
Expand Down Expand Up @@ -325,7 +325,7 @@ static void t_log_zlib_corruption(void)
};

for (i = 0; i < sizeof(message) - 1; i++)
message[i] = (uint8_t)(git_rand() % 64 + ' ');
message[i] = (uint8_t)(git_rand(0) % 64 + ' ');

reftable_writer_set_limits(w, 1, 1);

Expand Down
24 changes: 14 additions & 10 deletions wrapper.c
Original file line number Diff line number Diff line change
Expand Up @@ -479,7 +479,7 @@ int git_mkstemps_mode(char *pattern, int suffix_len, int mode)
for (count = 0; count < TMP_MAX; ++count) {
int i;
uint64_t v;
if (csprng_bytes(&v, sizeof(v)) < 0)
if (csprng_bytes(&v, sizeof(v), 0) < 0)
return error_errno("unable to get random bytes for temporary file");

/* Fill in the random bits. */
Expand Down Expand Up @@ -750,7 +750,7 @@ int open_nofollow(const char *path, int flags)
#endif
}

int csprng_bytes(void *buf, size_t len)
int csprng_bytes(void *buf, size_t len, MAYBE_UNUSED unsigned flags)
{
#if defined(HAVE_ARC4RANDOM) || defined(HAVE_ARC4RANDOM_LIBBSD)
/* This function never returns an error. */
Expand Down Expand Up @@ -785,14 +785,18 @@ int csprng_bytes(void *buf, size_t len)
return -1;
return 0;
#elif defined(HAVE_OPENSSL_CSPRNG)
int res = RAND_bytes(buf, len);
if (res == 1)
switch (RAND_pseudo_bytes(buf, len)) {
case 1:
return 0;
if (res == -1)
errno = ENOTSUP;
else
case 0:
if (flags & CSPRNG_BYTES_INSECURE)
return 0;
errno = EIO;
return -1;
return -1;
default:
errno = ENOTSUP;
return -1;
}
#else
ssize_t res;
char *p = buf;
Expand All @@ -816,11 +820,11 @@ int csprng_bytes(void *buf, size_t len)
#endif
}

uint32_t git_rand(void)
uint32_t git_rand(unsigned flags)
{
uint32_t result;

if (csprng_bytes(&result, sizeof(result)) < 0)
if (csprng_bytes(&result, sizeof(result), flags) < 0)
die(_("unable to get random bytes"));

return result;
Expand Down
16 changes: 12 additions & 4 deletions wrapper.h
Original file line number Diff line number Diff line change
Expand Up @@ -127,18 +127,26 @@ int open_nofollow(const char *path, int flags);

void sleep_millisec(int millisec);

enum {
/*
* Accept insecure bytes, which some CSPRNG implementations may return
* in case the entropy pool has been exhausted.
*/
CSPRNG_BYTES_INSECURE = (1 << 0),
};

/*
* Generate len bytes from the system cryptographically secure PRNG.
* Returns 0 on success and -1 on error, setting errno. The inability to
* satisfy the full request is an error.
* satisfy the full request is an error. Accepts CSPRNG flags.
*/
int csprng_bytes(void *buf, size_t len);
int csprng_bytes(void *buf, size_t len, unsigned flags);

/*
* Returns a random uint32_t, uniformly distributed across all possible
* values.
* values. Accepts CSPRNG flags.
*/
uint32_t git_rand(void);
uint32_t git_rand(unsigned flags);

/* Provide log2 of the given `size_t`. */
static inline unsigned log2u(uintmax_t sz)
Expand Down

0 comments on commit bc024b7

Please sign in to comment.