Skip to content

[Feature] Enhance ECKey validation and key handling #47

Description

@Federico2014

Summary

Enhance ECKey input validation, state consistency, and private-key handling so malformed or inconsistent key material is rejected at API boundaries. The enhancement also hardens keystore decryption and aligns ECKey provider APIs with the Bouncy Castle signing implementation.

Problem

Motivation

ECKey is widely used for transaction signing, witness block signing, and other secp256k1 operations. Its public APIs should validate key material consistently, fail early with clear errors, and avoid retaining or exposing sensitive mutable data.

Current State

  • Private-key byte arrays are converted to BigInteger without first enforcing a bounded secp256k1 encoding.
  • Private scalar validation is not consistently applied across constructors and factory methods.
  • Public-key inputs are not consistently checked for canonical encoding, curve membership, point validity, and infinity.
  • A private key can be paired with an unrelated public point, creating an inconsistent ECKey instance.
  • Cached address and node ID arrays are returned directly and can be modified by callers.
  • Constructors accept an arbitrary Provider, while signing only supports Bouncy Castle private keys.
  • Invalid decrypted keys can escape the keystore boundary as IllegalArgumentException, and the raw decrypted private-key buffer is not explicitly cleared.
  • Some APIs allow callers to bypass key-pair validation or render private keys as strings.

Limitations or Risks

  • Oversized or malformed inputs may consume unnecessary CPU and memory before failing.
  • Invalid or inconsistent key state may only surface during later cryptographic operations.
  • Mutable cached arrays allow callers to alter values retained by an ECKey instance.
  • Provider configuration appears more flexible than the actual signing implementation.
  • Decrypted private-key material may remain in memory longer than necessary.

Proposed Solution

Proposed Design

  1. Add bounded private-key byte validation and scalar-range validation for BigInteger values, while accepting the single leading zero sign byte used by BigInteger for valid high-bit scalars.
  2. Validate public-key encoding length and ensure decoded points are valid, non-infinity secp256k1 points.
  3. Verify that private and public keys supplied together belong to the same key pair.
  4. Return defensive copies of cached address and node ID arrays.
  5. Define ECKey equality by public-key identity.
  6. Restrict ECKey key generation and signing keys to the Bouncy Castle provider.
  7. Remove APIs that bypass key-pair validation or expose private keys through string output.
  8. At the Wallet.decrypt() boundary, convert invalid decrypted keys to CipherException and clear the raw decrypted private-key buffer in a finally block on both success and failure paths.

Key Changes

  • Crypto module: strengthen ECKey validation, key-pair consistency, cache encapsulation, provider handling, and equality behavior; normalize invalid keystore-key failures and clear decrypted private-key bytes in Wallet.decrypt().
  • Framework tests: verify invalid keystore keys propagate as WITNESS_KEYSTORE_LOAD without changing WitnessInitializer production logic.
  • Tests: cover private scalar boundaries, bounded and sign-padded encodings, malformed public keys, mismatched key pairs, defensive copies, keystore exception conversion, and decrypted-buffer cleanup.

Impact

  • Security: improves validation of untrusted key material and reduces exposure of sensitive key bytes.
  • Stability: rejects invalid state early with consistent exceptions instead of downstream cryptographic failures.
  • Performance: rejects malformed or oversized private-key input before elliptic-curve operations.
  • Developer Experience: makes ECKey behavior and provider support explicit and predictable.
  • Consensus: no consensus rule, signature verification, signature recovery, or state-transition behavior is changed.

Compatibility

  • Breaking Change: Yes.
  • Default Behavior Change: null, empty, zero, out-of-range, and non-canonical oversized private-key inputs are rejected with IllegalArgumentException; a valid scalar encoded with one leading zero sign byte remains accepted. Invalid public points and mismatched key pairs are also rejected.
  • Migration Required: callers relying on null returns for invalid private-key input must validate input or handle IllegalArgumentException. Callers using arbitrary-provider constructors or removed helper methods must migrate to the supported ECKey factory and constructor APIs. Keystore callers should handle invalid decrypted keys as CipherException.

Additional Notes

  • Do you have ideas regarding implementation? Yes
  • Are you willing to implement this feature? Yes
  • SM2 behavior is outside the scope of this enhancement.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions