Skip to content

Commit cb441e1

Browse files
committed
Merge branch 'ps/reftable-get-random-fix'
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
2 parents 57ebdd5 + 0b4f8af commit cb441e1

File tree

6 files changed

+33
-21
lines changed

6 files changed

+33
-21
lines changed

builtin/gc.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1909,7 +1909,7 @@ static int get_random_minute(void)
19091909
if (getenv("GIT_TEST_MAINT_SCHEDULER"))
19101910
return 13;
19111911

1912-
return git_rand() % 60;
1912+
return git_rand(0) % 60;
19131913
}
19141914

19151915
static int is_launchctl_available(void)

reftable/stack.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -493,7 +493,7 @@ static int reftable_stack_reload_maybe_reuse(struct reftable_stack *st,
493493
close(fd);
494494
fd = -1;
495495

496-
delay = delay + (delay * rand()) / RAND_MAX + 1;
496+
delay = delay + (delay * git_rand(CSPRNG_BYTES_INSECURE)) / UINT32_MAX + 1;
497497
sleep_millisec(delay);
498498
}
499499

@@ -659,7 +659,7 @@ int reftable_stack_add(struct reftable_stack *st,
659659
static int format_name(struct reftable_buf *dest, uint64_t min, uint64_t max)
660660
{
661661
char buf[100];
662-
uint32_t rnd = (uint32_t)git_rand();
662+
uint32_t rnd = git_rand(CSPRNG_BYTES_INSECURE);
663663
snprintf(buf, sizeof(buf), "0x%012" PRIx64 "-0x%012" PRIx64 "-%08x",
664664
min, max, rnd);
665665
reftable_buf_reset(dest);

t/helper/test-csprng.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ int cmd__csprng(int argc, const char **argv)
1515

1616
while (count) {
1717
unsigned long chunk = count < sizeof(buf) ? count : sizeof(buf);
18-
if (csprng_bytes(buf, chunk) < 0) {
18+
if (csprng_bytes(buf, chunk, 0) < 0) {
1919
perror("failed to read");
2020
return 5;
2121
}

t/unit-tests/t-reftable-readwrite.c

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -108,8 +108,8 @@ static void t_log_buffer_size(void)
108108
hash, to ensure that the compressed part is larger than the original.
109109
*/
110110
for (i = 0; i < REFTABLE_HASH_SIZE_SHA1; i++) {
111-
log.value.update.old_hash[i] = (uint8_t)(git_rand() % 256);
112-
log.value.update.new_hash[i] = (uint8_t)(git_rand() % 256);
111+
log.value.update.old_hash[i] = (uint8_t)(git_rand(0) % 256);
112+
log.value.update.new_hash[i] = (uint8_t)(git_rand(0) % 256);
113113
}
114114
reftable_writer_set_limits(w, update_index, update_index);
115115
err = reftable_writer_add_log(w, &log);
@@ -325,7 +325,7 @@ static void t_log_zlib_corruption(void)
325325
};
326326

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

330330
reftable_writer_set_limits(w, 1, 1);
331331

wrapper.c

Lines changed: 14 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -479,7 +479,7 @@ int git_mkstemps_mode(char *pattern, int suffix_len, int mode)
479479
for (count = 0; count < TMP_MAX; ++count) {
480480
int i;
481481
uint64_t v;
482-
if (csprng_bytes(&v, sizeof(v)) < 0)
482+
if (csprng_bytes(&v, sizeof(v), 0) < 0)
483483
return error_errno("unable to get random bytes for temporary file");
484484

485485
/* Fill in the random bits. */
@@ -750,7 +750,7 @@ int open_nofollow(const char *path, int flags)
750750
#endif
751751
}
752752

753-
int csprng_bytes(void *buf, size_t len)
753+
int csprng_bytes(void *buf, size_t len, MAYBE_UNUSED unsigned flags)
754754
{
755755
#if defined(HAVE_ARC4RANDOM) || defined(HAVE_ARC4RANDOM_LIBBSD)
756756
/* This function never returns an error. */
@@ -785,14 +785,18 @@ int csprng_bytes(void *buf, size_t len)
785785
return -1;
786786
return 0;
787787
#elif defined(HAVE_OPENSSL_CSPRNG)
788-
int res = RAND_bytes(buf, len);
789-
if (res == 1)
788+
switch (RAND_pseudo_bytes(buf, len)) {
789+
case 1:
790790
return 0;
791-
if (res == -1)
792-
errno = ENOTSUP;
793-
else
791+
case 0:
792+
if (flags & CSPRNG_BYTES_INSECURE)
793+
return 0;
794794
errno = EIO;
795-
return -1;
795+
return -1;
796+
default:
797+
errno = ENOTSUP;
798+
return -1;
799+
}
796800
#else
797801
ssize_t res;
798802
char *p = buf;
@@ -816,11 +820,11 @@ int csprng_bytes(void *buf, size_t len)
816820
#endif
817821
}
818822

819-
uint32_t git_rand(void)
823+
uint32_t git_rand(unsigned flags)
820824
{
821825
uint32_t result;
822826

823-
if (csprng_bytes(&result, sizeof(result)) < 0)
827+
if (csprng_bytes(&result, sizeof(result), flags) < 0)
824828
die(_("unable to get random bytes"));
825829

826830
return result;

wrapper.h

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -127,18 +127,26 @@ int open_nofollow(const char *path, int flags);
127127

128128
void sleep_millisec(int millisec);
129129

130+
enum {
131+
/*
132+
* Accept insecure bytes, which some CSPRNG implementations may return
133+
* in case the entropy pool has been exhausted.
134+
*/
135+
CSPRNG_BYTES_INSECURE = (1 << 0),
136+
};
137+
130138
/*
131139
* Generate len bytes from the system cryptographically secure PRNG.
132140
* Returns 0 on success and -1 on error, setting errno. The inability to
133-
* satisfy the full request is an error.
141+
* satisfy the full request is an error. Accepts CSPRNG flags.
134142
*/
135-
int csprng_bytes(void *buf, size_t len);
143+
int csprng_bytes(void *buf, size_t len, unsigned flags);
136144

137145
/*
138146
* Returns a random uint32_t, uniformly distributed across all possible
139-
* values.
147+
* values. Accepts CSPRNG flags.
140148
*/
141-
uint32_t git_rand(void);
149+
uint32_t git_rand(unsigned flags);
142150

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

0 commit comments

Comments
 (0)