Replace random_byte() with random_read().

This is in preparation for a PRNG revamp which will want to have a
well defined boundary for any given request-for-randomness, so that it
can destroy the evidence afterwards. So no more looping round calling
random_byte() and then stopping when we feel like it: now you say up
front how many random bytes you want, and call random_read() which
gives you that many in one go.

Most of the call sites that had to be fixed are fairly mechanical, and
quite a few ended up more concise afterwards. A few became more
cumbersome, such as mp_random_bits, in which the new API doesn't let
me load the random bytes directly into the target integer without
triggering undefined behaviour, so instead I have to allocate a
separate temporary buffer.

The _most_ interesting call site was in the PKCS#1 v1.5 padding code
in sshrsa.c (used in SSH-1), in which you need a stream of _nonzero_
random bytes. The previous code just looped on random_byte, retrying
if it got a zero. Now I'm doing a much more interesting thing with an
mpint, essentially scaling a binary fraction repeatedly to extract a
number in the range [0,255) and then adding 1 to it.

import.c

@@ -988,7 +988,7 @@ static bool openssh_pem_write(
      */
     if (passphrase) {
 	unsigned char keybuf[32];
-        int origlen, outlen, pad, i;
+        int origlen, outlen, pad;
 
         /*
          * Padding on OpenSSH keys is deterministic. The number of
@@ -1015,7 +1015,7 @@ static bool openssh_pem_write(
 	/*
 	 * Invent an iv, and derive the encryption key.
 	 */
-	for (i = 0; i < 8; i++) iv[i] = random_byte();
+        random_read(iv, 8);
 
         openssh_pem_derivekey(ptrlen_from_asciz(passphrase), iv, keybuf);
 
@@ -1498,7 +1498,7 @@ static bool openssh_new_write(
     const Filename *filename, ssh2_userkey *key, const char *passphrase)
 {
     strbuf *pubblob, *privblob, *cblob;
-    int padvalue, i;
+    int padvalue;
     unsigned checkint;
     bool ret = false;
     unsigned char bcrypt_salt[16];
@@ -1530,8 +1530,7 @@ static bool openssh_new_write(
     } else {
         strbuf *substr;
 
-        for (i = 0; i < (int)sizeof(bcrypt_salt); i++)
+        random_read(bcrypt_salt, sizeof(bcrypt_salt));
-            bcrypt_salt[i] = random_byte();
         put_stringz(cblob, "aes256-ctr");
         put_stringz(cblob, "bcrypt");
         substr = strbuf_new();
@@ -1551,9 +1550,9 @@ static bool openssh_new_write(
         strbuf *cpblob = strbuf_new();
 
         /* checkint. */
-        checkint = 0;
+        uint8_t checkint_buf[4];
-        for (i = 0; i < 4; i++)
+        random_read(checkint_buf, 4);
-            checkint = (checkint << 8) + random_byte();
+        checkint = GET_32BIT_MSB_FIRST(checkint_buf);
         put_uint32(cpblob, checkint);
         put_uint32(cpblob, checkint);
 
@@ -2279,8 +2278,9 @@ static bool sshcom_write(
     /* Pad encrypted blob to a multiple of cipher block size. */
     if (passphrase) {
 	int padding = -(outblob->len - (lenpos+4)) & 7;
-	while (padding--)
+        uint8_t padding_buf[8];
-	    put_byte(outblob, random_byte());
+        random_read(padding_buf, padding);
+        put_data(outblob, padding_buf, padding);
     }
     ciphertext = outblob->s + lenpos + 4;
     cipherlen = outblob->len - (lenpos + 4);

mpint.c

@@ -2302,21 +2302,20 @@ mp_int *monty_modsqrt(ModsqrtContext *sc, mp_int *x, unsigned *success)
     return toret;
 }
 
-mp_int *mp_random_bits_fn(size_t bits, int (*gen_byte)(void))
+mp_int *mp_random_bits_fn(size_t bits, random_read_fn_t random_read)
 {
     size_t bytes = (bits + 7) / 8;
-    size_t words = (bits + BIGNUM_INT_BITS - 1) / BIGNUM_INT_BITS;
+    uint8_t *randbuf = snewn(bytes, uint8_t);
-    mp_int *x = mp_make_sized(words);
+    random_read(randbuf, bytes);
-    for (size_t i = 0; i < bytes; i++) {
+    if (bytes)
-        BignumInt byte = gen_byte();
+        randbuf[0] &= (2 << ((bits-1) & 7)) - 1;
-        unsigned mask = (1 << size_t_min(8, bits-i*8)) - 1;
+    mp_int *toret = mp_from_bytes_be(make_ptrlen(randbuf, bytes));
-        x->w[i / BIGNUM_INT_BYTES] |=
+    smemclr(randbuf, bytes);
-            (byte & mask) << (8*(i % BIGNUM_INT_BYTES));
+    sfree(randbuf);
-    }
+    return toret;
-    return x;
 }
 
-mp_int *mp_random_in_range_fn(mp_int *lo, mp_int *hi, int (*gen_byte)(void))
+mp_int *mp_random_in_range_fn(mp_int *lo, mp_int *hi, random_read_fn_t rf)
 {
     mp_int *n_outcomes = mp_sub(hi, lo);
 
@@ -2329,8 +2328,7 @@ mp_int *mp_random_in_range_fn(mp_int *lo, mp_int *hi, int (*gen_byte)(void))
      * is acceptable on the grounds that you'd have to examine so many
      * outputs to even detect it.
      */
-    mp_int *unreduced = mp_random_bits_fn(
+    mp_int *unreduced = mp_random_bits_fn(mp_max_bits(n_outcomes) + 128, rf);
-        mp_max_bits(n_outcomes) + 128, gen_byte);
     mp_int *reduced = mp_mod(unreduced, n_outcomes);
     mp_add_into(reduced, reduced, lo);
     mp_free(unreduced);

mpint.h

@@ -364,9 +364,9 @@ mp_int *mp_rshift_fixed(mp_int *x, size_t shift);
 /*
  * Generate a random mp_int.
  *
- * The _function_ definitions here will expect to be given a gen_byte
+ * The _function_ definitions here will expect to be given a gen_data
  * function that provides random data. Normally you'd use this using
- * random_byte() from random.c, and the macro wrappers automate that.
+ * random_read() from random.c, and the macro wrappers automate that.
  *
  * (This is a bit of a dodge to avoid mpint.c having a link-time
  * dependency on random.c, so that programs can link against one but
@@ -376,10 +376,11 @@ mp_int *mp_rshift_fixed(mp_int *x, size_t shift);
  * mp_random_bits[_fn] returns an integer 0 <= n < 2^bits.
  * mp_random_in_range[_fn](lo,hi) returns an integer lo <= n < hi.
  */
-mp_int *mp_random_bits_fn(size_t bits, int (*gen_byte)(void));
+typedef void (*random_read_fn_t)(void *, size_t);
+mp_int *mp_random_bits_fn(size_t bits, random_read_fn_t randfn);
 mp_int *mp_random_in_range_fn(
-    mp_int *lo_inclusive, mp_int *hi_exclusive, int (*gen_byte)(void));
+    mp_int *lo_inclusive, mp_int *hi_exclusive, random_read_fn_t randfn);
-#define mp_random_bits(bits) mp_random_bits_fn(bits, random_byte)
+#define mp_random_bits(bits) mp_random_bits_fn(bits, random_read)
-#define mp_random_in_range(lo, hi) mp_random_in_range_fn(lo, hi, random_byte)
+#define mp_random_in_range(lo, hi) mp_random_in_range_fn(lo, hi, random_read)
 
 #endif /* PUTTY_MPINT_H */

pageant.c

@@ -21,11 +21,9 @@
  * won't generate true random numbers. So we must scream, panic,
  * and exit immediately if that should happen.
  */
-int random_byte(void)
+void random_read(void *buf, size_t size)
 {
     modalfatalbox("Internal error: attempt to use random numbers in Pageant");
-    exit(0);
-    return 0;                 /* unreachable, but placate optimiser */
 }
 
 static bool pageant_local = false;

putty.h

@@ -1674,7 +1674,7 @@ void luni_send(Ldisc *, const wchar_t * widebuf, int len, bool interactive);
  */
 
 void random_add_noise(void *noise, int length);
-int random_byte(void);
+void random_read(void *buf, size_t size);
 void random_get_savedata(void **data, int *len);
 extern int random_active;
 /* The random number subsystem is activated if at least one other entity

ssh.h

@@ -892,7 +892,7 @@ void SHATransform(uint32_t *digest, uint32_t *data);
 #   undef COMPILER_SUPPORTS_SHA_NI
 #endif
 
-int random_byte(void);
+void random_read(void *out, size_t size);
 void random_add_noise(void *noise, int length);
 void random_add_heavynoise(void *noise, int length);
 

ssh1bpp.c

@@ -295,7 +295,7 @@ static PktOut *ssh1_bpp_new_pktout(int pkt_type)
 
 static void ssh1_bpp_format_packet(struct ssh1_bpp_state *s, PktOut *pkt)
 {
-    int pad, biglen, i, pktoffs;
+    int pad, biglen, pktoffs;
     uint32_t crc;
     int len;
 
@@ -329,8 +329,7 @@ static void ssh1_bpp_format_packet(struct ssh1_bpp_state *s, PktOut *pkt)
     pktoffs = 8 - pad;
     biglen = len + pad;         /* len(padding+type+data+CRC) */
 
-    for (i = pktoffs; i < 4+8; i++)
+    random_read(pkt->data + pktoffs, 4+8 - pktoffs);
-        pkt->data[i] = random_byte();
     crc = crc32_ssh1(
         make_ptrlen(pkt->data + pktoffs + 4, biglen - 4)); /* all ex len */
     PUT_32BIT(pkt->data + pktoffs + 4 + biglen - 4, crc);

ssh1login-server.c

@@ -158,8 +158,7 @@ static void ssh1_login_server_process_queue(PacketProtocolLayer *ppl)
     if (s->ap_methods & AUTHMETHOD_CRYPTOCARD)
         s->supported_auths_mask |= (1U << SSH1_AUTH_CCARD);
 
-    for (i = 0; i < 8; i++)
+    random_read(s->cookie, 8);
-        s->cookie[i] = random_byte();
 
     pktout = ssh_bpp_new_pktout(s->ppl.bpp, SSH1_SMSG_PUBLIC_KEY);
     put_data(pktout, s->cookie, 8);
@@ -307,8 +306,7 @@ static void ssh1_login_server_process_queue(PacketProtocolLayer *ppl)
                 unsigned char *rsabuf =
                     snewn(s->authkey->bytes, unsigned char);
 
-                for (i = 0; i < 32; i++)
+                random_read(rsabuf, 32);
-                    rsabuf[i] = random_byte();
 
                 {
                     ssh_hash *h = ssh_hash_new(&ssh_md5);

ssh1login.c

@@ -201,8 +201,7 @@ static void ssh1_login_process_queue(PacketProtocolLayer *ppl)
     ssh1_compute_session_id(s->session_id, s->cookie,
                             &s->hostkey, &s->servkey);
 
-    for (i = 0; i < 32; i++)
+    random_read(s->session_key, 32);
-        s->session_key[i] = random_byte();
 
     /*
      * Verify that the `bits' and `bytes' parameters match.
@@ -986,10 +985,8 @@ static void ssh1_login_process_queue(PacketProtocolLayer *ppl)
                         put_stringz(pkt, s->cur_prompt->prompts[0]->result);
                         pq_push(s->ppl.out_pq, pkt);
                     } else {
-                        int j;
                         strbuf *random_data = strbuf_new();
-                        for (j = 0; j < i; j++)
+                        random_read(strbuf_append(random_data, i), i);
-                            put_byte(random_data, random_byte());
 
                         pkt = ssh_bpp_new_pktout(s->ppl.bpp, SSH1_MSG_IGNORE);
                         put_stringsb(pkt, random_data);
@@ -1009,9 +1006,8 @@ static void ssh1_login_process_queue(PacketProtocolLayer *ppl)
                 ppl_logevent("Sending length-padded password");
                 pkt = ssh_bpp_new_pktout(s->ppl.bpp, s->pwpkt_type);
                 put_asciz(padded_pw, s->cur_prompt->prompts[0]->result);
-                do {
+                size_t pad = 63 & -padded_pw->len;
-                    put_byte(padded_pw, random_byte());
+                random_read(strbuf_append(padded_pw, pad), pad);
-                } while (padded_pw->len % 64 != 0);
                 put_stringsb(pkt, padded_pw);
                 pq_push(s->ppl.out_pq, pkt);
             } else {

ssh2bpp.c

@@ -726,7 +726,8 @@ static void ssh2_bpp_format_packet_inner(struct ssh2_bpp_state *s, PktOut *pkt)
     maclen = s->out.mac ? ssh2_mac_alg(s->out.mac)->len : 0;
     origlen = pkt->length;
     for (i = 0; i < padding; i++)
-        put_byte(pkt, random_byte());
+        put_byte(pkt, 0);              /* make space for random padding */
+    random_read(pkt->data + origlen, padding);
     pkt->data[4] = padding;
     PUT_32BIT(pkt->data, origlen + padding - 4);
 
@@ -820,8 +821,10 @@ static void ssh2_bpp_format_packet(struct ssh2_bpp_state *s, PktOut *pkt)
 
             ignore_pkt = ssh2_bpp_new_pktout(SSH2_MSG_IGNORE);
             put_uint32(ignore_pkt, length);
-            while (length-- > 0)
+            size_t origlen = ignore_pkt->length;
-                put_byte(ignore_pkt, random_byte());
+            for (size_t i = 0; i < length; i++)
+                put_byte(ignore_pkt, 0);  /* make space for random padding */
+            random_read(ignore_pkt->data + origlen, length);
             ssh2_bpp_format_packet_inner(s, ignore_pkt);
             bufchain_add(s->bpp.out_raw, ignore_pkt->data, ignore_pkt->length);
             ssh_free_pktout(ignore_pkt);

ssh2kex-client.c

@@ -556,17 +556,12 @@ void ssh2kex_coroutine(struct ssh2_transport_state *s, bool *aborted)
         {
             int klen = ssh_rsakex_klen(s->rsa_kex_key);
             int nbits = klen - (2*s->kex_alg->hash->hlen*8 + 49);
-            int i, byte = 0;
             strbuf *buf, *outstr;
 
+            mp_int *tmp = mp_random_bits(nbits - 1);
             s->K = mp_power_2(nbits - 1);
-
+            mp_add_into(s->K, s->K, tmp);
-            for (i = 0; i < nbits; i++) {
+            mp_free(tmp);
-                if ((i & 7) == 0) {
-                    byte = random_byte();
-                }
-                mp_set_bit(s->K, i, (byte >> (i & 7)) & 1);
-            }
 
             /*
              * Encode this as an mpint.

ssh2transport.c

@@ -1048,11 +1048,7 @@ static void ssh2_transport_process_queue(PacketProtocolLayer *ppl)
      */
     s->client_kexinit->len = 0;
     put_byte(s->outgoing_kexinit, SSH2_MSG_KEXINIT);
-    {
+    random_read(strbuf_append(s->outgoing_kexinit, 16), 16);
-        int i;
-        for (i = 0; i < 16; i++)
-            put_byte(s->outgoing_kexinit, (unsigned char) random_byte());
-    }
     ssh2_write_kexinit_lists(
         BinarySink_UPCAST(s->outgoing_kexinit), s->kexlists,
         s->conf, s->ppl.remote_bugs,

sshecc.c

@@ -1269,8 +1269,8 @@ static void ssh_ecdhkex_w_setup(ecdh_key *dh)
 static void ssh_ecdhkex_m_setup(ecdh_key *dh)
 {
     strbuf *bytes = strbuf_new();
-    for (size_t i = 0; i < dh->curve->fieldBytes; ++i)
+    random_read(strbuf_append(bytes, dh->curve->fieldBytes),
-        put_byte(bytes, random_byte());
+                dh->curve->fieldBytes);
 
     bytes->u[0] &= 0xF8;
     bytes->u[bytes->len-1] &= 0x7F;

sshprime.c

@@ -387,7 +387,9 @@ void invent_firstbits(unsigned *one, unsigned *two)
      * i.e. the ones we actually invented.
      */
     do {
-        *one = 0x100 | random_byte();
+        uint8_t bytes[2];
-        *two = 0x100 | random_byte();
+        random_read(bytes, 2);
+        *one = 0x100 | bytes[0];
+        *two = 0x100 | bytes[1];
     } while (*one * *two < 0x20000);
 }

sshpubk.c

@@ -333,12 +333,10 @@ bool rsa_ssh1_savekey(const Filename *filename, RSAKey *key,
      * Two bytes, then the same two bytes repeated.
      */
     {
-        unsigned char b0 = random_byte();
+        uint8_t bytes[2];
-        unsigned char b1 = random_byte();
+        random_read(bytes, 2);
-        put_byte(buf, b0);
+        put_data(buf, bytes, 2);
-        put_byte(buf, b1);
+        put_data(buf, bytes, 2);
-        put_byte(buf, b0);
-        put_byte(buf, b1);
     }
 
     /*

sshrand.c

@@ -52,9 +52,10 @@ void random_add_noise(void *noise, int length) { }
 void random_add_heavynoise(void *noise, int length) { }
 void random_ref(void) { }
 void random_unref(void) { }
-int random_byte(void)
+void random_read(void *out, size_t size)
 {
     return 0x45; /* Chosen by eight fair coin tosses */
+    memset(out, 0x45, size); /* Chosen by eight fair coin tosses */
 }
 void random_get_savedata(void **data, int *len) { }
 #else /* !FUZZING */
@@ -319,14 +320,17 @@ void random_unref(void)
     random_active--;
 }
 
-int random_byte(void)
+void random_read(void *vout, size_t size)
 {
     assert(random_active);
 
+    uint8_t *out = (uint8_t *)vout;
+    while (size-- > 0) {
         if (pool.poolpos >= POOLSIZE)
             random_stir();
 
-    return pool.pool[pool.poolpos++];
+        *out++ = pool.pool[pool.poolpos++];
+    }
 }
 
 void random_get_savedata(void **data, int *len)

sshrsa.c

@@ -56,11 +56,37 @@ bool rsa_ssh1_encrypt(unsigned char *data, int length, RSAKey *key)
     data[0] = 0;
     data[1] = 2;
 
+    size_t npad = key->bytes - length - 3;
+    /*
+     * Generate a sequence of nonzero padding bytes. We do this in a
+     * reasonably uniform way and without having to loop round
+     * retrying the random number generation, by first generating an
+     * integer in [0,2^n) for an appropriately large n; then we
+     * repeatedly multiply by 255 to give an integer in [0,255*2^n),
+     * extract the top 8 bits to give an integer in [0,255), and mask
+     * those bits off before multiplying up again for the next digit.
+     * This gives us a sequence of numbers in [0,255), and of course
+     * adding 1 to each of them gives numbers in [1,256) as we wanted.
+     *
+     * (You could imagine this being a sort of fixed-point operation:
+     * given a uniformly random binary _fraction_, multiplying it by k
+     * and subtracting off the integer part will yield you a sequence
+     * of integers each in [0,k). I'm just doing that scaled up by a
+     * power of 2 to avoid the fractions.)
+     */
+    size_t random_bits = (npad + 16) * 8;
+    mp_int *randval = mp_new(random_bits + 8);
+    mp_int *tmp = mp_random_bits(random_bits);
+    mp_copy_into(randval, tmp);
+    mp_free(tmp);
     for (i = 2; i < key->bytes - length - 1; i++) {
-	do {
+        mp_mul_integer_into(randval, randval, 255);
-	    data[i] = random_byte();
+        uint8_t byte = mp_get_byte(randval, random_bits / 8);
-	} while (data[i] == 0);
+        assert(byte != 255);
+        data[i] = byte + 1;
+        mp_reduce_mod_2to(randval, random_bits);
     }
+    mp_free(randval);
     data[key->bytes - length - 1] = 0;
 
     b1 = mp_from_bytes_be(make_ptrlen(data, key->bytes));
@@ -804,8 +830,7 @@ strbuf *ssh_rsakex_encrypt(RSAKey *rsa, const ssh_hashalg *h, ptrlen in)
     /* Leading byte zero. */
     out[0] = 0;
     /* At position 1, the seed: HLEN bytes of random data. */
-    for (i = 0; i < HLEN; i++)
+    random_read(out + 1, HLEN);
-        out[i + 1] = random_byte();
     /* At position 1+HLEN, the data block DB, consisting of: */
     /* The hash of the label (we only support an empty label here) */
     {

testcrypt.c

@@ -49,13 +49,10 @@ static NORETURN void fatal_error(const char *p, ...)
 void out_of_memory(void) { fatal_error("out of memory"); }
 
 static bufchain random_data_queue;
-int random_byte(void)
+void random_read(void *buf, size_t size)
 {
-    unsigned char u;
+    if (!bufchain_try_fetch_consume(&random_data_queue, buf, size))
-    if (bufchain_try_fetch_consume(&random_data_queue, &u, 1))
-        return u;
         fatal_error("No random data in queue");
-    return 0;
 }
 
 #define VALUE_TYPES(X)                                                  \

unix/uxsftpserver.c

@@ -58,7 +58,6 @@ static int uss_dirhandle_cmp(void *av, void *bv)
 
 static SftpServer *uss_new(const SftpServerVtable *vt)
 {
-    int i;
     UnixSftpServer *uss = snew(UnixSftpServer);
 
     memset(uss, 0, sizeof(UnixSftpServer));
@@ -66,8 +65,7 @@ static SftpServer *uss_new(const SftpServerVtable *vt)
     uss->dirhandles = newtree234(uss_dirhandle_cmp);
     uss->srv.vt = vt;
 
-    for (i = 0; i < lenof(uss->handlekey); i++)
+    random_read(uss->handlekey, sizeof(uss->handlekey));
-        uss->handlekey[i] = random_byte();
 
     return &uss->srv;
 }

unix/uxshare.c

@@ -116,9 +116,7 @@ static char *make_dirname(const char *pi_name, char **logtext)
             /*
              * Invent some random data.
              */
-            for (i = 0; i < SALT_SIZE; i++) {
+            random_read(saltbuf, SALT_SIZE);
-                saltbuf[i] = random_byte();
-            }
             ret = write(saltfd, saltbuf, SALT_SIZE);
             /* POSIX atomicity guarantee: because we wrote less than
              * PIPE_BUF bytes, the write either completed in full or

x11fwd.c

@@ -93,8 +93,7 @@ struct X11FakeAuth *x11_invent_fake_auth(tree234 *authtree, int authtype)
         auth->xa1_firstblock = NULL;
 
         while (1) {
-            for (i = 0; i < auth->datalen; i++)
+            random_read(auth->data, auth->datalen);
-                auth->data[i] = random_byte();
             if (add234(authtree, auth) == auth)
                 break;
         }
@@ -111,8 +110,10 @@ struct X11FakeAuth *x11_invent_fake_auth(tree234 *authtree, int authtype)
         memset(auth->xa1_firstblock, 0, 8);
 
         while (1) {
-            for (i = 0; i < auth->datalen; i++)
+            random_read(auth->data, 15);
-                auth->data[i] = (i == 8 ? 0 : random_byte());
+            auth->data[15] = auth->data[8];
+            auth->data[8] = 0;
+
             memcpy(auth->xa1_firstblock, auth->data, 8);
             des_encrypt_xdmauth(auth->data + 9, auth->xa1_firstblock, 8);
             if (add234(authtree, auth) == auth)