diff --git a/secure_enclave/EnclaveCommon.h b/secure_enclave/EnclaveCommon.h index 7c5c7329..14b5b1b6 100644 --- a/secure_enclave/EnclaveCommon.h +++ b/secure_enclave/EnclaveCommon.h @@ -64,8 +64,6 @@ EXTERNC void LOG_TRACE(const char *_msg); extern uint32_t globalLogLevel_; -extern unsigned char *globalRandom; - extern domain_parameters curve; #define SAFE_FREE(__X__) \ diff --git a/secure_enclave/Signature.c b/secure_enclave/Signature.c index 24a8a3a8..3346e78b 100644 --- a/secure_enclave/Signature.c +++ b/secure_enclave/Signature.c @@ -127,8 +127,6 @@ void signature_sign(signature sig, mpz_t message, mpz_t private_key, domain_para SAFE_CHAR_BUF(rand_char, 32); - get_global_random((unsigned char *) rand_char, 32); - signature_sign_start: @@ -136,7 +134,13 @@ void signature_sign(signature sig, mpz_t message, mpz_t private_key, domain_para mpz_import(seed, 32, 1, sizeof(rand_char[0]), 0, 0, rand_char); - mpz_mod(k, seed, curve->p); + // Do not compute seed mod n: the 256-bit source range is not an exact + // multiple of the curve order, so modulo reduction would make some nonce + // values slightly more likely than others. Rejection sampling preserves a + // uniform distribution over the valid ECDSA nonce range [1, n - 1]. + if (mpz_sgn(seed) == 0 || mpz_cmp(seed, curve->n) >= 0) + goto signature_sign_start; + mpz_set(k, seed); //Calculate x point_multiplication(Q, k, curve->G, curve); @@ -148,17 +152,22 @@ void signature_sign(signature sig, mpz_t message, mpz_t private_key, domain_para goto signature_sign_start; - //Calculate s - //s = k¯¹(e+d*r) mod n = (k¯¹ mod n) * ((e+d*r) mod n) mod n - //number_theory_inverse(t1, k, curve->n);//t1 = k¯¹ mod n - mpz_invert(t1, k, curve->n); - mpz_mul(t2, private_key, r); //t2 = d*r - mpz_add(t3, message, t2); //t3 = e+t2 - mpz_mod(t4, t3, curve->n); //t2 = t3 mod n - mpz_mul(t5, t4, t1); //t3 = t2 * t1 - mpz_mod(s, t5, curve->n); //s = t3 mod n + // Calculate s + // s = k¯¹(e+d*r) mod n = (k¯¹ mod n) * ((e+d*r) mod n) mod n + + if (mpz_invert(t1, k, curve->n) == 0) // t1 = k¯¹ mod n + goto signature_sign_start; // should never happen - only a defensive check + + mpz_mul(t2, private_key, r); // t2 = d*r + mpz_add(t3, message, t2); // t3 = e+t2 + mpz_mod(t4, t3, curve->n); // t4 = t3 mod n + mpz_mul(t5, t4, t1); // t5 = t4 * t1 + mpz_mod(s, t5, curve->n); // s = t5 mod n + + if (mpz_sgn(s) == 0) // Start over if s=0 + goto signature_sign_start; - //Calculate v + // Calculate v mpz_mod_ui(rem, Q->y, 2); @@ -224,9 +233,9 @@ bool signature_verify(mpz_t message, signature sig, point public_key, domain_par bool result = false; - if (mpz_cmp(sig->r, one) < 0 && - mpz_cmp(curve->n, sig->r) <= 0 && - mpz_cmp(sig->s, one) < 0 && + if (mpz_cmp(sig->r, one) < 0 || + mpz_cmp(curve->n, sig->r) <= 0 || + mpz_cmp(sig->s, one) < 0 || mpz_cmp(curve->n, sig->s) <= 0) { goto clean; } diff --git a/secure_enclave/secure_enclave.c b/secure_enclave/secure_enclave.c index 95621036..8ef4ecd3 100644 --- a/secure_enclave/secure_enclave.c +++ b/secure_enclave/secure_enclave.c @@ -41,6 +41,7 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. #include #include "secure_enclave_t.h" +#include "sgx_error.h" #include "sgx_tcrypto.h" #include "sgx_tseal.h" #include @@ -118,8 +119,6 @@ void *reallocate_function(void *, size_t, size_t); void free_function(void *, size_t); -unsigned char *globalRandom = NULL; - // ----------------------------------------------------------------------------------------- // Helper functions // ----------------------------------------------------------------------------------------- @@ -216,15 +215,17 @@ void trustedEnclaveInit(uint64_t _logLevel) { enclave_init(); - LOG_INFO("Reading random"); - - globalRandom = calloc(32,1); + LOG_INFO("Verifying hardware RNG"); - int ret = sgx_read_rand(globalRandom, 32); + // Fail fast at init if the hardware RNG is unavailable, rather than + // discovering it later when generating keys. get_global_random reads from + // the same source on every call and also fails closed. + unsigned char rngSelfTest[32]; + sgx_status_t ret = sgx_read_rand(rngSelfTest, sizeof(rngSelfTest)); if(ret != SGX_SUCCESS) { - LOG_ERROR("sgx_read_rand failed. Aboring enclave."); + LOG_ERROR("sgx_read_rand failed. Aborting enclave."); abort(); } @@ -244,7 +245,6 @@ void trustedEnclaveInit(uint64_t _logLevel) { } void trustedEnclaveClear() { - free(globalRandom); enclave_clear(); } @@ -284,28 +284,21 @@ void *reallocate_function(void *ptr, size_t osize, size_t nsize) { return (void *) nptr; } -volatile uint64_t counter = 0; - void get_global_random(unsigned char *_randBuff, uint64_t _size) { - char errString[ENCLAVE_BUF_LEN]; - int status; - int *errStatus = &status; - - INIT_ERROR_STATE + // Randomness is read directly from the CPU's RDRAND-backed DRNG on every + // call. This is stateless and thread-safe: the hardware serves a distinct + // value per request across all logical cores. It is cryptographically unlikely + // that two threads will receive the same value + if (_randBuff == NULL || _size < 1 || _size > 32) { + LOG_ERROR("get_global_random called with invalid arguments. Aborting enclave."); + abort(); + } - CHECK_STATE(_size <= 32) - CHECK_STATE(_randBuff); - - const uint64_t counter_snapshot = ++counter; - sgx_sha_state_handle_t shaStateHandle; - CHECK_STATE(sgx_sha256_init(&shaStateHandle) == SGX_SUCCESS); - CHECK_STATE(sgx_sha256_update(globalRandom, 32, shaStateHandle) == SGX_SUCCESS); - CHECK_STATE(sgx_sha256_update((const uint8_t *)&counter_snapshot, sizeof(counter_snapshot), shaStateHandle) == SGX_SUCCESS); - unsigned char tmpBuffer[32]; - CHECK_STATE(sgx_sha256_get_hash(shaStateHandle, (sgx_sha256_hash_t *)tmpBuffer) == SGX_SUCCESS); - CHECK_STATE(sgx_sha256_close(shaStateHandle) == SGX_SUCCESS); - - memcpy(_randBuff, tmpBuffer, _size); + sgx_status_t status = sgx_read_rand(_randBuff, _size); + if (status != SGX_SUCCESS) { + LOG_ERROR("sgx_read_rand failed in get_global_random. Aborting enclave."); + abort(); + } } static void sealHexSEK(int *errStatus, char *errString, diff --git a/tests/integration/keys/ECDSAIntegrationTests.cpp b/tests/integration/keys/ECDSAIntegrationTests.cpp index 16e5ad7c..90ebe7c0 100644 --- a/tests/integration/keys/ECDSAIntegrationTests.cpp +++ b/tests/integration/keys/ECDSAIntegrationTests.cpp @@ -26,18 +26,55 @@ #include "zmq_src/ZMQClient.h" #include +#include +#include #include +#include #include #include #include #include +#include #include #include +#include #include using namespace jsonrpc; using namespace std; +namespace { + +struct EcdsaKeygenResult { + sgx_status_t status = SGX_ERROR_UNEXPECTED; + int errStatus = -1; + string pubKeyX; + string pubKeyY; +}; + +EcdsaKeygenResult generateEcdsaKeyOnceForRaceCheck() { + vector errMsg(BUF_LEN, 0); + int errStatus = 0; + vector encrPrivKey(BUF_LEN, 0); + vector pubKeyX(BUF_LEN, 0); + vector pubKeyY(BUF_LEN, 0); + uint64_t encLen = 0; + int exportable = 0; + + sgx_status_t status = trustedGenerateEcdsaKey( + eid, &errStatus, errMsg.data(), &exportable, encrPrivKey.data(), &encLen, + pubKeyX.data(), pubKeyY.data()); + + EcdsaKeygenResult result; + result.status = status; + result.errStatus = errStatus; + result.pubKeyX = string(pubKeyX.data()); + result.pubKeyY = string(pubKeyY.data()); + return result; +} + +} // namespace + class TestFixtureZMQSign { public: TestFixtureZMQSign() { @@ -231,3 +268,52 @@ TEST_CASE_METHOD(TestFixtureZMQSign, "ZMQ-ecdsa", std::for_each(workers.begin(), workers.end(), [](std::thread &t) { t.join(); }); } + +TEST_CASE_METHOD(TestFixture, "ECDSA global_random concurrency distinctness", + "[integration][ecdsa][security]") { + constexpr int kThreads = 8; + // A single burst of threads may not overlap on the RNG on any given run, so + // repeat the burst several times to raise the odds of real contention. The + // distinctness invariant holds across every attempt, not just within one. + constexpr int kAttempts = 15; + + // Accumulates every public key produced across all attempts. + set> seenPubKeys; + + for (int attempt = 0; attempt < kAttempts; ++attempt) { + INFO("attempt " << attempt); + + atomic start{false}; + + auto worker = [&start]() { + while (!start.load(memory_order_acquire)) { + this_thread::yield(); + } + return generateEcdsaKeyOnceForRaceCheck(); + }; + + vector> futures; + futures.reserve(kThreads); + for (int i = 0; i < kThreads; ++i) { + futures.push_back(async(launch::async, worker)); + } + // Release all threads at once to maximize simultaneous pressure on the RNG. + start.store(true, memory_order_release); + + for (auto &f : futures) { + REQUIRE(f.wait_for(chrono::seconds(10)) == future_status::ready); + + EcdsaKeygenResult r = f.get(); + REQUIRE(r.status == SGX_SUCCESS); + REQUIRE(r.errStatus == SGX_SUCCESS); + REQUIRE_FALSE(r.pubKeyX.empty()); + REQUIRE_FALSE(r.pubKeyY.empty()); + + // Fail if this exact key was already produced by any other thread, in + // this attempt or an earlier one — that is a nonce/key collision. + auto pubKey = make_pair(r.pubKeyX, r.pubKeyY); + REQUIRE(seenPubKeys.count(pubKey) == 0); + seenPubKeys.insert(pubKey); + } + } +}