@matheusbsbs / matheus-bagliano / commits / 1568d1562e

wrapper: allow generating insecure random bytes

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> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Patrick Steinhardt committed Jan 7, 2025 at 16:26 UTC 1568d1562eecc31d2062b6d22e37ec03fc3d6747

6 files changed +32 -20

builtin/gc.c

+1 -1

@@ -1909,7 +1909,7 @@ static int get_random_minute(void)

1909 if (getenv("GIT_TEST_MAINT_SCHEDULER"))

1910 return 13;

1911

1912 - return git_rand() % 60;

1912 + return git_rand(0) % 60;

1913 }

1914

1915 static int is_launchctl_available(void)

reftable/stack.c

+1 -1

@@ -659,7 +659,7 @@ int reftable_stack_add(struct reftable_stack *st,

659 static int format_name(struct reftable_buf *dest, uint64_t min, uint64_t max)

660 {

661 char buf[100];

662 - uint32_t rnd = (uint32_t)git_rand();

662 + uint32_t rnd = (uint32_t)git_rand(0);

663 snprintf(buf, sizeof(buf), "0x%012" PRIx64 "-0x%012" PRIx64 "-%08x",

664 min, max, rnd);

665 reftable_buf_reset(dest);

t/helper/test-csprng.c

+1 -1

@@ -15,7 +15,7 @@ int cmd__csprng(int argc, const char **argv)

16 while (count) {

17 unsigned long chunk = count < sizeof(buf) ? count : sizeof(buf);

18 - if (csprng_bytes(buf, chunk) < 0) {

18 + if (csprng_bytes(buf, chunk, 0) < 0) {

19 perror("failed to read");

20 return 5;

21 }

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

+3 -3

@@ -108,8 +108,8 @@ static void t_log_buffer_size(void)

108 hash, to ensure that the compressed part is larger than the original.

109 */

110 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);

113 }

114 reftable_writer_set_limits(w, update_index, update_index);

115 err = reftable_writer_add_log(w, &log);

@@ -325,7 +325,7 @@ static void t_log_zlib_corruption(void)

325 };

326

327 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 + ' ');

329

330 reftable_writer_set_limits(w, 1, 1);

331

wrapper.c

+14 -10

@@ -479,7 +479,7 @@ int git_mkstemps_mode(char *pattern, int suffix_len, int mode)

479 for (count = 0; count < TMP_MAX; ++count) {

480 int i;

481 uint64_t v;

482 - if (csprng_bytes(&v, sizeof(v)) < 0)

482 + if (csprng_bytes(&v, sizeof(v), 0) < 0)

483 return error_errno("unable to get random bytes for temporary file");

484

485 /* Fill in the random bits. */

@@ -750,7 +750,7 @@ int open_nofollow(const char *path, int flags)

750 #endif

751 }

752

753 -int csprng_bytes(void *buf, size_t len)

753 +int csprng_bytes(void *buf, size_t len, MAYBE_UNUSED unsigned flags)

754 {

755 #if defined(HAVE_ARC4RANDOM) || defined(HAVE_ARC4RANDOM_LIBBSD)

756 /* This function never returns an error. */

@@ -785,14 +785,18 @@ int csprng_bytes(void *buf, size_t len)

785 return -1;

786 return 0;

787 #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:

790 return 0;

791 - if (res == -1)

792 - errno = ENOTSUP;

793 - else

791 + case 0:

792 + if (flags & CSPRNG_BYTES_INSECURE)

793 + return 0;

794 errno = EIO;

795 - return -1;

795 + return -1;

796 + default:

797 + errno = ENOTSUP;

798 + return -1;

799 + }

800 #else

801 ssize_t res;

802 char *p = buf;

@@ -816,11 +820,11 @@ int csprng_bytes(void *buf, size_t len)

820 #endif

821 }

822

819 -uint32_t git_rand(void)

823 +uint32_t git_rand(unsigned flags)

824 {

825 uint32_t result;

826

823 - if (csprng_bytes(&result, sizeof(result)) < 0)

827 + if (csprng_bytes(&result, sizeof(result), flags) < 0)

828 die(_("unable to get random bytes"));

829

830 return result;

wrapper.h

+12 -4

@@ -127,18 +127,26 @@ int open_nofollow(const char *path, int flags);

127

128 void sleep_millisec(int millisec);

129

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 +

138 /*

139 * Generate len bytes from the system cryptographically secure PRNG.

140 * 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.

142 */

135 -int csprng_bytes(void *buf, size_t len);

143 +int csprng_bytes(void *buf, size_t len, unsigned flags);

144

145 /*

146 * Returns a random uint32_t, uniformly distributed across all possible

139 - * values.

147 + * values. Accepts CSPRNG flags.

148 */

141 -uint32_t git_rand(void);

149 +uint32_t git_rand(unsigned flags);

150

151 /* Provide log2 of the given `size_t`. */

152 static inline unsigned log2u(uintmax_t sz)