[Feature] Harden ECKey and signature validation

[Federico2014]

Summary

Add strict input validation across ECKey, Rsv, SignUtils, and WitnessInitializer to reject invalid or malformed key material at API boundaries, and ensure private key material is properly zeroed after use.

Problem

Background

ECKey (SECP256K1) is the sole cryptographic engine used in TRON production. All transaction signing, block witness signatures, and PBFT consensus messages rely on ECKey. The associated helper classes (Rsv, SignUtils) parse and validate signature bytes at multiple layers.

Note: These changes are purely defensive input validation hardening. They do not alter any cryptographic algorithm, consensus rule, or state transition logic. See the Consensus Safety section for a detailed call-site analysis.

Motivation

The crypto module lacks defensive validation at library boundaries. Invalid key material is silently accepted or produces cryptic downstream failures rather than failing fast with clear error messages.

Current State

• ECKey.fromPrivate(null) returns null silently instead of throwing
• ECKey.fromPublicOnly() and the constructor accept infinity points and off-curve points without validation
• ECKey.fromPrivateAndPrecalculatedPublic() exists as an unused factory method
• Rsv.fromSignature() does not validate null or short signature byte arrays — since signatures come from protobuf deserialization of network data, a malicious peer could send a short or empty signature, causing a raw ArrayIndexOutOfBoundsException instead of a clear error
• SignUtils does not validate key material before signing
• WitnessInitializer does not zero key material in error paths, risking private key leakage in heap dumps
• No isValidPrivateKey() utility exists for pre-validation

Limitations or Risks

• Silent null returns propagate invalid state deeper into the call stack
• Accepting invalid EC points (infinity, off-curve) undermines signature verification integrity
• Missing signature byte validation in Rsv can cause ArrayIndexOutOfBoundsException at runtime
• Private key material lingering in memory after errors increases exposure in heap dump scenarios

Proposed Solution

Proposed Design

1. Add null, empty, and length checks to ECKey.fromPrivate(), fromPublicOnly(), publicKeyFromPrivate(), signatureToKeyBytes() — throw IllegalArgumentException on invalid input
2. Add EC point validation (isInfinity(), isValid()) in the ECKey constructor and fromPublicOnly()
3. Remove unused fromPrivateAndPrecalculatedPublic() from both ECKey and SM2
4. Add isValidPrivateKey(byte[]) and isValidPrivateKey(BigInteger) to both ECKey and SM2
5. Add null and length validation in Rsv.fromSignature() (reject null or < 65 bytes)
6. Enforce key validity in SignUtils before signing operations
7. Harden WitnessInitializer to validate keys on startup and zero key material in a finally block
8. Add class-level Javadoc on ECKey documenting signature malleability and BIP-62

Key Changes

• Module: crypto — ECKey.java, Rsv.java, SignUtils.java, SM2.java (minimal isValidPrivateKey addition)
• Module: framework — WitnessInitializer.java
• Tests: ECKeyTest, WitnessInitializerTest

Impact

• Security: Prevents silent acceptance of invalid key material; ensures private keys are zeroed after use
• Stability: Fail-fast validation produces clear error messages instead of cryptic downstream failures
• Developer Experience: Consistent validation behavior across ECKey and SM2

Consensus Safety

These changes do not affect consensus. All modifications are defensive input validation at API boundaries — they reject invalid inputs earlier with clear error messages, without changing the behavior for any valid input.

The most sensitive change is Rsv.fromSignature(), which now throws IllegalArgumentException for null or short (< 65 bytes) signature arrays. A call-site analysis of all callers in the consensus path confirms no behavioral change:

Caller	Pre-check?	Exception handling	Impact
TransactionCapsule.validateSignature()	Yes — sig.size() < 65 before calling	try-catch → SignatureFormatException	New check is redundant, no change
BlockCapsule.validateSignature()	No length pre-check	try-catch → ValidateSignatureException	Previously ArrayIndexOutOfBoundsException, now IllegalArgumentException — both caught, block rejected either way
PrecompiledContracts.recoverAddrBySign()	Yes — isEmpty || length < 65	try-catch(Throwable)	New check is redundant, no change
PbftBaseMessage.analyzeSignature()	No length pre-check	try-catch → SignatureException	Exception type changes, message rejected either way
RelayService	No length pre-check	try-catch in handler	Exception type changes, message rejected either way
PbftDataSyncHandler	No length pre-check	try-catch in handler	Exception type changes, message rejected either way
TransactionResult (JSON-RPC)	N/A	Not in consensus path	Read-only API formatting

Conclusion: For all callers without a pre-check, the new validation converts an opaque ArrayIndexOutOfBoundsException into an explicit IllegalArgumentException. Both are caught by existing exception handlers. No valid signature that previously passed is now rejected, and no consensus outcome changes.

Compatibility

• Breaking Change: Yes
• Default Behavior Change: ECKey.fromPrivate(byte[]) now throws IllegalArgumentException on null/empty input instead of returning null; fromPublicOnly() and constructor reject invalid EC points
• Migration Required: Callers that relied on null return for invalid input must add pre-checks or catch exceptions; callers of fromPrivateAndPrecalculatedPublic() must switch to fromPrivate()

Additional Notes

• Do you have ideas regarding implementation? Yes
• Are you willing to implement this feature? Yes