@@ -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)
@@ -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 = (uint32_t)git_rand(0);
snprintf(buf, sizeof(buf), "0x%012" PRIx64 "-0x%012" PRIx64 "-%08x",
min, max, rnd);
reftable_buf_reset(dest);
@@ -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;
}
@@ -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);
@@ -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);
@@ -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. */
@@ -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. */
@@ -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;
@@ -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;
@@ -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)
The `csprng_bytes()` function generates randomness and writes it into a caller-provided buffer. It abstracts over a couple of implementations, where the exact one that is used depends on the platform. These implementations have different guarantees: while some guarantee to never fail (arc4random(3)), others may fail. There are two significant failures to distinguish from one another: - Systemic failure, where e.g. opening "/dev/urandom" fails or when OpenSSL doesn't have a provider configured. - Entropy failure, where the entropy pool is exhausted, and thus the function cannot guarantee strong cryptographic randomness. While we cannot do anything about the former, the latter failure can be acceptable in some situations where we don't care whether or not the randomness can be predicted. Introduce a new `CSPRNG_BYTES_INSECURE` flag that allows callers to opt into weak cryptographic randomness. The exact behaviour of the flag depends on the underlying implementation: - `arc4random_buf()` never returns an error, so it doesn't change. - `getrandom()` pulls from "/dev/urandom" by default, which never blocks on modern systems even when the entropy pool is empty. - `getentropy()` seems to block when there is not enough randomness available, and there is no way of changing that behaviour. - `GtlGenRandom()` doesn't mention anything about its specific failure mode. - The fallback reads from "/dev/urandom", which also returns bytes in case the entropy pool is drained in modern Linux systems. That only leaves OpenSSL with `RAND_bytes()`, which returns an error in case the returned data wouldn't be cryptographically safe. This function is replaced with a call to `RAND_pseudo_bytes()`, which can indicate whether or not the returned data is cryptographically secure via its return value. If it is insecure, and if the `CSPRNG_BYTES_INSECURE` flag is set, then we ignore the insecurity and return the data regardless. It is somewhat questionable whether we really need the flag in the first place, or whether we wouldn't just ignore the potentially-insecure data. But the risk of doing that is that we might have or grow callsites that aren't aware of the potential insecureness of the data in places where it really matters. So using a flag to opt-in to that behaviour feels like the more secure choice. Signed-off-by: Patrick Steinhardt <ps@pks.im> --- builtin/gc.c | 2 +- reftable/stack.c | 2 +- t/helper/test-csprng.c | 2 +- t/unit-tests/t-reftable-readwrite.c | 6 +++--- wrapper.c | 24 ++++++++++++++---------- wrapper.h | 16 ++++++++++++---- 6 files changed, 32 insertions(+), 20 deletions(-)