Skip to content

[Feature] Enhance SM2 security and simplify signing model #18

@Federico2014

Description

@Federico2014

Summary

Harden the SM2 cryptographic implementation: replace non-deterministic nonce generation with RFC 6979-equivalent deterministic nonces, add private key range validation and signature component range checks, simplify SM2 to a pure hash-signing model consistent with ECDSA usage on TRON, and remove all dead code paths.

Problem

Background

The community previously discussed removing SM2/SM3 entirely (see tronprotocol/java-tron#6588). The conclusion was that full removal is not worthwhile — SM2 was designed as a crypto extension point and the isECKeyCryptoEngine flag has no runtime impact, so the 75-file cleanup cost outweighs the benefit (see review feedback).

However, keeping SM2 as-is is not ideal either — the current implementation has real security risks (non-deterministic nonces) and accumulated dead code that creates a false sense of standards compliance. This proposal takes the pragmatic middle ground: harden and simplify the SM2 code in place, without touching the crypto engine abstraction layer.

Note: SM2 has never been enabled on the TRON network — all production nodes use ECKey (SECP256K1) exclusively. The isECKeyCryptoEngine flag is always true at runtime. Therefore, modifications to SM2 code have zero impact on the consensus mechanism and do not affect any on-chain transaction processing or block validation.

Motivation

The SM2 implementation contains security risks from non-deterministic nonce generation and accumulated dead code that creates a false sense of standards compliance.

Current State

  • SM2 signing uses RandomDSAKCalculator, which generates a random nonce k for each signature. If the RNG is weak or the same nonce is reused, the private key can be recovered
  • No private key range validation: SM2(byte[], boolean), fromPrivate(), fromPrivate(byte[]) accept arbitrary byte arrays without checking that the key is in the valid range [1, n). An out-of-range key (e.g. zero, negative, or >= curve order) leads to undefined behavior in scalar multiplication
  • No signature component range validation: recoverPubBytesFromSignature() only checks r >= 0 and s >= 0, but does not verify that r and s are strictly in (0, n). A zero or out-of-range r/s can cause incorrect recovery or exceptions in downstream math
  • Missing null/empty guards: Multiple public entry points (sign(), verify(), signatureToKeyBytes(), fromNodeId()) lack null checks, leading to raw NullPointerException instead of meaningful error messages
  • The userID code path accepts identity parameters via ParametersWithID in init() but silently ignores them — the addUserID call in getZ() is commented out
  • Message-level signing APIs (generateSignature, verifySignature, generateSM3Hash) compute SM3(Z || M) internally, which is inconsistent with the blockchain model where transactions are signed by hash
  • Dead methods exist: hash(), signMessage(), signMsg(), fromPrivateAndPrecalculatedPublic()
  • getSM2SignerForHash() naming does not reflect its actual purpose (verification)
  • SM2Signer.init() takes a forSigning boolean parameter that is redundant

Limitations or Risks

  • Nonce reuse vulnerability: The random K path is vulnerable to nonce reuse — two signatures with the same nonce leak the private key
  • Private key range bypass: Without [1, n) validation, invalid keys can be loaded and used for signing, producing invalid or exploitable signatures
  • Signature malleability: Without strict (0, n) range checks on r and s, crafted signatures could bypass verification or cause incorrect public key recovery
  • False compliance: The userID / ParametersWithID code suggests GB/T 32918 compliance but silently discards the identity parameter
  • API confusion: Message-level and hash-level signing APIs coexist, creating ambiguity about which path is used in production
  • Maintenance burden: Dead code paths increase review surface and risk of accidental misuse

Proposed Solution

Proposed Design

  1. Replace RandomDSAKCalculator with HMacDSAKCalculator(SM3Digest) for deterministic nonce generation — same (key, hash) pair always produces the same signature
  2. Add isValidPrivateKey(byte[]) and isValidPrivateKey(BigInteger) validation methods, enforce [1, n) range check on all key construction paths: SM2(byte[], boolean), fromPrivate(BigInteger), fromPrivate(byte[]), privateKeyFromBigInteger(), publicKeyFromPrivate()
  3. Tighten signature validation in recoverPubBytesFromSignature(): enforce r ∈ (0, n), s ∈ (0, n), recId ∈ [0, 3], messageHash.length == 32
  4. Add null/empty guards on all public entry points: sign(), verify(), signatureToKeyBytes(), fromNodeId(), fromPublicOnly()
  5. Remove the userID chain entirely: userID field, ParametersWithID handling, addUserID(), forSigning parameter from init()
  6. Remove message-level signing path: generateSignature, generateSM3Hash, getZ, addFieldElement, verifySignature, verifyMessage
  7. Remove dead methods: hash(), signMessage(), signMsg(), fromPrivateAndPrecalculatedPublic()
  8. Rename getSM2SignerForHash() to getVerifier()
  9. Add class-level documentation on SM2 and SM2Signer explicitly stating the non-standard design: signatures are not interoperable with standard GB/T 32918 implementations that apply the Z_A pre-hash

Key Changes

  • Module: cryptoSM2.java, SM2Signer.java
  • Tests: SM2KeyTest, PublicMethod.java

Impact

  • Security: Eliminates nonce-reuse risk via deterministic nonces (RFC 6979); enforces private key range [1, n) and signature component range (0, n) to prevent invalid key loading and signature malleability
  • Stability: Fewer code paths, clearer API contract
  • Developer Experience: Unambiguous signing model — only hash-signing is supported, matching ECDSA behavior

Security Analysis of Non-Standard Z_A Omission

The standard SM2 signing flow (GB/T 32918) computes e = SM3(Z_A ‖ M), where Z_A = SM3(ENTL_A ‖ ID_A ‖ a ‖ b ‖ x_G ‖ y_G ‖ x_A ‖ y_A) binds the signer's identity and public key to the message. This implementation omits Z_A and signs the 32-byte transaction hash directly. This does not introduce security risks in the blockchain context:

  1. Identity binding is enforced by the protocol layer: TRON recovers the public key mathematically from the signature, then derives the address from the public key. Even if an attacker constructs a different key pair that produces the same signature, the recovered public key — and therefore the address — will differ, failing the permission check. The Z_A identity binding is redundant in this model.

  2. Key-substitution attacks do not apply: Z_A was designed for traditional PKI scenarios where a verifier holds a public key certificate and an attacker could register a malicious public key. In TRON, address = hash(public key) — there is no external public key registration mechanism, so this attack vector does not exist.

  3. Core signature math is unaffected: The SM2 signature equations (r = (x₁ + e) mod n, s = (1+d)⁻¹ · (k - r·d) mod n) are secure regardless of whether e comes from SM3(Z_A ‖ M) or directly from SHA256(tx). The discrete logarithm security holds as long as e is a 32-byte hash output with 128-bit collision resistance.

  4. Deterministic nonces are stricter than the standard: Standard SM2 uses random k; this implementation uses HMacDSAKCalculator (RFC 6979 equivalent), which is strictly more secure — it eliminates nonce-reuse risk entirely.

  5. Non-interoperability is the only trade-off: Signatures produced by this implementation cannot be verified by standard GB/T 32918 verifiers, and vice versa. Since SM2 has never been enabled on the TRON network, this is acceptable and is explicitly documented in the code.

Compatibility

  • Breaking Change: Yes
  • Default Behavior Change: SM2 signatures are now deterministic (same key+hash = same signature); previously each signing produced a different result
  • Migration Required:
    • SM2Signer.init() — remove the forSigning parameter
    • Removed APIs: hash(), signMessage(), signMsg(), generateSM3Hash(), generateSignature(), verifySignature(), verifyMessage(), fromPrivateAndPrecalculatedPublic()
    • getSM2SignerForHash() renamed to getVerifier()
    • Code testing non-determinism of SM2 signatures must be updated

References (Optional)

Additional Notes

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

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