1
0
mirror of https://git.tartarus.org/simon/putty.git synced 2025-01-24 16:52:24 +00:00

sshprng.c: remove pointless pending_output buffer.

In an early draft of the new PRNG, before I decided to get rid of
random_byte() and replace it with random_read(), it was important
after generating a hash-worth of PRNG output to buffer it so as to
return it a byte at a time. So the PRNG data structure itself had to
keep a hash-sized buffer of pending output, and be able to return the
next byte from it on every random_byte() call.

But when random_read() came in, there was no need to do that any more,
because at the end of a read, the generator is re-seeded and the
remains of any generated data is deliberately thrown away. So the
pending_output buffer has no need to live in the persistent prng
object; it can be relegated to a local variable inside random_read
(and a couple of other functions that used the same buffer since it
was conveniently there).

A side effect of this is that we're no longer yielding the bytes of
each hash in reverse order, because only the previous silly code
structure made it convenient. Fortunately, of course, nothing is
depending on that - except the cryptsuite tests, which I've updated.
This commit is contained in:
Simon Tatham 2020-01-26 10:58:27 +00:00
parent 213723a718
commit 404f558705
2 changed files with 23 additions and 31 deletions

View File

@ -55,17 +55,9 @@ struct prng_impl {
* into it. The counter-mode generation is achieved by copying * into it. The counter-mode generation is achieved by copying
* that hash object, appending the counter value to the copy, and * that hash object, appending the counter value to the copy, and
* calling ssh_hash_final. * calling ssh_hash_final.
*
* pending_output is a buffer of size equal to the hash length,
* which receives each of those hashes as it's generated. The
* bytes of the hash are returned in reverse order, just because
* that made it marginally easier to deal with the
* pending_output_remaining field.
*/ */
ssh_hash *generator; ssh_hash *generator;
mp_int *counter; mp_int *counter;
uint8_t *pending_output;
size_t pending_output_remaining;
/* /*
* When re-seeding the generator, you call prng_seed_begin(), * When re-seeding the generator, you call prng_seed_begin(),
@ -121,8 +113,6 @@ prng *prng_new(const ssh_hashalg *hashalg)
pi->hashalg = hashalg; pi->hashalg = hashalg;
pi->keymaker = NULL; pi->keymaker = NULL;
pi->generator = NULL; pi->generator = NULL;
pi->pending_output = snewn(pi->hashalg->hlen, uint8_t);
pi->pending_output_remaining = 0;
pi->counter = mp_new(128); pi->counter = mp_new(128);
for (size_t i = 0; i < NCOLLECTORS; i++) for (size_t i = 0; i < NCOLLECTORS; i++)
pi->collectors[i] = ssh_hash_new(pi->hashalg); pi->collectors[i] = ssh_hash_new(pi->hashalg);
@ -138,7 +128,6 @@ void prng_free(prng *pr)
{ {
prng_impl *pi = container_of(pr, prng_impl, Prng); prng_impl *pi = container_of(pr, prng_impl, Prng);
sfree(pi->pending_output);
mp_free(pi->counter); mp_free(pi->counter);
for (size_t i = 0; i < NCOLLECTORS; i++) for (size_t i = 0; i < NCOLLECTORS; i++)
ssh_hash_free(pi->collectors[i]); ssh_hash_free(pi->collectors[i]);
@ -184,6 +173,7 @@ static void prng_seed_BinarySink_write(
void prng_seed_finish(prng *pr) void prng_seed_finish(prng *pr)
{ {
prng_impl *pi = container_of(pr, prng_impl, Prng); prng_impl *pi = container_of(pr, prng_impl, Prng);
unsigned char buf[MAX_HASH_LEN];
assert(pi->keymaker); assert(pi->keymaker);
@ -192,7 +182,7 @@ void prng_seed_finish(prng *pr)
/* /*
* Actually generate the key. * Actually generate the key.
*/ */
ssh_hash_final(pi->keymaker, pi->pending_output); ssh_hash_final(pi->keymaker, buf);
pi->keymaker = NULL; pi->keymaker = NULL;
/* /*
@ -201,15 +191,15 @@ void prng_seed_finish(prng *pr)
*/ */
assert(!pi->generator); assert(!pi->generator);
pi->generator = ssh_hash_new(pi->hashalg); pi->generator = ssh_hash_new(pi->hashalg);
put_data(pi->generator, pi->pending_output, pi->hashalg->hlen); put_data(pi->generator, buf, pi->hashalg->hlen);
smemclr(pi->pending_output, pi->hashalg->hlen);
pi->until_reseed = RESEED_DATA_SIZE; pi->until_reseed = RESEED_DATA_SIZE;
pi->last_reseed_time = prng_reseed_time_ms(); pi->last_reseed_time = prng_reseed_time_ms();
pi->pending_output_remaining = 0;
smemclr(buf, sizeof(buf));
} }
static inline void prng_generate(prng_impl *pi) static inline void prng_generate(prng_impl *pi, void *outbuf)
{ {
ssh_hash *h = ssh_hash_copy(pi->generator); ssh_hash *h = ssh_hash_copy(pi->generator);
@ -218,27 +208,29 @@ static inline void prng_generate(prng_impl *pi)
put_byte(h, 'G'); put_byte(h, 'G');
put_mp_ssh2(h, pi->counter); put_mp_ssh2(h, pi->counter);
mp_add_integer_into(pi->counter, pi->counter, 1); mp_add_integer_into(pi->counter, pi->counter, 1);
ssh_hash_final(h, pi->pending_output); ssh_hash_final(h, outbuf);
pi->pending_output_remaining = pi->hashalg->hlen;
} }
void prng_read(prng *pr, void *vout, size_t size) void prng_read(prng *pr, void *vout, size_t size)
{ {
prng_impl *pi = container_of(pr, prng_impl, Prng); prng_impl *pi = container_of(pr, prng_impl, Prng);
unsigned char buf[MAX_HASH_LEN];
assert(!pi->keymaker); assert(!pi->keymaker);
prngdebug("prng_read %"SIZEu"\n", size); prngdebug("prng_read %"SIZEu"\n", size);
uint8_t *out = (uint8_t *)vout; uint8_t *out = (uint8_t *)vout;
for (; size > 0; size--) { while (size > 0) {
if (pi->pending_output_remaining == 0) prng_generate(pi, buf);
prng_generate(pi); size_t to_use = size > pi->hashalg->hlen ? pi->hashalg->hlen : size;
pi->pending_output_remaining--; memcpy(out, buf, to_use);
*out++ = pi->pending_output[pi->pending_output_remaining]; out += to_use;
pi->pending_output[pi->pending_output_remaining] = 0; size -= to_use;
} }
smemclr(buf, sizeof(buf));
prng_seed_begin(&pi->Prng); prng_seed_begin(&pi->Prng);
prng_seed_finish(&pi->Prng); prng_seed_finish(&pi->Prng);
} }
@ -269,18 +261,19 @@ void prng_add_entropy(prng *pr, unsigned source_id, ptrlen data)
prng_reseed_time_ms() - pi->last_reseed_time >= 100) { prng_reseed_time_ms() - pi->last_reseed_time >= 100) {
prng_seed_begin(&pi->Prng); prng_seed_begin(&pi->Prng);
unsigned char buf[MAX_HASH_LEN];
uint32_t reseed_index = ++pi->reseeds; uint32_t reseed_index = ++pi->reseeds;
prngdebug("prng entropy reseed #%"PRIu32"\n", reseed_index); prngdebug("prng entropy reseed #%"PRIu32"\n", reseed_index);
for (size_t i = 0; i < NCOLLECTORS; i++) { for (size_t i = 0; i < NCOLLECTORS; i++) {
prngdebug("emptying collector %"SIZEu"\n", i); prngdebug("emptying collector %"SIZEu"\n", i);
ssh_hash_digest(pi->collectors[i], pi->pending_output); ssh_hash_digest(pi->collectors[i], buf);
put_data(&pi->Prng, pi->pending_output, pi->hashalg->hlen); put_data(&pi->Prng, buf, pi->hashalg->hlen);
ssh_hash_reset(pi->collectors[i]); ssh_hash_reset(pi->collectors[i]);
if (reseed_index & 1) if (reseed_index & 1)
break; break;
reseed_index >>= 1; reseed_index >>= 1;
} }
smemclr(buf, sizeof(buf));
prng_seed_finish(&pi->Prng); prng_seed_finish(&pi->Prng);
} }
} }

