diff --git a/src/bench.c b/src/bench.c index 0517a6a58..66dad6452 100644 --- a/src/bench.c +++ b/src/bench.c @@ -672,13 +672,16 @@ static void sphincs_shake_128f_commit_bench(benchmark_context *ctx) { /* ---- secp256k1 via ECC module (no direct secp includes) ---- */ -static void make_valid_privkey(uint8_t sk[32]) { +static dogecoin_bool make_valid_privkey(uint8_t sk[32]) { /* try random bytes until valid */ while (1) { - dogecoin_random_bytes(sk, 32, 0); - if (dogecoin_ecc_verify_privatekey(sk)) return; + if (!dogecoin_random_bytes(sk, 32, 0)) { + dogecoin_mem_zero(sk, 32); + return false; + } + if (dogecoin_ecc_verify_privatekey(sk)) return true; sk[0] ^= 0x01; /* nudge */ - if (dogecoin_ecc_verify_privatekey(sk)) return; + if (dogecoin_ecc_verify_privatekey(sk)) return true; } } @@ -688,7 +691,7 @@ static void make_msg32(const uint8_t *src, uint8_t out32[32]) { static void secp_keypair_bench(benchmark_context *ctx) { /* keypair = generate priv + derive compressed pub */ - uint8_t sk[32]; make_valid_privkey(sk); + uint8_t sk[32]; if (!make_valid_privkey(sk)) return; uint8_t pk[33]; size_t pklen = 33; dogecoin_ecc_get_pubkey(sk, pk, &pklen, true); ctx->end = gettimedouble(); ctx->endCycles = perf_cpucycles(); @@ -699,7 +702,7 @@ static void secp_sign_bench(benchmark_context *ctx) { /* reuse one key for stable sign throughput */ static int primed = 0; static uint8_t sk[32]; - if (!primed) { make_valid_privkey(sk); primed = 1; } + if (!primed) { if (!make_valid_privkey(sk)) return; primed = 1; } uint8_t msg32[32]; make_msg32(ctx->input, msg32); uint256_t msg_hash; memcpy(msg_hash, msg32, 32); @@ -718,7 +721,7 @@ static void secp_verify_bench(benchmark_context *ctx) { static unsigned char sigder[80]; static size_t siglen = 0; if (!primed) { - uint8_t sk[32]; make_valid_privkey(sk); + uint8_t sk[32]; if (!make_valid_privkey(sk)) return; pklen = 33; dogecoin_ecc_get_pubkey(sk, pk, &pklen, true); make_msg32(ctx->input, msg32); memcpy(msg_hash, msg32, 32); diff --git a/src/bip38.c b/src/bip38.c index 7ec060718..fa4f956d1 100644 --- a/src/bip38.c +++ b/src/bip38.c @@ -361,7 +361,7 @@ static dogecoin_bool bip38_ec_derived_key( BIP38_SCRYPT_DERIVED_SIZE); } -static void bip38_ownerentropy_generate( +static dogecoin_bool bip38_ownerentropy_generate( dogecoin_bool use_lot_sequence, uint32_t lot, uint32_t sequence, @@ -369,14 +369,19 @@ static void bip38_ownerentropy_generate( { if (use_lot_sequence) { uint32_t lotsequence = lot * 4096u + sequence; - dogecoin_random_bytes(ownerentropy_out, 4, 1); + if (!dogecoin_random_bytes(ownerentropy_out, 4, 1)) { + return false; + } ownerentropy_out[4] = (uint8_t)((lotsequence >> 24) & 0xff); ownerentropy_out[5] = (uint8_t)((lotsequence >> 16) & 0xff); ownerentropy_out[6] = (uint8_t)((lotsequence >> 8) & 0xff); ownerentropy_out[7] = (uint8_t)(lotsequence & 0xff); } else { - dogecoin_random_bytes(ownerentropy_out, 8, 1); + if (!dogecoin_random_bytes(ownerentropy_out, 8, 1)) { + return false; + } } + return true; } static dogecoin_bool bip38_parse_intermediate_code( @@ -1182,8 +1187,17 @@ void dogecoin_bip38_generate_lot_sequence( return; } - dogecoin_random_bytes((uint8_t*)&lot_raw, sizeof(lot_raw), 1); - dogecoin_random_bytes((uint8_t*)&sequence_raw, sizeof(sequence_raw), 1); + /* This is LIBDOGECOIN_API and returns void, so it cannot report failure + directly without breaking the signature. Emit lot 0 instead: every + caller of these values already rejects it (`lot == 0` at the top of both + encrypt paths), so an RNG failure surfaces there rather than producing a + lot and sequence derived from an unwritten buffer. */ + if (!dogecoin_random_bytes((uint8_t*)&lot_raw, sizeof(lot_raw), 1) + || !dogecoin_random_bytes((uint8_t*)&sequence_raw, sizeof(sequence_raw), 1)) { + *lot_out = 0; + *sequence_out = 0; + return; + } *lot_out = (lot_raw % BIP38_LOT_MAX) + 1u; *sequence_out = sequence_raw % (BIP38_SEQUENCE_MAX + 1u); } @@ -1352,7 +1366,9 @@ static dogecoin_bool bip38_encrypt_ec_with_passphrase( if (ownerentropy_override) { memcpy(ownerentropy, ownerentropy_override, 8); } else { - bip38_ownerentropy_generate(use_lot_sequence, lot, sequence, ownerentropy); + if (!bip38_ownerentropy_generate(use_lot_sequence, lot, sequence, ownerentropy)) { + return false; + } } if (!bip38_derive_passfactor(passphrase, ownerentropy, use_lot_sequence, passfactor)) { @@ -1364,8 +1380,12 @@ static dogecoin_bool bip38_encrypt_ec_with_passphrase( if (seedb_override) { memcpy(seedb, seedb_override, BIP38_SEEDB_LEN); - } else { - dogecoin_random_bytes(seedb, BIP38_SEEDB_LEN, 1); + } else if (!dogecoin_random_bytes(seedb, BIP38_SEEDB_LEN, 1)) { + /* seedb derives factorb, and the private key is passfactor * factorb + mod N. Continuing here would mint a key from whatever the RNG left + in the buffer. */ + dogecoin_mem_zero(seedb, BIP38_SEEDB_LEN); + return false; } chain = bip38_chain_from_address_hint(address_hint); @@ -1417,7 +1437,9 @@ dogecoin_bool dogecoin_bip38_generate_intermediate_code( if (ownerentropy_override) { memcpy(ownerentropy, ownerentropy_override, 8); } else { - bip38_ownerentropy_generate(use_lot_sequence, lot, sequence, ownerentropy); + if (!bip38_ownerentropy_generate(use_lot_sequence, lot, sequence, ownerentropy)) { + return false; + } } if (!bip38_derive_passfactor(passphrase, ownerentropy, use_lot_sequence, passfactor)) { @@ -1470,8 +1492,12 @@ dogecoin_bool dogecoin_bip38_encrypt_from_intermediate( if (seedb_override) { memcpy(seedb, seedb_override, BIP38_SEEDB_LEN); - } else { - dogecoin_random_bytes(seedb, BIP38_SEEDB_LEN, 1); + } else if (!dogecoin_random_bytes(seedb, BIP38_SEEDB_LEN, 1)) { + /* seedb derives factorb, and the private key is passfactor * factorb + mod N. Continuing here would mint a key from whatever the RNG left + in the buffer. */ + dogecoin_mem_zero(seedb, BIP38_SEEDB_LEN); + return false; } chain = bip38_chain_from_address_hint(address_chain_hint); diff --git a/src/cli/tool.c b/src/cli/tool.c index a8ad4319e..bf4f06e5f 100644 --- a/src/cli/tool.c +++ b/src/cli/tool.c @@ -141,7 +141,10 @@ dogecoin_bool hd_gen_master(const dogecoin_chainparams* chain, char* masterkeyhe { dogecoin_hdnode node; uint8_t seed[32]; - dogecoin_random_bytes(seed, 32, true); + if (!dogecoin_random_bytes(seed, 32, true)) { + dogecoin_mem_zero(seed, 32); + return false; + } dogecoin_hdnode_from_seed(seed, 32, &node); dogecoin_mem_zero(seed, 32); dogecoin_hdnode_serialize_private(&node, chain, masterkeyhex, strsize); diff --git a/src/openenclave/host/host.c b/src/openenclave/host/host.c index 78d0bffc4..95bf862d8 100644 --- a/src/openenclave/host/host.c +++ b/src/openenclave/host/host.c @@ -412,7 +412,10 @@ int main(int argc, char* argv[]) printf("Shared secret not provided, generating one...\n"); // Generate random 20 bytes (40 hex characters) unsigned char random_bytes[TOTP_SECRET_HEX_SIZE / 2]; - dogecoin_random_bytes(random_bytes, sizeof(random_bytes), 0); + if (!dogecoin_random_bytes(random_bytes, sizeof(random_bytes), 0)) { + fprintf(stderr, "failed to obtain entropy\n"); + return 1; + } shared_secret = malloc(TOTP_SECRET_HEX_SIZE + 1); if (!shared_secret) { diff --git a/src/optee/host/main.c b/src/optee/host/main.c index 53653ee29..14ad3f053 100644 --- a/src/optee/host/main.c +++ b/src/optee/host/main.c @@ -775,7 +775,10 @@ int main(int argc, const char* argv[]) printf("Shared secret not provided, generating one...\n"); // Generate random 20 bytes (40 hex characters) unsigned char random_bytes[TOTP_SECRET_HEX_SIZE / 2]; - dogecoin_random_bytes(random_bytes, sizeof(random_bytes), 0); + if (!dogecoin_random_bytes(random_bytes, sizeof(random_bytes), 0)) { + fprintf(stderr, "failed to obtain entropy\n"); + return 1; + } shared_secret = malloc(TOTP_SECRET_HEX_SIZE + 1); if (!shared_secret) { diff --git a/src/random.c b/src/random.c index a17e9ea12..fbe9eec6c 100644 --- a/src/random.c +++ b/src/random.c @@ -229,7 +229,15 @@ void random_seed(struct fast_random_context* this) dogecoin_random_init(); uint256_t seed; dogecoin_mem_zero(seed, 32); - dogecoin_random_bytes(seed, 32, 0); + if (!dogecoin_random_bytes(seed, 32, 0)) { + /* random_seed is reached through a void function pointer in + fast_random_context, so it cannot report failure without changing + that interface. Leave requires_seed set and do not key the RNG: a + later call re-enters here and tries again, rather than the context + coming up keyed with an all-zero seed and looking ready. */ + dogecoin_mem_zero(seed, 32); + return; + } this->rng->setkey(this->rng, seed, 32); this->requires_seed = false; } diff --git a/test/random_tests.c b/test/random_tests.c index 81df140ef..a7c4ef282 100644 --- a/test/random_tests.c +++ b/test/random_tests.c @@ -14,6 +14,7 @@ #include +#include #include #include @@ -80,3 +81,84 @@ void test_random() // switch back to the default random callback mapper dogecoin_rnd_set_mapper_default(); } + + +/* --- regression: a failing RNG must not yield key material --- */ + +static void rng_prop_init(void) {} + +static dogecoin_bool rng_prop_fail(uint8_t* buf, uint32_t len, const uint8_t update_seed) +{ + (void)buf; (void)len; (void)update_seed; + return false; +} + +/* + * Eleven call sites discarded dogecoin_random_bytes' return value, six of them + * in BIP38 -- including seedb, which derives factorb and thence the private + * key. With the return ignored, an RNG failure produced a key from whatever was + * already in the buffer. + * + * Driving the real failure path is not possible from a test, so this installs a + * mapper that always fails and checks the callers behave: the encrypt paths + * must refuse rather than mint a key. + */ +void test_random_failure_propagates() +{ + dogecoin_rnd_mapper mapper; + uint8_t privkey[DOGECOIN_ECKEY_PKEY_LENGTH]; + char encrypted[128]; + char confirmation[128]; + size_t enc_sz, conf_sz; + uint32_t lot = 12345, sequence = 42; + + mapper.dogecoin_random_init = rng_prop_init; + mapper.dogecoin_random_bytes = rng_prop_fail; + dogecoin_rnd_set_mapper(mapper); + + /* EC-multiplied encryption reaches seedb and the owner entropy. It must + fail rather than return a key derived from an unwritten buffer. */ + enc_sz = sizeof(encrypted); + conf_sz = sizeof(confirmation); + u_assert_int_eq((int)dogecoin_bip38_encrypt_ec_multiplied( + "passphrase", true, false, 0, 0, "DGYrGxANmgjcoZ9xJWncHr6fuA6Y1ZQ56Y", + privkey, encrypted, &enc_sz, confirmation, &conf_sz), 0); + + /* Same with lot/sequence, which additionally goes through the owner + entropy path. */ + enc_sz = sizeof(encrypted); + conf_sz = sizeof(confirmation); + u_assert_int_eq((int)dogecoin_bip38_encrypt_ec_multiplied( + "passphrase", true, true, 100000, 7, "DGYrGxANmgjcoZ9xJWncHr6fuA6Y1ZQ56Y", + privkey, encrypted, &enc_sz, confirmation, &conf_sz), 0); + + /* + * Isolate the seedb site. encrypt_from_intermediate takes a ready-made + * intermediate code -- the BIP38 spec vector -- so the owner entropy is + * already fixed and the RNG is reached for seedb alone. Without the check + * at that site this call still returns a key, because nothing upstream + * fails first. + */ + enc_sz = sizeof(encrypted); + conf_sz = sizeof(confirmation); + u_assert_int_eq((int)dogecoin_bip38_encrypt_from_intermediate( + "passphrasepxFy57B9v8HtUsszJYKReoNDV6VHjUSGt8EVJmux9n1J3Ltf1gRxyDGXqnf9qm", + true, NULL, "DGYrGxANmgjcoZ9xJWncHr6fuA6Y1ZQ56Y", + encrypted, &enc_sz, confirmation, &conf_sz), 0); + + /* dogecoin_bip38_generate_lot_sequence returns void and is public API, so + it signals failure with lot 0 -- a value every consumer of these already + rejects. */ + dogecoin_bip38_generate_lot_sequence(&lot, &sequence); + u_assert_uint32_eq(lot, 0U); + u_assert_uint32_eq(sequence, 0U); + + dogecoin_rnd_set_mapper_default(); + + /* And with the real RNG restored, the same call succeeds. */ + enc_sz = sizeof(encrypted); + conf_sz = sizeof(confirmation); + u_assert_int_eq((int)dogecoin_bip38_encrypt_ec_multiplied( + "passphrase", true, false, 0, 0, "DGYrGxANmgjcoZ9xJWncHr6fuA6Y1ZQ56Y", + privkey, encrypted, &enc_sz, confirmation, &conf_sz), 1); +} diff --git a/test/unittester.c b/test/unittester.c index 96dd35fae..c486e2a03 100644 --- a/test/unittester.c +++ b/test/unittester.c @@ -60,6 +60,7 @@ extern void test_memory(); extern void test_moon(); extern void test_op_return(); extern void test_random(); +extern void test_random_failure_propagates(); extern void test_rmd160(); extern void test_scrypt(); extern void test_serialize(); @@ -191,6 +192,7 @@ int main() u_run_test(test_moon); u_run_test(test_op_return); u_run_test(test_random); + u_run_test(test_random_failure_propagates); u_run_test(test_rmd160); u_run_test(test_scrypt); u_run_test(test_serialize);