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>

diff --git a/builtin/gc.c b/builtin/gc.c
index a9b1c36de27da2ffb16aaa45e979cbaeec18bfa1..3e754f25bba2aa16fbae9b2b6229961a53409395 100644 (file)
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -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)

diff --git a/reftable/stack.c b/reftable/stack.c
index 531660a49f0948c33041831ee0d740feacb22b2f..6d0aa774e7e29d5366ed55df19725944f8eef792 100644 (file)
--- a/reftable/stack.c
+++ b/reftable/stack.c
@@ -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);

diff --git a/t/helper/test-csprng.c b/t/helper/test-csprng.c
index a4a0aca61773b0b30de618955e5a5b61bba2d0cc..c86dcc4870fed8ab81aa7c6c17d77c0b32101cf8 100644 (file)
--- a/t/helper/test-csprng.c
+++ b/t/helper/test-csprng.c
@@ -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;
                }

diff --git a/t/unit-tests/t-reftable-readwrite.c b/t/unit-tests/t-reftable-readwrite.c
index 6b75a419b9d3bbfb720a3c39f741f940dfcd56aa..f22b9775639e3ec9167336100753d5736e5a2957 100644 (file)
--- a/t/unit-tests/t-reftable-readwrite.c
+++ b/t/unit-tests/t-reftable-readwrite.c
@@ -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);
 

diff --git a/wrapper.c b/wrapper.c
index fa79fd6ec9e144509f34e007bb2ffa458d69e695..8b985931490d62acb2030c147b8728b34917df8a 100644 (file)
--- a/wrapper.c
+++ b/wrapper.c
@@ -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;

diff --git a/wrapper.h b/wrapper.h
index a6b3e1f09ecdef72fdcc4a234df7a176c4daded6..7df824e34a906e9f822ad3bee27b8b1b0000621b 100644 (file)
--- a/wrapper.h
+++ b/wrapper.h
@@ -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)