View File

@ -1197,7 +1197,6 @@ class crypt(MyTestBase):
hashalg = 'sha256' hashalg = 'sha256'
seed = b"hello, world" seed = b"hello, world"
entropy = b'1234567890' * 100 entropy = b'1234567890' * 100
rev = lambda s: valbytes(reversed(bytevals(s)))
# Replicate the generation of some random numbers. to ensure # Replicate the generation of some random numbers. to ensure
# they really are the hashes of what they're supposed to be. # they really are the hashes of what they're supposed to be.
@ -1212,21 +1211,21 @@ class crypt(MyTestBase):
key1 = hash_str(hashalg, b'R' + seed) key1 = hash_str(hashalg, b'R' + seed)
expected_data1 = b''.join( expected_data1 = b''.join(
rev(hash_str(hashalg, key1 + b'G' + ssh2_mpint(counter))) hash_str(hashalg, key1 + b'G' + ssh2_mpint(counter))
for counter in range(4)) for counter in range(4))
# After prng_read finishes, we expect the PRNG to have # After prng_read finishes, we expect the PRNG to have
# automatically reseeded itself, so that if its internal state # automatically reseeded itself, so that if its internal state
# is revealed then the previous output can't be reconstructed. # is revealed then the previous output can't be reconstructed.
key2 = hash_str(hashalg, key1 + b'R') key2 = hash_str(hashalg, key1 + b'R')
expected_data2 = b''.join( expected_data2 = b''.join(
rev(hash_str(hashalg, key2 + b'G' + ssh2_mpint(counter))) hash_str(hashalg, key2 + b'G' + ssh2_mpint(counter))
for counter in range(4,8)) for counter in range(4,8))
# There will have been another reseed after the second # There will have been another reseed after the second
# prng_read, and then another due to the entropy. # prng_read, and then another due to the entropy.
key3 = hash_str(hashalg, key2 + b'R') key3 = hash_str(hashalg, key2 + b'R')
key4 = hash_str(hashalg, key3 + b'R' + hash_str(hashalg, entropy)) key4 = hash_str(hashalg, key3 + b'R' + hash_str(hashalg, entropy))
expected_data3 = b''.join( expected_data3 = b''.join(
rev(hash_str(hashalg, key4 + b'G' + ssh2_mpint(counter))) hash_str(hashalg, key4 + b'G' + ssh2_mpint(counter))
for counter in range(8,12)) for counter in range(8,12))
self.assertEqualBin(data1, expected_data1) self.assertEqualBin(data1, expected_data1)