Skip to content

Is/1481 improve randomness and ecdsa signatures - #532

Open
PropzSaladaz wants to merge 6 commits into
developfrom
IS/1481-improve-randomness-and-ecdsa-signatures
Open

Is/1481 improve randomness and ecdsa signatures#532
PropzSaladaz wants to merge 6 commits into
developfrom
IS/1481-improve-randomness-and-ecdsa-signatures

Conversation

@PropzSaladaz

Copy link
Copy Markdown
Contributor

Improve enclave randomness and harden ECDSA signing

Summary

Fixes a concurrency flaw in the enclave RNG that could cause ECDSA nonce reuse, corrects the nonce scalar domain, and hardens signing/verification edge cases. All changes are on the nonce-generation and internal-validation paths; signature output remains standard secp256k1 ECDSA.

Changes

RNG — fix concurrency race

  • get_global_random now reads directly from the hardware DRNG (sgx_read_rand) on every call, and fails closed (aborts) on any RNG error.
  • Removes the previous construction — a process-wide SHA256(seed ‖ counter) PRG whose counter used a non-atomic read-modify-write. Under the enclave's multi-TCS threading, two concurrent callers could observe the same counter and derive identical randomness → reused ECDSA nonces.

ECDSA nonce — correct scalar domain

  • Nonce k is now reduced against the group order n (was the field prime p) using rejection sampling to a uniform value in [1, n-1], eliminating modulo bias and the zero-nonce case.

ECDSA signing — robustness

  • Retry on a non-invertible k (mpz_invert return now checked) and on s == 0, alongside the existing r == 0 retry.

ECDSA verification — stricter input validation

  • Fixed the r/s range check in signature_verify (&&||), which was previously a no-op, so out-of-range values are now correctly rejected.

Tests

  • Added a concurrency distinctness test: repeated bursts of parallel key generation must never produce a colliding public key.

Backward compatibility

  • No impact on existing signatures. The nonce is never an input to verification, so previously produced signatures remain valid and all output stays standard ECDSA. The one verification-side change only tightens rejection of already-invalid (out-of-range) signatures and affects the enclave's internal verifier, not external validators.

@PropzSaladaz PropzSaladaz self-assigned this Jul 20, 2026
@PropzSaladaz PropzSaladaz linked an issue Jul 20, 2026 that may be closed by this pull request
Base automatically changed from techdebt/refactor-tests to develop July 24, 2026 14:34
@PropzSaladaz
PropzSaladaz marked this pull request as ready for review July 24, 2026 14:35
Copilot AI review requested due to automatic review settings July 24, 2026 14:35

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR hardens SGX enclave ECDSA signing by fixing the enclave RNG concurrency design, correcting ECDSA nonce generation to the proper scalar domain, and tightening signing/verification edge-case handling to reduce the risk of nonce reuse and invalid-signature acceptance.

Changes:

  • Replace the previous global seed+counter SHA256 PRG with direct sgx_read_rand usage (plus init-time RNG self-test) to avoid concurrency-driven repetition.
  • Update ECDSA signing to generate nonce k uniformly in [1, n-1] via rejection sampling, and add retries for non-invertible k and s == 0.
  • Fix ECDSA verification range checking (r/s) and add an integration test that stresses concurrent key generation distinctness.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File Description
tests/integration/keys/ECDSAIntegrationTests.cpp Adds a parallel keygen “distinctness under contention” integration test.
secure_enclave/Signature.c Hardens ECDSA nonce selection and signing retry logic; fixes verifier range check logic.
secure_enclave/secure_enclave.c Switches RNG to direct sgx_read_rand, adds RNG self-test at init, removes old global PRG state.
secure_enclave/EnclaveCommon.h Removes the exported globalRandom declaration consistent with RNG refactor.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread secure_enclave/secure_enclave.c Outdated
Comment thread secure_enclave/secure_enclave.c Outdated
@PropzSaladaz
PropzSaladaz force-pushed the IS/1481-improve-randomness-and-ecdsa-signatures branch from 869ac0d to 562ede5 Compare July 24, 2026 15:27
@PropzSaladaz
PropzSaladaz requested a review from a team as a code owner August 5, 2026 12:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Improve random generator & ECDSA signatures

2 participants